Skip to content
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.

Sign up for GitHub

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

feature to toggle off pkce #897

Merged
merged 4 commits into from
Feb 17, 2023
+316 −125
Merged

feature to toggle off pkce #897

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
43127ac
feature to togle off pkce
satanshiro Feb 16, 2023
d051cca
add documentation
satanshiro Feb 16, 2023
e5a9757
remove irrelevant test of current solution
satanshiro Feb 16, 2023
c995c28
requested changes
satanshiro Feb 17, 2023
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
7 changes: 6 additions & 1 deletion docs/oidc-client-ts.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ export interface OidcClientSettings {
client_secret?: string;
// @deprecated (undocumented)
clockSkewInSeconds?: number;
disablePKCE?: boolean;
display?: string;
extraQueryParams?: Record<string, string | number | boolean>;
// (undocumented)
Expand Down Expand Up @@ -373,7 +374,7 @@ export interface OidcClientSettings {

// @public
export class OidcClientSettingsStore {
constructor({ authority, metadataUrl, metadata, signingKeys, metadataSeed, client_id, client_secret, response_type, scope, redirect_uri, post_logout_redirect_uri, client_authentication, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, filterProtocolClaims, loadUserInfo, staleStateAgeInSeconds, clockSkewInSeconds, userInfoJwtIssuer, mergeClaims, stateStore, refreshTokenCredentials, revokeTokenAdditionalContentTypes, fetchRequestCredentials, refreshTokenAllowedScope, extraQueryParams, extraTokenParams, }: OidcClientSettings);
constructor({ authority, metadataUrl, metadata, signingKeys, metadataSeed, client_id, client_secret, response_type, scope, redirect_uri, post_logout_redirect_uri, client_authentication, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, filterProtocolClaims, loadUserInfo, staleStateAgeInSeconds, clockSkewInSeconds, userInfoJwtIssuer, mergeClaims, disablePKCE, stateStore, refreshTokenCredentials, revokeTokenAdditionalContentTypes, fetchRequestCredentials, refreshTokenAllowedScope, extraQueryParams, extraTokenParams, }: OidcClientSettings);
// (undocumented)
readonly acr_values: string | undefined;
// (undocumented)
Expand All @@ -387,6 +388,8 @@ export class OidcClientSettingsStore {
// (undocumented)
readonly clockSkewInSeconds: number;
// (undocumented)
readonly disablePKCE: boolean;
// (undocumented)
readonly display: string | undefined;
// (undocumented)
readonly extraQueryParams: Record<string, string | number | boolean>;
Expand Down Expand Up @@ -637,6 +640,8 @@ export interface SigninRequestArgs {
// (undocumented)
client_secret?: string;
// (undocumented)
disablePKCE?: boolean;
// (undocumented)
display?: string;
// (undocumented)
extraQueryParams?: Record<string, string | number | boolean>;
Expand Down
1 change: 1 addition & 0 deletions src/OidcClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ export class OidcClient {
client_secret: this.settings.client_secret,
skipUserInfo,
nonce,
disablePKCE: this.settings.disablePKCE,
});

// house cleaning
Expand Down
10 changes: 7 additions & 3 deletions src/OidcClientSettings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,10 @@ export interface OidcClientSettings {
* Will check the content type header of the response of the revocation endpoint to match these passed values (default: [])
*/
revokeTokenAdditionalContentTypes?: string[];

/**
* Will disable pkce validation, changing to true will not append to sign in request code_challenge and code_challenge_method. (default: false)
*/
disablePKCE?: boolean;
/**
* Sets the credentials for fetch requests. (default: "same-origin")
* Use this if you need to send cookies to the OIDC/OAuth2 provider or if you are using a proxy that requires cookies
Expand Down Expand Up @@ -178,7 +181,7 @@ export class OidcClientSettingsStore {
public readonly revokeTokenAdditionalContentTypes?: string[];
public readonly fetchRequestCredentials: RequestCredentials;
public readonly refreshTokenAllowedScope: string | undefined;

satanshiro marked this conversation as resolved.
Show resolved Hide resolved
public readonly disablePKCE: boolean;
public constructor({
// metadata related
authority, metadataUrl, metadata, signingKeys, metadataSeed,
Expand All @@ -195,6 +198,7 @@ export class OidcClientSettingsStore {
clockSkewInSeconds = DefaultClockSkewInSeconds,
userInfoJwtIssuer = "OP",
mergeClaims = false,
disablePKCE = false,
// other behavior
stateStore,
refreshTokenCredentials,
Expand Down Expand Up @@ -246,7 +250,7 @@ export class OidcClientSettingsStore {
this.clockSkewInSeconds = clockSkewInSeconds;
this.userInfoJwtIssuer = userInfoJwtIssuer;
this.mergeClaims = !!mergeClaims;

this.disablePKCE = !!disablePKCE;
this.revokeTokenAdditionalContentTypes = revokeTokenAdditionalContentTypes;

if (fetchRequestCredentials && refreshTokenCredentials) {
Expand Down
3 changes: 0 additions & 3 deletions src/ResponseValidator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,6 @@ export class ResponseValidator {
logger.throw(new Error("Expected code in response"));
}

if (!state.code_verifier && response.code) {
logger.throw(new Error("Unexpected code in response"));
pamapa marked this conversation as resolved.
Show resolved Hide resolved
}
}

protected async _processClaims(response: SigninResponse, skipUserInfo = false, validateSub = true): Promise<void> {
Expand Down
6 changes: 3 additions & 3 deletions src/SigninRequest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export interface SigninRequestArgs {
extraTokenParams?: Record<string, unknown>;
skipUserInfo?: boolean;
nonce?: string;

disablePKCE?: boolean;
/** custom "state", which can be used by a caller to have "data" round tripped */
state_data?: unknown;
}
Expand Down Expand Up @@ -87,7 +87,7 @@ export class SigninRequest {
this.state = new SigninState({
data: state_data,
request_type,
code_verifier: true,
code_verifier: !optionalParams.disablePKCE,
satanshiro marked this conversation as resolved.
Show resolved Hide resolved
client_id, authority, redirect_uri,
response_mode,
client_secret, scope, extraTokenParams,
Expand All @@ -104,7 +104,7 @@ export class SigninRequest {
}

parsedUrl.searchParams.append("state", this.state.id);
if (this.state.code_challenge) {
if (this.state.code_challenge && !optionalParams.disablePKCE) {
satanshiro marked this conversation as resolved.
Show resolved Hide resolved
parsedUrl.searchParams.append("code_challenge", this.state.code_challenge);
parsedUrl.searchParams.append("code_challenge_method", "S256");
}
Expand Down
3 changes: 0 additions & 3 deletions src/TokenClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,6 @@ export class TokenClient {
if (!args.code) {
logger.throw(new Error("A code is required"));
}
if (!args.code_verifier) {
logger.throw(new Error("A code_verifier is required"));
pamapa marked this conversation as resolved.
Show resolved Hide resolved
}

const params = new URLSearchParams({ grant_type, redirect_uri });
for (const [key, value] of Object.entries(args)) {
Expand Down