DuendeSoftware / Support

Support for Duende Software products
20 stars 0 forks source link

Invalid PostLogoutRedirectUri when state is included #1329

Closed LeishaBurford closed 4 weeks ago

LeishaBurford commented 1 month ago

Which version of Duende IdentityServer are you using? version 6

Which version of .NET are you using?: dotnet 6

Failing to redirect to the post_logout_redirect_uri when the uri includes a state query parameter

When identity server receives an end session request with an allowed post_logout_redirect_uri and a state query parameter, the warning "Invalid PostLogoutRedirectUri" is logged and the post_logout_redirect_uri is not included in the validated end session request.

To Reproduce

Signout from an external provider with a post_logout_redirect_uri and state query parameter in the end session request

Expected behavior

The post_logout_redirect_uri should be treated as valid (assuming it is correctly configured as an allowed redirect) and the user should be redirected to the value of the post_logout_redirect_uri

Additional context

It seems the code is checking the entire value of the post_logout_redirect_uri without removing the state query parameter.

i.e. The validator is checking if https://server/sitename/foo?state=1234 is valid where I would like to see it checking if https://server/sitename/foo is valid

The path through the code leading me to this theory:

// in EndSessionRequestValidator.cs
public async Task<EndSessionValidationResult> ValidateAsync(NameValueCollection parameters, ClaimsPrincipal subject)
{
    ...
        var redirectUri = parameters.Get(OidcConstants.EndSessionRequest.PostLogoutRedirectUri);
        if (redirectUri.IsPresent())
        {
            if (await UriValidator.IsPostLogoutRedirectUriValidAsync(redirectUri, validatedRequest.Client)) // <= this here
            {
                validatedRequest.PostLogOutUri = redirectUri;
            }
            else
            {
                Logger.LogWarning("Invalid PostLogoutRedirectUri: {postLogoutRedirectUri}", redirectUri);
            }
        }

        if (validatedRequest.PostLogOutUri != null)
        {
            var state = parameters.Get(OidcConstants.EndSessionRequest.State);
            if (state.IsPresent())
            {
                validatedRequest.State = state;
            }
        }
    ...
}

Sending us to the following two methods:

// in StrictRedirectUriValidator.cs
public virtual Task<bool> IsPostLogoutRedirectUriValidAsync(string requestedUri, Client client)
{
    return Task.FromResult(StringCollectionContainsString(client.PostLogoutRedirectUris, requestedUri));
}

protected bool StringCollectionContainsString(IEnumerable<string> uris, string requestedUri)
{
    if (IEnumerableExtensions.IsNullOrEmpty(uris)) return false;

    return uris.Contains(requestedUri, StringComparer.OrdinalIgnoreCase); 
    // ^ here is where the full value of the `post_logout_redirect_uri` (including the state query parameter) 
    // is checked against all the configured allowed post logout redirect uris
}

Current workaround

I am currently working around this by removing the state query parameter from the post_logout_redirect_uri and sending the state in the protocol message in the OnRedirectToIdentityProviderForSignOut event:

builder.Services.AddOpenIdConnect("oidc", options =>
{
    ...
    options.Events.OnRedirectToIdentityProviderForSignOut = async context =>
    {
        var redirectUri = GetRedirectUri(context);

        context.ProtocolMessage.PostLogoutRedirectUri = RemoveQueryParameters(redirectUri);
        context.ProtocolMessage.State = GetStateParameter(redirectUri);  
    }
    ...
}

private static string GetStateParameter(string redirectUrl)
{
    Uri uri = new Uri(redirectUrl);
    var queryString = uri.Query;
    var queryParameters = QueryHelpers.ParseQuery(queryString);
    return queryParameters["state"];
}

private static string RemoveQueryParameters(string url)
{
    var uri = new Uri(url);
    return uri.GetLeftPart(UriPartial.Path);
} 

Encountered this when configuring a legacy Identity Server 4 to federate with our Duende Identity Server 6

RolandGuijt commented 1 month ago

Can you please share an example of the full URL of the request to the end session endpoint? It should look something like https://idp.example.com/connect/endsession?post_logout_redirect_uri=https%3A%2F%2Fclient.example.com%2Fsignout-callback&state=xyz123

LeishaBurford commented 1 month ago

Of course. Here is the entire request to the end session endpoint

https://localhost:44319/connect/endsession?post_logout_redirect_uri=https%3A%2F%2Flocalhost%3A44389%2Fsignout-callback-oidc%3Fstate%3DCfDJ8Kn1iDLGZ1NJre2El4h3-0wICWf7OhmUlw3ai4QyCqipViq38kz7Hy1zOmXWOHarybxp3xhkqbQAUjiU6jrHYQKzofP07tp1jajJC-zRG8MI3HSoVO-9OGb2ZdBMd8AnhS0SVqX2OEAQLOa6h2rUOr-0S3YkcEypc3zn-x808GzVgxmg0BmXcyvZtm6pkshrqA&id_token_hint=eyJhbGciOiJSUzI1NiIsImtpZCI6IkQyNTlGRDMxNEJGQjgwOERFMEI4NDdGNjIwRDdFMjFGNzc4RkRERDZSUzI1NiIsIng1dCI6IjBsbjlNVXY3Z0kzZ3VFZjJJTmZpSDNlUDNkWSIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJodHRwczovL2hvc3QuZG9ja2VyLmludGVybmFsOjQ0MzE5IiwibmJmIjoxNzIxMjIyMDg2LCJpYXQiOjE3MjEyMjIwODYsImV4cCI6MTcyMTI2NTI4NiwiYXVkIjoibGVnYWN5aWRlbnRpdHkiLCJhbXIiOlsicHdkIl0sIm5vbmNlIjoiNjM4NTY4MTg4ODE4ODQ3NDY5Lk1qZGlOR016Tm1FdFpUVmhaUzAwT1dVMkxXSXdZMll0TjJReVpETTBPVFF6TXpjNU5UVTBNbUUyTkRJdFlXVXpNeTAwT0dObUxUZzBOVGd0TlRObFpUUXdPRFptTWpSbCIsImF0X2hhc2giOiJRNkpvUVBEVlktZW9DaTBia28tYUZ3Iiwic2lkIjoiQzYwQ0Q2RUM4MTM0OEQ5NjI0OUUwRDI4OEU4OEQxQTciLCJzdWIiOiJCcm9jayIsImF1dGhfdGltZSI6MTcyMTIyMjA4NiwiaWRwIjoibG9jYWwiLCJuYW1lIjoiQnJvY2siLCJjdWx0dXJlIjoiZW4ifQ.xLRCrpEsB44l-WRATkUiQoXsNO1Jyn3klWLH7mdKY7M0Gt8ctqFnq9l8T4QTXolP2ikga-HQGBiyh0eBnZBBQASPnt64iLaymvK8v1vR2n-_IQwVqCcofMgei8vaDZ00_hXe6abjnO6lwepUJrzhoxjuq1eveI7OccL333zxHCJBSUDufURSjb-bhu5A4ZKnPt-Rn2VTwpOq-IycQ8VSau_DdrM8C2WQ4FtnblNXY1epDFlQ7wwi_RDh2nc9HrwpDfV2UaOo7b1D7tnSuqrxAa8zHIu-wNbEdeTB2-0jEzk4r3vIZ9WYXk6kIBvocsTBW5tvCjYmNKX_yhyub_bMzg&state=CfDJ8CK3nBJUgsRMqgaQDW8YZXKMpHB-PROX0j7cFJJ2Pa6f_kONCZAmknVpZD_DxwhPYd-3IKuigC-98DrYt-DQ32gREXQndYc2kz0dV6k2lApgKdwHPa5f5kBmUtTQhYHEgUnsCzdDe1NgjVIt-rJk_xovKh5v3RejijWMXMhrJ24Z&x-client-SKU=ID_NETSTANDARD2_0&x-client-ver=5.6.0.0

RolandGuijt commented 1 month ago

This URL has a state query string parameter twice. One of which is URL encoded. It seems you're manipulating the URL directly with the state parameter causing that behavior. When setting the state please use the AuthenticatorProperties object to set the state instead of appending it to the URL manually.

LeishaBurford commented 1 month ago

Thanks! I will try to find where we are setting state incorrectly and fix it :)

RolandGuijt commented 4 weeks ago

Great, good luck.