Decision endpoint tests

packages/backend/src/server/oauth/OAuth2ProviderService.ts

@@ -115,15 +115,14 @@ class OAuth2Store {
 	#cache = new MemoryKVCache<OAuth2>(1000 * 60 * 5); // 5min
 
 	load(req: any, cb: (err: Error | null, txn?: OAuth2) => void): void {
-		console.log(req);
 		const { transaction_id } = req.body;
 		if (!transaction_id) {
-			cb(new Error('Missing transaction ID'));
+			cb(new AuthorizationError('Missing transaction ID', 'invalid_request'));
 			return;
 		}
 		const loaded = this.#cache.get(transaction_id);
 		if (!loaded) {
-			cb(new Error('Failed to load transaction'));
+			cb(new AuthorizationError('Failed to load transaction', 'access_denied'));
 			return;
 		}
 		cb(null, loaded);
@@ -187,6 +186,9 @@ export class OAuth2ProviderService {
 				console.log('HIT grant code:', client, redirectUri, token, ares, areq);
 				const code = secureRndstr(32, true);
 
+				if (!token) {
+					throw new AuthorizationError('No user', 'invalid_request');
+				}
 				const user = await this.cacheService.localUserByNativeTokenCache.fetch(token,
 					() => this.usersRepository.findOneBy({ token }) as Promise<LocalUser | null>);
 				if (!user) {

packages/backend/test/e2e/oauth.ts

@@ -19,6 +19,14 @@ const host = `http://127.0.0.1:${port}`;
 const clientPort = port + 1;
 const redirect_uri = `http://127.0.0.1:${clientPort}/redirect`;
 
+const basicAuthParams: AuthorizationParamsExtended = {
+	redirect_uri,
+	scope: 'write:notes',
+	state: 'state',
+	code_challenge: 'code',
+	code_challenge_method: 'S256',
+};
+
 interface OAuthErrorResponse {
 	error: string;
 	error_description: string;
@@ -95,7 +103,6 @@ async function fetchAuthorizationCode(user: misskey.entities.MeSignup, scope: st
 	} as AuthorizationParamsExtended));
 	assert.strictEqual(response.status, 200);
 
-	// TODO: this fetch-decision-code checks are everywhere, maybe get a helper for this.
 	const decisionResponse = await fetchDecisionFromResponse(response, user);
 	assert.strictEqual(decisionResponse.status, 302);
 
@@ -532,7 +539,7 @@ describe('OAuth', () => {
 		// RFC 6750 section 3.1 says 401 but it's SHOULD not MUST. 403 should be okay for now.
 		assert.strictEqual(createResponse.status, 403);
 
-		// TODO: error code (invalid_token)
+		// TODO: error code (wrong Authorization header should emit OAuth error instead of Misskey API error)
 	});
 
 	describe('Redirection', () => {
@@ -625,6 +632,65 @@ describe('OAuth', () => {
 		assert.ok(body.scopes_supported.includes('write:notes'));
 	});
 
+	describe('Decision endpoint', () => {
+		test('No login token', async () => {
+			const client = getClient();
+
+			const response = await fetch(client.authorizeURL(basicAuthParams));
+			assert.strictEqual(response.status, 200);
+
+			const { transactionId } = getMeta(await response.text());
+			assert.ok(transactionId);
+
+			const decisionResponse = await fetch(new URL('/oauth/decision', host), {
+				method: 'post',
+				body: new URLSearchParams({
+					transaction_id: transactionId,
+				}),
+				redirect: 'manual',
+				headers: {
+					'content-type': 'application/x-www-form-urlencoded',
+				},
+			});
+
+			assert.strictEqual(decisionResponse.status, 400);
+			assert.strictEqual((await decisionResponse.json() as OAuthErrorResponse).error, 'invalid_request');
+		});
+
+		test('No transaction ID', async () => {
+			const decisionResponse = await fetch(new URL('/oauth/decision', host), {
+				method: 'post',
+				body: new URLSearchParams({
+					login_token: alice.token,
+				}),
+				redirect: 'manual',
+				headers: {
+					'content-type': 'application/x-www-form-urlencoded',
+				},
+			});
+
+			assert.strictEqual(decisionResponse.status, 400);
+			assert.strictEqual((await decisionResponse.json() as OAuthErrorResponse).error, 'invalid_request');
+		});
+
+		test('Invalid transaction ID', async () => {
+			const decisionResponse = await fetch(new URL('/oauth/decision', host), {
+				method: 'post',
+				body: new URLSearchParams({
+					login_token: alice.token,
+					transaction_id: 'invalid_id',
+				}),
+				redirect: 'manual',
+				headers: {
+					'content-type': 'application/x-www-form-urlencoded',
+				},
+			});
+
+			assert.strictEqual(decisionResponse.status, 403);
+			assert.strictEqual((await decisionResponse.json() as OAuthErrorResponse).error, 'access_denied');
+		});
+	});
+
 	describe('Client Information Discovery', () => {
 		describe('Redirection', () => {
 			const tests: Record<string, (reply: FastifyReply) => void> = {
@@ -748,8 +814,6 @@ describe('OAuth', () => {
 		});
 	});
 
-	// TODO: Invalid decision endpoint parameters
-
 	// TODO: Unknown OAuth endpoint
 
 	// TODO: successful token exchange should invalidate the grant token (spec?)