omniauth / omniauth_openid_connect

MIT License
168 stars 187 forks source link

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

Open FLX-0x00 opened 10 months ago

FLX-0x00 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

maybe this helps @bufferoverflow

https://learn.microsoft.com/en-us/entra/identity-platform/v2-protocols-oidc#find-your-apps-openid-configuration-document-uri

bufferoverflow commented 10 months ago

@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 commented 3 months ago

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 commented 3 months ago

@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 commented 3 months ago

@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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

@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

felixstorm commented 3 months ago

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 commented 3 months ago

@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 commented 3 months ago

@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"=>"test.user1@....onmicrosoft.com",
 "name"=>"Test User1",
 "nonce"=>"fd37265ba8330710ffe5496f...",
 "oid"=>"438f989f-1683-44b1-8013-...",
 "preferred_username"=>"test.user1@....onmicrosoft.com",
 "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"=>"felix.storm@....com",
 "name"=>"Felix Storm",
 "nonce"=>"66bb54aeb0caa09e98ed45a4...",
 "oid"=>"e482612b-3716-4ddf-9d41-...",
 "preferred_username"=>"felix.storm@....com",
 "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 commented 3 months ago

@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 commented 3 months ago

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

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

see also https://github.com/nov/openid_connect/issues/95

bufferoverflow commented 2 months ago

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 https://github.com/nov/openid_connect/issues/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