diff --git a/docs/github-design.md b/docs/github-design.md index b7a05c2..ded899d 100644 --- a/docs/github-design.md +++ b/docs/github-design.md @@ -38,6 +38,26 @@ human-facing display value: the GitHub account login is the Context label and the installation ID remains stable metadata. Realmroot and Toolbox never use the display label as authorization input. +## Installation permission upgrades + +GitHub does not return permission-update acceptance to the App's Setup URL. +The Setup URL remains responsible for new installations and repository-access +changes, but it is not the completion signal for a newly added App permission. + +When an existing installation lacks a newly requested permission, the Adapter +keeps the original OAuth intent and opens the exact installation review URL at +`/settings/installations/{installation_id}/permissions/update`. An +Adapter-owned waiting page polls the preserved transaction while GitHub opens +the review in a separate window. The signed `installation` webhook with action +`new_permissions_accepted` updates Adapter-owned authority. Only after that +target installation covers every requested scope does the Adapter complete the +same OAuth transaction, close the GitHub window, and return the controller to +Realmroot automatically. + +Incomplete permission upgrades are not persisted as connected authority and do +not surface an intermediate Adapter error page during the normal approval +flow. + ## Permission translation The adapter reads the permissions configured on the GitHub App and exposes diff --git a/specs/github-adapter.feature b/specs/github-adapter.feature index 4dc1335..4f96821 100644 --- a/specs/github-adapter.feature +++ b/specs/github-adapter.feature @@ -24,11 +24,23 @@ Feature: GitHub App adapter Then the adapter keeps one stable external subject for the owner And replaces the installation authorization details with the newly authorized set + @journey:github-installation-permission-upgrade @entrypoint:http + Scenario: Authorization resumes after a GitHub installation permission upgrade + Given an existing GitHub App installation lacks a newly requested permission + When the owner authorizes that permission through Realmroot + Then the adapter preserves the original authorization transaction + And opens the exact target installation's permission review + And waits for GitHub's signed lifecycle event instead of expecting an unsupported browser callback + And resumes the same Realmroot authorization when that target installation accepts the permission + And returns automatically to the pending Realmroot approval + But it does not show an adapter insufficient-scope page + @journey:github-context-catalog @entrypoint:http Scenario: GitHub describes installation Contexts without exposing credentials Given the Adapter holds an active GitHub external authorization When Realmroot reads the advertised authorization-detail catalog with the connected subject token Then the adapter returns each active installation with its account name, stable installation ID, account type, and repository selection + And reports the exact scopes currently granted by each installation And the response uses the shared authorization-detail catalog representation And the authorization detail type is a stable URI owned by the Adapter But it does not expose installation credentials diff --git a/src/core/external-authorization-server.ts b/src/core/external-authorization-server.ts index c93d333..e912689 100644 --- a/src/core/external-authorization-server.ts +++ b/src/core/external-authorization-server.ts @@ -35,6 +35,7 @@ export type ExternalProviderAuthorization = { list(input: { subject: string; limit: number; offset: number }): Promise<{ items: Array<{ authorizationDetail: Record + grantedScopes?: string[] display: { label: string; description?: string; metadata?: Record } }> pagination: { limit: number; offset: number; total: number; hasMore: boolean; nextOffset: number | null } @@ -44,6 +45,11 @@ export type ExternalProviderAuthorization = { requested: Array> granted: Array> }): boolean + validateScopes?(input: { + subject: string + scopes: string[] + authorizationDetails: Array> + }): Promise validateGrant?(input: { subject: string scopes: string[] @@ -64,6 +70,14 @@ export type ExternalProviderAuthorization = { }): Promise< | { type: 'continue'; url: string; stage: string; data: Record; providerState: string } | { type: 'complete'; grant: Omit } + | { type: 'error'; error: 'access_denied' | 'server_error'; description: string } + > + resume?(input: { + intent: ExternalOAuthIntent + }): Promise< + | { type: 'pending' } + | { type: 'complete'; grant: Omit } + | { type: 'error'; error: 'access_denied' | 'server_error'; description: string } > } @@ -170,6 +184,19 @@ export async function createExternalAuthorizationServer(input: { app.get(providerCallbackPath, async (c) => { const state = required(c.req.query('state'), 'state') const intent = await input.store.intentByProviderState(await sha256(state)) + const providerError = c.req.query('error') + if (providerError) { + await input.store.cancelIntent(intent) + return c.redirect( + authorizationErrorRedirect( + intent, + providerError === 'access_denied' ? 'access_denied' : 'server_error', + providerError === 'access_denied' + ? 'Provider authorization was denied.' + : 'Provider authorization failed.', + ), + ) + } const result = await input.provider.complete({ callbackUrl: c.req.url, intent, @@ -185,17 +212,32 @@ export async function createExternalAuthorizationServer(input: { }) return c.redirect(result.url) } - const code = opaque('code') - await input.store.completeIntent( - intent, - { ...result.grant, providerId: input.provider.id, clientId: intent.clientId }, - code, - ) - const callback = new URL(intent.redirectUri) - callback.searchParams.set('code', code) - callback.searchParams.set('state', intent.realmrootState) - return c.redirect(callback.toString()) + if (result.type === 'error') { + await input.store.cancelIntent(intent) + return c.redirect(authorizationErrorRedirect(intent, result.error, result.description)) + } + return c.redirect(await completeAuthorization(input, intent, result.grant)) }) + const resume = input.provider.resume + if (resume) { + app.get(`/oauth/${input.provider.id}/continue`, async (c) => { + c.header('Cache-Control', 'no-store') + const intent = await input.store.intentByProviderState(await sha256(required(c.req.query('state'), 'state'))) + const result = await resume({ intent }) + if (result.type === 'pending') return c.json({ status: 'pending' }, 202) + if (result.type === 'error') { + await input.store.cancelIntent(intent) + return c.json({ + status: 'failed', + redirectUrl: authorizationErrorRedirect(intent, result.error, result.description), + }) + } + return c.json({ + status: 'complete', + redirectUrl: await completeAuthorization(input, intent, result.grant), + }) + }) + } app.post(`/oauth/${input.provider.id}/token`, async (c) => { const client = await authenticateClient(input.store, input.provider.id, c.req.raw) const form = await c.req.formData() @@ -262,10 +304,17 @@ export async function createExternalAuthorizationServer(input: { } const scopes = normalizeScopes(requiredForm(form, 'scope')) const subjectScopes = normalizeScopes(String(subject.payload.scope ?? '')) - if (scopes.some((scope) => !subjectScopes.includes(scope))) { + const authorizationDetails = parseAuthorizationDetails(form.get('authorization_details')) + const scopesGranted = input.provider.validateScopes + ? await input.provider.validateScopes({ + subject: String(subject.payload.sub), + scopes, + authorizationDetails, + }) + : scopes.every((scope) => subjectScopes.includes(scope)) + if (!scopesGranted) { throw oauthError('invalid_scope', 'Requested scope exceeds the connected account.') } - const authorizationDetails = parseAuthorizationDetails(form.get('authorization_details')) const subjectDetails = authorizationDetailsValue(subject.payload.authorization_details) if ( !(input.provider.authorizationDetailsSubset ?? authorizationDetailsSubset)({ @@ -472,6 +521,31 @@ export async function createExternalAuthorizationServer(input: { } } +async function completeAuthorization( + input: Pick[0], 'provider' | 'store'>, + intent: ExternalOAuthIntent, + grant: Omit, +) { + const code = opaque('code') + await input.store.completeIntent(intent, { ...grant, providerId: input.provider.id, clientId: intent.clientId }, code) + const callback = new URL(intent.redirectUri) + callback.searchParams.set('code', code) + callback.searchParams.set('state', intent.realmrootState) + return callback.toString() +} + +function authorizationErrorRedirect( + intent: ExternalOAuthIntent, + error: 'access_denied' | 'server_error', + description: string, +) { + const callback = new URL(intent.redirectUri) + callback.searchParams.set('error', error) + callback.searchParams.set('error_description', description) + callback.searchParams.set('state', intent.realmrootState) + return callback.toString() +} + async function authenticateClient(store: D1ExternalOAuthStore, providerId: string, request: Request) { const header = request.headers.get('authorization') if (!header?.startsWith('Basic ')) throw oauthError('invalid_client', 'Client authentication is required.', 401) diff --git a/src/core/external-oauth-store.ts b/src/core/external-oauth-store.ts index 11f44cc..38e0bdf 100644 --- a/src/core/external-oauth-store.ts +++ b/src/core/external-oauth-store.ts @@ -173,6 +173,14 @@ export class D1ExternalOAuthStore { if (results[1]?.meta.changes !== 1) throw unauthorized('External OAuth authorization was already completed.') } + async cancelIntent(intent: ExternalOAuthIntent) { + const result = await this.db + .prepare("DELETE FROM external_oauth_intent WHERE id = ? AND status = 'pending'") + .bind(intent.id) + .run() + if (result.meta.changes !== 1) throw unauthorized('External OAuth authorization was already completed.') + } + async consumeCode(input: { code: string; clientId: string; redirectUri: string; verifier: string }) { const codeHash = await sha256(input.code) const row = await this.db diff --git a/src/providers/github/adapter.ts b/src/providers/github/adapter.ts index 01badef..b7d76b8 100644 --- a/src/providers/github/adapter.ts +++ b/src/providers/github/adapter.ts @@ -152,7 +152,7 @@ export function createGitHubAdapter( const createToken = await provider.installationToken({ installationId: installation.installationId, permissions: scopesToPermissions(new Set(['pull_requests:write']), available), - repositories: [repository as string], + ...repositoryRestriction(installation, repository as string), }) const response = await createGitHubPullRequestWithRest({ provider, @@ -198,7 +198,7 @@ export function createGitHubAdapter( const commentToken = await provider.installationToken({ installationId: installation.installationId, permissions: scopesToPermissions(new Set([commentScope]), available), - repositories: [repository as string], + ...repositoryRestriction(installation, repository as string), }) const response = await createGitHubCommentWithRest({ provider, @@ -241,7 +241,7 @@ export function createGitHubAdapter( const mergeToken = await provider.installationToken({ installationId: installation.installationId, permissions: scopesToPermissions(new Set(['contents:write']), available), - repositories: [repository as string], + ...repositoryRestriction(installation, repository as string), }) const response = await mergeGitHubPullRequestWithRest({ provider, @@ -297,7 +297,7 @@ export function createGitHubAdapter( const token = await provider.installationToken({ installationId: installation.installationId, permissions, - repositories: [repository as string], + ...repositoryRestriction(installation, repository as string), }) const response = await provider.request( new Request(target.url, { @@ -341,7 +341,7 @@ export function createGitHubAdapter( const token = await provider.installationToken({ installationId: installation.installationId, permissions, - ...(repository ? { repositories: [repository] } : {}), + ...(repository ? repositoryRestriction(installation, repository) : {}), }) const upstreamRequest = new Request(upstream, { method: c.req.method, @@ -489,6 +489,10 @@ function repositoryTarget(path: string, installation: GitHubAuthorizationContext return repository } +function repositoryRestriction(installation: GitHubAuthorizationContext, repository: string) { + return installation.repositorySelection === 'selected' ? { repositories: [repository] } : {} +} + function installationId(value: string) { const parsed = Number(value) if (!Number.isSafeInteger(parsed) || parsed <= 0) throw badRequest('The GitHub installation ID is invalid.') diff --git a/src/providers/github/client.ts b/src/providers/github/client.ts index 4f776f8..bd785ee 100644 --- a/src/providers/github/client.ts +++ b/src/providers/github/client.ts @@ -14,6 +14,7 @@ const userInstallationsSchema = z.object({ installations: z.array( z.object({ id: z.number().int().positive(), + html_url: z.url().refine((value) => new URL(value).origin === 'https://github.com'), account: z.object({ login: z.string().min(1) }), target_type: z.string().min(1), permissions: permissionsSchema, @@ -99,7 +100,7 @@ export function createGitHubProvider(input: GitHubClientInput): GitHubProvider { signal: AbortSignal.timeout(mode === 'git' ? 10 * 60_000 : 30_000), }), ) - if (requireSuccess && !response.ok) throw providerFailure(response, request.url) + if (requireSuccess && !response.ok) throw await providerFailure(response, request.url) return response } } @@ -147,6 +148,7 @@ export function createGitHubConnectionProvider( return Promise.all( parsed.installations.map(async (installation) => ({ id: installation.id, + htmlUrl: installation.html_url, accountLogin: installation.account.login, targetType: installation.target_type, permissions: installation.permissions, @@ -166,6 +168,9 @@ export function createGitHubConnectionProvider( url.searchParams.set('state', state) return url.toString() }, + permissionUpdateUrl(installation) { + return `${installation.htmlUrl}/permissions/update` + }, } async function listInstallationRepositories(token: string, installationId: number) { @@ -224,10 +229,17 @@ function forwardedHeaders(input: Headers) { return headers } -function providerFailure(response: Response, target: string) { +async function providerFailure(response: Response, target: string) { const requestId = response.headers.get('x-github-request-id') + const body = z.object({ message: z.string().min(1).max(500) }).safeParse( + await response + .clone() + .json() + .catch(() => null), + ) + const message = body.success ? `: ${body.data.message}` : '' return failedDependency( - `GitHub rejected ${new URL(target, 'https://api.github.com').pathname} with ${response.status}${requestId ? ` (${requestId})` : ''}.`, + `GitHub rejected ${new URL(target, 'https://api.github.com').pathname} with ${response.status}${requestId ? ` (${requestId})` : ''}${message}.`, ) } diff --git a/src/providers/github/external-authorization.ts b/src/providers/github/external-authorization.ts index 0b00969..d21c7ff 100644 --- a/src/providers/github/external-authorization.ts +++ b/src/providers/github/external-authorization.ts @@ -71,6 +71,7 @@ export function createGitHubExternalAuthorization(input: { const contexts = (await input.connections.externalAuthorization(subject)).contexts const items = contexts.slice(offset, offset + limit).map((context) => ({ authorizationDetail: githubInstallationAuthorizationDetail(context), + grantedScopes: context.scopes, display: githubInstallationAuthorizationDetailDisplay(context), })) const nextOffset = offset + limit < contexts.length ? offset + limit : null @@ -87,13 +88,46 @@ export function createGitHubExternalAuthorization(input: { }, }, authorizationDetailsSubset, + async validateScopes({ subject, scopes, authorizationDetails }) { + const active = await input.connections.externalAuthorization(subject) + const requestedScopes = scopes.filter( + (scope) => !['openid', 'offline_access', authorizationDetailsCatalogScope].includes(scope), + ) + if (authorizationDetails.length === 0) { + return requestedScopes.every((scope) => active.scopes.includes(scope)) + } + const selectedContexts = authorizationDetails.map((detail) => + active.contexts.find( + (context) => + detail.type === GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE && + detail.installation_id === String(context.installationId), + ), + ) + return ( + selectedContexts.every( + (context) => context !== undefined && requestedScopes.every((scope) => context.scopes.includes(scope)), + ) && + authorizationDetailsSubset({ + requested: authorizationDetails, + granted: active.contexts.map(githubInstallationAuthorizationDetail), + }) + ) + }, async validateGrant({ subject, scopes, authorizationDetails }) { const active = await input.connections.externalAuthorization(subject) + const requestedScopes = scopes.filter( + (scope) => !['openid', 'offline_access', authorizationDetailsCatalogScope].includes(scope), + ) + const selectedContexts = authorizationDetails.map((detail) => + active.contexts.find( + (context) => + detail.type === GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE && + detail.installation_id === String(context.installationId), + ), + ) return ( - scopes.every( - (scope) => - ['openid', 'offline_access', authorizationDetailsCatalogScope].includes(scope) || - active.scopes.includes(scope), + selectedContexts.every( + (context) => context !== undefined && requestedScopes.every((scope) => context.scopes.includes(scope)), ) && authorizationDetailsSubset({ requested: authorizationDetails, @@ -117,7 +151,8 @@ export function createGitHubExternalAuthorization(input: { input.connection.getUser(userToken), input.connection.listUserInstallations(userToken), ]) - const expectedInstallationId = numberValue(intent.providerData.expectedInstallationId) + const expectedInstallationId = + numberValue(intent.providerData.expectedInstallationId) ?? requestedInstallationId(intent.authorizationDetails) if (expectedInstallationId && !installations.some((installation) => installation.id === expectedInstallationId)) { throw forbidden('The GitHub user cannot manage the selected App installation.') } @@ -134,14 +169,34 @@ export function createGitHubExternalAuthorization(input: { const selected = expectedInstallationId ? installations.filter((installation) => installation.id === expectedInstallationId) : installations - const contexts = await input.connections.upsertExternalAuthorization(user, selected) const providerScopes = new Set(selected.flatMap((installation) => permissionsToScopes(installation.permissions))) const requestedProviderScopes = intent.scopes.filter( (scope) => !['openid', 'offline_access', authorizationDetailsCatalogScope].includes(scope), ) if (requestedProviderScopes.some((scope) => !providerScopes.has(scope))) { - throw forbidden('The selected GitHub installation does not grant every requested scope.') + const selectedInstallation = selected.length === 1 ? selected[0] : undefined + if (!selectedInstallation) { + return { + type: 'error', + error: 'access_denied', + description: 'The selected GitHub installation permission update was not approved.', + } + } + const providerState = nextProviderState() + return { + type: 'continue', + providerState, + stage: 'permission-update', + data: { + expectedInstallationId: selectedInstallation.id, + permissionUpdateUrl: input.connection.permissionUpdateUrl(selectedInstallation), + subject: String(user.id), + displayName: user.name ?? user.login, + }, + url: `${input.origin}/github/permission-update?state=${encodeURIComponent(providerState)}`, + } } + const contexts = await input.connections.upsertExternalAuthorization(user, installations) return { type: 'complete', grant: { @@ -152,6 +207,34 @@ export function createGitHubExternalAuthorization(input: { }, } }, + async resume({ intent }) { + if (intent.providerStage !== 'permission-update') { + return { type: 'error', error: 'server_error', description: 'GitHub permission update state is invalid.' } + } + const subject = stringValue(intent.providerData.subject) + const displayName = stringValue(intent.providerData.displayName) + const expectedInstallationId = numberValue(intent.providerData.expectedInstallationId) + if (!subject || !displayName || !expectedInstallationId) { + return { type: 'error', error: 'server_error', description: 'GitHub permission update state is incomplete.' } + } + const active = await input.connections.externalAuthorization(subject) + const selected = active.contexts.find((context) => context.installationId === expectedInstallationId) + const requestedProviderScopes = intent.scopes.filter( + (scope) => !['openid', 'offline_access', authorizationDetailsCatalogScope].includes(scope), + ) + if (!selected || requestedProviderScopes.some((scope) => !selected.scopes.includes(scope))) { + return { type: 'pending' } + } + return { + type: 'complete', + grant: { + subject, + displayName, + scopes: intent.scopes, + authorizationDetails: active.contexts.map(githubInstallationAuthorizationDetail), + }, + } + }, } return { @@ -159,6 +242,30 @@ export function createGitHubExternalAuthorization(input: { installationCallback: { id: 'github-installation-callback', register(app: Hono) { + app.get('/github/permission-update', async (c) => { + const state = required(c.req.query('state'), 'state') + const intent = await input.oauthStore.intentByProviderState(await sha256(state)) + if (intent.providerId !== 'github' || intent.providerStage !== 'permission-update') { + throw badRequest('GitHub permission update state is invalid.') + } + const installationId = numberValue(intent.providerData.expectedInstallationId) + if (!installationId) throw badRequest('GitHub permission update installation is invalid.') + const updateUrl = stringValue(intent.providerData.permissionUpdateUrl) + if (!updateUrl) throw badRequest('GitHub permission update URL is invalid.') + const nonce = randomToken() + c.header('Cache-Control', 'no-store') + c.header( + 'Content-Security-Policy', + `default-src 'none'; connect-src 'self'; script-src 'nonce-${nonce}'; style-src 'nonce-${nonce}'; base-uri 'none'; form-action 'none'; frame-ancestors 'none'`, + ) + return c.html( + permissionUpdatePage({ + continueUrl: `${input.origin}/oauth/github/continue?state=${encodeURIComponent(state)}`, + nonce, + updateUrl, + }), + ) + }) app.get('/github/account-connection-installations', async (c) => { const state = required(c.req.query('state'), 'state') const installationId = Number(required(c.req.query('installation_id'), 'installation_id')) @@ -169,13 +276,17 @@ export function createGitHubExternalAuthorization(input: { if (intent.providerId !== 'github' || intent.providerStage !== 'install') { throw badRequest('GitHub installation authorization state is invalid.') } + const expectedInstallationId = numberValue(intent.providerData.expectedInstallationId) + if (expectedInstallationId && expectedInstallationId !== installationId) { + throw forbidden('GitHub returned a different App installation than the authorization requested.') + } const providerState = randomToken() await input.oauthStore.advanceIntent({ id: intent.id, expectedStage: 'install', providerStateHash: await sha256(providerState), providerStage: 'oauth-selected', - providerData: { expectedInstallationId: installationId }, + providerData: { ...intent.providerData, expectedInstallationId: installationId }, }) return c.redirect(input.connection.authorizationUrl(providerState)) }) @@ -188,6 +299,79 @@ function numberValue(value: unknown) { return typeof value === 'number' && Number.isSafeInteger(value) && value > 0 ? value : null } +function stringValue(value: unknown) { + return typeof value === 'string' && value.length > 0 ? value : null +} + +function permissionUpdatePage(input: { continueUrl: string; nonce: string; updateUrl: string }) { + const continueUrl = JSON.stringify(input.continueUrl) + const updateUrl = JSON.stringify(input.updateUrl) + return ` + + + + + Update GitHub permissions + + + +
+

Update GitHub permissions

+

Accept the requested permission in GitHub. This page will return to Realmroot automatically.

+ +
+ + +` +} + +function requestedInstallationId(authorizationDetails: Array>) { + const ids = authorizationDetails.flatMap((detail) => { + if (detail.type !== GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE) return [] + if (typeof detail.installation_id !== 'string' || !/^\d+$/.test(detail.installation_id)) return [] + const id = Number(detail.installation_id) + return Number.isSafeInteger(id) && id > 0 ? [id] : [] + }) + return ids.length === 1 ? ids[0] : null +} + function required(value: string | null | undefined, name: string) { if (!value) throw badRequest(`${name} is required.`) return value diff --git a/src/providers/github/types.ts b/src/providers/github/types.ts index 9169d87..1746880 100644 --- a/src/providers/github/types.ts +++ b/src/providers/github/types.ts @@ -17,6 +17,7 @@ export interface GitHubProvider { export type GitHubUser = Readonly<{ id: number; login: string; name: string | null }> export type GitHubInstallation = Readonly<{ id: number + htmlUrl: string accountLogin: string targetType: string permissions: GitHubPermissions @@ -33,4 +34,5 @@ export interface GitHubConnectionProvider { getUser(token: string): Promise listUserInstallations(token: string): Promise newInstallationUrl(state: string): Promise + permissionUpdateUrl(installation: GitHubInstallation): string } diff --git a/test/app.test.ts b/test/app.test.ts index 857a616..f7bd701 100644 --- a/test/app.test.ts +++ b/test/app.test.ts @@ -196,7 +196,6 @@ describe('GitHub adapter contract', () => { expect(provider.installationToken).toHaveBeenNthCalledWith(2, { installationId: 42, permissions: { pull_requests: 'write' }, - repositories: ['example'], }) }) @@ -255,7 +254,6 @@ describe('GitHub adapter contract', () => { expect(provider.installationToken).toHaveBeenNthCalledWith(2, { installationId: 42, permissions: { issues: 'write' }, - repositories: ['example'], }) }) @@ -300,7 +298,6 @@ describe('GitHub adapter contract', () => { expect(provider.installationToken).toHaveBeenNthCalledWith(2, { installationId: 42, permissions: { contents: 'write' }, - repositories: ['example'], }) }) @@ -336,7 +333,6 @@ describe('GitHub adapter contract', () => { expect(provider.installationToken).toHaveBeenLastCalledWith({ installationId: 42, permissions: { contents: 'read' }, - repositories: ['example'], }) const pushResponse = await app.request('/github/git/realmroot/example.git/git-receive-pack', { method: 'POST', @@ -346,7 +342,6 @@ describe('GitHub adapter contract', () => { expect(provider.installationToken).toHaveBeenLastCalledWith({ installationId: 42, permissions: { contents: 'write', workflows: 'write' }, - repositories: ['example'], }) }) @@ -435,7 +430,6 @@ describe('GitHub adapter contract', () => { expect(provider.installationToken).toHaveBeenCalledWith({ installationId: 42, permissions: { issues: 'write' }, - repositories: ['example'], }) }) diff --git a/test/core/external-authorization-server.test.ts b/test/core/external-authorization-server.test.ts index ab73132..588d302 100644 --- a/test/core/external-authorization-server.test.ts +++ b/test/core/external-authorization-server.test.ts @@ -171,6 +171,77 @@ describe('external authorization server', () => { ) }) + it('resumes a preserved provider authorization after an asynchronous provider decision', async () => { + const intent = { ...exampleIntent(), providerStage: 'permission-update' } + const { app, provider, store } = await testServer({ intent, resume: true }) + if (!provider.resume) throw new Error('Expected resumable provider authorization.') + vi.mocked(provider.resume) + .mockResolvedValueOnce({ type: 'pending' }) + .mockResolvedValueOnce({ + type: 'complete', + grant: { + subject: 'provider-user-1', + displayName: 'Provider User', + scopes: intent.scopes, + authorizationDetails: intent.authorizationDetails, + }, + }) + + const pending = await app.request('/oauth/example/continue?state=provider-state') + expect(pending.status).toBe(202) + expect(pending.headers.get('cache-control')).toBe('no-store') + await expect(pending.json()).resolves.toEqual({ status: 'pending' }) + + const completed = await app.request('/oauth/example/continue?state=provider-state') + expect(completed.status).toBe(200) + const body = (await completed.json()) as { status: string; redirectUrl: string } + expect(body.status).toBe('complete') + const callback = new URL(body.redirectUrl) + expect(callback.origin + callback.pathname).toBe('https://id.realmroot.dev/callback') + expect(callback.searchParams.get('state')).toBe('realmroot-state') + expect(callback.searchParams.get('code')).toMatch(/^code_/) + expect(store.completeIntent).toHaveBeenCalledWith(intent, expect.any(Object), callback.searchParams.get('code')) + }) + + it('returns provider denial to Realmroot without requiring an authorization code', async () => { + const intent = exampleIntent() + const { app, provider, store } = await testServer({ intent }) + + const response = await app.request( + '/oauth/example/provider/callback?state=provider-state&error=access_denied&error_description=The+user+cancelled', + ) + + expect(response.status).toBe(302) + const callback = new URL(response.headers.get('location') ?? '') + expect(callback.origin + callback.pathname).toBe('https://id.realmroot.dev/callback') + expect(callback.searchParams.get('state')).toBe('realmroot-state') + expect(callback.searchParams.get('error')).toBe('access_denied') + expect(callback.searchParams.get('error_description')).toBe('Provider authorization was denied.') + expect(store.cancelIntent).toHaveBeenCalledWith(intent) + expect(provider.complete).not.toHaveBeenCalled() + }) + + it('returns a provider completion denial to Realmroot instead of an Adapter error page', async () => { + const intent = exampleIntent() + const { app, provider, store } = await testServer({ intent }) + vi.mocked(provider.complete).mockResolvedValueOnce({ + type: 'error', + error: 'access_denied', + description: 'The requested provider permission was not approved.', + }) + + const response = await app.request('/oauth/example/provider/callback?state=provider-state&code=provider-code') + + expect(response.status).toBe(302) + const callback = new URL(response.headers.get('location') ?? '') + expect(callback.origin + callback.pathname).toBe('https://id.realmroot.dev/callback') + expect(callback.searchParams.get('state')).toBe('realmroot-state') + expect(callback.searchParams.get('error')).toBe('access_denied') + expect(callback.searchParams.get('error_description')).toBe('The requested provider permission was not approved.') + expect(store.cancelIntent).toHaveBeenCalledWith(intent) + expect(store.completeIntent).not.toHaveBeenCalled() + }) + it('supports a provider callback path already registered with the upstream OAuth application', async () => { const intent = exampleIntent() const { app, store } = await testServer({ intent, providerCallbackPath: '/example/oauth/callback' }) @@ -184,7 +255,9 @@ describe('external authorization server', () => { }) }) -async function testServer(options: { intent?: ExternalOAuthIntent; providerCallbackPath?: string } = {}) { +async function testServer( + options: { intent?: ExternalOAuthIntent; providerCallbackPath?: string; resume?: boolean } = {}, +) { const store = { registerClient: vi.fn(async () => undefined), client: vi.fn(async (_providerId: string, clientId: string) => ({ @@ -198,6 +271,7 @@ async function testServer(options: { intent?: ExternalOAuthIntent; providerCallb intentByProviderState: vi.fn(async () => options.intent ?? exampleIntent()), advanceIntent: vi.fn(async () => undefined), completeIntent: vi.fn(async () => undefined), + cancelIntent: vi.fn(async () => undefined), consumeCode: vi.fn(async () => ({ providerId: 'example', clientId: 'client-1', @@ -238,6 +312,11 @@ async function testServer(options: { intent?: ExternalOAuthIntent; providerCallb authorizationDetails: intent.authorizationDetails, }, })), + ...(options.resume + ? { + resume: vi.fn(async () => ({ type: 'pending' as const })), + } + : {}), } const { privateKey } = await generateKeyPair('ES256', { extractable: true }) const signingPrivateJwk = await exportJWK(privateKey) diff --git a/test/integration/external-oauth-store.test.ts b/test/integration/external-oauth-store.test.ts new file mode 100644 index 0000000..bc50da1 --- /dev/null +++ b/test/integration/external-oauth-store.test.ts @@ -0,0 +1,37 @@ +import { env } from 'cloudflare:test' +import { describe, expect, it } from 'vitest' +import { D1ExternalOAuthStore, type ExternalOAuthIntent, sha256 } from '../../src/core/external-oauth-store.js' + +describe('External OAuth intent persistence', () => { + it('[spec: github-adapter/github-installation-permission-upgrade] consumes a denied authorization intent once', async () => { + const store = new D1ExternalOAuthStore(env.DB) + const intent: ExternalOAuthIntent = { + id: 'github-permission-update-denied', + providerId: 'github', + clientId: 'github-permission-update-client', + redirectUri: 'https://id.realmroot.dev/oauth/account-connection/callback', + realmrootState: 'realmroot-state', + scopes: ['administration:write', 'openid'], + authorizationDetails: [], + codeChallenge: 'challenge', + providerStage: 'oauth-selected', + providerData: { expectedInstallationId: 701, permissionUpdateAttempted: true }, + expiresAt: Date.now() + 60_000, + } + await store.registerClient({ + clientId: intent.clientId, + providerId: 'github', + clientSecretHash: await sha256('secret'), + redirectUris: [intent.redirectUri], + jwksUri: 'https://id.realmroot.dev/api/auth/jwks', + }) + await store.createIntent(intent, await sha256('provider-state')) + + await store.cancelIntent(intent) + + await expect(store.intentByProviderState(await sha256('provider-state'))).rejects.toThrow( + 'External OAuth state is invalid or expired.', + ) + await expect(store.cancelIntent(intent)).rejects.toThrow('External OAuth authorization was already completed.') + }) +}) diff --git a/test/integration/worker.test.ts b/test/integration/worker.test.ts index b2b12e5..a8268d3 100644 --- a/test/integration/worker.test.ts +++ b/test/integration/worker.test.ts @@ -53,6 +53,7 @@ describe('Cloudflare Worker runtime', () => { [ { id: 101, + htmlUrl: 'https://github.com/organizations/realmroot/settings/installations/101', accountLogin: 'realmroot', targetType: 'Organization', permissions: { metadata: 'read', issues: 'write' }, @@ -62,6 +63,7 @@ describe('Cloudflare Worker runtime', () => { }, { id: 102, + htmlUrl: 'https://github.com/settings/installations/102', accountLogin: 'controller', targetType: 'User', permissions: { metadata: 'read', issues: 'write' }, @@ -114,6 +116,7 @@ describe('Cloudflare Worker runtime', () => { [ { id: 103, + htmlUrl: 'https://github.com/organizations/realmroot/settings/installations/103', accountLogin: 'realmroot', targetType: 'Organization', permissions: { metadata: 'read', issues: 'write' }, @@ -177,6 +180,7 @@ describe('Cloudflare Worker runtime', () => { [ { id: 701, + htmlUrl: 'https://github.com/organizations/realmroot/settings/installations/701', accountLogin: 'realmroot', targetType: 'Organization', permissions: { metadata: 'read', pull_requests: 'read' }, @@ -223,6 +227,7 @@ describe('Cloudflare Worker runtime', () => { [ { id: 201, + htmlUrl: 'https://github.com/organizations/realmroot/settings/installations/201', accountLogin: 'realmroot', targetType: 'Organization', permissions: { metadata: 'read' }, @@ -261,6 +266,7 @@ describe('Cloudflare Worker runtime', () => { [ { id: 202, + htmlUrl: 'https://github.com/organizations/realmroot/settings/installations/202', accountLogin: 'realmroot', targetType: 'Organization', permissions: { metadata: 'read' }, diff --git a/test/providers/github-client.test.ts b/test/providers/github-client.test.ts index 16a8e31..e0ef60c 100644 --- a/test/providers/github-client.test.ts +++ b/test/providers/github-client.test.ts @@ -103,6 +103,9 @@ describe('GitHub account connection OAuth boundary', () => { redirect_uri: 'https://adapters.realmroot.dev/github/oauth/callback', }, }) + expect(provider.permissionUpdateUrl({ htmlUrl: 'https://github.com/settings/installations/42' } as never)).toBe( + 'https://github.com/settings/installations/42/permissions/update', + ) }) it('loads selected repository membership with the installation context', async () => { @@ -122,6 +125,7 @@ describe('GitHub account connection OAuth boundary', () => { installations: [ { id: 42, + html_url: 'https://github.com/organizations/realmroot/settings/installations/42', account: { login: 'realmroot' }, target_type: 'Organization', permissions: { metadata: 'read' }, @@ -154,6 +158,7 @@ describe('GitHub account connection OAuth boundary', () => { expect(installations).toHaveLength(1) expect(installations[0]).toMatchObject({ id: 42, + htmlUrl: 'https://github.com/organizations/realmroot/settings/installations/42', accountLogin: 'realmroot', targetType: 'Organization', permissions: { metadata: 'read' }, diff --git a/test/providers/github-external-authorization.test.ts b/test/providers/github-external-authorization.test.ts index 4bacf0d..1ae68da 100644 --- a/test/providers/github-external-authorization.test.ts +++ b/test/providers/github-external-authorization.test.ts @@ -1,8 +1,10 @@ +import { Hono } from 'hono' import { describe, expect, it, vi } from 'vitest' import type { D1ExternalOAuthStore, ExternalOAuthIntent } from '../../src/core/external-oauth-store.js' import { GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE } from '../../src/providers/github/authorization-details.js' import type { D1GitHubConnections } from '../../src/providers/github/connections.js' import { createGitHubExternalAuthorization } from '../../src/providers/github/external-authorization.js' +import type { GitHubInstallation } from '../../src/providers/github/types.js' describe('GitHub external authorization', () => { it('[spec: github-adapter/github-context-catalog] exposes stable installation details with human labels', async () => { @@ -38,6 +40,7 @@ describe('GitHub external authorization', () => { target_type: 'Organization', repository_selection: 'all', }, + grantedScopes: ['metadata:read'], display: { label: 'realmroot', description: 'Organization GitHub App installation', @@ -53,9 +56,87 @@ describe('GitHub external authorization', () => { }) }) + it('rejects a scope that another installation grants but the selected installation does not', async () => { + const contexts = [ + { + installationId: 701, + accountLogin: 'saltbo', + targetType: 'User', + scopes: ['metadata:read'], + repositorySelection: 'all' as const, + repositories: [], + }, + { + installationId: 702, + accountLogin: 'realmroot', + targetType: 'Organization', + scopes: ['administration:write', 'metadata:read'], + repositorySelection: 'all' as const, + repositories: [], + }, + ] + const external = createGitHubExternalAuthorization({ + origin: 'https://adapter.example', + connection: {} as never, + connections: { + externalAuthorization: vi.fn(async () => ({ + scopes: ['administration:write', 'metadata:read'], + contexts, + })), + } as unknown as D1GitHubConnections, + oauthStore: {} as D1ExternalOAuthStore, + scopes: ['administration:write', 'metadata:read'], + }) + + await expect( + external.authorization.validateScopes?.({ + subject: '70', + scopes: ['administration:write'], + authorizationDetails: [{ type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, installation_id: '701' }], + }), + ).resolves.toBe(false) + }) + + it('accepts a scope granted by the selected installation after the subject token was issued', async () => { + const authorizationDetail = { + type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, + installation_id: '701', + } + const external = createGitHubExternalAuthorization({ + origin: 'https://adapter.example', + connection: {} as never, + connections: { + externalAuthorization: vi.fn(async () => ({ + scopes: ['metadata:read'], + contexts: [ + { + installationId: 701, + accountLogin: 'saltbo', + targetType: 'User', + scopes: ['actions:read', 'metadata:read'], + repositorySelection: 'all' as const, + repositories: [], + }, + ], + })), + } as unknown as D1GitHubConnections, + oauthStore: {} as D1ExternalOAuthStore, + scopes: ['actions:read', 'metadata:read'], + }) + + await expect( + external.authorization.validateScopes?.({ + subject: '70', + scopes: ['actions:read'], + authorizationDetails: [authorizationDetail], + }), + ).resolves.toBe(true) + }) + it('accepts read scopes implied by a write installation permission', async () => { const installation = { id: 701, + htmlUrl: 'https://github.com/organizations/realmroot/settings/installations/701', accountLogin: 'realmroot', targetType: 'Organization', permissions: { metadata: 'read', pull_requests: 'write' } as const, @@ -83,6 +164,7 @@ describe('GitHub external authorization', () => { getUser: vi.fn(async () => ({ id: 70, login: 'controller', name: 'Controller' })), listUserInstallations: vi.fn(async () => [installation]), newInstallationUrl: vi.fn(async () => 'https://github.com/apps/example/installations/new'), + permissionUpdateUrl: vi.fn((installation: GitHubInstallation) => `${installation.htmlUrl}/permissions/update`), }, connections: connections as unknown as D1GitHubConnections, oauthStore: {} as D1ExternalOAuthStore, @@ -100,8 +182,226 @@ describe('GitHub external authorization', () => { grant: { scopes: ['metadata:read', 'openid', 'pull_requests:read'] }, }) }) + + it('returns the complete installation snapshot after a concrete connection request', async () => { + const selected = githubInstallation({ administration: 'write' }) + const other = { + ...selected, + id: 702, + htmlUrl: 'https://github.com/settings/installations/702', + accountLogin: 'controller', + targetType: 'User', + } + const external = createGitHubExternalAuthorization({ + origin: 'https://adapter.example', + connection: githubConnection([selected, other]), + connections: { + upsertExternalAuthorization: vi.fn(async () => [connectionContext(selected), connectionContext(other)]), + } as unknown as D1GitHubConnections, + oauthStore: {} as D1ExternalOAuthStore, + scopes: ['administration:read'], + }) + + await expect( + external.authorization.complete({ + callbackUrl: 'https://adapter.example/github/oauth/callback?code=provider-code', + intent: { + ...intent(), + scopes: ['administration:read', 'openid'], + authorizationDetails: [ + { type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, installation_id: String(selected.id) }, + ], + }, + nextProviderState: () => 'next-state', + }), + ).resolves.toMatchObject({ + type: 'complete', + grant: { + authorizationDetails: [ + expect.objectContaining({ + type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, + installation_id: String(selected.id), + }), + expect.objectContaining({ + type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, + installation_id: String(other.id), + }), + ], + }, + }) + }) + + it('[spec: github-adapter/github-installation-permission-upgrade] continues through a target installation permission update', async () => { + const installation = githubInstallation({ administration: 'read' }) + const connections = { upsertExternalAuthorization: vi.fn() } + const connection = githubConnection([installation]) + const external = createGitHubExternalAuthorization({ + origin: 'https://adapter.example', + connection, + connections: connections as unknown as D1GitHubConnections, + oauthStore: {} as D1ExternalOAuthStore, + scopes: ['administration:read', 'administration:write'], + }) + + await expect( + external.authorization.complete({ + callbackUrl: 'https://adapter.example/github/oauth/callback?code=provider-code', + intent: { + ...intent(), + scopes: ['administration:write', 'openid'], + authorizationDetails: [{ type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, installation_id: '701' }], + }, + nextProviderState: () => 'permission-update-state', + }), + ).resolves.toEqual({ + type: 'continue', + providerState: 'permission-update-state', + stage: 'permission-update', + data: { + expectedInstallationId: 701, + permissionUpdateUrl: 'https://github.com/organizations/realmroot/settings/installations/701/permissions/update', + subject: '70', + displayName: 'Controller', + }, + url: 'https://adapter.example/github/permission-update?state=permission-update-state', + }) + expect(connection.newInstallationUrl).not.toHaveBeenCalled() + expect(connections.upsertExternalAuthorization).not.toHaveBeenCalled() + }) + + it('[spec: github-adapter/github-installation-permission-upgrade] resumes only after the target installation accepts the permission', async () => { + const before = githubInstallation({ administration: 'read' }) + const after = githubInstallation({ administration: 'write' }) + const other = { + ...after, + id: 702, + htmlUrl: 'https://github.com/settings/installations/702', + accountLogin: 'controller', + targetType: 'User', + } + const externalAuthorization = vi + .fn() + .mockResolvedValueOnce({ scopes: ['administration:read'], contexts: [connectionContext(before)] }) + .mockResolvedValueOnce({ + scopes: ['administration:read', 'administration:write'], + contexts: [connectionContext(after), connectionContext(other)], + }) + const external = createGitHubExternalAuthorization({ + origin: 'https://adapter.example', + connection: githubConnection([before]), + connections: { externalAuthorization } as unknown as D1GitHubConnections, + oauthStore: {} as D1ExternalOAuthStore, + scopes: ['administration:read', 'administration:write'], + }) + const permissionUpdateIntent = { + ...intent(), + scopes: ['administration:write', 'openid'], + authorizationDetails: [{ type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, installation_id: '701' }], + providerStage: 'permission-update', + providerData: { expectedInstallationId: 701, subject: '70', displayName: 'Controller' }, + } + + await expect(external.authorization.resume?.({ intent: permissionUpdateIntent })).resolves.toEqual({ + type: 'pending', + }) + await expect(external.authorization.resume?.({ intent: permissionUpdateIntent })).resolves.toEqual({ + type: 'complete', + grant: { + subject: '70', + displayName: 'Controller', + scopes: ['administration:write', 'openid'], + authorizationDetails: [ + expect.objectContaining({ + type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, + installation_id: '701', + }), + expect.objectContaining({ + type: GITHUB_INSTALLATION_AUTHORIZATION_DETAIL_TYPE, + installation_id: '702', + }), + ], + }, + }) + }) + + it('[spec: github-adapter/github-installation-permission-upgrade] opens the exact installation permission review and polls the preserved intent', async () => { + const oauthStore = { + intentByProviderState: vi.fn(async () => ({ + ...intent(), + providerStage: 'permission-update', + providerData: { + expectedInstallationId: 701, + permissionUpdateUrl: + 'https://github.com/organizations/realmroot/settings/installations/701/permissions/update', + subject: '70', + displayName: 'Controller', + }, + })), + } + const connection = githubConnection([]) + const external = createGitHubExternalAuthorization({ + origin: 'https://adapter.example', + connection, + connections: {} as D1GitHubConnections, + oauthStore: oauthStore as unknown as D1ExternalOAuthStore, + scopes: ['administration:write'], + }) + const app = new Hono() + external.installationCallback.register(app as never) + + const response = await app.request('https://adapter.example/github/permission-update?state=permission-update-state') + + expect(response.status).toBe(200) + expect(response.headers.get('cache-control')).toBe('no-store') + await expect(response.text()).resolves.toContain( + 'https://github.com/organizations/realmroot/settings/installations/701/permissions/update', + ) + }) }) +function githubInstallation(permissions: Record) { + return { + id: 701, + htmlUrl: 'https://github.com/organizations/realmroot/settings/installations/701', + accountLogin: 'realmroot', + targetType: 'Organization', + permissions: { metadata: 'read' as const, ...permissions }, + repositorySelection: 'all' as const, + repositories: [], + updatedAt: '2027-01-15T07:00:00.000Z', + } +} + +function githubConnection(installations: ReturnType[]) { + return { + authorizationUrl: vi.fn((state: string) => `https://github.com/login/oauth/authorize?state=${state}`), + exchangeUserCode: vi.fn(async () => 'user-token'), + getUser: vi.fn(async () => ({ id: 70, login: 'controller', name: 'Controller' })), + listUserInstallations: vi.fn(async () => installations), + newInstallationUrl: vi.fn( + async (state: string) => `https://github.com/apps/example/installations/new?state=${state}`, + ), + permissionUpdateUrl: vi.fn( + (installation: ReturnType) => `${installation.htmlUrl}/permissions/update`, + ), + } +} + +function connectionContext(installation: ReturnType) { + const permissions = installation.permissions as Record + return { + installationId: installation.id, + accountLogin: installation.accountLogin, + targetType: installation.targetType, + scopes: + permissions.administration === 'write' + ? ['administration:read', 'administration:write', 'metadata:read'] + : ['administration:read', 'metadata:read'], + repositorySelection: installation.repositorySelection, + repositories: [], + } +} + function intent(): ExternalOAuthIntent { return { id: 'intent-1',