fix(sso): preserve admin-assigned roles across SSO sign-in (#1862)

* fix(sso): preserve admin-assigned roles across SSO sign-in

An SSO/OIDC/LDAP user's role was overwritten by the IdP-derived role on
every sign-in, so a role an admin assigned in Settings > Users reverted to
the provider default on the next login. Gate role re-sync behind an opt-in
sso_role_sync setting (default off), so manual role edits persist unless the
operator explicitly enables IdP-authoritative sync. Email continues to sync
unconditionally.

Adds human-session-only GET/PUT /api/sso/config/role-sync endpoints with a
hub-side API-token rejection in the remote proxy, a frontend toggle, a
regenerated SSO settings screenshot, and matching docs.

Closes #1851

* fix(sso): satisfy CodeQL on role-sync log and test token hashing

Route three inline API-token creation blocks through the shared
createTestApiToken helper so the sha256 hashing lives in one place, and
log the role-sync toggle as a word instead of a raw boolean. No behavior
change; resolves the CodeQL js/insecure-hashing and log-injection alerts.

* fix(sso): harden role-sync gate, name the toggle, fix screenshot

Addresses pre-merge review findings on the SSO role-sync feature:
- Make the hub-side SSO config authz guard case-insensitive to match
  Express routing semantics, closing a case-variant API-token bypass.
- Give the IdP role-sync switch an accessible name.
- Capture the SSO settings screenshot at desktop size with the scroll
  area expanded so the role-sync control is fully visible.
This commit is contained in:
Anso
2026-08-28 13:26:22 +00:00
committed by GitHub
parent cc4a6571c7
commit 7cd42699d1
16 changed files with 852 additions and 51 deletions
+3
View File
@@ -133,6 +133,9 @@ describe('API token blocked endpoints', () => {
{ method: 'put', path: '/api/sso/config/ldap', body: { enabled: true } },
{ method: 'delete', path: '/api/sso/config/ldap' },
{ method: 'post', path: '/api/sso/config/ldap/test' },
// SSO role sync configuration
{ method: 'get', path: '/api/sso/config/role-sync' },
{ method: 'put', path: '/api/sso/config/role-sync', body: { enabled: true } },
// Node management
{ method: 'post', path: '/api/nodes', body: { name: 'test', type: 'local' } },
{ method: 'put', path: '/api/nodes/1', body: { name: 'updated' } },
@@ -0,0 +1,156 @@
/**
* Hub → remote proxy coverage for SSO configuration API-token rejection.
* The remote proxy replaces incoming credentials with a node-to-node JWT,
* so destination-side rejectApiTokenScope cannot identify the original API
* token. This test proves that the hub-side guard in remoteNodeProxy blocks
* full-admin API tokens targeting /api/sso/config on remote nodes.
*/
import { describe, it, expect, beforeAll, afterAll, beforeEach } from 'vitest';
import http from 'http';
import request from 'supertest';
import { setupTestDb, cleanupTestDb, TEST_JWT_SECRET } from './helpers/setupTestDb';
import jwt from 'jsonwebtoken';
import { createTestApiToken } from './helpers/apiTokenTestHelper';
import { PROXY_ROLE_HEADER } from '../services/license-headers';
let tmpDir: string;
let app: import('express').Express;
let adminToken: string;
let fullAdminApiToken: string;
let remoteNodeId: number;
interface CapturedHop {
method: string;
url: string;
roleHeader: string | undefined;
}
const capturedHops: CapturedHop[] = [];
function captureHop(req: http.IncomingMessage, into: CapturedHop[]): void {
into.push({
method: req.method ?? '',
url: req.url ?? '',
roleHeader: req.headers[PROXY_ROLE_HEADER] as string | undefined,
});
}
function createRemoteServer(): http.Server {
return http.createServer((req, res) => {
// Always serve /api/meta so the proxy can discover capabilities
if (req.url?.startsWith('/api/meta')) {
res.writeHead(200, { 'Content-Type': 'application/json' });
res.end(JSON.stringify({ version: '0.97.1', capabilities: ['cross-node-rbac'] }));
return;
}
captureHop(req, capturedHops);
res.writeHead(200, { 'Content-Type': 'application/json' });
res.end(JSON.stringify({ success: true }));
});
}
async function listen(server: http.Server): Promise<number> {
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve));
return (server.address() as import('net').AddressInfo).port;
}
let remoteServer: http.Server;
let remotePort: number;
beforeAll(async () => {
tmpDir = await setupTestDb();
({ app } = await import('../index'));
const { DatabaseService } = await import('../services/DatabaseService');
const db = DatabaseService.getInstance();
adminToken = jwt.sign({ username: 'testadmin', role: 'admin' }, TEST_JWT_SECRET, { expiresIn: '1h' });
// Create a full-admin API token via the shared test helper
const admin = db.getUserByUsername('testadmin');
fullAdminApiToken = createTestApiToken({
db: DatabaseService,
scope: 'full-admin',
userId: admin!.id,
name: `sso-config-proxy-test-${Date.now()}`,
});
// Set up the remote server
remoteServer = createRemoteServer();
remotePort = await listen(remoteServer);
remoteNodeId = db.addNode({
name: 'sso-config-remote',
type: 'remote',
mode: 'proxy',
compose_dir: '/tmp',
is_default: false,
api_url: `http://127.0.0.1:${remotePort}`,
api_token: 'remote-sso-token',
});
});
afterAll(async () => {
await new Promise<void>((resolve) => remoteServer.close(() => resolve()));
cleanupTestDb(tmpDir);
});
beforeEach(() => {
capturedHops.length = 0;
});
describe('Hub-side API-token rejection for remote SSO config', () => {
const blockedRequests: Array<{ method: 'get' | 'put'; path: string; body?: Record<string, unknown> }> = [
{ method: 'get', path: '/api/sso/config/role-sync' },
{ method: 'get', path: '/api/sso/config' },
{ method: 'put', path: '/api/sso/config/role-sync', body: { enabled: true } },
{ method: 'get', path: '/api/sso/config/ldap' },
{ method: 'put', path: '/api/sso/config/ldap', body: { enabled: true } },
// Case variants: Express routes case-insensitively, so the hub guard must
// reject them too (regression for a case-sensitive guard bypass).
{ method: 'get', path: '/api/SSO/config/role-sync' },
{ method: 'put', path: '/api/SSO/config/role-sync', body: { enabled: true } },
{ method: 'get', path: '/api/Sso/Config/Role-Sync' },
{ method: 'put', path: '/api/Sso/Config/Role-Sync', body: { enabled: true } },
];
for (const { method, path, body } of blockedRequests) {
it(`returns 403 SCOPE_DENIED for ${method.toUpperCase()} ${path}, no upstream hop`, async () => {
let req = request(app)[method](path)
.set('Authorization', `Bearer ${fullAdminApiToken}`)
.set('x-node-id', String(remoteNodeId));
if (body) req = req.send(body);
const res = await req;
expect(res.status).toBe(403);
expect(res.body.code).toBe('SCOPE_DENIED');
expect(capturedHops).toHaveLength(0);
});
}
it('Browser admin session reaches upstream for GET /api/sso/config/role-sync', async () => {
const res = await request(app)
.get('/api/sso/config/role-sync')
.set('Authorization', `Bearer ${adminToken}`)
.set('x-node-id', String(remoteNodeId));
expect(res.status).toBe(200);
const hop = capturedHops.find((h) => h.url?.includes('/sso/config/role-sync'));
expect(hop).toBeDefined();
expect(hop!.method).toBe('GET');
expect(hop!.roleHeader).toBe('admin');
});
it('Browser admin session reaches upstream for PUT /api/sso/config/role-sync', async () => {
const res = await request(app)
.put('/api/sso/config/role-sync')
.set('Authorization', `Bearer ${adminToken}`)
.set('x-node-id', String(remoteNodeId))
.send({ enabled: true });
expect(res.status).toBe(200);
const hop = capturedHops.find((h) => h.url?.includes('/sso/config/role-sync'));
expect(hop).toBeDefined();
expect(hop!.method).toBe('PUT');
expect(hop!.roleHeader).toBe('admin');
});
});
+286 -39
View File
@@ -1,10 +1,11 @@
import { describe, it, expect, beforeAll, afterAll, vi, type MockInstance } from 'vitest';
import { describe, it, expect, beforeAll, afterAll, beforeEach, afterEach, vi, type MockInstance } 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';
import { generateApiToken } from '../utils/apiTokenFormat';
import { createTestApiToken } from './helpers/apiTokenTestHelper';
let tmpDir: string;
let app: Express;
@@ -368,56 +369,138 @@ describe('Database migration - SSO columns', () => {
});
describe('SSO Role Sync on Re-Login', () => {
afterAll(() => {
afterEach(async () => {
// Ensure sso_role_sync is reset to default between tests so the
// enabled-case does not leak into subsequent suites.
const { DatabaseService } = await import('../services/DatabaseService');
DatabaseService.getInstance().updateGlobalSetting('sso_role_sync', '0');
vi.restoreAllMocks();
});
it('provisionUser promotes user when IdP role changes to admin', async () => {
const { SSOService } = await import('../services/SSOService');
const sso = SSOService.getInstance();
// Create a viewer
const user1 = sso.provisionUser({
authProvider: 'oidc_okta',
providerId: 'okta-role-sync-test',
preferredUsername: 'rolesync_user',
email: 'rolesync@example.com',
role: 'viewer',
});
expect(user1.role).toBe('viewer');
// Re-login with admin role from IdP
const user2 = sso.provisionUser({
authProvider: 'oidc_okta',
providerId: 'okta-role-sync-test',
preferredUsername: 'rolesync_user',
email: 'rolesync@example.com',
role: 'admin',
});
expect(user2.id).toBe(user1.id);
expect(user2.role).toBe('admin');
});
it('provisionUser demotes user when IdP role changes to viewer', async () => {
it('provisionUser preserves existing role when sso_role_sync setting is missing', async () => {
const { SSOService } = await import('../services/SSOService');
const { DatabaseService } = await import('../services/DatabaseService');
const sso = SSOService.getInstance();
const db = DatabaseService.getInstance();
// Look up user from previous test (should be admin now)
const existing = db.getUserByProviderIdentity('oidc_okta', 'okta-role-sync-test');
expect(existing).toBeDefined();
expect(existing!.role).toBe('admin');
// Spy: return settings without sso_role_sync to simulate a missing key
const realSettings = { ...db.getGlobalSettings() };
delete realSettings['sso_role_sync'];
vi.spyOn(db, 'getGlobalSettings').mockReturnValue(Object.freeze(realSettings));
// Re-login with viewer role (e.g., removed from admin group)
const user = sso.provisionUser({
// Create a viewer
const user1 = sso.provisionUser({
authProvider: 'oidc_okta',
providerId: 'okta-role-sync-test',
preferredUsername: 'rolesync_user',
email: 'rolesync@example.com',
providerId: 'okta-role-sync-missing',
preferredUsername: 'rolesync_missing',
email: 'rolesync_missing@example.com',
role: 'viewer',
});
expect(user.role).toBe('viewer');
expect(user1.role).toBe('viewer');
// Re-login with admin role from IdP - should NOT overwrite when sync is off
const user2 = sso.provisionUser({
authProvider: 'oidc_okta',
providerId: 'okta-role-sync-missing',
preferredUsername: 'rolesync_missing',
email: 'rolesync_missing_new@example.com',
role: 'admin',
});
expect(user2.id).toBe(user1.id);
expect(user2.role).toBe('viewer'); // preserved, not promoted
expect(user2.email).toBe('rolesync_missing_new@example.com'); // email still syncs
});
it('provisionUser preserves existing role when sso_role_sync is off', async () => {
const { SSOService } = await import('../services/SSOService');
const { DatabaseService } = await import('../services/DatabaseService');
const sso = SSOService.getInstance();
const db = DatabaseService.getInstance();
// Explicitly set to '0'
db.updateGlobalSetting('sso_role_sync', '0');
// Create a viewer
const user1 = sso.provisionUser({
authProvider: 'oidc_okta',
providerId: 'okta-role-sync-off',
preferredUsername: 'rolesync_off',
email: 'rolesync_off@example.com',
role: 'viewer',
});
expect(user1.role).toBe('viewer');
// Re-login with admin role - should NOT overwrite when sync is off
const user2 = sso.provisionUser({
authProvider: 'oidc_okta',
providerId: 'okta-role-sync-off',
preferredUsername: 'rolesync_off',
email: 'rolesync_off_new@example.com',
role: 'admin',
});
expect(user2.id).toBe(user1.id);
expect(user2.role).toBe('viewer'); // preserved
expect(user2.email).toBe('rolesync_off_new@example.com'); // email still syncs
});
it('provisionUser applies IdP role when sso_role_sync is enabled', async () => {
const { SSOService } = await import('../services/SSOService');
const { DatabaseService } = await import('../services/DatabaseService');
const sso = SSOService.getInstance();
const db = DatabaseService.getInstance();
// Enable sync
db.updateGlobalSetting('sso_role_sync', '1');
// Create a viewer
const user1 = sso.provisionUser({
authProvider: 'oidc_okta',
providerId: 'okta-role-sync-on',
preferredUsername: 'rolesync_on',
email: 'rolesync_on@example.com',
role: 'viewer',
});
expect(user1.role).toBe('viewer');
// Re-login with admin role - SHOULD overwrite when sync is enabled
const user2 = sso.provisionUser({
authProvider: 'oidc_okta',
providerId: 'okta-role-sync-on',
preferredUsername: 'rolesync_on',
email: 'rolesync_on_new@example.com',
role: 'admin',
});
expect(user2.id).toBe(user1.id);
expect(user2.role).toBe('admin'); // promoted
expect(user2.email).toBe('rolesync_on_new@example.com'); // email still syncs
});
it('provisionUser applies IdP role demotion when sso_role_sync is enabled', async () => {
const { SSOService } = await import('../services/SSOService');
const { DatabaseService } = await import('../services/DatabaseService');
const sso = SSOService.getInstance();
const db = DatabaseService.getInstance();
db.updateGlobalSetting('sso_role_sync', '1');
// Create an admin
const user1 = sso.provisionUser({
authProvider: 'oidc_okta',
providerId: 'okta-role-sync-demote',
preferredUsername: 'rolesync_demote',
email: 'rolesync_demote@example.com',
role: 'admin',
});
expect(user1.role).toBe('admin');
// Re-login with viewer role (removed from admin group) - SHOULD demote
const user = sso.provisionUser({
authProvider: 'oidc_okta',
providerId: 'okta-role-sync-demote',
preferredUsername: 'rolesync_demote',
email: 'rolesync_demote@example.com',
role: 'viewer',
});
expect(user.role).toBe('viewer'); // demoted
});
});
@@ -633,6 +716,170 @@ describe('SSO Config - API Token Denied', () => {
});
});
describe('SSO Role Sync Config Endpoints', () => {
let viewerToken: string;
beforeAll(async () => {
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' });
});
beforeEach(async () => {
// Ensure order independence: reset to default before each test
const { DatabaseService } = await import('../services/DatabaseService');
DatabaseService.getInstance().updateGlobalSetting('sso_role_sync', '0');
});
afterEach(() => {
// Restore any spies after each test
vi.restoreAllMocks();
});
it('Administrator GET returns { enabled: false } initially', async () => {
const res = await supertest(app)
.get('/api/sso/config/role-sync')
.set('Authorization', `Bearer ${adminToken}`);
expect(res.status).toBe(200);
expect(res.body).toEqual({ enabled: false });
});
it('Administrator PUT persists and returns { success: true }', async () => {
const res = await supertest(app)
.put('/api/sso/config/role-sync')
.set('Authorization', `Bearer ${adminToken}`)
.send({ enabled: true });
expect(res.status).toBe(200);
expect(res.body).toEqual({ success: true });
});
it('Subsequent GET returns { enabled: true } after PUT', async () => {
// PUT first
await supertest(app)
.put('/api/sso/config/role-sync')
.set('Authorization', `Bearer ${adminToken}`)
.send({ enabled: true });
// Then GET
const res = await supertest(app)
.get('/api/sso/config/role-sync')
.set('Authorization', `Bearer ${adminToken}`);
expect(res.status).toBe(200);
expect(res.body).toEqual({ enabled: true });
});
it('PUT with missing body returns 400', async () => {
const res = await supertest(app)
.put('/api/sso/config/role-sync')
.set('Authorization', `Bearer ${adminToken}`)
.send({});
expect(res.status).toBe(400);
expect(res.body.error).toBe('enabled must be a boolean');
});
it('PUT with non-boolean enabled returns 400', async () => {
const res = await supertest(app)
.put('/api/sso/config/role-sync')
.set('Authorization', `Bearer ${adminToken}`)
.send({ enabled: 'yes' });
expect(res.status).toBe(400);
expect(res.body.error).toBe('enabled must be a boolean');
});
it('PUT with null enabled returns 400', async () => {
const res = await supertest(app)
.put('/api/sso/config/role-sync')
.set('Authorization', `Bearer ${adminToken}`)
.send({ enabled: null });
expect(res.status).toBe(400);
expect(res.body.error).toBe('enabled must be a boolean');
});
it('Unauthenticated GET returns 401', async () => {
const res = await supertest(app).get('/api/sso/config/role-sync');
expect(res.status).toBe(401);
});
it('Unauthenticated PUT returns 401', async () => {
const res = await supertest(app)
.put('/api/sso/config/role-sync')
.send({ enabled: true });
expect(res.status).toBe(401);
});
it('Viewer GET returns 403 ADMIN_REQUIRED', async () => {
const res = await supertest(app)
.get('/api/sso/config/role-sync')
.set('Authorization', `Bearer ${viewerToken}`);
expect(res.status).toBe(403);
expect(res.body.code).toBe('ADMIN_REQUIRED');
});
it('Viewer PUT returns 403 ADMIN_REQUIRED', async () => {
const res = await supertest(app)
.put('/api/sso/config/role-sync')
.set('Authorization', `Bearer ${viewerToken}`)
.send({ enabled: true });
expect(res.status).toBe(403);
expect(res.body.code).toBe('ADMIN_REQUIRED');
});
it('Direct API token GET returns 403 SCOPE_DENIED', async () => {
const { DatabaseService } = await import('../services/DatabaseService');
const db = DatabaseService.getInstance();
const admin = db.getUserByUsername('testadmin');
const rawToken = createTestApiToken({
db: DatabaseService,
scope: 'full-admin',
userId: admin!.id,
name: `role-sync-test-${Date.now()}`,
});
const res = await supertest(app)
.get('/api/sso/config/role-sync')
.set('Authorization', `Bearer ${rawToken}`);
expect(res.status).toBe(403);
expect(res.body.code).toBe('SCOPE_DENIED');
});
it('Direct API token PUT returns 403 SCOPE_DENIED', async () => {
const { DatabaseService } = await import('../services/DatabaseService');
const db = DatabaseService.getInstance();
const admin = db.getUserByUsername('testadmin');
const rawToken = createTestApiToken({
db: DatabaseService,
scope: 'full-admin',
userId: admin!.id,
name: `role-sync-put-test-${Date.now()}`,
});
const res = await supertest(app)
.put('/api/sso/config/role-sync')
.set('Authorization', `Bearer ${rawToken}`)
.send({ enabled: true });
expect(res.status).toBe(403);
expect(res.body.code).toBe('SCOPE_DENIED');
});
it('Database failure returns controlled 500 response on PUT', async () => {
const { DatabaseService } = await import('../services/DatabaseService');
const db = DatabaseService.getInstance();
// Spy on updateGlobalSetting (only called in the route handler, not auth
// middleware) to simulate a database write failure.
vi.spyOn(db, 'updateGlobalSetting').mockImplementation(() => {
throw new Error('DB write failure');
});
const res = await supertest(app)
.put('/api/sso/config/role-sync')
.set('Authorization', `Bearer ${adminToken}`)
.send({ enabled: true });
expect(res.status).toBe(500);
expect(res.body.error).toBe('Failed to update role-sync setting');
});
});
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)
+13
View File
@@ -50,6 +50,7 @@ import {
} from '../middleware/permissions';
import type { PermissionAction } from '../middleware/permissions';
import { SETTING_WRITE_PERMISSIONS } from '../routes/settings';
import { rejectApiTokenScope } from '../middleware/apiTokenScope';
/**
* Per-request hop timing for the critical hydration GETs, kept off the Request
@@ -378,6 +379,18 @@ export function createRemoteProxyMiddleware(): RequestHandler {
return;
}
// SSO configuration routes are human-session-only. The destination-side
// rejectApiTokenScope cannot detect the original API token because this
// proxy replaces incoming credentials with a node-to-node JWT. Reject
// API-token-authenticated requests here, covering the /sso/config collection
// and all descendant paths (req.path is post-/api strip). Express matches
// routes case-insensitively, so this guard must too (the i flag).
if (/^\/sso\/config(?:\/|$)/i.test(req.path)) {
if (rejectApiTokenScope(req, res, 'API tokens cannot access SSO configuration.')) {
return;
}
}
const node = NodeRegistry.getInstance().getNode(req.nodeId);
if (!node || node.type !== 'remote') {
next();
+25
View File
@@ -41,6 +41,31 @@ ssoConfigRouter.get('/', (req: Request, res: Response): void => {
}
});
ssoConfigRouter.get('/role-sync', (req: Request, res: Response): void => {
if (rejectApiTokenScope(req, res, SSO_SCOPE_MESSAGE)) return;
if (!requireAdmin(req, res)) return;
const enabled = DatabaseService.getInstance().getGlobalSettings()['sso_role_sync'] === '1';
res.json({ enabled });
});
ssoConfigRouter.put('/role-sync', (req: Request, res: Response): void => {
if (rejectApiTokenScope(req, res, SSO_SCOPE_MESSAGE)) return;
if (!requireAdmin(req, res)) return;
const enabled = req.body?.enabled;
if (typeof enabled !== 'boolean') {
res.status(400).json({ error: 'enabled must be a boolean' });
return;
}
try {
DatabaseService.getInstance().updateGlobalSetting('sso_role_sync', enabled ? '1' : '0');
console.log(`[SSO] role-sync updated: ${enabled ? 'enabled' : 'disabled'}`);
res.json({ success: true });
} catch (error) {
console.error('[SSO] Failed to update role-sync setting:', error);
res.status(500).json({ error: 'Failed to update role-sync setting' });
}
});
ssoConfigRouter.get('/:provider', (req: Request, res: Response): void => {
if (rejectApiTokenScope(req, res, SSO_SCOPE_MESSAGE)) return;
if (!requireAdmin(req, res)) return;
+4 -1
View File
@@ -2111,7 +2111,10 @@ export class DatabaseService {
// behave); admins who want a strict absolute session ceiling can turn
// it off in Settings > Users.
stmt.run('session_sliding_refresh', '1');
stmt.run('gitops_schema_version', '1');
stmt.run('gitops_schema_version', '1');
// SSO role sync defaults off: admin-set roles persist across SSO sign-ins;
// operators who want IdP group membership to drive roles opt in via Settings > SSO.
stmt.run('sso_role_sync', '0');
// Seed the default local node if none exists
const nodeCount = (this.db.prepare('SELECT COUNT(*) as count FROM nodes').get() as any)?.count || 0;
+7 -2
View File
@@ -601,8 +601,13 @@ export class SSOService {
updates.email = params.email;
}
// Sync role from identity provider on every login
if (params.role !== existing.role) {
// Preserve the stored role by default; IdP role changes are opt-in
// (sso_role_sync). By default the role assigned at first provisioning is
// preserved so an admin's manual edit in Settings → Users survives
// subsequent sign-ins (issue #1851). When sso_role_sync is '1', the
// provider-derived role is applied on each login. Email, in contrast, is
// synced whenever it changed, regardless of sso_role_sync.
if (db.getGlobalSettings()['sso_role_sync'] === '1' && params.role !== existing.role) {
updates.role = params.role;
}