Skip to content

Fix parseHash state validation bypass #2788

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

Open
parasol-aser wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix parseHash state validation bypass #2788

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 0 additions & 36 deletions src/__tests__/core/web_api/__snapshots__/p2_api.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,42 +200,6 @@ exports[`Auth0APIClient logIn with social/enterprise (without username and email
}
`;

exports[`Auth0APIClient parseHash should pass __enableIdPInitiatedLogin when options._enableIdPInitiatedLogin===true 1`] = `
[
{
"__enableIdPInitiatedLogin": true,
"hash": "hash",
"nonce": undefined,
"state": undefined,
},
"cb",
]
`;

exports[`Auth0APIClient parseHash should pass __enableIdPInitiatedLogin when options._enableImpersonation===true 1`] = `
[
{
"__enableIdPInitiatedLogin": true,
"hash": "hash",
"nonce": undefined,
"state": undefined,
},
"cb",
]
`;

exports[`Auth0APIClient parseHash should pass __enableIdPInitiatedLogin:false when options._enableImpersonation and options._enableIdPInitiatedLogin are not present 1`] = `
[
{
"__enableIdPInitiatedLogin": false,
"hash": "hash",
"nonce": undefined,
"state": undefined,
},
"cb",
]
`;

exports[`Auth0APIClient passwordlessStart should call client.passwordlessStart 1`] = `
[
{
Expand Down
170 changes: 155 additions & 15 deletions src/__tests__/core/web_api/p2_api.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const getClient = (options = {}) => {
};

const getAuth0ClientMock = () => require('auth0-js');
const getParseHashMock = () => getAuth0ClientMock().WebAuth.mock.instances[0].parseHash.mock;
const assertCallWithCallback = (mock, callbackFunction) => {
expect(mock.calls.length).toBe(1);
expect(mock.calls[0][0]).toMatchSnapshot();
Expand Down Expand Up @@ -570,33 +571,172 @@ describe('Auth0APIClient', () => {
expect(mock.calls[0]).toMatchSnapshot();
});
describe('parseHash', () => {
it('should pass __enableIdPInitiatedLogin:false when options._enableImpersonation and options._enableIdPInitiatedLogin are not present', () => {
const assertParseHashOptions = (
options,
{ hash = 'hash', nonce = undefined, state = undefined } = {}
) => {
expect(options).toEqual({
hash,
nonce,
state
});
expect(options).not.toHaveProperty('__enableIdPInitiatedLogin');
};

const getOnlyParseHashCall = () => {
const parseHashMock = getParseHashMock();
expect(parseHashMock.calls.length).toBe(1);
return parseHashMock.calls[0];
};

it('should omit __enableIdPInitiatedLogin when private options are not present', () => {
const client = getClient({});
client.parseHash('hash', 'cb');
const mock = getAuth0ClientMock();
const parseHashMock = mock.WebAuth.mock.instances[0].parseHash.mock;
expect(parseHashMock.calls.length).toBe(1);
expect(parseHashMock.calls[0]).toMatchSnapshot();
const [parseOptions, cb] = getOnlyParseHashCall();
assertParseHashOptions(parseOptions);
expect(cb).toBe('cb');
});
it('should pass __enableIdPInitiatedLogin when options._enableImpersonation===true', () => {
it('should omit __enableIdPInitiatedLogin when options._enableImpersonation===true', () => {
const client = getClient({
_enableImpersonation: true
});
client.parseHash('hash', 'cb');
const mock = getAuth0ClientMock();
const parseHashMock = mock.WebAuth.mock.instances[0].parseHash.mock;
expect(parseHashMock.calls.length).toBe(1);
expect(parseHashMock.calls[0]).toMatchSnapshot();
const [parseOptions, cb] = getOnlyParseHashCall();
assertParseHashOptions(parseOptions);
expect(cb).toBe('cb');
});
it('should pass __enableIdPInitiatedLogin when options._enableIdPInitiatedLogin===true', () => {
it('should omit __enableIdPInitiatedLogin when options._enableIdPInitiatedLogin===true', () => {
const client = getClient({
_enableIdPInitiatedLogin: true
});
client.parseHash('hash', 'cb');
const mock = getAuth0ClientMock();
const parseHashMock = mock.WebAuth.mock.instances[0].parseHash.mock;
expect(parseHashMock.calls.length).toBe(1);
expect(parseHashMock.calls[0]).toMatchSnapshot();
const [parseOptions, cb] = getOnlyParseHashCall();
assertParseHashOptions(parseOptions);
expect(cb).toBe('cb');
});
it('should omit __enableIdPInitiatedLogin when both private options are true', () => {
const client = getClient({
_enableImpersonation: true,
_enableIdPInitiatedLogin: true
});
client.parseHash('hash', 'cb');
const [parseOptions, cb] = getOnlyParseHashCall();
assertParseHashOptions(parseOptions);
expect(cb).toBe('cb');
});
it('should omit __enableIdPInitiatedLogin when private options are truthy non-booleans', () => {
const client = getClient({
_enableImpersonation: 'true',
_enableIdPInitiatedLogin: 1
});
client.parseHash('hash', 'cb');
const [parseOptions, cb] = getOnlyParseHashCall();
assertParseHashOptions(parseOptions);
expect(cb).toBe('cb');
});
it('should not forward a root __enableIdPInitiatedLogin constructor option', () => {
const client = getClient({
__enableIdPInitiatedLogin: true
});
client.parseHash('hash', 'cb');
const [parseOptions, cb] = getOnlyParseHashCall();
assertParseHashOptions(parseOptions);
expect(cb).toBe('cb');
});
it('should not forward a params.__enableIdPInitiatedLogin constructor option', () => {
const client = getClient({
params: {
__enableIdPInitiatedLogin: true
}
});
client.parseHash('hash', 'cb');
const [parseOptions, cb] = getOnlyParseHashCall();
assertParseHashOptions(parseOptions);
expect(cb).toBe('cb');
});
it('should pass an empty string hash when parseHash is called without a hash', () => {
const client = getClient({});
client.parseHash(undefined, 'cb');
const [parseOptions, cb] = getOnlyParseHashCall();
assertParseHashOptions(parseOptions, { hash: '' });
expect(cb).toBe('cb');
});
it('should pass unsolicited token hashes without the IdP initiated bypass flag', () => {
const hash =
'#id_token=ATTACKER_ID_TOKEN&access_token=ATTACKER_ACCESS_TOKEN&token_type=Bearer';
const client = getClient({
_enableImpersonation: true,
_enableIdPInitiatedLogin: true
});
client.parseHash(hash, 'cb');
const [parseOptions, cb] = getOnlyParseHashCall();
assertParseHashOptions(parseOptions, { hash });
expect(cb).toBe('cb');
});
it('should pass OAuth error hashes without the IdP initiated bypass flag', () => {
const hash = '#error=access_denied&error_description=Unauthorized';
const client = getClient({
_enableIdPInitiatedLogin: true
});
client.parseHash(hash, 'cb');
const [parseOptions, cb] = getOnlyParseHashCall();
assertParseHashOptions(parseOptions, { hash });
expect(cb).toBe('cb');
});
it('should pass hashes with no auth result without the IdP initiated bypass flag', () => {
const hash = '#foo=bar';
const client = getClient({
_enableImpersonation: true
});
client.parseHash(hash, 'cb');
const [parseOptions, cb] = getOnlyParseHashCall();
assertParseHashOptions(parseOptions, { hash });
expect(cb).toBe('cb');
});
it('should pass configured nonce and state', () => {
const client = getClient({
nonce: 'nonce',
state: 'state'
});
client.parseHash('hash', 'cb');
const [parseOptions, cb] = getOnlyParseHashCall();
assertParseHashOptions(parseOptions, {
nonce: 'nonce',
state: 'state'
});
expect(cb).toBe('cb');
});
it('should pass configured params.nonce and params.state', () => {
const client = getClient({
params: {
nonce: 'params-nonce',
state: 'params-state'
}
});
client.parseHash('hash', 'cb');
const [parseOptions, cb] = getOnlyParseHashCall();
assertParseHashOptions(parseOptions, {
nonce: 'params-nonce',
state: 'params-state'
});
expect(cb).toBe('cb');
});
it('should prefer params.nonce and params.state over root nonce and state', () => {
const client = getClient({
nonce: 'root-nonce',
state: 'root-state',
params: {
nonce: 'params-nonce',
state: 'params-state'
}
});
client.parseHash('hash', 'cb');
const [parseOptions, cb] = getOnlyParseHashCall();
assertParseHashOptions(parseOptions, {
nonce: 'params-nonce',
state: 'params-state'
});
expect(cb).toBe('cb');
});
});
});
2 changes: 0 additions & 2 deletions src/core/web_api/p2_api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ class Auth0APIClient {
this.authOpt = null;
this.domain = domain;
this.isUniversalLogin = window.location.host === domain;
this._enableIdPInitiatedLogin = !!(opts._enableIdPInitiatedLogin || opts._enableImpersonation);
const telemetry = this.getTelemetryInfo(opts._telemetryInfo);

var state = opts.state;
Expand Down Expand Up @@ -174,7 +173,6 @@ class Auth0APIClient {
parseHash(hash = '', cb) {
return this.client.parseHash(
{
__enableIdPInitiatedLogin: this._enableIdPInitiatedLogin,
hash,
nonce: this.authOpt.nonce,
state: this.authOpt.state
Expand Down
8 changes: 6 additions & 2 deletions types/auth0-lock-tests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,15 +160,19 @@ const allOptions: Auth0LockConstructorOptions = {
theme: { primaryColor: "#ea5323" },
useCustomPasswordlessConnection: false,
usernameStyle: "username",
_enableImpersonation: false,
_enableIdPInitiatedLogin: false,
_sendTelemetry: true,
_telemetryInfo: { name: "my-sdk", version: "1.0.0", env: { "auth0.js": "9.0.0" } },
__useTenantInfo: false,
};

new Auth0Lock(CLIENT_ID, DOMAIN, allOptions);

// Private state-validation bypass options must not be public constructor options.
// @ts-expect-error
new Auth0Lock(CLIENT_ID, DOMAIN, { _enableImpersonation: true });
// @ts-expect-error
new Auth0Lock(CLIENT_ID, DOMAIN, { _enableIdPInitiatedLogin: true });

// Passwordless
new Auth0LockPasswordless(CLIENT_ID, DOMAIN);

Expand Down
2 changes: 0 additions & 2 deletions types/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,6 @@ interface Auth0LockConstructorOptions {
theme?: Auth0LockThemeOptions | undefined;
useCustomPasswordlessConnection?: boolean | undefined;
usernameStyle?: "email" | "username" | undefined;
_enableImpersonation?: boolean | undefined;
_enableIdPInitiatedLogin?: boolean | undefined;
_sendTelemetry?: boolean | undefined;
_telemetryInfo?: { name?: string | undefined; version?: string | undefined; env?: Record<string, string> | undefined } | undefined;
__useTenantInfo?: boolean | undefined;
Expand Down