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

feat: identify scratch orgs if hub is known during auth #148

Merged
merged 28 commits into from
May 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
0caaa78
feat: identify scratch orgs if hub is known during auth
mshanemc May 7, 2021
2a15217
chore: remove codecov
mshanemc May 10, 2021
94c5c34
chore: clean up github folder
mshanemc May 10, 2021
0e0c20e
refactor: don't use prod-like loginUrl to exit
mshanemc May 10, 2021
cde6c44
chore: prune unnecessary ts-comment
mshanemc May 10, 2021
4023911
test: temporarily remove scratch-org-id to verify tests
mshanemc May 10, 2021
39c852f
style: typos
mshanemc May 11, 2021
9cc16ce
test: improve logging, temporarily use console
mshanemc May 11, 2021
87f9489
chore: back to regular logging
mshanemc May 11, 2021
0f5df91
test: stubs for authinfo.hasAuthentications
mshanemc May 11, 2021
4e9a1ab
style: typos
mshanemc May 11, 2021
86279b6
test: try single test
mshanemc May 11, 2021
e8a224b
test: return a promise?
mshanemc May 11, 2021
b70c417
test: check authentications from instance
mshanemc May 11, 2021
9eca74d
test: stub method used by hasAuthentications
mshanemc May 11, 2021
c502206
test: run all the tests again
mshanemc May 11, 2021
754d860
test: try grant using stubbed listAll
mshanemc May 11, 2021
86380bd
test: stub listAll for web:login
mshanemc May 11, 2021
bfb68df
test: store uses stub
mshanemc May 11, 2021
74f93f4
style: better error messages
mshanemc May 12, 2021
0b01070
Merge branch 'main' into sm/scratch-org-id
mshanemc May 12, 2021
377b2c3
Merge branch 'sm/scratch-org-id' of https://github.com/salesforcecli/…
mshanemc May 12, 2021
5e32c89
style: typo in nut name!
mshanemc May 12, 2021
706eca8
refactor: changes from review comments
mshanemc May 12, 2021
c914648
refactor: getFields from authinfo in scratch-id
mshanemc May 12, 2021
123444c
Revert "refactor: getFields from authinfo in scratch-id"
mshanemc May 14, 2021
bd85a41
style: comment for why we pass in the fields object
mshanemc May 14, 2021
f80129d
fix: use decrypted fields, not result of getFields
mshanemc May 14, 2021
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: 0 additions & 5 deletions .github/CODEOWNERS

This file was deleted.

39 changes: 0 additions & 39 deletions .github/ISSUE_TEMPLATE/Bug_report.md

This file was deleted.

16 changes: 0 additions & 16 deletions .github/ISSUE_TEMPLATE/Feature_request.md

This file was deleted.

2 changes: 0 additions & 2 deletions .github/autointegrator.yml

This file was deleted.

9 changes: 0 additions & 9 deletions .github/no-response.yml

This file was deleted.

1 change: 1 addition & 0 deletions src/commands/auth/device/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ export default class Login extends SfdxCommand {
const authInfo = await deviceOauthService.authorizeAndSave(approval);
await Common.handleSideEffects(authInfo, this.flags);
const fields = authInfo.getFields(true);
await Common.identifyPossibleScratchOrgs(fields, authInfo);
const successMsg = messages.getMessage('success', [fields.username]);
this.ux.log(successMsg);
return fields;
Expand Down
1 change: 1 addition & 0 deletions src/commands/auth/jwt/grant.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export default class Grant extends SfdxCommand {
const authInfo = await this.initAuthInfo();
await Common.handleSideEffects(authInfo, this.flags);
result = authInfo.getFields(true);
await Common.identifyPossibleScratchOrgs(result, authInfo);
} catch (err) {
const msg = getString(err, 'message');
throw SfdxError.create('@salesforce/plugin-auth', 'jwt.grant', 'JwtGrantError', [msg]);
Expand Down
2 changes: 2 additions & 0 deletions src/commands/auth/sfdxurl/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ export default class Store extends SfdxCommand {
await Common.handleSideEffects(authInfo, this.flags);

const result = authInfo.getFields(true);
await Common.identifyPossibleScratchOrgs(result, authInfo);

const successMsg = commonMessages.getMessage('authorizeCommandSuccess', [result.username, result.orgId]);
this.ux.log(successMsg);
return result;
Expand Down
2 changes: 2 additions & 0 deletions src/commands/auth/web/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ export default class Login extends SfdxCommand {
const authInfo = await this.executeLoginFlow(oauthConfig);
await Common.handleSideEffects(authInfo, this.flags);
const fields = authInfo.getFields(true);
await Common.identifyPossibleScratchOrgs(fields, authInfo);

const successMsg = commonMessages.getMessage('authorizeCommandSuccess', [fields.username, fields.orgId]);
this.ux.log(successMsg);
return fields;
Expand Down
59 changes: 57 additions & 2 deletions src/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

import { AuthInfo, Logger, SfdcUrl, SfdxProject, Messages, SfdxError } from '@salesforce/core';
import { basename } from 'path';
import { QueryResult } from 'jsforce';
import { AuthInfo, AuthFields, Logger, SfdcUrl, SfdxProject, Messages, Org, SfdxError, sfdc } from '@salesforce/core';
import { getString, isObject, Optional } from '@salesforce/ts-types';

Messages.importMessagesDirectory(__dirname);
Expand Down Expand Up @@ -54,4 +55,58 @@ export class Common {
logger.debug(`loginUrl: ${loginUrl}`);
return loginUrl;
}

// fields property is passed in because the consumers of this method have performed the decrypt.
// This is so we don't have to call authInfo.getFields(true) and decrypt again OR accidentally save an
public static async identifyPossibleScratchOrgs(fields: AuthFields, orgAuthInfo: AuthInfo): Promise<void> {
mshanemc marked this conversation as resolved.
Show resolved Hide resolved
const logger = await Logger.child('Common', { tag: 'identifyPossibleScratchOrgs' });

// return if we already know the hub or we know it is a devhub or prod-like
if (fields.isDevHub || fields.devHubUsername) return;
// there are no hubs to ask, so quit early
if (!(await AuthInfo.hasAuthentications())) return;
logger.debug('getting devHubs from authfiles');

// TODO: return if url is not sandbox-like to avoid constantly asking about production orgs
// TODO: someday we make this easier by asking the org if it is a scratch org

const hubAuthInfos = await this.getDevHubAuthInfos();
logger.debug(`found ${hubAuthInfos.length} DevHubs`);
if (hubAuthInfos.length === 0) return;

// ask all those orgs if they know this orgId
await Promise.all(
Copy link
Contributor

Choose a reason for hiding this comment

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

@mshanemc feels like this could get expensive; API limits, time, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it scales with the number of devhubs you have. So for most users, fairly mild. We'd ask one hub about one ScratchOrgInfo.

If you did have a lot of hubs, we're still only one query per hub (re: limits) and executing in parallel (re: time)

What's a better alternative when the requirement is "at auth time, identify the hub for an org that may be a scratch org?"

Copy link
Contributor

Choose a reason for hiding this comment

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

@mshanemc no good answer really, maybe this will get better as we move towards sf, with a combined auth file.

hubAuthInfos.map(async (hubAuthInfo) => {
try {
const devHubOrg = await Org.create({ aliasOrUsername: hubAuthInfo.getUsername() });
const conn = devHubOrg.getConnection();
const data = await conn.query<QueryResult<{ Id: string }>>(
`select Id from ScratchOrgInfo where ScratchOrg = '${sfdc.trimTo15(fields.orgId)}'`
);
if (data.totalSize > 0) {
// if any return a result
logger.debug(`found orgId ${fields.orgId} in devhub ${hubAuthInfo.getUsername()}`);
try {
await orgAuthInfo.save({ ...fields, devHubUsername: hubAuthInfo.getUsername() });
logger.debug(`set ${hubAuthInfo.getUsername()} as devhub for scratch org ${orgAuthInfo.getUsername()}`);
} catch (error) {
logger.debug(`error updating auth file for ${orgAuthInfo.getUsername()}`, error);
}
}
} catch (error) {
logger.error(`Error connecting to devhub ${hubAuthInfo.getUsername()}`, error);
}
})
);
}

public static async getDevHubAuthInfos(): Promise<AuthInfo[]> {
return (
await Promise.all(
(await AuthInfo.listAllAuthFiles())
.map((fileName) => basename(fileName, '.json'))
.map((username) => AuthInfo.create({ username }))
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an expensive operation. Any alternatives?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't come up with any other way to figure out what auths are hubs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this approach limits the # of orgs that get queried in the above Promise.all

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the fastest alternative would be to read the auth files directly (not go through core) but that opens up its own set of problems.

We could also add a method in core to deliver the raw auth file contents as json instead of AuthInfos. It's still potentially a lot of fs, but we could parallelize the read/parse better than core does today

Copy link
Contributor Author

Choose a reason for hiding this comment

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

one more idea (kinda hacky) is to filter out obvious scratch-org-type names (foo@example.com, that thing where a timestamp is added to the main user to create secondary users, etc).

)
).filter((possibleHub) => possibleHub?.getFields()?.isDevHub);
}
}
4 changes: 3 additions & 1 deletion test/commands/auth/device/login.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

/* eslint-disable @typescript-eslint/ban-ts-comment */
/* eslint-disable camelcase */

import { $$, expect, test } from '@salesforce/command/lib/test';
Expand Down Expand Up @@ -88,6 +87,9 @@ describe('auth:device:login', async () => {
}

stubMethod($$.SANDBOX, AuthInfo, 'create').callsFake(async () => authInfoStub);
$$.SANDBOX.stub(AuthInfo, 'listAllAuthFiles').callsFake(async () => {
return [`${authFields.username}.json`];
});
}

test
Expand Down
8 changes: 4 additions & 4 deletions test/commands/auth/jwt/grant.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ describe('auth:jwt:grant', async () => {
getFields: () => authFields,
});

$$.SANDBOX.stub(AuthInfo, 'listAllAuthFiles').callsFake(async () => {
return [`${authFields.username}.json`];
});

if (options.authInfoCreateFails) {
$$.SANDBOX.stub(AuthInfo, 'create').throws(new Error('invalid client id'));
} else if (options.existingAuth) {
Expand All @@ -36,10 +40,6 @@ describe('auth:jwt:grant', async () => {
.throws(new SfdxError('auth exists', 'AuthInfoOverwriteError'))
.onSecondCall()
.callsFake(async () => authInfoStub);

$$.SANDBOX.stub(AuthInfo, 'listAllAuthFiles').callsFake(async () => {
return [`${authFields.username}.json`];
});
} else {
stubMethod($$.SANDBOX, AuthInfo, 'create').callsFake(async () => authInfoStub);
}
Expand Down
83 changes: 83 additions & 0 deletions test/commands/auth/scratch-identify.nut.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
* Copyright (c) 2020, salesforce.com, inc.
* All rights reserved.
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import * as path from 'path';
import { expect } from 'chai';

import { execCmd, TestSession } from '@salesforce/cli-plugins-testkit';
import { Env } from '@salesforce/kit';
import { AnyJson, ensureString, getString, isArray } from '@salesforce/ts-types';
import { AuthFields, Authorization, fs } from '@salesforce/core';

describe('verify discovery/id of scratch org', () => {
let testSession: TestSession;
let hubUsername: string;
let orgUsername: string;
let jwtKey: string;
let orgInstanceUrl: string;

before('prepare session and ensure environment variables', async () => {
const env = new Env();
ensureString(env.getString('TESTKIT_JWT_KEY'));
ensureString(env.getString('TESTKIT_JWT_CLIENT_ID'));
ensureString(env.getString('TESTKIT_HUB_INSTANCE'));
hubUsername = ensureString(env.getString('TESTKIT_HUB_USERNAME'));
testSession = await TestSession.create({
project: { name: 'ScratchIDProject' },
setupCommands: [
`sfdx force:org:create -d 1 -s -f ${path.join('config', 'project-scratch-def.json')} --json`,
'sfdx force:org:display --json',
],
});
if (isArray<AnyJson>(testSession.setup)) {
orgUsername = getString(testSession.setup[0], 'result.username');
orgInstanceUrl = getString(testSession.setup[1], 'result.instanceUrl', 'https://test.salesforce.com').replace(
'.com/',
'.com'
);
}
// we'll need this path for testing
jwtKey = path.join(testSession.homeDir, 'jwtKey');
});

after(async () => {
await testSession?.clean();
});

it('should have the scratch org in auth files', () => {
const list = execCmd<Authorization[]>('auth:list --json', { ensureExitCode: 0 }).jsonOutput;
const found = !!list.result.find((r) => r.username === orgUsername);
expect(found).to.be.true;
});

it('should logout from the org)', () => {
execCmd(`auth:logout -u ${orgUsername} --noprompt`, { ensureExitCode: 0 });
});

it('should NOT have the scratch org in auth files', () => {
const list = execCmd<Authorization[]>('auth:list --json', { ensureExitCode: 0 }).jsonOutput;
const found = !!list.result.find((r) => r.username === orgUsername);
expect(found).to.be.false;
});

it('should login to the org via jwt grant', async function () {
const env = new Env();
const command = `auth:jwt:grant -f ${jwtKey} --username ${orgUsername} --clientid ${env.getString(
'TESTKIT_JWT_CLIENT_ID'
)} -r ${orgInstanceUrl} --json`;
const output = execCmd<AuthFields>(command, {
ensureExitCode: 0,
}).jsonOutput.result;
expect(output.username).to.equal(orgUsername);
});

it('should have the devhubUsername in the auth file', async () => {
const fileContents = (await fs.readJson(path.join(testSession.homeDir, '.sfdx', `${orgUsername}.json`))) as {
devHubUsername: string;
};
expect(fileContents.devHubUsername).to.equal(hubUsername);
});
});
7 changes: 4 additions & 3 deletions test/commands/auth/sfdxurl/store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ describe('auth:sfdxurl:store', async () => {
getFields: () => authFields,
});

$$.SANDBOX.stub(AuthInfo, 'listAllAuthFiles').callsFake(async () => {
return [`${authFields.username}.json`];
});

if (!options.fileDoesNotExist) {
$$.SANDBOX.stub(fs, 'readFile').callsFake(
async () => 'force://PlatformCLI::CoffeeAndBacon@su0503.my.salesforce.com'
Expand All @@ -38,9 +42,6 @@ describe('auth:sfdxurl:store', async () => {
if (options.authInfoCreateFails) {
$$.SANDBOX.stub(AuthInfo, 'create').throws(new Error('invalid client id'));
} else if (options.existingAuth) {
$$.SANDBOX.stub(AuthInfo, 'listAllAuthFiles').callsFake(async () => {
return [`${authFields.username}.json`];
});
stubMethod($$.SANDBOX, AuthInfo, 'create').callsFake(async () => authInfoStub);
} else {
stubMethod($$.SANDBOX, AuthInfo, 'create').callsFake(async () => authInfoStub);
Expand Down
11 changes: 7 additions & 4 deletions test/commands/auth/web/login.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,17 @@ describe('auth:web:login', () => {
stubMethod($$.SANDBOX, Login.prototype, 'executeLoginFlow').callsFake(async () => {
return authInfoStub;
});
$$.SANDBOX.stub(AuthInfo, 'listAllAuthFiles').callsFake(async () => {
return [`${authFields.username}.json`];
});
uxStub = stubInterface<UX>($$.SANDBOX, {
prompt: () => promptAnswer,
});

const login = new Login([], config);
// @ts-ignore because protected memeber
// @ts-ignore because protected member
login.ux = uxStub;
// @ts-ignore because protected memeber
// @ts-ignore because protected member
login.flags = Object.assign({}, { noprompt: true }, flags);
return login;
}
Expand All @@ -58,9 +61,9 @@ describe('auth:web:login', () => {
uxStub = stubInterface<UX>($$.SANDBOX, {});

const login = new Login([], config);
// @ts-ignore because protected memeber
// @ts-ignore because protected member
login.ux = uxStub;
// @ts-ignore because protected memeber
// @ts-ignore because protected member
login.flags = { noprompt: true };
return login;
}
Expand Down