use errorHandler()

This commit is contained in:
Kagami Sascha Rosylight 2023-04-15 23:15:37 +02:00
parent 937e9be34e
commit 8e7fc1ed98
2 changed files with 68 additions and 45 deletions

View file

@ -5,7 +5,7 @@ import { Inject, Injectable } from '@nestjs/common';
import { JSDOM } from 'jsdom';
import httpLinkHeader from 'http-link-header';
import ipaddr from 'ipaddr.js';
import oauth2orize, { type OAuth2 } from 'oauth2orize';
import oauth2orize, { type OAuth2, AuthorizationError } from 'oauth2orize';
import * as oauth2Query from 'oauth2orize/lib/response/query.js';
import oauth2Pkce from 'oauth2orize-pkce';
import expressSession from 'express-session';
@ -33,13 +33,13 @@ function validateClientId(raw: string): URL {
const url = ((): URL => {
try {
return new URL(raw);
} catch { throw new Error('client_id must be a valid URL'); }
} catch { throw new AuthorizationError('client_id must be a valid URL', 'invalid_request'); }
})();
// Client identifier URLs MUST have either an https or http scheme
// XXX: but why allow http in 2023?
if (!['http:', 'https:'].includes(url.protocol)) {
throw new Error('client_id must be either https or http URL');
throw new AuthorizationError('client_id must be either https or http URL', 'invalid_request');
}
// MUST contain a path component (new URL() implicitly adds one)
@ -48,17 +48,17 @@ function validateClientId(raw: string): URL {
// url.
const segments = url.pathname.split('/');
if (segments.includes('.') || segments.includes('..')) {
throw new Error('client_id must not contain dot path segments');
throw new AuthorizationError('client_id must not contain dot path segments', 'invalid_request');
}
// MUST NOT contain a fragment component
if (url.hash) {
throw new Error('client_id must not contain a fragment component');
throw new AuthorizationError('client_id must not contain a fragment component', 'invalid_request');
}
// MUST NOT contain a username or password component
if (url.username || url.password) {
throw new Error('client_id must not contain a username or a password');
throw new AuthorizationError('client_id must not contain a username or a password', 'invalid_request');
}
// (MAY contain a port)
@ -66,7 +66,7 @@ function validateClientId(raw: string): URL {
// host names MUST be domain names or a loopback interface and MUST NOT be
// IPv4 or IPv6 addresses except for IPv4 127.0.0.1 or IPv6 [::1].
if (!url.hostname.match(/\.\w+$/) && !['localhost', '127.0.0.1', '[::1]'].includes(url.hostname)) {
throw new Error('client_id must have a domain name as a host name');
throw new AuthorizationError('client_id must have a domain name as a host name', 'invalid_request');
}
return url;
@ -100,7 +100,7 @@ async function discoverClientInformation(httpRequestService: HttpRequestService,
name,
};
} catch {
throw new Error('Failed to fetch client information');
throw new AuthorizationError('Failed to fetch client information', 'server_error');
}
}
@ -342,7 +342,7 @@ export class OAuth2ProviderService {
const user = await this.cacheService.localUserByNativeTokenCache.fetch(token,
() => this.usersRepository.findOneBy({ token }) as Promise<LocalUser | null>);
if (!user) {
throw new Error('No such user');
throw new AuthorizationError('No such user', 'invalid_request');
}
TEMP_GRANT_CODES[code] = {
@ -360,6 +360,7 @@ export class OAuth2ProviderService {
const granted = TEMP_GRANT_CODES[code];
console.log(granted, body, code, redirectUri);
if (!granted) {
// TODO: throw TokenError?
return [false];
}
delete TEMP_GRANT_CODES[code];
@ -435,21 +436,26 @@ export class OAuth2ProviderService {
const oauth2 = (request.raw as any).oauth2 as OAuth2;
console.log(oauth2, request.raw.session);
if (request.query.response_type !== 'code') {
throw new Error('`response_type` parameter must be set as "code"');
}
if (typeof request.query.code_challenge !== 'string') {
throw new Error('`code_challenge` parameter is required');
}
if (request.query.code_challenge_method !== 'S256') {
throw new Error('`code_challenge_method` parameter must be set as S256');
}
const scopes = [...new Set(oauth2.req.scope)].filter(s => kinds.includes(s));
if (!scopes.length) {
throw new Error('`scope` parameter has no known scope');
try {
if (!scopes.length) {
throw new AuthorizationError('`scope` parameter has no known scope', 'invalid_scope');
}
oauth2.req.scope = scopes;
if (request.query.response_type !== 'code') {
throw new AuthorizationError('`response_type` parameter must be set as "code"', 'invalid_request');
}
if (typeof request.query.code_challenge !== 'string') {
throw new AuthorizationError('`code_challenge` parameter is required', 'invalid_request');
}
if (request.query.code_challenge_method !== 'S256') {
throw new AuthorizationError('`code_challenge_method` parameter must be set as S256', 'invalid_request');
}
} catch (err: any) {
this.#server.errorHandler()(err, request.raw, reply.raw, null as any);
return;
}
oauth2.req.scope = scopes;
reply.header('Cache-Control', 'no-store');
return await reply.view('oauth', {
@ -483,19 +489,20 @@ export class OAuth2ProviderService {
if (process.env.NODE_ENV !== 'test' || process.env.MISSKEY_TEST_DISALLOW_LOOPBACK === '1') {
const lookup = await dns.lookup(clientUrl.hostname);
if (ipaddr.parse(lookup.address).range() === 'loopback') {
throw new Error('client_id unexpectedly resolves to loopback IP.');
throw new AuthorizationError('client_id unexpectedly resolves to loopback IP.', 'invalid_request');
}
}
// Find client information from the remote.
const clientInfo = await discoverClientInformation(this.httpRequestService, clientUrl.href);
if (!clientInfo.redirectUris.includes(redirectUri)) {
throw new Error('Invalid redirect_uri');
throw new AuthorizationError('Invalid redirect_uri', 'invalid_request');
}
return [clientInfo, redirectUri];
})().then(args => done(null, ...args), err => done(err));
}));
fastify.use('/oauth/authorize', this.#server.errorHandler()); // TODO: use mode: indirect?
// for (const middleware of this.#server.decision()) {
fastify.use('/oauth/decision', bodyParser.urlencoded({ extended: false }));
@ -504,11 +511,13 @@ export class OAuth2ProviderService {
req.user = (req as any).body.login_token;
done(null, undefined);
}));
fastify.use('/oauth/decision', this.#server.errorHandler());
// Clients may use JSON or urlencoded
fastify.use('/oauth/token', bodyParser.urlencoded({ extended: false }));
fastify.use('/oauth/token', bodyParser.json({ strict: true }));
fastify.use('/oauth/token', this.#server.token());
fastify.use('/oauth/token', this.#server.errorHandler());
// }
// fastify.use('/oauth', this.#provider.callback());

View file

@ -14,6 +14,11 @@ const host = `http://127.0.0.1:${port}`;
const clientPort = port + 1;
const redirect_uri = `http://127.0.0.1:${clientPort}/redirect`;
interface OAuthError {
error: string;
code: string;
}
function getClient(): AuthorizationCode<'client_id'> {
return new AuthorizationCode({
client: {
@ -233,7 +238,8 @@ describe('OAuth', () => {
scope: 'write:notes',
state: 'state',
}));
assert.ok(!response.ok);
assert.strictEqual(response.status, 400);
assert.strictEqual((await response.json() as any).error, 'invalid_request');
// Pattern 2: Only code_challenge
response = await fetch(client.authorizeURL({
@ -242,7 +248,8 @@ describe('OAuth', () => {
state: 'state',
code_challenge: 'code',
}));
assert.ok(!response.ok);
assert.strictEqual(response.status, 400);
assert.strictEqual((await response.json() as any).error, 'invalid_request');
// Pattern 2: Only code_challenge_method
response = await fetch(client.authorizeURL({
@ -251,7 +258,8 @@ describe('OAuth', () => {
state: 'state',
code_challenge_method: 'S256',
}));
assert.ok(!response.ok);
assert.strictEqual(response.status, 400);
assert.strictEqual((await response.json() as any).error, 'invalid_request');
// Pattern 3: Unsupported code_challenge_method
response = await fetch(client.authorizeURL({
@ -261,7 +269,8 @@ describe('OAuth', () => {
code_challenge: 'code',
code_challenge_method: 'SSSS',
}));
assert.ok(!response.ok);
assert.strictEqual(response.status, 400);
assert.strictEqual((await response.json() as any).error, 'invalid_request');
});
test('Verify PKCE', async () => {
@ -347,8 +356,8 @@ describe('OAuth', () => {
code_challenge_method: 'S256',
}));
// TODO: But 500 is not a valid code, should be 403 or such. Check the OAuth spec
assert.strictEqual(response.status, 500);
assert.strictEqual(response.status, 400);
assert.strictEqual((await response.json() as any).error, 'invalid_scope');
});
test('Empty scope', async () => {
@ -362,8 +371,8 @@ describe('OAuth', () => {
code_challenge_method: 'S256',
}));
// TODO: But 500 is not a valid code, should be 403 or such. Check the OAuth spec
assert.strictEqual(response.status, 500);
assert.strictEqual(response.status, 400);
assert.strictEqual((await response.json() as any).error, 'invalid_scope');
});
test('Unknown scopes', async () => {
@ -377,8 +386,8 @@ describe('OAuth', () => {
code_challenge_method: 'S256',
}));
// TODO: But 500 is not a valid code, should be 403 or such. Check the OAuth spec
assert.strictEqual(response.status, 500);
assert.strictEqual(response.status, 400);
assert.strictEqual((await response.json() as any).error, 'invalid_scope');
});
test('Partially known scopes', async () => {
@ -566,8 +575,9 @@ describe('OAuth', () => {
code_challenge: 'code',
code_challenge_method: 'S256',
}));
// TODO: status code
assert.strictEqual(response.status, 500);
assert.strictEqual(response.status, 400);
assert.strictEqual((await response.json() as any).error, 'invalid_request');
});
test('Invalid redirect_uri including the valid one at authorization endpoint', async () => {
@ -580,8 +590,9 @@ describe('OAuth', () => {
code_challenge: 'code',
code_challenge_method: 'S256',
}));
// TODO: status code
assert.strictEqual(response.status, 500);
assert.strictEqual(response.status, 400);
assert.strictEqual((await response.json() as any).error, 'invalid_request');
});
test('No redirect_uri at authorization endpoint', async () => {
@ -593,8 +604,9 @@ describe('OAuth', () => {
code_challenge: 'code',
code_challenge_method: 'S256',
}));
// TODO: status code
assert.strictEqual(response.status, 500);
assert.strictEqual(response.status, 400);
assert.strictEqual((await response.json() as any).error, 'invalid_request');
});
test('Invalid redirect_uri at token endpoint', async () => {
@ -812,8 +824,9 @@ describe('OAuth', () => {
code_challenge: 'code',
code_challenge_method: 'S256',
}));
// TODO: status code
assert.strictEqual(response.status, 500);
assert.strictEqual(response.status, 400);
assert.strictEqual((await response.json() as any).error, 'invalid_request');
});
});
@ -828,8 +841,9 @@ describe('OAuth', () => {
code_challenge: 'code',
code_challenge_method: 'S256',
}));
// TODO: status code
assert.strictEqual(response.status, 500);
assert.strictEqual(response.status, 400);
assert.strictEqual((await response.json() as any).error, 'invalid_request');
});
test('Missing name', async () => {
@ -856,5 +870,5 @@ describe('OAuth', () => {
});
});
// TODO: Error format required by OAuth spec
// TODO: Invalid decision endpoint parameters
});