UI: Add TOTP secrets engine #29751

lane-wetmore · 2025-02-27T18:31:26Z

Description

This is a culmination of PRs into this sidebranch which have been individually reviewed. These changes include enabling the TOTP secrets engine according to current design specs + tests.

TOTP.secrets.engine.mov

enterprise tests ✅

TODO only if you're a HashiCorp employee

Backport Labels: If this fix needs to be backported, use the appropriate backport/ label that matches the desired release branch. Note that in the CE repo, the latest release branch will look like backport/x.x.x, but older release branches will be backport/ent/x.x.x+ent.
- LTS: If this fixes a critical security vulnerability or severity 1 bug, it will also need to be backported to the current LTS versions of Vault. To ensure this, use all available enterprise labels.
ENT Breakage: If this PR either 1) removes a public function OR 2) changes the signature
of a public function, even if that change is in a CE file, double check that
applying the patch for this PR to the ENT repo and running tests doesn't
break any tests. Sometimes ENT only tests rely on public functions in CE
files.
Jira: If this change has an associated Jira, it's referenced either
in the PR description, commit message, or branch name.
RFC: If this change has an associated RFC, please link it in the description.
ENT PR: If this change has an associated ENT PR, please link it in the
description. Also, make sure the changelog is in this PR, not in your ENT PR.

* TOTP secrets in the web UI * Use credentials route to show TOTP code * Refactor create and edit of TOTP keys * TOTP model validation * Add TOTP provider mode * Use withExpandedAttributes in TOTP model * Simplify attributes * Make TOTP period optional * Better TOTP editor labels * Fix disabled attribute for select and checkbox

github-actions · 2025-02-27T18:39:35Z

CI Results:
All Go tests succeeded! ✅

hellobontempo · 2025-02-27T18:45:38Z

ui/app/adapters/totp.js

