Fix the sliding cookie configuration when using frontend tests#2041
Merged
Erwinvandervalk merged 1 commit intomainfrom Jun 10, 2025
Merged
Fix the sliding cookie configuration when using frontend tests#2041Erwinvandervalk merged 1 commit intomainfrom
Erwinvandervalk merged 1 commit intomainfrom
Conversation
4cdfe58 to
882dddf
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the sliding cookie configuration when using frontend tests, ensuring compatibility between the legacy (V3) and new (V4) configuration styles. It introduces explicit wiring of the TimeProvider in tests and production code, supports both manual and frontend-managed BFF setups via parameterized tests, and adjusts cookie configuration events for sliding expiration.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| bff/test/Bff.Tests/TestInfra/TestHost.cs | Adds TimeProvider to the DI container for testing purposes. |
| bff/test/Bff.Tests/TestInfra/TestDataBuilder.cs | Introduces a helper method for generating UserSessionsFilter for tests. |
| bff/test/Bff.Tests/TestInfra/TestData.cs | Provides a FakeTimeProvider instance and current time accessor. |
| bff/test/Bff.Tests/TestInfra/BffTestBase.cs | Implements a new parameterized test configuration with BFF setup type options. |
| bff/test/Bff.Tests/SessionManagement/CookieSlidingTests.cs | Updates cookie sliding tests to run with both V3 and V4 setup configurations and refines clock advancing logic. |
| bff/src/Bff/SessionManagement/Configuration/PostConfigureSlidingExpirationCheck.cs | Adjusts the cookie sliding expiration configuration to check multiple scheme types. |
| bff/src/Bff/SessionManagement/Configuration/PostConfigureApplicationCookieTicketStore.cs | Applies similar scheme checking logic for the application cookie ticket store. |
| bff/src/Bff/DynamicFrontends/BffConfigureOpenIdConnectOptions.cs | Injects and assigns the TimeProvider to OpenIdConnect options. |
| bff/src/Bff/DynamicFrontends/BffConfigureCookieOptions.cs | Injects and assigns the TimeProvider to Cookie options. |
| bff/src/Bff/BffBuilder.cs | Minor cleanup in the builder class. |
damianh
approved these changes
Jun 9, 2025
This file contains hidden or 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
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.
What issue does this PR address?
The sliding cookie tests were still in the 'old' testing style.
Turns out that the new frontend style of wiring things wasn't working for sliding cookies. So now the tests explicitly run the 'old' and the 'new' style of wiring things up. This will likely be the pattern I'll use for future tests also, because I want to make sure you can still use V3 style (explicit auth wireup) and v4 style (automatic wireup)
Also, it turns out that, if you do AddScheme, aspnet core also adds a specific post configuration middleware:
https://github.com/dotnet/aspnetcore/blob/main/src/Security/Authentication/Core/src/AuthenticationBuilder.cs#L52
We have to do the same in our dynamic scheme wireup. (basically, it only adds the timeprovider, but still, quite important)
Important: Any code or remarks in your Pull Request are under the following terms:
If You provide us with any comments, bug reports, feedback, enhancements, or modifications proposed or suggested by You for the Software, such Feedback is provided on a non-confidential basis (notwithstanding any notice to the contrary You may include in any accompanying communication), and Licensor shall have the right to use such Feedback at its discretion, including, but not limited to the incorporation of such suggested changes into the Software. You hereby grant Licensor a perpetual, irrevocable, transferable, sublicensable, nonexclusive license under all rights necessary to incorporate and use your Feedback for any purpose, including to make and sell any products and services.
(see our license, section 7)