Improve OpenID login flow security

prisma/migrations/20231206231959_add_issuer_and_client_to_flows/migration.sql

@@ -0,0 +1,12 @@
+/*
+  Warnings:
+
+  - Added the required column `issuerId` to the `OpenIdLoginFlow` table without a default value. This is not possible if the table is not empty.
+
+*/
+-- AlterTable
+ALTER TABLE "OpenIdLoginFlow" ADD COLUMN     "applicationId" UUID,
+ADD COLUMN     "issuerId" TEXT NOT NULL;
+
+-- AddForeignKey
+ALTER TABLE "OpenIdLoginFlow" ADD CONSTRAINT "OpenIdLoginFlow_applicationId_fkey" FOREIGN KEY ("applicationId") REFERENCES "Application"("id") ON DELETE CASCADE ON UPDATE CASCADE;

prisma/migrations/20231206232131_make_client_id_unique/migration.sql

@@ -0,0 +1,8 @@
+/*
+  Warnings:
+
+  - A unique constraint covering the columns `[client_id]` on the table `Application` will be added. If there are existing duplicate values, this will fail.
+
+*/
+-- CreateIndex
+CREATE UNIQUE INDEX "Application_client_id_key" ON "Application"("client_id");

prisma/schema.prisma

@@ -10,16 +10,17 @@ datasource db {
 }
 
 model Application {
-  id            String   @id @default(dbgenerated("uuid_generate_v7()")) @db.Uuid
+  id               String            @id @default(dbgenerated("uuid_generate_v7()")) @db.Uuid
-  name          String
+  name             String
-  website       String?
+  website          String?
-  vapid_key     String?
+  vapid_key        String?
-  client_id     String
+  client_id        String            @unique
-  secret        String
+  secret           String
-  scopes        String
+  scopes           String
-  redirect_uris String
+  redirect_uris    String
-  statuses      Status[] // One to many relation with Status
+  statuses         Status[] // One to many relation with Status
-  tokens        Token[] // One to many relation with Token
+  tokens           Token[] // One to many relation with Token
+  openIdLoginFlows OpenIdLoginFlow[]
 }
 
 model Emoji {
@@ -140,8 +141,11 @@ model Token {
 }
 
 model OpenIdLoginFlow {
-  id           String @id @default(dbgenerated("uuid_generate_v7()")) @db.Uuid
+  id            String       @id @default(dbgenerated("uuid_generate_v7()")) @db.Uuid
-  codeVerifier String
+  codeVerifier  String
+  issuerId      String
+  application   Application? @relation(fields: [applicationId], references: [id], onDelete: Cascade)
+  applicationId String?      @db.Uuid
 }
 
 model Attachment {

server/api/oauth/authorize-external/index.ts

@@ -5,8 +5,10 @@ import type { MatchedRoute } from "bun";
 import {
 	calculatePKCECodeChallenge,
 	discoveryRequest,
+	generateRandomCodeVerifier,
 	processDiscoveryResponse,
 } from "oauth4webapi";
+import { client } from "~database/datasource";
 
 export const meta = applyConfig({
 	allowedMethods: ["GET"],
@@ -63,7 +65,22 @@ export default async (
 		algorithm: "oidc",
 	}).then(res => processDiscoveryResponse(issuerUrl, res));
 
-	const codeVerifier = "tempString";
+	const codeVerifier = generateRandomCodeVerifier();
+
+	// Store into database
+
+	const newFlow = await client.openIdLoginFlow.create({
+		data: {
+			codeVerifier,
+			application: {
+				connect: {
+					client_id: clientId,
+				},
+			},
+			issuerId,
+		},
+	});
+
 	const codeChallenge = await calculatePKCECodeChallenge(codeVerifier);
 
 	return Response.redirect(
@@ -72,7 +89,7 @@ export default async (
 			new URLSearchParams({
 				client_id: issuer.client_id,
 				redirect_uri:
-					oauthRedirectUri(issuerId) + `?clientId=${clientId}`,
+					oauthRedirectUri(issuerId) + `?flow=${newFlow.id}`,
 				response_type: "code",
 				scope: "openid profile email",
 				// PKCE

server/api/oauth/callback/[issuer]/index.ts

@@ -52,8 +52,18 @@ export default async (
 	// Remove state query parameter from URL
 	currentUrl.searchParams.delete("state");
 	const issuerParam = matchedRoute.params.issuer;
-	// This is the Lysand client's client_id, not the external OAuth provider's client_id
+	const flow = await client.openIdLoginFlow.findFirst({
-	const clientId = matchedRoute.query.clientId;
+		where: {
+			id: matchedRoute.query.flow,
+		},
+		include: {
+			application: true,
+		},
+	});
+
+	if (!flow) {
+		return redirectToLogin("Invalid flow");
+	}
 
 	const config = getConfig();
 
@@ -95,8 +105,8 @@ export default async (
 			client_secret: issuer.client_secret,
 		},
 		parameters,
-		oauthRedirectUri(issuerParam) + `?clientId=${clientId}`,
+		oauthRedirectUri(issuerParam) + `?flow=${flow.id}`,
-		"tempString"
+		flow.codeVerifier
 	);
 
 	const result = await processAuthorizationCodeOpenIDResponse(
@@ -153,24 +163,19 @@ export default async (
 		return redirectToLogin("No user found with that account");
 	}
 
-	const application = await client.application.findFirst({
+	// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
-		where: {
+	if (!flow.application) return redirectToLogin("Invalid client_id");
-			client_id: clientId,
-		},
-	});
-
-	if (!application) return redirectToLogin("Invalid client_id");
 
 	const code = randomBytes(32).toString("hex");
 
 	await client.application.update({
-		where: { id: application.id },
+		where: { id: flow.application.id },
 		data: {
 			tokens: {
 				create: {
 					access_token: randomBytes(64).toString("base64url"),
 					code: code,
-					scope: application.scopes,
+					scope: flow.application.scopes,
 					token_type: TokenType.BEARER,
 					user: {
 						connect: {
@@ -183,5 +188,8 @@ export default async (
 	});
 
 	// Redirect back to application
-	return Response.redirect(`${application.redirect_uris}?code=${code}`, 302);
+	return Response.redirect(
+		`${flow.application.redirect_uris}?code=${code}`,
+		302
+	);
 };