From 8212f3c023bc2b569c7dbe2af701821bd0d426e3 Mon Sep 17 00:00:00 2001 From: jubnl Date: Fri, 10 Apr 2026 00:54:59 +0200 Subject: [PATCH] feat(oauth): add trips:share scope and redesign consent screen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce trips:share as a dedicated OAuth scope for managing public share links, decoupled from trips:read and trips:write. Share link tools (get/create/delete_share_link) now gate on canShareTrips() instead of the generic read/write scopes. Scope added to both client and server definitions with full test coverage. Redesign the consent screen from a narrow single-column card (max-w-sm) to a two-panel layout (max-w-2xl): app identity and action buttons on the left, scrollable scope list on the right. Responsive — stacks vertically on mobile. --- client/src/api/oauthScopes.ts | 3 +- client/src/pages/OAuthAuthorizePage.tsx | 77 +++++++++++++------------ server/src/mcp/scopes.ts | 14 ++++- server/src/mcp/tools/trips.ts | 9 +-- server/tests/unit/mcp/scopes.test.ts | 40 +++++++++++++ 5 files changed, 97 insertions(+), 46 deletions(-) diff --git a/client/src/api/oauthScopes.ts b/client/src/api/oauthScopes.ts index aacd992a..53ae8fdd 100644 --- a/client/src/api/oauthScopes.ts +++ b/client/src/api/oauthScopes.ts @@ -8,9 +8,10 @@ export interface ScopeInfo { } export const SCOPE_GROUPS: Record = { - 'trips:read': { label: 'View trips & itineraries', description: 'Read trips, days, day notes, members, and share links', group: 'Trips' }, + 'trips:read': { label: 'View trips & itineraries', description: 'Read trips, days, day notes, and members', group: 'Trips' }, 'trips:write': { label: 'Edit trips & itineraries', description: 'Create and update trips, days, notes, and manage members', group: 'Trips' }, 'trips:delete': { label: 'Delete trips', description: 'Permanently delete entire trips — this action is irreversible', group: 'Trips' }, + 'trips:share': { label: 'Manage share links', description: 'Create, update, and revoke public share links for trips', group: 'Trips' }, 'places:read': { label: 'View places & map data', description: 'Read places, day assignments, tags, categories, and visited countries', group: 'Places' }, 'places:write': { label: 'Manage places', description: 'Create, update, and delete places, assignments, tags, and atlas entries', group: 'Places' }, 'packing:read': { label: 'View packing lists', description: 'Read packing items, bags, and category assignees', group: 'Packing' }, diff --git a/client/src/pages/OAuthAuthorizePage.tsx b/client/src/pages/OAuthAuthorizePage.tsx index 03d50475..24bd10fd 100644 --- a/client/src/pages/OAuthAuthorizePage.tsx +++ b/client/src/pages/OAuthAuthorizePage.tsx @@ -173,43 +173,64 @@ export default function OAuthAuthorizePage(): React.ReactElement { // pageState === 'consent' return (
-
- {/* Header */} -
-
+
+ + {/* Left panel — app identity + actions */} +
+
+
+

Authorization Request

+

+ {validation?.client?.name || clientId} +

+

+ This application is requesting access to your TREK account. +

+
-
-

Authorization Request

-

- {validation?.client?.name || clientId} -

-

- This application is requesting access to your TREK account. + +

+

+ Only grant access to applications you trust. Your data stays on your server.

+ +
- {/* Scope list */} -
-

+ {/* Right panel — scopes */} +

+

Permissions requested

-
+
{Object.entries(scopesByGroup).map(([group, groupScopes]) => (
-

{group}

+

{group}

{groupScopes.map(s => { const info = SCOPE_GROUPS[s] return (
- + {s.endsWith(':delete') ? '🗑️' : s.endsWith(':write') ? '✏️' : '👁️'} -
+

{info?.label || s}

{info?.description || ''}

@@ -222,26 +243,6 @@ export default function OAuthAuthorizePage(): React.ReactElement {
- {/* Footer / actions */} -
-

- Only grant access to applications you trust. Your data stays on your server. -

- - -
) diff --git a/server/src/mcp/scopes.ts b/server/src/mcp/scopes.ts index cd2d3966..f0531456 100644 --- a/server/src/mcp/scopes.ts +++ b/server/src/mcp/scopes.ts @@ -6,6 +6,7 @@ export const SCOPES = { TRIPS_READ: 'trips:read', TRIPS_WRITE: 'trips:write', TRIPS_DELETE: 'trips:delete', + TRIPS_SHARE: 'trips:share', PLACES_READ: 'places:read', PLACES_WRITE: 'places:write', PACKING_READ: 'packing:read', @@ -34,9 +35,10 @@ export interface ScopeInfo { } export const SCOPE_INFO: Record = { - 'trips:read': { label: 'View trips & itineraries', description: 'Read trips, days, day notes, members, and share links', group: 'Trips' }, + 'trips:read': { label: 'View trips & itineraries', description: 'Read trips, days, day notes, and members', group: 'Trips' }, 'trips:write': { label: 'Edit trips & itineraries', description: 'Create and update trips, days, notes, and manage members', group: 'Trips' }, 'trips:delete': { label: 'Delete trips', description: 'Permanently delete entire trips — this action is irreversible', group: 'Trips' }, + 'trips:share': { label: 'Manage share links', description: 'Create, update, and revoke public share links for trips', group: 'Trips' }, 'places:read': { label: 'View places & map data', description: 'Read places, day assignments, tags, categories, and visited countries', group: 'Places' }, 'places:write': { label: 'Manage places', description: 'Create, update, and delete places, assignments, tags, and atlas entries', group: 'Places' }, 'packing:read': { label: 'View packing lists', description: 'Read packing items, bags, and category assignees', group: 'Packing' }, @@ -59,10 +61,10 @@ export const SCOPE_INFO: Record = { // null scopes = static trek_ token = full access // --------------------------------------------------------------------------- -/** trips:read OR trips:write OR trips:delete all grant read access to trips */ +/** trips:read OR trips:write OR trips:delete OR trips:share all grant read access to trips */ export function canReadTrips(scopes: string[] | null): boolean { if (!scopes) return true; - return scopes.some(s => s === 'trips:read' || s === 'trips:write' || s === 'trips:delete'); + return scopes.some(s => s === 'trips:read' || s === 'trips:write' || s === 'trips:delete' || s === 'trips:share'); } /** group:write grants write access; for trips canReadTrips handles read */ @@ -83,6 +85,12 @@ export function canDeleteTrips(scopes: string[] | null): boolean { return scopes.includes('trips:delete'); } +/** trips:share is a separate scope for managing public share links */ +export function canShareTrips(scopes: string[] | null): boolean { + if (!scopes) return true; + return scopes.includes('trips:share'); +} + export function validateScopes(requestedScopes: string[]): { valid: boolean; invalid: string[] } { const invalid = requestedScopes.filter(s => !ALL_SCOPES.includes(s as Scope)); return { valid: invalid.length === 0, invalid }; diff --git a/server/src/mcp/tools/trips.ts b/server/src/mcp/tools/trips.ts index 97598460..f03f142c 100644 --- a/server/src/mcp/tools/trips.ts +++ b/server/src/mcp/tools/trips.ts @@ -24,12 +24,13 @@ import { TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, demoDenied, noAccess, ok, } from './_shared'; -import { canReadTrips, canWrite, canDeleteTrips } from '../scopes'; +import { canReadTrips, canWrite, canDeleteTrips, canShareTrips } from '../scopes'; export function registerTripTools(server: McpServer, userId: number, scopes: string[] | null): void { const R = canReadTrips(scopes); const W = canWrite(scopes, 'trips'); const D = canDeleteTrips(scopes); + const S = canShareTrips(scopes); // --- TRIPS --- @@ -280,7 +281,7 @@ export function registerTripTools(server: McpServer, userId: number, scopes: str } ); - if (R) server.registerTool( + if (S) server.registerTool( 'get_share_link', { description: 'Get the current public share link for a trip, including its permission flags. Returns null if no share link exists.', @@ -296,7 +297,7 @@ export function registerTripTools(server: McpServer, userId: number, scopes: str } ); - if (W) server.registerTool( + if (S) server.registerTool( 'create_share_link', { description: 'Create or update the public share link for a trip. Set permission flags to control what is visible to guests.', @@ -324,7 +325,7 @@ export function registerTripTools(server: McpServer, userId: number, scopes: str } ); - if (W) server.registerTool( + if (S) server.registerTool( 'delete_share_link', { description: 'Revoke the public share link for a trip. Guests will no longer be able to access the shared view.', diff --git a/server/tests/unit/mcp/scopes.test.ts b/server/tests/unit/mcp/scopes.test.ts index cad9c283..df4e7b02 100644 --- a/server/tests/unit/mcp/scopes.test.ts +++ b/server/tests/unit/mcp/scopes.test.ts @@ -9,6 +9,7 @@ import { canWrite, canRead, canDeleteTrips, + canShareTrips, ALL_SCOPES, SCOPE_INFO, } from '../../../src/mcp/scopes'; @@ -22,6 +23,7 @@ describe('ALL_SCOPES', () => { expect(ALL_SCOPES).toContain('trips:read'); expect(ALL_SCOPES).toContain('trips:write'); expect(ALL_SCOPES).toContain('trips:delete'); + expect(ALL_SCOPES).toContain('trips:share'); expect(ALL_SCOPES).toContain('budget:read'); expect(ALL_SCOPES).toContain('budget:write'); expect(ALL_SCOPES).toContain('packing:read'); @@ -122,6 +124,10 @@ describe('canReadTrips', () => { expect(canReadTrips(['trips:delete'])).toBe(true); }); + it('returns true when trips:share is present', () => { + expect(canReadTrips(['trips:share'])).toBe(true); + }); + it('returns false when only unrelated scopes are present', () => { expect(canReadTrips(['budget:read', 'packing:write'])).toBe(false); }); @@ -214,3 +220,37 @@ describe('canDeleteTrips', () => { expect(canDeleteTrips([])).toBe(false); }); }); + +// --------------------------------------------------------------------------- +// canShareTrips +// --------------------------------------------------------------------------- + +describe('canShareTrips', () => { + it('returns true when scopes is null (full access)', () => { + expect(canShareTrips(null)).toBe(true); + }); + + it('returns true when trips:share is present', () => { + expect(canShareTrips(['trips:share'])).toBe(true); + }); + + it('returns false when only trips:read is present', () => { + expect(canShareTrips(['trips:read'])).toBe(false); + }); + + it('returns false when only trips:write is present', () => { + expect(canShareTrips(['trips:write'])).toBe(false); + }); + + it('returns false when only trips:delete is present', () => { + expect(canShareTrips(['trips:delete'])).toBe(false); + }); + + it('returns false for unrelated scopes', () => { + expect(canShareTrips(['budget:write', 'packing:read'])).toBe(false); + }); + + it('returns false for empty scopes array', () => { + expect(canShareTrips([])).toBe(false); + }); +});