Skip to content

Commit a951a08

Browse files
committed
refactor(api): 🛂 Rewrite OpenID auth code to use state for data instead of query parameters
1 parent 5436be0 commit a951a08

6 files changed

Lines changed: 51 additions & 46 deletions

File tree

bun.lock

Lines changed: 0 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,5 @@
215215
"zod": "catalog:",
216216
"zod-openapi": "catalog:",
217217
"zod-validation-error": "catalog:"
218-
},
219-
"patchedDependencies": {
220-
"openid-client@6.8.1": "patches/openid-client@6.8.1.patch"
221218
}
222219
}

packages/api/routes/api/v1/sso/index.ts

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { apiRoute, auth, handleZodError } from "@versia-server/kit/api";
55
import { Client, db } from "@versia-server/kit/db";
66
import { OpenIdLoginFlows } from "@versia-server/kit/tables";
77
import { randomUUIDv7 } from "bun";
8+
import { sign } from "hono/jwt";
89
import { describeRoute, resolver, validator } from "hono-openapi";
910
import * as client from "openid-client";
1011
import { z } from "zod";
@@ -114,10 +115,6 @@ export default apiRoute((app) => {
114115
code_challenge_method: "S256",
115116
};
116117

117-
if (!oidcConfig.serverMetadata().supportsPKCE()) {
118-
parameters.state = client.randomState();
119-
}
120-
121118
const redirectUri = oauthRedirectUri(
122119
context.get("config").http.base_url,
123120
issuerId,
@@ -149,14 +146,24 @@ export default apiRoute((app) => {
149146
.returning()
150147
)[0];
151148

149+
const jwt = await sign(
150+
{
151+
flow: newFlow.id,
152+
link: "true",
153+
user_id: user.id,
154+
exp: Math.floor(Date.now() / 1000) + 300, // 5 minutes expiration
155+
iss: config.http.base_url.toString(),
156+
iat: Math.floor(Date.now() / 1000),
157+
},
158+
config.authentication.key,
159+
);
160+
161+
parameters.state = jwt;
162+
152163
parameters.redirect_uri = `${oauthRedirectUri(
153164
config.http.base_url,
154165
issuerId,
155-
)}?${new URLSearchParams({
156-
flow: newFlow.id,
157-
link: "true",
158-
user_id: user.id,
159-
})}`;
166+
)}`;
160167

161168
const redirectTo = client.buildAuthorizationUrl(
162169
oidcConfig,

packages/api/routes/oauth/sso/[issuer]/callback.ts

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import {
22
Account as AccountSchema,
33
RolePermission,
4-
zBoolean,
54
} from "@versia/client/schemas";
65
import { config } from "@versia-server/config";
76
import { ApiError } from "@versia-server/kit";
@@ -16,7 +15,7 @@ import {
1615
import { randomUUIDv7 } from "bun";
1716
import { and, eq, isNull, type SQL } from "drizzle-orm";
1817
import { setCookie } from "hono/cookie";
19-
import { sign } from "hono/jwt";
18+
import { sign, verify } from "hono/jwt";
2019
import { describeRoute, validator } from "hono-openapi";
2120
import * as client from "openid-client";
2221
import { z } from "zod";
@@ -48,15 +47,13 @@ export default apiRoute((app) => {
4847
validator(
4948
"query",
5049
z.object({
51-
flow: z.string(),
52-
link: zBoolean.default(false),
53-
user_id: z.uuid().optional(),
50+
state: z.string(),
5451
}),
5552
handleZodError,
5653
),
5754
async (context) => {
5855
const { issuer: issuerId } = context.req.valid("param");
59-
const { flow: flowId, user_id, link } = context.req.valid("query");
56+
const { state } = context.req.valid("query");
6057

6158
const issuer = config.authentication.openid_providers.find(
6259
(provider) => provider.id === issuerId,
@@ -66,6 +63,16 @@ export default apiRoute((app) => {
6663
throw new ApiError(422, "Unknown or invalid issuer");
6764
}
6865

66+
const jwtPayload = (await verify(state, config.authentication.key, {
67+
iss: config.http.base_url.toString(),
68+
})) as {
69+
flow: string;
70+
link?: boolean;
71+
user_id?: string;
72+
};
73+
74+
const { flow: flowId, link, user_id } = jwtPayload;
75+
6976
const flow = await db.query.OpenIdLoginFlows.findFirst({
7077
where: (flow): SQL | undefined => eq(flow.id, flowId),
7178
with: {
@@ -104,7 +111,7 @@ export default apiRoute((app) => {
104111
context.req.raw,
105112
{
106113
pkceCodeVerifier: flow.codeVerifier,
107-
expectedState: flow.state ?? undefined,
114+
expectedState: state,
108115
idTokenExpected: true,
109116
},
110117
);

packages/api/routes/oauth/sso/[issuer]/index.ts

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { apiRoute, handleZodError, jsonOrForm } from "@versia-server/kit/api";
44
import { Client, db } from "@versia-server/kit/db";
55
import { OpenIdLoginFlows } from "@versia-server/kit/tables";
66
import { randomUUIDv7 } from "bun";
7+
import { sign } from "hono/jwt";
78
import { describeRoute, validator } from "hono-openapi";
89
import * as client from "openid-client";
910
import { z } from "zod";
@@ -43,8 +44,12 @@ export default apiRoute((app) => {
4344
),
4445
async (context) => {
4546
// This is the Versia client's client_id, not the external OAuth provider's client_id
46-
const { client_id, redirect_uri, scopes, state } =
47-
context.req.valid("json");
47+
const {
48+
client_id,
49+
redirect_uri,
50+
scopes,
51+
state: clientState,
52+
} = context.req.valid("json");
4853
const { issuer: issuerId } = context.req.valid("param");
4954

5055
const issuer = config.authentication.openid_providers.find(
@@ -84,10 +89,6 @@ export default apiRoute((app) => {
8489
code_challenge_method: "S256",
8590
};
8691

87-
if (!oidcConfig.serverMetadata().supportsPKCE()) {
88-
parameters.state = client.randomState();
89-
}
90-
9192
// Store into database
9293
const newFlow = (
9394
await db
@@ -96,7 +97,7 @@ export default apiRoute((app) => {
9697
id: randomUUIDv7(),
9798
codeVerifier,
9899
state: parameters.state,
99-
clientState: state,
100+
clientState,
100101
clientRedirectUri: redirect_uri,
101102
clientScopes: scopes,
102103
clientId: application.id,
@@ -105,12 +106,22 @@ export default apiRoute((app) => {
105106
.returning()
106107
)[0];
107108

109+
const jwt = await sign(
110+
{
111+
flow: newFlow.id,
112+
exp: Math.floor(Date.now() / 1000) + 300, // 5 minutes expiration
113+
iss: config.http.base_url.toString(),
114+
iat: Math.floor(Date.now() / 1000),
115+
},
116+
config.authentication.key,
117+
);
118+
119+
parameters.state = jwt;
120+
108121
parameters.redirect_uri = `${oauthRedirectUri(
109122
context.get("config").http.base_url,
110123
issuerId,
111-
)}?${new URLSearchParams({
112-
flow: newFlow.id,
113-
})}`;
124+
)}`;
114125

115126
const redirectTo = client.buildAuthorizationUrl(
116127
oidcConfig,

patches/openid-client@6.8.1.patch

Lines changed: 0 additions & 14 deletions
This file was deleted.

0 commit comments

Comments
 (0)