Skip to content

Add nonce support #769

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

59 changes: 59 additions & 0 deletions src/client/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -728,6 +728,65 @@ describe("OAuth Authorization", () => {
expect(authorizationUrl.searchParams.get("prompt")).toBe("consent");
});

it("generates nonce automatically for OpenID Connect flows", async () => {
const { authorizationUrl, nonce } = await startAuthorization(
"https://auth.example.com",
{
clientInformation: validClientInfo,
redirectUrl: "http://localhost:3000/callback",
scope: "openid profile email",
}
);

expect(nonce).toBeDefined();
expect(authorizationUrl.searchParams.get("nonce")).toBe(nonce);
expect(nonce).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/);
});

it("uses provided nonce for OpenID Connect flows", async () => {
const providedNonce = "test-nonce-123";
const { authorizationUrl, nonce } = await startAuthorization(
"https://auth.example.com",
{
clientInformation: validClientInfo,
redirectUrl: "http://localhost:3000/callback",
scope: "openid profile",
nonce: providedNonce,
}
);

expect(nonce).toBe(providedNonce);
expect(authorizationUrl.searchParams.get("nonce")).toBe(providedNonce);
});

it("does not include nonce for non-OpenID Connect flows", async () => {
const { authorizationUrl, nonce } = await startAuthorization(
"https://auth.example.com",
{
clientInformation: validClientInfo,
redirectUrl: "http://localhost:3000/callback",
scope: "read write",
}
);

expect(nonce).toBeUndefined();
expect(authorizationUrl.searchParams.has("nonce")).toBe(false);
});

it("generates nonce when openid scope is included with other scopes", async () => {
const { authorizationUrl, nonce } = await startAuthorization(
"https://auth.example.com",
{
clientInformation: validClientInfo,
redirectUrl: "http://localhost:3000/callback",
scope: "read openid write profile",
}
);

expect(nonce).toBeDefined();
expect(authorizationUrl.searchParams.get("nonce")).toBe(nonce);
});

it("uses metadata authorization_endpoint when provided", async () => {
const { authorizationUrl } = await startAuthorization(
"https://auth.example.com",
Expand Down
13 changes: 11 additions & 2 deletions src/client/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,7 @@ export async function discoverOAuthMetadata(

/**
* Begins the authorization flow with the given server, by generating a PKCE challenge and constructing the authorization URL.
* For OpenID Connect flows (when scope includes 'openid'), automatically generates a nonce if not provided.
*/
export async function startAuthorization(
authorizationServerUrl: string | URL,
Expand All @@ -557,16 +558,18 @@ export async function startAuthorization(
redirectUrl,
scope,
state,
nonce,
resource,
}: {
metadata?: OAuthMetadata;
clientInformation: OAuthClientInformation;
redirectUrl: string | URL;
scope?: string;
state?: string;
nonce?: string;
resource?: URL;
},
): Promise<{ authorizationUrl: URL; codeVerifier: string }> {
): Promise<{ authorizationUrl: URL; codeVerifier: string; nonce?: string }> {
const responseType = "code";
const codeChallengeMethod = "S256";

Expand Down Expand Up @@ -625,7 +628,13 @@ export async function startAuthorization(
authorizationUrl.searchParams.set("resource", resource.href);
}

return { authorizationUrl, codeVerifier };
let generatedNonce: string | undefined;
if (scope?.includes("openid")) {
generatedNonce = nonce ?? crypto.randomUUID();
authorizationUrl.searchParams.set("nonce", generatedNonce);
}

return { authorizationUrl, codeVerifier, nonce: generatedNonce };
}

/**
Expand Down
59 changes: 59 additions & 0 deletions src/server/auth/handlers/authorize.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ describe('Authorization Handler', () => {
app.use('/authorize', handler);
});

afterEach(() => {
jest.restoreAllMocks();
});

describe('HTTP method validation', () => {
it('rejects non-GET/POST methods', async () => {
const response = await supertest(app)
Expand Down Expand Up @@ -302,6 +306,61 @@ describe('Authorization Handler', () => {
expect.any(Object)
);
});

it('propagates nonce parameter for OpenID Connect flows', async () => {
const mockProviderWithNonce = jest.spyOn(mockProvider, 'authorize');

const response = await supertest(app)
.get('/authorize')
.query({
client_id: 'valid-client',
redirect_uri: 'https://example.com/callback',
response_type: 'code',
code_challenge: 'challenge123',
code_challenge_method: 'S256',
scope: 'profile email',
nonce: 'test-nonce-123'
});

expect(response.status).toBe(302);
expect(mockProviderWithNonce).toHaveBeenCalledWith(
validClient,
expect.objectContaining({
nonce: 'test-nonce-123',
redirectUri: 'https://example.com/callback',
codeChallenge: 'challenge123',
scopes: ['profile', 'email']
}),
expect.any(Object)
);
});

it('handles authorization without nonce parameter', async () => {
const mockProviderWithoutNonce = jest.spyOn(mockProvider, 'authorize');

const response = await supertest(app)
.get('/authorize')
.query({
client_id: 'valid-client',
redirect_uri: 'https://example.com/callback',
response_type: 'code',
code_challenge: 'challenge123',
code_challenge_method: 'S256',
scope: 'profile email'
});

expect(response.status).toBe(302);
expect(mockProviderWithoutNonce).toHaveBeenCalledWith(
validClient,
expect.objectContaining({
nonce: undefined,
redirectUri: 'https://example.com/callback',
codeChallenge: 'challenge123',
scopes: ['profile', 'email']
}),
expect.any(Object)
);
});
});

describe('Successful authorization', () => {
Expand Down
4 changes: 3 additions & 1 deletion src/server/auth/handlers/authorize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const RequestAuthorizationParamsSchema = z.object({
code_challenge_method: z.literal("S256"),
scope: z.string().optional(),
state: z.string().optional(),
nonce: z.string().optional(),
resource: z.string().url().optional(),
});

Expand Down Expand Up @@ -115,7 +116,7 @@ export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: A
throw new InvalidRequestError(parseResult.error.message);
}

const { scope, code_challenge, resource } = parseResult.data;
const { scope, code_challenge, nonce, resource } = parseResult.data;
state = parseResult.data.state;

// Validate scopes
Expand All @@ -138,6 +139,7 @@ export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: A
scopes: requestedScopes,
redirectUri: redirect_uri,
codeChallenge: code_challenge,
nonce,
resource: resource ? new URL(resource) : undefined,
}, res);
} catch (error) {
Expand Down
1 change: 1 addition & 0 deletions src/server/auth/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export type AuthorizationParams = {
scopes?: string[];
codeChallenge: string;
redirectUri: string;
nonce?: string;
resource?: URL;
};

Expand Down