diff --git a/src/features/directory/DirectorySetupPage.tsx b/src/features/directory/DirectorySetupPage.tsx index c6adf96..f8dd1ab 100644 --- a/src/features/directory/DirectorySetupPage.tsx +++ b/src/features/directory/DirectorySetupPage.tsx @@ -98,6 +98,7 @@ export function DirectorySetupPage() { bindPassword: '', allowInvalidCerts: false, issuer: '', + audience: '', usernameDomain: '', }); const [savedId, setSavedId] = useState(null); @@ -307,7 +308,7 @@ export function DirectorySetupPage() { ? /^ldaps?:\/\//.test(conn.url.trim()) && conn.baseDn.trim() !== '' && (savedId !== null || conn.bindDn.trim() === '' || conn.bindPassword !== '') - : /^https:\/\//.test(conn.issuer.trim()); + : /^https:\/\//.test(conn.issuer.trim()) && conn.audience.trim() !== ''; return ( {field('issuer', t('dirSetup.issuer', 'Issuer URL'), preset.example.issuer)} + {field( + 'audience', + t('dirSetup.audience', 'Client ID (audience)'), + 'inbuxa', + t( + 'dirSetup.audienceHint', + 'The client ID inbuxa is registered with at the provider. Only tokens issued for it sign in, not tokens the provider gave other applications.', + ), + )} {field( 'usernameDomain', t('dirSetup.usernameDomain', 'Domain for plain user names (optional)'), diff --git a/src/features/directory/presets.test.ts b/src/features/directory/presets.test.ts index 7103623..0a40f2e 100644 --- a/src/features/directory/presets.test.ts +++ b/src/features/directory/presets.test.ts @@ -14,6 +14,7 @@ const conn: Connection = { bindPassword: 'pw', allowInvalidCerts: false, issuer: '', + audience: '', usernameDomain: '', }; @@ -39,13 +40,18 @@ describe('directoryObject', () => { expect(directoryObject(presetById('openldap'), { ...conn, bindPassword: '' }, '')).not.toHaveProperty('bindSecret'); }); - it('builds an OIDC directory from the issuer', () => { + it('builds an OIDC directory from the issuer and the audience', () => { expect( - directoryObject(presetById('keycloak'), { ...conn, issuer: ' https://sso.example.org/realms/x ' }, 'KC'), + directoryObject( + presetById('keycloak'), + { ...conn, issuer: ' https://sso.example.org/realms/x ', audience: ' inbuxa ' }, + 'KC', + ), ).toEqual({ '@type': 'Oidc', description: 'KC', issuerUrl: 'https://sso.example.org/realms/x', + requireAudience: 'inbuxa', usernameDomain: null, claimUsername: 'email', claimName: 'name', diff --git a/src/features/directory/presets.ts b/src/features/directory/presets.ts index 095f880..52d99af 100644 --- a/src/features/directory/presets.ts +++ b/src/features/directory/presets.ts @@ -139,6 +139,8 @@ export interface Connection { bindPassword: string; allowInvalidCerts: boolean; issuer: string; + /** The client ID registered for this server at the provider: a token must name it as its audience. */ + audience: string; usernameDomain: string; } @@ -149,6 +151,7 @@ export function directoryObject(p: Preset, c: Connection, description: string): '@type': 'Oidc', description, issuerUrl: c.issuer.trim(), + requireAudience: c.audience.trim(), usernameDomain: c.usernameDomain.trim() || null, ...p.fields, }; diff --git a/src/features/security/checks.test.ts b/src/features/security/checks.test.ts index 8c059bd..308e040 100644 --- a/src/features/security/checks.test.ts +++ b/src/features/security/checks.test.ts @@ -179,4 +179,25 @@ describe('the checks (acceptance tests 1-8)', () => { const locked = { ...fresh(), metrics: { prometheus: { '@type': 'Enabled', authSecret: '********' } } }; expect(ids(locked)).not.toContain('SS-12'); }); + + it('OpenID Connect directories without an audience, or taking opaque tokens on user info, are flagged', () => { + const dir = { oidc: true, requireAudience: 'inbuxa', opaqueTokens: null }; + const s: Snapshot = { + ...fresh(), + directories: [ + { ...dir, id: 'a', name: 'Keycloak' }, + { ...dir, id: 'b', name: 'Old Keycloak', requireAudience: null, opaqueTokens: 'userinfo' }, + { ...dir, id: 'c', name: 'Introspected', opaqueTokens: 'introspection' }, + { ...dir, id: 'd', name: 'LDAP', oidc: false, requireAudience: null }, + ], + }; + const items = evaluate(s, defaults).items.filter((i) => i.check === 'SS-30' || i.check === 'SS-31'); + expect(items.map((i) => [i.check, i.subject])).toEqual([ + ['SS-30', 'b'], + ['SS-31', 'b'], + ]); + expect(items[0].title).toContain('Old Keycloak'); + expect(items[0].action).toEqual({ kind: 'review', viewName: 'x:Directory', id: 'b' }); + expect(ids(fresh())).not.toContain('SS-30'); + }); }); diff --git a/src/features/security/checks.ts b/src/features/security/checks.ts index cdcede0..ce04a43 100644 --- a/src/features/security/checks.ts +++ b/src/features/security/checks.ts @@ -86,6 +86,17 @@ export interface DomainSnapshot { dmarcPolicy: string | null; } +/** An external directory, as SS-30 and SS-31 read it. */ +export interface DirectorySnapshot { + id: string; + name: string; + /** OpenID Connect: the only kind these checks look at. */ + oidc: boolean; + requireAudience: string | null; + /** reject, userinfo or introspection; null when not set. */ + opaqueTokens: string | null; +} + export interface Snapshot { now: number; imap?: { allowPlainTextAuth?: boolean }; @@ -101,6 +112,7 @@ export interface Snapshot { metrics?: { prometheus?: { '@type'?: string; authSecret?: unknown } }; mtaSts?: { mode?: string }; domains?: DomainSnapshot[]; + directories?: DirectorySnapshot[]; /** The legacy-protocols switch: true while any legacy protocol is on. */ legacyOn?: boolean; /** Why a part couldn't be read, by part name. */ @@ -122,7 +134,8 @@ export type Part = | 'mtaSts' | 'domains' | 'dns' - | 'legacy'; + | 'legacy' + | 'directories'; /** The schema's defaults, as the console knows them. */ export type Defaults = (object: string, field: string) => unknown; @@ -551,6 +564,36 @@ export function evaluate(s: Snapshot, defaults: Defaults): Result { check('SS-14', 'Certificates expiring within 30 days', ['certificates'], certItems(30 * DAY, 'important', 'SS-14')); + // SS-30 and SS-31 are numbered past the spec's requirements (SS-19 to + // SS-29), which share the numbering. + check('SS-30', 'OpenID Connect directories without an audience', ['directories'], () => + (s.directories ?? []) + .filter((d) => d.oidc && !d.requireAudience) + .map((d) => ({ + check: 'SS-30', + severity: 'important' as const, + subject: d.id, + value: null, + title: `${d.name || d.id} accepts tokens issued to other applications`, + why: 'It names no required audience, so a token its provider issued to any other application signs people in here. Set it to the client ID inbuxa is registered with at the provider. A later release will require it.', + action: { kind: 'review' as const, viewName: 'x:Directory', id: d.id }, + })), + ); + + check('SS-31', 'Opaque tokens taken on the user info endpoint’s word', ['directories'], () => + (s.directories ?? []) + .filter((d) => d.oidc && d.opaqueTokens === 'userinfo') + .map((d) => ({ + check: 'SS-31', + severity: 'important' as const, + subject: d.id, + value: 'userinfo', + title: `${d.name || d.id} accepts any token its provider issued`, + why: 'A token that isn’t a JWT signs in whenever the provider’s user info endpoint answers for it, and it does for tokens issued to any application. Check them by introspection, with the client ID and secret inbuxa has at the provider, or reject them.', + action: { kind: 'review' as const, viewName: 'x:Directory', id: d.id }, + })), + ); + // Good practice check('SS-15', 'Legacy mail protocols', ['legacy'], () => diff --git a/src/features/security/load.ts b/src/features/security/load.ts index 76b621e..ecb07ab 100644 --- a/src/features/security/load.ts +++ b/src/features/security/load.ts @@ -64,6 +64,12 @@ const CALLS: Call[] = [ { part: 'senderAuth', method: 'x:SenderAuth/get', ids: ['singleton'], properties: ['dmarcVerify'] }, { part: 'metrics', method: 'x:Metrics/get', ids: ['singleton'], properties: ['prometheus'] }, { part: 'mtaSts', method: 'x:MtaSts/get', ids: ['singleton'], properties: ['mode'] }, + { + part: 'directories', + method: 'x:Directory/get', + ids: null, + properties: ['@type', 'description', 'requireAudience', 'opaqueTokens'], + }, { part: 'domains', method: 'x:Domain/get', @@ -166,6 +172,15 @@ export async function loadSnapshot(signal?: AbortSignal, critical = false): Prom }; }); } + if (lists.directories) { + snapshot.directories = lists.directories.map((d) => ({ + id: String(d.id), + name: String(d.description ?? ''), + oidc: d['@type'] === 'Oidc', + requireAudience: typeof d.requireAudience === 'string' ? d.requireAudience : null, + opaqueTokens: typeof d.opaqueTokens === 'string' ? d.opaqueTokens : null, + })); + } if (lists.tlsStrategies) { snapshot.tlsStrategies = lists.tlsStrategies.map((t) => ({ id: String(t.id), diff --git a/src/help/securityChecks.test.ts b/src/help/securityChecks.test.ts index 4a7c921..18cecfe 100644 --- a/src/help/securityChecks.test.ts +++ b/src/help/securityChecks.test.ts @@ -24,6 +24,7 @@ const PARTS: Part[] = [ 'domains', 'dns', 'legacy', + 'directories', ]; describe('security page help', () => { diff --git a/src/help/securityChecks.ts b/src/help/securityChecks.ts index 6697e65..a6d675d 100644 --- a/src/help/securityChecks.ts +++ b/src/help/securityChecks.ts @@ -103,6 +103,16 @@ export const SECURITY_CHECK_HELP: { check: string; label: string; text: string } label: 'DMARC policies', text: 'Good practice for each domain whose DMARC record says p=none. Once DMARC reports show your own mail passing, move to quarantine or reject at your DNS host.', }, + { + check: 'SS-30', + label: 'OpenID Connect directories without an audience', + text: 'Important for each OpenID Connect directory saved without a required audience (a new one can’t be). A token its provider issued to any other application signs people in here. Review opens the directory: set the audience to the client ID inbuxa is registered with at the provider.', + }, + { + check: 'SS-31', + label: 'Opaque tokens taken on the user info endpoint’s word', + text: 'Important for each OpenID Connect directory that accepts tokens which aren’t JWTs whenever the provider’s user info endpoint answers for them, as directories saved before the choice existed do. Review opens the directory: choose introspection, with inbuxa’s client ID and secret at the provider, or reject.', + }, ]; export const SECURITY_PAGE_HELP: PageHelp = { diff --git a/src/i18n/en.json b/src/i18n/en.json index 30db239..9d06be6 100644 --- a/src/i18n/en.json +++ b/src/i18n/en.json @@ -887,7 +887,9 @@ "defaultMoves": "Every other domain without its own directory signs in through {{dir}}.", "done": "Sign-in now goes through {{dir}}.", "doneHint": "Have someone sign in to the webmail with their directory password. Their account is created on first sign-in, with the name, aliases and groups the directory gives.", - "tabTitle": "Sign-in directory" + "tabTitle": "Sign-in directory", + "audience": "Client ID (audience)", + "audienceHint": "The client ID inbuxa is registered with at the provider. Only tokens issued for it sign in, not tokens the provider gave other applications." }, "held": { "noSubject": "(no subject)",