search

coreos / go-oidc

A Go OpenID Connect client.
Apache License 2.0
2k stars 400 forks source link

Figure out a strategy for handling Azure #344

Open ericchiang opened 2 years ago

ericchiang commented 2 years ago

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:

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 1 year ago

Just for info on Azure AD Auth. In the past I had implemented this completely myself. But today I tried this library.

I'm currently only using the lib for token validation and I'm using gin as the http handler.

Here is my implementation as gin middleware

Folder Structure:

.
├── pkg
│   └── auth
│         ├── middleware.go
│         ├── models.go
│         └── token.go
└── main.go

Example Project: https://github.com/fullstack-devops/golang-oauth2-example

corysullivan commented 4 months ago

I'm currently working on implementing this with the "common" tenant scenario, and I'm wondering if there have been any updates regarding workarounds.

Current workarounds implemented in go-oidc are the ability to create custom Provider and RemoteKeySet objects without using discovery:

https://pkg.go.dev/github.com/coreos/go-oidc/v3/oidc#ProviderConfig https://pkg.go.dev/github.com/coreos/go-oidc/v3/oidc#NewRemoteKeySet

Are there any examples of how to implement this workaround?