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

Figure out a strategy for handling Azure #344

Open
ericchiang opened this issue Jul 1, 2022 · 4 comments
Open

Figure out a strategy for handling Azure #344

ericchiang opened this issue Jul 1, 2022 · 4 comments

Comments

@ericchiang
Copy link
Collaborator

ericchiang commented Jul 1, 2022

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:

  • Azure takes and returns a dynamic "issuer" value in its metadata. Sometimes it has a placeholder "{tenantid}" value, sometimes it doesn't.
  • Azure requires a URL query parameter as part of discovery ("?p={policyName}").
  • I've also heard that the returned ID token "iss" can sometimes be global and other times tenant specific.

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:

  • What is the correct issuer value to expect in an ID Token issued by Azure AD? Are these dependent on tenant or policy?
  • What is the correct discovery URL to use for Azure AD? Are these dependent on tenant or policy?
  • What are the URL parameters consumed by Azure AD? What are the correct values for these?

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:

The issuer value returned MUST be identical to the Issuer URL that was directly used to retrieve the configuration information.

https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationValidation

The returned Issuer location MUST be a URI RFC 3986 [RFC3986] with a scheme component that MUST be https, a host component, and optionally, port and path components and no query or fragment components

https://openid.net/specs/openid-connect-discovery-1_0.html#IssuerDiscovery

OpenID Providers supporting Discovery MUST make a JSON document available at the path formed by concatenating the string /.well-known/openid-configuration to the Issuer.

https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfig

@raggi
Copy link
Contributor

raggi commented Jul 5, 2022

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.

@ericchiang
Copy link
Collaborator Author

@raggi thanks so much for the details!

https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-protocols-oidc#fetch-the-openid-connect-metadata-document

$ curl -s https://login.microsoftonline.com/common/v2.0/.well-known/openid-configuration | jq .issuer
"https://login.microsoftonline.com/{tenantid}/v2.0"
$ curl -s https://login.microsoftonline.com/consumers/v2.0/.well-known/openid-configuration | jq .issuer
"https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/v2.0"
$ curl -s https://login.microsoftonline.com/organizations/v2.0/.well-known/openid-configuration | jq .issuer
"https://login.microsoftonline.com/{tenantid}/v2.0"

The consumers issuer sounds like its a common one:

https://docs.microsoft.com/en-us/azure/active-directory/develop/id-tokens#payload-claims

The GUID that indicates that the user is a consumer user from a Microsoft account is 9188040d-6c67-4c5b-b112-36a304b66dad.

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?

@raggi
Copy link
Contributor

raggi commented Jul 6, 2022

If you go through the "common" or "organizations" flow, do you have any idea what the correct issuer in the ID Token would be?

yes, for "Personal Accounts" it will be 9188040d-6c67-4c5b-b112-36a304b66dad, for other accounts it will be the users tenant-id, so an arbitrary value that should be stable for the selected user credential. I am lead to believe from some MS docs that one user object can however have some existence in more than one tenant, and by these means I think it is possible for the same user object to authenticate with different tenant id's at different times. In these cases the token subject would be different, and so would the oid. The token subject changes on a per-clientid basis, the oid changes on a per-tenant basis. Furthermore if the user is a "guest user", then the tenant id may not be that of the tenant of which the user is a guest (https://docs.microsoft.com/en-us/azure/active-directory/develop/id-tokens#payload-claims) - that is to say, a strategy to say "bind a tenant id to a particular group of users" and subsequently try to use that as the issuer in application requests would fall over for guest users from other tenants.

https://docs.microsoft.com/en-us/azure/active-directory/develop/access-tokens#validating-tokens sadly does not provide guidance here.

@eksrha

This comment was marked as off-topic.

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

No branches or pull requests

3 participants