Skip to content

Commit

Permalink
Adding fixes for permission errors (403) (#925)
Browse files Browse the repository at this point in the history 
* Adding fixes for permission errors

* Updating the warning message

* Adding fixes for too_many_requests (429) error

---------

Co-authored-by: KunalOfficial <35455566+developerkunal@users.noreply.github.com>
  • Loading branch information
@nandan-bhat @developerkunal
nandan-bhat and developerkunal committed Jul 19, 2024
1 parent f803c2d commit ef19532
Show file tree
Hide file tree
Showing 3 changed files with 152 additions and 48 deletions.
5 changes: 5 additions & 0 deletions src/tools/auth0/handlers/connections.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,11 @@ export default class ConnectionsHandler extends DefaultAPIHandler {
config: this.config,
});

// We add the expected changes to the scimHandler so it can track the progress of the SCIM changes.
// This is necessary because import / deploy actions are concurrent and we need to know when all updates are complete.
const { update, create, conflicts } = proposedChangesWithExcludedProperties;
this.scimHandler.expectedChanges = update.length + create.length + conflicts.length;

return proposedChangesWithExcludedProperties;
}

Expand Down
176 changes: 131 additions & 45 deletions src/tools/auth0/handlers/scimHandler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ interface scimRequestParams {

interface scimBodyParams {
user_id_attribute: string;
mapping: { scim: string; auth0: string; }[];
mapping: { scim: string; auth0: string }[];
}

interface ScimScopes { read: boolean; create: boolean; update: boolean; delete: boolean }

/**
* The current version of this sdk use `node-auth0` v3. But `SCIM` features are not natively supported by v3.
* This is a workaround to make this SDK support SCIM without `node-auth0` upgrade.
Expand All @@ -27,6 +29,18 @@ export default class ScimHandler {
private tokenProvider: any;
private config: any;
private connectionsManager: any;
private updateQueue: any[] = [];
private isExecuting = false;
private scimScopes: ScimScopes = { read: true, create: true, update: true, delete: true };
private scopeMethodMap = {
get: 'read',
post: 'create',
patch: 'update',
delete: 'delete'
}

expectedChanges: number = 0;
completedChanges: number = 0;

constructor(config, tokenProvider, connectionsManager) {
this.config = config;
Expand Down Expand Up @@ -54,20 +68,18 @@ export default class ScimHandler {
this.idMap.clear();

for (let connection of connections) {
if (!this.scimScopes.read) return;
if (!this.isScimStrategy(connection.strategy)) continue;

try {
this.idMap.set(connection.id, { strategy: connection.strategy, hasConfig: false });
await this.getScimConfiguration({ id: connection.id });
this.idMap.set(connection.id, { ...this.idMap.get(connection.id)!, hasConfig: true });

// To avoid rate limiter error, we making API requests with a small delay.
// TODO: However, this logic needs to be re-worked.
await sleep(500);
} catch (err) {
// Skip the connection if it returns 404. This can happen if `SCIM` is not enabled on a `SCIM` connection.
if (err !== 'SCIM_NOT_FOUND') throw err;
}

// To avoid rate limiter error, we making API requests with a small delay.
// TODO: However, this logic needs to be re-worked.
await sleep(250);

this.idMap.set(connection.id, { strategy: connection.strategy, hasConfig: false });
const scimConfiguration = await this.getScimConfiguration({ id: connection.id });
if (!scimConfiguration) continue;

this.idMap.set(connection.id, { ...this.idMap.get(connection.id)!, hasConfig: true });
}
}

Expand All @@ -81,51 +93,76 @@ export default class ScimHandler {
*/
async applyScimConfiguration(connections: Asset[]) {
for (let connection of connections) {
if (!this.scimScopes.read) return connections;
if (!this.isScimStrategy(connection.strategy)) continue;

// To avoid rate limiter error, we making API requests with a small delay.
// TODO: However, this logic needs to be re-worked.
await sleep(250);

try {
const { user_id_attribute, mapping } = await this.getScimConfiguration({ id: connection.id });
connection.scim_configuration = { user_id_attribute, mapping }

// To avoid rate limiter error, we making API requests with a small delay.
// TODO: However, this logic needs to be re-worked.
await sleep(500);
} catch (err) {
// Skip the connection if it returns 404. This can happen if `SCIM` is not enabled on a `SCIM` connection.
if (err !== 'SCIM_NOT_FOUND') throw err;

const warningMessage = `SCIM configuration not found on connection \"${connection.id}\".`;
log.warn(warningMessage);
}
const scimConfiguration = await this.getScimConfiguration({ id: connection.id });
if (!scimConfiguration) continue;

const { user_id_attribute, mapping } = scimConfiguration;
connection.scim_configuration = { user_id_attribute, mapping };
}
}

/**
* HTTP request wrapper on axios.
*/
private async scimHttpRequest(method: string, options: [string, ...Record<string, any>[]]): Promise<AxiosResponse> {
return await this.withErrorHandling(async () => {
* HTTP request wrapper on axios.
*/
private async scimHttpRequest(method: string, options: [string, ...Record<string, any>[]]): Promise<AxiosResponse> {
return await this.withErrorHandling(async () => {
// @ts-ignore
const accessToken = await this.tokenProvider?.getAccessToken();
const headers = {
'Accept': 'application/json',
'Authorization': `Bearer ${ accessToken }`
}
options = [...options, { headers }];

return await axios[method](...options);
});
}, method, options[0]);
}

/**
* Error handler wrapper.
*/
async withErrorHandling(callback) {
async withErrorHandling(callback, method: string, url: string) {
try {
return await callback();
} catch (error) {
// Extract connection_id from the url.
const regex = /api\/v2\/connections\/(.*?)\/scim-configuration/;
const match = url.match(regex);
const connectionId = match ? match[1] : null;

// Extract error data
const errorData = error?.response?.data;
if (errorData?.statusCode === 404) throw "SCIM_NOT_FOUND";

// Skip the connection if it returns 404. This can happen if `SCIM` is not enabled on a `SCIM` connection.
if (errorData?.statusCode === 404) {
const warningMessage = `SCIM configuration is not enabled on connection \"${ connectionId }\".`;
log.warn(warningMessage);
return { data: null };
};

// Skip the connection if it returns 403. Looks like "scim_config" permissions are not enabled on Management API.
if (errorData?.statusCode === 403) {
const scope = this.scopeMethodMap[method];
this.scimScopes[scope] = false;
const warningMessage = `Insufficient scope, "${ scope }:scim_config". Looks like "scim_config" permissions are not enabled your application.`;
log.warn(warningMessage);
return { data: null };
}

// Skip the connection if it returns 400. This can happen if `SCIM` configuration already exists on a `SCIM` connection.
// When `read:scim_config` is disabled and `create:scim_config` is enabled, we cannot check if `SCIM` configuration exists on a connection.
// So, it'll run into 400 error.
if (errorData?.statusCode === 400 && errorData?.message?.includes('already exists')) {
const warningMessage = `SCIM configuration already exists on connection \"${ connectionId }\".`;
log.warn(warningMessage);
return { data: null };
}

const statusCode = errorData?.statusCode || error?.response?.status;
const errorCode = errorData?.errorCode || errorData?.error || error?.response?.statusText;
Expand All @@ -149,7 +186,7 @@ export default class ScimHandler {
* Creates a new `SCIM` configuration.
*/
async createScimConfiguration({ id: connection_id }: scimRequestParams, { user_id_attribute, mapping }: scimBodyParams): Promise<AxiosResponse> {
log.debug(`Creating SCIM configuration for connection ${ connection_id }`);
log.debug(`Creating SCIM configuration on connection ${ connection_id }`);
const url = this.getScimEndpoint(connection_id);
return (await this.scimHttpRequest('post', [ url, { user_id_attribute, mapping } ])).data;
}
Expand All @@ -176,7 +213,7 @@ export default class ScimHandler {
* Deletes an existing `SCIM` configuration.
*/
async deleteScimConfiguration({ id: connection_id }: scimRequestParams): Promise<AxiosResponse> {
log.debug(`Deleting SCIM configuration of connection ${ connection_id }`);
log.debug(`Deleting SCIM configuration on connection ${ connection_id }`);
const url = this.getScimEndpoint(connection_id);
return (await this.scimHttpRequest('delete', [ url ])).data;
}
Expand All @@ -190,24 +227,29 @@ export default class ScimHandler {
// First, update `connections`.
const updated = await this.connectionsManager.update(requestParams, bodyParams);
const idMapEntry = this.idMap.get(requestParams.id);
this.completedChanges ++;

// Now, update `scim_configuration` inside the updated connection.
// If `scim_configuration` exists in both local and remote -> updateScimConfiguration(...)
// If `scim_configuration` exists in remote but local -> deleteScimConfiguration(...)
// If `scim_configuration` exists in local but remote -> createScimConfiguration(...)
if (idMapEntry?.hasConfig) {
if (scimBodyParams) {
await this.updateScimConfiguration(requestParams, scimBodyParams);
this.updateQueue.push({action: 'update', requestParams, scimBodyParams});
} else {
if (this.config('AUTH0_ALLOW_DELETE')) {
log.warn(`Deleting scim_configuration on connection ${ requestParams.id }.`);
await this.deleteScimConfiguration(requestParams);
this.updateQueue.push({action: 'delete', requestParams});
} else {
log.warn('Skipping DELETE scim_configuration. Enable deletes by setting AUTH0_ALLOW_DELETE to true in your config.');
}
}
} else if (scimBodyParams) {
await this.createScimConfiguration(requestParams, scimBodyParams);
this.updateQueue.push({action: 'create', requestParams, scimBodyParams});
}

// Execute the queue.
if (this.completedChanges >= this.expectedChanges) {
await this.executeQueue();
}

// Return response from connections.update(...).
Expand All @@ -222,12 +264,56 @@ export default class ScimHandler {

// First, create the new `connection`.
const created = await this.connectionsManager.create(bodyParams);
this.completedChanges ++;

if (scimBodyParams) {
// Now, create the `scim_configuration` for newly created `connection`.
await this.createScimConfiguration({ id: created.id }, scimBodyParams);
this.updateQueue.push({action: 'create', requestParams: {id: created.id}, scimBodyParams});
}

// Execute the queue.
if (this.completedChanges >= this.expectedChanges) {
await this.executeQueue();
}

// Return response from connections.create(...).
return created;
}
}

/**
* If we perform `connectionsManager.update` and `updateScimConfiguration` together, they may result in rate limit error.
* The reason is that both of them make API requests to the same `api/v2/connections` endpoint.
* We cannot control it with delay because both `updateOverride` and `createOverride` are async functions. And being called concurrently by `PromisePoolExecutor`.
* To avoid this, we are queuing the `SCIM` actions and executing them one by one separately, only after `connectionsManager.update` is completed.
*
* This is true for both `create` and `update` actions.
* @returns {Promise<void>}
*/
async executeQueue() {
if (this.isExecuting) return;

this.isExecuting = true;
while (this.updateQueue.length > 0) {
// Rate limit error handling
await sleep(250);
const { action, requestParams, scimBodyParams } = this.updateQueue.shift();

switch (action) {
case 'create':
if (this.scimScopes.create) await this.createScimConfiguration(requestParams, scimBodyParams);
break;
case 'update':
if (this.scimScopes.update) await this.updateScimConfiguration(requestParams, scimBodyParams);
break;
case 'delete':
if (this.scimScopes.delete) await this.deleteScimConfiguration(requestParams);
break;
}
}

this.isExecuting = false;
this.expectedChanges = 0;
this.completedChanges = 0;
}
}

19 changes: 16 additions & 3 deletions test/tools/auth0/handlers/scimHandler.tests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('ScimHandler', () => {
const connections = [
{ id: 'con_KYp633cmKtnEQ31C', strategy: 'samlp' }, // SCIM connection.
{ id: 'con_Njd1bxE3QTqTRwAk', strategy: 'auth0' }, // Non-SCIM connection.
{ id: 'con_d3tmuoAkaUQgxN1f', strategy: 'gmail' } // Connection which doesn't exist.
{ id: 'con_d3tmuoAkaUQgxN1f', strategy: 'gmail' }, // Connection which doesn't exist.
];
const getScimConfigurationStub = sinon.stub(scimHandler, 'getScimConfiguration');
getScimConfigurationStub.withArgs({ id: 'con_KYp633cmKtnEQ31C' }).resolves({ user_id_attribute: 'externalId-115', mapping: [{ auth0: 'auth0_key', scim: 'scim_key' }] });
Expand Down Expand Up @@ -77,7 +77,7 @@ describe('ScimHandler', () => {
const getScimConfigurationStub = sinon.stub(scimHandler, 'getScimConfiguration');
getScimConfigurationStub.withArgs({ id: 'con_KYp633cmKtnEQ31C' }).resolves({ user_id_attribute: 'externalId-1', mapping: [{ auth0: 'auth0_key', scim: 'scim_key' }] });
getScimConfigurationStub.withArgs({ id: 'con_Njd1bxE3QTqTRwAk' }).resolves({ user_id_attribute: 'externalId-2', mapping: [{ auth0: 'auth0_key', scim: 'scim_key' }] });
getScimConfigurationStub.withArgs({ id: 'con_d3tmuoAkaUQgxN1f' }).rejects({ response: { status: 404 } });
getScimConfigurationStub.withArgs({ id: 'con_d3tmuoAkaUQgxN1f' }).rejects({ response: { data: { statusCode: 404 } } });

await scimHandler.applyScimConfiguration(connections);

Expand Down Expand Up @@ -132,24 +132,37 @@ describe('ScimHandler', () => {

const axiosStub = sinon.stub(axios, 'get').resolves({ data: scimConfiguration, status: 201 });
const response = await scimHandler.getScimConfiguration(requestParams);
// eslint-disable-next-line no-unused-expressions
expect(response).to.deep.equal(scimConfiguration);

axiosStub.restore();
});

it('should throw error for non-existing SCIM configuration', async () => {
const requestParams = { id: 'con_KYp633cmKtnEQ31C' };
const axiosStub = sinon.stub(axios, 'get').rejects({ response: { status: 404, errorCode: 'not-found', statusText: 'The connection does not exist' } });
const axiosStub = sinon.stub(axios, 'get').rejects({ response: { status: 404, errorCode: 'not_found', statusText: 'The connection does not exist.' } });

try {
await scimHandler.getScimConfiguration(requestParams);
expect.fail('Expected getScimConfiguration to throw an error');
} catch (error) {
// eslint-disable-next-line no-unused-expressions
expect(error.response.status).to.equal(404);
}

axiosStub.restore();
});

it('should not throw error for scim connections when SCIM permissions disabled.', async () => {
const requestParams = { id: 'con_KYp633cmKtnEQ31C' };
const axiosStub = sinon.stub(axios, 'get').rejects({ response: { data: { statusCode: 403, errorCode: 'insufficient_scope', statusText: 'Insufficient scope, expected any of: read:scim_config.' } } });

const data = await scimHandler.getScimConfiguration(requestParams);
// eslint-disable-next-line no-unused-expressions
expect(data).to.be.null;

axiosStub.restore();
});
});

describe('#createScimConfiguration', () => {
Expand Down

0 comments on commit ef19532

Please sign in to comment.