Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Extract and show csr details #31

Merged
merged 7 commits into from
Jun 27, 2024
Merged

feat: Extract and show csr details #31

merged 7 commits into from
Jun 27, 2024

Conversation

saltiyazan
Copy link
Contributor

@saltiyazan saltiyazan commented Jun 21, 2024

Description

  • Extract more details from the CSR
  • Show the extracted details
  • Show N/A if a value is missing
  • Minor refactor in the code to improve the decoding of the PEM string

Screenshot 2024-06-24 at 11 51 49

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that validate the behaviour of the software
  • I validated that new and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@saltiyazan saltiyazan changed the title [WIP] feat: Extract and show csr details feat: Extract and show csr details Jun 24, 2024
@saltiyazan saltiyazan marked this pull request as ready for review June 24, 2024 07:55
@saltiyazan saltiyazan requested a review from a team as a code owner June 24, 2024 07:55
Copy link
Contributor

@kayra1 kayra1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mostly LGTM

ui/src/app/certificate_requests/row.tsx Outdated Show resolved Hide resolved
ui/src/app/nav.tsx Outdated Show resolved Hide resolved
ui/src/app/utils.ts Show resolved Hide resolved
Copy link
Contributor

@kayra1 kayra1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tentatively approved

ui/src/app/certificate_requests/row.tsx Outdated Show resolved Hide resolved
ui/src/app/certificate_requests/row.tsx Show resolved Hide resolved
ui/src/app/certificate_requests/row.tsx Outdated Show resolved Hide resolved
@saltiyazan saltiyazan requested a review from gruyaume June 26, 2024 14:32
Copy link
Collaborator

@gruyaume gruyaume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last detail, the rest looks good

ui/src/app/certificate_requests/row.tsx Outdated Show resolved Hide resolved
Copy link
Collaborator

@gruyaume gruyaume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad for not catching this earlier, the following fields should be displayed (at least when their value is not empty):

  • Country
  • State or Province
  • Locality
  • Organization
  • Organizational Unit
  • Common Name
  • Alternative Names
  • Email Address

Those are the fields you get to choose when using OpenSSL. Out of those, we are missing "State or Province" and "Organizational Unit".

ui/src/app/certificate_requests/row.tsx Outdated Show resolved Hide resolved
ui/src/app/certificate_requests/row.tsx Outdated Show resolved Hide resolved
@saltiyazan saltiyazan requested a review from gruyaume June 27, 2024 10:50
@saltiyazan saltiyazan enabled auto-merge (squash) June 27, 2024 12:08
@saltiyazan saltiyazan disabled auto-merge June 27, 2024 12:11
@saltiyazan saltiyazan enabled auto-merge (squash) June 27, 2024 12:12
@saltiyazan saltiyazan merged commit d26cc03 into main Jun 27, 2024
10 checks passed
@saltiyazan saltiyazan deleted the TLSENG-276 branch June 27, 2024 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants