Google OIDC IdP returns iss parameter without declaring authorization_response_iss_parameter_supported in metadata

[amummaprojectmanager]

Personal contribution

• I'm not interested in submitting a pull request and understand that this provider may not be fixed in the near future without my contribution.

Version

n/a

Provider name

Google

Describe the bug

This is just a heads up in the event we get issues logged here. Google has recently started returning responses which do not follow the RFC.

https://issuetracker.google.com/issues/479882107

To reproduce

It depends on how google decides to respond to your identity request.

Exceptions (if any)

[amummaprojectmanager]

Unless the RFC and specs are carefully read, a user could mistakenly think OpenIddict is at fault for this issue. Hence the reason for logging this so it's visible somewhere.

[tspence]

For additional clarity: The problem as users experience it is as follows:

• User clicks on the "Sign in with Google" button.
• User is redirected to Google.
• User goes through the Google process, authenticates, and selects an account.
• When the user is redirected back to our OpenIDDict instance, they receive this error message:

Unfortunately, this problem is not 100% consistent by domain name, but it is 100% consistent by user. My company has at least two users who get this error reliably, but other users succeed reliably. Our best guess is that Google is doing a phased rollout of this change and it's not clear under what conditions a user would get this error.

Is this something we can correct with a configuration setting? We tried upgrading to the latest published version of OpenIdDict but that did not change the results.

[kevinchalet]

Thanks for reporting this (interesting) issue, @amummaprojectmanager!

Unless the RFC and specs are carefully read, a user could mistakenly think OpenIddict is at fault for this issue.

👍🏻

OpenIddict indeed implements the behavior recommended by the specification:

Note

Clients SHOULD discard authorization responses with the iss parameter from authorization servers which do not indicate their support for the parameter.

That said, returning an iss parameter while not returning authorization_response_iss_parameter_supported is actually not a total violation of the specification, unlike what the bug report on the Google tracker says:

Note

However, there might be legitimate authorization servers that provide the iss parameter without indicating their support in their metadata. Local policy or configuration can determine whether to accept such responses and specific guidance is out of scope for this specification.

Of course, doing that per-user is a terrible idea given the specification recommends discarding requests containing an iss when authorization_response_iss_parameter_supported is not true (hence the bug you're seeing 🤣)... and of course, they haven't warned anyone when introducing such a subtle change 🤣

Is this something we can correct with a configuration setting?

@tspence I'll keep an eye on the link shared by @amummaprojectmanager and see if we need to introduce a built-in "disable iss parameter validation" flag on our side in the next few days/weeks.

In the meantime, you can work around this issue using OpenIddict's events model (which doesn't require releasing a new version). For that, you need to:

1. Remove the built-in event handler responsible for validating the iss parameter.
2. Re-register a clone of it that is provider-aware and won't reject problematic requests.

Something like this:

builder.Services.AddOpenIddict()
    .AddClient(options =>
    {
        options.RemoveEventHandler(OpenIddictClientHandlers.ValidateIssuerParameter.Descriptor);

        options.AddEventHandler<OpenIddictClientEvents.ProcessAuthenticationContext>(builder =>
        {
            builder.Import(OpenIddictClientHandlers.ValidateIssuerParameter.Descriptor);
            builder.SetType(OpenIddictClientHandlerType.Custom);

            builder.UseInlineHandler(context =>
            {
                // To help mitigate mix-up attacks, the identity of the issuer can be returned by
                // authorization servers that support it as part of the "iss" parameter, which
                // allows comparing it to the issuer in the state token. Depending on the selected
                // response_type, the same information could be retrieved from the identity token
                // that is expected to contain an "iss" claim containing the issuer identity.
                //
                // This handler eagerly validates the "iss" parameter if the authorization server
                // is known to support it (and automatically rejects the request if it doesn't).
                // Validation based on the identity token is performed later in the pipeline.
                //
                // See https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics-19#section-4.4
                // for more information.
                var issuer = (string?) context.Request[Parameters.Iss];

                if (context.Configuration.AuthorizationResponseIssParameterSupported is true ||
                    (context.Registration.ProviderType is OpenIddictClientWebIntegrationConstants.ProviderTypes.Google &&
                    !string.IsNullOrEmpty(issuer)))
                {
                    // Reject authorization responses that don't contain the "iss" parameter
                    // if the server configuration indicates this parameter should be present.
                    if (string.IsNullOrEmpty(issuer))
                    {
                        context.Reject(
                            error: Errors.InvalidRequest,
                            description: OpenIddictResources.FormatID2029(Parameters.Iss),
                            uri: OpenIddictResources.FormatID8000(OpenIddictResources.ID2029));

                        return ValueTask.CompletedTask;
                    }

                    // If the two values don't match, this may indicate a mix-up attack attempt.
                    if (!Uri.TryCreate(issuer, UriKind.Absolute, out Uri? uri) || uri != context.Registration.Issuer)
                    {
                        context.Reject(
                            error: Errors.InvalidRequest,
                            description: OpenIddictResources.FormatID2119(Parameters.Iss),
                            uri: OpenIddictResources.FormatID8000(OpenIddictResources.ID2119));

                        return ValueTask.CompletedTask;
                    }
                }

                // Reject authorization responses containing an "iss" parameter if the configuration
                // doesn't indicate this parameter is supported, as recommended by the specification.
                // See https://datatracker.ietf.org/doc/html/draft-ietf-oauth-iss-auth-resp-05#section-2.4
                // for more information.
                else if (context.Registration.ProviderType is not OpenIddictClientWebIntegrationConstants.ProviderTypes.Google &&
                    !string.IsNullOrEmpty(issuer))
                {
                    context.Reject(
                        error: Errors.InvalidRequest,
                        description: OpenIddictResources.FormatID2120(Parameters.Iss, Metadata.AuthorizationResponseIssParameterSupported),
                        uri: OpenIddictResources.FormatID8000(OpenIddictResources.ID2120));

                    return ValueTask.CompletedTask;
                }

                return ValueTask.CompletedTask;
            });
        });
    });

[tspence]

@kevinchalet thank you so very much for this prompt and helpful reply! We are testing this change today and will let you know if this solves the issue.

[tspence]

First feedback from QA: Our internal users who were blocked are now able to log in!

Our internal guess is: Maybe Google added a feature flag, but the feature flag only controls the sending of the ISS value and it doesn't control the visibility of the AuthorizationResponseIssParameterSupported flag. Or perhaps when we retrieve metadata, we do it in such a way as to lose context and so Google can't use the feature flag to determine whether to send AuthorizationResponseIssParameterSupported as true. Without internals knowledge I doubt I'll ever know for sure.

Either way, thank you so much for the response and the feedback. Feel free to reach out if you want any additional details.

[kevinchalet]

First feedback from QA: Our internal users who were blocked are now able to log in!

That's great news! 👍🏻

Or perhaps when we retrieve metadata, we do it in such a way as to lose context and so Google can't use the feature flag to determine whether to send AuthorizationResponseIssParameterSupported as true.

Server metadata is always server-wide so there's sadly no "context" a server can use to return per-client/customized server metadata. I'm sure the "phased rollout" theory you mentioned earlier is the right one, but obviously, doing it this way wasn't a great idea. And the fact they still haven't replied to a bug report submitted 25 days ago is a bit... concerning to say the least 😄

Either way, thank you so much for the response and the feedback.

My pleasure! 😃

If you find OpenIddict useful, please consider encouraging your employer to sponsor the project. If you're interested, you can find more information here: https://github.com/sponsors/kevinchalet.

Cheers.

[runxc1]

Just to chime in with others, the change in behavior by google broke the ability to log in with google but the InlineHandler above does work around the issue and at least so far I haven't seen that it breaks any other providers I use (custom + facebook)

[kevinchalet]

@runxc1 do you have a rough idea when it started affecting your users?

[StellaAlexis]

Following this as well and much appreciate the quick workaround Kevin!
We got in the first reports on Sat Feb 20, by estimation it affected 1-2% of our user base. The snippet provided here fixes it safely for us too. (other providers are Discord and Facebook).

[kevinchalet]

Thanks!

@StellaAlexis @runxc1 please add your voice to https://issuetracker.google.com/issues/479882107: if enough people complain, maybe we'll get an "official" statement at some point? 🤣

Speaking of Facebook, @StellaAlexis @runxc1, that would be great if you could send a PR to update the API version currently used by the Facebook provider. I'm not a FB user myself so I sadly can't test it 😅

[runxc1]

@runxc1 do you have a rough idea when it started affecting your users?

Yup it started two days ago.