diff --git a/CHANGELOG.md b/CHANGELOG.md index d5f80e0df..0e1514aef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,6 +72,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `automod` categories - Command directory loading now ignores `*.spec.*` and `*.test.*` modules so test files are never registered as slash commands +- Dashboard guild authorization now tolerates per-guild context failures + instead of dropping the full `/api/guilds` response when one guild fails +- Discord guild permission parsing now supports payload drift + (`permissions`/`permissions_new`) and safely handles invalid permission values +- `/api/guilds` now maps recoverable Discord OAuth/scope/session failures to + actionable auth responses (401/403) and maps upstream Discord outages to 502 +- `GET /api/guilds/:id/me` no longer requires `overview` module access so the + dashboard can always bootstrap member context for authorized users +- Server selector now distinguishes true empty authorization from + fetch/auth/session failures, showing retry and re-auth actions for failure + states instead of a misleading empty result +- Features route guard mapping is now consistent under the `automation` module + across frontend route guards, sidebar module checks, and backend route guards +- `/servers` is now always accessible for authenticated users (not blocked by + module RBAC guards), while server/module pages remain module-gated +- Guild auto-selection now picks the first server where Lucky is already added; + when no server has Lucky installed, dashboard keeps no selected server and + shows explicit selection guidance +- Refs: PR `#169` ### Changed diff --git a/README.md b/README.md index 67b528602..6bf06b0e9 100644 --- a/README.md +++ b/README.md @@ -86,8 +86,11 @@ packages/ - Neo-editorial shell with responsive sidebar and contextual page framing - Dashboard, Servers, and Last.fm pages aligned to shared status/empty-state primitives - Module/command toggle per server -- Guild RBAC by Discord role (`view`/`manage`) with deny-by-default for - non-admin users +- Guild RBAC by Discord role (`view`/`manage`) with hybrid fallback: + owners/admin/manage-server users keep baseline access when grants are absent, + while role grants control non-admin module access +- `/servers` remains available to authenticated users even when module-level + access is restricted, so server discovery/invite flows stay reachable - Sidebar identity resolution chain: `nick > globalName > username` - Dashboard guild metrics now use live bot/API counts, rendering unknown values as `—` instead of `0` @@ -181,6 +184,16 @@ deployments should keep `VITE_API_BASE_URL` aligned with the public backend. Authenticated frontend shell routes now bootstrap guild selection globally, so the server selector is populated immediately after login without requiring a visit to `/servers` first. +Guild auto-selection prioritizes the first server where Lucky is already added; +if none are bot-added, the dashboard keeps no selected server and shows a clear +selection/empty guidance state. +Server selector empty/error states are split: +- `No accessible servers found` means authentication worked but no authorized + guilds matched your access policy. +- `Could not load servers` means auth/session/network/upstream fetch failed; + use `Retry` or `Re-authenticate` from the selector. +- `Select a Server` in dashboard after login means no bot-added server was + auto-selected yet; open `/servers` to invite Lucky to additional servers. Without `VITE_API_BASE_URL`, frontend uses same-origin `/api` for `*.lucassantana.tech` hosts and `api.luk-homeserver.com.br` for `*.luk-homeserver.com.br`. diff --git a/packages/backend/src/routes/guilds.ts b/packages/backend/src/routes/guilds.ts index 489c4cc0b..d78e4bf25 100644 --- a/packages/backend/src/routes/guilds.ts +++ b/packages/backend/src/routes/guilds.ts @@ -25,14 +25,70 @@ async function getSessionData(req: AuthenticatedRequest) { return sessionData } +function getStatusCode(error: unknown): number | null { + if (typeof error !== 'object' || error === null) { + return null + } + + const errorObject = error as { statusCode?: unknown; status?: unknown } + if (typeof errorObject.statusCode === 'number') { + return errorObject.statusCode + } + + if (typeof errorObject.status === 'number') { + return errorObject.status + } + + return null +} + +function mapGuildAccessError(error: unknown): Error { + if (error instanceof AppError) { + return error + } + + const statusCode = getStatusCode(error) + + if (statusCode === 401) { + return AppError.unauthorized( + 'Discord session expired. Please sign in again.', + ) + } + + if (statusCode === 403) { + return AppError.forbidden( + 'Discord OAuth scope is missing. Re-authenticate and try again.', + ) + } + + if (statusCode === 429 || (statusCode !== null && statusCode >= 500)) { + return new AppError(502, 'Discord API is temporarily unavailable.') + } + + return error instanceof Error + ? error + : new Error('Internal server error') +} + +async function runGuildAccessOperation( + operation: () => Promise, +): Promise { + try { + return await operation() + } catch (error) { + throw mapGuildAccessError(error) + } +} + export function setupGuildRoutes(app: Express): void { app.get( '/api/guilds', requireAuth, asyncHandler(async (req: AuthenticatedRequest, res: Response) => { const sessionData = await getSessionData(req) - const guilds = - await guildAccessService.listAuthorizedGuilds(sessionData) + const guilds = await runGuildAccessOperation(() => + guildAccessService.listAuthorizedGuilds(sessionData), + ) res.json({ guilds }) }), @@ -46,8 +102,9 @@ export function setupGuildRoutes(app: Express): void { const id = getGuildId(req) const sessionData = await getSessionData(req) - const guilds = - await guildAccessService.listAuthorizedGuilds(sessionData) + const guilds = await runGuildAccessOperation(() => + guildAccessService.listAuthorizedGuilds(sessionData), + ) const guildDetails = guilds.find((guild) => guild.id === id) if (!guildDetails) { @@ -79,7 +136,9 @@ export function setupGuildRoutes(app: Express): void { const guildContext = req.guildContext ?? - (await guildAccessService.resolveGuildContext(sessionData, id)) + (await runGuildAccessOperation(() => + guildAccessService.resolveGuildContext(sessionData, id), + )) if (!guildContext) { throw AppError.forbidden('No access to this server') } diff --git a/packages/backend/src/services/DiscordOAuthService.ts b/packages/backend/src/services/DiscordOAuthService.ts index 4e0442031..ad0b6a644 100644 --- a/packages/backend/src/services/DiscordOAuthService.ts +++ b/packages/backend/src/services/DiscordOAuthService.ts @@ -16,9 +16,21 @@ export interface DiscordGuild { icon: string | null owner: boolean permissions: string + permissions_new?: string features: string[] } +export class DiscordApiError extends Error { + constructor( + message: string, + public readonly statusCode: number, + public readonly endpoint: string, + ) { + super(message) + this.name = 'DiscordApiError' + } +} + interface TokenResponse { access_token: string token_type: string @@ -30,6 +42,80 @@ interface TokenResponse { class DiscordOAuthService { private readonly apiBaseUrl = 'https://discord.com/api/v10' + private normalizePermissionValue(value: unknown): string | null { + if (typeof value === 'string') { + const normalized = value.trim() + return normalized.length > 0 ? normalized : null + } + + if ( + typeof value === 'number' && + Number.isFinite(value) && + value >= 0 && + Number.isInteger(value) + ) { + return String(value) + } + + return null + } + + private parsePermissionBits( + permissions: string | null | undefined, + ): bigint | null { + const normalized = this.normalizePermissionValue(permissions) + if (!normalized) { + return null + } + + try { + return BigInt(normalized) + } catch { + return null + } + } + + private normalizeGuildPayload(payload: unknown): DiscordGuild[] { + if (!Array.isArray(payload)) { + return [] + } + + const guilds: DiscordGuild[] = [] + + for (const rawGuild of payload) { + if (typeof rawGuild !== 'object' || rawGuild === null) { + continue + } + + const guild = rawGuild as Record + if (typeof guild.id !== 'string' || typeof guild.name !== 'string') { + continue + } + + const permissions = this.normalizePermissionValue(guild.permissions) + const permissionsNew = this.normalizePermissionValue( + guild.permissions_new, + ) + + guilds.push({ + id: guild.id, + name: guild.name, + icon: typeof guild.icon === 'string' ? guild.icon : null, + owner: guild.owner === true, + permissions: permissions ?? permissionsNew ?? '0', + permissions_new: permissionsNew ?? undefined, + features: Array.isArray(guild.features) + ? guild.features.filter( + (feature): feature is string => + typeof feature === 'string', + ) + : [], + }) + } + + return guilds + } + private getClientId(): string { const clientId = process.env.CLIENT_ID if (!clientId) { @@ -74,8 +160,10 @@ class DiscordOAuthService { if (!response.ok) { const errorText = await response.text() - throw new Error( + throw new DiscordApiError( `Token exchange failed: ${response.status} ${errorText}`, + response.status, + '/oauth2/token', ) } @@ -98,8 +186,10 @@ class DiscordOAuthService { if (!response.ok) { const errorText = await response.text() - throw new Error( + throw new DiscordApiError( `Failed to fetch user info: ${response.status} ${errorText}`, + response.status, + '/users/@me', ) } @@ -128,12 +218,14 @@ class DiscordOAuthService { if (!response.ok) { const errorText = await response.text() - throw new Error( + throw new DiscordApiError( `Failed to fetch user guilds: ${response.status} ${errorText}`, + response.status, + '/users/@me/guilds', ) } - const guilds = (await response.json()) as DiscordGuild[] + const guilds = this.normalizeGuildPayload(await response.json()) debugLog({ message: 'Successfully fetched user guilds', data: { count: guilds.length }, @@ -145,8 +237,18 @@ class DiscordOAuthService { } } - hasAdminPermission(permissions: string): boolean { - const permissionsBigInt = BigInt(permissions) + hasAdminPermission( + permissions: string | null | undefined, + permissionsNew?: string | null, + ): boolean { + const permissionsBigInt = + this.parsePermissionBits(permissionsNew) ?? + this.parsePermissionBits(permissions) + + if (permissionsBigInt === null) { + return false + } + const administratorPermission = BigInt(0x8) const manageGuildPermission = BigInt(0x20) @@ -160,7 +262,7 @@ class DiscordOAuthService { filterAdminGuilds(guilds: DiscordGuild[]): DiscordGuild[] { return guilds.filter((guild) => - this.hasAdminPermission(guild.permissions), + this.hasAdminPermission(guild.permissions, guild.permissions_new), ) } @@ -181,8 +283,10 @@ class DiscordOAuthService { if (!response.ok) { const errorText = await response.text() - throw new Error( + throw new DiscordApiError( `Token refresh failed: ${response.status} ${errorText}`, + response.status, + '/oauth2/token', ) } diff --git a/packages/backend/src/services/GuildAccessService.ts b/packages/backend/src/services/GuildAccessService.ts index 6cf5031c8..78aa29584 100644 --- a/packages/backend/src/services/GuildAccessService.ts +++ b/packages/backend/src/services/GuildAccessService.ts @@ -4,9 +4,15 @@ import { type EffectiveAccessMap, type ModuleKey, } from '@lucky/shared/services' -import { discordOAuthService, type DiscordGuild } from './DiscordOAuthService' +import { errorLog } from '@lucky/shared/utils' +import { + DiscordApiError, + discordOAuthService, + type DiscordGuild, +} from './DiscordOAuthService' import { guildService, type GuildWithBotStatus } from './GuildService' import type { SessionData } from './SessionService' +import { AppError } from '../errors/AppError' export interface GuildAccessContext { guildId: string @@ -25,10 +31,61 @@ export interface AuthorizedGuild extends GuildWithBotStatus { } class GuildAccessService { + private extractStatusCode(error: unknown): number | null { + if (error instanceof DiscordApiError) { + return error.statusCode + } + + if (typeof error === 'object' && error !== null) { + const errorObject = error as { + statusCode?: unknown + status?: unknown + } + + if (typeof errorObject.statusCode === 'number') { + return errorObject.statusCode + } + + if (typeof errorObject.status === 'number') { + return errorObject.status + } + } + + return null + } + private async fetchUserGuilds( accessToken: string, ): Promise { - return discordOAuthService.getUserGuilds(accessToken) + try { + return await discordOAuthService.getUserGuilds(accessToken) + } catch (error) { + const statusCode = this.extractStatusCode(error) + + if (statusCode === 401) { + throw AppError.unauthorized( + 'Discord session expired. Please sign in again.', + ) + } + + if (statusCode === 403) { + throw AppError.forbidden( + 'Discord OAuth scope is missing. Re-authenticate and try again.', + ) + } + + if ( + statusCode === 429 || + (statusCode !== null && statusCode >= 500) + ) { + throw new AppError( + 502, + 'Discord API is temporarily unavailable. Please retry.', + ) + } + + throw error + } } private async buildContext( @@ -37,11 +94,32 @@ class GuildAccessService { ): Promise { const isAdmin = guild.owner || - discordOAuthService.hasAdminPermission(guild.permissions) - const hasBot = await guildService.hasBotInGuild(guild.id) + discordOAuthService.hasAdminPermission( + guild.permissions, + guild.permissions_new, + ) + + const hasBot = await guildService.hasBotInGuild(guild.id).catch((error) => { + errorLog({ + message: 'Failed to resolve bot presence for guild access', + error, + data: { guildId: guild.id }, + }) + throw error + }) + const memberContext = hasBot && !isAdmin - ? await guildService.getGuildMemberContext(guild.id, userId) + ? await guildService + .getGuildMemberContext(guild.id, userId) + .catch((error) => { + errorLog({ + message: 'Failed to resolve guild member context', + error, + data: { guildId: guild.id, userId }, + }) + throw error + }) : { nickname: null, roleIds: [] as string[] } const effectiveAccess = @@ -80,10 +158,31 @@ class GuildAccessService { ): Promise { const guilds = await this.fetchUserGuilds(session.accessToken) const contexts = await Promise.all( - guilds.map((guild) => this.buildContext(guild, session.user.id)), + guilds.map(async (guild) => { + try { + return await this.buildContext(guild, session.user.id) + } catch (error) { + errorLog({ + message: 'Skipping guild due access context failure', + error, + data: { guildId: guild.id, userId: session.user.id }, + }) + return null + } + }), ) - const authorizedContexts = contexts.filter((context) => + const resolvedContexts = contexts.filter( + (context): context is GuildAccessContext => context !== null, + ) + if (guilds.length > 0 && resolvedContexts.length === 0) { + throw new AppError( + 502, + 'Unable to resolve server access right now. Please retry.', + ) + } + + const authorizedContexts = resolvedContexts.filter((context) => this.isAuthorized(context), ) const authorizedContextByGuildId = new Map( diff --git a/packages/backend/tests/unit/services/DiscordOAuthService.test.ts b/packages/backend/tests/unit/services/DiscordOAuthService.test.ts index 60c19c333..cc0ab76f2 100644 --- a/packages/backend/tests/unit/services/DiscordOAuthService.test.ts +++ b/packages/backend/tests/unit/services/DiscordOAuthService.test.ts @@ -246,6 +246,12 @@ describe('DiscordOAuthService', () => { true, ) }) + + test('should return false when permission payload is invalid', () => { + expect( + discordOAuthService.hasAdminPermission('not-a-number'), + ).toBe(false) + }) }) describe('filterAdminGuilds', () => { @@ -282,5 +288,22 @@ describe('DiscordOAuthService', () => { expect(result).toHaveLength(2) }) + + test('should use permissions_new when permissions is stale', () => { + const guilds = [ + { + ...MOCK_DISCORD_GUILDS[0], + permissions: '0', + permissions_new: '32', + } as (typeof MOCK_DISCORD_GUILDS)[number] & { + permissions_new: string + }, + ] + + const result = discordOAuthService.filterAdminGuilds(guilds) + + expect(result).toHaveLength(1) + expect(result[0].id).toBe(MOCK_DISCORD_GUILDS[0].id) + }) }) }) diff --git a/packages/backend/tests/unit/services/GuildAccessService.test.ts b/packages/backend/tests/unit/services/GuildAccessService.test.ts index 622db56d0..a2ea2dc29 100644 --- a/packages/backend/tests/unit/services/GuildAccessService.test.ts +++ b/packages/backend/tests/unit/services/GuildAccessService.test.ts @@ -3,7 +3,17 @@ import type { DiscordGuild } from '../../../src/services/DiscordOAuthService' import type { SessionData } from '../../../src/services/SessionService' const mockGetUserGuilds = jest.fn, [string]>() -const mockHasAdminPermission = jest.fn() +const mockHasAdminPermission = jest.fn< + boolean, + [string | null | undefined, string | null | undefined] +>() + +class MockDiscordApiError extends Error { + constructor(public readonly statusCode: number, message = 'Discord API error') { + super(message) + this.name = 'DiscordApiError' + } +} const mockHasBotInGuild = jest.fn, [string]>() const mockGetGuildMemberContext = jest.fn< @@ -20,9 +30,12 @@ const mockHasAccess = jest.fn< >() jest.mock('../../../src/services/DiscordOAuthService', () => ({ + DiscordApiError: MockDiscordApiError, discordOAuthService: { getUserGuilds: (...args: [string]) => mockGetUserGuilds(...args), - hasAdminPermission: (...args: [string]) => + hasAdminPermission: ( + ...args: [string | null | undefined, string | null | undefined] + ) => mockHasAdminPermission(...args), }, })) @@ -49,6 +62,7 @@ jest.mock('@lucky/shared/services', () => ({ })) import { guildAccessService } from '../../../src/services/GuildAccessService' +import { DiscordApiError } from '../../../src/services/DiscordOAuthService' const EMPTY_ACCESS = { overview: 'none', @@ -172,6 +186,173 @@ describe('GuildAccessService', () => { expect(result[1].effectiveAccess.moderation).toBe('view') }) + test('listAuthorizedGuilds skips guilds that fail context resolution', async () => { + const guilds = [makeGuild('101', { owner: true }), makeGuild('202')] + const adminAccess = { + overview: 'manage', + settings: 'manage', + moderation: 'manage', + automation: 'manage', + music: 'manage', + integrations: 'manage', + } + + mockGetUserGuilds.mockResolvedValue(guilds) + mockHasBotInGuild.mockImplementation(async (guildId: string) => { + if (guildId === '202') { + throw new Error('Discord guild fetch failed') + } + return true + }) + mockResolveEffectiveAccess.mockResolvedValue(adminAccess) + + const result = await guildAccessService.listAuthorizedGuilds(SESSION) + + expect(result.map((guild) => guild.id)).toEqual(['101']) + expect(mockEnrichGuildsWithBotStatus).toHaveBeenCalledWith([guilds[0]]) + }) + + test('listAuthorizedGuilds maps Discord 401 errors to unauthorized AppError', async () => { + mockGetUserGuilds.mockRejectedValue( + new DiscordApiError(401, 'invalid token'), + ) + + await expect(guildAccessService.listAuthorizedGuilds(SESSION)).rejects.toMatchObject({ + statusCode: 401, + message: 'Discord session expired. Please sign in again.', + }) + }) + + test('listAuthorizedGuilds maps Discord 403 errors to forbidden AppError', async () => { + mockGetUserGuilds.mockRejectedValue({ + statusCode: 403, + message: 'missing scope', + }) + + await expect(guildAccessService.listAuthorizedGuilds(SESSION)).rejects.toMatchObject({ + statusCode: 403, + message: 'Discord OAuth scope is missing. Re-authenticate and try again.', + }) + }) + + test('listAuthorizedGuilds maps Discord upstream failures to 502 AppError', async () => { + mockGetUserGuilds.mockRejectedValue({ + status: 429, + message: 'rate limited', + }) + + await expect(guildAccessService.listAuthorizedGuilds(SESSION)).rejects.toMatchObject({ + statusCode: 502, + message: 'Discord API is temporarily unavailable. Please retry.', + }) + }) + + test('listAuthorizedGuilds rethrows unexpected guild-fetch errors', async () => { + const unknownError = new Error('boom') + mockGetUserGuilds.mockRejectedValue(unknownError) + + await expect(guildAccessService.listAuthorizedGuilds(SESSION)).rejects.toBe( + unknownError, + ) + }) + + test('listAuthorizedGuilds skips guild when access resolution throws', async () => { + const guilds = [makeGuild('101', { owner: true }), makeGuild('202')] + const adminAccess = { + overview: 'manage', + settings: 'manage', + moderation: 'manage', + automation: 'manage', + music: 'manage', + integrations: 'manage', + } + + mockGetUserGuilds.mockResolvedValue(guilds) + mockHasBotInGuild.mockResolvedValue(true) + mockResolveEffectiveAccess.mockImplementation(async (guildId: string) => { + if (guildId === '202') { + throw new Error('policy lookup failed') + } + return adminAccess + }) + + const result = await guildAccessService.listAuthorizedGuilds(SESSION) + + expect(result).toHaveLength(1) + expect(result[0].id).toBe('101') + }) + + test('listAuthorizedGuilds returns retryable error when all context lookups fail', async () => { + const guilds = [makeGuild('101'), makeGuild('202')] + + mockGetUserGuilds.mockResolvedValue(guilds) + mockHasBotInGuild.mockResolvedValue(true) + mockResolveEffectiveAccess.mockRejectedValue( + new Error('rbac dependency unavailable'), + ) + + await expect(guildAccessService.listAuthorizedGuilds(SESSION)).rejects.toMatchObject({ + statusCode: 502, + message: 'Unable to resolve server access right now. Please retry.', + }) + }) + + test('resolveGuildContext throws when guild member context lookup fails', async () => { + const guild = makeGuild('808') + + mockGetUserGuilds.mockResolvedValue([guild]) + mockHasBotInGuild.mockResolvedValue(true) + mockGetGuildMemberContext.mockRejectedValue( + new Error('member context unavailable'), + ) + + await expect( + guildAccessService.resolveGuildContext(SESSION, guild.id), + ).rejects.toThrow('member context unavailable') + + expect(mockResolveEffectiveAccess).not.toHaveBeenCalled() + }) + + test('listAuthorizedGuilds returns retryable error when all member-context lookups fail', async () => { + const guilds = [makeGuild('101'), makeGuild('202')] + + mockGetUserGuilds.mockResolvedValue(guilds) + mockHasBotInGuild.mockResolvedValue(true) + mockGetGuildMemberContext.mockRejectedValue( + new Error('member context unavailable'), + ) + + await expect(guildAccessService.listAuthorizedGuilds(SESSION)).rejects.toMatchObject({ + statusCode: 502, + message: 'Unable to resolve server access right now. Please retry.', + }) + + expect(mockResolveEffectiveAccess).not.toHaveBeenCalled() + }) + + test('listAuthorizedGuilds throws when enriched guild has no context', async () => { + const guild = makeGuild('101', { owner: true }) + const adminAccess = { + overview: 'manage', + settings: 'manage', + moderation: 'manage', + automation: 'manage', + music: 'manage', + integrations: 'manage', + } + + mockGetUserGuilds.mockResolvedValue([guild]) + mockHasBotInGuild.mockResolvedValue(true) + mockResolveEffectiveAccess.mockResolvedValue(adminAccess) + mockEnrichGuildsWithBotStatus.mockResolvedValueOnce([ + { ...guild, id: 'unknown-guild', hasBot: true }, + ]) + + await expect(guildAccessService.listAuthorizedGuilds(SESSION)).rejects.toThrow( + 'Missing authorized context for guild unknown-guild', + ) + }) + test('resolveGuildContext returns null when guild is not in user guild list', async () => { mockGetUserGuilds.mockResolvedValue([makeGuild('101')]) diff --git a/packages/frontend/src/App.authRoutes.test.tsx b/packages/frontend/src/App.authRoutes.test.tsx index ed17beb30..f2c3e15ce 100644 --- a/packages/frontend/src/App.authRoutes.test.tsx +++ b/packages/frontend/src/App.authRoutes.test.tsx @@ -20,6 +20,10 @@ vi.mock('./pages/Login', () => ({ default: () =>

Login Page

, })) +vi.mock('./pages/ServersPage', () => ({ + default: () =>

Servers Page

, +})) + vi.mock('./pages/Moderation', () => ({ default: () =>

Moderation Page

, })) @@ -28,6 +32,10 @@ vi.mock('./pages/TwitchNotifications', () => ({ default: () =>

Twitch Notifications Page

, })) +vi.mock('./pages/Features', () => ({ + default: () =>

Features Page

, +})) + type AuthState = { isAuthenticated: boolean isLoading: boolean @@ -59,6 +67,24 @@ const defaultGuildState: GuildState = { memberContextLoading: false, } +const MANAGE_ACCESS: EffectiveAccessMap = { + overview: 'manage', + settings: 'manage', + moderation: 'manage', + automation: 'manage', + music: 'manage', + integrations: 'manage', +} + +const NONE_ACCESS: EffectiveAccessMap = { + overview: 'none', + settings: 'none', + moderation: 'none', + automation: 'none', + music: 'none', + integrations: 'none', +} + function mockAuthStore(overrides: Partial = {}) { const state = { ...defaultAuthState, ...overrides } const storeImpl = ((selector?: (value: AuthState) => unknown) => @@ -131,14 +157,7 @@ describe('App authenticated routing', () => { selectedGuild: { id: '123', name: 'Guild', - effectiveAccess: { - overview: 'manage', - settings: 'manage', - moderation: 'manage', - automation: 'manage', - music: 'manage', - integrations: 'manage', - }, + effectiveAccess: MANAGE_ACCESS, }, memberContextLoading: true, }) @@ -159,12 +178,10 @@ describe('App authenticated routing', () => { id: '123', name: 'Guild', effectiveAccess: { + ...NONE_ACCESS, overview: 'manage', settings: 'manage', moderation: 'view', - automation: 'none', - music: 'none', - integrations: 'none', }, }, memberContextLoading: false, @@ -184,12 +201,8 @@ describe('App authenticated routing', () => { id: '123', name: 'Guild', effectiveAccess: { + ...NONE_ACCESS, overview: 'view', - settings: 'none', - moderation: 'none', - automation: 'none', - music: 'none', - integrations: 'none', }, }, memberContextLoading: false, @@ -204,4 +217,49 @@ describe('App authenticated routing', () => { ), ).toBeInTheDocument() }) + + test('guards /features route with automation module access', async () => { + mockAuthStore({ isAuthenticated: true }) + mockGuildStore({ + selectedGuild: { + id: '123', + name: 'Guild', + effectiveAccess: { + ...NONE_ACCESS, + overview: 'manage', + settings: 'manage', + moderation: 'manage', + }, + }, + memberContextLoading: false, + }) + + renderAt('/features') + + expect(await screen.findByText('Access denied')).toBeInTheDocument() + expect( + screen.getByText( + 'You do not have permission to view the automation module for this server.', + ), + ).toBeInTheDocument() + expect(screen.queryByText('Features Page')).not.toBeInTheDocument() + }) + + test('keeps /servers accessible even without overview module access', async () => { + mockAuthStore({ isAuthenticated: true }) + mockGuildStore({ + selectedGuild: { + id: '123', + name: 'Guild', + effectiveAccess: NONE_ACCESS, + }, + memberContextLoading: false, + }) + + renderAt('/servers') + + expect( + await screen.findByRole('heading', { name: 'Servers Page' }), + ).toBeInTheDocument() + }) }) diff --git a/packages/frontend/src/App.tsx b/packages/frontend/src/App.tsx index e9c41eb48..0797a008e 100644 --- a/packages/frontend/src/App.tsx +++ b/packages/frontend/src/App.tsx @@ -101,7 +101,7 @@ function AuthenticatedRoutes() { /> )} + element={} /> { selectedGuild: mockGuild, selectedGuildId: mockGuild.id, isLoading: false, + guildLoadError: null, memberContext: null, memberContextLoading: false, serverSettings: null, @@ -227,6 +228,7 @@ describe('Sidebar', () => { guilds: [], selectedGuild: null, selectedGuildId: null, + guildLoadError: null, }) const user = userEvent.setup() @@ -248,6 +250,97 @@ describe('Sidebar', () => { }) }) + test('shows retry and re-auth CTAs when guild fetch fails from auth state', async () => { + const user = userEvent.setup() + mockGuildStoreState({ + guilds: [], + selectedGuild: null, + selectedGuildId: null, + guildLoadError: { + kind: 'auth', + status: 401, + message: 'Session expired', + }, + } as Partial>) + + renderSidebar() + + await user.click( + screen.getByRole('button', { name: /select a server/i }), + ) + + await waitFor(() => { + expect(screen.getByText('Could not load servers')).toBeInTheDocument() + expect(screen.getByRole('button', { name: 'Retry' })).toBeInTheDocument() + expect( + screen.getByRole('link', { name: 'Re-authenticate' }), + ).toBeInTheDocument() + }) + + await user.click(screen.getByRole('button', { name: 'Retry' })) + expect(mockFetchGuilds).toHaveBeenCalledTimes(1) + }) + + test('shows re-auth CTA when guild fetch fails from forbidden state', async () => { + const user = userEvent.setup() + mockGuildStoreState({ + guilds: [], + selectedGuild: null, + selectedGuildId: null, + guildLoadError: { + kind: 'forbidden', + status: 403, + message: 'Missing oauth scope', + }, + } as Partial>) + + renderSidebar() + + await user.click( + screen.getByRole('button', { name: /select a server/i }), + ) + + await waitFor(() => { + expect( + screen.getByText('Discord access is missing required scope.'), + ).toBeInTheDocument() + expect( + screen.getByRole('link', { name: 'Re-authenticate' }), + ).toBeInTheDocument() + }) + }) + + test('shows network guidance without re-auth CTA on network failures', async () => { + const user = userEvent.setup() + mockGuildStoreState({ + guilds: [], + selectedGuild: null, + selectedGuildId: null, + guildLoadError: { + kind: 'network', + status: 0, + message: 'Network down', + }, + } as Partial>) + + renderSidebar() + + await user.click( + screen.getByRole('button', { name: /select a server/i }), + ) + + await waitFor(() => { + expect( + screen.getByText( + 'Network connection failed. Check connectivity and retry.', + ), + ).toBeInTheDocument() + expect( + screen.queryByRole('link', { name: 'Re-authenticate' }), + ).not.toBeInTheDocument() + }) + }) + test('opens and closes mobile sidebar', async () => { const user = userEvent.setup() renderSidebar() diff --git a/packages/frontend/src/components/Layout/Sidebar.tsx b/packages/frontend/src/components/Layout/Sidebar.tsx index 76b548014..4735a0aa2 100644 --- a/packages/frontend/src/components/Layout/Sidebar.tsx +++ b/packages/frontend/src/components/Layout/Sidebar.tsx @@ -1,7 +1,8 @@ -import { useEffect, useState } from 'react' +import { useEffect, useState, type Dispatch, type SetStateAction } from 'react' import { Link, useLocation } from 'react-router-dom' import { AnimatePresence, motion } from 'framer-motion' import { + AlertTriangle, ChevronDown, History, LayoutDashboard, @@ -27,7 +28,9 @@ import { useAuthStore } from '@/stores/authStore' import { useGuildStore } from '@/stores/guildStore' import { cn } from '@/lib/utils' import { hasModuleAccess } from '@/lib/rbac' -import type { ModuleKey } from '@/types' +import type { Guild, ModuleKey } from '@/types' +import type { GuildLoadErrorState } from '@/stores/guildStore' +import { api } from '@/services/api' interface NavItem { path: string @@ -143,11 +146,207 @@ const navSections: NavSection[] = [ }, ] +function getGuildLoadMessage(guildLoadError: GuildLoadErrorState): string { + switch (guildLoadError.kind) { + case 'auth': + return 'Your session expired. Please sign in again.' + case 'forbidden': + return 'Discord access is missing required scope.' + case 'network': + return 'Network connection failed. Check connectivity and retry.' + default: + return guildLoadError.message + } +} + +interface ServerSelectorProps { + guilds: Guild[] + selectedGuild: Guild | null + guildLoadError: GuildLoadErrorState | null + isLoading: boolean + serverDropdownOpen: boolean + setServerDropdownOpen: Dispatch> + fetchGuilds: () => Promise + selectGuild: (guild: Guild | null) => void +} + +function ServerSelector({ + guilds, + selectedGuild, + guildLoadError, + isLoading, + serverDropdownOpen, + setServerDropdownOpen, + fetchGuilds, + selectGuild, +}: ServerSelectorProps) { + const emptyStateText = isLoading + ? 'Loading servers...' + : 'No accessible servers found' + const discordLoginUrl = api.auth.getDiscordLoginUrl() + const guildLoadMessage = guildLoadError + ? getGuildLoadMessage(guildLoadError) + : null + + return ( +
+

Server context

+
+ + + + {serverDropdownOpen && ( + + + {guilds.length === 0 ? ( +
+ {guildLoadError ? ( + <> +
+ +
+

+ Could not load servers +

+

+ {guildLoadMessage} +

+
+ + {(guildLoadError.kind === 'auth' || + guildLoadError.kind === + 'forbidden') && ( + + Re-authenticate + + )} +
+ + ) : ( +

+ {emptyStateText} +

+ )} +
+ ) : ( + guilds.map((guild) => ( + + )) + )} + + + )} + +
+
+ ) +} + function Sidebar() { const location = useLocation() const { user, logout } = useAuthStore() - const { guilds, selectedGuild, selectGuild, memberContext } = - useGuildStore() + const { + guilds, + selectedGuild, + selectGuild, + memberContext, + guildLoadError, + fetchGuilds, + isLoading, + } = useGuildStore() const [mobileOpen, setMobileOpen] = useState(false) const [serverDropdownOpen, setServerDropdownOpen] = useState(false) @@ -204,120 +403,16 @@ function Sidebar() { -
-

- Server context -

-
- - - - {serverDropdownOpen && ( - - - {guilds.length === 0 ? ( -
-

- No accessible servers found -

-
- ) : ( - guilds.map((guild) => ( - - )) - )} - - - )} - -
-
+