From ecdd1c115a9dfbab3a86fdb04aa5161f39f75eb3 Mon Sep 17 00:00:00 2001
From: Kagami Sascha Rosylight <saschanaz@outlook.com>
Date: Sat, 17 Jun 2023 22:03:03 +0200
Subject: [PATCH] Revoke access token if the code is reused

---
 .../src/server/oauth/OAuth2ProviderService.ts | 30 ++++++++++++--
 packages/backend/test/e2e/oauth.ts            | 41 ++++++++++++++++++-
 2 files changed, 67 insertions(+), 4 deletions(-)

diff --git a/packages/backend/src/server/oauth/OAuth2ProviderService.ts b/packages/backend/src/server/oauth/OAuth2ProviderService.ts
index 7a5c90d87b..4c639c7fa6 100644
--- a/packages/backend/src/server/oauth/OAuth2ProviderService.ts
+++ b/packages/backend/src/server/oauth/OAuth2ProviderService.ts
@@ -224,6 +224,11 @@ export class OAuth2ProviderService {
 			redirectUri: string,
 			codeChallenge: string,
 			scopes: string[],
+
+			// fields to prevent multiple code use
+			grantedToken?: string,
+			revoked?: boolean,
+			used?: boolean,
 		}>(1000 * 60 * 5); // 5m
 
 		// https://datatracker.ietf.org/doc/html/rfc7636.html
@@ -262,7 +267,21 @@ export class OAuth2ProviderService {
 				if (!granted) {
 					return;
 				}
-				grantCodeCache.delete(code);
+
+				// https://datatracker.ietf.org/doc/html/rfc6749.html#section-4.1.2
+				// "If an authorization code is used more than once, the authorization server
+				// MUST deny the request and SHOULD revoke (when possible) all tokens
+				// previously issued based on that authorization code."
+				if (granted.used) {
+					this.#logger.info(`Detected multiple code use from ${granted.clientId} for user ${granted.userId}. Revoking the code.`);
+					grantCodeCache.delete(code);
+					granted.revoked = true;
+					if (granted.grantedToken) {
+						await accessTokensRepository.delete({ token: granted.grantedToken });
+					}
+					return;
+				}
+				granted.used = true;
 
 				// https://datatracker.ietf.org/doc/html/rfc6749.html#section-4.1.3
 				if (body.client_id !== granted.clientId) return;
@@ -273,10 +292,8 @@ export class OAuth2ProviderService {
 				if (!(await verifyChallenge(body.code_verifier as string, granted.codeChallenge))) return;
 
 				const accessToken = secureRndstr(128, true);
-
 				const now = new Date();
 
-				// Insert access token doc
 				await accessTokensRepository.insert({
 					id: idService.genId(),
 					createdAt: now,
@@ -288,6 +305,13 @@ export class OAuth2ProviderService {
 					permission: granted.scopes,
 				});
 
+				if (granted.revoked) {
+					this.#logger.info('Canceling the token as the authorization code was revoked in parallel during the process.');
+					await accessTokensRepository.delete({ token: accessToken });
+					return;
+				}
+
+				granted.grantedToken = accessToken;
 				this.#logger.info(`Generated access token for ${granted.clientId} for user ${granted.userId}, with scope: [${granted.scopes}]`);
 
 				return [accessToken, undefined, { scope: granted.scopes.join(' ') }];
diff --git a/packages/backend/test/e2e/oauth.ts b/packages/backend/test/e2e/oauth.ts
index 776f00c723..921b140448 100644
--- a/packages/backend/test/e2e/oauth.ts
+++ b/packages/backend/test/e2e/oauth.ts
@@ -394,7 +394,6 @@ describe('OAuth', () => {
 	// "If an authorization code is used more than once, the authorization server
 	// MUST deny the request and SHOULD revoke (when possible) all tokens
 	// previously issued based on that authorization code."
-	// TODO: implement the "revoke all tokens" part, since we currently only deny the request.
 	describe('Revoking authorization code', () => {
 		test('On success', async () => {
 			const { code_challenge, code_verifier } = await pkceChallenge(128);
@@ -434,6 +433,46 @@ describe('OAuth', () => {
 				return true;
 			});
 		});
+
+		test('Revoke the already granted access token', async () => {
+			const { code_challenge, code_verifier } = await pkceChallenge(128);
+			const { client, code } = await fetchAuthorizationCode(alice, 'write:notes', code_challenge);
+
+			const token = await client.getToken({
+				code,
+				redirect_uri,
+				code_verifier,
+			} as AuthorizationTokenConfigExtended);
+
+			const createResponse = await relativeFetch('api/notes/create', {
+				method: 'POST',
+				headers: {
+					Authorization: `Bearer ${token.token.access_token}`,
+					'Content-Type': 'application/json',
+				},
+				body: JSON.stringify({ text: 'test' }),
+			});
+			assert.strictEqual(createResponse.status, 200);
+
+			await assert.rejects(client.getToken({
+				code,
+				redirect_uri,
+				code_verifier,
+			} as AuthorizationTokenConfigExtended), (err: GetTokenError) => {
+				assert.strictEqual(err.data.payload.error, 'invalid_grant');
+				return true;
+			});
+
+			const createResponse2 = await relativeFetch('api/notes/create', {
+				method: 'POST',
+				headers: {
+					Authorization: `Bearer ${token.token.access_token}`,
+					'Content-Type': 'application/json',
+				},
+				body: JSON.stringify({ text: 'test' }),
+			});
+			assert.strictEqual(createResponse2.status, 403);
+		});
 	});
 
 	test('Cancellation', async () => {