Merge pull request #125 from forcedotcom/develop

merge develop into master

src/config/aliases.ts

@@ -8,6 +8,7 @@
 import { asString, Dictionary, JsonMap, Optional } from '@salesforce/ts-types';
 import { SfdxError } from '../sfdxError';
 import { ConfigGroup } from './configGroup';
+import { ConfigContents, ConfigValue } from './configStore';
 
 const ALIAS_FILE_NAME = 'alias.json';
 
@@ -102,4 +103,9 @@ export class Aliases extends ConfigGroup<ConfigGroup.Options> {
   public constructor(options: ConfigGroup.Options) {
     super(options);
   }
+
+  // Don't use kit's set to prevent nested object save
+  protected setMethod(contents: ConfigContents, key: string, value?: ConfigValue) {
+    contents[key] = value;
+  }
 }

src/config/configGroup.ts

@@ -5,7 +5,6 @@
  * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
 
-import { set } from '@salesforce/kit';
 import { definiteEntriesOf, definiteValuesOf, Dictionary, getJsonMap, JsonMap, Optional } from '@salesforce/ts-types';
 import { SfdxError } from '../sfdxError';
 import { ConfigFile } from './configFile';
@@ -43,7 +42,7 @@ export class ConfigGroup<T extends ConfigGroup.Options> extends ConfigFile<T> {
     return configGroupOptions;
   }
 
-  private defaultGroup = 'default';
+  protected defaultGroup = 'default';
 
   /**
    * Sets the default group for all {@link BaseConfigStore} methods to use.
@@ -213,7 +212,7 @@ export class ConfigGroup<T extends ConfigGroup.Options> extends ConfigFile<T> {
       super.set(group, {});
     }
     content = this.getGroup(group) || {};
-    set(content, key, value);
+    this.setMethod(content, key, value);
 
     return content;
   }

src/config/configStore.ts

@@ -119,7 +119,7 @@ export abstract class BaseConfigStore<T extends BaseConfigStore.Options> extends
    * @param value The value.
    */
   public set(key: string, value: ConfigValue): ConfigContents {
-    set(this.contents, key, value);
+    this.setMethod(this.contents, key, value);
     return this.contents;
   }
 
@@ -212,9 +212,15 @@ export abstract class BaseConfigStore<T extends BaseConfigStore.Options> extends
   public setContentsFromObject<U extends object>(obj: U): void {
     this.contents = {};
     Object.entries(obj).forEach(([key, value]) => {
-      set(this.contents, key, value);
+      this.setMethod(this.contents, key, value);
     });
   }
+
+  // Allows extended classes the ability to override the set method. i.e. maybe they don't want
+  // nexted object set from kit.
+  protected setMethod(contents: ConfigContents, key: string, value?: ConfigValue) {
+    set(contents, key, value);
+  }
 }
 
 /**

src/keyChainImpl.ts

@@ -565,7 +565,27 @@ export class GenericUnixKeychainAccess extends GenericKeychainAccess {
 /**
  * @ignore
  */
