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.

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: Unconsent permissions #2063

Merged
merged 97 commits into from
Nov 8, 2022
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
97 commits
Select commit Hold shift + click to select a range
08f736c
Modify permissions tab user interface
MaryannGitonga Aug 23, 2022
b1e3a13
Merge branch 'dev' of https://github.com/microsoftgraph/microsoft-gra…
MaryannGitonga Aug 23, 2022
e00096d
Add unconsent permissions logic
MaryannGitonga Aug 26, 2022
568a0dc
update redux with new scope
Onokaev Aug 26, 2022
0dabe41
put returned permissions in an array before updating redux store
Onokaev Aug 26, 2022
aa263c0
get new token
Onokaev Aug 29, 2022
5aa85db
add revoking state
Onokaev Aug 30, 2022
bf1f78d
do default checks before unconsenting
Onokaev Aug 30, 2022
493568e
Merge branch 'dev' of https://github.com/microsoftgraph/microsoft-gra…
MaryannGitonga Aug 30, 2022
14587d3
Merge branch 'feature/unconsent-permissions' of https://github.com/mi…
MaryannGitonga Aug 30, 2022
07bb907
update status message
Onokaev Aug 30, 2022
daa5d42
log user in to get a fresh token
Onokaev Aug 30, 2022
2796f99
Merge branch 'feature/unconsent-permissions' of https://github.com/mi…
MaryannGitonga Aug 30, 2022
54e8ff4
Add default scopes for unconsenting permissions as a graph constant
MaryannGitonga Aug 31, 2022
e74094e
Merge branch 'dev' of https://github.com/microsoftgraph/microsoft-gra…
MaryannGitonga Aug 31, 2022
f50ba35
Merge branch 'dev' of https://github.com/microsoftgraph/microsoft-gra…
MaryannGitonga Sep 1, 2022
804622e
Merge branch 'dev' into feature/unconsent-permissions
MaryannGitonga Sep 1, 2022
df42943
Add check for required permissions, remove the button on the panel li…
MaryannGitonga Sep 1, 2022
3b41856
Merge branch 'dev' of https://github.com/microsoftgraph/microsoft-gra…
MaryannGitonga Sep 1, 2022
832c74d
Merge branch 'feature/unconsent-permissions' of https://github.com/mi…
MaryannGitonga Sep 1, 2022
900a11a
Refactor logic
MaryannGitonga Sep 2, 2022
00dae51
Refactor graph client
MaryannGitonga Sep 5, 2022
70bbd0a
streamline functions
thewahome Sep 5, 2022
0228fa6
Refactor unconsenting permissions code
MaryannGitonga Sep 5, 2022
dda725d
Merge branch 'feature/unconsent-permissions' of https://github.com/mi…
MaryannGitonga Sep 5, 2022
a000cf3
fix filtering on getting grants
Onokaev Sep 6, 2022
cc57352
Remove comments & remove patchQuery
MaryannGitonga Sep 7, 2022
b0f8ad2
add status code to received errors
Onokaev Sep 7, 2022
30dd437
retry getting an access token once
Onokaev Sep 7, 2022
c293348
Make changes suggested on PR
MaryannGitonga Sep 7, 2022
7b7d90e
Merge branch 'dev' into feature/unconsent-permissions
MaryannGitonga Sep 7, 2022
9149a7b
Remove unused code
MaryannGitonga Sep 7, 2022
8bb61df
Merge branch 'feature/unconsent-permissions' of https://github.com/mi…
MaryannGitonga Sep 7, 2022
4defa7b
Remove unused code
MaryannGitonga Sep 7, 2022
456ff99
Change unconsent to revoke
MaryannGitonga Sep 7, 2022
e09db37
fix failing test
Onokaev Sep 8, 2022
a7d31d7
remove unused import
Onokaev Sep 8, 2022
f12a532
fix crashing on permissions panel
Onokaev Sep 8, 2022
1b07855
add padding to Modify Permissions tab
Onokaev Sep 12, 2022
505d5cb
use the abstracted makeGraphRequest function
Onokaev Sep 12, 2022
712dae8
change getPermissionResponse to a more generic name
Onokaev Sep 12, 2022
a8bf214
remove unused import
Onokaev Sep 12, 2022
036aeb8
Merge branch 'dev' into feature/unconsent-permissions
Onokaev Sep 14, 2022
307df29
refactor revokeScopes function
Onokaev Sep 15, 2022
8770911
add telemetry for revoking permissions
Onokaev Sep 15, 2022
f134b17
remove comments
Onokaev Sep 15, 2022
7313eb4
update failing test
Onokaev Sep 15, 2022
a429a32
add Consented string
Onokaev Sep 16, 2022
b0a3316
remove selection on panel list
Onokaev Oct 6, 2022
5eb5a28
remove overflow from cellName
Onokaev Oct 6, 2022
e0b1aff
Merge branch 'dev' into feature/unconsent-permissions
Onokaev Oct 6, 2022
2d6d36e
add required text
Onokaev Oct 6, 2022
8edfd06
Merge branch 'dev' into feature/unconsent-permissions
Onokaev Oct 11, 2022
9ef7da4
prevent users from dissenting to admin pre-consented permissions
Onokaev Oct 11, 2022
140e62d
fix code smells
Onokaev Oct 11, 2022
45b573f
remove unnecessary await
Onokaev Oct 11, 2022
391f039
Merge branch 'dev' into feature/unconsent-permissions
Onokaev Oct 14, 2022
f136f7d
check if user dissenting is an admin
Onokaev Oct 17, 2022
da3a174
allow admins to revoke all principal grants
Onokaev Oct 18, 2022
bc92d2a
prevent all users from dissenting to AllPrincipal permissions
Onokaev Oct 18, 2022
ad50d68
add messages to Ge.json file
Onokaev Oct 18, 2022
aacd5c8
remove admin roles from graph constants
Onokaev Oct 18, 2022
d97346e
add telemetry for all principal scopes
Onokaev Oct 18, 2022
eebcd91
allow tenant admin to revoke allprincipal permissions
Onokaev Oct 24, 2022
7361106
Merge branch 'dev' into feature/unconsent-permissions
Onokaev Oct 24, 2022
3202ed3
check for required scopes within allprincipal grant
Onokaev Oct 25, 2022
030bdb5
Merge branch 'feature/unconsent-permissions' of https://github.com/mi…
Onokaev Oct 25, 2022
51c9003
Task: Add consent type column (#2180)
Onokaev Oct 31, 2022
52983a1
remove styling on status column
Onokaev Oct 31, 2022
cbcad9f
Merge branch 'dev' into feature/unconsent-permissions
Onokaev Oct 31, 2022
38ed307
fix merge conflicts
Onokaev Oct 31, 2022
49bd93a
fix case
Onokaev Oct 31, 2022
14b49b4
add tests
Onokaev Oct 31, 2022
953b361
fix code smells
Onokaev Oct 31, 2022
38602df
Merge branch 'dev' into feature/unconsent-permissions
Onokaev Oct 31, 2022
b0fdd53
fix failing test
Onokaev Oct 31, 2022
58302cc
Merge branch 'feature/unconsent-permissions' of https://github.com/mi…
Onokaev Oct 31, 2022
7e86d1f
fix failing tests
Onokaev Oct 31, 2022
3bb5481
add link telemetry
Onokaev Oct 31, 2022
d50fe6e
update error status reporting
Onokaev Oct 31, 2022
49e2c69
add check for allgrants availability
Onokaev Nov 1, 2022
9ba635a
adjust tests
Onokaev Nov 1, 2022
c7fa6d5
refactor revokeScopes function
Onokaev Nov 1, 2022
650bbea
show consentType regardless of tiken being present
Onokaev Nov 1, 2022
3399c89
add icon to Unconsent button
Onokaev Nov 1, 2022
d2bcfe9
fix button size
Onokaev Nov 1, 2022
3a74f45
fix tests
Onokaev Nov 1, 2022
0c549b5
fix code smells
Onokaev Nov 2, 2022
f49b263
remove string[] assertion
Onokaev Nov 3, 2022
b474166
update message when revoking scopes fails
Onokaev Nov 3, 2022
0cea417
add more styling to info icon
Onokaev Nov 3, 2022
7648117
update message when permission propagation delays
Onokaev Nov 7, 2022
d23717d
Merge branch 'dev' into feature/unconsent-permissions
Onokaev Nov 7, 2022
267c649
fix merge conflicts
Onokaev Nov 7, 2022
8802bdd
fix failing test
Onokaev Nov 8, 2022
c3c0ee2
increase tab header height
Onokaev Nov 8, 2022
696ccf9
dynamically adjust header height
Onokaev Nov 8, 2022
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
5 changes: 5 additions & 0 deletions src/app/services/actions/autocomplete-action-creators.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ const mockState: IRootState = {
pending: false,
data: {},
error: null
},
unconsentingScopes: {
pending: false,
data: [],
error: null
}
}

Expand Down
5 changes: 5 additions & 0 deletions src/app/services/actions/permissions-action-creator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,11 @@ const mockState: IRootState = {
pending: false,
data: {},
error: null
},
unconsentingScopes: {
pending: false,
data: [],
error: null
}
}
const currentState = store.getState();
Expand Down
201 changes: 199 additions & 2 deletions src/app/services/actions/permissions-action-creator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,29 @@ import { sanitizeQueryUrl } from '../../utils/query-url-sanitization';
import { parseSampleUrl } from '../../utils/sample-url-generation';
import { translateMessage } from '../../utils/translate-messages';
import { getConsentAuthErrorHint } from '../../../modules/authentication/authentication-error-hints';
import { ACCOUNT_TYPE, PERMS_SCOPE } from '../graph-constants';
import {
ACCOUNT_TYPE, DEFAULT_USER_SCOPES, GRAPH_URL, PERMS_SCOPE,
UNCONSENTING_PERMISSIONS_REQUIRED_SCOPES
} from '../graph-constants';
import {
FETCH_SCOPES_ERROR,
FETCH_FULL_SCOPES_PENDING,
FETCH_URL_SCOPES_PENDING,
FETCH_FULL_SCOPES_SUCCESS,
FETCH_URL_SCOPES_SUCCESS
FETCH_URL_SCOPES_SUCCESS,
REVOKE_PERMISSION_PENDING,
REVOKE_PERMISSION_SUCCESS,
REVOKE_PERMISSION_ERROR
} from '../redux-constants';
import {
getAuthTokenSuccess,
getConsentedScopesSuccess
} from './auth-action-creators';
import { getProfileInfo } from './profile-action-creators';
import { setQueryResponseStatus } from './query-status-action-creator';
import { GraphClient } from '../graph-client';
import { IQuery } from '../../../types/query-runner';
import { makeGraphRequest, parseResponse } from './query-action-creator-util';

export function fetchFullScopesSuccess(response: object): IAction {
return {
Expand All @@ -50,6 +59,27 @@ export function fetchScopesError(response: object): IAction {
};
}

export function revokePermissionPending(response: boolean): any {
return {
type: REVOKE_PERMISSION_PENDING,
response
}
}

export function revokePermissionSuccess(response: boolean): any {
return {
type: REVOKE_PERMISSION_SUCCESS,
response
}
}

export function revokePermissionError(response: object): any {
return {
type: REVOKE_PERMISSION_ERROR,
response
}
}

export function fetchScopes(): Function {
return async (dispatch: Function, getState: Function) => {
try {
Expand Down Expand Up @@ -142,3 +172,170 @@ export function consentToScopes(scopes: string[]): Function {
}
};
}

export function unconsentToScopes(permissionToDelete: string): Function {
return async (dispatch: Function, getState: Function) => {
const { consentedScopes, profile } = getState();
const requiredPermissions = UNCONSENTING_PERMISSIONS_REQUIRED_SCOPES.split(' ');
const defaultScopes = DEFAULT_USER_SCOPES.split(' ');
let response = null;

if (userUnconsentingToDefaultScopes(defaultScopes, permissionToDelete)) {
dispatch(
setQueryResponseStatus({
statusText: translateMessage('Default scope'),
status: translateMessage('Cannot delete default scope'),
ok: false,
messageType: MessageBarType.error
})
);
return;
}

if (!userHasRequiredPermissions(requiredPermissions, consentedScopes)) {
dispatch(
setQueryResponseStatus({
statusText: translateMessage('Unable to dissent'),
status: translateMessage('You require the following permissions to unconsent'),
ok: false,
messageType: MessageBarType.error
})
);
return;
}

try {
if (!consentedScopes && consentedScopes.length < 1) {
return;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't need to be in the try catch as it has no chances of error. We can move it just above as it is defensive


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

const servicePrincipalAppId = await getCurrentAppId(consentedScopes);
response = await getPermissionGrant(consentedScopes, servicePrincipalAppId, profile.id);
const permissionGrantId = response.id;

await revokePermission(consentedScopes, permissionGrantId, newScopesString);

response = await getPermissionGrant(consentedScopes, servicePrincipalAppId, profile.id);
const updatedScopes = response.scope.split(' ');

if (updatedScopes.length !== newScopesArray.length) {
return;
}

const authResponse = await getNewAuthObject(updatedScopes);
console.log('We have the authentication response. Now going back')

if (authResponse && authResponse.accessToken) {
console.log('We are here now after getting the auth results')
dispatch(getAuthTokenSuccess(true));
dispatch(getConsentedScopesSuccess(authResponse.scopes));
dispatch(revokePermissionSuccess(true));
dispatch(
setQueryResponseStatus({
statusText: translateMessage('Success'),
status: translateMessage('Permission unconsented'),
ok: true,
messageType: MessageBarType.success
})
);
// if (authResponse.account && authResponse.account.localAccountId !== profile?.id) {
// dispatch(getProfileInfo());
// }
}

}
catch (error: any) {
dispatch(revokePermissionError(error));
dispatch(
setQueryResponseStatus({
statusText: translateMessage('Unable to dissent'),
// eslint-disable-next-line max-len
status: error ? error : translateMessage('An error was encountered when dissenting. Confirm that you have the right permissions'),
ok: false,
messageType: MessageBarType.error
})
);
}
}
}

const getQuery: IQuery = {
selectedVerb: 'GET',
sampleHeaders: [
{
name: 'Cache-Control',
value: 'no-cache'
}
],
selectedVersion: '',
sampleUrl: ''
};

const patchQuery: IQuery = {
selectedVerb: 'PATCH',
sampleHeaders: [
{
name: 'Cache-Control',
value: 'no-cache'
}
],
selectedVersion: '',
sampleUrl: ''
};

const userHasRequiredPermissions = (requiredPermissions: string[],
consentedScopes: string[]) => {
return requiredPermissions.every(scope => consentedScopes.includes(scope));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The functionality here can also be moved to the getPermissionResponse function so that we do not need to import the Graph Client. getPermissionResponse can be renamed to something more generic

}

const userUnconsentingToDefaultScopes = (currentScopes: string[], permissionToDelete: string) => {
return currentScopes.includes(permissionToDelete);
}

const getCurrentAppId = async (scopes: string[]) => {
const currentAppId = process.env.REACT_APP_CLIENT_ID;
getQuery.sampleUrl = `${GRAPH_URL}/v1.0/servicePrincipals?$filter=appId eq '${currentAppId}'`;
const response = await getPermissionResponse(scopes, getQuery);
return response.value[0].id;
}
const revokePermission = async (oldScopes: string[], permissionGrantId: string, newScopes: string) => {
patchQuery.sampleUrl = `${GRAPH_URL}/v1.0/oauth2PermissionGrants/${permissionGrantId}`;
patchQuery.sampleBody = JSON.stringify({
scope: newScopes
});
await getPermissionResponse(oldScopes, patchQuery);
}

const getPermissionGrant = async (scopes: string[], servicePrincipalAppId: string, principalid: string) => {
getQuery.sampleUrl = `${GRAPH_URL}/v1.0/oauth2PermissionGrants?$filter=clientId eq '${servicePrincipalAppId}'`;
const response = await getPermissionResponse(scopes, getQuery);

if (response && response.value.length > 1) {
const filteredResponse = response.value.filter((permissionGrant: any) =>
permissionGrant.principalId === principalid);
return filteredResponse[0];
}
return response.value[0];
}

const getPermissionResponse = async (scopes: string[], query: IQuery) => {
const respHeaders: any = {};
const response = await makeGraphRequest(scopes)(query);
return parseResponse(response, respHeaders);
}

const getNewAuthObject = async (updatedScopes: string[]) => {
let authResponse = await authenticationWrapper.consentToScopes(updatedScopes);
let count = 4;

while ((authResponse.scopes.length !== updatedScopes.length) && count > 0) {
authResponse = await authenticationWrapper.consentToScopes(updatedScopes);
count--;
}

return authResponse;
}

3 changes: 2 additions & 1 deletion src/app/services/actions/query-action-creator-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ function createAuthenticatedRequest(
sampleHeaders[header.name] = header.value;
});
}
const updatedHeaders = { ...sampleHeaders, 'cache-control': 'no-cache', pragma: 'no-cache' }

const msalAuthOptions: AuthCodeMSALBrowserAuthenticationProviderOptions = {
account: authenticationWrapper.getAccount()!,
Expand All @@ -109,7 +110,7 @@ function createAuthenticatedRequest(
return GraphClient.getInstance()
.api(encodeHashCharacters(query))
.middlewareOptions([middlewareOptions])
.headers(sampleHeaders)
.headers(updatedHeaders)
.responseType(ResponseType.RAW);
}

Expand Down
2 changes: 1 addition & 1 deletion src/app/services/actions/query-action-creators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
} from './query-action-creator-util';
import { setQueryResponseStatus } from './query-status-action-creator';
import { addHistoryItem } from './request-history-action-creators';

//Boku9765
export function runQuery(query: IQuery): Function {
return (dispatch: Function, getState: Function) => {
const tokenPresent = !!getState()?.authToken?.token;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,11 @@ const mockState: IRootState = {
pending: false,
data: {},
error: null
},
unconsentingScopes: {
pending: false,
data: [],
error: null
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/app/services/graph-constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,5 @@ export const MOZILLA_CORS_DOCUMENTATION_LINK =
'https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS';
export const USER_ORGANIZATION_URL = `${GRAPH_URL}/v1.0/organization`;
export const NPS_FEEDBACK_URL = 'https://petrol.office.microsoft.com/v1/feedback';
// eslint-disable-next-line max-len
export const UNCONSENTING_PERMISSIONS_REQUIRED_SCOPES = 'DelegatedPermissionGrant.ReadWrite.All Directory.ReadWrite.All';
3 changes: 2 additions & 1 deletion src/app/services/reducers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { dimensions } from './dimensions-reducers';
import { graphExplorerMode } from './graph-explorer-mode-reducer';
import { policies } from './ocps-reducers';
import { permissionsPanelOpen } from './permissions-panel-reducer';
import { scopes } from './permissions-reducer';
import { scopes, unconsentingScopes } from './permissions-reducer';
import { profile } from './profile-reducer';
import { proxyUrl } from './proxy-url-reducer';
import { sampleQuery } from './query-input-reducers';
Expand Down Expand Up @@ -45,6 +45,7 @@ export default combineReducers({
sampleQuery,
samples,
scopes,
unconsentingScopes,
sidebarProperties,
snippets,
termsOfUse,
Expand Down
37 changes: 35 additions & 2 deletions src/app/services/reducers/permissions-reducer.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { IAction } from '../../../types/action';
import { IPermissionsResponse, IScopes } from '../../../types/permissions';
import { IPermissionsResponse, IScopes, IUnconsent } from '../../../types/permissions';
import {
FETCH_SCOPES_ERROR, FETCH_URL_SCOPES_PENDING, FETCH_FULL_SCOPES_SUCCESS,
FETCH_URL_SCOPES_SUCCESS, FETCH_FULL_SCOPES_PENDING
FETCH_URL_SCOPES_SUCCESS, FETCH_FULL_SCOPES_PENDING,
REVOKE_PERMISSION_PENDING, REVOKE_PERMISSION_SUCCESS, REVOKE_PERMISSION_ERROR
} from '../redux-constants';

const initialState: IScopes = {
Expand Down Expand Up @@ -55,3 +56,35 @@ export function scopes(state: IScopes = initialState, action: IAction): any {
return state;
}
}

const unconsentInitialState: IUnconsent = {
pending: false,
error: null,
data: []
}

export function unconsentingScopes(state: IUnconsent = unconsentInitialState, action: IAction): any {
switch (action.type) {
case REVOKE_PERMISSION_PENDING:
return {
pending: action.response,
error: null,
data: []
}
case REVOKE_PERMISSION_SUCCESS:
return {
pending: false,
error: null,
data: action.response
}

case REVOKE_PERMISSION_ERROR:
return {
pending: false,
error: action.response,
data: []
}
default:
return state;
}
}
MaryannGitonga marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 3 additions & 0 deletions src/app/services/redux-constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,6 @@ export const RESOURCEPATHS_ADD_SUCCESS = 'RESOURCEPATHS_ADD_SUCCESS';
export const RESOURCEPATHS_DELETE_SUCCESS = 'RESOURCEPATHS_DELETE_SUCCESS';
export const BULK_ADD_HISTORY_ITEMS_SUCCESS = 'BULK_ADD_HISTORY_ITEMS_SUCCESS';
export const SET_SNIPPET_TAB_SUCCESS = 'SET_SNIPPET_TAB_SUCCESS';
export const REVOKE_PERMISSION_PENDING = 'REVOKE_PERMISSION_PENDING';
export const REVOKE_PERMISSION_SUCCESS = 'REVOKE_PERMISSION_SUCCESS';
export const REVOKE_PERMISSION_ERROR = 'REVOKE_PERMISSION_ERROR';
1 change: 1 addition & 0 deletions src/app/views/authentication/Authentication.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const Authentication = (props: any) => {

try {
const authResponse = await authenticationWrapper.logIn();
console.log('Here is the auth response ', authResponse);
if (authResponse) {
setLoginInProgress(false);
dispatch(getAuthTokenSuccess(!!authResponse.accessToken));
Expand Down
Loading