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.

Sign up for GitHub

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

Jump to bottom

Jmprieur/test cert rotation #2496

Merged
merged 7 commits into from
Oct 3, 2023
Merged

Jmprieur/test cert rotation #2496

merged 7 commits into from
Oct 3, 2023

Conversation

jmprieur
Copy link
Collaborator

@jmprieur jmprieur commented Oct 2, 2023 •
edited
Loading

Improving client certificate rotation

Description

  • Adding a ResetCertificates with an override for an enumeration of CredentialDescription
  • Adding a way for the apps to observe that the certs are selected or unselected: fixes [Feature Request] Provide an API to query the certificate that is currently in use for authentication  #2458
  • Fix an issue in the rotation of client certificates.
  • Adds a test that create a daemon app registration (if not already there). Adds 2 certs (one that expires in 3mins, and one that is valid from in now+2 mins and expires in 10 mins, and add them to the app registration, We wait a few seconds (because the app registration is not immediate)

Then we acquire tokens for 5 mins. After 3 mins, IdWeb switches from the first certificate to the second.

image

jennyf19
jennyf19 approved these changes Oct 2, 2023
View reviewed changes
Copy link
Collaborator

@jennyf19 jennyf19 left a comment

Choose a reason for hiding this comment

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

:shipit:

@jmprieur
- Adding a ResetCertificates with an override for an enumeration of C…
c5586d1 
…redentialDescription

- Adding a way for the apps to observe that the certs are selected or unselected
- Fix an issue in the rotation of client certificates.
- Add an integration test.
@jmprieur
Fixing warnings
3399cfa 
Not running in AzureDevOps
@jmprieur
Adding comments
3fa5b1f 
Adding the certificate observability as experimental
Moved the CertificateRotationTests to TokenAcquirerTests (from Ms.Id.Web.Test.Integration) and use
Microsoft.Identity.Web.GraphServiceClient instead of Microsoft.Identity.Web.MicrosoftGraph (because
we need Graph 5 for the CertificateRotationTests)
jennyf19
jennyf19 approved these changes Oct 3, 2023
View reviewed changes
Copy link
Collaborator

@jennyf19 jennyf19 left a comment

Choose a reason for hiding this comment

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

this is great. thanks @jmprieur
:shipit:

@jmprieur jmprieur merged commit 826ff82 into master Oct 3, 2023
5 checks passed
@jmprieur jmprieur deleted the jmprieur/testCertRotation branch October 12, 2023 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jennyf19 jennyf19 jennyf19 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature Request] Provide an API to query the certificate that is currently in use for authentication
2 participants
@jmprieur @jennyf19