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

Adds support for interactive mode for disambiguation prompts. Closes #5053 #5192

Closed
Closed
Show file tree
Hide file tree
Changes from 3 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
5 changes: 2 additions & 3 deletions src/cli/Cli.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ class MockCommandWithHandleMultipleResultsFound extends AnonymousCommand {
return 'Mock command with interactive prompt';
}
public async commandAction(): Promise<void> {
await Cli.handleMultipleResultsFound(`Multiple values with name found. Pick one`, `Multiple values with name found.`, { '1': { 'id': '1', 'title': 'Option1' }, '2': { 'id': '2', 'title': 'Option2' } });
await Cli.handleMultipleResultsFound(`Multiple values with name found.`, { '1': { 'id': '1', 'title': 'Option1' }, '2': { 'id': '2', 'title': 'Option2' } });
}
}

Expand Down Expand Up @@ -1251,9 +1251,8 @@ describe('Cli', () => {
});

it('throws error when interactive mode not set', async () => {
const error = `Multiple values with name found.`;
sinon.stub(Cli.getInstance(), 'getSettingWithDefaultValue').callsFake((() => false));
await assert.rejects((Cli.handleMultipleResultsFound(`Multiple values with name found. Pick one`, error, { '1': { 'id': '1', 'title': 'Option1' }, '2': { 'id': '2', 'title': 'Option2' } })
await assert.rejects((Cli.handleMultipleResultsFound(`Multiple values with name found.`, { '1': { 'id': '1', 'title': 'Option1' }, '2': { 'id': '2', 'title': 'Option2' } })
), 'error');
});

Expand Down
6 changes: 3 additions & 3 deletions src/cli/Cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -988,17 +988,17 @@ export class Cli {
return response;
}

public static async handleMultipleResultsFound(promptMessage: string, errorMessage: string, values: { [key: string]: object }): Promise<object | CommandError> {
public static async handleMultipleResultsFound<T>(message: string, values: { [key: string]: T }): Promise<T> {
const prompt: boolean = Cli.getInstance().getSettingWithDefaultValue<boolean>(settingsNames.prompt, false);
if (!prompt) {
throw errorMessage;
throw new Error(`${message} Found: ${Object.keys(values).join(', ')}.`);
}

const response = await Cli.prompt<{ select: string }>({
type: 'list',
name: 'select',
default: 0,
message: promptMessage,
message: `${message} Please choose one:`,
choices: Object.keys(values)
});

Expand Down
46 changes: 44 additions & 2 deletions src/m365/aad/commands/app/app-get.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ describe(commands.APP_GET, () => {
fs.existsSync,
fs.readFileSync,
fs.writeFileSync,
cli.getSettingWithDefaultValue
cli.getSettingWithDefaultValue,
Cli.handleMultipleResultsFound
]);
});

Expand Down Expand Up @@ -121,7 +122,48 @@ describe(commands.APP_GET, () => {
options: {
name: 'My app'
}
}), new CommandError(`Multiple Azure AD application registration with name My app found. Please disambiguate (app object IDs): 9b1b1e42-794b-4c71-93ac-5ed92488b67f, 9b1b1e42-794b-4c71-93ac-5ed92488b67g`));
}), new CommandError(`Multiple Azure AD application registration with name 'My app' found. Found: 9b1b1e42-794b-4c71-93ac-5ed92488b67f, 9b1b1e42-794b-4c71-93ac-5ed92488b67g.`));
});

it('handles selecting single result when multiple apps with the specified name found and cli is set to prompt', async () => {
sinon.stub(request, 'get').callsFake(async opts => {
if (opts.url === `https://graph.microsoft.com/v1.0/myorganization/applications?$filter=displayName eq 'My%20App'&$select=id`) {
return {
value: [
{ id: '9b1b1e42-794b-4c71-93ac-5ed92488b67f' },
{ id: '9b1b1e42-794b-4c71-93ac-5ed92488b67g' }
]
};
}

if (opts.url === 'https://graph.microsoft.com/v1.0/myorganization/applications/9b1b1e42-794b-4c71-93ac-5ed92488b67f') {
return {
"id": "340a4aa3-1af6-43ac-87d8-189819003952",
"appId": "9b1b1e42-794b-4c71-93ac-5ed92488b67f",
"createdDateTime": "2019-10-29T17:46:55Z",
"displayName": "My App",
"description": null
};
}

throw `Invalid request ${JSON.stringify(opts)}`;
});

sinon.stub(Cli, 'handleMultipleResultsFound').resolves({ id: '9b1b1e42-794b-4c71-93ac-5ed92488b67f' });

await command.action(logger, {
options: {
name: 'My App'
}
});
const call: sinon.SinonSpyCall = loggerLogSpy.lastCall;
assert.deepEqual(call.args[0], {
"id": "340a4aa3-1af6-43ac-87d8-189819003952",
"appId": "9b1b1e42-794b-4c71-93ac-5ed92488b67f",
"createdDateTime": "2019-10-29T17:46:55Z",
"displayName": "My App",
"description": null
});
});

it('handles error when retrieving information about app through appId failed', async () => {
Expand Down
5 changes: 4 additions & 1 deletion src/m365/aad/commands/app/app-get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { validation } from '../../../../utils/validation.js';
import GraphCommand from '../../../base/GraphCommand.js';
import { M365RcJson } from '../../../base/M365RcJson.js';
import commands from '../../commands.js';
import { Cli } from '../../../../cli/Cli.js';

interface CommandArgs {
options: Options;
Expand Down Expand Up @@ -119,7 +120,9 @@ class AadAppGetCommand extends GraphCommand {
throw `No Azure AD application registration with ${applicationIdentifier} found`;
}

throw `Multiple Azure AD application registration with name ${name} found. Please disambiguate (app object IDs): ${res.value.map(a => a.id).join(', ')}`;
const resultAsKeyValuePair = formatting.convertArrayToHashTable('id', res.value);
const result = await Cli.handleMultipleResultsFound<{ id: string }>(`Multiple Azure AD application registration with name '${name}' found.`, resultAsKeyValuePair);
return result.id;
}

private async getAppInfo(appObjectId: string): Promise<Application> {
Expand Down
34 changes: 31 additions & 3 deletions src/m365/aad/commands/app/app-remove.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ describe(commands.APP_REMOVE, () => {
request.get,
request.delete,
Cli.prompt,
cli.getSettingWithDefaultValue
cli.getSettingWithDefaultValue,
Cli.handleMultipleResultsFound
]);
});

Expand Down Expand Up @@ -253,7 +254,7 @@ describe(commands.APP_REMOVE, () => {
};
}

throw "Multiple Azure AD application registration with name myapp found. Please choose one of the object IDs: d75be2e1-0204-4f95-857d-51a37cf40be8, 340a4aa3-1af6-43ac-87d8-189819003952";
throw "Multiple Azure AD application registration with name 'myapp' found.";
});

await assert.rejects(command.action(logger, {
Expand All @@ -262,6 +263,33 @@ describe(commands.APP_REMOVE, () => {
name: 'myapp',
force: true
}
}), new CommandError("Multiple Azure AD application registration with name myapp found. Please choose one of the object IDs: d75be2e1-0204-4f95-857d-51a37cf40be8, 340a4aa3-1af6-43ac-87d8-189819003952"));
}), new CommandError("Multiple Azure AD application registration with name 'myapp' found. Found: d75be2e1-0204-4f95-857d-51a37cf40be8, 340a4aa3-1af6-43ac-87d8-189819003952."));
});

it('handles selecting single result when multiple apps with the specified name found and cli is set to prompt', async () => {
sinonUtil.restore(request.get);
sinon.stub(request, 'get').callsFake(async (opts) => {
if (opts.url === `https://graph.microsoft.com/v1.0/myorganization/applications?$filter=displayName eq 'myapp'&$select=id`) {
return {
"@odata.context": "https://graph.microsoft.com/v1.0/$metadata#applications",
"value": [
{ "id": "d75be2e1-0204-4f95-857d-51a37cf40be8" },
{ "id": "340a4aa3-1af6-43ac-87d8-189819003952" }
]
};
}

throw "Multiple Azure AD application registration with name 'myapp' found.";
});

sinon.stub(Cli, 'handleMultipleResultsFound').resolves({ id: 'd75be2e1-0204-4f95-857d-51a37cf40be8' });

await command.action(logger, {
options: {
name: 'myapp',
force: true
}
});
assert(deleteRequestStub.called);
});
});
4 changes: 3 additions & 1 deletion src/m365/aad/commands/app/app-remove.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,9 @@ class AadAppRemoveCommand extends GraphCommand {
throw `No Azure AD application registration with ${applicationIdentifier} found`;
}

throw `Multiple Azure AD application registration with name ${name} found. Please choose one of the object IDs: ${res.value.map(a => a.id).join(', ')}`;
const resultAsKeyValuePair = formatting.convertArrayToHashTable('id', res.value);
const result = await Cli.handleMultipleResultsFound<{ id: string }>(`Multiple Azure AD application registration with name '${name}' found.`, resultAsKeyValuePair);
return result.id;
}
}

Expand Down
84 changes: 82 additions & 2 deletions src/m365/aad/commands/app/app-role-add.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ describe(commands.APP_ROLE_ADD, () => {
sinonUtil.restore([
request.get,
request.patch,
cli.getSettingWithDefaultValue
cli.getSettingWithDefaultValue,
Cli.handleMultipleResultsFound
]);
});

Expand Down Expand Up @@ -342,7 +343,86 @@ describe(commands.APP_ROLE_ADD, () => {
allowedMembers: 'usersGroups',
claim: 'Custom.Role'
}
}), new CommandError(`Multiple Azure AD application registration with name My app found. Please disambiguate (app object IDs): 9b1b1e42-794b-4c71-93ac-5ed92488b67f, 9b1b1e42-794b-4c71-93ac-5ed92488b67g`));
}), new CommandError(`Multiple Azure AD application registration with name 'My app' found. Found: 9b1b1e42-794b-4c71-93ac-5ed92488b67f, 9b1b1e42-794b-4c71-93ac-5ed92488b67g.`));
});

it('handles selecting single result when multiple apps with the specified name found and cli is set to prompt', async () => {
let updateRequestIssued = false;

sinon.stub(request, 'get').callsFake(async opts => {
if (opts.url === `https://graph.microsoft.com/v1.0/myorganization/applications?$filter=displayName eq 'My%20app'&$select=id`) {
return {
value: [
{ id: '9b1b1e42-794b-4c71-93ac-5ed92488b67f' },
{ id: '9b1b1e42-794b-4c71-93ac-5ed92488b67g' }
]
};
}

if (opts.url === 'https://graph.microsoft.com/v1.0/myorganization/applications/5b31c38c-2584-42f0-aa47-657fb3a84230?$select=id,appRoles') {
return {
id: '5b31c38c-2584-42f0-aa47-657fb3a84230',
appRoles: [{
"allowedMemberTypes": [
"User"
],
"description": "Managers",
"displayName": "Managers",
"id": "c4352a0a-494f-46f9-b843-479855c173a7",
"isEnabled": true,
"lang": null,
"origin": "Application",
"value": "managers"
}]
};
}

throw `Invalid request ${JSON.stringify(opts)}`;
});

sinon.stub(Cli, 'handleMultipleResultsFound').resolves({ id: '5b31c38c-2584-42f0-aa47-657fb3a84230' });

sinon.stub(request, 'patch').callsFake(async opts => {
if (opts.url === 'https://graph.microsoft.com/v1.0/myorganization/applications/5b31c38c-2584-42f0-aa47-657fb3a84230' &&
opts.data &&
opts.data.appRoles.length === 2) {
const appRole = opts.data.appRoles[1];
if (JSON.stringify({
"allowedMemberTypes": [
"User"
],
"description": "Managers",
"displayName": "Managers",
"id": "c4352a0a-494f-46f9-b843-479855c173a7",
"isEnabled": true,
"lang": null,
"origin": "Application",
"value": "managers"
}) === JSON.stringify(opts.data.appRoles[0]) &&
appRole.displayName === 'Role' &&
appRole.description === 'Custom role' &&
appRole.value === 'Custom.Role' &&
JSON.stringify(appRole.allowedMemberTypes) === JSON.stringify(['Application'])) {

updateRequestIssued = true;
return;
}
}

throw `Invalid request ${JSON.stringify(opts)}`;
});

await command.action(logger, {
Adam-it marked this conversation as resolved.
Show resolved Hide resolved
options: {
appName: 'My app',
name: 'Role',
description: 'Custom role',
allowedMembers: 'applications',
claim: 'Custom.Role'
}
});

assert(updateRequestIssued);
});

it('handles error when retrieving information about app through appId failed', async () => {
Expand Down
6 changes: 4 additions & 2 deletions src/m365/aad/commands/app/app-role-add.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import request, { CliRequestOptions } from '../../../../request.js';
import { formatting } from '../../../../utils/formatting.js';
import GraphCommand from '../../../base/GraphCommand.js';
import commands from '../../commands.js';
import { Cli } from '../../../../cli/Cli.js';

interface CommandArgs {
options: Options;
Expand Down Expand Up @@ -201,8 +202,9 @@ class AadAppRoleAddCommand extends GraphCommand {
throw `No Azure AD application registration with ${applicationIdentifier} found`;
}

throw `Multiple Azure AD application registration with name ${appName} found. Please disambiguate (app object IDs): ${res.value.map(a => a.id).join(', ')}`;

const resultAsKeyValuePair = formatting.convertArrayToHashTable('id', res.value);
const result = await Cli.handleMultipleResultsFound<{ id: string }>(`Multiple Azure AD application registration with name '${appName}' found.`, resultAsKeyValuePair);
return result.id;
}
}

Expand Down
77 changes: 75 additions & 2 deletions src/m365/aad/commands/app/app-role-list.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ describe(commands.APP_ROLE_LIST, () => {
afterEach(() => {
sinonUtil.restore([
request.get,
cli.getSettingWithDefaultValue
cli.getSettingWithDefaultValue,
Cli.handleMultipleResultsFound
]);
});

Expand Down Expand Up @@ -360,7 +361,79 @@ describe(commands.APP_ROLE_LIST, () => {
options: {
appName: 'My app'
}
}), new CommandError(`Multiple Azure AD application registration with name My app found. Please disambiguate (app object IDs): 9b1b1e42-794b-4c71-93ac-5ed92488b67f, 9b1b1e42-794b-4c71-93ac-5ed92488b67g`));
}), new CommandError(`Multiple Azure AD application registration with name 'My app' found. Found: 9b1b1e42-794b-4c71-93ac-5ed92488b67f, 9b1b1e42-794b-4c71-93ac-5ed92488b67g.`));
});

it('handles selecting single result when multiple apps with the specified name found and cli is set to prompt', async () => {
sinon.stub(request, 'get').callsFake(async opts => {
if (opts.url === `https://graph.microsoft.com/v1.0/myorganization/applications?$filter=displayName eq 'My%20app'&$select=id`) {
return {
value: [
{ id: '9b1b1e42-794b-4c71-93ac-5ed92488b67f' },
{ id: '9b1b1e42-794b-4c71-93ac-5ed92488b67g' }
]
};
}

if (opts.url === `https://graph.microsoft.com/v1.0/myorganization/applications/5b31c38c-2584-42f0-aa47-657fb3a84230/appRoles`) {
return {
value: [
{
"allowedMemberTypes": [
"User"
],
"description": "Readers",
"displayName": "Readers",
"id": "ca12d0da-cd83-4dc9-8e4c-b6a529bebbb4",
"isEnabled": true,
"origin": "Application",
"value": "readers"
},
{
"allowedMemberTypes": [
"User"
],
"description": "Writers",
"displayName": "Writers",
"id": "85c03d41-b438-48ea-bccd-8389c0e327bc",
"isEnabled": true,
"origin": "Application",
"value": "writers"
}
]
};
}

throw `Invalid request ${JSON.stringify(opts)}`;
});

sinon.stub(Cli, 'handleMultipleResultsFound').resolves({ id: '5b31c38c-2584-42f0-aa47-657fb3a84230' });

await command.action(logger, { options: { appName: 'My app' } });
assert(loggerLogSpy.calledWith([
{
"allowedMemberTypes": [
"User"
],
"description": "Readers",
"displayName": "Readers",
"id": "ca12d0da-cd83-4dc9-8e4c-b6a529bebbb4",
"isEnabled": true,
"origin": "Application",
"value": "readers"
},
{
"allowedMemberTypes": [
"User"
],
"description": "Writers",
"displayName": "Writers",
"id": "85c03d41-b438-48ea-bccd-8389c0e327bc",
"isEnabled": true,
"origin": "Application",
"value": "writers"
}
]));
});

it('handles error when retrieving information about app through appId failed', async () => {
Expand Down
Loading