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

[oidc] error on unsuccessful Discovery URL #402

Merged
merged 1 commit into from Sep 2, 2022
Merged

[oidc] error on unsuccessful Discovery URL #402

merged 1 commit into from Sep 2, 2022

Conversation

joriszwart
Copy link
Contributor

@joriszwart joriszwart commented Apr 19, 2021

The issue

For Discovery URLs that did not generate a successful HTTP status code, no error was returned when instantiating the OpenID Connect provider. This resulted in openidConnect.New () appearing successful on Discovery URLs that did return a valid JSON and a non-success HTTP status code as well.

This makes it difficult to check whether the provider's instantiation was successful or not.

Further analysis

An example of this is the Discovery URL login.microsoftonline.com/unknown-tenant/v2.0/.well-known/openid-configuration (the tenant does not exist). The resulting JSON looks like this:

{
    "error": "invalid_tenant",
    "error_description": "AADSTS90002: Tenant 'unknown-tenant'
not found. This may happen if there are no active subscriptions
for the tenant. Check to make sure you have the correct tenant ID.
Check with your subscription administrator.\r\nTrace ID:
5982dc19-d48d-4d2f-bdbb-956704df6a00\r\nCorrelation ID:
156dbcd3-4546-40b5-b656-350fd7b6453a\r\nTimestamp:
2021-04-19 14:30:03Z",
    "error_codes": [90002],
    "timestamp": "2021-04-19 14:30:03Z",
    "trace_id": "5982dc19-d48d-4d2f-bdbb-956704df6a00",
    "correlation_id": "156dbcd3-4546-40b5-b656-350fd7b6453a",
    "error_uri": "https://login.microsoftonline.com/error?code=90002"
}

As a result AuthEndpoint, TokenEndpoint and Issuer will be empty in OpenIDConfig and errors will occur later during the actual authentication.

The solution

This PR:

  1. Checks the HTTP status code returned by the Discovery URL.
  2. Fixes a minor typing error in the JSON tag for EndSessionEndpoint.

@joriszwart
Copy link
Contributor Author

Could just be related to #107 and maybe other providers too? Didn't investigate it.

@joriszwart
Copy link
Contributor Author

joriszwart commented Sep 1, 2022

Any review comments? Or can this be closed?

@techknowlogick techknowlogick merged commit 3d66163 into markbates:master Sep 2, 2022
@techknowlogick
Copy link
Collaborator

Thanks :) Sorry for missing this.

@joriszwart joriszwart deleted the patch-1 branch September 3, 2022 11:51
@joriszwart
Copy link
Contributor Author

You're welcome and thank you!

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

Successfully merging this pull request may close these issues.

None yet

2 participants