Skip to content

Commit

Permalink
Throw error if there is incorrect relatesTo in IDX response (#1421)
Browse files Browse the repository at this point in the history
OKTA-612326 Throw error if there is incorrect `relatesTo` in IDX response
  • Loading branch information
denysoblohin-okta authored Jun 27, 2023
1 parent 933fff3 commit b3991a2
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 3 deletions.
12 changes: 11 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
# Changelog

## 7.4.0

### Fixes

- [#1421](https://github.com/okta/okta-auth-js/pull/1421) Throw error if there is incorrect `relatesTo` in IDX response

### Other

- [#1409](https://github.com/okta/okta-auth-js/pull/1409) Adds password page to React myaccount sample app

## 7.3.0

### Features
Expand Down Expand Up @@ -35,7 +45,7 @@

- [#1343](https://github.com/okta/okta-auth-js/pull/1343) Supports Step Up MFA against `/authorize` and `/interact` endpoints

# Other
### Other

- [#1342](https://github.com/okta/okta-auth-js/pull/1342) - fixes possible RCE in jsonpath-plus

Expand Down
2 changes: 1 addition & 1 deletion lib/idx/authenticator/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export function findMatchedOption(authenticators, options) {
let option;
for (let authenticator of authenticators) {
option = options
.find(({ relatesTo }) => relatesTo.key === authenticator.key);
.find(({ relatesTo }) => relatesTo.key && relatesTo.key === authenticator.key);
if (option) {
break;
}
Expand Down
3 changes: 3 additions & 0 deletions lib/idx/idxState/v1/idxResponseParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { OktaAuthIdxInterface } from '../../types'; // auth-js/types
import { generateRemediationFunctions } from './remediationParser';
import generateIdxAction from './generateIdxAction';
import { jsonpath } from '../../../util/jsonpath';
import { AuthSdkError } from '../../../errors';

const SKIP_FIELDS = Object.fromEntries([
'remediation', // remediations are put into proceed/neededToProceed
Expand Down Expand Up @@ -79,6 +80,8 @@ const expandRelatesTo = (idxResponse, value) => {
if (result) {
value[k] = result;
return;
} else {
throw new AuthSdkError(`Cannot resolve relatesTo: ${query}`);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/idx/remediators/Base/SelectAuthenticator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export class SelectAuthenticator<T extends SelectAuthenticatorValues = SelectAut
let option;
for (let authenticator of authenticators) {
option = options
.find(({ relatesTo }) => relatesTo.key === authenticator.key);
.find(({ relatesTo }) => relatesTo.key && relatesTo.key === authenticator.key);
if (option) {
break;
}
Expand Down
12 changes: 12 additions & 0 deletions test/spec/idx/authenticator/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,17 @@ describe('authenticator/util', () => {
const res = findMatchedOption([authenticator], [option]);
expect(res).toBe(undefined);
});
it('will not match if key is missing', () => {
const authenticator = {
id: 'a'
};
const option = {
relatesTo: {
id: 'a'
}
};
const res = findMatchedOption([authenticator], [option]);
expect(res).toBe(undefined);
});
});
});
9 changes: 9 additions & 0 deletions test/spec/idx/idxState/unit/v1/idxResponseParser.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ const mockComplexContextIdxResponse = require('../../mocks/poll-for-password');
const mockTerminalIdxResponse = require('../../mocks/terminal-return-email');
const mockMessageIdxResponse = require('../../mocks/unknown-user');
const mockSuccessIdxResponse = require('../../mocks/success');
const mockIdxResponseWithBadRelationship = () => {
const mock = require('../../mocks/authenticator-verification-password');
mock.remediation.value[1].value[0].options[0].relatesTo = '$.authenticatorEnrollments.value[999]';
return mock;
};

jest.mock('../../../../../../lib/idx/idxState/v1/generateIdxAction');
jest.mock('../../../../../../lib/idx/idxState/v1/remediationParser');
Expand Down Expand Up @@ -164,5 +169,9 @@ describe('idxResponseParser', () => {
expect( remediations[0]).toMatchSnapshot();
});

it('throws error if relatesTo can\'t be resolved', () => {
const fn = () => parseIdxResponse( {}, mockIdxResponseWithBadRelationship() );
expect(fn).toThrowError('Cannot resolve relatesTo: $.authenticatorEnrollments.value[999]');
});
});
});
32 changes: 32 additions & 0 deletions test/spec/idx/remediators/SelectAuthenticator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { SelectAuthenticatorAuthenticate } from '../../../../lib/idx/remediators';
import {
SelectAuthenticatorAuthenticateRemediationFactory,
AuthenticatorValueFactory,
PhoneAuthenticatorOptionFactory,
} from '@okta/test.support/idx';

describe('remediators/Base/SelectAuthenticator', () => {
describe('canRemediate', () => {
it('retuns false if can\'t find matched authenticator by key', () => {
const remediation = SelectAuthenticatorAuthenticateRemediationFactory.build({
value: [
AuthenticatorValueFactory.build({
options: [
PhoneAuthenticatorOptionFactory.params({
// prevent resolving of authenticator by `relatesTo` in purpose
// eslint-disable-next-line @typescript-eslint/no-explicit-any
_authenticator: 'cant_be_resolved' as any
}).build(),
]
})
]
});

const authenticators = [
{ id: 'foo' }
];
const r = new SelectAuthenticatorAuthenticate(remediation, { authenticators });
expect(r.canRemediate()).toBe(false);
});
});
});

0 comments on commit b3991a2

Please sign in to comment.