Skip to content

feat(fe): cap the account delegation at the SSO session length - #4190

Merged
sea-snake merged 5 commits into
feat/sso-attribute-expiryfrom
feat/sso-delegation-cap
Aug 7, 2026
Merged

feat(fe): cap the account delegation at the SSO session length#4190
sea-snake merged 5 commits into
feat/sso-attribute-expiryfrom
feat/sso-delegation-cap

Conversation

@sea-snake

@sea-snake sea-snake commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

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

  • The sign-in flow carries the organization's session length from the discovery result onto the authenticated session, for both the fresh-sign-in and last-used paths.
  • The delegation handler bounds the maxTimeToLive it requests for the account delegation by that value, via cappedMaxTimeToLive. 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

  • New unit tests for the clamp: non-SSO sessions pass through, a longer request is capped, a shorter request is untouched, and the organization's value applies when nothing was requested.
  • 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

@sea-snake
sea-snake marked this pull request as ready for review August 6, 2026 13:25
@sea-snake
sea-snake requested a review from a team as a code owner August 6, 2026 13:25
Copilot AI lite review requested due to automatic review settings August 6, 2026 13:25
@zeropath-ai

zeropath-ai Bot commented Aug 6, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to 793862f.

Security Overview
Detected Code Changes
Change Type Relevant files
Enhancement ► src/frontend/src/lib/flows/authFlow.svelte.ts
    Modify to extract and propagate ssoSessionMaxAgeNs from ssoResult, pass it to openIdJwtSignIn, and store it in authentication
Enhancement ► src/frontend/src/lib/stores/authentication.store.ts
    Update Authenticated.authMethod.openid to include ssoSessionMaxAgeNs (optional)
Enhancement ► src/frontend/src/lib/stores/channelHandlers/delegation.ts
    Read authMethod to obtain ssoSessionMaxAgeNs and adjust maxTimeToLive accordingly

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 cappedMaxTimeToLive utility 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.

Comment thread src/frontend/src/lib/stores/channelHandlers/delegation.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  • cappedMaxTimeToLive can return a non-positive requestedNanos unchanged. Since params.maxTimeToLive is decoded via BigInt(...) and can be 0/negative, this can later be passed to prepare_account_delegation whose candid arg is Nat64 (see src/frontend/src/lib/generated/internet_identity_idl.js:1287), causing a serialization/runtime error. Consider treating non-positive requestedNanos (and defensively non-positive ssoSessionMaxAgeNanos) as undefined, consistent with how sessionDurationCeilingSeconds treats 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
sea-snake force-pushed the feat/sso-delegation-cap branch from 49425cd to d918404 Compare August 6, 2026 13:46
@sea-snake
sea-snake force-pushed the feat/sso-delegation-cap branch from 0ade4df to 56d03cc Compare August 6, 2026 14:15
sea-snake and others added 5 commits August 6, 2026 16:30
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
sea-snake force-pushed the feat/sso-delegation-cap branch from 56d03cc to 793862f Compare August 6, 2026 14:30
@sea-snake
sea-snake merged commit 7e256d3 into main Aug 7, 2026
83 checks passed
@sea-snake
sea-snake deleted the feat/sso-delegation-cap branch August 7, 2026 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants