-
Notifications
You must be signed in to change notification settings - Fork 403
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
Figure out a strategy for handling Azure #344
Comments
The "common" issuer is something specific: it enables an application to be initially non-specific about the sign-in. If an application wishes to provide a global SSO login entrypoint, it can use "common", and observe the returned issuer for more specificity. Using this method, the application can allow both personal Microsoft accounts, and Azure AD accounts to sign in using the same UI/UX. If the application only wants to support personal Microsoft accounts, then using a discovery document URL containing the public GUID works end-to-end without special cases. This configuration will not allow Azure AD accounts to sign-in however. Microsoft have not made it entirely clear what the bounds of token subjects are, and they provide an additional identifier field that is intended to be globally unique - that has potentially additional privacy implications and is not provided in the default claims. It is not clear, given the documentation, if the token subject is entirely correct for use as a unique identifier for a user when using the "common" issuer entrypoint. Subjects thankfully do not appear to be attacker controllable. Per the first point, Microsoft do provide tenant specific discovery documents. One inefficient but possible strategy for handling the common tenant could be to perform the token flow using the common issuer, and then to validate the returned token against the tenant specific issuer URL returned in the token (i.e. pulling a new tenant specific discovery document). Validating the token twice in this way, and making the extra requests is slow, and may only in practice catch infrastructure failures on the Microsoft side, however it could also be considered more specification correct. Application side, adding an HTTP cache to the discovery document requests may mitigate some of the additional cost (MS seem to serve the documents with a 24 hour expiry currently). If a user signs in to an application using an Azure AD account, and the application is using the common tenant, that user will no longer be able to sign in to the application with another account using the application SSO flow (without client state removals). The UX automates itself, removing all opportunity to pick other accounts. This is not consistent with personal accounts - personal account SSO using the same configuration requires the user go through the username/account picker every time. |
@raggi thanks so much for the details!
The consumers issuer sounds like its a common one: https://docs.microsoft.com/en-us/azure/active-directory/develop/id-tokens#payload-claims
If you go through the "common" or "organizations" flow, do you have any idea what the correct issuer in the ID Token would be? I'm assuming the "9188040d-6c67-4c5b-b112-36a304b66dad" value for personal accounts and whatever your tenant ID is for Azure AD? |
yes, for "Personal Accounts" it will be https://docs.microsoft.com/en-us/azure/active-directory/develop/access-tokens#validating-tokens sadly does not provide guidance here. |
This comment was marked as off-topic.
This comment was marked as off-topic.
I'm currently working on implementing this with the "common" tenant scenario, and I'm wondering if there have been any updates regarding workarounds.
Are there any examples of how to implement this workaround? |
This PR adds JSON tags to allow parsing a ProviderConfig directly from the OpenID Connect JSON metadata document. Since this is the preferred workaround for providers that don't support discovery in a spec-compliant way, such as returning the wrong issuer, or requiring a URL parameter, make this path easier and add an example to the godoc. Updates coreos#445 Updates coreos#444 Updates coreos#439 Updates coreos#442 Updates coreos#344 Fixes coreos#290
This PR adds JSON tags to allow parsing a ProviderConfig directly from the OpenID Connect JSON metadata document. Since this is the preferred workaround for providers that don't support discovery in a spec-compliant way, such as returning the wrong issuer, or requiring a URL parameter, make this path easier and add an example to the godoc. Updates coreos#445 Updates coreos#444 Updates coreos#439 Updates coreos#442 Updates coreos#344 Fixes coreos#290
This PR adds JSON tags to allow parsing a ProviderConfig directly from the OpenID Connect JSON metadata document. Since this is the preferred workaround for providers that don't support discovery in a spec-compliant way, such as returning the wrong issuer, or requiring a URL parameter, make this path easier and add an example to the godoc. Updates coreos#445 Updates coreos#444 Updates coreos#439 Updates coreos#442 Updates coreos#344 Fixes coreos#290
This PR adds JSON tags to allow parsing a ProviderConfig directly from the OpenID Connect JSON metadata document. Since this is the preferred workaround for providers that don't support discovery in a spec-compliant way, such as returning the wrong issuer, or requiring a URL parameter, make this path easier and add an example to the godoc. Updates #445 Updates #444 Updates #439 Updates #442 Updates #344 Fixes #290
It would be good if this package correctly and safely picked the issuer and discovery values for Azure AD. There are nuances about the tentantID and policyName that I'm not super familiar with, and these continue to be reported as issues:
Based off of what I've seen, these can be grouped as:
Upstream issues with Azure:
Current workarounds implemented in go-oidc are the ability to create custom Provider and RemoteKeySet objects without using discovery:
Open questions are:
I've been hesitant to provide concrete advice, since choosing the wrong issuer can be an easy way to end up with a vulnerable application. Maybe it's time to figure this out?
The relevant spec links:
https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationValidation
https://openid.net/specs/openid-connect-discovery-1_0.html#IssuerDiscovery
https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfig
The text was updated successfully, but these errors were encountered: