Phase 10 — Admin Role & Settings (ADMIN-01/02/03) #17

Merged
luckberg merged 25 commits from gsd/phase-10-admin-role-settings into main 2026-06-13 17:10:47 -04:00
8 changed files with 89 additions and 56 deletions
Showing only changes of commit 79fe3e0e04 - Show all commits
+31 -30
View File
@@ -167,35 +167,36 @@ const meNoEchoHook = (result: { success: boolean }, c: Context) => {
} }
}; };
meRouter.post( meRouter.post('/credential', zValidator('json', meCredentialSchema, meNoEchoHook), async (c) => {
'/credential', // Pitfall 6: ALWAYS resolve currentUserId from the session — never from the body.
zValidator('json', meCredentialSchema, meNoEchoHook), const currentUserId = await resolveUserId(c);
async (c) => { if (!currentUserId) {
// Pitfall 6: ALWAYS resolve currentUserId from the session — never from the body. return c.json({ error: 'Unauthorized' }, 401);
const currentUserId = await resolveUserId(c); }
if (!currentUserId) {
return c.json({ error: 'Unauthorized' }, 401); const { fastmailEmail, appPassword, providerType } = c.req.valid('json');
// T-10-10: NEVER log appPassword or c.req.valid('json') here
try {
// D-07: identical validate→encrypt→store→sync path as admin, but always with
// currentUserId (not a body userId). Admin passes the target member's userId;
// self-service passes the authenticated session userId. Same helper, same argument order.
await validateEncryptAndStoreCredential(
currentUserId,
fastmailEmail,
appPassword,
providerType,
);
} catch (err) {
if (err instanceof CredentialValidationError) {
return c.json({ error: 'Invalid request' }, 400);
} }
console.error(
'[me/POST /credential] Unexpected error:',
err instanceof Error ? err.message : String(err),
);
return c.json({ error: 'Service unavailable' }, 503);
}
const { fastmailEmail, appPassword, providerType } = c.req.valid('json'); return c.json({ ok: true }, 200);
// T-10-10: NEVER log appPassword or c.req.valid('json') here });
try {
// D-07: identical validate→encrypt→store→sync path as admin, but always with
// currentUserId (not a body userId). Admin passes the target member's userId;
// self-service passes the authenticated session userId. Same helper, same argument order.
await validateEncryptAndStoreCredential(currentUserId, fastmailEmail, appPassword, providerType);
} catch (err) {
if (err instanceof CredentialValidationError) {
return c.json({ error: 'Invalid request' }, 400);
}
console.error(
'[me/POST /credential] Unexpected error:',
err instanceof Error ? err.message : String(err),
);
return c.json({ error: 'Service unavailable' }, 503);
}
return c.json({ ok: true }, 200);
},
);
+18 -2
View File
@@ -314,7 +314,15 @@ describe('upsertUser', () => {
} }
// Re-fetch after insert // Re-fetch after insert
return makeSelectChain([ return makeSelectChain([
{ id: 10, oidcIss: iss, oidcSub: sub, displayName: null, color: COLOR_PALETTE[0], isAdmin: true, createdAt: new Date() }, {
id: 10,
oidcIss: iss,
oidcSub: sub,
displayName: null,
color: COLOR_PALETTE[0],
isAdmin: true,
createdAt: new Date(),
},
]); ]);
}); });
mockDb.insert.mockReturnValue(makeInsertChain([{ id: 10 }])); mockDb.insert.mockReturnValue(makeInsertChain([{ id: 10 }]));
@@ -344,7 +352,15 @@ describe('upsertUser', () => {
return makeSelectChain([{ count: 1 }]); return makeSelectChain([{ count: 1 }]);
} }
return makeSelectChain([ return makeSelectChain([
{ id: 11, oidcIss: iss, oidcSub: sub, displayName: null, color: COLOR_PALETTE[1], isAdmin: false, createdAt: new Date() }, {
id: 11,
oidcIss: iss,
oidcSub: sub,
displayName: null,
color: COLOR_PALETTE[1],
isAdmin: false,
createdAt: new Date(),
},
]); ]);
}); });
mockDb.insert.mockReturnValue(makeInsertChain([{ id: 11 }])); mockDb.insert.mockReturnValue(makeInsertChain([{ id: 11 }]));
+7 -1
View File
@@ -24,7 +24,13 @@ vi.mock('../../src/db/client.js', () => ({
// Bring in the ContextVariableMap augmentation (sets up c.get('user') typing) // Bring in the ContextVariableMap augmentation (sets up c.get('user') typing)
vi.mock('../../src/auth/devBypass.js', () => ({ vi.mock('../../src/auth/devBypass.js', () => ({
DEV_USER: { id: 1, oidcIss: 'dev', oidcSub: 'dev-user', displayName: 'Dev User', color: '#4A90D9' }, DEV_USER: {
id: 1,
oidcIss: 'dev',
oidcSub: 'dev-user',
displayName: 'Dev User',
color: '#4A90D9',
},
devAuthBypass: () => async (_c: unknown, next: () => Promise<void>) => next(), devAuthBypass: () => async (_c: unknown, next: () => Promise<void>) => next(),
COLOR_PALETTE: ['#4A90D9'], COLOR_PALETTE: ['#4A90D9'],
})); }));
+24 -13
View File
@@ -167,13 +167,16 @@ describe('GET /api/me — isAdmin + needsProviderSetup (Plan 10-02, D-03)', () =
let callCount = 0; let callCount = 0;
vi.mocked(db.select).mockImplementation(() => { vi.mocked(db.select).mockImplementation(() => {
callCount++; callCount++;
const limitFn = callCount === 1 const limitFn =
? vi.fn().mockResolvedValue([{ isAdmin: true }]) // users.isAdmin lookup callCount === 1
: vi.fn().mockResolvedValue([]); // memberCredentials lookup (none) ? vi.fn().mockResolvedValue([{ isAdmin: true }]) // users.isAdmin lookup
: vi.fn().mockResolvedValue([]); // memberCredentials lookup (none)
return { return {
from: vi.fn().mockReturnValue({ from: vi.fn().mockReturnValue({
where: vi.fn().mockReturnValue({ limit: limitFn }), where: vi.fn().mockReturnValue({ limit: limitFn }),
innerJoin: vi.fn().mockReturnValue({ innerJoin: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue([]) }) }), innerJoin: vi.fn().mockReturnValue({
innerJoin: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue([]) }),
}),
}), }),
// eslint-disable-next-line @typescript-eslint/no-explicit-any // eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any; } as any;
@@ -183,7 +186,9 @@ describe('GET /api/me — isAdmin + needsProviderSetup (Plan 10-02, D-03)', () =
const res = await app.request('/api/me'); const res = await app.request('/api/me');
expect(res.status).toBe(200); expect(res.status).toBe(200);
const body = (await res.json()) as { user: { id: number; isAdmin: boolean; needsProviderSetup: boolean } }; const body = (await res.json()) as {
user: { id: number; isAdmin: boolean; needsProviderSetup: boolean };
};
expect(body.user).toHaveProperty('isAdmin'); expect(body.user).toHaveProperty('isAdmin');
expect(body.user.isAdmin).toBe(true); // DB returns true, not hardcoded expect(body.user.isAdmin).toBe(true); // DB returns true, not hardcoded
}); });
@@ -194,13 +199,16 @@ describe('GET /api/me — isAdmin + needsProviderSetup (Plan 10-02, D-03)', () =
let callCount = 0; let callCount = 0;
vi.mocked(db.select).mockImplementation(() => { vi.mocked(db.select).mockImplementation(() => {
callCount++; callCount++;
const limitFn = callCount === 1 const limitFn =
? vi.fn().mockResolvedValue([{ isAdmin: false }]) // users.isAdmin lookup callCount === 1
: vi.fn().mockResolvedValue([]); // no member_credentials row ? vi.fn().mockResolvedValue([{ isAdmin: false }]) // users.isAdmin lookup
: vi.fn().mockResolvedValue([]); // no member_credentials row
return { return {
from: vi.fn().mockReturnValue({ from: vi.fn().mockReturnValue({
where: vi.fn().mockReturnValue({ limit: limitFn }), where: vi.fn().mockReturnValue({ limit: limitFn }),
innerJoin: vi.fn().mockReturnValue({ innerJoin: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue([]) }) }), innerJoin: vi.fn().mockReturnValue({
innerJoin: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue([]) }),
}),
}), }),
// eslint-disable-next-line @typescript-eslint/no-explicit-any // eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any; } as any;
@@ -221,13 +229,16 @@ describe('GET /api/me — isAdmin + needsProviderSetup (Plan 10-02, D-03)', () =
let callCount = 0; let callCount = 0;
vi.mocked(db.select).mockImplementation(() => { vi.mocked(db.select).mockImplementation(() => {
callCount++; callCount++;
const limitFn = callCount === 1 const limitFn =
? vi.fn().mockResolvedValue([{ isAdmin: false }]) // users.isAdmin lookup callCount === 1
: vi.fn().mockResolvedValue([{ id: 7 }]); // has member_credentials row ? vi.fn().mockResolvedValue([{ isAdmin: false }]) // users.isAdmin lookup
: vi.fn().mockResolvedValue([{ id: 7 }]); // has member_credentials row
return { return {
from: vi.fn().mockReturnValue({ from: vi.fn().mockReturnValue({
where: vi.fn().mockReturnValue({ limit: limitFn }), where: vi.fn().mockReturnValue({ limit: limitFn }),
innerJoin: vi.fn().mockReturnValue({ innerJoin: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue([]) }) }), innerJoin: vi.fn().mockReturnValue({
innerJoin: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue([]) }),
}),
}), }),
// eslint-disable-next-line @typescript-eslint/no-explicit-any // eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any; } as any;
+1 -3
View File
@@ -100,9 +100,7 @@ test.describe('Non-admin user — admin nav entry hidden + /admin redirect', ()
await page.waitForURL(/\/calendar/, { timeout: 10_000 }); await page.waitForURL(/\/calendar/, { timeout: 10_000 });
// Should have landed on /calendar // Should have landed on /calendar
const url = new URL(page.url()); const url = new URL(page.url());
expect(url.pathname, `Expected /calendar but got ${url.pathname}`).toMatch( expect(url.pathname, `Expected /calendar but got ${url.pathname}`).toMatch(/^\/(calendar)?$/);
/^\/(calendar)?$/,
);
// "Admin Settings" heading must NOT be present // "Admin Settings" heading must NOT be present
await expect(page.getByRole('heading', { name: 'Admin Settings' })).toHaveCount(0); await expect(page.getByRole('heading', { name: 'Admin Settings' })).toHaveCount(0);
}); });
+3 -1
View File
@@ -296,7 +296,9 @@ export function CredentialSheet({
id="credential-helper" id="credential-helper"
style={{ style={{
fontSize: 'var(--text-label-size, 13px)', fontSize: 'var(--text-label-size, 13px)',
color: validationError ? 'var(--color-destructive, #DC2626)' : 'var(--color-text-secondary)', color: validationError
? 'var(--color-destructive, #DC2626)'
: 'var(--color-text-secondary)',
lineHeight: 1.4, lineHeight: 1.4,
marginBottom: 'var(--space-6, 24px)', marginBottom: 'var(--space-6, 24px)',
display: 'flex', display: 'flex',
+2 -1
View File
@@ -30,6 +30,7 @@ import { CredentialSheet } from './CredentialSheet.js';
export function SetupBanner() { export function SetupBanner() {
const [sheetOpen, setSheetOpen] = useState(false); const [sheetOpen, setSheetOpen] = useState(false);
// Use HTMLButtonElement for the ref (assignable to the CredentialSheet's HTMLElement trigger)
const ctaRef = useRef<HTMLButtonElement>(null); const ctaRef = useRef<HTMLButtonElement>(null);
const meQuery = useQuery({ const meQuery = useQuery({
@@ -124,7 +125,7 @@ export function SetupBanner() {
onClose={() => setSheetOpen(false)} onClose={() => setSheetOpen(false)}
mode="self-service" mode="self-service"
memberName={memberName} memberName={memberName}
triggerRef={ctaRef as React.RefObject<HTMLElement | null>} triggerRef={ctaRef}
/> />
</> </>
); );
+3 -5
View File
@@ -77,8 +77,7 @@ export function AdminPage() {
}); });
// Derive current saved shared calendar id from the data // Derive current saved shared calendar id from the data
const currentSharedId = const currentSharedId = calendarsQuery.data?.calendars.find((c) => c.isShared)?.id ?? null;
calendarsQuery.data?.calendars.find((c) => c.isShared)?.id ?? null;
// Effective selected = user pick OR fallback to current saved // Effective selected = user pick OR fallback to current saved
const effectiveSelected = selectedCalendarId ?? currentSharedId; const effectiveSelected = selectedCalendarId ?? currentSharedId;
@@ -97,8 +96,7 @@ export function AdminPage() {
// Open credential sheet for a member // Open credential sheet for a member
function openSheet(member: AdminMember, buttonRef: React.RefObject<HTMLButtonElement | null>) { function openSheet(member: AdminMember, buttonRef: React.RefObject<HTMLButtonElement | null>) {
// Capture the button so focus can return on close // Capture the button so focus can return on close
(triggerRef as React.MutableRefObject<HTMLElement | null>).current = (triggerRef as React.MutableRefObject<HTMLElement | null>).current = buttonRef.current;
buttonRef.current;
setSheetMember(member); setSheetMember(member);
setSheetMode(member.hasCredential ? 'admin-rotate' : 'admin-add'); setSheetMode(member.hasCredential ? 'admin-rotate' : 'admin-add');
setSheetOpen(true); setSheetOpen(true);
@@ -298,7 +296,7 @@ export function AdminPage() {
mode={sheetMode} mode={sheetMode}
memberName={sheetMember.displayName} memberName={sheetMember.displayName}
memberId={sheetMember.id} memberId={sheetMember.id}
triggerRef={triggerRef as React.RefObject<HTMLElement | null>} triggerRef={triggerRef}
/> />
)} )}
</div> </div>