-export class GenericWindowsKeychainAccess extends GenericKeychainAccess {}
+export class GenericWindowsKeychainAccess extends GenericKeychainAccess {
+  protected async isValidFileAccess(cb: (error: Nullable<Error>) => Promise<void>): Promise<void> {
+    await super.isValidFileAccess(async err => {
+      if (err != null) {
+        await cb(err);
+      } else {
+        try {
+          const secretFile: string = path.join(
+            await ConfigFile.resolveRootFolder(true),
+            Global.STATE_FOLDER,
+            ensure(KeychainConfig.getDefaultOptions().filename)
+          );
+          await fs.access(secretFile, fs.constants.R_OK | fs.constants.W_OK);
+          await cb(null);
+        } catch (e) {
+          await cb(err);
+        }
+      }
+    });
+  }
+}
 
 /**
  * @ignore

src/logger.ts

@@ -655,6 +655,7 @@ type FilteredKey = string | { name: string; regex: string };
 // Ok to log clientid
 const FILTERED_KEYS: FilteredKey[] = [
   'sid',
+  'Authorization',
   // Any json attribute that contains the words "access" and "token" will have the attribute/value hidden
   { name: 'access_token', regex: 'access[^\'"]*token' },
   // Any json attribute that contains the words "refresh" and "token" will have the attribute/value hidden

src/sfdxProject.ts

@@ -33,6 +33,8 @@ import { sfdc } from './util/sfdc';
  * **See** [force:project:create](https://developer.salesforce.com/docs/atlas.en-us.sfdx_dev.meta/sfdx_dev/sfdx_dev_ws_create_new.htm)
  */
 export class SfdxProjectJson extends ConfigFile<ConfigFile.Options> {
+  public static BLACKLIST = ['packageAliases'];
+
   public static getFileName() {
     return SFDX_PROJECT_JSON;
   }
@@ -51,13 +53,23 @@ export class SfdxProjectJson extends ConfigFile<ConfigFile.Options> {
     const contents = await super.read();
 
     // Verify that the configObject does not have upper case keys; throw if it does.  Must be heads down camel case.
-    const upperCaseKey = sfdc.findUpperCaseKeys(this.toObject());
+    const upperCaseKey = sfdc.findUpperCaseKeys(this.toObject(), SfdxProjectJson.BLACKLIST);
     if (upperCaseKey) {
       throw SfdxError.create('@salesforce/core', 'core', 'InvalidJsonCasing', [upperCaseKey, this.getPath()]);
     }
     return contents;
   }
 
+  public async write(newContents?: ConfigContents): Promise<ConfigContents> {
+    // Verify that the configObject does not have upper case keys; throw if it does.  Must be heads down camel case.
+    const upperCaseKey = sfdc.findUpperCaseKeys(newContents, SfdxProjectJson.BLACKLIST);
+    if (upperCaseKey) {
+      throw SfdxError.create('@salesforce/core', 'core', 'InvalidJsonCasing', [upperCaseKey, this.getPath()]);
+    }
+
+    return super.write(newContents);
+  }
+
   public getDefaultOptions(options?: ConfigFile.Options): ConfigFile.Options {
     const defaultOptions: ConfigFile.Options = {
       isState: false

src/testSetup.ts

@@ -60,7 +60,7 @@ export interface ConfigStub {
    * A function that controls all aspects of {@link ConfigFile.write}. For example, it won't read the contents unless
    * explicitly done. Only use this if you know what you are doing. Use updateContents instead.
    */
-  writeFn?: () => Promise<void>;
+  writeFn?: (contents: AnyJson) => Promise<void>;
   /**
    * The contents that are used when @{link ConfigFile.read} unless retrieveContents is set. This will also contain the
    * new config when @{link ConfigFile.write} is called. This will persist through config instances,
@@ -291,7 +291,7 @@ const _testSetup = (sinon?: any) => {
       if (request === `${this.instanceUrl}/services/data`) {
         return Promise.resolve([{ version: '42.0' }]);
       }
-      return testContext.fakeConnectionRequest.call(this, request, options);
+      return testContext.fakeConnectionRequest.call(this, request, options as AnyJson);
     });
 
     // Always start with the default and tests beforeEach or it methods can override it.

src/util/sfdc.ts

@@ -90,13 +90,17 @@ export const sfdc = {
    * Returns the first key within the object that has an upper case first letter.
    *
    * @param data The object in which to check key casing.
+   * @param sectionBlacklist properties in the object to exclude from the search. e.g. a blacklist of `["a"]` and data of `{ "a": { "B" : "b"}}` would ignore `B` because it is in the object value under `a`.
    */
-  findUpperCaseKeys: (data?: JsonMap): Optional<string> => {
+  findUpperCaseKeys: (data?: JsonMap, sectionBlacklist: string[] = []): Optional<string> => {
     let key: Optional<string>;
     findKey(data, (val: AnyJson, k: string) => {
       if (k[0] === k[0].toUpperCase()) {
         key = k;
       } else if (isJsonMap(val)) {
+        if (sectionBlacklist.includes(k)) {
+          return key;
+        }
         key = sfdc.findUpperCaseKeys(asJsonMap(val));
       }
       return key;

test/unit/config/aliasesTest.ts

@@ -41,6 +41,29 @@ describe('Alias', () => {
         orgs: { test: 'val' }
       });
     });
+
+    it('supports aliases with dots', async () => {
+      const aliases = await Aliases.create(Aliases.getDefaultOptions());
+      aliases.set('test.with.dots', 'val');
+      await aliases.write();
+      expect(sinon.assert.calledOnce(ConfigGroup.prototype.write as sinon.SinonSpy));
+      expect($$.getConfigStubContents('Aliases')).to.deep.equal({
+        orgs: { 'test.with.dots': 'val' }
+      });
+    });
+  });
+
+  it('gets aliases with dots', async () => {
+    const testContents = {
+      contents: {
+        orgs: { 'test.with.dots': 'val' }
+      }
+    };
+
+    $$.setConfigStubContents('Aliases', testContents);
+
+    const aliases = await Aliases.create(Aliases.getDefaultOptions());
+    expect(aliases.get('test.with.dots')).to.equal('val');
   });
 
   describe('#unset', () => {

test/unit/keyChainTest.ts

@@ -4,11 +4,14 @@
  * SPDX-License-Identifier: BSD-3-Clause
  * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
+import { Nullable } from '@salesforce/ts-types';
 import { expect } from 'chai';
 import * as _ from 'lodash';
+import * as path from 'path';
 import { retrieveKeychain } from '../../src/keyChain';
 import { GenericUnixKeychainAccess, GenericWindowsKeychainAccess, keyChainImpl } from '../../src/keyChainImpl';
 import { testSetup } from '../../src/testSetup';
+import { fs } from '../../src/util/fs';
 
 // Setup the test environment.
 const $$ = testSetup();
@@ -68,4 +71,39 @@ describe('keyChain', () => {
     expect(windowsKeychain).to.be.instanceOf(GenericWindowsKeychainAccess);
     expect(win32Keychain).to.be.instanceOf(GenericWindowsKeychainAccess);
   });
+
+  it('no key.json file should result in a PasswordNotFoundError', async () => {
+    const TEST_PASSWORD = 'foo';
+    const windowsKeychain = await retrieveKeychain('windows');
+
+    const accessSpy = $$.SANDBOX.spy(fs, 'access');
+    const writeFileStub = $$.SANDBOX.stub(fs, 'writeFile').returns(Promise.resolve(null));
+    const mkdirpStub = $$.SANDBOX.stub(fs, 'mkdirp').returns(Promise.resolve());
+
+    const service = 'sfdx';
+    const account = 'local';
+
+    return windowsKeychain
+      .getPassword({ service, account }, (getPasswordError: Nullable<Error>) => {
+        expect(getPasswordError).have.property('name', 'PasswordNotFoundError');
+        expect(accessSpy.called).to.be.true;
+        const arg: string = accessSpy.args[0][0];
+        expect(arg.endsWith('.sfdx')).to.be.true;
+        accessSpy.resetHistory();
+      })
+      .then(() =>
+        windowsKeychain.setPassword(
+          { service, account, password: TEST_PASSWORD },
+          (setPasswordError: Nullable<Error>) => {
+            expect(setPasswordError).to.be.null;
+            expect(mkdirpStub.called).to.be.true;
+            expect(writeFileStub.called).to.be.true;
+            expect(writeFileStub.args).to.have.length(1);
+            expect(writeFileStub.args[0]).to.have.length(3);
+            expect(writeFileStub.args[0][0]).to.include(path.join('.sfdx', 'key.json'));
+            expect(writeFileStub.args[0][1]).to.include(TEST_PASSWORD);
+          }
+        )
+      );
+  });
 });

test/unit/projectTest.ts

@@ -19,6 +19,20 @@ describe('SfdxProject', async () => {
   beforeEach(async () => {
     projectPath = await $$.localPathRetriever($$.id);
   });
+
+  describe('json', async () => {
+    it('allows uppercase packaging aliases on write', async () => {
+      const json = await SfdxProjectJson.create({});
+      await json.write({ packageAliases: { MyName: 'somePackage' } });
+      expect($$.getConfigStubContents('SfdxProjectJson').packageAliases['MyName']).to.equal('somePackage');
+    });
+    it('allows uppercase packaging aliases on read', async () => {
+      $$.setConfigStubContents('SfdxProjectJson', { contents: { packageAliases: { MyName: 'somePackage' } } });
+      const json = await SfdxProjectJson.create({});
+      expect(json.get('packageAliases')['MyName']).to.equal('somePackage');
+    });
+  });
+
   describe('resolve', () => {
     it('with working directory', async () => {
       $$.SANDBOX.stub(internal, 'resolveProjectPath').callsFake(() => projectPath);

test/unit/util/sfdcTest.ts

@@ -124,5 +124,14 @@ describe('util/sfdc', () => {
       };
       expect(sfdc.findUpperCaseKeys(testObj)).to.be.undefined;
     });
+
+    it('should return the first nested upper case key unless blacklisted', () => {
+      const testObj = {
+        lowercase: true,
+        uppercase: false,
+        nested: { NestedUpperCase: true }
+      };
+      expect(sfdc.findUpperCaseKeys(testObj, ['nested'])).to.equal(undefined);
+    });
   });
 });