feat(fe): cap the account delegation at the SSO session length - #4190
Merged
Conversation
|
✅ No security or compliance issues detected. Reviewed everything up to 793862f. Security Overview
Detected Code Changes
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the frontend delegation flow to ensure the requested account delegation TTL cannot exceed an SSO organization’s configured session length, keeping delegation-expiry behavior aligned with SSO session validity.
Changes:
- Introduce
cappedMaxTimeToLiveutility to bound delegation TTL by SSO session max age. - Plumb SSO session max age (
sessionMaxAgeNs) from SSO discovery into the authenticated session state. - Apply the cap when preparing account delegations, and add unit tests for the new utility.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/frontend/src/lib/utils/sessionDuration.ts | Adds cappedMaxTimeToLive helper to cap delegation TTL using SSO session max age. |
| src/frontend/src/lib/utils/sessionDuration.test.ts | Adds unit tests covering cappedMaxTimeToLive behavior. |
| src/frontend/src/lib/stores/channelHandlers/delegation.ts | Uses cappedMaxTimeToLive when computing maxTimeToLive for account delegations. |
| src/frontend/src/lib/stores/authentication.store.ts | Extends authenticated session state with optional ssoSessionMaxAgeNs. |
| src/frontend/src/lib/flows/authLastUsedFlow.svelte.ts | Persists SSO session max age from discovery for “last used” SSO sign-ins. |
| src/frontend/src/lib/flows/authFlow.svelte.ts | Threads SSO session max age through SSO sign-in flow into authenticationStore. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
sea-snake
force-pushed
the
feat/sso-delegation-cap
branch
from
August 6, 2026 13:34
4c0ac06 to
49425cd
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/frontend/src/lib/utils/sessionDuration.ts:123
cappedMaxTimeToLivecan return a non-positiverequestedNanosunchanged. Sinceparams.maxTimeToLiveis decoded viaBigInt(...)and can be0/negative, this can later be passed toprepare_account_delegationwhose candid arg isNat64(seesrc/frontend/src/lib/generated/internet_identity_idl.js:1287), causing a serialization/runtime error. Consider treating non-positiverequestedNanos(and defensively non-positivessoSessionMaxAgeNanos) asundefined, consistent with howsessionDurationCeilingSecondstreats malformed requests.
export const cappedMaxTimeToLive = (
requestedNanos: bigint | undefined,
ssoSessionMaxAgeNanos: bigint | undefined,
): bigint | undefined => {
if (ssoSessionMaxAgeNanos === undefined) {
return requestedNanos;
}
if (requestedNanos === undefined) {
return ssoSessionMaxAgeNanos;
}
return requestedNanos > ssoSessionMaxAgeNanos
? ssoSessionMaxAgeNanos
: requestedNanos;
};
sea-snake
force-pushed
the
feat/sso-delegation-cap
branch
from
August 6, 2026 13:46
49425cd to
d918404
Compare
sea-snake
force-pushed
the
feat/sso-delegation-cap
branch
from
August 6, 2026 14:15
0ade4df to
56d03cc
Compare
The sign-in flow carries the organization's session length from discovery onto the authenticated session, and the delegation handler bounds the `maxTimeToLive` it requests by it. An app asking for 30 days on an eight-hour domain gets eight hours. This is a UX cap, not a security boundary: it lets the frontend's own delegation-expiry check drive re-authentication instead of needing to know about attribute expiry. The organization's deadline is enforced by the expiry inside the certified attribute bundle.
The pre-existing comment said the backend default applies when neither the picker nor the app set a duration, which is no longer true for an SSO session: the organization's cap supplies one. Merged into a single accurate comment rather than two that disagree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A two-branch min does not need an exported helper and a test file entry of its own; the clamp now sits where it is used, and sessionDuration.ts is back to its original contents. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Drops the returning-session path. Capping there needed the policy captured out of the discovery promise that keeps the popup in the click task, which is a chunk of code for a case this PR does not need to cover. A returning SSO session therefore keeps whatever duration it asks for until the user signs in through the full flow again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The cap describes how the user authenticated, so it belongs on the union that says exactly that rather than as a sibling field. SSO sign-ins ride the openid variant, so it sits there as an optional value, absent for the built-in providers which publish no such policy. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sea-snake
force-pushed
the
feat/sso-delegation-cap
branch
from
August 6, 2026 14:30
56d03cc to
793862f
Compare
MRmarioruci
approved these changes
Aug 7, 2026
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.
Top of stack, stacked on #4189.
Motivation
The sign-in screen and the relying party both key re-authentication off delegation expiry. Without this, an organization could set an eight-hour session and a dapp holding a 30-day delegation would carry on working long past it, with only attribute verification failing.
Changes
maxTimeToLiveit requests for the account delegation by that value, viacappedMaxTimeToLive. An app asking for 30 days on an eight-hour domain gets eight hours; a shorter request is left alone.This is a UX cap, not a security boundary, and it is applied in the frontend deliberately: a delegation identifies the identity rather than the SSO session, so any other access method on the anchor can mint a fresh one regardless. The organization's deadline is enforced by the expiry inside the certified attribute bundle (#4189).
Tests
tsc --project tsconfig.all.json: clean.eslint: clean. Frontend unit tests pass.Administrator-facing documentation is in dfinity/developer-docs#340.
🤖 Generated with Claude Code