diff --git a/packages/backend/src/server/oauth/OAuth2ProviderService.ts b/packages/backend/src/server/oauth/OAuth2ProviderService.ts index da84d876a..5b954cbf6 100644 --- a/packages/backend/src/server/oauth/OAuth2ProviderService.ts +++ b/packages/backend/src/server/oauth/OAuth2ProviderService.ts @@ -18,11 +18,11 @@ import type { Config } from '@/config.js'; import { DI } from '@/di-symbols.js'; import { bindThis } from '@/decorators.js'; import type { AccessTokensRepository, UsersRepository } from '@/models/index.js'; -import type { IdService } from '@/core/IdService.js'; -import type { CacheService } from '@/core/CacheService.js'; +import { IdService } from '@/core/IdService.js'; +import { CacheService } from '@/core/CacheService.js'; import type { LocalUser } from '@/models/entities/User.js'; import { MemoryKVCache } from '@/misc/cache.js'; -import type { LoggerService } from '@/core/LoggerService.js'; +import { LoggerService } from '@/core/LoggerService.js'; import Logger from '@/logger.js'; import type { ServerResponse } from 'node:http'; import type { FastifyInstance } from 'fastify'; @@ -376,9 +376,9 @@ export class OAuth2ProviderService { } areq.scope = scopes; - if (type !== 'code') { - throw new AuthorizationError('`response_type` parameter must be set as "code"', 'invalid_request'); - } + // Require PKCE parameters. + // Recommended by https://indieauth.spec.indieweb.org/#authorization-request, but also prevents downgrade attack: + // https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics#name-pkce-downgrade-attack if (typeof codeChallenge !== 'string') { throw new AuthorizationError('`code_challenge` parameter is required', 'invalid_request'); } diff --git a/packages/backend/test/e2e/oauth.ts b/packages/backend/test/e2e/oauth.ts index dbef0e457..b515eabac 100644 --- a/packages/backend/test/e2e/oauth.ts +++ b/packages/backend/test/e2e/oauth.ts @@ -6,7 +6,7 @@ process.env.NODE_ENV = 'test'; import * as assert from 'assert'; -import { AuthorizationCode, type AuthorizationTokenConfig } from 'simple-oauth2'; +import { AuthorizationCode, ResourceOwnerPassword, type AuthorizationTokenConfig, ClientCredentials } from 'simple-oauth2'; import pkceChallenge from 'pkce-challenge'; import { JSDOM } from 'jsdom'; import type * as misskey from 'misskey-js'; @@ -375,7 +375,10 @@ describe('OAuth', () => { code, redirect_uri, code_verifier: wrong_verifier, - } as AuthorizationTokenConfigExtended)); + } as AuthorizationTokenConfigExtended), (err: any) => { + assert.strictEqual(err.data.payload.error, 'invalid_grant'); + return true; + }); }); } }); @@ -400,20 +403,29 @@ describe('OAuth', () => { code, redirect_uri, code_verifier, - } as AuthorizationTokenConfigExtended)); + } as AuthorizationTokenConfigExtended), (err: any) => { + assert.strictEqual(err.data.payload.error, 'invalid_grant'); + return true; + }); }); test('On failure', async () => { const { code_challenge, code_verifier } = await pkceChallenge(128); const { client, code } = await fetchAuthorizationCode(alice, 'write:notes', code_challenge); - await assert.rejects(client.getToken({ code, redirect_uri })); + await assert.rejects(client.getToken({ code, redirect_uri }), (err: any) => { + assert.strictEqual(err.data.payload.error, 'invalid_grant'); + return true; + }); await assert.rejects(client.getToken({ code, redirect_uri, code_verifier, - } as AuthorizationTokenConfigExtended)); + } as AuthorizationTokenConfigExtended), (err: any) => { + assert.strictEqual(err.data.payload.error, 'invalid_grant'); + return true; + }); }); }); @@ -660,7 +672,10 @@ describe('OAuth', () => { code, redirect_uri: 'http://127.0.0.2/', code_verifier, - } as AuthorizationTokenConfigExtended)); + } as AuthorizationTokenConfigExtended), (err: any) => { + assert.strictEqual(err.data.payload.error, 'invalid_grant'); + return true; + }); }); test('Invalid redirect_uri including the valid one at token endpoint', async () => { @@ -672,7 +687,10 @@ describe('OAuth', () => { code, redirect_uri: 'http://127.0.0.1/redirection', code_verifier, - } as AuthorizationTokenConfigExtended)); + } as AuthorizationTokenConfigExtended), (err: any) => { + assert.strictEqual(err.data.payload.error, 'invalid_grant'); + return true; + }); }); test('No redirect_uri at token endpoint', async () => { @@ -683,7 +701,10 @@ describe('OAuth', () => { await assert.rejects(client.getToken({ code, code_verifier, - } as AuthorizationTokenConfigExtended)); + } as AuthorizationTokenConfigExtended), (err: any) => { + assert.strictEqual(err.data.payload.error, 'invalid_grant'); + return true; + }); }); }); @@ -752,6 +773,61 @@ describe('OAuth', () => { }); }); + // Only authorization code grant is supported + describe('Grant type', () => { + test('Implicit grant is not supported', async () => { + const url = new URL('/oauth/authorize', host); + url.searchParams.append('response_type', 'token'); + const response = await fetch(url); + assertDirectError(response, 501, 'unsupported_response_type'); + }); + + test('Resource owner grant is not supported', async () => { + const client = new ResourceOwnerPassword({ + client: { + id: `http://127.0.0.1:${clientPort}/`, + secret: '', + }, + auth: { + tokenHost: host, + tokenPath: '/oauth/token', + }, + options: { + authorizationMethod: 'body', + }, + }); + + await assert.rejects(client.getToken({ + username: 'alice', + password: 'test', + }), (err: any) => { + assert.strictEqual(err.data.payload.error, 'unsupported_grant_type'); + return true; + }); + }); + + test('Client credential grant is not supported', async () => { + const client = new ClientCredentials({ + client: { + id: `http://127.0.0.1:${clientPort}/`, + secret: '', + }, + auth: { + tokenHost: host, + tokenPath: '/oauth/token', + }, + options: { + authorizationMethod: 'body', + }, + }); + + await assert.rejects(client.getToken({}), (err: any) => { + assert.strictEqual(err.data.payload.error, 'unsupported_grant_type'); + return true; + }); + }); + }); + // https://indieauth.spec.indieweb.org/#client-information-discovery describe('Client Information Discovery', () => { describe('Redirection', () => {