ory / ory/polis

IdP-initiated SAML responses can be replayed; no used-assertion state is kept

Open
#4,077 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
2.3k
Forks
233
PR merge metrics
No merged PRs in 30d

Description

Issue Summary

An IdP-initiated SAMLResponse can be posted to /api/oauth/saml repeatedly, and each post mints a fresh authorization code. Nothing records which assertions have been used. SP-initiated is unaffected, since the session is deleted at npm/src/controller/oauth.ts:892.

SAML 2.0 Profiles section 4.1.4.5 asks the SP to keep the set of used assertion IDs. @boxyhq/saml20 1.16.0 takes an assertionReplayValidator; ValidateOption (npm/src/saml/lib.ts:35-41) has no such field and npm/src/controller/oauth.ts:827 passes none.

Steps to Reproduce
  1. Run with IDP_ENABLED=true.
  2. Post an IdP-initiated SAMLResponse to /api/oauth/saml.
  3. Post the same body again. You get a second valid code.

As a test, save this as npm/test/sso/assertion_replay.test.ts. In-memory store, no network, saml.validate stubbed as in test/sso/saml_idp_oauth.test.ts.

import * as fs from 'fs';
import * as path from 'path';
import sinon from 'sinon';
import tap from 'tap';
import saml from '@boxyhq/saml20';
import controllers from '../../src/index';
import {
  IConnectionAPIController,
  IOAuthController,
  SAMLResponsePayload,
  SAMLSSOConnectionWithRawMetadata,
} from '../../src/typings';
import { jacksonOptions } from '../utils';

const TENANT = 'boxyhq.com';
const PRODUCT = 'crm';
const REDIRECT_URI = 'http://localhost:3366/sso/oauth/completed';
const ISSUER = 'https://accounts.google.com/o/saml2';

let conn: IConnectionAPIController;
let oauth: IOAuthController;
let rawResponse: string;

// The same assertion the IdP posted once, replayed verbatim.
const post = async (relayState: string | null = '') => {
  const stub = sinon.stub(saml, 'validate').resolves({
    audience: '',
    issuer: ISSUER,
    sessionIndex: 'session-1',
    assertionId: '_the_same_assertion_id',
    claims: { id: 'alice@boxyhq.com', email: 'alice@boxyhq.com' },
  } as any);

  try {
    return await oauth.samlResponse(<SAMLResponsePayload>{ SAMLResponse: rawResponse, RelayState: relayState });
  } finally {
    stub.restore();
  }
};

const codeFrom = (redirectUrl?: string) =>
  redirectUrl ? new URLSearchParams(new URL(redirectUrl).search).get('code') : null;

tap.before(async () => {
  const c = await controllers({ ...jacksonOptions, idpEnabled: true });
  conn = c.connectionAPIController;
  oauth = c.oauthController;

  await conn.createSAMLConnection({
    defaultRedirectUrl: REDIRECT_URI,
    redirectUrl: '["http://localhost:3366"]',
    tenant: TENANT,
    product: PRODUCT,
    name: TENANT,
    rawMetadata: fs.readFileSync(path.join(__dirname, '/data/metadata/boxyhq.xml'), 'utf8'),
  } as SAMLSSOConnectionWithRawMetadata);

  rawResponse = fs.readFileSync(path.join(__dirname, '/data/saml_response'), 'utf8').trim();
});

tap.teardown(async () => {
  process.exit(0);
});

tap.test('an assertion is single use', async (t) => {
  t.test('SP-initiated: the second post is refused', async (t) => {
    const { redirect_url } = (await oauth.authorize(<any>{
      tenant: TENANT,
      product: PRODUCT,
      client_id: `tenant=${TENANT}&product=${PRODUCT}`,
      redirect_uri: REDIRECT_URI,
      state: 'state-123',
    })) as { redirect_url: string };

    const relayState = new URLSearchParams(new URL(redirect_url).search).get('RelayState');

    t.ok(codeFrom((await post(relayState)).redirect_url), 'the first post is accepted');

    try {
      await post(relayState);
      t.fail('Expecting JacksonError.');
    } catch (err: any) {
      t.equal(err.message, 'Unable to validate state from the origin request.', 'the replay is refused');
    }
  });

  t.test('IdP-initiated: the second post is refused', async (t) => {
    t.ok(codeFrom((await post()).redirect_url), 'the first post is accepted');

    const secondCode = codeFrom((await post()).redirect_url);
    t.notOk(secondCode, `the replay is refused, got code: ${secondCode}`);
  });
});
ok  - SP-initiated: the second post is refused
not ok - the replay is refused, got code: 751b1e66390ab7b0f2c003c88ce534c723d18c95f1a789396350e170010f98c0.4a51...

I expected the second post to be rejected the way the SP-initiated one is.

Scope: IDP_ENABLED is off by default (lib/env.ts:65), the response has to be captured first, and the window is the assertion's NotOnOrAfter, which I did not measure. Low on its own, reported as a spec-conformance gap.

Technical details
  • Root cause: npm/src/controller/oauth.ts:827 builds validateOpts without an assertionReplayValidator, and ValidateOption (npm/src/saml/lib.ts:35-41) has no field for one.
  • SP-initiated path for comparison: the session delete at npm/src/controller/oauth.ts:892 already makes a second post fail, so only the IdP-initiated path is exposed.
  • Polis e13ed6541ec026dc37243f975b6d88be9488b687, @boxyhq/saml20 1.16.0, Node.js v26.7.0.
  • Fix I would suggest: a db.store('saml:assertion', opts.db.ttl) next to the session and code stores in npm/src/index.ts:101-103, threaded through as the validator and keyed by assertion ID. Happy to open a PR.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with npm/src/controller/oauth.ts:827 and npm/src/saml/lib.ts:35-41, then compare the existing SP-initiated handling near oauth.ts:892 and store setup in npm/src/index.ts:101-103. Use the proposed npm/test/sso/assertion_replay.test.ts and test/sso/saml_idp_oauth.test.ts as the starting tests; done means a repeated IdP-initiated assertion cannot mint another authorization code within its validity window.

Written by the indexing model from the issue text.

Assessment

Tech stack
node.js, typescript
Domain
authentication, backend-api-design, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.