PolyGerrit: don't show current HTTP password With the move to NoteDB, the per-account data (including the HTTP password) will be stored in a user-branch in the All-Users repo, where it is subject to Gerrit ACLs. Since these are notoriously hard to set up correctly, we want to avoid storing the password in plaintext. The simplest solution is to store the password in hashed form, which precludes showing the current passwords in the settings UI. This change removes the password GET call from PolyGerrit. Bug: Issue 5373 Change-Id: I4f93873bdee0b82aaaf85090d0aa271a94ea8e17
diff --git a/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password.html b/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password.html index e58f1f2..d96237e 100644 --- a/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password.html +++ b/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password.html
@@ -15,7 +15,9 @@ --> <link rel="import" href="../../../bower_components/polymer/polymer.html"> +<link rel="import" href="../../../styles/gr-settings-styles.html"> <link rel="import" href="../../shared/gr-button/gr-button.html"> +<link rel="import" href="../../shared/gr-overlay/gr-overlay.html"> <link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html"> <dom-module id="gr-http-password"> @@ -24,9 +26,24 @@ .password { font-family: var(--monospace-font-family); } - .noPassword { - color: #777; + #generatedPasswordOverlay { + padding: 2em; + width: 50em; + } + #generatedPasswordDisplay { + margin: 1em 0; + } + #generatedPasswordDisplay .value { + font-family: var(--monospace-font-family); + } + #passwordWarning { font-style: italic; + text-align: center; + } + .closeButton { + bottom: 2em; + position: absolute; + right: 2em; } </style> <style include="gr-settings-styles"></style> @@ -35,28 +52,28 @@ <span class="title">Username</span> <span class="value">[[_username]]</span> </section> - <section> - <span class="title">Password</span> - <span hidden$="[[!_hasPassword]]"> - <span class="value" hidden$="[[_passwordVisible]]"> - <gr-button - link - on-tap="_handleViewPasswordTap">Click to view</gr-button> - </span> - <span - class="value password" - hidden$="[[!_passwordVisible]]">[[_password]]</span> - </span> - <span class="value noPassword" hidden$="[[_hasPassword]]">(None)</span> - </section> <gr-button id="generateButton" on-tap="_handleGenerateTap">Generate New Password</gr-button> - <gr-button - id="clearButton" - on-tap="_handleClearTap" - disabled="[[!_hasPassword]]">Clear Password</gr-button> </div> + <gr-overlay + id="generatedPasswordOverlay" + on-iron-overlay-closed="_generatedPasswordOverlayClosed" + with-backdrop> + <div class="gr-settings-styles"> + <section id="generatedPasswordDisplay"> + <span class="title">New Password:</span> + <span class="value">[[_generatedPassword]]</span> + </section> + <section id="passwordWarning"> + This password will not be displayed again.<br> + If you lose it, you will need to generate a new one. + </section> + <gr-button + class="closeButton" + on-tap="_closeOverlay">Close</gr-button> + </div> + </gr-overlay> <gr-rest-api-interface id="restAPI"></gr-rest-api-interface> </template> <script src="gr-http-password.js"></script>
diff --git a/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password.js b/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password.js index 9248632..bde36aa 100644 --- a/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password.js +++ b/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password.js
@@ -17,65 +17,31 @@ Polymer({ is: 'gr-http-password', - /** - * Fired when getting the password fails with non-404. - * - * @event network-error - */ - properties: { - _serverConfig: Object, _username: String, - _password: String, - _passwordVisible: { - type: Boolean, - value: false, - }, - _hasPassword: Boolean, + _generatedPassword: String, }, loadData: function() { - var promises = []; - - promises.push(this.$.restAPI.getAccount().then(function(account) { + return this.$.restAPI.getAccount().then(function(account) { this._username = account.username; - }.bind(this))); - - promises.push(this.$.restAPI - .getAccountHttpPassword(this._handleGetPasswordError.bind(this)) - .then(function(pass) { - this._password = pass; - this._hasPassword = !!pass; - }.bind(this))); - - return Promise.all(promises); - }, - - _handleGetPasswordError: function(response) { - if (response.status === 404) { - this._hasPassword = false; - } else { - this.fire('network-error', {response: response}); - } - }, - - _handleViewPasswordTap: function() { - this._passwordVisible = true; + }.bind(this)); }, _handleGenerateTap: function() { + this._generatedPassword = 'Generating...'; + this.$.generatedPasswordOverlay.open(); this.$.restAPI.generateAccountHttpPassword().then(function(newPassword) { - this._hasPassword = true; - this._passwordVisible = true; - this._password = newPassword; + this._generatedPassword = newPassword; }.bind(this)); }, - _handleClearTap: function() { - this.$.restAPI.deleteAccountHttpPassword().then(function() { - this._password = ''; - this._hasPassword = false; - }.bind(this)); + _closeOverlay: function() { + this.$.generatedPasswordOverlay.close(); + }, + + _generatedPasswordOverlayClosed: function() { + this._generatedPassword = null; }, }); })();
diff --git a/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password_test.html b/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password_test.html index 36c9abf..e675de2 100644 --- a/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password_test.html +++ b/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password_test.html
@@ -31,7 +31,7 @@ </test-fixture> <script> - suite('gr-http-password tests (already has password)', function() { + suite('gr-http-password tests', function() { var element; var account; var password; @@ -51,106 +51,30 @@ element.loadData().then(function() { flush(done); }); }); - test('loads data', function() { - assert.equal(element._username, 'user name'); - assert.equal(element._password, 'the password'); - assert.isFalse(element._passwordVisible); - assert.isTrue(element._hasPassword); - }); - - test('view password', function() { - var button = element.$$('.value gr-button'); - assert.isFalse(element._passwordVisible); - MockInteractions.tap(button); - assert.isTrue(element._passwordVisible); - }); - test('generate password', function() { var button = element.$.generateButton; var nextPassword = 'the new password'; + var generateResolve; var generateStub = sinon.stub(element.$.restAPI, 'generateAccountHttpPassword', function() { - return Promise.resolve(nextPassword); + return new Promise(function(resolve) { + generateResolve = resolve; + }); }); - assert.isTrue(element._hasPassword); - assert.isFalse(element._passwordVisible); - assert.equal(element._password, 'the password'); + assert.isNotOk(element._generatedPassword); MockInteractions.tap(button); assert.isTrue(generateStub.called); + assert.equal(element._generatedPassword, 'Generating...'); + + generateResolve(nextPassword); + generateStub.lastCall.returnValue.then(function() { - assert.isTrue(element._passwordVisible); - assert.isTrue(element._hasPassword); - assert.equal(element._password, 'the new password'); - }); - }); - - test('clear password', function() { - var button = element.$.clearButton; - var clearStub = sinon.stub(element.$.restAPI, 'deleteAccountHttpPassword', - function() { return Promise.resolve(); }); - - assert.isTrue(element._hasPassword); - assert.equal(element._password, 'the password'); - - MockInteractions.tap(button); - - assert.isTrue(clearStub.called); - clearStub.lastCall.returnValue.then(function() { - assert.isFalse(element._hasPassword); - assert.equal(element._password, ''); + assert.equal(element._generatedPassword, nextPassword); }); }); }); - suite('gr-http-password tests (has no password)', function() { - var element; - var account; - - setup(function(done) { - account = {username: 'user name'}; - - stub('gr-rest-api-interface', { - getAccount: function() { return Promise.resolve(account); }, - getAccountHttpPassword: function(errFn) { - errFn({status: 404}); - return Promise.resolve(''); - }, - }); - - element = fixture('basic'); - element.loadData().then(function() { flush(done); }); - }); - - test('loads data', function() { - assert.equal(element._username, 'user name'); - assert.isNotOk(element._password); - assert.isFalse(element._passwordVisible); - assert.isFalse(element._hasPassword); - }); - - test('generate password', function() { - var button = element.$.generateButton; - var nextPassword = 'the new password'; - var generateStub = sinon.stub(element.$.restAPI, - 'generateAccountHttpPassword', function() { - return Promise.resolve(nextPassword); - }); - - assert.isFalse(element._hasPassword); - assert.isFalse(element._passwordVisible); - assert.isNotOk(element._password); - - MockInteractions.tap(button); - - assert.isTrue(generateStub.called); - generateStub.lastCall.returnValue.then(function() { - assert.isTrue(element._passwordVisible); - assert.isOk(element._hasPassword); - assert.equal(element._password, 'the new password'); - }); - }); - }); </script>