spring-projects / spring-authorization-server

Spring Authorization Server
https://spring.io/projects/spring-authorization-server
Apache License 2.0
4.84k stars 1.27k forks source link

Allow logout request to be sent without an id_token_hint #1603

Closed onghwand closed 4 months ago

onghwand commented 5 months ago

Expected Behavior When requesting a logout without an id_token_hint, logging out should be possible.

Current Behavior The current version encounters an error with the OidcLogoutAuthenticationConverter when attempting a logout request without an id_token_hint.

// id_token_hint (REQUIRED)     // RECOMMENDED as per spec
String idTokenHint = parameters.getFirst("id_token_hint");
if (!StringUtils.hasText(idTokenHint) ||
        parameters.get("id_token_hint").size() != 1) {
    throwError(OAuth2ErrorCodes.INVALID_REQUEST, "id_token_hint");
}

Context There is a test case for the RP Initiated Logout Profile regarding requests made without an id_token_hint.

image

In the OpenID Connect RP-Initiated Logout 1.0 document (https://openid.net/specs/openid-connect-rpinitiated-1_0.html ), the id_token_hint parameter is RECOMMENDED, but not mandatory.

image

I want to successfully execute the test case using the Spring Authorization Server library. Is there any particular reason for implementing the code to throw an error when there is no id_token_hint?

How about modifying the code as follows:

if (StringUtils.hasText(idTokenHint) &&
        parameters.get("id_token_hint").size() != 1) {
    throwError(OAuth2ErrorCodes.INVALID_REQUEST, "id_token_hint");
}

If you agree with my suggestion, I would like to contribute to the Spring Authorization Server library.

jgrandja commented 4 months ago

@onghwand

Is there any particular reason for implementing the code to throw an error when there is no id_token_hint?

As per spec, id_token_hint is RECOMMENDED, and the implementation of OidcLogoutAuthenticationProvider requires the ID Token, as it relies on it to perform validation of iss, aud, sid and exp claims, as well as, post_logout_redirect_uri parameter.

Quite honestly, I'm not sure why id_token_hint is not REQUIRED. However, we decided to make it REQUIRED as per RECOMMENDED.

I'm going to close this based on the explanation provided.