+import ApplicationAdapter from './application';
+import { encodePath } from 'vault/utils/path-encoding-helpers';
+
+export default ApplicationAdapter.extend({


Modernize this adapter to match current Ember patterns, confirm each method is actually necessary (and not just copy pasta)

hellobontempo · 2025-02-27T18:55:04Z

ui/app/styles/components/generate-credentials-totp.scss

+
+.code-validity {
+  padding-left: $spacing-12;
+  cursor: unset;


I'm not really sure unset is necessary here? We can double check, but ideally it'd be nice to remove this component file and use generic css spacing helpers for the padding

hellobontempo · 2025-02-27T18:58:25Z

ui/app/components/secret-list/totp-list-item.js

+import Component from '@glimmer/component';
+import { tracked } from '@glimmer/tracking';
+
+export default class SecretListItemComponent extends Component {


component names should match the file

Suggested change

export default class SecretListItemComponent extends Component {

export default class TotpListItem extends Component {

hellobontempo · 2025-02-27T19:00:49Z

ui/app/components/totp-edit.js

+  persist(method, successCallback) {
+    const model = this.model;
+    return model[method]().then(() => {
+      if (!model.isError) {
+        if (model.backend === 'totp' && model.generate) {
+          this.hasGenerated = true;
+          this.successCallback = successCallback;
+        } else {
+          successCallback(model);
+        }
+      }
+    });
+  }


Writing a delete method specific for this item should remove the need for some conditionals here. But I'd still like to confirm what the intended functionality is here. Is this called when clicking "Delete" from the popup menu?

Also - why is hasGenerated something that needs to be tracked here?

hellobontempo · 2025-02-27T19:02:41Z

ui/app/models/totp.js

+  })
+  backend;
+
+  @alias('account_name') name;


I think the original intention here was to use account_name as name, but reviewing the docs they're separate parameters and some users may not want to use the same value for both.

hellobontempo · 2025-02-27T19:02:54Z

ui/app/models/totp.js

+
+@withModelValidations(validations)
+@withExpandedAttributes()
+export default class TotpModel extends Model {


We want to audit the params here and make sure they match the docs: https://developer.hashicorp.com/vault/api-docs/secret/totp

hellobontempo · 2025-02-27T19:05:12Z

ui/app/models/totp.js

+    fieldValue: 'name',
+    editDisabled: true,
+  })
+  account_name;


we're inconsistent but typically use camel case syntax for model params accountName

hellobontempo · 2025-02-27T19:08:59Z

ui/app/models/totp.js

+  @computed('generate', function () {
+    const defaultFields = ['generate'];
+    const options = ['algorithm', 'digits', 'period'];
+    const providerOptions = [];
+
+    if (this.generate) {
+      providerOptions.push('key_size', 'skew', 'exported', 'qr_size');
+    } else {
+      defaultFields.push('url', 'key');
+    }
+
+    defaultFields.push('account_name', 'issuer');
+
+    const groups = [
+      { default: defaultFields },
+      {
+        Options: [...options],
+      },
+    ];
+
+    if (this.generate) {
+      groups.push({
+        'Provider options': [...providerOptions],
+      });
+    }
+
+    return this._expandGroups(groups);
+  })
+  fieldGroups;


The newer pattern is to keep models "thin" and any form logic should be handled by the form component, not the model. Instead, we can use the @withFormFields decorator which will "expand" all of the model attributes into the form structure we want (with the options object that houses editType and all of that info).

The decorator can be called with args, like this: https://github.com/hashicorp/vault/blob/main/ui/app/models/pki/issuer.js#L27

hellobontempo · 2025-02-27T19:20:18Z

ui/lib/core/addon/components/form-field.hbs

@@ -335,7 +336,7 @@
  {{else if (or (eq @attr.type "boolean") (eq @attr.options.editType "boolean"))}}
    <div class="b-checkbox">
      <input
-        disabled={{this.disabled}}
+        disabled={{and @attr.options.editDisabled (not @model.isNew)}}


looking at the disabled getter in the component file shows that a @disabled arg can be passed in. we still need to account for that case and this logic should be added to the getter

We could then call this.disabled here and above. It might be worth updating the other areas of and @attr.options.editDisabled (not @model.isNew) so we're consistent (though as it is, not all inputs are even setup to accept a disabled arg...so the consistency is somewhat moot) This component file is always a challenge to balance keeping scope down without breaking backwards compatibility for an arg.

hellobontempo · 2025-02-27T19:31:42Z

ui/app/components/generate-credentials-totp.js

+import Component from '@glimmer/component';
+import { action } from '@ember/object';
+import { tracked } from '@glimmer/tracking';
+import { later, cancel } from '@ember/runloop';


we should use ember-concurrency here instead as we want to move away from ember/runloop (docs)

there are some examples in the codebase but here are the docs

hellobontempo · 2025-02-27T19:34:16Z

ui/app/templates/components/generate-credentials-totp.hbs

this file should be co-located and moved beside the generate-credentials-totp.js file. (the separate file trees was necessary pre ember-octane)

hellobontempo · 2025-02-27T19:37:28Z

ui/app/templates/components/secret-list/totp-list-item.hbs

this file should be co-located and live beside totp-list-item.js (we'll want to double check that doesn't message with the rendering expectations for the generic <Component> helper the file path will be the same (secret-list/totp-list-item), so it should be fine

hellobontempo · 2025-02-27T19:38:11Z

ui/app/templates/components/totp-edit.hbs

this should be co-located as well (we'll want to double check that doesn't mess with the generator pattern and how these are rendered)

hellobontempo · 2025-02-27T19:49:44Z

ui/app/templates/components/generate-credentials-totp.hbs

+  {{did-insert this.startTimer}}
+  {{will-destroy this.cancelTimer}}


with ember concurrency we shouldn't need these. the task can be initiated in the constructor() for the component and are automatically canceled when the component is torn down.

hellobontempo · 2025-02-27T19:58:29Z

ui/app/models/totp.js

+  algorithm;
+  @attr('number', {


nit: line break between each @attr

Suggested change

algorithm;

@attr('number', {

algorithm;

@attr('number', {

hellobontempo · 2025-02-27T19:59:16Z

ui/app/models/totp.js

+  @attr('string', {
+    editDisabled: true,
+  })
+  issuer;


small attr options blocks can be on a single line

Suggested change

@attr('string', {

editDisabled: true,

})

issuer;

@attr('string', { editDisabled: true })

issuer;

hellobontempo · 2025-02-27T20:01:00Z

ui/app/models/totp.js

+  @attr('number', {
+    possibleValues: SKEW,
+    //defaultValue: 1,
+    editDisabled: true,


it seems like edit is disabled for all of these? we just hide disable edit higher up

* a few changes to use newer extends pattern * refactor totp adapter * refactor generate-credentials-totp * update component name * refactor totp-edit * update naming from totp to totp-key for clarity and convention * update to use totp-key instead of totp * break totp actions into separate components * remove support for editing totp key * move new totp code logic into generate credentials totp (fixes delay in new code) * move hbs files to be alongside their js counterparts * refactor totp model and update form fields decorator to handle combined groups * Update ui/app/components/generate-credentials-totp.hbs Co-authored-by: claire bontempo <68122737+hellobontempo@users.noreply.github.com> * remove unnecessary update method and improve method separation of concerns * Update ui/app/components/generate-credentials-totp.js Co-authored-by: claire bontempo <68122737+hellobontempo@users.noreply.github.com> * pass only relevant model data into component * update totp credentials * url improvement * remove unnecessary cleanup method and move totp code generation * add todo * remove old totp code refresh * update directory location and name * combine conditionals * remove unused code * add todos * update model id assignment * move field display logic out of model * combine display conditional blocks * remove TOTP from unsupported engines list --------- Co-authored-by: claire bontempo <68122737+hellobontempo@users.noreply.github.com>

* update getter to return non generated or generated form fields * update field text and labels to match design * only show exported for generate and fix toggle * add model validations and invalid form alert * update validations to match doc specifications * fix key validation * fix validations * add totp list item test * add tests for creating vault provided key, navigating to details, and deleting key * add jsdoc for totp-edit * remove todo * update jsdoc and convert create method to task * fix stying * make delete method async and remove persist * make method names more clear * remove extra div * remove unneeded test selector * update validators and messages * use existing toggle button type * add withExpandedAttributes back in * move common logic into before each hook * add test for non vault generated key * use general selectors * test improvements * test improvements * add success messages * update assertion message * update mountBackend to navigate to relevant page if not already there * update doc * fix model inconsistencies and update serializer * remove extra css and reverse progress bar for totp code * update naming * pass group fields to serializer as well * update tests * remove mode and update naming for clarity * remove comment * remove unneeded css file * remove unneeded mode getter, update docs, update delete fn and message * update reset to be more resilient and rename arg * update field naming for clarity and add back in namespace reminder * remove unneeded div, update test selector, clean up * clean up strings, remove unneeded labels, * remove aliases for capabilities * update and fix tests * remove unneeded test file

* match read key and generate code flow to design * add qr code view to acceptance tests * add component tests * remove unneeded variables * add backend and id * updated tests * flash error message in delete key * move test selector * add generate code link from details view * update back button to be dynamic * Update ui/tests/acceptance/secrets/backend/totp/key-test.js Co-authored-by: Shannon Roberts (Beagin) <beagins@users.noreply.github.com> * save id for success message and update routing for back button --------- Co-authored-by: Shannon Roberts (Beagin) <beagins@users.noreply.github.com>

github-actions · 2025-04-15T17:37:38Z

Build Results:
All builds succeeded! ✅

Monkeychip · 2025-04-17T15:06:15Z

ui/app/components/generate-credentials-totp.js

+    const { totpCodePeriod } = this.args;
+
+    if (!totpCodePeriod) {
+      return 0; // TODO improve this


Does this TODO need to be addressed before we merge?

I don't believe so. I think that code block shouldn't be possible to reach anymore as we should always have a totpCodePeriod, but I left it in to be safe for now.

Monkeychip

I have not been too involved in this sidebranch process. There are a couple of small changes I was considering recommending, but I understand the background context to this sidebranch and know at this point it's about getting this functionality out to users. Nice work on this!

mpflanzer and others added 2 commits February 13, 2025 12:50

Merge branch 'main' into ui-totp-sidebranch

98b3e31

github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Feb 27, 2025

lane-wetmore added the ui label Feb 27, 2025

vercel bot deployed to Preview February 27, 2025 18:38 View deployment

hellobontempo reviewed Feb 27, 2025

View reviewed changes

lane-wetmore and others added 2 commits March 7, 2025 15:38

Merge branch 'main' into ui-totp-sidebranch

6e3a95a

vercel bot deployed to Preview March 7, 2025 21:46 View deployment

Merge branch 'main' into ui-totp-sidebranch

86f6df1

vercel bot deployed to Preview March 13, 2025 20:17 View deployment

lane-wetmore added 2 commits March 21, 2025 11:02

Merge branch 'main' into ui-totp-sidebranch

f1d8c4a

vercel bot deployed to Preview March 24, 2025 19:00 View deployment

Merge branch 'main' into ui-totp-sidebranch

e8305ad

vercel bot deployed to Preview April 1, 2025 21:36 View deployment

lane-wetmore and others added 2 commits April 15, 2025 12:30

Merge branch 'main' into ui-totp-sidebranch

d62865d

lane-wetmore marked this pull request as ready for review April 15, 2025 17:34

lane-wetmore requested a review from a team as a code owner April 15, 2025 17:34

vercel bot deployed to Preview April 15, 2025 17:40 View deployment

lane-wetmore modified the milestones: 1.19.2, 1.20.0-rc Apr 15, 2025

lane-wetmore added 2 commits April 16, 2025 11:04

Merge branch 'main' into ui-totp-sidebranch

d44f875

add changelog

c9ccbb6

lane-wetmore requested a review from a team as a code owner April 16, 2025 16:21

lane-wetmore requested review from anurag925 and removed request for anurag925 April 16, 2025 16:21

lane-wetmore added 2 commits April 16, 2025 11:24

update changelog

802bd5e

remove space

5a2d096

Monkeychip reviewed Apr 17, 2025

View reviewed changes

Monkeychip approved these changes Apr 17, 2025

View reviewed changes

lane-wetmore merged commit 55674dd into main Apr 17, 2025
33 checks passed

lane-wetmore deleted the ui-totp-sidebranch branch April 17, 2025 17:59

hellobontempo mentioned this pull request Jun 30, 2025

UI: Yield HDS dropdown text #31142

Merged

6 tasks

	export default class SecretListItemComponent extends Component {
	export default class TotpListItem extends Component {

		{{did-insert this.startTimer}}
		{{will-destroy this.cancelTimer}}

UI: Add TOTP secrets engine #29751

UI: Add TOTP secrets engine #29751

Uh oh!

Conversation

lane-wetmore commented Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

TODO only if you're a HashiCorp employee

Uh oh!

github-actions bot commented Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hellobontempo Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hellobontempo Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hellobontempo Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hellobontempo Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hellobontempo Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Monkeychip left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lane-wetmore commented Feb 27, 2025 •

edited

Loading

github-actions bot commented Feb 27, 2025 •

edited

Loading

hellobontempo Feb 27, 2025 •

edited

Loading

hellobontempo Feb 27, 2025 •

edited

Loading

hellobontempo Feb 27, 2025 •

edited

Loading

hellobontempo Feb 27, 2025 •

edited

Loading

hellobontempo Feb 27, 2025 •

edited

Loading