-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Azure AD v2.0 support #1110
Comments
Here is the full dump of
|
And here is the issued token:
GUID for consumers tenant is used to issue the token when I signup with my personal email. I'll try it with my business account later. |
So looks like adding options disabling issuer check to NewProvider won't allow me to solve the issue... |
Just use the URL with your tenant ID
That URL should work fine. It even implements discovery:
That being said, I'm open to implementing a proper Azure AD 2.0 connector instead of only supporting it through the OpenID Connect connector. Particularly if we could get an implementation that deals with refresh tokens and groups. |
I'll check tomorrow whether this URL works with work account or not (if I'm interpreting the documentation correctly, it shouldn't). And if it does I'll stick to using this URL. |
As I expected, this approach doesn't work with business accounts. If I use If I use So looks like Azure AD 2.0 doesn't really follow OpenID spec and to support it we either need to implement it as a separate connector or add exceptions for it to go-oidc (definitely not good). |
#1131 merged (👏) -- I suppose this can be closed? 🎉 |
EDIT: sorry, filed it here instead of coreos/go-oidc by mistake
Azure AD v2.0 doesn't strictly follow OpenID connect spec and returns malformed issuer in '.well-known/openid-configuration' response:
$ http https://login.microsoftonline.com/common/v2.0/.well-known/openid-configuration | jq .issuer "https://login.microsoftonline.com/{tenantid}/v2.0"
As per the documentation, the following values are accepted as tenant id:
common
- Users with both a personal Microsoft account and a work or school account from Azure Active Directory (Azure AD) can sign in to the application.organizations
- Only users with work or school accounts from Azure AD can sign in to the application.consumers
- Only users with a personal Microsoft account can sign in to the application.8eaef023-2b34-4da1-9baa-8bc8c9d6a490
orcontoso.onmicrosoft.com
- Only users with a work or school account from a specific Azure AD tenant can sign in to the application. Either the friendly domain name of the Azure AD tenant or the tenant's GUID identifier can be used.Obviously, the response from Azure AD doesn't follow the spec and Dex can't work with it.
But I'd very much like to workaround this. I've seen coreos/go-oidc#121 and I'm not sure that that was the correct way to handle it, as the GUID used there is for personal accounts only. We'd like to support both personal and business/school accounts as well.
Will you accept the patch that adds optional options to NewProvider (will not break API) to disable Issuer checking?
The text was updated successfully, but these errors were encountered: