Merge pull request #138 from jonnyhork/jhork/prevent-auth-overwrite

Jhork/prevent auth overwrite

messages/core.json

@@ -1,18 +1,20 @@
 {
-    "JsonParseError": "Parse error in file %s on line %s\n%s\n",
-    "AuthInfoCreationError": "Must pass a username and/or OAuth options when creating an AuthInfo instance.",
-    "AuthCodeExchangeError": "Error authenticating with auth code due to: %s",
-    "AuthCodeUsernameRetrievalError": "Could not retrieve the username after successful auth code exchange in org: %s.\nDue to: %s",
-    "JWTAuthError": "Error authenticating with JWT config due to: %s",
-    "RefreshTokenAuthError": "Error authenticating with the refresh token due to: %s",
-    "OrgDataNotAvailableError": "An attempt to refresh the authentication token failed with a 'Data Not Found Error'. The org identified by username %s does not appear to exist. Likely cause is that the org was deleted by another user or has expired.",
-    "OrgDataNotAvailableErrorAction1": "Run `sfdx force:org:list --clean` to remove stale org authentications.",
-    "OrgDataNotAvailableErrorAction2": "Use `sfdx force:config:set` to update the defaultusername.",
-    "OrgDataNotAvailableErrorAction3": "Use `sfdx force:org:create` to create a new org.",
-    "OrgDataNotAvailableErrorAction4": "Use `sfdx force:auth` to authenticate an existing org.",
-    "NamedOrgNotFound": "No AuthInfo found for name %s",
-    "NoAliasesFound": "Nothing to set",
-    "InvalidFormat": "Setting aliases must be in the format <key>=<value> but found: [%s]",
-    "NoAuthInfoFound": "No authorization information can be found.",
-    "InvalidJsonCasing": "All JSON input must have heads down camelcase keys.  E.g., { sfdcLoginUrl: \"https://login.salesforce.com\" }\nFound \"%s\" at %s"
-}
+  "JsonParseError": "Parse error in file %s on line %s\n%s\n",
+  "AuthInfoCreationError": "Must pass a username and/or OAuth options when creating an AuthInfo instance.",
+  "AuthInfoOverwriteError": "Cannot create an AuthInfo instance that will overwrite existing auth data.",
+  "AuthInfoOverwriteErrorAction": "Create the AuthInfo instance using existing auth data by just passing the username.  E.g., AuthInfo.create({ username: 'my@user.org' });",
+  "AuthCodeExchangeError": "Error authenticating with auth code due to: %s",
+  "AuthCodeUsernameRetrievalError": "Could not retrieve the username after successful auth code exchange in org: %s.\nDue to: %s",
+  "JWTAuthError": "Error authenticating with JWT config due to: %s",
+  "RefreshTokenAuthError": "Error authenticating with the refresh token due to: %s",
+  "OrgDataNotAvailableError": "An attempt to refresh the authentication token failed with a 'Data Not Found Error'. The org identified by username %s does not appear to exist. Likely cause is that the org was deleted by another user or has expired.",
+  "OrgDataNotAvailableErrorAction1": "Run `sfdx force:org:list --clean` to remove stale org authentications.",
+  "OrgDataNotAvailableErrorAction2": "Use `sfdx force:config:set` to update the defaultusername.",
+  "OrgDataNotAvailableErrorAction3": "Use `sfdx force:org:create` to create a new org.",
+  "OrgDataNotAvailableErrorAction4": "Use `sfdx force:auth` to authenticate an existing org.",
+  "NamedOrgNotFound": "No AuthInfo found for name %s",
+  "NoAliasesFound": "Nothing to set",
+  "InvalidFormat": "Setting aliases must be in the format <key>=<value> but found: [%s]",
+  "NoAuthInfoFound": "No authorization information can be found.",
+  "InvalidJsonCasing": "All JSON input must have heads down camelcase keys.  E.g., { sfdcLoginUrl: \"https://login.salesforce.com\" }\nFound \"%s\" at %s"
+}

src/authInfo.ts

@@ -568,6 +568,26 @@ export class AuthInfo extends AsyncCreatable<AuthInfo.Options> {
       throw SfdxError.create('@salesforce/core', 'core', 'AuthInfoCreationError');
     }
 
+    // If a username AND oauth options were passed, ensure an auth file for the username doesn't
+    // already exist.  Throw if it does so we don't overwrite the auth file.
+    if (this.options.username && this.options.oauth2Options) {
+      const authInfoConfig = await AuthInfoConfig.create({
+        ...AuthInfoConfig.getOptions(this.options.username),
+        throwOnNotFound: false
+      });
+      if (await authInfoConfig.exists()) {
+        throw SfdxError.create(
+          new SfdxErrorConfig(
+            '@salesforce/core',
+            'core',
+            'AuthInfoOverwriteError',
+            undefined,
+            'AuthInfoOverwriteErrorAction'
+          )
+        );
+      }
+    }
+
     this.fields.username = this.options.username || getString(options, 'username') || undefined;
 
     // If the username is an access token, use that for auth and don't persist

test/unit/authInfoTest.ts

