fix(sso): harden Custom OIDC provider and SSO configuration (#630)

- Add Host header injection validation for OAuth callback URL derivation
  when SSO_CALLBACK_URL is not set (extracted into shared getSSOBaseUrl helper)
- Add startup warning when OIDC providers are enabled without SSO_CALLBACK_URL
- Add diagnostic logging for claim fallback, email changes on re-login,
  and admin seat limit enforcement (gated behind Developer Mode)
- Add Custom OIDC environment variables to .env.example
- Fix stale AdmiralGate comment in SSOSection.tsx
- Fix .env.example section header referencing removed Admiral tier gate
- Fix docs: correct Custom OIDC display name default, replace nonexistent
  DEBUG=true env var reference with Developer Mode toggle
- Add tests for role enforcement (viewer 403), API token scope denial,
  OIDC claim edge cases, Custom OIDC test connection, callback error params
- Refresh SSO settings screenshots
This commit is contained in:
Anso
2026-04-16 08:10:28 -04:00
committed by GitHub
parent 9036096c02
commit 6890224903
10 changed files with 179 additions and 7 deletions
+12 -1
View File
@@ -29,7 +29,7 @@ API_RATE_LIMIT=200
# Increase for environments with many concurrent browser sessions behind shared NAT.
API_POLLING_RATE_LIMIT=300
# ─── SSO / LDAP Configuration (Admiral) ───────────────────────────
# ─── SSO / LDAP Configuration ────────────────────────────────────
# LDAP / Active Directory
SSO_LDAP_ENABLED=false
@@ -59,6 +59,17 @@ SSO_OIDC_OKTA_ISSUER_URL=
SSO_OIDC_OKTA_CLIENT_ID=
SSO_OIDC_OKTA_CLIENT_SECRET=
# Custom OIDC (Keycloak, Authentik, Authelia, Zitadel, etc.)
SSO_OIDC_CUSTOM_ENABLED=false
SSO_OIDC_CUSTOM_DISPLAY_NAME=Custom OIDC
SSO_OIDC_CUSTOM_ISSUER_URL=
SSO_OIDC_CUSTOM_CLIENT_ID=
SSO_OIDC_CUSTOM_CLIENT_SECRET=
SSO_OIDC_CUSTOM_SCOPES=openid email profile
SSO_OIDC_CUSTOM_ID_CLAIM=
SSO_OIDC_CUSTOM_USERNAME_CLAIM=
SSO_OIDC_CUSTOM_EMAIL_CLAIM=
# Role mapping (shared across OIDC providers)
SSO_OIDC_ADMIN_CLAIM=groups
SSO_OIDC_ADMIN_CLAIM_VALUE=sencho-admins
+117
View File
@@ -2,6 +2,7 @@ import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest';
import { setupTestDb, cleanupTestDb, TEST_JWT_SECRET } from './helpers/setupTestDb';
import supertest from 'supertest';
import jwt from 'jsonwebtoken';
import crypto from 'crypto';
import type { Express } from 'express';
let tmpDir: string;
@@ -505,4 +506,120 @@ describe('SSO Claim Mapping', () => {
// Claim present but no match
expect(resolve({ roles: 'user-role' }, { oidcAdminClaim: 'roles', oidcAdminClaimValue: 'admin-role', oidcDefaultRole: 'viewer' })).toBe('viewer');
});
it('resolveRoleFromOidc handles edge cases', async () => {
const { SSOService } = await import('../services/SSOService');
const sso = SSOService.getInstance();
const resolve = (sso as unknown as {
resolveRoleFromOidc: (userInfo: Record<string, unknown>, config: { oidcAdminClaim?: string; oidcAdminClaimValue?: string; oidcDefaultRole?: string }) => string;
}).resolveRoleFromOidc.bind(sso);
// Empty oidcAdminClaimValue falls back to default 'sencho-admins' via || operator,
// so a matching claim still resolves to admin
expect(resolve({ groups: ['sencho-admins'] }, { oidcAdminClaim: 'groups', oidcAdminClaimValue: '', oidcDefaultRole: 'viewer' })).toBe('admin');
// Claim exists but is an empty array (no match possible)
expect(resolve({ groups: [] }, { oidcAdminClaim: 'groups', oidcAdminClaimValue: 'sencho-admins', oidcDefaultRole: 'viewer' })).toBe('viewer');
// Claim exists but is an empty string (no match)
expect(resolve({ groups: '' }, { oidcAdminClaim: 'groups', oidcAdminClaimValue: 'sencho-admins', oidcDefaultRole: 'viewer' })).toBe('viewer');
// Claim value is a number (should be coerced to string for comparison)
expect(resolve({ role_id: 42 }, { oidcAdminClaim: 'role_id', oidcAdminClaimValue: '42', oidcDefaultRole: 'viewer' })).toBe('admin');
});
});
describe('SSO Config - Role Enforcement', () => {
let viewerToken: string;
beforeAll(async () => {
// Create a viewer user in the DB so the auth middleware can resolve it
const { DatabaseService } = await import('../services/DatabaseService');
const db = DatabaseService.getInstance();
if (!db.getUserByUsername('sso_test_viewer')) {
db.addUser({ username: 'sso_test_viewer', password_hash: '$2b$10$fake', role: 'viewer' });
}
viewerToken = jwt.sign({ username: 'sso_test_viewer', role: 'viewer' }, TEST_JWT_SECRET, { expiresIn: '1h' });
});
it('GET /api/sso/config returns 403 for viewer role', async () => {
const res = await supertest(app)
.get('/api/sso/config')
.set('Authorization', `Bearer ${viewerToken}`);
expect(res.status).toBe(403);
expect(res.body.code).toBe('ADMIN_REQUIRED');
});
it('PUT /api/sso/config/:provider returns 403 for viewer role', async () => {
const res = await supertest(app)
.put('/api/sso/config/ldap')
.set('Authorization', `Bearer ${viewerToken}`)
.send({ enabled: false });
expect(res.status).toBe(403);
expect(res.body.code).toBe('ADMIN_REQUIRED');
});
it('DELETE /api/sso/config/:provider returns 403 for viewer role', async () => {
const res = await supertest(app).delete('/api/sso/config/ldap').set('Authorization', `Bearer ${viewerToken}`);
expect(res.status).toBe(403);
expect(res.body.code).toBe('ADMIN_REQUIRED');
});
});
describe('SSO Config - API Token Denied', () => {
let apiRawToken: string;
beforeAll(async () => {
const { DatabaseService } = await import('../services/DatabaseService');
const db = DatabaseService.getInstance();
apiRawToken = jwt.sign({ scope: 'api_token', jti: crypto.randomUUID() }, TEST_JWT_SECRET, { expiresIn: '1h' });
const tokenHash = crypto.createHash('sha256').update(apiRawToken).digest('hex');
const admin = db.getUserByUsername('testadmin');
db.addApiToken({ token_hash: tokenHash, name: `sso-scope-${Date.now()}`, scope: 'full-admin', user_id: admin!.id, created_at: Date.now(), expires_at: null });
});
it('GET /api/sso/config returns 403 SCOPE_DENIED for API token', async () => {
const res = await supertest(app)
.get('/api/sso/config')
.set('Authorization', `Bearer ${apiRawToken}`);
expect(res.status).toBe(403);
expect(res.body.code).toBe('SCOPE_DENIED');
});
it('PUT /api/sso/config/:provider returns 403 SCOPE_DENIED for API token', async () => {
const res = await supertest(app)
.put('/api/sso/config/ldap')
.set('Authorization', `Bearer ${apiRawToken}`)
.send({ enabled: false });
expect(res.status).toBe(403);
expect(res.body.code).toBe('SCOPE_DENIED');
});
});
describe('SSO Test Connection - Custom OIDC', () => {
it('POST /api/sso/config/oidc_custom/test returns failure when not configured', async () => {
const res = await supertest(app)
.post('/api/sso/config/oidc_custom/test')
.set('Authorization', `Bearer ${adminToken}`);
expect(res.status).toBe(200);
expect(res.body.success).toBe(false);
expect(res.body.error).toBeDefined();
});
});
describe('SSO OIDC Callback - Additional Error Handling', () => {
it('handles error param without error_description', async () => {
const res = await supertest(app)
.get('/api/auth/sso/oidc/oidc_google/callback?error=server_error');
expect(res.status).toBe(302);
expect(res.headers.location).toContain('sso_error');
expect(res.headers.location).toContain('server_error');
});
it('handles error with description for oidc_custom provider', async () => {
const res = await supertest(app)
.get('/api/auth/sso/oidc/oidc_custom/callback?error=consent_required&error_description=User+must+consent');
expect(res.status).toBe(302);
expect(res.headers.location).toContain('User');
});
});
+18 -2
View File
@@ -858,6 +858,20 @@ const ssoRateLimiter = rateLimit({
message: { error: 'Too many SSO attempts. Please try again later.' },
});
/** Derive the OAuth callback base URL from SSO_CALLBACK_URL or the request Host header, with injection validation. */
function getSSOBaseUrl(req: Request, res: Response): string | null {
const host = req.get('host') || '';
if (!process.env.SSO_CALLBACK_URL && /[\s<>\r\n]/.test(host)) {
console.error('[SSO] Rejected suspicious Host header');
res.redirect('/?sso_error=Invalid+request');
return null;
}
if (!process.env.SSO_CALLBACK_URL && isDebugEnabled()) {
console.debug('[SSO:debug] SSO_CALLBACK_URL not set; using Host header for callback URL:', host);
}
return process.env.SSO_CALLBACK_URL || `${req.protocol}://${host}`;
}
// List enabled SSO providers (for login page)
app.get('/api/auth/sso/providers', (_req: Request, res: Response): void => {
try {
@@ -929,7 +943,8 @@ app.get('/api/auth/sso/oidc/:provider/authorize', ssoRateLimiter, async (req: Re
return;
}
const baseUrl = process.env.SSO_CALLBACK_URL || `${req.protocol}://${req.get('host')}`;
const baseUrl = getSSOBaseUrl(req, res);
if (!baseUrl) return;
const callbackUrl = `${baseUrl}/api/auth/sso/oidc/${provider}/callback`;
const { url, state, codeVerifier } = await SSOService.getInstance().getOIDCAuthorizationUrl(provider, callbackUrl);
@@ -995,7 +1010,8 @@ app.get('/api/auth/sso/oidc/:provider/callback', ssoRateLimiter, async (req: Req
return;
}
const baseUrl = process.env.SSO_CALLBACK_URL || `${req.protocol}://${req.get('host')}`;
const baseUrl = getSSOBaseUrl(req, res);
if (!baseUrl) return;
const callbackUrl = `${baseUrl}/api/auth/sso/oidc/${provider}/callback`;
const result = await SSOService.getInstance().handleOIDCCallback(
+29 -1
View File
@@ -91,6 +91,15 @@ export class SSOService {
this.seedOidcFromEnv('oidc_github', 'SSO_OIDC_GITHUB');
this.seedOidcFromEnv('oidc_okta', 'SSO_OIDC_OKTA');
this.seedOidcFromEnv('oidc_custom', 'SSO_OIDC_CUSTOM');
// Warn if OIDC providers are enabled but SSO_CALLBACK_URL is not set
if (!process.env.SSO_CALLBACK_URL) {
const enabled = DatabaseService.getInstance().getEnabledSSOConfigs();
const hasOidc = enabled.some(c => c.provider !== 'ldap');
if (hasOidc) {
console.warn('[SSO] SSO_CALLBACK_URL is not set. OAuth callback URLs will be derived from the request Host header. Set SSO_CALLBACK_URL to your external URL for deployments behind a reverse proxy.');
}
}
}
private seedLdapFromEnv(): void {
@@ -430,6 +439,18 @@ export class SSOService {
const usernameClaimName = config.oidcUsernameClaim || 'preferred_username';
const emailClaimName = config.oidcEmailClaim || 'email';
if (isDebugEnabled()) {
if (config.oidcIdClaim && !(config.oidcIdClaim in userInfo)) {
console.debug(`[SSO:debug] Custom ID claim '${config.oidcIdClaim}' not found in token; falling back to 'sub'`);
}
if (config.oidcUsernameClaim && !(config.oidcUsernameClaim in userInfo)) {
console.debug(`[SSO:debug] Custom username claim '${config.oidcUsernameClaim}' not found in token; falling back to standard claims`);
}
if (config.oidcEmailClaim && !(config.oidcEmailClaim in userInfo)) {
console.debug(`[SSO:debug] Custom email claim '${config.oidcEmailClaim}' not found in token; falling back to 'email'`);
}
}
const sub = String(userInfo[idClaimName] ?? userInfo.sub ?? userInfo.id ?? '');
if (!sub) {
return { success: false, error: 'Could not determine user identity from provider' };
@@ -559,6 +580,10 @@ export class SSOService {
// Update email if changed
if (params.email && params.email !== existing.email) {
if (debug) console.debug('[SSO:debug] Email changed on re-login', {
userId: existing.id, username: existing.username,
oldEmail: existing.email || '(none)', newEmail: params.email,
});
updates.email = params.email;
}
@@ -568,8 +593,11 @@ export class SSOService {
const seatLimits = LicenseService.getInstance().getSeatLimits();
if (seatLimits.maxAdmins === null || db.getAdminCount() < seatLimits.maxAdmins) {
updates.role = params.role;
} else if (debug) {
console.debug('[SSO:debug] Admin seat limit reached; keeping current role for existing user', {
userId: existing.id, username: existing.username,
});
}
// If admin seats full, keep current role rather than upgrading
} else {
// Always allow demotion (e.g., removed from admin group)
updates.role = params.role;
+2 -2
View File
@@ -192,7 +192,7 @@ Environment variables are useful for initial deployment or infrastructure-as-cod
| Variable | Default | Description |
|----------|---------|-------------|
| `SSO_OIDC_CUSTOM_ENABLED` | `false` | Enable the custom OIDC provider |
| `SSO_OIDC_CUSTOM_DISPLAY_NAME` | `oidc_custom` | Label shown on the login button |
| `SSO_OIDC_CUSTOM_DISPLAY_NAME` | `Custom OIDC` | Label shown on the login button |
| `SSO_OIDC_CUSTOM_ISSUER_URL` | - | OIDC issuer URL (the base of the discovery endpoint) |
| `SSO_OIDC_CUSTOM_CLIENT_ID` | - | OAuth client ID from your identity provider |
| `SSO_OIDC_CUSTOM_CLIENT_SECRET` | - | OAuth client secret (encrypted at rest) |
@@ -298,7 +298,7 @@ Fix: set the Issuer URL to exactly match the `issuer` field returned by your pro
If users are created with incorrect usernames or missing emails:
- Enable debug mode (`DEBUG=true` environment variable) to see the raw claims Sencho receives from the provider
- Enable **Developer Mode** (Settings > Developer Mode toggle) to see the raw claims Sencho receives from the provider in the server logs
- Check your provider's documentation for which claims it includes in the ID token and userinfo response
- Verify that the scopes you configured include the necessary permissions (some providers require explicit `profile` or `email` scopes)
- Set the appropriate claim names in the Custom OIDC claim mapping fields
Binary file not shown.

Before

Width:  |  Height:  |  Size: 143 KiB

After

Width:  |  Height:  |  Size: 70 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 34 KiB

After

Width:  |  Height:  |  Size: 75 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 21 KiB

After

Width:  |  Height:  |  Size: 67 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 128 KiB

After

Width:  |  Height:  |  Size: 62 KiB

+1 -1
View File
@@ -386,7 +386,7 @@ export function SSOSection() {
try {
const res = await apiFetch('/sso/config');
if (res.ok) setConfigs(await res.json());
} catch { /* ignore - AdmiralGate will handle non-pro */ }
} catch { /* ignore fetch errors */ }
};
// eslint-disable-next-line react-hooks/set-state-in-effect