-
-
Notifications
You must be signed in to change notification settings - Fork 77
feature/478-state_storage: replace state cookie #492
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
ca043ce to
c072262
Compare
|
Should this PR wait for dotnet 9 PR to be merged? |
src/ActiveLogin.Authentication.BankId.AspNetCore/Auth/AuthenticationBuilderBankIdExtensions.cs
Outdated
Show resolved
Hide resolved
src/ActiveLogin.Authentication.BankId.AspNetCore/BankIdConstants.cs
Outdated
Show resolved
Hide resolved
src/ActiveLogin.Authentication.BankId.AspNetCore/Sign/ServiceCollectionBankIdExtensions.cs
Outdated
Show resolved
Hide resolved
elinohlsson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard to review and verify that the it is working as expected. Needs to be tested. Only had a few comments of smaller changes.
test/ActiveLogin.Authentication.BankId.AspNetCore.Test/BankId_Ui_Tests_Base.cs
Outdated
Show resolved
Hide resolved
test/ActiveLogin.Authentication.BankId.AspNetCore.Test/BankId_Ui_Tests_Base.cs
Outdated
Show resolved
Hide resolved
|
@Zonnex also add to documentation how to create your own implementation of IStateStorage. |
c42196e to
aa5a85f
Compare
|
This feature needs more work. The proposed solution is too simple, and should perhaps support a distributed environment out of the box. A new implementation should perhaps use HybridCache or IDistributedCache. I will convert this to draft to avoid mistakes by merging in work that isn't sufficient |
d9c066b to
f612b4e
Compare
0e59cb7 to
fcb4c2c
Compare
85d4bc9 to
2557a87
Compare
2557a87 to
786f80b
Compare
This PR fixes #478.
High level overview of this PR: