-
Couldn't load subscription status.
- Fork 378
Revise MSI v2 token revocation spec to focus on certificate revocation #5498
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
base: main
Are you sure you want to change the base?
Conversation
Updated the document to reflect changes from 'Short-Lived Credential (SLC)' to 'Certificate' terminology and clarified the handling of certificate revocation scenarios.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about telemetry. Maybe start with having those values logged. Let's focus more on server telemetry.
docs/msi_v2/msiv2_revocation_spec.md
Outdated
| participant eSTS | ||
|
|
||
| Application ->> MSAL: 1. Request Access Token | ||
| MSAL ->> IMDS: 2. Request Certificate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CSR metadata request is part of the flow. I think you should add it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still don't see it
| MSAL ->> eSTS: 4. Exchange Certificate for Access Token | ||
| eSTS -->> MSAL: 5. Response (HTTP 200 / error) | ||
|
|
||
| alt Token Revoked / Attestation invalid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
both alts are the same except for 5a and 5b. Do you need to repeat the entire portion of the graph after those sections? Can you just group 5a and 5b together on the graph?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these are two different error conditions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please explain the difference to me. Why aren't the diagrams the same for each condition?
| participant Application | ||
| participant MSAL | ||
| participant IMDS | ||
| participant eSTS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is there a "IMDS/eSTS" section? Why not point towards the already existing "IMDS" and "eSTS" sections?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one is for cert revocation, that MSAL handles internally. The other is for token revocation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't clear on the diagram. Can you use this wording on the diagram?
| | **AADSTS1000610–14 Auto-Remediation (Retry Fails)** | Same initial condition as above. New cert minted, retry still fails deterministically (e.g., repeated same code). | Failure surfaced after retry. (`CredentialOutcome=Retry Failed`) | | ||
| | **Unspecified Credential Issue** | eSTS returns `invalid_client` without codes. MSAL forces new certificate and retries. | Token succeeds or failure surfaced (assert correct `CredentialOutcome`). | | ||
| | **Claims Challenge Path** | Resource 401 with claims; app supplies claims; MSAL re-mints cert (`bypass_cache=true`) and retries with claims. | New token with claims. (`CredentialOutcome=Success`) | | ||
| | **IMDS/IssueCredential Failure Path** | `/issuecredential` call fails (network / service / malformed). | Failure returned; no infinite retry. (`CredentialOutcome=Retry Failed` if after a retry attempt) | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
specify which retry policy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure I follow this comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mention "retry" several times in this spec. I'm asking you: what retry policy are we using for these retries?
Updated revocation scenarios and clarified certificate request process.
| @@ -0,0 +1,196 @@ | |||
| # Certificate Revocation Specification | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this going to be implemented for preview?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, this is not for preview.
|
|
||
| This document outlines the design and implementation details for **certificate and token revocation** in MSI V2 scenarios. | ||
|
|
||
| In the MSI v2 authentication flow, MSAL first obtains a credential (certificate) from IMDS and uses it to request a token from eSTS. In some cases, eSTS may respond with an error indicating that the certificate/attestation is no longer valid. When this occurs, **MSAL must obtain a new certificate** before retrying the token request with eSTS. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MSAL first "makes a CSR metadata request"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will re-review after you'd had a chance to respond to the comments I just left.
Updated the document to reflect changes from 'Short-Lived Credential (SLC)' to 'Certificate' terminology and clarified the handling of certificate revocation scenarios.
Fixes - Spec update
Changes proposed in this request
This pull request updates the documentation for MSI V2 credential revocation, clarifying and expanding the specification to focus on certificate revocation (rather than short-lived credentials) and aligning terminology, flows, error handling, and acceptance tests with current implementation and Azure AD error codes. The changes provide detailed guidance on how MSAL should handle certificate revocation, claims challenges, and telemetry.
Key documentation improvements:
Terminology and Flow Updates:
Error Handling and Remediation:
Claims Challenge Handling:
Acceptance Tests and Telemetry:
MsalMsiCounterto reflect the new tags and expected values for improved diagnostics.Testing
n/a
Performance impact
n/a
Documentation
n/a