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

Quirk for Dynamic (Tenant) based issuer of Microsoft Entra ID #401

Open
MrYawe opened this issue Nov 14, 2024 · 3 comments
Open

Quirk for Dynamic (Tenant) based issuer of Microsoft Entra ID #401

MrYawe opened this issue Nov 14, 2024 · 3 comments

Comments

@MrYawe
Copy link

MrYawe commented Nov 14, 2024

Description

Using oidcc 3.2.5 and oidcc_plug 0.2.0-beta.1.

I'm trying to set up the OpenID Connect flow for Microsoft using the https://login.microsoftonline.com/common/v2.0 issuer.

Microsoft is known to not be compliant with the spec because the issuer in their OpenID configuration is https://login.microsoftonline.com/{tenantid}/v2.0.

Issue 1: issuer_mismatch when loading configuration

The first issue I encountered was when starting the configuration worker:

10:08:28.852 [error] GenServer Parrot.MicrosoftOpenIdProvider terminating
** (stop) {:configuration_load_failed, {:issuer_mismatch, "https://login.microsoftonline.com/{tenantid}/v2.0"}}
Last message: {:continue, :load_configuration}

But I found the allow_issuer_mismatch quirck to disable this check ✅ .

Issue 2: failed pkce challenge

In my callback I had this error:

{:error,
 {:http_error, 400,
  %{
    "correlation_id" => "1bd0c1fc-15f1-4a2e-bf7f-f5ced1393473",
    "error" => "invalid_grant",
    "error_codes" => [501481],
    "error_description" => "AADSTS501481: The Code_Verifier does not match the code_challenge supplied in the authorization request. Trace ID: 97b61674-2c4d-4ce2-91a1-cc9f06fe6200 Correlation ID: 1bd0c1fc-15f1-4a2e-bf7f-f5ced1393473 Timestamp: 2024-11-14 09:19:56Z",
    "timestamp" => "2024-11-14 09:19:56Z",
    "trace_id" => "97b61674-2c4d-4ce2-91a1-cc9f06fe6200"
  }}}

Since I'm using the Oidcc.Plug.Authorize, the pkce challenge is enabled by default and in the Microsoft provider configuration document_overrides is undefined.
I found another quirck to override to provider configuration and make it works ✅. Here is my ProviderConfigurationWorker configuration at this point:

Supervisor.child_spec(
          {ProviderConfigurationWorker,
           %{
             issuer: "https://login.microsoftonline.com/common/v2.0",
             name: Parrot.MicrosoftOpenIdProvider,
             provider_configuration_opts: %{
               quirks: %{
                 allow_issuer_mismatch: true,
                  document_overrides: %{"code_challenge_methods_supported" => ["S256", "plain"]}
               }
             }
           }},
          id: :microsoft_open_id_provider
        )

Issue 3: iss mismatch in token

This last issue is still unresolved.

{:error,
 {:missing_claim, {"iss", "https://login.microsoftonline.com/{tenantid}/v2.0"},
  %{
    "iss" => "https://login.microsoftonline.com/47271ac9-8ccd-4488-8a9f-59e76664581f/v2.0",
    "tid" => "47271ac9-8ccd-4488-8a9f-59e76664581f",
    ...
  }}}

When validating the token, the iss doesn't match because the {tenantid} in the iss claim is replaced by the actual tenant id value found in the tid claim.

Is there another quick I can use here?

Here is the full oidcc_plug configuration I use:

@base_config [
    provider: Parrot.MicrosoftOpenIdProvider,
    client_id: &__MODULE__.client_id/0,
    client_secret: &__MODULE__.client_secret/0,
    redirect_uri: &__MODULE__.callback_uri/0
  ]

  @authorize_config [
    scopes: ["openid", "profile", "offline_access", "User.Read"],
    url_extension: [{"response_mode", "query"}, {"prompt", "select_account"}]
  ]

  @callback_config [retrieve_userinfo: false]

  plug(
    Authorize,
    @base_config ++ @authorize_config
    when action in [:authorize]
  )

  plug(
    AuthorizationCallback,
    @base_config ++ @callback_config
    when action in [:callback]
  )
@maennchen
Copy link
Member

maennchen commented Nov 14, 2024

@MrYawe

Issue 1 & 3: Use the quirks / document_overrides to override the issuer to a sensible value.

Issue 2: should be solved with #399, just released as v3.2.6.

@MrYawe
Copy link
Author

MrYawe commented Nov 14, 2024

@maennchen Maybe I am missing something but we can only find the actual tenant ID value by looking at the token iss claim and the token is immediately validated right after. Can I override the issuer after fetching the token but before the token validation?

See MicrosoftDocs/azure-docs#95994 (comment)

@maennchen
Copy link
Member

@MrYawe Oh right. The issuer changes based on the users tenant.

This is absolutely not compliant from OpenID in any way. The specification is very clear that the issuer has to be checked against (exact equality) the value from the configuration.

If this was not Microsoft but any other smaller provider, I would now tell you to go hassle the provider to be actually compliant with the specification. Unfortunately, Microsoft is not willing to fix it (can't seem to locate the issue, but it was raised before) but is also too large to ignore.

To implement a generic and safe solution, I would be willing to accept a PR, which allows to pass a Regex instead of a string for the issuer. When the token is validated, the provided iss claim would then be validated against the provided regex.

@maennchen maennchen changed the title Add quirck to skip token iss check for Microsoft? Quirk for Dynamic (Tenant) based issuer of Microsoft Entra ID Nov 14, 2024
@maennchen maennchen reopened this Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants