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

Enhances spo web roleassignment commands with Microsoft Entra groups, Closes #6193 #6463

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 15 additions & 3 deletions docs/docs/cmd/spo/web/web-roleassignment-add.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,19 @@ m365 spo web roleassignment add [options]
: URL of the site.

`--principalId [principalId]`
: SharePoint ID of principal it may be either user id or group id we want to add permissions to. Specify either `principalId`, `upn`, or `groupName` but not multiple.
: SharePoint ID of principal it may be either user id or group id we want to add permissions to. Specify either `principalId`, `upn`, `groupName`, `entraGroupId` or `entraGroupName` but not multiple.

`--upn [upn]`
: The upn/email of user to assign role to. Specify either `principalId`, `upn`, or `groupName` but not multiple.
: The upn/email of user to assign role to. Specify either `principalId`, `upn`, `groupName`, `entraGroupId` or `entraGroupName` but not multiple.

`--groupName [groupName]`
: The group name of the SharePoint group Specify either `principalId`, `upn`, or `groupName` but not multiple.
: The group name of the SharePoint group. Use this option exclusively for SharePoint Online groups. Specify either `principalId`, `upn`, `groupName`, `entraGroupId` or `entraGroupName` but not multiple.

`--entraGroupId [entraGroupId]`
: ID of the Microsoft Entra group to add. Specify either `principalId`, `upn`, `groupName`, `entraGroupId` or `entraGroupName` but not multiple.

`--entraGroupName [entraGroupName]`
: Display name of the Microsoft Entra group to add. Specify either `principalId`, `upn`, `groupName`, `entraGroupId` or `entraGroupName` but not multiple.

`--roleDefinitionId [roleDefinitionId]`
: ID of role definition. Specify either `roleDefinitionId` or `roleDefinitionName` but not both.
Expand Down Expand Up @@ -60,6 +66,12 @@ add role assignment to a site for principal id _11_ and role definition name _Fu
m365 spo web roleassignment add --webUrl "https://contoso.sharepoint.com/sites/project-x" --principalId 11 --roleDefinitionName "Full Control"
```

add role assignment using a Entra Group ID and a role definition

```sh
m365 spo web roleassignment add --webUrl "https://contoso.sharepoint.com/sites/project-x" --entraGroupId '27ae47f1-48f1-46f3-980b-d3c1470e398d' --roleDefinitionId 1073741829
Copy link
Member

Choose a reason for hiding this comment

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

we could use double quotes just to align with what we already have

Suggested change
m365 spo web roleassignment add --webUrl "https://contoso.sharepoint.com/sites/project-x" --entraGroupId '27ae47f1-48f1-46f3-980b-d3c1470e398d' --roleDefinitionId 1073741829
m365 spo web roleassignment add --webUrl "https://contoso.sharepoint.com/sites/project-x" --entraGroupId "27ae47f1-48f1-46f3-980b-d3c1470e398d" --roleDefinitionId 1073741829

```

## Response

The command won't return a response on success.
18 changes: 15 additions & 3 deletions docs/docs/cmd/spo/web/web-roleassignment-remove.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,19 @@ m365 spo web roleassignment remove [options]
: URL of the site.

`--principalId [principalId]`
: SharePoint ID of principal it may be either user id or group id we want to add permissions to. Specify either `principalId`, `upn`, or `groupName` but not multiple.
: SharePoint ID of principal it may be either user id or group id we want to add permissions to. Specify either `principalId`, `upn`, `groupName`, `entraGroupId` or `entraGroupName` but not multiple.

`--upn [upn]`
: The upn/email of user to assign role to. Specify either `principalId`, `upn`, or `groupName` but not multiple.
: The upn/email of user to assign role to. Specify either `principalId`, `upn`, `groupName`, `entraGroupId` or `entraGroupName` but not multiple.

`--groupName [groupName]`
: The group name of the SharePoint group Specify either `principalId`, `upn`, or `groupName` but not multiple.
: The group name of the SharePoint group. Use this option exclusively for SharePoint Online groups. Specify either `principalId`, `upn`, `groupName`, `entraGroupId` or `entraGroupName` but not multiple.

`--entraGroupId [entraGroupId]`
: ID of the Microsoft Entra group to remove. Specify either `principalId`, `upn`, `groupName`, `entraGroupId` or `entraGroupName` but not multiple.

`--entraGroupName [entraGroupName]`
: Display name of the Microsoft Entra group to remove. Specify either `principalId`, `upn`, `groupName`, `entraGroupId` or `entraGroupName` but not multiple.

`-f, --force`
: Don't prompt for confirming removing the roleassignment.
Expand Down Expand Up @@ -57,6 +63,12 @@ Remove roleassignment from web based on principal Id without prompting for confi
m365 spo web roleassignment remove --webUrl "https://contoso.sharepoint.com/sites/contoso-sales" --principalId 2 --force
```

Remove roleassignment from web based on Entra Group Id
Copy link
Member

Choose a reason for hiding this comment

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

lets add a remark that this is force delete as it someone just do a copy/past and execute than this may have some not pleasent implications

Suggested change
Remove roleassignment from web based on Entra Group Id
Remove roleassignment from web based on Entra Group Id and don't prompt for confirmation


```sh
m365 spo web roleassignment remove --webUrl "https://contoso.sharepoint.com/sites/contoso-sales" --entraGroupId '27ae47f1-48f1-46f3-980b-d3c1470e398d' --force
Copy link
Member

Choose a reason for hiding this comment

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

same here

Suggested change
m365 spo web roleassignment remove --webUrl "https://contoso.sharepoint.com/sites/contoso-sales" --entraGroupId '27ae47f1-48f1-46f3-980b-d3c1470e398d' --force
m365 spo web roleassignment remove --webUrl "https://contoso.sharepoint.com/sites/contoso-sales" --entraGroupId "27ae47f1-48f1-46f3-980b-d3c1470e398d" --force

```

## Response

The command won't return a response on success.
121 changes: 120 additions & 1 deletion src/m365/spo/commands/web/web-roleassignment-add.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,65 @@ import spoGroupGetCommand from '../group/group-get.js';
import spoRoleDefinitionListCommand from '../roledefinition/roledefinition-list.js';
import spoUserGetCommand from '../user/user-get.js';
import command from './web-roleassignment-add.js';
import { entraGroup } from '../../../../utils/entraGroup.js';
import { spo } from '../../../../utils/spo.js';

const graphGroup = {
id: '27ae47f1-48f1-46f3-980b-d3c1470e398d',
deletedDateTime: null,
classification: null,
createdDateTime: '2024-03-22T20:18:37Z',
creationOptions: [],
description: null,
displayName: 'Marketing',
expirationDateTime: null,
groupTypes: [
'Unified'
],
isAssignableToRole: null,
mail: '[email protected]',
mailEnabled: true,
mailNickname: 'Marketing',
membershipRule: null,
membershipRuleProcessingState: null,
onPremisesDomainName: null,
onPremisesLastSyncDateTime: null,
onPremisesNetBiosName: null,
onPremisesSamAccountName: null,
onPremisesSecurityIdentifier: null,
onPremisesSyncEnabled: null,
preferredDataLocation: null,
preferredLanguage: null,
proxyAddresses: [
'SPO:SPO_de7704ba-415d-4dd0-9bbd-fa565007a87e@SPO_18c58817-3bc9-489d-ac63-f7264fb357e5',
'SMTP:[email protected]'
],
renewedDateTime: '2024-03-22T20:18:37Z',
resourceBehaviorOptions: [],
resourceProvisioningOptions: [],
securityEnabled: true,
securityIdentifier: 'S-1-12-1-665733105-1190349041-3268610968-2369326662',
theme: null,
uniqueName: null,
visibility: 'Private',
onPremisesProvisioningErrors: [],
serviceProvisioningErrors: []
};

const entraGroupResponse = {
Id: 11,
IsHiddenInUI: false,
LoginName: 'c:0o.c|federateddirectoryclaimprovider|27ae47f1-48f1-46f3-980b-d3c1470e398d',
Title: 'Marketing members',
PrincipalType: 1,
Email: '',
Expiration: '',
IsEmailAuthenticationGuestUser: false,
IsShareByEmailGuestUser: false,
IsSiteAdmin: false,
UserId: null,
UserPrincipalName: null
};

describe(commands.WEB_ROLEASSIGNMENT_ADD, () => {
let log: any[];
Expand Down Expand Up @@ -48,7 +107,10 @@ describe(commands.WEB_ROLEASSIGNMENT_ADD, () => {
afterEach(() => {
sinonUtil.restore([
request.post,
cli.executeCommandWithOutput
cli.executeCommandWithOutput,
entraGroup.getGroupById,
entraGroup.getGroupByDisplayName,
spo.ensureEntraGroup
]);
});

Expand Down Expand Up @@ -95,6 +157,17 @@ describe(commands.WEB_ROLEASSIGNMENT_ADD, () => {
assert.strictEqual(actual, true);
});


it('fails validation if the entaGroupId is not a valid guid', async () => {
Comment on lines 158 to +161
Copy link
Member

Choose a reason for hiding this comment

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

Lets keep no more than single blank line in code

Suggested change
});
it('fails validation if the entaGroupId is not a valid guid', async () => {
});
it('fails validation if the entaGroupId is not a valid guid', async () => {

const actual = await command.validate({ options: { webUrl: 'https://contoso.sharepoint.com', entraGroupId: 'invalid', roleDefinitionId: '1073741827' } }, commandInfo);
assert.notStrictEqual(actual, true);
});

it('passes validation if the entaGroupId is a valid guid', async () => {
const actual = await command.validate({ options: { webUrl: 'https://contoso.sharepoint.com', entraGroupId: '27ae47f1-48f1-46f3-980b-d3c1470e398d', roleDefinitionId: 1073741827 } }, commandInfo);
assert.strictEqual(actual, true);
});

it('add role assignment on web by role definition id', async () => {
sinon.stub(request, 'post').callsFake(async (opts) => {
if ((opts.url as string).indexOf('_api/web/roleassignments/addroleassignment(principalid=\'11\',roledefid=\'1073741827\')') > -1) {
Expand Down Expand Up @@ -257,6 +330,52 @@ describe(commands.WEB_ROLEASSIGNMENT_ADD, () => {
});
});

it('adds role assignment on web by role definition id and Entra group ID', async () => {
sinon.stub(entraGroup, 'getGroupById').withArgs(graphGroup.id).resolves(graphGroup);
sinon.stub(spo, 'ensureEntraGroup').withArgs('https://contoso.sharepoint.com', graphGroup).resolves(entraGroupResponse);

sinon.stub(request, 'post').callsFake(async (opts) => {
if ((opts.url as string).indexOf('_api/web/roleassignments/addroleassignment(principalid=\'11\',roledefid=\'1073741827\')') > -1) {
return;
}

throw 'Invalid request';
});

await command.action(logger, {
options: {
debug: true,
webUrl: 'https://contoso.sharepoint.com',
entraGroupId: '27ae47f1-48f1-46f3-980b-d3c1470e398d',
principalId: 11,
roleDefinitionId: 1073741827
}
});
});

it('adds role assignment on web by role definition id and Entra group name', async () => {
sinon.stub(entraGroup, 'getGroupByDisplayName').withArgs(graphGroup.displayName).resolves(graphGroup);
sinon.stub(spo, 'ensureEntraGroup').withArgs('https://contoso.sharepoint.com', graphGroup).resolves(entraGroupResponse);

sinon.stub(request, 'post').callsFake(async (opts) => {
if ((opts.url as string).indexOf('_api/web/roleassignments/addroleassignment(principalid=\'11\',roledefid=\'1073741827\')') > -1) {
return;
}

throw 'Invalid request';
});

await command.action(logger, {
options: {
debug: true,
webUrl: 'https://contoso.sharepoint.com',
entraGroupName: 'Marketing',
principalId: 11,
roleDefinitionId: 1073741827
}
});
});

it('correctly handles error when role definition does not exist', async () => {
sinon.stub(request, 'post').callsFake(async (opts) => {
if ((opts.url as string).indexOf('/_api/web/roleassignments/addroleassignment(principalid=\'11\',roledefid=\'1073741827\')') > -1) {
Expand Down
41 changes: 36 additions & 5 deletions src/m365/spo/commands/web/web-roleassignment-add.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Group } from '@microsoft/microsoft-graph-types';
Copy link
Member

Choose a reason for hiding this comment

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

When we apply my previous comment we may get rid of this line

Suggested change
import { Group } from '@microsoft/microsoft-graph-types';

import { cli, CommandOutput } from '../../../../cli/cli.js';
import { Logger } from '../../../../cli/Logger.js';
import Command from '../../../../Command.js';
Expand All @@ -10,6 +11,8 @@ import spoGroupGetCommand, { Options as SpoGroupGetCommandOptions } from '../gro
import spoRoleDefinitionListCommand, { Options as SpoRoleDefinitionListCommandOptions } from '../roledefinition/roledefinition-list.js';
import { RoleDefinition } from '../roledefinition/RoleDefinition.js';
import spoUserGetCommand, { Options as SpoUserGetCommandOptions } from '../user/user-get.js';
import { entraGroup } from '../../../../utils/entraGroup.js';
import { spo } from '../../../../utils/spo.js';

interface CommandArgs {
options: Options;
Expand All @@ -20,6 +23,8 @@ interface Options extends GlobalOptions {
principalId?: number;
upn?: string;
groupName?: string;
entraGroupId?: string;
entraGroupName?: string;
roleDefinitionId?: number;
roleDefinitionName?: string;
}
Expand Down Expand Up @@ -48,6 +53,8 @@ class SpoWebRoleAssignmentAddCommand extends SpoCommand {
principalId: typeof args.options.principalId !== 'undefined',
upn: typeof args.options.upn !== 'undefined',
groupName: typeof args.options.groupName !== 'undefined',
entraGroupId: typeof args.options.entraGroupId !== 'undefined',
entraGroupName: typeof args.options.entraGroupName !== 'undefined',
roleDefinitionId: typeof args.options.roleDefinitionId !== 'undefined',
roleDefinitionName: typeof args.options.roleDefinitionName !== 'undefined'
});
Expand All @@ -68,6 +75,12 @@ class SpoWebRoleAssignmentAddCommand extends SpoCommand {
{
option: '--groupName [groupName]'
},
{
option: '--entraGroupId [entraGroupId]'
},
{
option: '--entraGroupName [entraGroupName]'
},
{
option: '--roleDefinitionId [roleDefinitionId]'
},
Expand All @@ -93,14 +106,18 @@ class SpoWebRoleAssignmentAddCommand extends SpoCommand {
return `Specified roleDefinitionId ${args.options.roleDefinitionId} is not a number`;
}

if (args.options.entraGroupId && !validation.isValidGuid(args.options.entraGroupId)) {
return `'${args.options.entraGroupId}' is not a valid GUID for option entraGroupId.`;
}

return true;
}
);
}

#initOptionSets(): void {
this.optionSets.push(
{ options: ['principalId', 'upn', 'groupName'] },
{ options: ['principalId', 'upn', 'groupName', 'entraGroupId', 'entraGroupName'] },
{ options: ['roleDefinitionId', 'roleDefinitionName'] }
);
}
Expand All @@ -115,15 +132,29 @@ class SpoWebRoleAssignmentAddCommand extends SpoCommand {

if (args.options.upn) {
args.options.principalId = await this.getUserPrincipalId(args.options);
await this.addRoleAssignment(logger, args.options);
}
else if (args.options.groupName) {
args.options.principalId = await this.getGroupPrincipalId(args.options);
await this.addRoleAssignment(logger, args.options);
}
else {
await this.addRoleAssignment(logger, args.options);
else if (args.options.entraGroupId || args.options.entraGroupName) {
if (this.verbose) {
await logger.logToStderr('Retrieving group information...');
}

let group: Group;
if (args.options.entraGroupId) {
group = await entraGroup.getGroupById(args.options.entraGroupId);
}
else {
group = await entraGroup.getGroupByDisplayName(args.options.entraGroupName!);
}
Comment on lines +144 to +150
Copy link
Member

Choose a reason for hiding this comment

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

similar like in the listItem PR we may just simplify this and remove the type. The type is returned by the method so it is applied allready no need to mention it I guess.

Suggested change
let group: Group;
if (args.options.entraGroupId) {
group = await entraGroup.getGroupById(args.options.entraGroupId);
}
else {
group = await entraGroup.getGroupByDisplayName(args.options.entraGroupName!);
}
const group = args.options.entraGroupId ? await entraGroup.getGroupById(args.options.entraGroupId) : await entraGroup.getGroupByDisplayName(args.options.entraGroupName!);


const siteUser = await spo.ensureEntraGroup(args.options.webUrl, group);
args.options.principalId = siteUser.Id;
}

await this.addRoleAssignment(logger, args.options);

}
catch (err: any) {
this.handleRejectedODataJsonPromise(err);
Expand Down
Loading