Skip to content

Commit 35b4113

Browse files
winetree94winetree94
committed
fix(security): reject unsafe redirect and csrf requests
Validate local HTTP redirect hosts with strict loopback rules, fail closed on cookie-authenticated API requests without request provenance, and add regression coverage for config and migration surfaces.
1 parent 323f01e commit 35b4113

10 files changed

Lines changed: 570 additions & 1 deletion

File tree

‎packages/server/src/entrypoints/app.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
} from '../lib/config/index.ts';
88
import { createLogger } from '../lib/logger.ts';
99
import { createOpenApiDocumentation } from '../lib/openapi.ts';
10+
import { csrfProtection } from '../middleware/csrf.ts';
1011
import { loggerMiddleware } from '../middleware/logger.ts';
1112
import { mikroOrmMiddleware } from '../middleware/mikro-orm.ts';
1213
import { servicesMiddleware } from '../middleware/services.ts';
@@ -50,6 +51,13 @@ export async function createApp(
5051
const app = new Hono()
5152
.onError((err, c) => {
5253
if (err instanceof TinyAuthError) {
54+
if (err.code === 'insufficient_scope') {
55+
c.header(
56+
'WWW-Authenticate',
57+
'Bearer error="insufficient_scope", scope="openid"',
58+
);
59+
}
60+
5361
return c.json(err.toJson(), err.status);
5462
}
5563

@@ -73,6 +81,7 @@ export async function createApp(
7381
),
7482
)
7583
.use('*', trustedProxyGuard(config.server.trust_proxy))
84+
.use('/api/*', csrfProtection(config.server.public_origin))
7685
.use('*', servicesMiddleware(services))
7786
.use('*', mikroOrmMiddleware)
7887
.route('/', routes)

‎packages/server/src/entrypoints/database/migrations.test.ts‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,25 @@ import { Migration20260512120000_add_scheduler_jobs as SqliteSchedulerJobsMigrat
88
import { postgres } from './postgres/postgres.ts';
99
import { sqlite } from './sqlite/sqlite.ts';
1010

11+
type MigrationClass =
12+
| typeof PostgresInitialMigration
13+
| typeof PostgresSchedulerJobsMigration
14+
| typeof SqliteInitialMigration
15+
| typeof SqliteSchedulerJobsMigration;
16+
17+
interface MigrationLike {
18+
up(): void | Promise<void>;
19+
getQueries(): Array<{ toString(): string }>;
20+
}
21+
22+
async function collectMigrationQueries(
23+
MigrationConstructor: MigrationClass,
24+
): Promise<string[]> {
25+
const migration: MigrationLike = Reflect.construct(MigrationConstructor, []);
26+
await migration.up();
27+
return migration.getQueries().map((query) => query.toString());
28+
}
29+
1130
describe('database migrations', () => {
1231
test('postgres uses explicit migration imports', async () => {
1332
const options = await postgres({
@@ -71,4 +90,69 @@ describe('database migrations', () => {
7190

7291
expect(options.debug).toBe(true);
7392
});
93+
94+
test('postgres initial migration creates core auth tables and constraints', async () => {
95+
const queries = await collectMigrationQueries(PostgresInitialMigration);
96+
expect(queries).toContain(
97+
`alter table "jwt_key" add constraint "jwt_key_status_check" check ("status" in ('next', 'active', 'previous', 'retired'));`,
98+
);
99+
expect(queries).toContain(
100+
`alter table "oauth_code" add constraint "oauth_code_code_challenge_method_check" check ("code_challenge_method" in ('S256', 'plain'));`,
101+
);
102+
expect(queries).toContain(
103+
`alter table "revoked_tokens" add constraint "revoked_tokens_token_type_check" check ("token_type" in ('access_token', 'refresh_token'));`,
104+
);
105+
expect(
106+
queries.some((query) => query.includes('create table "user_totp"')),
107+
).toBe(true);
108+
expect(
109+
queries.some((query) =>
110+
query.includes('create table "pending_oauth_registration"'),
111+
),
112+
).toBe(true);
113+
});
114+
115+
test('sqlite initial migration creates core auth tables and constraints', async () => {
116+
const queries = await collectMigrationQueries(SqliteInitialMigration);
117+
expect(queries).toContain(
118+
`create table \`jwt_key\` (\`kid\` text not null primary key, \`created_at\` datetime not null, \`updated_at\` datetime not null, \`private_key\` text not null, \`public_key\` text not null, \`algorithm\` text not null default 'RS256', \`status\` text check (\`status\` in ('next', 'active', 'previous', 'retired')) not null default 'next', \`activated_at\` datetime null, \`deactivated_at\` datetime null, \`retired_at\` datetime null, \`expires_at\` datetime null) /* RSA key pairs for JWT signing (RS256) */;`,
119+
);
120+
expect(queries).toContain(
121+
`create table \`oauth_code\` (\`id\` text not null primary key, \`created_at\` datetime not null, \`updated_at\` datetime not null, \`code_hash\` text not null, \`client_id\` text not null, \`user_sub\` text not null, \`redirect_uri\` text null, \`scope\` json not null default '[]', \`nonce\` text not null, \`code_challenge\` text not null, \`code_challenge_method\` text check (\`code_challenge_method\` in ('S256', 'plain')) not null default 'S256', \`expired_at\` datetime not null, \`consumed_at\` datetime null, \`auth_time\` integer null, constraint \`oauth_code_client_id_foreign\` foreign key (\`client_id\`) references \`oauth_client\` (\`id\`), constraint \`oauth_code_user_sub_foreign\` foreign key (\`user_sub\`) references \`user\` (\`sub\`)) /* Issued OAuth authorization codes */;`,
122+
);
123+
expect(
124+
queries.some((query) => query.includes('create table `user_totp`')),
125+
).toBe(true);
126+
expect(
127+
queries.some((query) =>
128+
query.includes('create table `pending_oauth_registration`'),
129+
),
130+
).toBe(true);
131+
});
132+
133+
test('scheduler migrations create durable lease and queue tables', async () => {
134+
const postgresQueries = await collectMigrationQueries(
135+
PostgresSchedulerJobsMigration,
136+
);
137+
const sqliteQueries = await collectMigrationQueries(
138+
SqliteSchedulerJobsMigration,
139+
);
140+
141+
expect(postgresQueries).toContain(
142+
`create index "background_jobs_status_available_at_idx" on "background_jobs" ("status", "available_at");`,
143+
);
144+
expect(sqliteQueries).toContain(
145+
`create index \`background_jobs_status_available_at_idx\` on \`background_jobs\` (\`status\`, \`available_at\`);`,
146+
);
147+
expect(
148+
postgresQueries.some((query) =>
149+
query.includes('"locked_until" timestamptz null'),
150+
),
151+
).toBe(true);
152+
expect(
153+
sqliteQueries.some((query) =>
154+
query.includes('`locked_until` datetime null'),
155+
),
156+
).toBe(true);
157+
});
74158
});

‎packages/server/src/lib/config/client.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
11
import z from 'zod';
2+
import { isSecureRedirectUri } from './url-policy.js';
3+
4+
const RedirectUriSchema = z.string().refine(isSecureRedirectUri, {
5+
message:
6+
'Redirect URI must use HTTPS or local HTTP and must not contain fragments or wildcards.',
7+
});
28

39
/**
410
* OAuth/OIDC client configuration.
@@ -28,7 +34,7 @@ export const ClientConfigSchema = z
2834
'OAuth client_secret for confidential clients. Omit for public clients.',
2935
),
3036
redirect_uris: z
31-
.array(z.string())
37+
.array(RedirectUriSchema)
3238
.describe('Allowed redirect URIs after authorization.'),
3339
response_types: z
3440
.array(z.string())

‎packages/server/src/lib/config/resolved.test.ts‎

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,55 @@ function createSchedulerConfig(): SchedulerConfig {
4343
};
4444
}
4545

46+
function createClientConfig(redirectUris: string[]) {
47+
return {
48+
id: 'client-config-id',
49+
name: 'Client',
50+
client_id: 'oauth-client-id',
51+
redirect_uris: redirectUris,
52+
response_types: ['code'],
53+
grant_types: ['authorization_code'],
54+
scope: 'openid',
55+
};
56+
}
57+
58+
function createIdentityProviderConfig(
59+
overrides: Record<string, string | string[] | null>,
60+
) {
61+
return {
62+
id: 'generic-provider',
63+
type: 'generic_oauth',
64+
enabled: true,
65+
display_name: 'Generic Provider',
66+
client_id: 'generic-client-id',
67+
client_secret: 'generic-client-secret',
68+
authorization_url: 'https://vendor.example/authorize',
69+
token_url: 'https://vendor.example/token',
70+
userinfo_url: 'https://vendor.example/userinfo',
71+
scopes: ['openid', 'email'],
72+
email_conflict_strategy: 'auto_link',
73+
userinfo_mapping: {
74+
id: 'sub',
75+
email: 'email',
76+
email_verified: 'email_verified',
77+
},
78+
...overrides,
79+
};
80+
}
81+
82+
function expectConfigIssue(input: unknown, expectedPath: string) {
83+
const result = TinyAuthRuntimeConfigSchema.safeParse(input);
84+
85+
expect(result.success).toBe(false);
86+
if (result.success) {
87+
throw new Error('Expected config parsing to fail.');
88+
}
89+
90+
expect(result.error.issues.map((issue) => issue.path.join('.'))).toContain(
91+
expectedPath,
92+
);
93+
}
94+
4695
describe('TinyAuthRuntimeConfigSchema', () => {
4796
test('parses the minimal unresolved config and applies omitted defaults', () => {
4897
const parsed = TinyAuthRuntimeConfigSchema.parse(MINIMAL_INPUT_CONFIG);
@@ -226,6 +275,50 @@ describe('TinyAuthRuntimeConfigSchema', () => {
226275
).not.toThrow();
227276
});
228277

278+
test('rejects insecure remote OAuth client redirect URIs', () => {
279+
expectConfigIssue(
280+
{
281+
...MINIMAL_INPUT_CONFIG,
282+
clients: [createClientConfig(['http://example.com/callback'])],
283+
},
284+
'clients.0.redirect_uris.0',
285+
);
286+
});
287+
288+
test('allows HTTPS and local HTTP OAuth client redirect URIs', () => {
289+
expect(() =>
290+
TinyAuthRuntimeConfigSchema.parse({
291+
...MINIMAL_INPUT_CONFIG,
292+
clients: [
293+
createClientConfig([
294+
'https://app.example/callback',
295+
'http://localhost:3000/callback',
296+
'http://127.0.0.1:3000/callback',
297+
'http://[::1]:3000/callback',
298+
]),
299+
],
300+
}),
301+
).not.toThrow();
302+
});
303+
304+
test('rejects redirect URIs with fragments or wildcards', () => {
305+
expectConfigIssue(
306+
{
307+
...MINIMAL_INPUT_CONFIG,
308+
clients: [createClientConfig(['https://app.example/callback#token'])],
309+
},
310+
'clients.0.redirect_uris.0',
311+
);
312+
313+
expectConfigIssue(
314+
{
315+
...MINIMAL_INPUT_CONFIG,
316+
clients: [createClientConfig(['https://app.example/*'])],
317+
},
318+
'clients.0.redirect_uris.0',
319+
);
320+
});
321+
229322
test('allows HTTPS and local HTTP JWKS URLs for OIDC providers', () => {
230323
const parsed = TinyAuthRuntimeConfigSchema.parse({
231324
...MINIMAL_INPUT_CONFIG,
@@ -313,6 +406,72 @@ describe('TinyAuthRuntimeConfigSchema', () => {
313406
).toThrow();
314407
});
315408

409+
test('rejects insecure remote identity provider endpoint URLs', () => {
410+
expectConfigIssue(
411+
{
412+
...MINIMAL_INPUT_CONFIG,
413+
identity_providers: [
414+
createIdentityProviderConfig({
415+
authorization_url: 'http://example.com/authorize',
416+
}),
417+
],
418+
},
419+
'identity_providers.0.authorization_url',
420+
);
421+
422+
expectConfigIssue(
423+
{
424+
...MINIMAL_INPUT_CONFIG,
425+
identity_providers: [
426+
createIdentityProviderConfig({
427+
token_url: 'http://example.com/token',
428+
}),
429+
],
430+
},
431+
'identity_providers.0.token_url',
432+
);
433+
434+
expectConfigIssue(
435+
{
436+
...MINIMAL_INPUT_CONFIG,
437+
identity_providers: [
438+
createIdentityProviderConfig({
439+
userinfo_url: 'http://example.com/userinfo',
440+
}),
441+
],
442+
},
443+
'identity_providers.0.userinfo_url',
444+
);
445+
446+
expectConfigIssue(
447+
{
448+
...MINIMAL_INPUT_CONFIG,
449+
identity_providers: [
450+
createIdentityProviderConfig({
451+
email_url: 'http://example.com/emails',
452+
}),
453+
],
454+
},
455+
'identity_providers.0.email_url',
456+
);
457+
});
458+
459+
test('allows local HTTP identity provider endpoint URLs', () => {
460+
expect(() =>
461+
TinyAuthRuntimeConfigSchema.parse({
462+
...MINIMAL_INPUT_CONFIG,
463+
identity_providers: [
464+
createIdentityProviderConfig({
465+
authorization_url: 'http://localhost:3000/authorize',
466+
token_url: 'http://127.0.0.1:3000/token',
467+
userinfo_url: 'http://[::1]:3000/userinfo',
468+
email_url: 'http://localhost:3000/emails',
469+
}),
470+
],
471+
}),
472+
).not.toThrow();
473+
});
474+
316475
test('rejects invalid and insecure remote JWKS URLs', () => {
317476
const providerConfig = {
318477
id: 'bad-jwks-provider',

‎packages/server/src/lib/config/url-policy.test.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import { describe, expect, test } from 'vitest';
2+
import { isHttpsOrLocalHttpUrl } from './url-policy.js';
3+
4+
describe('URL policy', () => {
5+
test.each([
6+
'http://localhost/callback',
7+
'http://foo.localhost/callback',
8+
'http://127.0.0.1/callback',
9+
'http://127.1.2.3/callback',
10+
'http://[::1]/callback',
11+
])('allows local HTTP URL %s', (url) => {
12+
expect(isHttpsOrLocalHttpUrl(url)).toBe(true);
13+
});
14+
15+
test.each([
16+
'http://127.evil/callback',
17+
'http://127.0.0.1.evil/callback',
18+
])('rejects lookalike 127 hostname %s', (url) => {
19+
expect(isHttpsOrLocalHttpUrl(url)).toBe(false);
20+
});
21+
});

‎packages/server/src/lib/config/url-policy.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import { parseIPv4 } from '../ip-utils.js';
2+
3+
function parseUrl(value: string): URL | null {
4+
try {
5+
return new URL(value);
6+
} catch {
7+
return null;
8+
}
9+
}
10+
11+
function isIPv4Loopback(hostname: string): boolean {
12+
const ipv4 = parseIPv4(hostname);
13+
return ipv4 !== null && (ipv4 & 0xff000000) === 0x7f000000;
14+
}
15+
16+
export function isLocalHttpHostname(hostname: string): boolean {
17+
return (
18+
hostname === 'localhost' ||
19+
hostname.endsWith('.localhost') ||
20+
isIPv4Loopback(hostname) ||
21+
hostname === '[::1]' ||
22+
hostname === '::1'
23+
);
24+
}
25+
26+
export function isHttpsOrLocalHttpUrl(value: string): boolean {
27+
const url = parseUrl(value);
28+
29+
if (!url) {
30+
return false;
31+
}
32+
33+
if (url.protocol === 'https:') {
34+
return true;
35+
}
36+
37+
return url.protocol === 'http:' && isLocalHttpHostname(url.hostname);
38+
}
39+
40+
export function isSecureRedirectUri(value: string): boolean {
41+
const url = parseUrl(value);
42+
43+
if (!url) {
44+
return false;
45+
}
46+
47+
return (
48+
!value.includes('*') && url.hash === '' && isHttpsOrLocalHttpUrl(value)
49+
);
50+
}

0 commit comments

Comments
 (0)