Skip to content

Commit

Permalink
Fix: Add retry handler for revoking permissions (#2515)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Onokaev
Onokaev authored Apr 26, 2023
1 parent a82e0cb commit d7d8931
Show file tree
Hide file tree
Showing 6 changed files with 281 additions and 133 deletions.
79 changes: 66 additions & 13 deletions src/app/services/actions/permissions-action-creator.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import {
FETCH_SCOPES_ERROR,
FETCH_FULL_SCOPES_SUCCESS,
FETCH_URL_SCOPES_PENDING,
QUERY_GRAPH_STATUS
FETCH_URL_SCOPES_PENDING
} from '../../../app/services/redux-constants';

import {
Expand Down Expand Up @@ -299,7 +298,7 @@ describe('Permissions action creators', () => {
});

describe('Revoke scopes', () => {
it('should return Default Scope error when user tries to dissent to default scope', () => {
it('should return Default Scope error when user tries to dissent to default scope', async () => {
// Arrange
const store_ = mockStore(mockState);
jest.spyOn(RevokePermissionsUtil, 'getServicePrincipalId').mockResolvedValue('1234');
Expand Down Expand Up @@ -332,11 +331,48 @@ describe('Permissions action creators', () => {
'@odata.context': 'https://graph.microsoft.com/v1.0/$metadata#permissionGrants'
})

const revokePermissionsUtil = await RevokePermissionsUtil.initialize('kkk');

jest.spyOn(revokePermissionsUtil, 'getUserPermissionChecks').mockResolvedValue({
userIsTenantAdmin: false,
permissionBeingRevokedIsAllPrincipal: true,
grantsPayload: {
value: [
{
clientId: '1234',
consentType: 'Principal',
principalId: '1234',
resourceId: '1234',
scope: 'profile.read User.Read',
id: 'SomeNiceId'
},
{
clientId: '',
consentType: 'AllPrincipal',
principalId: null,
resourceId: '1234',
scope: 'profile.read User.Read Directory.Read.All'
}
],
'@odata.context': 'https://graph.microsoft.com/v1.0/$metadata#permissionGrants'
}
})

const expectedActions = [
{ type: 'REVOKE_SCOPES_PENDING', response: null },
{ type: 'REVOKE_SCOPES_ERROR', response: null },
{
type: QUERY_GRAPH_STATUS,
type: 'QUERY_GRAPH_STATUS',
response: {
statusText: 'Revoking ',
status: 'Please wait while we revoke this permission',
ok: false,
messageType: 0
}
},
{ type: 'REVOKE_SCOPES_ERROR', response: null },
{
type: 'QUERY_GRAPH_STATUS',
response: {
statusText: 'Failed',
status: 'An error occurred when unconsenting. Please try again',
Expand All @@ -356,7 +392,7 @@ describe('Permissions action creators', () => {
});
});

it('should return 401 when user does not have required permissions', () => {
it('should return 401 when user does not have required permissions', async () => {
// Arrange
const store_ = mockStore(mockState);
jest.spyOn(RevokePermissionsUtil, 'getServicePrincipalId').mockResolvedValue('1234');
Expand Down Expand Up @@ -393,7 +429,17 @@ describe('Permissions action creators', () => {
{ type: 'REVOKE_SCOPES_PENDING', response: null },
{ type: 'REVOKE_SCOPES_ERROR', response: null },
{
type: QUERY_GRAPH_STATUS,
type: 'QUERY_GRAPH_STATUS',
response: {
statusText: 'Revoking ',
status: 'Please wait while we revoke this permission',
ok: false,
messageType: 0
}
},
{ type: 'REVOKE_SCOPES_ERROR', response: null },
{
type: 'QUERY_GRAPH_STATUS',
response: {
statusText: 'Failed',
status: 'An error occurred when unconsenting. Please try again',
Expand All @@ -414,7 +460,7 @@ describe('Permissions action creators', () => {
});

//revisit
it('should raise error when user attempts to dissent to an admin granted permission', () => {
it('should raise error when user attempts to dissent to an admin granted permission', async () => {
// Arrange
const store_ = mockStore(mockState);
jest.spyOn(RevokePermissionsUtil, 'getServicePrincipalId').mockResolvedValue('1234');
Expand Down Expand Up @@ -454,7 +500,17 @@ describe('Permissions action creators', () => {
{ type: 'REVOKE_SCOPES_PENDING', response: null },
{ type: 'REVOKE_SCOPES_ERROR', response: null },
{
type: QUERY_GRAPH_STATUS,
type: 'QUERY_GRAPH_STATUS',
response: {
statusText: 'Revoking ',
status: 'Please wait while we revoke this permission',
ok: false,
messageType: 0
}
},
{ type: 'REVOKE_SCOPES_ERROR', response: null },
{
type: 'QUERY_GRAPH_STATUS',
response: {
statusText: 'Failed',
status: 'An error occurred when unconsenting. Please try again',
Expand All @@ -466,11 +522,8 @@ describe('Permissions action creators', () => {

// Act and Assert
// @ts-ignore
return store_.dispatch(revokeScopes('Access.Read'))
// @ts-ignore
.then(() => {
expect(store_.getActions()).toEqual(expectedActions);
});
await store_.dispatch(revokeScopes('Access.Read'));
expect(store_.getActions()).toEqual(expectedActions);
});
})
})
142 changes: 99 additions & 43 deletions src/app/services/actions/permissions-action-creator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,13 +228,26 @@ const validateConsentedScopes = (scopeToBeConsented: string[], consentedScopes:
return expectedScopes;
}

interface IPermissionUpdate {
permissionBeingRevokedIsAllPrincipal: boolean;
userIsTenantAdmin: boolean;
revokePermissionUtil: RevokePermissionsUtil;
grantsPayload: IOAuthGrantPayload;
profile: IUser;
permissionToRevoke: string;
newScopesArray: string[];
retryCount: number;
retryDelay: number;
dispatch: Function;
}
export function revokeScopes(permissionToRevoke: string) {
return async (dispatch: Function, getState: Function) => {
const { consentedScopes, profile } = getState();
const requiredPermissions = REVOKING_PERMISSIONS_REQUIRED_SCOPES.split(' ');
const defaultUserScopes = DEFAULT_USER_SCOPES.split(' ');
const revokePermissionUtil = await RevokePermissionsUtil.initialize(profile.id);
dispatch(revokeScopesPending());
dispatchScopesStatus(dispatch, 'Please wait while we revoke this permission', 'Revoking ', 0);

if (!consentedScopes || consentedScopes.length === 0) {
dispatch(revokeScopesError());
Expand All @@ -243,38 +256,35 @@ export function revokeScopes(permissionToRevoke: string) {
}

const newScopesArray: string[] = consentedScopes.filter((scope: string) => scope !== permissionToRevoke);
const newScopesString: string = newScopesArray.join(' ');

try {
const { userIsTenantAdmin, permissionBeingRevokedIsAllPrincipal, grantsPayload } = await revokePermissionUtil.
getUserPermissionChecks({ consentedScopes, requiredPermissions, defaultUserScopes, permissionToRevoke });

let updatedScopes;
if (permissionBeingRevokedIsAllPrincipal && userIsTenantAdmin) {
updatedScopes = await revokePermissionUtil.
getUpdatedAllPrincipalPermissionGrant(grantsPayload, permissionToRevoke);
}
else {
updatedScopes = await revokePermissionUtil.
updateSinglePrincipalPermissionGrant(grantsPayload, profile, newScopesString);
const retryCount = 0;
const retryDelay = 100;
const permissionsUpdateObject: IPermissionUpdate = {
permissionBeingRevokedIsAllPrincipal, userIsTenantAdmin, revokePermissionUtil, grantsPayload,
profile, permissionToRevoke, newScopesArray, retryCount, dispatch, retryDelay }

const updatedScopes = await updatePermissions(permissionsUpdateObject)

if (updatedScopes) {
dispatchScopesStatus(dispatch, 'Permission revoked', 'Success', 4);
dispatch(getConsentedScopesSuccess(updatedScopes));
dispatch(revokeScopesSuccess());
trackRevokeConsentEvent(REVOKE_STATUS.success, permissionToRevoke);
}

if (updatedScopes.length !== newScopesArray.length) {
else{
throw new RevokeScopesError({
errorText: 'Scopes not updated', statusText: 'An error occurred when unconsenting',
status: '500', messageType: 1
})
}

dispatchRevokeScopesStatus(dispatch, 'Permission revoked', 'Success', 4);
dispatch(getConsentedScopesSuccess(updatedScopes));
dispatch(revokeScopesSuccess());
trackRevokeConsentEvent(REVOKE_STATUS.success, permissionToRevoke);
}
catch (errorMessage: any) {
if (errorMessage instanceof RevokeScopesError || errorMessage instanceof Function) {
const { errorText, statusText, status, messageType } = errorMessage
dispatchRevokeScopesStatus(dispatch, statusText, status, messageType);
dispatchScopesStatus(dispatch, statusText, status, messageType);
const permissionObject = {
permissionToRevoke,
statusCode: statusText,
Expand All @@ -285,13 +295,46 @@ export function revokeScopes(permissionToRevoke: string) {
else {
const { code, message } = errorMessage;
trackRevokeConsentEvent(REVOKE_STATUS.failure, 'Failed to revoke consent');
dispatchRevokeScopesStatus(dispatch, message ? message : 'Failed to revoke consent', code ? code : 'Failed', 1);
dispatchScopesStatus(dispatch, message ? message : 'Failed to revoke consent', code ? code : 'Failed', 1);
}
}
}
}

const dispatchRevokeScopesStatus = (dispatch: Function, statusText: string, status: string, messageType: number) => {
async function updatePermissions(permissionsUpdateObject: IPermissionUpdate):
Promise<string[] | null> {
const {
permissionBeingRevokedIsAllPrincipal, userIsTenantAdmin, revokePermissionUtil, grantsPayload,
profile, permissionToRevoke, newScopesArray, retryCount, dispatch, retryDelay } = permissionsUpdateObject;
let isRevokeSuccessful;
const maxRetryCount = 7;
const newScopesString = newScopesArray.join(' ');

if (permissionBeingRevokedIsAllPrincipal && userIsTenantAdmin) {
isRevokeSuccessful = await revokePermissionUtil.getUpdatedAllPrincipalPermissionGrant(grantsPayload,
permissionToRevoke);
} else {
isRevokeSuccessful = await revokePermissionUtil.updateSinglePrincipalPermissionGrant(grantsPayload, profile,
newScopesString);
}

if (isRevokeSuccessful) {
return newScopesString.split(' ');
}
else if((retryCount < maxRetryCount) && !isRevokeSuccessful) {
await new Promise(resolve => setTimeout(resolve, retryDelay * 2));
dispatchScopesStatus(dispatch, 'We are retrying the revoking operation', 'Retrying', 5);

permissionsUpdateObject.retryCount += 1;
return updatePermissions(permissionsUpdateObject);
}
else{
return null;
}

}

const dispatchScopesStatus = (dispatch: Function, statusText: string, status: string, messageType: number) => {
dispatch(revokeScopesError());
dispatch(
setQueryResponseStatus({
Expand All @@ -315,29 +358,18 @@ export function fetchAllPrincipalGrants() {
return async (dispatch: Function, getState: Function) => {
try {
const { profile, consentedScopes, scopes } = getState();
let tenantWideGrant: IOAuthGrantPayload = scopes.data.tenantWidePermissionsGrant;
let revokePermissionUtil = await RevokePermissionsUtil.initialize(profile.id);
const servicePrincipalAppId = revokePermissionUtil.getServicePrincipalAppId();
dispatch(getAllPrincipalGrantsPending(true));
let requestCounter = 0;

if (servicePrincipalAppId) {
tenantWideGrant = revokePermissionUtil.getGrantsPayload();
if(tenantWideGrant){
if (!allScopesHaveConsentType(consentedScopes, tenantWideGrant, profile.id)){
while (requestCounter < 10 && profile && profile.id &&
!allScopesHaveConsentType(consentedScopes, tenantWideGrant, profile.id)) {
requestCounter += 1;
revokePermissionUtil = await RevokePermissionsUtil.initialize(profile.id);
dispatch(getAllPrincipalGrantsPending(true));
tenantWideGrant = revokePermissionUtil.getGrantsPayload();
}
dispatchGrantsStatus(dispatch, tenantWideGrant.value)
}
else{
dispatchGrantsStatus(dispatch, tenantWideGrant.value)
}
}
const tenantWideGrant: IOAuthGrantPayload = scopes.data.tenantWidePermissionsGrant;
const revokePermissionUtil = await RevokePermissionsUtil.initialize(profile.id);
if (revokePermissionUtil && revokePermissionUtil.getGrantsPayload() !== null){
const servicePrincipalAppId = revokePermissionUtil.getServicePrincipalAppId();
dispatch(getAllPrincipalGrantsPending(true));
const requestCounter = 0;

await checkScopesConsentType(servicePrincipalAppId, tenantWideGrant, revokePermissionUtil,
consentedScopes, profile, requestCounter, dispatch);
}
else{
dispatchScopesStatus(dispatch, 'Permissions', 'You require the following permissions to read', 0)
}
} catch (error: any) {
dispatch(getAllPrincipalGrantsPending(false));
Expand Down Expand Up @@ -381,3 +413,27 @@ export const getSinglePrincipalGrant = (tenantWideGrant: IPermissionGrant[], pri
}
return [];
}
async function checkScopesConsentType(servicePrincipalAppId: string, tenantWideGrant: IOAuthGrantPayload,
revokePermissionUtil: RevokePermissionsUtil, consentedScopes: string[], profile: IUser,
requestCounter: number, dispatch: Function) {
if (servicePrincipalAppId) {
tenantWideGrant = revokePermissionUtil.getGrantsPayload();
if (tenantWideGrant) {
if (!allScopesHaveConsentType(consentedScopes, tenantWideGrant, profile.id)) {
while (requestCounter < 10 && profile && profile.id &&
!allScopesHaveConsentType(consentedScopes, tenantWideGrant, profile.id)) {
requestCounter += 1;
await new Promise((resolve) => setTimeout(resolve, 400 * requestCounter));
revokePermissionUtil = await RevokePermissionsUtil.initialize(profile.id);
dispatch(getAllPrincipalGrantsPending(true));
tenantWideGrant = revokePermissionUtil.getGrantsPayload();
}
dispatchGrantsStatus(dispatch, tenantWideGrant.value);
}
else {
dispatchGrantsStatus(dispatch, tenantWideGrant.value);
}
}
}
}

Loading

0 comments on commit d7d8931

Please sign in to comment.