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 application permission check on certain command groups / specific commands #5862

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
53 changes: 53 additions & 0 deletions src/m365/base/DelegatedGraphCommand.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import assert from 'assert';
import sinon from 'sinon';
import auth from '../../Auth.js';
import { telemetry } from '../../telemetry.js';
import DelegatedGraphCommand from './DelegatedGraphCommand.js';
import { accessToken } from '../../utils/accessToken.js';
import { CommandError } from '../../Command.js';

class MockCommand extends DelegatedGraphCommand {
public get name(): string {
return 'mock';
}

public get description(): string {
return 'Mock command';
}

public async commandAction(): Promise<void> {
}

public commandHelp(): void {
}
}

describe('ToDoCommand', () => {
MathijsVerbeeck marked this conversation as resolved.
Show resolved Hide resolved
const cmd = new MockCommand();

before(() => {
sinon.stub(telemetry, 'trackEvent').returns();
auth.connection.active = true;
auth.connection.accessTokens[auth.defaultResource] = {
expiresOn: 'abc',
accessToken: 'abc'
};
});

after(() => {
sinon.restore();
auth.connection.active = false;
auth.connection.accessTokens = {};
});

it(`doesn't throw error when not connected`, () => {
auth.connection.active = false;
(cmd as any).initAction({ options: {} }, {});
auth.connection.active = true;
});

it('throws error when using application-only permissions', () => {
sinon.stub(accessToken, 'isAppOnlyAccessToken').returns(true);
assert.throws(() => (cmd as any).initAction({ options: {} }, {}), new CommandError('This command does not support application-only permissions.'));
});
});
18 changes: 18 additions & 0 deletions src/m365/base/DelegatedGraphCommand.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import auth from '../../Auth.js';
import { CommandArgs } from '../../Command.js';
import { Logger } from '../../cli/Logger.js';
import { accessToken } from '../../utils/accessToken.js';
import GraphCommand from './GraphCommand.js';

export default abstract class DelegatedGraphCommand extends GraphCommand {
MathijsVerbeeck marked this conversation as resolved.
Show resolved Hide resolved
protected initAction(args: CommandArgs, logger: Logger): void {
super.initAction(args, logger);

if (!auth.connection.active) {
// we fail no login in the base command command class
return;
}

accessToken.assertDelegatedAccessToken();
}
}
23 changes: 18 additions & 5 deletions src/m365/base/PowerAppsCommand.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import auth, { CloudType } from '../../Auth.js';
import { CommandError } from '../../Command.js';
import { telemetry } from '../../telemetry.js';
import PowerAppsCommand from './PowerAppsCommand.js';
import { accessToken } from '../../utils/accessToken.js';
import { sinonUtil } from '../../utils/sinonUtil.js';

class MockCommand extends PowerAppsCommand {
public get name(): string {
Expand All @@ -27,10 +29,18 @@ describe('PowerAppsCommand', () => {

before(() => {
sinon.stub(telemetry, 'trackEvent').returns();
sinon.stub(accessToken, 'isAppOnlyAccessToken').returns(false);
auth.connection.active = true;
auth.connection.accessTokens[auth.defaultResource] = {
expiresOn: 'abc',
accessToken: 'abc'
};
});

after(() => {
sinon.restore();
auth.connection.active = false;
auth.connection.accessTokens = {};
MathijsVerbeeck marked this conversation as resolved.
Show resolved Hide resolved
});

it('returns correct resource', () => {
Expand All @@ -41,35 +51,38 @@ describe('PowerAppsCommand', () => {
it(`doesn't throw error when not connected`, () => {
auth.connection.active = false;
MathijsVerbeeck marked this conversation as resolved.
Show resolved Hide resolved
(cmd as any).initAction({ options: {} }, {});
auth.connection.active = true;
MathijsVerbeeck marked this conversation as resolved.
Show resolved Hide resolved
});

it('throws error when connected to USGov cloud', () => {
auth.connection.active = true;
auth.connection.cloudType = CloudType.USGov;
assert.throws(() => (cmd as any).initAction({ options: {} }, {}), cloudError);
});

it('throws error when connected to USGovHigh cloud', () => {
auth.connection.active = true;
auth.connection.cloudType = CloudType.USGovHigh;
assert.throws(() => (cmd as any).initAction({ options: {} }, {}), cloudError);
});

it('throws error when connected to USGovDoD cloud', () => {
auth.connection.active = true;
auth.connection.cloudType = CloudType.USGovDoD;
assert.throws(() => (cmd as any).initAction({ options: {} }, {}), cloudError);
});

it('throws error when connected to China cloud', () => {
auth.connection.active = true;
auth.connection.cloudType = CloudType.China;
assert.throws(() => (cmd as any).initAction({ options: {} }, {}), cloudError);
});

it(`doesn't throw error when connected to public cloud`, () => {
auth.connection.active = true;
auth.connection.cloudType = CloudType.Public;
assert.doesNotThrow(() => (cmd as any).initAction({ options: {} }, {}));
});

it('throws error when using application-only permissions', () => {
sinonUtil.restore(accessToken.isAppOnlyAccessToken);
sinon.stub(accessToken, 'isAppOnlyAccessToken').returns(true);
auth.connection.cloudType = CloudType.Public;
assert.throws(() => (cmd as any).initAction({ options: {} }, {}), new CommandError('This command does not support application-only permissions.'));
});
});
3 changes: 3 additions & 0 deletions src/m365/base/PowerAppsCommand.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import auth, { CloudType } from '../../Auth.js';
import { Logger } from '../../cli/Logger.js';
import Command, { CommandArgs, CommandError } from '../../Command.js';
import { accessToken } from '../../utils/accessToken.js';

export default abstract class PowerAppsCommand extends Command {
protected get resource(): string {
Expand All @@ -18,5 +19,7 @@ export default abstract class PowerAppsCommand extends Command {
if (auth.connection.cloudType !== CloudType.Public) {
throw new CommandError(`Power Apps commands only support the public cloud at the moment. We'll add support for other clouds in the future. Sorry for the inconvenience.`);
}

accessToken.assertDelegatedAccessToken();
}
}
23 changes: 18 additions & 5 deletions src/m365/base/PowerAutomateCommand.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import auth, { CloudType } from '../../Auth.js';
import { CommandError } from '../../Command.js';
import { telemetry } from '../../telemetry.js';
import PowerAutomateCommand from './PowerAutomateCommand.js';
import { accessToken } from '../../utils/accessToken.js';
import { sinonUtil } from '../../utils/sinonUtil.js';

class MockCommand extends PowerAutomateCommand {
public get name(): string {
Expand All @@ -27,10 +29,18 @@ describe('PowerAutomateCommand', () => {

before(() => {
sinon.stub(telemetry, 'trackEvent').returns();
sinon.stub(accessToken, 'isAppOnlyAccessToken').returns(false);
auth.connection.active = true;
auth.connection.accessTokens[auth.defaultResource] = {
expiresOn: 'abc',
accessToken: 'abc'
};
});

after(() => {
sinon.restore();
auth.connection.active = false;
auth.connection.accessTokens = {};
MathijsVerbeeck marked this conversation as resolved.
Show resolved Hide resolved
});

it('returns correct resource', () => {
Expand All @@ -41,35 +51,38 @@ describe('PowerAutomateCommand', () => {
it(`doesn't throw error when not connected`, () => {
auth.connection.active = false;
(cmd as any).initAction({ options: {} }, {});
auth.connection.active = true;
MathijsVerbeeck marked this conversation as resolved.
Show resolved Hide resolved
});

it('throws error when connected to USGov cloud', () => {
auth.connection.active = true;
auth.connection.cloudType = CloudType.USGov;
assert.throws(() => (cmd as any).initAction({ options: {} }, {}), cloudError);
});

it('throws error when connected to USGovHigh cloud', () => {
auth.connection.active = true;
auth.connection.cloudType = CloudType.USGovHigh;
assert.throws(() => (cmd as any).initAction({ options: {} }, {}), cloudError);
});

it('throws error when connected to USGovDoD cloud', () => {
auth.connection.active = true;
auth.connection.cloudType = CloudType.USGovDoD;
assert.throws(() => (cmd as any).initAction({ options: {} }, {}), cloudError);
});

it('throws error when connected to China cloud', () => {
auth.connection.active = true;
auth.connection.cloudType = CloudType.China;
assert.throws(() => (cmd as any).initAction({ options: {} }, {}), cloudError);
});

it(`doesn't throw error when connected to public cloud`, () => {
auth.connection.active = true;
auth.connection.cloudType = CloudType.Public;
assert.doesNotThrow(() => (cmd as any).initAction({ options: {} }, {}));
});

it('throws error when using application-only permissions', () => {
sinonUtil.restore(accessToken.isAppOnlyAccessToken);
sinon.stub(accessToken, 'isAppOnlyAccessToken').returns(true);
auth.connection.cloudType = CloudType.Public;
assert.throws(() => (cmd as any).initAction({ options: {} }, {}), new CommandError('This command does not support application-only permissions.'));
});
});
3 changes: 3 additions & 0 deletions src/m365/base/PowerAutomateCommand.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import auth, { CloudType } from '../../Auth.js';
import { Logger } from '../../cli/Logger.js';
import Command, { CommandArgs, CommandError } from '../../Command.js';
import { accessToken } from '../../utils/accessToken.js';

export default abstract class PowerAutomateCommand extends Command {
protected get resource(): string {
Expand All @@ -18,5 +19,7 @@ export default abstract class PowerAutomateCommand extends Command {
if (auth.connection.cloudType !== CloudType.Public) {
throw new CommandError(`Power Automate commands only support the public cloud at the moment. We'll add support for other clouds in the future. Sorry for the inconvenience.`);
}

accessToken.assertDelegatedAccessToken();
}
}
22 changes: 19 additions & 3 deletions src/m365/base/PowerBICommand.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { pid } from '../../utils/pid.js';
import { session } from '../../utils/session.js';
import { sinonUtil } from '../../utils/sinonUtil.js';
import PowerBICommand from './PowerBICommand.js';
import { accessToken } from '../../utils/accessToken.js';

class MockCommand extends PowerBICommand {
public get name(): string {
Expand All @@ -27,9 +28,14 @@ class MockCommand extends PowerBICommand {

describe('PowerBICommand', () => {
before(() => {
sinon.stub(telemetry, 'trackEvent').callsFake(() => { });
sinon.stub(pid, 'getProcessName').callsFake(() => '');
sinon.stub(session, 'getId').callsFake(() => '');
sinon.stub(telemetry, 'trackEvent').returns();
sinon.stub(pid, 'getProcessName').returns('');
sinon.stub(session, 'getId').returns('');
sinon.stub(accessToken, 'isAppOnlyAccessToken').returns(false);
auth.connection.accessTokens[auth.defaultResource] = {
expiresOn: 'abc',
accessToken: 'abc'
};
MathijsVerbeeck marked this conversation as resolved.
Show resolved Hide resolved
});

afterEach(() => {
Expand All @@ -38,6 +44,8 @@ describe('PowerBICommand', () => {

after(() => {
sinon.restore();
auth.connection.active = true;
MathijsVerbeeck marked this conversation as resolved.
Show resolved Hide resolved
auth.connection.accessTokens = {};
});

it('correctly reports an error while restoring auth info', async () => {
Expand Down Expand Up @@ -96,4 +104,12 @@ describe('PowerBICommand', () => {
const command = new MockCommand();
assert.strictEqual((command as any).resource, 'https://api.powerbi.com');
});

it('throws error when using application-only permissions', () => {
const cmd = new MockCommand();
sinonUtil.restore(accessToken.isAppOnlyAccessToken);
sinon.stub(accessToken, 'isAppOnlyAccessToken').returns(true);
auth.connection.active = true;
MathijsVerbeeck marked this conversation as resolved.
Show resolved Hide resolved
assert.throws(() => (cmd as any).initAction({ options: {} }, {}), new CommandError('This command does not support application-only permissions.'));
});
});
17 changes: 16 additions & 1 deletion src/m365/base/PowerBICommand.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,22 @@
import Command from '../../Command.js';
import Command, { CommandArgs } from '../../Command.js';
import auth from '../../Auth.js';
import { accessToken } from '../../utils/accessToken.js';
import { Logger } from '../../cli/Logger.js';

export default abstract class PowerBICommand extends Command {
protected get resource(): string {
return 'https://api.powerbi.com';
}

protected initAction(args: CommandArgs, logger: Logger): void {
super.initAction(args, logger);

if (!auth.connection.active) {
// we fail no login in the base command command class
return;
}

accessToken.assertDelegatedAccessToken();
}

}
22 changes: 17 additions & 5 deletions src/m365/base/PowerPlatformCommand.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import auth, { CloudType } from '../../Auth.js';
import { CommandError } from '../../Command.js';
import { telemetry } from '../../telemetry.js';
import PowerPlatformCommand from './PowerPlatformCommand.js';
import { accessToken } from '../../utils/accessToken.js';
import { sinonUtil } from '../../utils/sinonUtil.js';

class MockCommand extends PowerPlatformCommand {
public get name(): string {
Expand All @@ -27,10 +29,17 @@ describe('PowerPlatformCommand', () => {

before(() => {
sinon.stub(telemetry, 'trackEvent').returns();
sinon.stub(accessToken, 'isAppOnlyAccessToken').returns(false);
auth.connection.accessTokens[auth.defaultResource] = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add auth.connection.active = true;

expiresOn: 'abc',
accessToken: 'abc'
};
});

after(() => {
sinon.restore();
auth.connection.active = false;
auth.connection.accessTokens = {};
MathijsVerbeeck marked this conversation as resolved.
Show resolved Hide resolved
});

it('returns correct bapi resource', () => {
Expand All @@ -41,35 +50,38 @@ describe('PowerPlatformCommand', () => {
it(`doesn't throw error when not connected`, () => {
auth.connection.active = false;
(cmd as any).initAction({ options: {} }, {});
auth.connection.active = true;
MathijsVerbeeck marked this conversation as resolved.
Show resolved Hide resolved
});

it('throws error when connected to USGov cloud', () => {
auth.connection.active = true;
auth.connection.cloudType = CloudType.USGov;
assert.throws(() => (cmd as any).initAction({ options: {} }, {}), cloudError);
});

it('throws error when connected to USGovHigh cloud', () => {
auth.connection.active = true;
auth.connection.cloudType = CloudType.USGovHigh;
assert.throws(() => (cmd as any).initAction({ options: {} }, {}), cloudError);
});

it('throws error when connected to USGovDoD cloud', () => {
auth.connection.active = true;
auth.connection.cloudType = CloudType.USGovDoD;
assert.throws(() => (cmd as any).initAction({ options: {} }, {}), cloudError);
});

it('throws error when connected to China cloud', () => {
auth.connection.active = true;
auth.connection.cloudType = CloudType.China;
assert.throws(() => (cmd as any).initAction({ options: {} }, {}), cloudError);
});

it(`doesn't throw error when connected to public cloud`, () => {
auth.connection.active = true;
auth.connection.cloudType = CloudType.Public;
assert.doesNotThrow(() => (cmd as any).initAction({ options: {} }, {}));
});

it('throws error when using application-only permissions', () => {
sinonUtil.restore(accessToken.isAppOnlyAccessToken);
sinon.stub(accessToken, 'isAppOnlyAccessToken').returns(true);
auth.connection.cloudType = CloudType.Public;
assert.throws(() => (cmd as any).initAction({ options: {} }, {}), new CommandError('This command does not support application-only permissions.'));
});
});