Repository navigation
Add server-side OpenID Connect sessions - #902
Conversation
…ack-office account check
… subject, cleanup query
…ck-office sessions
…treat refresher failures as no answer
…d in again A re-sign-in after a password, e-mail or username change used the two-argument SignIn, which dropped grand:oidcSession, so the new cookie escaped idle and maximum lifetime, revalidation and back-channel logout. The two-argument SignIn now carries the session of the current cookie over when it belongs to the same customer.
If ending the server-side session threw, SignOutAsync never ran and the customer stayed signed in. The failure is now logged as a warning with the session id and both schemes are always signed out.
It dereferenced the missing entity and threw, while MongoDB treats no match as a no-op. Conditional updates such as ending an OpenID Connect session that has already ended rely on the MongoDB behavior.
…r, not only back-office ones
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| catch (Exception ex) | ||
| { | ||
| //no cancellation token is passed, so a cancellation is a provider timeout: no answer, like any failure | ||
| _logger.LogWarning("Refreshing session {SessionId} failed: {ExceptionType}", session.Id, ex.GetType().Name); | ||
| result = OidcRefreshResult.Unavailable(); | ||
| } |
There was a problem hiding this comment.
Intentional. Any failure of the refresher (network, timeout, a provider answering with something unparsable) has to count as "no answer" (Unavailable). The session then continues for up to two revalidation intervals instead of failing the request. The exception type is logged. The message and body are not, because they can contain the refresh token.
| catch (Exception ex) | ||
| { | ||
| logService.LogError(ex, "UpgradeProcess - OidcSessions indexes (2.5)"); | ||
| } |
There was a problem hiding this comment.
Intentional, and the same pattern as the other upgrade migrations: a failure is logged and the remaining steps still run. An index that could not be created only costs query speed and does not block the upgrade.
| catch (Exception ex) | ||
| { | ||
| logService.LogError(ex, "UpgradeProcess - OidcSessions schedule task (2.5)"); | ||
| } |
There was a problem hiding this comment.
Same as above: the schedule task insert is logged on failure, like in the other upgrade migrations, so one failed step does not abort the whole upgrade.
|
Code review finding:\n\nFile and line: src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs:32\n\nRule violated: .ai/constraints.md forbids static mutable state.\n\nImpact: RefreshesInProgress is process-local mutable state. It does not protect refresh-token rotation across multiple app instances, so two instances can still refresh the same session concurrently and one can prematurely end the session. It also creates hidden cross-request/test state.\n\nSuggested fix: move the refresh-in-progress claim into persisted state, for example a short session refresh lease updated atomically through the repository, or document and implement an explicit distributed-safe strategy.\n\nReview scope: security, .NET, architecture, scheduled task, database/MongoDB, plugin/module rules, and repo code-review checklist. I did not run the test suites. |
…ions deleted, access tokens revoked only on a provider refusal
…ession it replaces
| catch (Exception ex) | ||
| { | ||
| //the cookie that referenced it is replaced anyway; the record goes with the cleanup task | ||
| _logger.LogWarning(ex, "Could not end the OpenID Connect session {SessionId}", sessionId); | ||
| } |
|
Agreed, fixed in f6aa28d. The process-local The same commit also fixes three related points:
The section on limits in the guide no longer lists the multi-instance race. |
Resolves no public issue
Type: feature
Issue
OpenID Connect browser sign-in currently relies on the authentication cookie and provider tokens without a server-side session record. GrandNode cannot centrally end provider-backed sessions, clean up expired session data, revoke refresh tokens after subject revocation, or consistently revalidate long-lived back-office sessions against the identity provider.
Solution
Add core OpenID Connect session support: a persisted OidcSession entity, session service, token revocation service, cookie session claim, cleanup scheduled task, migration, default configuration, and tests around session lifecycle, refresh token handling, provider revalidation, and LiteDB no-match updates.
Breaking changes
None expected for existing installations. The change adds new persisted session data, settings, migration, and service contracts used by the OIDC flow.
Testing
Not run during PR creation.