@@ -170,6 +170,8 @@ class MetaAuthDataMock {
       set(configContents, 'accessToken', this.encryptedAccessToken);
       set(configContents, 'privateKey', '123456');
       return Promise.resolve(configContents);
+    } else if (path.includes('_username_RefreshToken') || '_username_SaveTest1') {
+      return Promise.resolve({});
     } else {
       return Promise.reject(new SfdxError('Not mocked - unhandled test case', 'UnsupportedTestCase'));
     }
@@ -475,6 +477,10 @@ describe('AuthInfo', () => {
         expect(authInfoUpdate.called).to.be.true;
         expect(authInfoBuildJwtConfig.called).to.be.true;
         expect(authInfoBuildJwtConfig.firstCall.args[0]).to.include(jwtConfig);
+        expect(
+          testMetadata.authInfoLookupCount === 1,
+          'should have read an auth file once to ensure auth data did not already exist'
+        ).to.be.true;
         expect(readFileStub.called).to.be.true;
         expect(AuthInfoConfig.getOptions(testMetadata.jwtUsername).filename).to.equal(
           `${testMetadata.jwtUsername}.json`
@@ -522,7 +528,7 @@ describe('AuthInfo', () => {
           authInfoBuildJwtConfig.called,
           'should NOT have called AuthInfo.buildJwtConfig() - should get from cache'
         ).to.be.false;
-        expect(testMetadata.authInfoLookupCount === 0, 'should NOT have called Global.fetchConfigInfo() for auth info')
+        expect(testMetadata.authInfoLookupCount === 1, 'should NOT have called Global.fetchConfigInfo() for auth info')
           .to.be.true;
         expect(AuthInfoConfig.getOptions(testMetadata.jwtUsername).filename).to.equal(
           `${testMetadata.jwtUsername}.json`
@@ -576,6 +582,36 @@ describe('AuthInfo', () => {
       expect(AuthInfoConfig.getOptions(username).filename).to.equal(`${username}.json`);
     });
 
+    it('should throw an AuthInfoOverwriteError when both username and oauth data passed and auth file exists', async () => {
+      const username = 'authInfoTest_username_jwt_from_auth_file';
+      const jwtConfig = {
+        clientId: testMetadata.clientId,
+        loginUrl: testMetadata.loginUrl,
+        privateKey: 'authInfoTest/jwt/server.key'
+      };
+
+      // Make the file read stub return JWT auth data
+      const jwtData = {};
+      set(jwtData, 'accessToken', testMetadata.encryptedAccessToken);
+      set(jwtData, 'clientId', testMetadata.clientId);
+      set(jwtData, 'loginUrl', testMetadata.loginUrl);
+      set(jwtData, 'instanceUrl', testMetadata.instanceUrl);
+      set(jwtData, 'privateKey', 'authInfoTest/jwt/server.key');
+      testMetadata.fetchConfigInfo = () => {
+        return Promise.resolve(jwtData);
+      };
+
+      $$.SANDBOX.stub(AuthInfoConfig.prototype, 'exists').returns(Promise.resolve(true));
+
+      // Create the JWT AuthInfo instance
+      try {
+        await AuthInfo.create({ username, oauth2Options: jwtConfig });
+        assert.fail('Error thrown', 'No Error thrown', 'Expected AuthInfo.create() to throw an AuthInfoOverwriteError');
+      } catch (err) {
+        expect(err.name).to.equal('AuthInfoOverwriteError');
+      }
+    });
+
     it('should throw a JWTAuthError when auth fails via a OAuth2.jwtAuthorize()', async () => {
       const username = 'authInfoTest_username_jwt_ERROR1';
       const jwtConfig = {

test/unit/orgTest.ts

@@ -17,6 +17,7 @@ import { tmpdir as osTmpdir } from 'os';
 import { join as pathJoin } from 'path';
 import { AuthFields, AuthInfo } from '../../src/authInfo';
 import { Aliases } from '../../src/config/aliases';
+import { AuthInfoConfig } from '../../src/config/authInfoConfig';
 import { Config } from '../../src/config/config';
 import { ConfigAggregator } from '../../src/config/configAggregator';
 import { ConfigFile } from '../../src/config/configFile';
@@ -336,6 +337,8 @@ describe('Org Tests', () => {
         return Promise.resolve(responseBody);
       });
 
+      $$.SANDBOX.stub(AuthInfoConfig.prototype, 'exists').returns(Promise.resolve(false));
+
       for (const user of users) {
         userAuthResponse = {
           access_token: user.accessToken,

yarn.lock

@@ -287,9 +287,9 @@
     tslint-microsoft-contrib "^5"
     typescript "3.1.6"
 
-"@salesforce/dev-scripts@^0.3.14":
+"@salesforce/dev-scripts@0.3.14":
   version "0.3.14"
-  resolved "https://registry.npmjs.org/@salesforce/dev-scripts/-/dev-scripts-0.3.14.tgz#29ed5afe451ca70b633eb63e45bfad860b32670a"
+  resolved "https://registry.yarnpkg.com/@salesforce/dev-scripts/-/dev-scripts-0.3.14.tgz#29ed5afe451ca70b633eb63e45bfad860b32670a"
   integrity sha512-OQIKPSt1wX/VlzHknn2uXb/lsq9Wpawkur4OyKYmTrqBMqRJnsgqzfolXcPFrbgdQJrm4Dd+DWj6fWhIcrt6tg==
   dependencies:
     "@commitlint/cli" "^7"