initial commit spring security 16489 before improvements

[GuusArts]

User description

Closes spring-projectsgh-16488

---

PR Type

Enhancement, Tests, Documentation

---

Description

• Introduced ServerGenerateOneTimeTokenRequestResolver for customizable one-time token generation.

  • Added DefaultServerGenerateOneTimeTokenRequestResolver as the default implementation.
  • Enabled customization of token expiration time via setExpiresIn.

• Updated GenerateOneTimeTokenWebFilter to use ServerGenerateOneTimeTokenRequestResolver.

• Enhanced Kotlin DSL and Java configuration to support custom ServerGenerateOneTimeTokenRequestResolver.

• Added comprehensive tests for the new resolver and its integration.

• Updated documentation to guide customization of GenerateOneTimeTokenRequest.

---

Changes walkthrough 📝

	Relevant files
Enhancement	5 files ServerHttpSecurity.java Added support for ServerGenerateOneTimeTokenRequestResolver in ServerHttpSecurity. +34/-1    DefaultServerGenerateOneTimeTokenRequestResolver.java Introduced DefaultServerGenerateOneTimeTokenRequestResolver for token generation. +62/-0    GenerateOneTimeTokenWebFilter.java Integrated ServerGenerateOneTimeTokenRequestResolver into GenerateOneTimeTokenWebFilter. +16/-7    ServerGenerateOneTimeTokenRequestResolver.java Added ServerGenerateOneTimeTokenRequestResolver interface for token request resolution. +40/-0    ServerOneTimeTokenLoginDsl.kt Enhanced Kotlin DSL to support custom token request resolver. +5/-1
Tests	3 files OneTimeTokenLoginSpecTests.java Updated tests for one-time token generation with new resolver. +4/-4      DefaultServerGenerateOneTimeTokenRequestResolverTests.java Added unit tests for DefaultServerGenerateOneTimeTokenRequestResolver. +74/-0    ServerOneTimeTokenLoginDslTests.kt Added integration tests for custom token expiration in Kotlin DSL. +78/-1
Documentation	1 files onetimetoken.adoc Documented customization of GenerateOneTimeTokenRequest and resolver. +32/-0

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[sourcery-ai]

Reviewer's Guide by Sourcery

This pull request introduces the ServerGenerateOneTimeTokenRequestResolver interface and its default implementation, DefaultServerGenerateOneTimeTokenRequestResolver, to allow resolving GenerateOneTimeTokenRequest from ServerWebExchange. It also adds a test case to verify that a custom token expiration time can be set and used during one-time token generation.

Sequence diagram for GenerateOneTimeTokenWebFilter with ServerGenerateOneTimeTokenRequestResolver

sequenceDiagram
    participant Exchange as ServerWebExchange
    participant Filter as GenerateOneTimeTokenWebFilter
    participant Resolver as ServerGenerateOneTimeTokenRequestResolver
    participant Service as ReactiveOneTimeTokenService
    participant Handler as ServerOneTimeTokenGenerationSuccessHandler

    Exchange->>Filter: filter(exchange, chain)
    Filter->>Resolver: resolve(exchange)
    activate Resolver
    Resolver-->>Filter: GenerateOneTimeTokenRequest
    deactivate Resolver
    Filter->>Service: generate(GenerateOneTimeTokenRequest)
    activate Service
    Service-->>Filter: OneTimeToken
    deactivate Service
    Filter->>Handler: handle(exchange, token)
    activate Handler
    Handler-->>Filter: Mono<Void>
    deactivate Handler
    Filter-->>Exchange: Mono<Void>

Loading

Updated class diagram for OneTimeTokenLoginSpec

classDiagram
    class OneTimeTokenLoginSpec {
        -ServerSecurityContextRepository securityContextRepository
        -ServerGenerateOneTimeTokenRequestResolver requestResolver
        -String loginProcessingUrl
        -String defaultSubmitPageUrl
        +OneTimeTokenLoginSpec authenticationConverter(ServerAuthenticationConverter authenticationConverter)
        +OneTimeTokenLoginSpec generateRequestResolver(ServerGenerateOneTimeTokenRequestResolver requestResolver)
    }

Loading

File-Level Changes

Change	Details	Files
Introduces the ServerGenerateOneTimeTokenRequestResolver interface and its default implementation, DefaultServerGenerateOneTimeTokenRequestResolver, to resolve GenerateOneTimeTokenRequest from ServerWebExchange.	Added ServerGenerateOneTimeTokenRequestResolver interface. Created DefaultServerGenerateOneTimeTokenRequestResolver class. Modified GenerateOneTimeTokenWebFilter to use ServerGenerateOneTimeTokenRequestResolver. Updated OneTimeTokenLoginSpec to allow configuring a custom ServerGenerateOneTimeTokenRequestResolver.	config/src/main/java/org/springframework/security/config/web/server/ServerHttpSecurity.java web/src/main/java/org/springframework/security/web/server/authentication/ott/GenerateOneTimeTokenWebFilter.java web/src/main/java/org/springframework/security/web/server/authentication/ott/DefaultServerGenerateOneTimeTokenRequestResolver.java web/src/main/java/org/springframework/security/web/server/authentication/ott/ServerGenerateOneTimeTokenRequestResolver.java config/src/main/kotlin/org/springframework/security/config/web/server/ServerOneTimeTokenLoginDsl.kt
Adds a test case to verify that a custom token expiration time can be set and used during one-time token generation.	Added OneTimeTokenConfigWithCustomTokenExpirationTime configuration class. Implemented a test method oneTimeToken when custom token expiration time set then authenticate to validate the custom expiration time. Added a helper method getCurrentMinutes to calculate the difference in minutes between the expiration time and the current time.	config/src/test/kotlin/org/springframework/security/config/web/server/ServerOneTimeTokenLoginDslTests.kt
Adds a test class for DefaultServerGenerateOneTimeTokenRequestResolver	Added DefaultServerGenerateOneTimeTokenRequestResolverTests test class. Implemented test methods to verify the resolver's behavior when the username parameter is present or absent. Implemented test methods to verify the resolver's behavior when the expires in parameter is set.	web/src/test/java/org/springframework/security/web/server/authentication/ott/DefaultServerGenerateOneTimeTokenRequestResolverTests.java

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!
• Generate a plan of action for an issue: Comment @sourcery-ai plan on
  an issue to generate a plan of action for it.

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[qodo-merge-pro]

PR Reviewer Guide 🔍

(Review updated until commit f4484d8)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Null Username Handling The resolver returns empty Mono when username parameter is not present, but it might be better to return a more specific error or handle this case more explicitly to provide better feedback to users. public Mono<GenerateOneTimeTokenRequest> resolve(ServerWebExchange exchange) { // @formatter:off return exchange.getFormData() .mapNotNull((data) -> data.getFirst(USERNAME)) .switchIfEmpty(Mono.empty()) .map((username) -> new GenerateOneTimeTokenRequest(username, this.expiresIn)); // @formatter:on } Time-based Test The test for token expiration relies on calculating time differences between current time and expiration time, which could be flaky if execution happens near minute boundaries. Assertions.assertThat(getCurrentMinutes(token!!.expiresAt)).isEqualTo(10) } private fun getCurrentMinutes(expiresAt:Instant): Int { val expiresMinutes = expiresAt.atZone(ZoneOffset.UTC).minute val currentMinutes = Instant.now().atZone(ZoneOffset.UTC).minute return expiresMinutes - currentMinutes }

[qodo-merge-pro]

PR Code Suggestions ✨

Latest suggestions up to f4484d8

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Remove redundant empty Mono switch The current implementation returns Mono.empty() when the username is not found, but then immediately switches to an empty Mono. This creates a redundant operation since mapNotNull already returns an empty Mono when the result is null. Simplify by removing the redundant switchIfEmpty(Mono.empty()) call. web/src/main/java/org/springframework/security/web/server/authentication/ott/DefaultServerGenerateOneTimeTokenRequestResolver.java [43-51] @Override public Mono<GenerateOneTimeTokenRequest> resolve(ServerWebExchange exchange) { // @formatter:off return exchange.getFormData() .mapNotNull((data) -> data.getFirst(USERNAME)) - .switchIfEmpty(Mono.empty()) .map((username) -> new GenerateOneTimeTokenRequest(username, this.expiresIn)); // @formatter:on } Apply this suggestion Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies a redundant operation in the reactive chain. The mapNotNull already returns an empty Mono when the result is null, making the switchIfEmpty(Mono.empty()) call unnecessary. Removing it simplifies the code without changing functionality.	Low
More

---

Previous suggestions

Suggestions up to commit f4484d8

Category	Suggestion	Impact
General	Remove redundant empty Mono The current implementation uses switchIfEmpty(Mono.empty()) which is redundant since mapNotNull already returns an empty Mono when the mapping function returns null. This creates unnecessary complexity in the reactive chain. web/src/main/java/org/springframework/security/web/server/authentication/ott/DefaultServerGenerateOneTimeTokenRequestResolver.java [43-51] @Override public Mono<GenerateOneTimeTokenRequest> resolve(ServerWebExchange exchange) { // @formatter:off return exchange.getFormData() .mapNotNull((data) -> data.getFirst(USERNAME)) - .switchIfEmpty(Mono.empty()) .map((username) -> new GenerateOneTimeTokenRequest(username, this.expiresIn)); // @formatter:on } Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies a redundant operation in the reactive chain. The switchIfEmpty(Mono.empty()) is unnecessary since mapNotNull already returns an empty Mono when the mapping function returns null. Removing it simplifies the code without changing functionality.	Low

Suggestions up to commit f4484d8

Category	Suggestion	Impact
General	Remove redundant switchIfEmpty operation The current implementation uses switchIfEmpty(Mono.empty()) which is redundant and can be removed. The mapNotNull operation already handles empty values by returning an empty Mono, making the additional switchIfEmpty unnecessary. web/src/main/java/org/springframework/security/web/server/authentication/ott/DefaultServerGenerateOneTimeTokenRequestResolver.java [43-51] @Override public Mono<GenerateOneTimeTokenRequest> resolve(ServerWebExchange exchange) { // @formatter:off return exchange.getFormData() .mapNotNull((data) -> data.getFirst(USERNAME)) - .switchIfEmpty(Mono.empty()) .map((username) -> new GenerateOneTimeTokenRequest(username, this.expiresIn)); // @formatter:on } Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies a redundant operation in the reactive chain. The mapNotNull already returns an empty Mono when the username is null, making the switchIfEmpty(Mono.empty()) unnecessary. Removing it simplifies the code without changing its behavior.	Low

[sourcery-ai]

Hey @GuusArts - I've reviewed your changes - here's some feedback:

Overall Comments:

• Consider adding a test case for when the username parameter is missing in the request.
• It would be helpful to provide a more detailed explanation or example of how to configure a custom ServerGenerateOneTimeTokenRequestResolver in the reference documentation.

Here's what I looked at during the review

• 🟢 General issues: all looks good
• 🟢 Security: all looks good
• 🟡 Testing: 1 issue found
• 🟢 Complexity: all looks good
• 🟢 Documentation: all looks good

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

suggestion (testing): Missing test cases for invalid input

It would be beneficial to add tests that cover scenarios with invalid input in the request, such as an empty username, whitespace-only username, or a very long username. This ensures the resolver handles these edge cases gracefully and provides appropriate error handling or defaults.

[GuusArts]

@s

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud