Skip to content

Commit

Permalink
fix: remove refs to legacy client id
Browse files Browse the repository at this point in the history
@W-11659812@
  • Loading branch information
peternhale committed Nov 23, 2022
1 parent 468d815 commit 82689ea
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 25 deletions.
11 changes: 6 additions & 5 deletions src/org/authInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,15 +179,16 @@ function parseIdUrl(idUrl: string): { userId: string | undefined; orgId: string

export const DEFAULT_CONNECTED_APP_INFO = {
clientId: 'PlatformCLI',
clientSecret: '',
// Legacy. The connected app info is owned by the thing that
// creates new AuthInfos. Currently that is the auth:* commands which
// aren't owned by this core library. These values need to be here
// for any old auth files where the id and secret aren't stored.
//
// Ideally, this would be removed at some point in the distant future
// when all auth files now have the clientId stored in it.
legacyClientId: 'SalesforceDevelopmentExperience',
legacyClientSecret: '1384510088588713504',
// legacyClientId: 'SalesforceDevelopmentExperience',
// legacyClientSecret: '1384510088588713504',
};

/**
Expand Down Expand Up @@ -573,7 +574,7 @@ export class AuthInfo extends AsyncOptionalCreatable<AuthInfo.Options> {
}

public getClientId(): string {
return this.getFields()?.clientId ?? DEFAULT_CONNECTED_APP_INFO.legacyClientId;
return this.getFields()?.clientId ?? DEFAULT_CONNECTED_APP_INFO.clientId;
}

public getRedirectUri(): string {
Expand Down Expand Up @@ -960,8 +961,8 @@ export class AuthInfo extends AsyncOptionalCreatable<AuthInfo.Options> {
// Ideally, this would be removed at some point in the distant future when all auth files
// now have the clientId stored in it.
if (!options.clientId) {
options.clientId = DEFAULT_CONNECTED_APP_INFO.legacyClientId;
options.clientSecret = DEFAULT_CONNECTED_APP_INFO.legacyClientSecret;
options.clientId = DEFAULT_CONNECTED_APP_INFO.clientId;
options.clientSecret = DEFAULT_CONNECTED_APP_INFO.clientSecret;
}

if (!options.redirectUri) {
Expand Down
8 changes: 2 additions & 6 deletions src/org/scratchOrgInfoGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,8 @@ import { SfError } from '../sfError';
import { Org } from './org';
import { ScratchOrgInfo } from './scratchOrgTypes';
import { ScratchOrgFeatureDeprecation } from './scratchOrgFeatureDeprecation';
import { DEFAULT_CONNECTED_APP_INFO } from './authInfo';

const defaultConnectedAppInfo = {
clientId: 'PlatformCLI',
legacyClientId: 'SalesforceDevelopmentExperience',
legacyClientSecret: '1384510088588713504',
};
Messages.importMessagesDirectory(__dirname);
const messages = Messages.loadMessages('@salesforce/core', 'scratchOrgInfoGenerator');

Expand Down Expand Up @@ -230,7 +226,7 @@ export const generateScratchOrgInfo = async ({
// Use the Hub org's client ID value, if one wasn't provided to us, or the default
if (!scratchOrgInfoPayload.connectedAppConsumerKey) {
scratchOrgInfoPayload.connectedAppConsumerKey =
hubOrg.getConnection().getAuthInfoFields().clientId ?? defaultConnectedAppInfo.clientId;
hubOrg.getConnection().getAuthInfoFields().clientId ?? DEFAULT_CONNECTED_APP_INFO.clientId;
}

if (!nonamespace && sfProject?.get('namespace')) {
Expand Down
21 changes: 7 additions & 14 deletions test/unit/org/authInfoTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import * as dns from 'dns';
import * as jwt from 'jsonwebtoken';
import { cloneJson, env, includes } from '@salesforce/kit';
import { spyMethod, stubMethod } from '@salesforce/ts-sinon';
import { AnyJson, ensureString, getJsonMap, JsonMap, toJsonMap } from '@salesforce/ts-types';
import { AnyJson, getJsonMap, JsonMap, toJsonMap } from '@salesforce/ts-types';
import { expect } from 'chai';
import { Transport } from 'jsforce/lib/transport';

Expand All @@ -33,8 +33,8 @@ class AuthInfoMockOrg extends MockTestOrgData {
public expirationDate = '12-02-20';
public encryptedAccessToken = this.accessToken;
public defaultConnectedAppInfo = {
clientId: 'SalesforceDevelopmentExperience',
clientSecret: '1384510088588713504',
clientId: 'PlatformCLI',
clientSecret: '',
};

public async getConfig(): Promise<AuthFields> {
Expand Down Expand Up @@ -153,7 +153,6 @@ describe('AuthInfo', () => {
// verify the returned object doesn't have secrets
expect(() => walkAndSearchForSecrets(toJsonMap(fields) || {}, decryptedRefreshToken)).to.not.throw();

expect(strObj).does.not.include(ensureString(testOrg.defaultConnectedAppInfo.clientSecret));
expect(strObj).does.not.include(decryptedRefreshToken);
});
});
Expand All @@ -177,7 +176,6 @@ describe('AuthInfo', () => {
expect(() => walkAndSearchForSecrets(toJsonMap(fields) || {}, decryptedRefreshToken)).to.not.throw();

// double check the stringified objects don't have secrets.
expect(strObj).does.not.include(ensureString(testOrg.defaultConnectedAppInfo.clientSecret));
expect(strObj).does.not.include(decryptedRefreshToken);
});
});
Expand All @@ -190,7 +188,6 @@ describe('AuthInfo', () => {
expect(() => walkAndSearchForSecrets(toJsonMap(authInfo) || {}, decryptedRefreshToken)).to.not.throw();

// double check the stringified objects don't have secrets.
expect(authInfoString).does.not.include(ensureString(testOrg.defaultConnectedAppInfo.clientSecret));
expect(authInfoString).does.not.include(decryptedRefreshToken);
});
});
Expand Down Expand Up @@ -1024,8 +1021,8 @@ describe('AuthInfo', () => {
isDevHub: false,
// clientId and clientSecret are now stored in the file, even the defaults.
// We just hard code the legacy values here to ensure old auth files will still work.
clientId: 'SalesforceDevelopmentExperience',
clientSecret: '1384510088588713504',
clientId: 'PlatformCLI',
clientSecret: undefined,
expirationDate: testOrg.expirationDate,
};
// Note that this also verifies the clientId and clientSecret are not persisted,
Expand Down Expand Up @@ -1226,9 +1223,7 @@ describe('AuthInfo', () => {
});

expect(authInfo.getSfdxAuthUrl()).to.contain(
`force://SalesforceDevelopmentExperience:1384510088588713504:${
testOrg.refreshToken
}@${testOrg.instanceUrl.replace('https://', '')}`
`force://PlatformCLI::${testOrg.refreshToken}@${testOrg.instanceUrl.replace('https://', '')}`
);
});

Expand All @@ -1254,9 +1249,7 @@ describe('AuthInfo', () => {
// delete the client secret
delete authInfo.getFields().clientSecret;
const instanceUrl = testOrg.instanceUrl.replace('https://', '');
expect(authInfo.getSfdxAuthUrl()).to.contain(
`force://SalesforceDevelopmentExperience::${testOrg.refreshToken}@${instanceUrl}`
);
expect(authInfo.getSfdxAuthUrl()).to.contain(`force://PlatformCLI::${testOrg.refreshToken}@${instanceUrl}`);
});

it('should handle undefined refresh token', async () => {
Expand Down

0 comments on commit 82689ea

Please sign in to comment.