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

fix: identify sandboxes during authentication #862

Merged
merged 5 commits into from
Aug 2, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
49 changes: 45 additions & 4 deletions src/org/authInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import { Messages } from '../messages';
import { getLoginAudienceCombos, SfdcUrl } from '../util/sfdcUrl';
import { Connection, SFDX_HTTP_HEADERS } from './connection';
import { OrgConfigProperties } from './orgConfigProperties';
import { Org } from './org';
import { Org, SandboxFields } from './org';

Messages.importMessagesDirectory(__dirname);
const messages = Messages.loadMessages('@salesforce/core', 'core');
Expand Down Expand Up @@ -373,7 +373,7 @@ export class AuthInfo extends AsyncOptionalCreatable<AuthInfo.Options> {

/**
* Given a set of decrypted fields and an authInfo, determine if the org belongs to an available
* dev hub.
* dev hub, or if the org is a sandbox of another CLI authed production org.
*
* @param fields
* @param orgAuthInfo
Expand All @@ -387,7 +387,7 @@ export class AuthInfo extends AsyncOptionalCreatable<AuthInfo.Options> {
// return if we already know the hub org we know it is a devhub or prod-like or no orgId present
if (fields.isDevHub || fields.devHubUsername || !fields.orgId) return;

logger.debug('getting devHubs');
logger.debug('getting devHubs to identify scratch orgs and sandboxes');

// 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
Expand Down Expand Up @@ -419,7 +419,48 @@ export class AuthInfo extends AsyncOptionalCreatable<AuthInfo.Options> {
logger.debug(`error updating auth file for ${orgAuthInfo.getUsername()}`, error);
}
} catch (error) {
logger.error(`Error connecting to devhub ${hubAuthInfo.username}`, error);
if (error instanceof Error && error.name === 'SingleRecordQuery_NoRecords' && fields.orgId) {
shetzel marked this conversation as resolved.
Show resolved Hide resolved
// Not a scratch org owned by this hub. Check if it's a sandbox.
try {
const prodOrg = await Org.create({ aliasOrUsername: hubAuthInfo.username });
const sbxProcess = await prodOrg.querySandboxProcessByOrgId(fields.orgId);
logger.debug(`${fields.orgId} is a sandbox of ${hubAuthInfo.username}`);

try {
await orgAuthInfo.save({
...fields,
isScratch: false,
isSandbox: true,
});
} catch (err) {
logger.debug(`error updating auth file for: ${orgAuthInfo.getUsername()}`, err);
}

try {
Copy link
Contributor

Choose a reason for hiding this comment

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

would anything bad happen if you failed to update the AuthFile but then updated the SandboxConfig?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. That's most likely a bad state though so probably just abort the mission (return;) if there's an error updating the auth file. I'll make that change.

// set the sandbox config value
const sfSandbox = {
sandboxUsername: fields.username,
sandboxOrgId: fields.orgId,
prodOrgUsername: hubAuthInfo.username,
sandboxName: sbxProcess.SandboxName,
sandboxProcessId: sbxProcess.Id,
sandboxInfoId: sbxProcess.SandboxInfoId,
timestamp: new Date().toISOString(),
} as SandboxFields;

const stateAggregator = await StateAggregator.getInstance();
stateAggregator.sandboxes.set(fields.orgId, sfSandbox);
logger.debug(`writing sandbox auth file for: ${orgAuthInfo.getUsername()} with ID: ${fields.orgId}`);
await stateAggregator.sandboxes.write(fields.orgId);
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems safe from the ConfigFile bugs because it should only ever hit one of these.

Just be sure that the auth command is calling this identifyPossibleScratchOrgs NOT in parallel with anything else that might be touching the authFile.

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 checked all places I could find and nothing was done in parallel so hopefully it's ok.

} catch (e) {
logger.debug(`error writing sandbox auth file for: ${orgAuthInfo.getUsername()}`, e);
}
} catch (err) {
logger.debug(`${fields.orgId} is not a sandbox of ${hubAuthInfo.username}`);
}
} else {
logger.error(`Error connecting to devhub ${hubAuthInfo.username}`, error);
}
}
})
);
Expand Down
15 changes: 11 additions & 4 deletions src/org/org.ts
Original file line number Diff line number Diff line change
Expand Up @@ -908,7 +908,6 @@ export class Org extends AsyncOptionalCreatable<Org.Options> {
* query SandboxProcess via sandbox name
*
* @param name SandboxName to query for
* @private
*/
public async querySandboxProcessBySandboxName(name: string): Promise<SandboxProcessObject> {
return this.querySandboxProcess(`SandboxName='${name}'`);
Expand All @@ -918,7 +917,6 @@ export class Org extends AsyncOptionalCreatable<Org.Options> {
* query SandboxProcess via SandboxInfoId
*
* @param id SandboxInfoId to query for
* @private
*/
public async querySandboxProcessBySandboxInfoId(id: string): Promise<SandboxProcessObject> {
return this.querySandboxProcess(`SandboxInfoId='${id}'`);
Expand All @@ -928,12 +926,21 @@ export class Org extends AsyncOptionalCreatable<Org.Options> {
* query SandboxProcess via Id
*
* @param id SandboxProcessId to query for
* @private
*/
public async querySandboxProcessById(id: string): Promise<SandboxProcessObject> {
return this.querySandboxProcess(`Id='${id}'`);
}

/**
* query SandboxProcess via SandboxOrganization (sandbox Org ID)
*
* @param sandboxOrgId SandboxOrganization ID to query for
*/
public async querySandboxProcessByOrgId(sandboxOrgId: string): Promise<SandboxProcessObject> {
// Must query with a 15 character Org ID
return this.querySandboxProcess(`SandboxOrganization='${trimTo15(sandboxOrgId)}'`);
}

/**
* Initialize async components.
*/
Expand Down Expand Up @@ -1291,7 +1298,7 @@ export class Org extends AsyncOptionalCreatable<Org.Options> {
// save auth info for new sandbox
await authInfo.save();

const sandboxOrgId = authInfo.getFields().orgId as string;
const sandboxOrgId = authInfo.getFields().orgId;

if (!sandboxOrgId) {
throw messages.createError('AuthInfoOrgIdUndefined');
Expand Down
53 changes: 47 additions & 6 deletions test/unit/org/authInfoTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ import { Transport } from 'jsforce/lib/transport';

import { JwtOAuth2Config, OAuth2 } from 'jsforce';
import { SinonSpy, SinonStub } from 'sinon';
import { AuthFields, AuthInfo } from '../../../src/org';
import { AuthFields, AuthInfo, Org } from '../../../src/org';
import { MockTestOrgData, shouldThrow, shouldThrowSync, TestContext } from '../../../src/testSetup';
import { OrgConfigProperties } from '../../../src/org/orgConfigProperties';
import { AliasAccessor, OrgAccessor } from '../../../src/stateAggregator';
import { AliasAccessor, OrgAccessor, StateAggregator } from '../../../src/stateAggregator';
import { Crypto } from '../../../src/crypto/crypto';
import { Config } from '../../../src/config/config';
import { SfdcUrl } from '../../../src/util/sfdcUrl';
Expand Down Expand Up @@ -1779,7 +1779,7 @@ describe('AuthInfo', () => {
user1 = new MockTestOrgData();
});

it('should not update org - no dev hubs', async () => {
it('should not update auth file - no dev hubs', async () => {
await $$.stubAuths(adminTestData, user1);

const authInfo = await AuthInfo.create({
Expand All @@ -1796,7 +1796,7 @@ describe('AuthInfo', () => {
expect(authInfoSaveStub.callCount).to.be.equal(0);
});

it('should not update org - state already known', async () => {
it('should not update auth file - state already known', async () => {
adminTestData.makeDevHub();
user1.isScratchOrg = true;
user1.devHubUsername = adminTestData.username;
Expand All @@ -1817,7 +1817,7 @@ describe('AuthInfo', () => {
expect(authInfoSaveStub.callCount).to.be.equal(0);
});

it('should not update org - no fields.orgId', async () => {
it('should not update auth file - no fields.orgId', async () => {
adminTestData.makeDevHub();
user1.isScratchOrg = true;
// @ts-expect-error - operand must be optional
Expand All @@ -1839,7 +1839,7 @@ describe('AuthInfo', () => {
expect(authInfoSaveStub.callCount).to.be.equal(0);
});

it('should update org', async () => {
it('should update auth file as a scratch org', async () => {
adminTestData.makeDevHub();

await $$.stubAuths(adminTestData, user1);
Expand All @@ -1859,6 +1859,47 @@ describe('AuthInfo', () => {
expect(queryScratchOrgStub.callCount).to.be.equal(1);
expect(authInfoSaveStub.callCount).to.be.equal(1);
});

it('should update auth file as a sandbox', async () => {
adminTestData.makeDevHub();

await $$.stubAuths(adminTestData, user1);

const authInfo = await AuthInfo.create({
username: user1.username,
});

const stateAggregator = await StateAggregator.getInstance();
const stateAggregatorStub = stubMethod($$.SANDBOX, StateAggregator, 'getInstance');
const sandboxSetStub = stubMethod($$.SANDBOX, stateAggregator.sandboxes, 'set');
const sandboxWriteStub = stubMethod($$.SANDBOX, stateAggregator.sandboxes, 'write');
stateAggregatorStub.resolves(stateAggregator);
const authInfoSaveStub = stubMethod($$.SANDBOX, AuthInfo.prototype, 'save').resolves();
const queryScratchOrgStub = stubMethod($$.SANDBOX, AuthInfo, 'queryScratchOrg');
const queryScratchOrgError = new Error('not a scratch org');
queryScratchOrgError.name = 'SingleRecordQuery_NoRecords';
queryScratchOrgStub.throws(queryScratchOrgError);
const sbxQueryStub = stubMethod($$.SANDBOX, Org.prototype, 'querySandboxProcessByOrgId');
sbxQueryStub.resolves({
Id: '0GRB0000000L0ZVOA0',
Status: 'Completed',
SandboxName: 'TestSandbox',
SandboxInfoId: '0GQB0000000PCOdOAO',
LicenseType: 'DEVELOPER',
CreatedDate: '2021-01-22T22:49:52.000+0000',
});

await AuthInfo.identifyPossibleScratchOrgs(authInfo.getFields(), authInfo);

expect(authInfoSaveStub.calledTwice).to.be.true;
expect(authInfoSaveStub.secondCall.args[0]).to.have.property('isSandbox', true);
expect(authInfoSaveStub.secondCall.args[0]).to.have.property('isScratch', false);
expect(sandboxSetStub.calledOnce).to.be.true;
expect(sandboxSetStub.firstCall.args[0]).to.equal(authInfo.getFields().orgId);
expect(sandboxSetStub.firstCall.args[1]).to.have.property('prodOrgUsername', adminTestData.username);
expect(sandboxWriteStub.calledOnce).to.be.true;
expect(sandboxWriteStub.firstCall.args[0]).to.equal(authInfo.getFields().orgId);
});
});

describe('determineIfDevHub', () => {
Expand Down