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

Problem using microsoft oauth2 as provider because of dynamic issuer #166

Open
FLX-0x00 opened this issue Nov 29, 2023 · 22 comments
Open

Problem using microsoft oauth2 as provider because of dynamic issuer #166

FLX-0x00 opened this issue Nov 29, 2023 · 22 comments

Comments

@FLX-0x00
Copy link

FLX-0x00 commented Nov 29, 2023

Hey community / maintainers - Want to reach you out because I want to implement Microsoft Entra with OpenID Connect into my Rails App using the gem omniauth_openid_connect with rodauth-omniauth.

I have come up with the following config

    omniauth_provider :openid_connect, {
      name: :microsoft,
      scope: [:openid, :email],
      issuer: 'https://login.microsoftonline.com/{tenantid}/v2.0', # not sure - this is my stucking part
      client_options: {
        host: 'login.microsoftonline.com',
        authorization_endpoint: 'https://login.microsoftonline.com/common/oauth2/v2.0/authorize',
        token_endpoint: 'https://login.microsoftonline.com/common/oauth2/v2.0/token',
        userinfo_endpoint: 'https://graph.microsoft.com/oidc/userinfo',
        jwks_uri: 'https://login.microsoftonline.com/common/discovery/v2.0/keys',
        end_session_endpoint: 'https://login.microsoftonline.com/common/oauth2/v2.0/logout',
        identifier: Rails.application.credentials[:identifier],
        secret: Rails.application.credentials[:oauth_secret],
        redirect_uri: Rails.application.config.microsoft_openid_connect_redirect_uri
      }
    }

The authentication works until the callback phase:

