fix: tighten edge cases across services (#3158)

fluxer_api/src/api/oauth/OAuth2Service.ts

@@ -304,7 +304,10 @@ export class OAuth2Service {
 			if (authCode.userId && !(await this.findActiveUser(authCode.userId))) {
 				throw new InvalidGrantError();
 			}
-			await this.tokens.deleteAuthorizationCode(code);
+			if (!(await this.tokens.consumeAuthorizationCode(code, authCode.applicationId))) {
+				Logger.debug({code_len: code.length}, 'OAuth2 tokenExchange: authorization code already redeemed');
+				throw new InvalidGrantError();
+			}
 			const res = await this.issueTokens({
 				application,
 				userId: authCode.userId,
@@ -328,7 +331,9 @@ export class OAuth2Service {
 		if (!(await this.findActiveUser(refresh.userId))) {
 			throw new InvalidGrantError();
 		}
-		await this.tokens.deleteRefreshToken(params.refreshToken!, refresh.applicationId, refresh.userId);
+		if (!(await this.tokens.consumeRefreshToken(params.refreshToken!, refresh.applicationId, refresh.userId))) {
+			throw new InvalidGrantError();
+		}
 		const res = await this.issueTokens({
 			application,
 			userId: refresh.userId,

fluxer_api/src/api/oauth/repositories/IOAuth2TokenRepository.ts

@@ -14,13 +14,14 @@ export interface IOAuth2TokenRepository {
 	createAuthorizationCode(data: OAuth2AuthorizationCodeRow): Promise<OAuth2AuthorizationCode>;
 	getAuthorizationCode(code: string): Promise<OAuth2AuthorizationCode | null>;
 	deleteAuthorizationCode(code: string): Promise<void>;
+	consumeAuthorizationCode(code: string, applicationId: ApplicationID): Promise<boolean>;
 	createAccessToken(data: OAuth2AccessTokenRow): Promise<OAuth2AccessToken>;
 	getAccessToken(token: string): Promise<OAuth2AccessToken | null>;
 	deleteAccessToken(token: string, applicationId: ApplicationID, userId: UserID | null): Promise<void>;
 	deleteAllAccessTokensForUser(userId: UserID): Promise<void>;
 	createRefreshToken(data: OAuth2RefreshTokenRow): Promise<OAuth2RefreshToken>;
 	getRefreshToken(token: string): Promise<OAuth2RefreshToken | null>;
-	deleteRefreshToken(token: string, applicationId: ApplicationID, userId: UserID): Promise<void>;
+	consumeRefreshToken(token: string, applicationId: ApplicationID, userId: UserID): Promise<boolean>;
 	deleteAllRefreshTokensForUser(userId: UserID): Promise<void>;
 	listRefreshTokensForUser(userId: UserID): Promise<Array<OAuth2RefreshToken>>;
 	deleteAllTokensForUserAndApplication(userId: UserID, applicationId: ApplicationID): Promise<void>;

fluxer_api/src/api/oauth/repositories/OAuth2TokenRepository.ts

@@ -1,7 +1,14 @@
 // SPDX-License-Identifier: AGPL-3.0-or-later
 
 import type {ApplicationID, UserID} from '@app/api/BrandedTypes';
-import {BatchBuilder, deleteOneOrMany, fetchMany, fetchOne, upsertOne} from '@app/api/database/CassandraQueryExecution';
+import {
+	BatchBuilder,
+	deleteOneOrMany,
+	executeConditional,
+	fetchMany,
+	fetchOne,
+	upsertOne,
+} from '@app/api/database/CassandraQueryExecution';
 import type {
 	OAuth2AccessTokenByUserRow,
 	OAuth2AccessTokenRow,
@@ -71,6 +78,10 @@ export class OAuth2TokenRepository implements IOAuth2TokenRepository {
 		await deleteOneOrMany(OAuth2AuthorizationCodes.deleteByPk({code}));
 	}
 
+	async consumeAuthorizationCode(code: string, applicationId: ApplicationID): Promise<boolean> {
+		return executeConditional(OAuth2AuthorizationCodes.conditionalDeleteByPk({code}, {application_id: applicationId}));
+	}
+
 	async createAccessToken(data: OAuth2AccessTokenRow): Promise<OAuth2AccessToken> {
 		const batch = new BatchBuilder();
 		batch.addPrepared(OAuth2AccessTokens.insertWithTtl(data, ACCESS_TOKEN_TTL_SECONDS));
@@ -142,11 +153,14 @@ export class OAuth2TokenRepository implements IOAuth2TokenRepository {
 		return row ? new OAuth2RefreshToken(row) : null;
 	}
 
-	async deleteRefreshToken(token: string, _applicationId: ApplicationID, userId: UserID): Promise<void> {
-		const batch = new BatchBuilder();
-		batch.addPrepared(OAuth2RefreshTokens.deleteByPk({token_: token}));
-		batch.addPrepared(OAuth2RefreshTokensByUser.deleteByPk({user_id: userId, token_: token}));
-		await batch.execute();
+	async consumeRefreshToken(token: string, applicationId: ApplicationID, userId: UserID): Promise<boolean> {
+		const consumed = await executeConditional(
+			OAuth2RefreshTokens.conditionalDeleteByPk({token_: token}, {application_id: applicationId, user_id: userId}),
+		);
+		if (consumed) {
+			await deleteOneOrMany(OAuth2RefreshTokensByUser.deleteByPk({user_id: userId, token_: token}));
+		}
+		return consumed;
 	}
 
 	async deleteAllRefreshTokensForUser(userId: UserID): Promise<void> {

fluxer_api/src/api/oauth/tests/OAuth2ConcurrentRedemption.test.ts

@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+import {
+	authorizeOAuth2,
+	createOAuth2TestSetup,
+	exchangeOAuth2AuthorizationCode,
+} from '@app/api/oauth/tests/OAuthTestUtils';
+import {type ApiTestHarness, createApiTestHarness} from '@app/api/test/ApiTestHarness';
+import {InMemoryCassandraQueryExecutor} from '@app/api/test/InMemoryCassandraQueryExecutor';
+import {HTTP_STATUS} from '@app/api/test/TestConstants';
+import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest';
+
+const CONCURRENT_REQUESTS = 8;
+
+interface TokenResult {
+	status: number;
+	accessToken: string | null;
+}
+
+function addQueryLatency(): void {
+	const executeQuery = InMemoryCassandraQueryExecutor.prototype.executeQuery;
+	vi.spyOn(InMemoryCassandraQueryExecutor.prototype, 'executeQuery').mockImplementation(async function (
+		this: InMemoryCassandraQueryExecutor,
+		...args: Parameters<typeof executeQuery>
+	) {
+		await new Promise((resolve) => setTimeout(resolve, 1));
+		return executeQuery.apply(this, args);
+	} as typeof executeQuery);
+}
+
+async function postToken(
+	harness: ApiTestHarness,
+	clientId: string,
+	clientSecret: string,
+	form: Record<string, string>,
+): Promise<TokenResult> {
+	const response = await harness.app.request('/oauth2/token', {
+		method: 'POST',
+		headers: {
+			'Content-Type': 'application/x-www-form-urlencoded',
+			Authorization: `Basic ${Buffer.from(`${clientId}:${clientSecret}`).toString('base64')}`,
+			'x-forwarded-for': '127.0.0.1',
+		},
+		body: new URLSearchParams(form).toString(),
+	});
+	const body = (await response.json().catch(() => null)) as {access_token?: string} | null;
+	return {status: response.status, accessToken: body?.access_token ?? null};
+}
+
+async function postConcurrently(
+	harness: ApiTestHarness,
+	clientId: string,
+	clientSecret: string,
+	form: Record<string, string>,
+): Promise<Array<TokenResult>> {
+	addQueryLatency();
+	try {
+		return await Promise.all(
+			Array.from({length: CONCURRENT_REQUESTS}, () => postToken(harness, clientId, clientSecret, form)),
+		);
+	} finally {
+		vi.restoreAllMocks();
+	}
+}
+
+function expectSingleSuccess(results: Array<TokenResult>): void {
+	const succeeded = results.filter((result) => result.status === HTTP_STATUS.OK);
+	expect(succeeded).toHaveLength(1);
+	expect(succeeded[0]!.accessToken).toBeTruthy();
+	expect(results.filter((result) => result.status === HTTP_STATUS.BAD_REQUEST)).toHaveLength(CONCURRENT_REQUESTS - 1);
+}
+
+describe('OAuth2 concurrent grant redemption', () => {
+	let harness: ApiTestHarness;
+	beforeEach(async () => {
+		harness = await createApiTestHarness();
+	});
+	afterEach(async () => {
+		vi.restoreAllMocks();
+		await harness?.shutdown();
+	});
+
+	test('redeems an authorization code once when requests overlap', async () => {
+		const {endUser, redirectURI, application} = await createOAuth2TestSetup(harness);
+		const {code} = await authorizeOAuth2(harness, endUser.token, {
+			client_id: application.id,
+			redirect_uri: redirectURI,
+			scope: 'identify',
+		});
+		const results = await postConcurrently(harness, application.id, application.client_secret, {
+			grant_type: 'authorization_code',
+			code,
+			redirect_uri: redirectURI,
+			client_id: application.id,
+		});
+		expectSingleSuccess(results);
+	});
+
+	test('rotates a refresh token once when requests overlap', async () => {
+		const {endUser, redirectURI, application} = await createOAuth2TestSetup(harness);
+		const {code} = await authorizeOAuth2(harness, endUser.token, {
+			client_id: application.id,
+			redirect_uri: redirectURI,
+			scope: 'identify',
+		});
+		const initial = await exchangeOAuth2AuthorizationCode(harness, {
+			client_id: application.id,
+			client_secret: application.client_secret,
+			code,
+			redirect_uri: redirectURI,
+		});
+		const results = await postConcurrently(harness, application.id, application.client_secret, {
+			grant_type: 'refresh_token',
+			refresh_token: initial.refresh_token!,
+			client_id: application.id,
+		});
+		expectSingleSuccess(results);
+	});
+});