spring-projects / spring-authorization-server

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

Empty auth_code parameter results in 500 on /token #1671

Closed aijazkeerio closed 1 month ago

aijazkeerio commented 1 month ago

Describe the bug Empty auth_code parameter results in 500 on /token This is caused by https://github.com/spring-projects/spring-authorization-server/blob/main/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/CodeVerifierAuthenticator.java#L141

    private static boolean authorizationCodeGrant(Map<String, Object> parameters) {
        // @formatter:off
        return AuthorizationGrantType.AUTHORIZATION_CODE.getValue().equals(
                parameters.get(OAuth2ParameterNames.GRANT_TYPE)) &&
                parameters.get(OAuth2ParameterNames.CODE) != null;
        // @formatter:on
    }

returns true even though code has no value.

To Reproduce Create a request for /token and add param code with no value/string

Expected behavior Empty auth_code parameter results in 400 on /token

Sample At the point of https://github.com/spring-projects/spring-authorization-server/blob/main/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/CodeVerifierAuthenticator.java#L141 it should check if param has some value for example

private static boolean authorizationCodeGrant(Map<String, Object> parameters) {
        // @formatter:off
        return AuthorizationGrantType.AUTHORIZATION_CODE.getValue().equals(
                parameters.get(OAuth2ParameterNames.GRANT_TYPE)) &&
                parameters.get(OAuth2ParameterNames.CODE) != null &&
                !parameters.get(OAuth2ParameterNames.CODE).toString().isBlank();
        // @formatter:on
    }
jgrandja commented 1 month ago

Thanks for reporting this @aijazkeerio. I've confirmed this is a bug.

Would you be interested in submitting a fix for this?

aijazkeerio commented 1 month ago

Thanks for reporting this @aijazkeerio. I've confirmed this is a bug.

Would you be interested in submitting a fix for this?

Thanks @jgrandja Yes I am. I will create a draft PR soon.

jgrandja commented 1 month ago

Closing in favour of gh-1680