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.

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

Jump to bottom

Refactor spo list roleassignment remove to use util instead of calling other command. Closes #5158 #5230

Closed
wants to merge 2 commits into from
Closed

Refactor spo list roleassignment remove to use util instead of calling other command. Closes #5158 #5230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1c77933
first commit
nicodecleyre Jun 30, 2023
e9918fe
Added logging
nicodecleyre Jul 17, 2023
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
88 changes: 42 additions & 46 deletions src/m365/spo/commands/list/list-roleassignment-remove.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,47 @@ import { pid } from '../../../../utils/pid.js';
import { session } from '../../../../utils/session.js';
import { sinonUtil } from '../../../../utils/sinonUtil.js';
import commands from '../../commands.js';
import spoGroupGetCommand from '../group/group-get.js';
import spoUserGetCommand from '../user/user-get.js';
import command from './list-roleassignment-remove.js';
import { spo } from '../../../../utils/spo.js';

describe(commands.LIST_ROLEASSIGNMENT_REMOVE, () => {
let log: any[];
let logger: Logger;
let commandInfo: CommandInfo;
let requests: any[];
let promptOptions: any;
const groupResponse = {
Id: 11,
IsHiddenInUI: false,
LoginName: "groupname",
Title: "groupname",
PrincipalType: 8,
AllowMembersEditMembership: false,
AllowRequestToJoinLeave: false,
AutoAcceptRequestToJoinLeave: false,
Description: "",
OnlyAllowMembersViewMembership: true,
OwnerTitle: "John Doe",
RequestToJoinLeaveEmailSetting: null
};

const userResponse = {
Id: 11,
IsHiddenInUI: false,
LoginName: 'i:0#.f|membership|[email protected]',
Title: 'John Doe',
PrincipalType: 1,
Email: '[email protected]',
Expiration: '',
IsEmailAuthenticationGuestUser: false,
IsShareByEmailGuestUser: false,
IsSiteAdmin: false,
UserId: {
NameId: '10032002473c5ae3',
NameIdIssuer: 'urn:federation:microsoftonline'
},
UserPrincipalName: '[email protected]'
};

before(() => {
sinon.stub(auth, 'restoreAuth').resolves();
Expand Down Expand Up @@ -54,7 +85,8 @@ describe(commands.LIST_ROLEASSIGNMENT_REMOVE, () => {
afterEach(() => {
sinonUtil.restore([
request.post,
Cli.executeCommandWithOutput,
spo.getGroupByName,
spo.getUserByEmail,
Cli.prompt
]);
});
Expand Down Expand Up @@ -171,15 +203,7 @@ describe(commands.LIST_ROLEASSIGNMENT_REMOVE, () => {
throw 'Invalid request';
});

sinon.stub(Cli, 'executeCommandWithOutput').callsFake(async (command): Promise<any> => {
if (command === spoUserGetCommand) {
return {
stdout: '{"Id": 11,"IsHiddenInUI": false,"LoginName": "i:0#.f|membership|[email protected]","Title": "Some Account","PrincipalType": 1,"Email": "[email protected]","Expiration": "","IsEmailAuthenticationGuestUser": false,"IsShareByEmailGuestUser": false,"IsSiteAdmin": true,"UserId": {"NameId": "1003200097d06dd6","NameIdIssuer": "urn:federation:microsoftonline"},"UserPrincipalName": "[email protected]"}'
};
}

throw new CommandError('Unknown case');
});
sinon.stub(spo, 'getUserByEmail').resolves(userResponse);

await command.action(logger, {
options: {
Expand All @@ -201,14 +225,8 @@ describe(commands.LIST_ROLEASSIGNMENT_REMOVE, () => {
throw 'Invalid request';
});

const error = 'no user found';
sinon.stub(Cli, 'executeCommandWithOutput').callsFake(async (command): Promise<any> => {
if (command === spoUserGetCommand) {
throw error;
}

throw new CommandError('Unknown case');
});
const error = 'User cannot be found';
sinon.stub(spo, 'getUserByEmail').rejects(new Error(error));

await assert.rejects(command.action(logger, {
options: {
Expand All @@ -230,15 +248,7 @@ describe(commands.LIST_ROLEASSIGNMENT_REMOVE, () => {
throw 'Invalid request';
});

sinon.stub(Cli, 'executeCommandWithOutput').callsFake(async (command): Promise<any> => {
if (command === spoGroupGetCommand) {
return {
stdout: '{"Id": 11,"IsHiddenInUI": false,"LoginName": "otherGroup","Title": "otherGroup","PrincipalType": 8,"AllowMembersEditMembership": false,"AllowRequestToJoinLeave": false,"AutoAcceptRequestToJoinLeave": false,"Description": "","OnlyAllowMembersViewMembership": true,"OwnerTitle": "Some Account","RequestToJoinLeaveEmailSetting": null}'
};
}

throw new CommandError('Unknown case');
});
sinon.stub(spo, 'getGroupByName').resolves(groupResponse);

await command.action(logger, {
options: {
Expand All @@ -260,14 +270,8 @@ describe(commands.LIST_ROLEASSIGNMENT_REMOVE, () => {
throw 'Invalid request';
});

const error = 'no group found';
sinon.stub(Cli, 'executeCommandWithOutput').callsFake(async (command): Promise<any> => {
if (command === spoGroupGetCommand) {
throw error;
}

throw new CommandError('Unknown case');
});
const error = 'Group cannot be found';
sinon.stub(spo, 'getGroupByName').rejects(new Error(error));

await assert.rejects(command.action(logger, {
options: {
Expand Down Expand Up @@ -339,15 +343,7 @@ describe(commands.LIST_ROLEASSIGNMENT_REMOVE, () => {
throw 'Invalid request';
});

sinon.stub(Cli, 'executeCommandWithOutput').callsFake(async (command): Promise<any> => {
if (command === spoGroupGetCommand) {
return {
stdout: '{"Id": 11,"IsHiddenInUI": false,"LoginName": "otherGroup","Title": "otherGroup","PrincipalType": 8,"AllowMembersEditMembership": false,"AllowRequestToJoinLeave": false,"AutoAcceptRequestToJoinLeave": false,"Description": "","OnlyAllowMembersViewMembership": true,"OwnerTitle": "Some Account","RequestToJoinLeaveEmailSetting": null}'
};
}

throw new CommandError('Unknown case');
});
sinon.stub(spo, 'getGroupByName').resolves(groupResponse);

sinonUtil.restore(Cli.prompt);
sinon.stub(Cli, 'prompt').resolves({ continue: true });
Expand Down
40 changes: 5 additions & 35 deletions src/m365/spo/commands/list/list-roleassignment-remove.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
import { Cli } from '../../../../cli/Cli.js';
import { Logger } from '../../../../cli/Logger.js';
import Command from '../../../../Command.js';
import GlobalOptions from '../../../../GlobalOptions.js';
import request, { CliRequestOptions } from '../../../../request.js';
import { formatting } from '../../../../utils/formatting.js';
import { spo } from '../../../../utils/spo.js';
import { urlUtil } from '../../../../utils/urlUtil.js';
import { validation } from '../../../../utils/validation.js';
import SpoCommand from '../../../base/SpoCommand.js';
import commands from '../../commands.js';
import spoGroupGetCommand, { Options as SpoGroupGetCommandOptions } from '../group/group-get.js';
import spoUserGetCommand, { Options as SpoUserGetCommandOptions } from '../user/user-get.js';

interface CommandArgs {
options: Options;
Expand Down Expand Up @@ -135,11 +133,13 @@ class SpoListRoleAssignmentRemoveCommand extends SpoCommand {
}

if (args.options.upn) {
args.options.principalId = await this.getUserPrincipalId(args.options);
const user = await spo.getUserByEmail(args.options.webUrl, args.options.upn, logger, this.verbose);
args.options.principalId = user.Id;
await this.removeRoleAssignment(requestUrl, logger, args.options);
}
else if (args.options.groupName) {
args.options.principalId = await this.getGroupPrincipalId(args.options);
const group = await spo.getGroupByName(args.options.webUrl, args.options.groupName, logger, this.verbose);
args.options.principalId = group.Id;
await this.removeRoleAssignment(requestUrl, logger, args.options);
}
else {
Expand Down Expand Up @@ -181,36 +181,6 @@ class SpoListRoleAssignmentRemoveCommand extends SpoCommand {

return request.post(requestOptions);
}

private async getGroupPrincipalId(options: Options): Promise<number> {
const groupGetCommandOptions: SpoGroupGetCommandOptions = {
webUrl: options.webUrl,
name: options.groupName,
output: 'json',
debug: this.debug,
verbose: this.verbose
};

const output = await Cli.executeCommandWithOutput(spoGroupGetCommand as Command, { options: { ...groupGetCommandOptions, _: [] } });
const getGroupOutput = JSON.parse(output.stdout);
return getGroupOutput.Id;
}

private async getUserPrincipalId(options: Options): Promise<number> {
const userGetCommandOptions: SpoUserGetCommandOptions = {
webUrl: options.webUrl,
email: options.upn,
id: undefined,
output: 'json',
debug: this.debug,
verbose: this.verbose
};

const output = await Cli.executeCommandWithOutput(spoUserGetCommand as Command, { options: { ...userGetCommandOptions, _: [] } });

const getUserOutput = JSON.parse(output.stdout);
return getUserOutput.Id;
}
}

export default new SpoListRoleAssignmentRemoveCommand();
26 changes: 13 additions & 13 deletions src/utils/spo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -640,15 +640,15 @@ export const spo = {
},

/**
* Retrieves the spo user by email.
* @param webUrl Web url
* @param email The email of the user
* @param logger the Logger object
* @param debug set if debug logging should be logged
*/
async getUserByEmail(webUrl: string, email: string, logger: Logger, debug?: boolean): Promise<any> {
if (debug) {
await logger.logToStderr(`Retrieving the spo user by email ${email}`);
* Retrieves the spo user by email.
* @param webUrl Web url
* @param email The email of the user
* @param logger the Logger object
* @param verbose Set for verbose logging
*/
async getUserByEmail(webUrl: string, email: string, logger?: Logger, verbose?: boolean): Promise<any> {
if (verbose && logger) {
logger.logToStderr(`Retrieving the spo user by email ${email}`);
}
const requestUrl = `${webUrl}/_api/web/siteusers/GetByEmail('${formatting.encodeQueryParameter(email)}')`;

Expand Down Expand Up @@ -728,11 +728,11 @@ export const spo = {
* @param webUrl Web url
* @param name The name of the group
* @param logger the Logger object
* @param debug set if debug logging should be logged
* @param verbose Set for verbose logging
*/
async getGroupByName(webUrl: string, name: string, logger: Logger, debug?: boolean): Promise<any> {
if (debug) {
await logger.logToStderr(`Retrieving the group by name ${name}`);
async getGroupByName(webUrl: string, name: string, logger?: Logger, verbose?: boolean): Promise<any> {
if (verbose && logger) {
logger.logToStderr(`Retrieving the group by name ${name}`);
}
const requestUrl = `${webUrl}/_api/web/sitegroups/GetByName('${formatting.encodeQueryParameter(name)}')`;

Expand Down