From 52a1e82982b09f8b9609eb3eaaea3bf5e0662fa9 Mon Sep 17 00:00:00 2001 From: smallbun <30397655+leshalv@users.noreply.github.com> Date: Fri, 1 Dec 2023 23:01:52 +0800 Subject: [PATCH 1/5] Improve the error log and enhancing OAuth2Error https://github.com/spring-projects/spring-authorization-server/issues/1240 --- .../OAuth2AuthorizationCodeAuthenticationProvider.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java index 04751dd17..51f9c7b3d 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java @@ -143,11 +143,16 @@ public Authentication authenticate(Authentication authentication) throws Authent this.logger.warn(LogMessage.format("Invalidated authorization code used by registered client '%s'", registeredClient.getId())); } } - throw new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_GRANT); + OAuth2Error error = new OAuth2Error(OAuth2ErrorCodes.INVALID_GRANT,"The authorization code is invalid or has expired.",ERROR_URI); + throw new OAuth2AuthenticationException(error); } if (StringUtils.hasText(authorizationRequest.getRedirectUri()) && !authorizationRequest.getRedirectUri().equals(authorizationCodeAuthentication.getRedirectUri())) { + if (this.logger.isWarnEnabled()) { + this.logger.warn(LogMessage.format("Invalidated redirect_uri used by registered client '%s'", registeredClient.getId())); + } + OAuth2Error error = new OAuth2Error(OAuth2ErrorCodes.INVALID_GRANT,"The redirect_uri does not match the redirection URI used in the authorization request.",ERROR_URI); throw new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_GRANT); } @@ -165,7 +170,8 @@ public Authentication authenticate(Authentication authentication) throws Authent } } } - throw new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_GRANT); + OAuth2Error error = new OAuth2Error(OAuth2ErrorCodes.INVALID_GRANT,"The authorization code is invalid or has expired.",ERROR_URI); + throw new OAuth2AuthenticationException(error); } if (this.logger.isTraceEnabled()) { From 38ab25802b0b744897fc0d8c4bd205e4aac13ff3 Mon Sep 17 00:00:00 2001 From: smallbun <30397655+leshalv@users.noreply.github.com> Date: Fri, 1 Dec 2023 23:16:00 +0800 Subject: [PATCH 2/5] Improvement --- .../OAuth2AuthorizationCodeAuthenticationProvider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java index 51f9c7b3d..45631f2b4 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java @@ -153,7 +153,7 @@ public Authentication authenticate(Authentication authentication) throws Authent this.logger.warn(LogMessage.format("Invalidated redirect_uri used by registered client '%s'", registeredClient.getId())); } OAuth2Error error = new OAuth2Error(OAuth2ErrorCodes.INVALID_GRANT,"The redirect_uri does not match the redirection URI used in the authorization request.",ERROR_URI); - throw new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_GRANT); + throw new OAuth2AuthenticationException(error); } if (!authorizationCode.isActive()) { From 5ffa069fd0d9a662004f2537b622950e03dcc080 Mon Sep 17 00:00:00 2001 From: smallbun <2689170096@qq.com> Date: Fri, 15 Dec 2023 11:27:38 +0800 Subject: [PATCH 3/5] Improve logging in OAuth2AuthorizationCodeAuthenticationProvider --- .../OAuth2AuthorizationCodeAuthenticationProvider.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java index 45631f2b4..146a550e9 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProvider.java @@ -143,8 +143,7 @@ public Authentication authenticate(Authentication authentication) throws Authent this.logger.warn(LogMessage.format("Invalidated authorization code used by registered client '%s'", registeredClient.getId())); } } - OAuth2Error error = new OAuth2Error(OAuth2ErrorCodes.INVALID_GRANT,"The authorization code is invalid or has expired.",ERROR_URI); - throw new OAuth2AuthenticationException(error); + throw new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_GRANT); } if (StringUtils.hasText(authorizationRequest.getRedirectUri()) && @@ -152,8 +151,7 @@ public Authentication authenticate(Authentication authentication) throws Authent if (this.logger.isWarnEnabled()) { this.logger.warn(LogMessage.format("Invalidated redirect_uri used by registered client '%s'", registeredClient.getId())); } - OAuth2Error error = new OAuth2Error(OAuth2ErrorCodes.INVALID_GRANT,"The redirect_uri does not match the redirection URI used in the authorization request.",ERROR_URI); - throw new OAuth2AuthenticationException(error); + throw new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_GRANT); } if (!authorizationCode.isActive()) { @@ -170,8 +168,7 @@ public Authentication authenticate(Authentication authentication) throws Authent } } } - OAuth2Error error = new OAuth2Error(OAuth2ErrorCodes.INVALID_GRANT,"The authorization code is invalid or has expired.",ERROR_URI); - throw new OAuth2AuthenticationException(error); + throw new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_GRANT); } if (this.logger.isTraceEnabled()) { From 2624df8054863c2a58f124748577f63710935b88 Mon Sep 17 00:00:00 2001 From: smallbun <2689170096@qq.com> Date: Fri, 15 Dec 2023 11:45:13 +0800 Subject: [PATCH 4/5] Improve logging --- ...AuthorizationCodeRequestAuthenticationProvider.java | 4 ++++ .../OAuth2ClientCredentialsAuthenticationProvider.java | 4 ++++ ...viceAuthorizationRequestAuthenticationProvider.java | 4 ++++ .../OAuth2RefreshTokenAuthenticationProvider.java | 10 ++++++++++ 4 files changed, 22 insertions(+) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java index aecc5b385..7b8fae6c7 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java @@ -23,6 +23,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.core.log.LogMessage; import org.springframework.security.authentication.AnonymousAuthenticationToken; import org.springframework.security.authentication.AuthenticationProvider; import org.springframework.security.core.Authentication; @@ -120,6 +121,9 @@ public Authentication authenticate(Authentication authentication) throws Authent this.authenticationValidator.accept(authenticationContext); if (!registeredClient.getAuthorizationGrantTypes().contains(AuthorizationGrantType.AUTHORIZATION_CODE)) { + if (this.logger.isTraceEnabled()) { + this.logger.warn(LogMessage.format("Invalidated grant_type used by registered client '%s'", registeredClient.getId())); + } throwError(OAuth2ErrorCodes.UNAUTHORIZED_CLIENT, OAuth2ParameterNames.CLIENT_ID, authorizationCodeRequestAuthentication, registeredClient); } diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2ClientCredentialsAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2ClientCredentialsAuthenticationProvider.java index dc0fcff13..06a0eb4ee 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2ClientCredentialsAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2ClientCredentialsAuthenticationProvider.java @@ -22,6 +22,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.core.log.LogMessage; import org.springframework.security.authentication.AuthenticationProvider; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; @@ -93,6 +94,9 @@ public Authentication authenticate(Authentication authentication) throws Authent } if (!registeredClient.getAuthorizationGrantTypes().contains(AuthorizationGrantType.CLIENT_CREDENTIALS)) { + if (this.logger.isTraceEnabled()) { + this.logger.warn(LogMessage.format("Invalidated grant_type used by registered client '%s'", registeredClient.getId())); + } throw new OAuth2AuthenticationException(OAuth2ErrorCodes.UNAUTHORIZED_CLIENT); } diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProvider.java index d21ed9c41..05326838c 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProvider.java @@ -23,6 +23,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.core.log.LogMessage; import org.springframework.lang.Nullable; import org.springframework.security.authentication.AuthenticationProvider; import org.springframework.security.core.Authentication; @@ -101,6 +102,9 @@ public Authentication authenticate(Authentication authentication) throws Authent } if (!registeredClient.getAuthorizationGrantTypes().contains(AuthorizationGrantType.DEVICE_CODE)) { + if (this.logger.isTraceEnabled()) { + this.logger.warn(LogMessage.format("Invalidated grant_type used by registered client '%s'", registeredClient.getId())); + } throwError(OAuth2ErrorCodes.UNAUTHORIZED_CLIENT, OAuth2ParameterNames.CLIENT_ID); } diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProvider.java index 72440957d..10e254938 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProvider.java @@ -24,6 +24,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.core.log.LogMessage; import org.springframework.security.authentication.AuthenticationProvider; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; @@ -103,6 +104,9 @@ public Authentication authenticate(Authentication authentication) throws Authent OAuth2Authorization authorization = this.authorizationService.findByToken( refreshTokenAuthentication.getRefreshToken(), OAuth2TokenType.REFRESH_TOKEN); if (authorization == null) { + if (this.logger.isTraceEnabled()) { + this.logger.trace("The refresh token is invalid."); + } throw new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_GRANT); } @@ -115,6 +119,9 @@ public Authentication authenticate(Authentication authentication) throws Authent } if (!registeredClient.getAuthorizationGrantTypes().contains(AuthorizationGrantType.REFRESH_TOKEN)) { + if (this.logger.isTraceEnabled()) { + this.logger.warn(LogMessage.format("Invalidated grant_type used by registered client '%s'", registeredClient.getId())); + } throw new OAuth2AuthenticationException(OAuth2ErrorCodes.UNAUTHORIZED_CLIENT); } @@ -123,6 +130,9 @@ public Authentication authenticate(Authentication authentication) throws Authent // As per https://tools.ietf.org/html/rfc6749#section-5.2 // invalid_grant: The provided authorization grant (e.g., authorization code, // resource owner credentials) or refresh token is invalid, expired, revoked [...]. + if (this.logger.isTraceEnabled()) { + this.logger.trace("The refresh token is expired."); + } throw new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_GRANT); } From 3e19ace81bf3a15044228c9754f1c6a00d0971a9 Mon Sep 17 00:00:00 2001 From: smallbun <2689170096@qq.com> Date: Fri, 15 Dec 2023 15:12:15 +0800 Subject: [PATCH 5/5] Improve logging --- .../OAuth2AuthorizationCodeRequestAuthenticationProvider.java | 2 +- .../OAuth2ClientCredentialsAuthenticationProvider.java | 2 +- .../OAuth2DeviceAuthorizationRequestAuthenticationProvider.java | 2 +- .../OAuth2RefreshTokenAuthenticationProvider.java | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java index 7b8fae6c7..2053d7cf5 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java @@ -122,7 +122,7 @@ public Authentication authenticate(Authentication authentication) throws Authent if (!registeredClient.getAuthorizationGrantTypes().contains(AuthorizationGrantType.AUTHORIZATION_CODE)) { if (this.logger.isTraceEnabled()) { - this.logger.warn(LogMessage.format("Invalidated grant_type used by registered client '%s'", registeredClient.getId())); + this.logger.warn(LogMessage.format("Invalid request: requested grant_type is not allowed for registered client '%s'", registeredClient.getId())); } throwError(OAuth2ErrorCodes.UNAUTHORIZED_CLIENT, OAuth2ParameterNames.CLIENT_ID, authorizationCodeRequestAuthentication, registeredClient); diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2ClientCredentialsAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2ClientCredentialsAuthenticationProvider.java index 06a0eb4ee..6963a1fb7 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2ClientCredentialsAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2ClientCredentialsAuthenticationProvider.java @@ -95,7 +95,7 @@ public Authentication authenticate(Authentication authentication) throws Authent if (!registeredClient.getAuthorizationGrantTypes().contains(AuthorizationGrantType.CLIENT_CREDENTIALS)) { if (this.logger.isTraceEnabled()) { - this.logger.warn(LogMessage.format("Invalidated grant_type used by registered client '%s'", registeredClient.getId())); + this.logger.warn(LogMessage.format("Invalid request: requested grant_type is not allowed for registered client '%s'", registeredClient.getId())); } throw new OAuth2AuthenticationException(OAuth2ErrorCodes.UNAUTHORIZED_CLIENT); } diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProvider.java index 05326838c..436cf25be 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2DeviceAuthorizationRequestAuthenticationProvider.java @@ -103,7 +103,7 @@ public Authentication authenticate(Authentication authentication) throws Authent if (!registeredClient.getAuthorizationGrantTypes().contains(AuthorizationGrantType.DEVICE_CODE)) { if (this.logger.isTraceEnabled()) { - this.logger.warn(LogMessage.format("Invalidated grant_type used by registered client '%s'", registeredClient.getId())); + this.logger.warn(LogMessage.format("Invalid request: requested grant_type is not allowed for registered client '%s'", registeredClient.getId())); } throwError(OAuth2ErrorCodes.UNAUTHORIZED_CLIENT, OAuth2ParameterNames.CLIENT_ID); } diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProvider.java index 10e254938..6b866ecfe 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProvider.java @@ -120,7 +120,7 @@ public Authentication authenticate(Authentication authentication) throws Authent if (!registeredClient.getAuthorizationGrantTypes().contains(AuthorizationGrantType.REFRESH_TOKEN)) { if (this.logger.isTraceEnabled()) { - this.logger.warn(LogMessage.format("Invalidated grant_type used by registered client '%s'", registeredClient.getId())); + this.logger.warn(LogMessage.format("Invalid request: requested grant_type is not allowed for registered client '%s'", registeredClient.getId())); } throw new OAuth2AuthenticationException(OAuth2ErrorCodes.UNAUTHORIZED_CLIENT); }