feat: implement storage of session in database - #52
Conversation
jhbritton-RSK
changed the title
…d also follow same pattern as existig stores from IdS4
3248415 to
2426413
Compare
| <ProjectReference Include="..\..\src\Open.IdentityServer.csproj"/> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup> |
There was a problem hiding this comment.
Why is the MockLogger required in this assembly? Does TestLogger.Create<T> not suffice?
There was a problem hiding this comment.
I added this for a mock logger I could validate logs were made. TestLogger is used in may places so wanted avoid touching it. I also shared between two test projects, might be better to spin-up a shared tests project for things like that.
|
|
||
| if (existingSession == null) | ||
| { | ||
| logger.LogInformation("failed renewing '{SessionKey}' session in database, session with key doesn't exists", key); |
There was a problem hiding this comment.
typo: exists shouldn't be plural
There was a problem hiding this comment.
Also this should be log warning not information
| /// | ||
| /// </summary> | ||
| /// <param name="seedData"></param> | ||
| public InMemorySessionStore(IDictionary<string, IdentityServerServerSideSessions> seedData): this() |
There was a problem hiding this comment.
Seeding data for sessions? Is this for unit testing?
There was a problem hiding this comment.
I was following original pattern of other stores. But doesn't really make sense for server side sessions other than for testing.
35dcc6b to
e3344e5
Compare
3 participants
Description
Implemented initial storage of session in database following the existing db schema created for compatibility.
Type of change
Does this PR introduce a breaking change?
Testing
Describe how the change has been tested. New code should be covered by appropriate unit and/or integration tests.
LLM Usage
Used for guidance on setting up integration tests, and understanding more about how the sessions work in the existing IdentityServer4 codebase.
Other context
n/a