fix(provider): don't pin expected issuer for the Azure consumers endpoint - #2796
Open
breken-ai wants to merge 1 commit into
Open
fix(provider): don't pin expected issuer for the Azure consumers endpoint#2796breken-ai wants to merge 1 commit into
breken-ai wants to merge 1 commit into
Conversation
…oint The documented /consumers tenant URL (personal Microsoft accounts) set ExpectedIssuer to the consumers alias, but Microsoft issues those tokens from the well-known consumers tenant GUID (9188040d-6c67-4c5b-b112-36a304b66dad). common and organizations were already exempted from pinning; consumers was missed, so personal- account logins failed with 'Error getting user profile from external provider'. Exempt it the same way. ParseIDToken still validates signature and audience via discovery, so unpinning is safe. Fixes supabase#2795. Reporter legwork (manifest signInAudience confirmation, ruling out /common) by @iohinted-prog.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2795.
Setting the Azure provider URL to the documented
https://login.microsoftonline.com/consumers(the only correct value for personal Microsoft accounts) madeNewAzureProviderpinExpectedIssuerto the/consumers/v2.0alias - but Microsoft never issues ID tokens from the alias: personal-account tokens carryiss = https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/v2.0(the well-known consumers tenant GUID). The issuer check then rejected every personal-account login with "Error getting user profile from external provider".commonandorganizationswere already exempted from pinning for exactly this reason;consumerswas the missed third alias. This PR adds it to the same condition.Security note: unpinning the alias is safe. A consumers-only app registration only receives tokens minted for that app's client ID, and
ParseIDTokenstill validates the signature and audience through OIDC discovery - the issuer pin was only ever belt-and-braces for the alias case, and it was the wrong value anyway.Credit: @iohinted-prog did the legwork in #2795 - manifest-level
signInAudienceconfirmation and ruling out/commonbefore the issuer comparison fell out. #1549 and #1274 look like the same symptom family; maintainers may want to link them.Verification: new table case is red against pre-patch code (
ExpectedIssuerpinned to consumers/v2.0), green with the fix; the 7 existing cases unaffected; full provider package suite green; go vet clean (go 1.27.0).