Bug: Cannot logout of the auth provider

[albertwangnz]

I use AWS Cognito as the auth provider. As required by Cognito, the below screenshot shows the logout GET request format.

In the code, it sends token, token_type_hint, client_id and post_logout_redirect_uri as the parameters, which does not match the Cognito format.

Steps To Reproduce

1. Configure Cognito as the auth provider
2. Login and logout

The current behavior

Can logout from my app. When log out from Cognito, shows error: Required String parameter 'redirect_uri' is not present.

The expected behavior

Can logout from my app and Cognito.

[albertwangnz]

I think the issue can be resolved by https://github.com/soofstad/react-oauth2-pkce/pull/52. However, I am not familiar with Github and npm, so I don't know how to pack my fork and test it on my local.

Hi @soofstad , I see the package.json in the root has package-development as the name and 0.1.0 as the version. And the package.json in the src folder has the correct name and version.

How did you build and pack the project?

[soofstad]

Hi, if you see the develop part of the README. It's describes how you can run the library locally withouth building a packaging. So basically just run yarn/npm startin the root folder.

However, not sure this needs to be solved by "extraLogoutParameters". Variants of this "logout_uri/redirect_uri" is almost standard, and so can be added to all requests. The value is already a part of the config.

I don't think there is any harm in making a request with a few more parameters than what the provider expects.

[anthony-vito]

Taking this a bit further.... OAuth2.0 itself doesn't really have a client side concept for "logout"... However, OpenID Connect does... We could lean on that specification for guidance and eventually be compliant with it. I could also see us allowing a "mapping" of parameter names like "post_logout_redirect_uri" --> "logout_uri" or "redirect_uri" or such for support of non-compliant servers.

The first question is... can AWS Cognito accept an OIDC compliant logout request? ( I suspect it can't )

OpenID Connect specifies these logout parameters.

id_token_hint [RECOMMENDED]..... ID Token previously issued by the OP to the RP passed to the Logout Endpoint as a hint about the End-User's current authenticated session with the Client. This is used as an indication of the identity of the End-User that the RP is requesting be logged out by the OP.

logout_hint [OPTIONAL]..... Hint to the Authorization Server about the End-User that is logging out. The value and meaning of this parameter is left up to the OP's discretion. For instance, the value might contain an email address, phone number, username, or session identifier pertaining to the RP's session with the OP for the End-User.

client_id [OPTIONAL]...... OAuth 2.0 Client Identifier valid at the Authorization Server. When both client_id and id_token_hint are present, the OP MUST verify that the Client Identifier matches the one used when issuing the ID Token. The most common use case for this parameter is to specify the Client Identifier when post_logout_redirect_uri is used but id_token_hint is not. Another use is for symmetrically encrypted ID Tokens used as id_token_hint values that require the Client Identifier to be specified by other means, so that the ID Tokens can be decrypted by the OP.

post_logout_redirect_uri [OPTIONAL]....... URI to which the RP is requesting that the End-User's User Agent be redirected after a logout has been performed. This URI SHOULD use the https scheme and MAY contain port, path, and query parameter components; however, it MAY use the http scheme, provided that the Client Type is confidential, as defined in Section 2.1 of OAuth 2.0 [RFC6749], and provided the OP allows the use of http RP URIs. The URI MAY use an alternate scheme, such as one that is intended to identify a callback into a native application. The value MUST have been previously registered with the OP, either using the post_logout_redirect_uris Registration parameter or via another mechanism. An id_token_hint is also RECOMMENDED when this parameter is included.

state [OPTIONAL]...... Opaque value used by the RP to maintain state between the logout request and the callback to the endpoint specified by the post_logout_redirect_uri parameter. If included in the logout request, the OP passes this value back to the RP using the state parameter when redirecting the User Agent back to the RP.

ui_locales [OPTIONAL]...... End-User's preferred languages and scripts for the user interface, represented as a space-separated list of BCP47 [RFC5646] language tag values, ordered by preference. For instance, the value "fr-CA fr en" represents a preference for French as spoken in Canada, then French (without a region designation), followed by English (without a region designation). An error SHOULD NOT result if some or all of the requested locales are not supported by the OpenID Provider.

[anthony-vito]

It looks like currently the library implements OAuth 2.0 Token Revocation https://www.rfc-editor.org/rfc/rfc7009

Yeah... I think we implement OIDC and then put a switch in it.... logoutProtocol = OAuth2 | OIDC

Beyond that we could try to support one offs that don't follow the spec like AWS Cognito.....

[albertwangnz]

It looks like currently the library implements OAuth 2.0 Token Revocation https://www.rfc-editor.org/rfc/rfc7009

Yeah... I think we implement OIDC and then put a switch in it.... logoutProtocol = OAuth2 | OIDC

Beyond that we could try to support one offs that don't follow the spec like AWS Cognito.....

I totally agree with your points. It will be great if we can support auth providers like AWS Cognito. Thanks.

[soofstad]

Agreed that we should also support the OIDC spec for logout/token revocation. However I would like to reduce the amount of config parameters exposed to the library users. The list is already quite long, and it can be hard to find all the correct options.

I suggest we always use all parameters, both for RFC7009, OIDC, and some of the most used "aliases" for post_logout_redirect_uri.

I will try and create a PR for this during the holiday, if noone else would like to give it a shot :slightly_smiling_face:

[anthony-vito]

I like the idea of using all the parameters for both protocols by default. However I would suggest adding one optional configuration of 'logoutProtocol' to narrow the set of logout parameters if the user sets it. That configuration value can also be used to support specific non-standard protocols ( cough cognito ) without blowing up the number of parameters provided in the logout request itself.

If you get up a PR I'll make time to test it. If you skip the "logoutProtocol" bit for now to save time that can be layered on in a later PR.

[soofstad]

This should now be resolved with the much appreciated PR from @albertwangnz (#52), and #53.

I tested this okey, but if you would like to try it out as well @anthony-vito, that would be great. I'll leave the PR a few days before merging anyhow.