02:24:16 web.1  | E, [2023-11-29T02:24:16.168085 #271321] ERROR -- omniauth: (microsoft) Authentication failure! Invalid ID token: Issuer does not match: OpenIDConnect::ResponseObject::IdToken::InvalidIssuer, Invalid ID token: Issuer does not match

The corresponding code is in
vendor/bundle/ruby/3.2.0/gems/openid_connect-2.2.0/lib/openid_connect/response_object/id_token.rb#L26

After debugging I noticed that the issuer that is expected changes if another account (of another org for example) logs into (this should be the tenantid. The Microsoft documentation https://login.microsoftonline.com/common/v2.0/.well-known/openid-configuration says, that the issuer is determined by the tenantid of the users account. This is where I hopefully overthinked or overlooked something. How can I get a dynamic issuer value to my provider config? Hope anyone can understand the current issue.

Cheers!

@bufferoverflow
Copy link
Member

I never tested with common, only with a specific tenant. There you must use the tenant specific endpoints all over or set discovery: true to avoid the client_options, see https://docs.gitlab.com/ee/integration/azure.html

@FLX-0x00
Copy link
Author

Hey thank you for the fast response. I was not able to get to a successful configuration where discovery works. So you mean that I have to set the issuer to the url with my tenant id? I was reading through the documentation and every documents I found uses the common one as this specifies if users can authenticate with private accounts or not. But I will give it a try.

@FLX-0x00
Copy link
Author

If i set the tenant id in the issuer field and discovery to true, than nobody except permitted users can login. The selected user account does not exist in the "Microsoft Services" client and cannot access the "TENANTID" application in this client. The account must first be added to the client as an external user. Use a different account.

@FLX-0x00
Copy link
Author

Or is this a problem related to the gem openid_connect? Because I have to use common as there is no specific tenant i prefer. Only want to use Microsoft as openid provider to create an account on my end or login.

@FLX-0x00
Copy link
Author

I have seen in the openid_connect gem that there is a way when initializing OpenID to ignore the issuer validation. Maybe this is a way in this specific scenario if this gem here supports this flag. Now I have monkey patched the validation from exact match to regex. Not ideal but do the job without loosing security

@FLX-0x00
Copy link
Author

FLX-0x00 commented Nov 29, 2023

@bufferoverflow
Copy link
Member

@FLX-0x00 I'm using this gem as part of GitLab since about a year with Entra ID and before with another IdP, works smooth. As written above: There you must use the tenant specific endpoints all over or set discovery: true to avoid the client_options, see https://docs.gitlab.com/ee/integration/azure.html

@felixstorm
Copy link

I have a very similar problem - I also need to use the common endpoint (https://login.microsoftonline.com/common/v2.0) since we have users from multiple Azure tenants and I already do use "discovery" => true but I still get the error "Issuer is not a valid url and issuer mismatch".

When looking at the discovery document from Azure, the issuer gets specified as "issuer": "https://login.microsoftonline.com/{tenantid}/v2.0", i.e. it literally contains the term /{tenantid}/. This might be the reason for the "not a valid url" part of the error message. And when looking at the token returned, it contains the full tenant id (https://login.microsoftonline.com/810017af-c1d4-4a3b-9e18-.../v2.0) - this might be the reason for the second part of the error message ("issuer mismatch").

@bufferoverflow I am open for any suggestions, but I honestly do not see a way to use this gem with users from multiple Azure/Entra Id tenants. Or is there anything else I could do or try to make it work?
My config in general seems to be fine as it works without problems if I exchange the common in the issuer with an actual tenant id, but this is not an option since is prevents users from other tenants from logging in.

My config (also GitLab):

gitlab_rails['omniauth_providers'] = [
  {
    "name"  => "azure_oauth2",
    "label" => "Microsoft Entra ID",
    "args" => {
      "name"               => "azure_oauth2",
      "strategy_class"     => "OmniAuth::Strategies::OpenIDConnect",
      "issuer"             => "https://login.microsoftonline.com/common/v2.0",
      "discovery"          => true,
      "scope"              => ["openid", "profile", "email"],
      "pkce"               => true,
      "uid_field"          => "oid",
      "client_options" => {
        "identifier"   => "...",
        "secret"       => "...",
        "redirect_uri" => "https://gitlab.example.com/users/auth/azure_oauth2/callback",
      }
    }
  }
]

In case anybody else has the same problem and sees this issue: I had to resort to use OmniAuth::Strategies::AzureActivedirectoryV2 (https://docs.gitlab.com/ee/integration/azure.html) instead which seems to work fine also for users from multiple tenants - even though I would have very much preferred to use this gem instead.

@FLX-0x00
Copy link
Author

FLX-0x00 commented Jul 3, 2024

@felixstorm Totally forgot to mention this but yes - we have switched to omniauth-azure-activedirectory-v2 gem and it works out of the box.

@bufferoverflow
Copy link
Member

bufferoverflow commented Jul 3, 2024

@felixstorm as I see within https://login.microsoftonline.com/common/v2.0/.well-known/openid-configuration the {tenantid} placeholder is used. So for this case please try with the config mentioned within the issue description or below, but set your tenantid within the issuer as https://login.microsoftonline.com/810017af-c1d4-4a3b-9e18-.../v2.0 and using the tenantid were you registered the app:

omniauth_provider :openid_connect, {
      name: :microsoft,
      scope: [:openid, :email],
      issuer: 'https://login.microsoftonline.com/810017af-c1d4-4a3b-9e18-.../v2.0', # use your tenant id
      client_options: {
        host: 'login.microsoftonline.com',
        authorization_endpoint: 'https://login.microsoftonline.com/common/oauth2/v2.0/authorize',
        token_endpoint: 'https://login.microsoftonline.com/common/oauth2/v2.0/token',
        userinfo_endpoint: 'https://graph.microsoft.com/oidc/userinfo',
        jwks_uri: 'https://login.microsoftonline.com/common/discovery/v2.0/keys',
        end_session_endpoint: 'https://login.microsoftonline.com/common/oauth2/v2.0/logout',
        identifier: Rails.application.credentials[:identifier],
        secret: Rails.application.credentials[:oauth_secret],
        redirect_uri: Rails.application.config.microsoft_openid_connect_redirect_uri
      }
    }

I suspect autodiscovery does not work as the issuer is tenant specific.

@bufferoverflow
Copy link
Member

ok, @felixstorm @FLX-0x00 I just verified and it works smooth. Here is the config I used to verify within my gdk:

    - { name: "openid_connect_aad",
        label: "Microsoft Entra ID",
        args: {
          name: "openid_connect_aad",
          strategy_class: "OmniAuth::Strategies::OpenIDConnect",
          scope: ["openid","profile","email"],
          response_type: "code",
          issuer: "https://login.microsoftonline.com/*****************************************/v2.0",
          client_auth_method: "query",
          client_options: {
            identifier: "*****************************************",
            secret: "*****************************************",
            authorization_endpoint: "https://login.microsoftonline.com/common/oauth2/v2.0/authorize",
            token_endpoint: "https://login.microsoftonline.com/common/oauth2/v2.0/token",
            userinfo_endpoint: "https://graph.microsoft.com/oidc/userinfo",
            jwks_uri: "https://login.microsoftonline.com/common/discovery/v2.0/keys",
            end_session_endpoint: "https://login.microsoftonline.com/common/oauth2/v2.0/logout",
            redirect_uri: "https://example.com/users/auth/openid_connect_aad/callback"
          }
        }
      }

I tested organizations beside of common, works as well. For further details regarding the differences of those, see https://learn.microsoft.com/en-us/entra/identity-platform/v2-protocols-oidc#find-your-apps-openid-configuration-document-uri

@FLX-0x00
Copy link
Author

FLX-0x00 commented Jul 4, 2024

Thanks for clarifying and for your time on this! I can confirm, this configuration works as expected. So maybe a thing for the docs. Happy coding!

@felixstorm
Copy link

Unfortunately, it does still not work for me. It works fine as long as the user that logs in belongs to the same tenant that has been specified as issuer in the config, but as soon as a user from a different tenant logs in I get the error message "Invalid id token: issuer does not match". Looking at the id token returned from Azure this seems to be correct as the issuer now contains the tenant id of the user logging in but not the one that has been specified in issuer in the config (where the application has been registered).

@bufferoverflow, @FLX-0x00 Have both of you also tried to log in with a user from a different tenant successfully?

@bufferoverflow
Copy link
Member

@felixstorm I just had a personal account and one from the tenant to test. Could you maybe share the id_token received for both use-cases? Maybe we must tweak https://github.com/omniauth/omniauth_openid_connect/blob/master/lib/omniauth/strategies/openid_connect.rb#L470 as we do within #179

@bufferoverflow bufferoverflow reopened this Jul 5, 2024
@felixstorm
Copy link

Thanks for reopening! I will try to provide both tokens over the weekend - do you need the full signed token in base64 format or would the JSON content be sufficient?
Background is that in case of just the JSON content I could use one token from a productive user and obfuscate a few values but if you prefer the full and signed token, then I will need to authorize a user from another test tenant first (but this should also be feasible for me to do)…

@bufferoverflow
Copy link
Member

@felixstorm maybe add a pp(id_token) at https://github.com/omniauth/omniauth_openid_connect/blob/master/lib/omniauth/strategies/openid_connect.rb#L473 , and provide aud and iss fields you see there. I guess we need another verification mechanism there for the common provider.

@felixstorm
Copy link

felixstorm commented Jul 6, 2024

@bufferoverflow Here we go - a user from the same tenant that the application has been registered in:

{"aud"=>"2c621092-4e90-495a-ad0f-...",
 "iss"=>
  "https://login.microsoftonline.com/810017af-c1d4-4a3b-9e18-.../v2.0",
 "iat"=>1720283638,
 "nbf"=>1720283638,
 "exp"=>1720287538,
 "email"=>"[email protected]",
 "name"=>"Test User1",
 "nonce"=>"fd37265ba8330710ffe5496f...",
 "oid"=>"438f989f-1683-44b1-8013-...",
 "preferred_username"=>"[email protected]",
 "rh"=>"0.ATEArxcAgdTBO0qeGNaTwSsN8ZIQYiyQTlpJrQ_...",
 "sub"=>"ifCP3eoTxW1nsZrZr8r02bTOaMsO56RFOLZ...",
 "tid"=>"810017af-c1d4-4a3b-9e18-...",
 "uti"=>"AnTFwIzXCkO1EO52v...",
 "ver"=>"2.0"}

And now another user from a different tenant:

{"aud"=>"2c621092-4e90-495a-ad0f-...",
 "iss"=>
  "https://login.microsoftonline.com/a53834b7-42bc-46a3-b004-.../v2.0",
 "iat"=>1720283675,
 "nbf"=>1720283675,
 "exp"=>1720287575,
 "email"=>"[email protected]",
 "name"=>"Felix Storm",
 "nonce"=>"66bb54aeb0caa09e98ed45a4...",
 "oid"=>"e482612b-3716-4ddf-9d41-...",
 "preferred_username"=>"[email protected]",
 "rh"=>"0.AVwAtzQ4pbxCo0awBDaXNcOs-ZIQYiyQTlpJrQ_....",
 "sub"=>"ICdQA-Bf4UxN30G4DLaR9l3uNYyN2t-...",
 "tid"=>"a53834b7-42bc-46a3-b004-...",
 "uti"=>"E56PAb42Uki08shsm...",
 "ver"=>"2.0"}

My config was identical to the one you posted here apart from the callback uri, i.e. the issuer in the config is https://login.microsoftonline.com/810017af-c1d4-4a3b-9e18-.../v2.0 (where the application has been registered).

P.S.: pp(id_token) in verify_id_token! only gave me the base64 encoded token but putting pp(decoded) into decode_id_token did the job.

@bufferoverflow
Copy link
Member

@felixstorm thanks for the details! Could you please set the new option audience introduced with #179 and maybe uncomment https://github.com/omniauth/omniauth_openid_connect/blob/master/lib/omniauth/strategies/openid_connect.rb#L475 to ignore the issuer for a quick test. If that works we can consider making the issuer check optional and maybe accept a list of valid issuers.

@bufferoverflow
Copy link
Member

There is an issuer check: https://github.com/nov/openid_connect/blob/main/lib/openid_connect/response_object/id_token.rb#L26 so either pass the issuer we received to ignore it or add a list of valid issuers as config options should do the trick

@felixstorm
Copy link

There is an issuer check: https://github.com/nov/openid_connect/blob/main/lib/openid_connect/response_object/id_token.rb#L26 so either pass the issuer we received to ignore it or add a list of valid issuers as config options should do the trick

Just commenting out that line made it work (I did not have to update the gem and/or set the audience option). IMHO we would need to have a flag to ignore the issuer completely since a list would not really work if you want to be open to users from any Azure tenant.

@bufferoverflow
Copy link
Member

see also nov/openid_connect#95

@bufferoverflow
Copy link
Member

Just commenting out that line made it work (I did not have to update the gem and/or set the audience option). IMHO we would need to have a flag to ignore the issuer completely since a list would not really work if you want to be open to users from any Azure tenant.

I recommend to set audience so this is verified.

Regarding issuer, I agree we need a way to skip this check. I see two options:

  1. add this option to the openid_connec gem and use it here (solves Issues with Microsoft Entra integration nov/openid_connect#95 as well)
  2. extract the issuer from the id_token and pass it as issuer here: https://github.com/omniauth/omniauth_openid_connect/blob/master/lib/omniauth/strategies/openid_connect.rb#L475

bufferoverflow added a commit to bufferoverflow/openid_connect that referenced this issue Jul 7, 2024
This is especially useful when using Microsoft Entra ID common
endpoint, as the issuer could be from another tenant.

When using this parameter it is recommended to set the audience as this
stays the same even if the issuer is from another tenant.

Related omniauth/omniauth_openid_connect#166
Closes nov#95
bufferoverflow added a commit to bufferoverflow/openid_connect that referenced this issue Jul 7, 2024
This is especially useful when using Microsoft Entra ID common
endpoint, as the issuer could be from another tenant.

When using this parameter it is recommended to set the audience as this
stays the same even if the issuer is from another tenant.

Related omniauth/omniauth_openid_connect#166

Closes nov#95
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