mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-06-19 13:21:46 +00:00
feat(oauth): add trips:share scope and redesign consent screen
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.
This commit is contained in:
@@ -8,9 +8,10 @@ export interface ScopeInfo {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export const SCOPE_GROUPS: Record<string, ScopeInfo> = {
|
export const SCOPE_GROUPS: Record<string, ScopeInfo> = {
|
||||||
'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: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: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: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' },
|
'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' },
|
'packing:read': { label: 'View packing lists', description: 'Read packing items, bags, and category assignees', group: 'Packing' },
|
||||||
|
|||||||
@@ -173,43 +173,64 @@ export default function OAuthAuthorizePage(): React.ReactElement {
|
|||||||
// pageState === 'consent'
|
// pageState === 'consent'
|
||||||
return (
|
return (
|
||||||
<div className="min-h-screen flex items-center justify-center p-4" style={{ background: 'var(--bg-primary)' }}>
|
<div className="min-h-screen flex items-center justify-center p-4" style={{ background: 'var(--bg-primary)' }}>
|
||||||
<div className="w-full max-w-sm rounded-xl shadow-lg overflow-hidden" style={{ background: 'var(--bg-card)' }}>
|
<div className="w-full max-w-2xl rounded-xl shadow-lg overflow-hidden flex flex-col sm:flex-row" style={{ background: 'var(--bg-card)' }}>
|
||||||
{/* Header */}
|
|
||||||
<div className="px-8 pt-8 pb-5 text-center space-y-3">
|
{/* Left panel — app identity + actions */}
|
||||||
<div className="flex justify-center">
|
<div className="sm:w-64 sm:flex-shrink-0 flex flex-col px-8 py-8 sm:border-r" style={{ borderColor: 'var(--border-primary)' }}>
|
||||||
|
<div className="flex-1 space-y-4">
|
||||||
<div className="w-12 h-12 rounded-full flex items-center justify-center" style={{ background: 'var(--bg-secondary)' }}>
|
<div className="w-12 h-12 rounded-full flex items-center justify-center" style={{ background: 'var(--bg-secondary)' }}>
|
||||||
<ShieldCheck className="w-6 h-6" style={{ color: 'var(--accent-primary, #4f46e5)' }} />
|
<ShieldCheck className="w-6 h-6" style={{ color: 'var(--accent-primary, #4f46e5)' }} />
|
||||||
</div>
|
</div>
|
||||||
|
<div>
|
||||||
|
<p className="text-xs font-medium uppercase tracking-wide mb-1" style={{ color: 'var(--text-tertiary)' }}>Authorization Request</p>
|
||||||
|
<h1 className="text-lg font-semibold leading-snug" style={{ color: 'var(--text-primary)' }}>
|
||||||
|
{validation?.client?.name || clientId}
|
||||||
|
</h1>
|
||||||
|
<p className="text-sm mt-2" style={{ color: 'var(--text-secondary)' }}>
|
||||||
|
This application is requesting access to your TREK account.
|
||||||
|
</p>
|
||||||
|
</div>
|
||||||
</div>
|
</div>
|
||||||
<div>
|
|
||||||
<p className="text-xs font-medium uppercase tracking-wide mb-1" style={{ color: 'var(--text-tertiary)' }}>Authorization Request</p>
|
<div className="mt-8 space-y-2">
|
||||||
<h1 className="text-lg font-semibold" style={{ color: 'var(--text-primary)' }}>
|
<p className="text-xs mb-3" style={{ color: 'var(--text-tertiary)' }}>
|
||||||
{validation?.client?.name || clientId}
|
Only grant access to applications you trust. Your data stays on your server.
|
||||||
</h1>
|
|
||||||
<p className="text-sm mt-1" style={{ color: 'var(--text-secondary)' }}>
|
|
||||||
This application is requesting access to your TREK account.
|
|
||||||
</p>
|
</p>
|
||||||
|
<button
|
||||||
|
onClick={() => submitConsent(true)}
|
||||||
|
disabled={submitting}
|
||||||
|
className="w-full px-4 py-2.5 rounded-lg text-sm font-medium text-white disabled:opacity-60 transition-opacity"
|
||||||
|
style={{ background: 'var(--accent-primary, #4f46e5)' }}>
|
||||||
|
{submitting ? 'Authorizing…' : 'Approve Access'}
|
||||||
|
</button>
|
||||||
|
<button
|
||||||
|
onClick={() => submitConsent(false)}
|
||||||
|
disabled={submitting}
|
||||||
|
className="w-full px-4 py-2.5 rounded-lg text-sm font-medium border transition-colors hover:bg-slate-50 dark:hover:bg-slate-800 disabled:opacity-60"
|
||||||
|
style={{ borderColor: 'var(--border-primary)', color: 'var(--text-secondary)' }}>
|
||||||
|
Deny
|
||||||
|
</button>
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
{/* Scope list */}
|
{/* Right panel — scopes */}
|
||||||
<div className="px-8 pb-5">
|
<div className="flex-1 px-6 py-8 overflow-y-auto max-h-[80vh] sm:max-h-[600px]">
|
||||||
<p className="text-xs font-medium uppercase tracking-wide mb-3" style={{ color: 'var(--text-tertiary)' }}>
|
<p className="text-xs font-medium uppercase tracking-wide mb-4" style={{ color: 'var(--text-tertiary)' }}>
|
||||||
Permissions requested
|
Permissions requested
|
||||||
</p>
|
</p>
|
||||||
<div className="space-y-3">
|
<div className="space-y-5">
|
||||||
{Object.entries(scopesByGroup).map(([group, groupScopes]) => (
|
{Object.entries(scopesByGroup).map(([group, groupScopes]) => (
|
||||||
<div key={group}>
|
<div key={group}>
|
||||||
<p className="text-xs font-semibold mb-1.5" style={{ color: 'var(--text-secondary)' }}>{group}</p>
|
<p className="text-xs font-semibold mb-2" style={{ color: 'var(--text-secondary)' }}>{group}</p>
|
||||||
<div className="space-y-1.5">
|
<div className="space-y-1.5">
|
||||||
{groupScopes.map(s => {
|
{groupScopes.map(s => {
|
||||||
const info = SCOPE_GROUPS[s]
|
const info = SCOPE_GROUPS[s]
|
||||||
return (
|
return (
|
||||||
<div key={s} className="flex items-start gap-2.5 px-3 py-2 rounded-lg" style={{ background: 'var(--bg-secondary)' }}>
|
<div key={s} className="flex items-start gap-2.5 px-3 py-2 rounded-lg" style={{ background: 'var(--bg-secondary)' }}>
|
||||||
<span className="mt-0.5 text-base leading-none">
|
<span className="mt-0.5 text-base leading-none flex-shrink-0">
|
||||||
{s.endsWith(':delete') ? '🗑️' : s.endsWith(':write') ? '✏️' : '👁️'}
|
{s.endsWith(':delete') ? '🗑️' : s.endsWith(':write') ? '✏️' : '👁️'}
|
||||||
</span>
|
</span>
|
||||||
<div>
|
<div className="min-w-0">
|
||||||
<p className="text-sm font-medium" style={{ color: 'var(--text-primary)' }}>{info?.label || s}</p>
|
<p className="text-sm font-medium" style={{ color: 'var(--text-primary)' }}>{info?.label || s}</p>
|
||||||
<p className="text-xs mt-0.5" style={{ color: 'var(--text-tertiary)' }}>{info?.description || ''}</p>
|
<p className="text-xs mt-0.5" style={{ color: 'var(--text-tertiary)' }}>{info?.description || ''}</p>
|
||||||
</div>
|
</div>
|
||||||
@@ -222,26 +243,6 @@ export default function OAuthAuthorizePage(): React.ReactElement {
|
|||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
{/* Footer / actions */}
|
|
||||||
<div className="px-8 pb-8 space-y-2">
|
|
||||||
<p className="text-xs text-center mb-3" style={{ color: 'var(--text-tertiary)' }}>
|
|
||||||
Only grant access to applications you trust. Your data stays on your server.
|
|
||||||
</p>
|
|
||||||
<button
|
|
||||||
onClick={() => submitConsent(true)}
|
|
||||||
disabled={submitting}
|
|
||||||
className="w-full px-4 py-2.5 rounded-lg text-sm font-medium text-white disabled:opacity-60 transition-opacity"
|
|
||||||
style={{ background: 'var(--accent-primary, #4f46e5)' }}>
|
|
||||||
{submitting ? 'Authorizing…' : 'Approve Access'}
|
|
||||||
</button>
|
|
||||||
<button
|
|
||||||
onClick={() => submitConsent(false)}
|
|
||||||
disabled={submitting}
|
|
||||||
className="w-full px-4 py-2.5 rounded-lg text-sm font-medium border transition-colors hover:bg-slate-50 dark:hover:bg-slate-800 disabled:opacity-60"
|
|
||||||
style={{ borderColor: 'var(--border-primary)', color: 'var(--text-secondary)' }}>
|
|
||||||
Deny
|
|
||||||
</button>
|
|
||||||
</div>
|
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ export const SCOPES = {
|
|||||||
TRIPS_READ: 'trips:read',
|
TRIPS_READ: 'trips:read',
|
||||||
TRIPS_WRITE: 'trips:write',
|
TRIPS_WRITE: 'trips:write',
|
||||||
TRIPS_DELETE: 'trips:delete',
|
TRIPS_DELETE: 'trips:delete',
|
||||||
|
TRIPS_SHARE: 'trips:share',
|
||||||
PLACES_READ: 'places:read',
|
PLACES_READ: 'places:read',
|
||||||
PLACES_WRITE: 'places:write',
|
PLACES_WRITE: 'places:write',
|
||||||
PACKING_READ: 'packing:read',
|
PACKING_READ: 'packing:read',
|
||||||
@@ -34,9 +35,10 @@ export interface ScopeInfo {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export const SCOPE_INFO: Record<Scope, ScopeInfo> = {
|
export const SCOPE_INFO: Record<Scope, ScopeInfo> = {
|
||||||
'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: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: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: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' },
|
'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' },
|
'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<Scope, ScopeInfo> = {
|
|||||||
// null scopes = static trek_ token = full access
|
// 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 {
|
export function canReadTrips(scopes: string[] | null): boolean {
|
||||||
if (!scopes) return true;
|
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 */
|
/** 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');
|
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[] } {
|
export function validateScopes(requestedScopes: string[]): { valid: boolean; invalid: string[] } {
|
||||||
const invalid = requestedScopes.filter(s => !ALL_SCOPES.includes(s as Scope));
|
const invalid = requestedScopes.filter(s => !ALL_SCOPES.includes(s as Scope));
|
||||||
return { valid: invalid.length === 0, invalid };
|
return { valid: invalid.length === 0, invalid };
|
||||||
|
|||||||
@@ -24,12 +24,13 @@ import {
|
|||||||
TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT,
|
TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT,
|
||||||
demoDenied, noAccess, ok,
|
demoDenied, noAccess, ok,
|
||||||
} from './_shared';
|
} 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 {
|
export function registerTripTools(server: McpServer, userId: number, scopes: string[] | null): void {
|
||||||
const R = canReadTrips(scopes);
|
const R = canReadTrips(scopes);
|
||||||
const W = canWrite(scopes, 'trips');
|
const W = canWrite(scopes, 'trips');
|
||||||
const D = canDeleteTrips(scopes);
|
const D = canDeleteTrips(scopes);
|
||||||
|
const S = canShareTrips(scopes);
|
||||||
|
|
||||||
// --- TRIPS ---
|
// --- 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',
|
'get_share_link',
|
||||||
{
|
{
|
||||||
description: 'Get the current public share link for a trip, including its permission flags. Returns null if no share link exists.',
|
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',
|
'create_share_link',
|
||||||
{
|
{
|
||||||
description: 'Create or update the public share link for a trip. Set permission flags to control what is visible to guests.',
|
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',
|
'delete_share_link',
|
||||||
{
|
{
|
||||||
description: 'Revoke the public share link for a trip. Guests will no longer be able to access the shared view.',
|
description: 'Revoke the public share link for a trip. Guests will no longer be able to access the shared view.',
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ import {
|
|||||||
canWrite,
|
canWrite,
|
||||||
canRead,
|
canRead,
|
||||||
canDeleteTrips,
|
canDeleteTrips,
|
||||||
|
canShareTrips,
|
||||||
ALL_SCOPES,
|
ALL_SCOPES,
|
||||||
SCOPE_INFO,
|
SCOPE_INFO,
|
||||||
} from '../../../src/mcp/scopes';
|
} from '../../../src/mcp/scopes';
|
||||||
@@ -22,6 +23,7 @@ describe('ALL_SCOPES', () => {
|
|||||||
expect(ALL_SCOPES).toContain('trips:read');
|
expect(ALL_SCOPES).toContain('trips:read');
|
||||||
expect(ALL_SCOPES).toContain('trips:write');
|
expect(ALL_SCOPES).toContain('trips:write');
|
||||||
expect(ALL_SCOPES).toContain('trips:delete');
|
expect(ALL_SCOPES).toContain('trips:delete');
|
||||||
|
expect(ALL_SCOPES).toContain('trips:share');
|
||||||
expect(ALL_SCOPES).toContain('budget:read');
|
expect(ALL_SCOPES).toContain('budget:read');
|
||||||
expect(ALL_SCOPES).toContain('budget:write');
|
expect(ALL_SCOPES).toContain('budget:write');
|
||||||
expect(ALL_SCOPES).toContain('packing:read');
|
expect(ALL_SCOPES).toContain('packing:read');
|
||||||
@@ -122,6 +124,10 @@ describe('canReadTrips', () => {
|
|||||||
expect(canReadTrips(['trips:delete'])).toBe(true);
|
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', () => {
|
it('returns false when only unrelated scopes are present', () => {
|
||||||
expect(canReadTrips(['budget:read', 'packing:write'])).toBe(false);
|
expect(canReadTrips(['budget:read', 'packing:write'])).toBe(false);
|
||||||
});
|
});
|
||||||
@@ -214,3 +220,37 @@ describe('canDeleteTrips', () => {
|
|||||||
expect(canDeleteTrips([])).toBe(false);
|
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);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user