Fix: add length validation to prevent 500 error on invalid usercode #1309
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
spring-projects-issues
added
the
status: waiting-for-triage
martinr0x
changed the title
sjohnr
added
type: bug
sjohnr
requested changes
Jul 20, 2023
There was a problem hiding this comment.
Thanks for the PR @martinr0x!
Please update the PR to fix codestyle issues (I believe we are not yet checking code style in the CI pipeline). Also, please add one additional test for OAuth2DeviceAuthorizationConsentAuthenticationConverter as well.
Comment on lines
97
to
99
| static boolean validateUserCode(String userCode){ | ||
| return userCode != null && userCode.replaceAll("[^A-Z\\d]+", "").length() == 8; | ||
| } |
There was a problem hiding this comment.
Please format this method according to code style conventions.
There was a problem hiding this comment.
Hi @sjohnr ,
I have applied the .editorconfig to the file. Is there any other formatting that I need to do?
There was a problem hiding this comment.
I don't see anything after scanning it briefly. I'll get this merged this week. Thanks!
...r/authorization/web/authentication/OAuth2DeviceVerificationAuthenticationConverterTests.java
Show resolved
Hide resolved
|
Hi @sjohnr , |
|
Thanks @martinr0x! This is now merged via 82b7059 with minor polish 215c101. |
tjholmes66
added a commit
to tjholmes66/spring-authorization-server
that referenced
this pull request
Oct 2, 2023
* Update to io.spring.javaformat:spring-javaformat-checkstyle:0.0.35 Closes spring-projectsgh-1089 * Update to jackson-bom:2.14.2 Closes spring-projectsgh-1090 * Update to junit-jupiter:5.9.2 Closes spring-projectsgh-1091 * Release 1.0.1 * Next Development Version * Update to Spring Security 6.1.0-M1 Closes spring-projectsgh-1093 * Update to nimbus-jose-jwt:9.30.2 Closes spring-projectsgh-1094 * Update to assertj-core:3.24.2 Closes spring-projectsgh-1095 * Update to mockito-core:4.11.0 Closes spring-projectsgh-1096 * Release 1.1.0-M1 * Next Development Version * Add user property to deploy_docs workflow * Fix broken support link Closes spring-projectsgh-1092 * Fix client secret encoding when client dynamically registered Closes spring-projectsgh-1056 * Polish spring-projectsgh-1056 * Allow PasswordEncoder to be configured in OidcClientRegistrationAuthenticationProvider Issue spring-projectsgh-1056 * Upgrade client secret when available Closes spring-projectsgh-1099 * Polish spring-projectsgh-1105 * Add support for OAuth 2.0 Device Authorization Grant Closes spring-projectsgh-44 * Switch to spring-security SNAPSHOT dependencies Issue spring-projectsgh-44 * Use spring-security 6.1 in snapshot tests Issue spring-projectsgh-1106 * Update to actions/checkout@v3 Closes spring-projectsgh-1117 * Use spring-io/spring-gradle-build-action Closes spring-projectsgh-1120 * Use spring-io/spring-gradle-build-action Closes spring-projectsgh-1120 * Revert accidental change in versions Issue spring-projectsgh-1120 * Polish spring-projectsgh-1106 * Update to Spring Framework 6.0.7 Closes spring-projectsgh-1130 * Update to Spring Security 1.1.0-M2 Closes spring-projectsgh-1131 * Update to nimbus-jose-jwt:9.31 Closes spring-projectsgh-1132 * Update to Spring Framework 6.0.7 in buildSrc Issue spring-projectsgh-1130 * Release 1.1.0-M2 * Next Development Version * Polish spring-projectsgh-1106 Device Authorization Grant * Avoid persisting client principal in device authorization request Issue spring-projectsgh-1106 * Polish spring-projectsgh-1068 Issue spring-projectsgh-1077 * Add OidcLogoutAuthenticationToken.isPrincipalAuthenticated() Issue spring-projectsgh-1077 * Ensure ID Token is active before processing logout request Issue spring-projectsgh-1077 * Allow localhost in redirect_uri Closes spring-projectsgh-651 * Fix refresh token error code INVALID_CLIENT to INVALID_GRANT Closes spring-projectsgh-1139 * Do not require authorizationRequest for device grant Issue spring-projectsgh-1127 * Add tests for OAuth 2.0 Device Authorization Grant This commit adds tests for the following components: * AuthenticationConverters * AuthenticationProviders * Endpoint Filters Issue spring-projectsgh-44 Closes spring-projectsgh-1127 * JDBC device_code authorization Issue spring-projectsgh-1156 * Polish spring-projectsgh-1143 * Add tests and update examples in docs Closes spring-projectsgh-1156 * Polish ref-doc Issue spring-projectsgh-1156 * Add customization to support public clients for device grant Issue spring-projectsgh-1157 * Polish samples Closes spring-projectsgh-1157 * Add documentation for OAuth 2.0 Device Authorization Grant Closes spring-projectsgh-1158 * Polish spring-projectsgh-1127 * Polish spring-projectsgh-1158 * Add documentation for OpenID Connect 1.0 Logout Endpoint Closes spring-projectsgh-1069 * Update Stack Overflow tag to spring-authorization-server * Update to Spring Framework 5.3.27 Closes spring-projectsgh-1162 * Update to Spring Security 5.8.3 Closes spring-projectsgh-1163 * Update to io.spring.javaformat:spring-javaformat-checkstyle:0.0.38 Closes spring-projectsgh-1164 * Update to Spring Framework 6.0.8 Closes spring-projectsgh-1165 * Update to Spring Security 6.0.3 Closes spring-projectsgh-1166 * Update to io.spring.javaformat:spring-javaformat-checkstyle:0.0.38 Closes spring-projectsgh-1167 * Update to Spring Framework 6.0.8 Closes spring-projectsgh-1168 * Update to Spring Security 6.1.0-RC1 Closes spring-projectsgh-1169 * Update to io.spring.javaformat:spring-javaformat-checkstyle:0.0.38 Closes spring-projectsgh-1170 * Update to json-path:2.8.0 Closes spring-projectsgh-1171 * Release 0.4.2 * Next Development Version * Release 1.0.2 * Next Development Version * Release 1.1.0-RC1 * Next Development Version * Update to org.jfrog.buildinfo:build-info-extractor-gradle:4.29.0 Closes spring-projectsgh-1175 * Apply ArtifactoryPlugin to SpringRootProjectPlugin Closes spring-projectsgh-1177 * Fix artifact build properties for Artifactory - Apply SpringArtifactoryPlugin in SpringRootProjectPlugin (which applies ArtifactoryPlugin) - In SpringArtifactoryPlugin don't set publication if MavenPublishPlugin is not applied Closes spring-projectsgh-1179 * Add test for dynamic client registration with custom metadata Issue spring-projectsgh-1172 * Add logout success page to default client sample Sample client (located in 'samples/messages-client' directory) is configured with a custom logout success page where the end-user is redirected to upon successful logout action. Fixes spring-projectsgh-1142 * Add sample featured-authorizationserver Issue spring-projectsgh-1189 * Merge custom-consent-authorizationserver into featured-authorizationserver Issue spring-projectsgh-1189 * Merge federated-identity-authorizationserver into featured-authorizationserver Issue spring-projectsgh-1189 * Update io.spring.ge.conventions plugin to 0.0.13 Closes spring-projectsgh-1190 * Update spring-asciidoctor-backends to 0.0.5 Closes spring-projectsgh-1192 * Merge device-grant-authorizationserver into featured-authorizationserver Issue spring-projectsgh-1189 * Merge device-client into messages-client Issue spring-projectsgh-1189 * Use custom consent page for device code flow Issue spring-projectsgh-1189 * Use current authentication for device authorization Issue spring-projectsgh-1189 * Reuse error handling Issue spring-projectsgh-1189 * Handle web client response error Issue spring-projectsgh-1189 * Update @SInCE * Rename featured-authorizationserver to demo-authorizationserver Issue spring-projectsgh-1189 * Rename messages-client to demo-client Issue spring-projectsgh-1189 * Update sample README Issue spring-projectsgh-1189 * Add integration tests for device grant Issue spring-projectsgh-1116 * Update web ui design for demo-client Issue spring-projectsgh-1196 * Polish web ui design for demo-client Issue spring-projectsgh-1196 * Update web ui design for demo-authorizationserver Issue spring-projectsgh-1196 * Polish web ui design for demo-client Issue spring-projectsgh-1196 * Polish demo sample Issue spring-projectsgh-1189 * Update to Spring Boot 3.1.0-RC1 Closes spring-projectsgh-1198 * Refresh Getting Started example Closes spring-projectsgh-1186 * Use Spring Boot starter in samples Closes spring-projectsgh-1187 * Invalidate tokens previously issued when code is reused Closes spring-projectsgh-1152 * Polish spring-projectsgh-1152 * Add How-to: Authenticate using Social Login Closes spring-projectsgh-538 * Add How-to: Authenticate using a Single Page Application with PKCE Closes spring-projectsgh-539 * Hash the sid claim in the ID Token Closes spring-projectsgh-1207 * Simplified federated login in demo sample Closes spring-projectsgh-1208 * Polish spring-projectsgh-1186 * Polish spring-projectsgh-538 * Remove FederatedIdentityConfigurer from demo sample Issue spring-projectsgh-1208 * Update exception handling config in ref-doc Closes spring-projectsgh-1205 * Update exception handling config in samples Closes spring-projectsgh-1204 * Polish ref-doc Issue spring-projectsgh-1205 * Polish tests * Add How-to: Implement an Extension Authorization Grant Type Closes spring-projectsgh-686 * Update to Spring Framework 6.0.9 Closes spring-projectsgh-1213 * Update to Spring Security 6.1.0 Closes spring-projectsgh-1214 * Update to jackson-bom 2.15.0 Closes spring-projectsgh-1215 * Update to junit-jupiter 5.9.3 Closes spring-projectsgh-1216 * Release 1.1.0 * Next Development Version * Revert serialVersionUID to 0.4.0 Closes spring-projectsgh-1218 * Revert serialVersionUID to 1.0.0 Closes spring-projectsgh-1219 * Revert serialVersionUID to 1.1.0 Closes spring-projectsgh-1220 * Exclude project dependency from spring-boot-dependencies Closes spring-projectsgh-1228 * Update to Spring Boot 3.1.0 * Update com.gradle.enterprise plugin to 3.13.3 Closes spring-projectsgh-1234 Issue spring-projectsgh-1231 * Prepare for automated validation scripts See https://github.com/gradle/gradle-enterprise-build-validation-scripts/blob/main/Gradle.md Issue spring-projectsgh-1231 * ID Token contains sid claim after refresh_token grant Closes spring-projectsgh-1224 * Use substring instead of replaceFirst in OAuth2AuthorizationConsent Closes spring-projectsgh-1222 * Validate authorized principal instead of sub during logout Closes spring-projectsgh-1235 * Revert "Prepare for automated validation scripts" This reverts commit ece9f10. Issue spring-projectsgh-1231 * Add debug log entries Closes spring-projectsgh-1245 Closes spring-projectsgh-1246 Closes spring-projectsgh-1247 Closes spring-projectsgh-1248 * Polish additional logging Issue spring-projectsgh-1245, spring-projectsgh-1246, spring-projectsgh-1247, spring-projectsgh-1248 * Enable caching of :asciidoctor gradle task * Use direct code import Issue spring-projectsgh-1231 * Next Minor Version * Fix NPE on access token in OAuth2AuthorizationCodeAuthenticationProvider Closes spring-projectsgh-1233 * Polish spring-projectsgh-1233 * Fix to save all values for multi-valued request parameters Fixes spring-projectsgh-1250 * Polish spring-projectsgh-1252 * Fix to save all values for multi-valued device grant parameters Fixes spring-projectsgh-1269 * Polish spring-projectsgh-1252 * Update to Spring Framework 5.3.28 Closes spring-projectsgh-1272 * Update to Spring Security 5.8.4 Closes spring-projectsgh-1273 * Update to jackson-bom 2.14.3 Closes spring-projectsgh-1274 * Update to Spring Framework 6.0.10 Closes spring-projectsgh-1276 * Update to Spring Security 6.0.4 Closes spring-projectsgh-1277 * Update to Spring Framework 6.0.10 Closes spring-projectsgh-1278 * Update to Spring Security 6.1.1 Closes spring-projectsgh-1279 * Update to junit-jupiter 5.9.3 Closes spring-projectsgh-1280 * Update to junit-jupiter 5.9.3 Closes spring-projectsgh-1281 * Update to jackson-bom 2.15.2 Closes spring-projectsgh-1282 * Update feature planning section to using GitHub Projects * Release 1.1.1 * Next Development Version * Fix generating ID token with null sid when refresh_token grant Closes spring-projectsgh-1283 * Polish spring-projectsgh-1289 * Fix NPE in DefaultErrorController Closes spring-projectsgh-1286 * Migrate docs to Antora Issue spring-projectsgh-1295 * Polish Antora migration Issue spring-projectsgh-1292 Closes spring-projectsgh-1295 * Remove unused antora-playbook.yml * Replaces 'install' with 'publishToMavenLocal' command in README * Adds how-to guide on adding authorities to access tokens Closes spring-projectsgh-542 * Polish spring-projectsgh-1264 * Update order of guides in nav.adoc * Fix userCode validation Issue spring-projectsgh-44 * Polish spring-projectsgh-1309 * Add Revved up by Gradle Enterprise badge * Move badges to header This is similar to Spring Boot: https://github.com/spring-projects/spring-boot/blob/main/README.adoc * Fix workflow status link * Fix samples test suite execution and failing tests Closes spring-projectsgh-1324 * Polish spring-projectsgh-1325 * Move deploy-docs.yml to correct folder Issue spring-projectsgh-1295 * Remove manual list of guides Issue spring-projectsgh-1295 * Remove reference to gh-pages Issue spring-projectsgh-1295 * Update to Spring Framework 6.0.11 Closes spring-projectsgh-1338 * Update to Spring Security 6.1.2 Closes spring-projectsgh-1339 * Update to org.hsqldb:hsqldb 2.7.2 Closes spring-projectsgh-1340 * Release 1.1.2 * Next Development Version * Adds ability to inject custom metadata at client registration Closes spring-projectsgh-1172 * Polish spring-projectsgh-1326 * Adds dynamic client registration how-to guide Closes spring-projectsgh-647 * Polish spring-projectsgh-1320 * Add code challenge methods for oidc provider configuration response Closes spring-projectsgh-1302 * Update to Spring Framework 6.1.0-M5 Closes spring-projectsgh-1364 * Update to Spring Security 6.2.0-M3 Closes spring-projectsgh-1365 * Update to nimbus-jose-jwt 9.35 Closes spring-projectsgh-1366 * Update to junit-jupiter 5.10.0 Closes spring-projectsgh-1367 * Update to okhttp 4.11.0 Closes spring-projectsgh-1368 * Release 1.2.0-M1 * Next Development Version --------- Co-authored-by: Joe Grandja <jgrandja@vmware.com> Co-authored-by: Siva Kumar Edupuganti <esivakumar18@gmail.com> Co-authored-by: Yuta Saito <uc4w6c@bma.biglobe.ne.jp> Co-authored-by: Shannon Pamperl <shanman190@gmail.com> Co-authored-by: Steve Riesenberg <sriesenberg@vmware.com> Co-authored-by: HuiYeong <huiyeong@lguplus.co.kr> Co-authored-by: Xu Xiaowei <xuxiaowei@xuxiaowei.com.cn> Co-authored-by: Janne Valkealahti <janne.valkealahti@gmail.com> Co-authored-by: Dmitriy Dubson <ddubson@vmware.com> Co-authored-by: neochae <neochae@lguplus.co.kr> Co-authored-by: heartape <heartape@163.com> Co-authored-by: Dejan Varmedja <114813331+fndejan@users.noreply.github.com> Co-authored-by: Jerome Prinet <jprinet@gradle.com> Co-authored-by: Pavel Efros <efros.pavel@gmail.com> Co-authored-by: Martin Lindström <martin.lindstrom@litsec.se> Co-authored-by: cbilodeau <cbilodeau@upgrade.com> Co-authored-by: Rob Winch <rwinch@users.noreply.github.com> Co-authored-by: Dmitriy Dubson <d.dubson@gmail.com> Co-authored-by: Martin Bogusz <m.bogusz@celonis.com> Co-authored-by: Eric Haag <ehaag@gradle.com> Co-authored-by: Tuxzx <tuxzx@outlook.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi SAS team!
Currently, a non empty, invalid usercode results in a 500 error as the assertion in the normalizeUserCode method fails.