From 6fe735e08cf7bf6164e29be562fa6dd2d5e4bcb3 Mon Sep 17 00:00:00 2001 From: jafaircl Date: Fri, 27 Oct 2023 07:36:54 -0400 Subject: [PATCH] Add support for custom URL state values (#1212) * add support for custom url state values --------- Co-authored-by: Jonathan Faircloth --- docs/index.md | 10 ++++++++++ docs/oidc-client-ts.api.md | 20 +++++++++++++++++--- src/OidcClient.test.ts | 4 ++++ src/OidcClient.ts | 2 ++ src/ResponseValidator.test.ts | 11 +++++++++++ src/ResponseValidator.ts | 1 + src/SigninRequest.test.ts | 12 ++++++++++++ src/SigninRequest.ts | 12 +++++++++--- src/SigninResponse.test.ts | 18 ++++++++++++++++++ src/SigninResponse.ts | 10 +++++++++- src/SigninState.test.ts | 2 ++ src/SigninState.ts | 2 ++ src/State.test.ts | 12 +++++++++++- src/State.ts | 4 ++++ src/User.ts | 3 +++ src/UserManager.test.ts | 1 + src/UserManager.ts | 2 +- src/errors/ErrorResponse.test.ts | 7 +++++++ src/errors/ErrorResponse.ts | 5 ++++- src/utils/UrlUtils.ts | 5 +++++ 20 files changed, 133 insertions(+), 10 deletions(-) diff --git a/docs/index.md b/docs/index.md index 7960e3f7f..6934ef5a0 100644 --- a/docs/index.md +++ b/docs/index.md @@ -117,6 +117,16 @@ After successful sign-in the custom state is part of the [User](classes/User.htm This custom state should not be confused with the URL state parameter. The latter is internally used to match against the authentication state object to finish the authentication process. +## Custom state in request url +If you would like to encode a custom state string in the sign in request url, you can do so with the `url_state` parameter. You may want to do this in order to pass user state to the authentication server and/or a proxy and return that state as part of the response. + +```javascript +const mgr = new UserManager(); +mgr.signinRedirect({ url_state: 'custom url state' }) +``` + +The `url_state` will be appended to the opaque, unique value created by the library when sending the request. It should survive the round trip to your authentication server and will be part of the [User](classes/User.html#url_state) object as `url_state`. + ## Projects using oidc-client diff --git a/docs/oidc-client-ts.api.md b/docs/oidc-client-ts.api.md index 65372e72a..29833fd28 100644 --- a/docs/oidc-client-ts.api.md +++ b/docs/oidc-client-ts.api.md @@ -69,6 +69,7 @@ export class ErrorResponse extends Error { error_uri?: string | null; userState?: unknown; session_state?: string | null; + url_state?: string; }, form?: URLSearchParams | undefined); readonly error: string | null; @@ -79,6 +80,8 @@ export class ErrorResponse extends Error { // (undocumented) readonly session_state: string | null; state?: unknown; + // (undocumented) + url_state?: string; } // @public @@ -91,7 +94,7 @@ export class ErrorTimeout extends Error { export type ExtraHeader = string | (() => string); // @public (undocumented) -export type ExtraSigninRequestArgs = Pick; +export type ExtraSigninRequestArgs = Pick; // @public (undocumented) export type ExtraSignoutRequestArgs = Pick; @@ -298,7 +301,7 @@ export class OidcClient { // (undocumented) clearStaleState(): Promise; // (undocumented) - createSigninRequest({ state, request, request_uri, request_type, id_token_hint, login_hint, skipUserInfo, nonce, response_type, scope, redirect_uri, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, extraQueryParams, extraTokenParams, }: CreateSigninRequestArgs): Promise; + createSigninRequest({ state, request, request_uri, request_type, id_token_hint, login_hint, skipUserInfo, nonce, url_state, response_type, scope, redirect_uri, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, extraQueryParams, extraTokenParams, }: CreateSigninRequestArgs): Promise; // (undocumented) createSignoutRequest({ state, id_token_hint, client_id, request_type, post_logout_redirect_uri, extraQueryParams, }?: CreateSignoutRequestArgs): Promise; // (undocumented) @@ -629,7 +632,7 @@ export type SigninRedirectArgs = RedirectParams & ExtraSigninRequestArgs; // @public (undocumented) export class SigninRequest { - constructor({ url, authority, client_id, redirect_uri, response_type, scope, state_data, response_mode, request_type, client_secret, nonce, resource, skipUserInfo, extraQueryParams, extraTokenParams, disablePKCE, ...optionalParams }: SigninRequestArgs); + constructor({ url, authority, client_id, redirect_uri, response_type, scope, state_data, response_mode, request_type, client_secret, nonce, url_state, resource, skipUserInfo, extraQueryParams, extraTokenParams, disablePKCE, ...optionalParams }: SigninRequestArgs); // (undocumented) readonly state: SigninState; // (undocumented) @@ -687,6 +690,8 @@ export interface SigninRequestArgs { ui_locales?: string; // (undocumented) url: string; + // (undocumented) + url_state?: string; } // @public (undocumented) @@ -726,6 +731,8 @@ export class SigninResponse { readonly state: string | null; // (undocumented) token_type: string; + // (undocumented) + url_state?: string; userState: unknown; } @@ -739,6 +746,7 @@ export class SigninState extends State { data?: unknown; created?: number; request_type?: string; + url_state?: string; code_verifier?: string | boolean; authority: string; client_id: string; @@ -832,6 +840,7 @@ export class State { data?: unknown; created?: number; request_type?: string; + url_state?: string; }); // (undocumented) static clearStaleState(storage: StateStore, age: number): Promise; @@ -846,6 +855,8 @@ export class State { readonly request_type: string | undefined; // (undocumented) toStorageString(): string; + // (undocumented) + readonly url_state: string | undefined; } // @public (undocumented) @@ -872,6 +883,7 @@ export class User { profile: UserProfile; expires_at?: number; userState?: unknown; + url_state?: string; }); access_token: string; get expired(): boolean | undefined; @@ -890,6 +902,8 @@ export class User { token_type: string; // (undocumented) toStorageString(): string; + // (undocumented) + readonly url_state?: string; } // @public (undocumented) diff --git a/src/OidcClient.test.ts b/src/OidcClient.test.ts index 68b35497a..577d23937 100644 --- a/src/OidcClient.test.ts +++ b/src/OidcClient.test.ts @@ -87,6 +87,7 @@ describe("OidcClient", () => { request: "req", request_uri: "req_uri", nonce: "rnd", + url_state: "url_state", }); // assert @@ -108,6 +109,7 @@ describe("OidcClient", () => { expect(url).toContain("request_uri=req_uri"); expect(url).toContain("response_mode=fragment"); expect(url).toContain("nonce=rnd"); + expect(url.match(/state=.*%3Burl_state/)).toBeTruthy(); }); it("should pass state in place of data to SigninRequest", async () => { @@ -128,6 +130,7 @@ describe("OidcClient", () => { login_hint: "lh", acr_values: "av", resource: "res", + url_state: "url_state", }); // assert @@ -145,6 +148,7 @@ describe("OidcClient", () => { expect(url).toContain("login_hint=lh"); expect(url).toContain("acr_values=av"); expect(url).toContain("resource=res"); + expect(url.match(/state=.*%3Burl_state/)).toBeTruthy(); }); it("should fail if implicit flow requested", async () => { diff --git a/src/OidcClient.ts b/src/OidcClient.ts index b4f341664..83ba82421 100644 --- a/src/OidcClient.ts +++ b/src/OidcClient.ts @@ -92,6 +92,7 @@ export class OidcClient { login_hint, skipUserInfo, nonce, + url_state, response_type = this.settings.response_type, scope = this.settings.scope, redirect_uri = this.settings.redirect_uri, @@ -122,6 +123,7 @@ export class OidcClient { response_type, scope, state_data: state, + url_state, prompt, display, max_age, ui_locales, id_token_hint, login_hint, acr_values, resource, request, request_uri, extraQueryParams, extraTokenParams, request_type, response_mode, client_secret: this.settings.client_secret, diff --git a/src/ResponseValidator.test.ts b/src/ResponseValidator.test.ts index 1a88e6b77..f8cded3db 100644 --- a/src/ResponseValidator.test.ts +++ b/src/ResponseValidator.test.ts @@ -85,6 +85,17 @@ describe("ResponseValidator", () => { // assert expect(stubResponse.userState).toEqual({ some: "data" }); }); + + it("should return url_state for successful responses", () => { + // arrange + Object.assign(stubResponse, { url_state: "url_state" }); + + // act + subject.validateSignoutResponse(stubResponse, stubState); + + // assert + expect(stubResponse.url_state).toEqual("url_state"); + }); }); describe("validateSigninResponse", () => { diff --git a/src/ResponseValidator.ts b/src/ResponseValidator.ts index 7ea2d0c25..1c1f845ed 100644 --- a/src/ResponseValidator.ts +++ b/src/ResponseValidator.ts @@ -133,6 +133,7 @@ export class ResponseValidator { // this is important for both success & error outcomes logger.debug("state validated"); response.userState = state.data; + response.url_state = state.url_state; // if there's no scope on the response, then assume all scopes granted (per-spec) and copy over scopes from original request response.scope ??= state.scope; diff --git a/src/SigninRequest.test.ts b/src/SigninRequest.test.ts index 44e1d8a7c..77c3ad258 100644 --- a/src/SigninRequest.test.ts +++ b/src/SigninRequest.test.ts @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. import { SigninRequest, type SigninRequestArgs } from "./SigninRequest"; +import { URL_STATE_DELIMITER } from "./utils"; describe("SigninRequest", () => { @@ -248,5 +249,16 @@ describe("SigninRequest", () => { // assert expect(subject.url).toContain("nonce="); }); + + it("should include url_state", () => { + // arrange + settings.url_state = "foo"; + + // act + subject = new SigninRequest(settings); + + // assert + expect(subject.url).toContain("state=" + subject.state.id + encodeURIComponent(URL_STATE_DELIMITER + "foo")); + }); }); }); diff --git a/src/SigninRequest.ts b/src/SigninRequest.ts index c5066cb87..d6b3a15fe 100644 --- a/src/SigninRequest.ts +++ b/src/SigninRequest.ts @@ -1,7 +1,7 @@ // Copyright (c) Brock Allen & Dominick Baier. All rights reserved. // Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. -import { Logger } from "./utils"; +import { Logger, URL_STATE_DELIMITER } from "./utils"; import { SigninState } from "./SigninState"; /** @@ -42,6 +42,7 @@ export interface SigninRequestArgs { disablePKCE?: boolean; /** custom "state", which can be used by a caller to have "data" round tripped */ state_data?: unknown; + url_state?: string; } /** @@ -57,7 +58,7 @@ export class SigninRequest { // mandatory url, authority, client_id, redirect_uri, response_type, scope, // optional - state_data, response_mode, request_type, client_secret, nonce, + state_data, response_mode, request_type, client_secret, nonce, url_state, resource, skipUserInfo, extraQueryParams, @@ -93,6 +94,7 @@ export class SigninRequest { this.state = new SigninState({ data: state_data, request_type, + url_state, code_verifier: !disablePKCE, client_id, authority, redirect_uri, response_mode, @@ -109,7 +111,11 @@ export class SigninRequest { parsedUrl.searchParams.append("nonce", nonce); } - parsedUrl.searchParams.append("state", this.state.id); + let state = this.state.id; + if (url_state) { + state = `${state}${URL_STATE_DELIMITER}${url_state}`; + } + parsedUrl.searchParams.append("state", state); if (this.state.code_challenge) { parsedUrl.searchParams.append("code_challenge", this.state.code_challenge); parsedUrl.searchParams.append("code_challenge_method", "S256"); diff --git a/src/SigninResponse.test.ts b/src/SigninResponse.test.ts index 7bf04c7ba..318858c44 100644 --- a/src/SigninResponse.test.ts +++ b/src/SigninResponse.test.ts @@ -49,6 +49,24 @@ describe("SigninResponse", () => { expect(subject.state).toEqual("foo"); }); + it("should read url_state", () => { + // act + const subject = new SigninResponse(new URLSearchParams("state=foo;bar")); + + // assert + expect(subject.state).toEqual("foo"); + expect(subject.url_state).toEqual("bar"); + }); + + it("should return url_state that uses the delimiter unmodified", () => { + // act + const subject = new SigninResponse(new URLSearchParams("state=foo;bar;baz")); + + // assert + expect(subject.state).toEqual("foo"); + expect(subject.url_state).toEqual("bar;baz"); + }); + it("should read code", () => { // act const subject = new SigninResponse(new URLSearchParams("code=foo")); diff --git a/src/SigninResponse.ts b/src/SigninResponse.ts index 10ae67ccc..6d619bcbd 100644 --- a/src/SigninResponse.ts +++ b/src/SigninResponse.ts @@ -1,7 +1,7 @@ // Copyright (c) Brock Allen & Dominick Baier. All rights reserved. // Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. -import { Timer } from "./utils"; +import { Timer, URL_STATE_DELIMITER } from "./utils"; import type { UserProfile } from "./User"; const OidcScope = "openid"; @@ -44,6 +44,7 @@ export class SigninResponse { /** custom state data set during the initial signin request */ public userState: unknown; + public url_state?: string; /** @see {@link User.profile} */ public profile: UserProfile = {} as UserProfile; @@ -51,6 +52,13 @@ export class SigninResponse { public constructor(params: URLSearchParams) { this.state = params.get("state"); this.session_state = params.get("session_state"); + if (this.state) { + const splitState = decodeURIComponent(this.state).split(URL_STATE_DELIMITER); + this.state = splitState[0]; + if (splitState.length > 1) { + this.url_state = splitState.slice(1).join(URL_STATE_DELIMITER); + } + } this.error = params.get("error"); this.error_description = params.get("error_description"); diff --git a/src/SigninState.test.ts b/src/SigninState.test.ts index 0f026bf68..3a150a74e 100644 --- a/src/SigninState.test.ts +++ b/src/SigninState.test.ts @@ -18,12 +18,14 @@ describe("SigninState", () => { redirect_uri: "http://cb", request_type: "type", scope: "scope", + url_state: "foo", }); // assert expect(subject.id).toEqual("5"); expect(subject.created).toEqual(6); expect(subject.data).toEqual(7); + expect(subject.url_state).toEqual("foo"); }); it("should accept redirect_uri", () => { diff --git a/src/SigninState.ts b/src/SigninState.ts index 203157ed9..df84eeb2a 100644 --- a/src/SigninState.ts +++ b/src/SigninState.ts @@ -37,6 +37,7 @@ export class SigninState extends State { data?: unknown; created?: number; request_type?: string; + url_state?: string; code_verifier?: string | boolean; authority: string; @@ -79,6 +80,7 @@ export class SigninState extends State { data: this.data, created: this.created, request_type: this.request_type, + url_state: this.url_state, code_verifier: this.code_verifier, authority: this.authority, diff --git a/src/State.test.ts b/src/State.test.ts index 87113e869..d9dff40c9 100644 --- a/src/State.test.ts +++ b/src/State.test.ts @@ -89,12 +89,22 @@ describe("State", () => { // assert expect(subject.request_type).toEqual("xoxo"); }); + + it("should accept url_state", () => { + // act + const subject = new State({ + url_state: "foo", + }); + + // assert + expect(subject.url_state).toEqual("foo"); + }); }); it("can serialize and then deserialize", () => { // arrange const subject1 = new State({ - data: { foo: "test" }, created: 1000, request_type:"type", + data: { foo: "test" }, created: 1000, request_type:"type", url_state: "foo", }); // act diff --git a/src/State.ts b/src/State.ts index 668a6b1f2..971dde12a 100644 --- a/src/State.ts +++ b/src/State.ts @@ -11,6 +11,7 @@ export class State { public readonly id: string; public readonly created: number; public readonly request_type: string | undefined; + public readonly url_state: string | undefined; /** custom "state", which can be used by a caller to have "data" round tripped */ public readonly data?: unknown; @@ -20,6 +21,7 @@ export class State { data?: unknown; created?: number; request_type?: string; + url_state?: string; }) { this.id = args.id || CryptoUtils.generateUUIDv4(); this.data = args.data; @@ -31,6 +33,7 @@ export class State { this.created = Timer.getEpochTime(); } this.request_type = args.request_type; + this.url_state = args.url_state; } public toStorageString(): string { @@ -40,6 +43,7 @@ export class State { data: this.data, created: this.created, request_type: this.request_type, + url_state: this.url_state, }); } diff --git a/src/User.ts b/src/User.ts index 99a037f5d..f1f4d1f51 100644 --- a/src/User.ts +++ b/src/User.ts @@ -50,6 +50,7 @@ export class User { /** custom state data set during the initial signin request */ public readonly state: unknown; + public readonly url_state?: string; public constructor(args: { id_token?: string; @@ -61,6 +62,7 @@ export class User { profile: UserProfile; expires_at?: number; userState?: unknown; + url_state?: string; }) { this.id_token = args.id_token; this.session_state = args.session_state ?? null; @@ -72,6 +74,7 @@ export class User { this.profile = args.profile; this.expires_at = args.expires_at; this.state = args.userState; + this.url_state = args.url_state; } /** Computed number of seconds the access token has remaining. */ diff --git a/src/UserManager.test.ts b/src/UserManager.test.ts index 30ef5c5c0..afaf997fe 100644 --- a/src/UserManager.test.ts +++ b/src/UserManager.test.ts @@ -248,6 +248,7 @@ describe("UserManager", () => { nonce: "random_nonce", redirect_uri: "http://app/extra_callback", prompt: "login", + url_state: "url_state", }; // act diff --git a/src/UserManager.ts b/src/UserManager.ts index 174d99517..bb53ec896 100644 --- a/src/UserManager.ts +++ b/src/UserManager.ts @@ -19,7 +19,7 @@ import type { SigninResponse } from "./SigninResponse"; /** * @public */ -export type ExtraSigninRequestArgs = Pick; +export type ExtraSigninRequestArgs = Pick; /** * @public */ diff --git a/src/errors/ErrorResponse.test.ts b/src/errors/ErrorResponse.test.ts index 8a6bd83f9..93e6a83f6 100644 --- a/src/errors/ErrorResponse.test.ts +++ b/src/errors/ErrorResponse.test.ts @@ -53,6 +53,13 @@ describe("ErrorResponse", () => { expect(subject.state).toEqual("foo"); }); + it("should read url_state", () => { + // act + const subject = new ErrorResponse({ error:"error", url_state:"foo" }); + + // assert + expect(subject.url_state).toEqual("foo"); + }); }); describe("message", () => { diff --git a/src/errors/ErrorResponse.ts b/src/errors/ErrorResponse.ts index 249b56f44..cd35ec6c8 100644 --- a/src/errors/ErrorResponse.ts +++ b/src/errors/ErrorResponse.ts @@ -29,10 +29,12 @@ export class ErrorResponse extends Error { public readonly session_state: string | null; + public url_state?: string; + public constructor( args: { error?: string | null; error_description?: string | null; error_uri?: string | null; - userState?: unknown; session_state?: string | null; + userState?: unknown; session_state?: string | null; url_state?: string; }, /** The x-www-form-urlencoded request body sent to the authority server */ public readonly form?: URLSearchParams, @@ -50,5 +52,6 @@ export class ErrorResponse extends Error { this.state = args.userState; this.session_state = args.session_state ?? null; + this.url_state = args.url_state; } } diff --git a/src/utils/UrlUtils.ts b/src/utils/UrlUtils.ts index e54a0d861..36767c714 100644 --- a/src/utils/UrlUtils.ts +++ b/src/utils/UrlUtils.ts @@ -13,3 +13,8 @@ export class UrlUtils { return new URLSearchParams(params.slice(1)); } } + +/** + * @internal + */ +export const URL_STATE_DELIMITER = ";"; \ No newline at end of file