TOTP secrets in the web UI

[mpflanzer]

Extends the UI to create, delete TOTP token generators and read TOTP tokens. The details view for an account refreshes automatically to generate the next token.

The "provider" mode of the TOTP engine is currently not supported. This could easily be added in another PR if there is a use case for it.

Closes: #12698 and #16786

[hashicorp-cla-app]

All committers have signed the CLA.

[mpflanzer]

[mpflanzer]

I haven't added any test yet but can do if required. But it would be great to get a first review if the approach in general is fine.

[hashishaw]

Wow! 🤩 Amazing start on this, and kudos for navigating the notoriously difficult secrets area of the codebase!

Overall, I think your approach for the TOTP keys makes sense, as it closely follows CRUD. However, for the code workflow I think we should copy a combination of AWS and Database:

• list view -- rename "Accounts" to "Keys" to better match documentation
• click on item takes you to /vault/secrets/totp/credentials/:keyName
• in the credentials route, if the backend type is totp we go ahead and get the credentials and show them on a credentials view, similar to how database credentials works
• From the keys list view, you can click the popup menu to access show or edit pages for the key details

For the code generation and verification, since the payload and responses are both fairly small it might be worth removing the code-totp model/adapter and using a custom adapter method for those endpoints instead.

The "show" and "edit" views should show/edit the TOTP Key data, not the code. The code generation will be moved to the credentials page.

Again, this is a great start and please feel free to tag @hashicorp/vault-ui if you have any questions!

[hashishaw]

We typically define validation on the model (search @withModelValidations in the codebase for examples) and then it would get invoked/read via this.args.model.validate(). Using that pattern should simplify this component a bit!

[mpflanzer]

I added the @withModelValidations decorator but I'm not sure where to call the validate since totp-edit.js extends the role-edit.js. Could I just add the validate() call in the createOrUpdate action or would that break for models without the decorator?

vault/ui/app/components/role-edit.js

Line 69 in 7f0e644

createOrUpdate(type, event) {

[hashishaw]

Even though validation is a POST request, it doesn't create a new item in the CRUD sense, so for validation I would probably make some custom adapter method that could get invoked anywhere like this.store.adapterFor('code-totp').validateCode(backend, code, payload). transit-key adapter has a custom method called keyAction that you could look at as an example of something similar.

[mpflanzer]

Thanks for the detailed feedback. I'll try to find some time this weekend to make the changes according to your suggestions.

[mpflanzer]

I adapted the create/edit and generate workflow to your suggestions @hashishaw. I think I addressed most of your comments. Some open questions that I still have:

To which extend does it make sense to allow editing the key entries?
I don't see the use case to be able to edit keys for which Vault operates as generator. Maybe changing the account name makes sense but probably not the parameters for the generator. In provider mode editing could make sense but once the key is created the two modes cannot be distinguished anymore.

How to deal with the URL vs individual fields to create a new key
In generator mode keys can be created either with the otpauth:// URL or by providing all the required information separately. In provider mode the URL option doesn't exists. One option would be parse the URL in the frontend, populate the individual fields with the extracted values and never send the url parameter to the backend. Or based on which fields are filled the request to the backend is build.

Is the key name equal to the TOTP account_name?
The TOTP model has an account_name attribute which typically would be used as the name for the key. If we want the same we need to at least extract the account name from the otpauth:// URL chooses that option to create a new key.

Support for provider mode?
Do we want to support creating keys in provider mode at all? Doesn't make a big difference. Just after creating a new key the shared secret would need to be displayed. That step doesn't exist for the provider mode. Except for that the two modes cannot be distinguished later on (I think).

@hashicorp/vault-ui

[mpflanzer]

I added the provider mode as well. After the key created the user is presented with the QR code:

[mpflanzer]

The create dialog is still not great because of the ambiguity between the URL field and the other fields.

[mpflanzer]

Is it possible to provide default values for the create/edit form without having to set them here on the model?

[hashishaw]

Sort of -- do you want the default value to be dynamic, or why isn't setting it on the model preferable?

[mpflanzer]

No, the main reason was that everything which is set on the model is currently serialized. But some options are mutually exclusive in the POST request. I just realized that it's probably better to tackle this in the serializer and conditionally remove attributes from the serialized data.
The other reason was that some attributes are only used in the POST request but never returned by the GET request. But with the default values it looks like they got returned from the request as well.

[hashishaw]

Got it! Yes, we don't want to show "default" values that are not actually reflected in the backend. One way we get around this limitation (which we run into fairly often) is to instead set the "initial values" in the createRecord invocation. So in this case, in the credentials route, we would have:

getTOTPCode(backend, keyName) {
    return this.store.adapterFor('totp').generateCode(backend, keyName, { algorithm: 'SHA1' });
  },

with whatever other values we want to set initially, and remove the defaultValue unless this is truly the default value that the backend has. Good question!

[hashishaw]

Thank you, this is excellent progress! Pardon me while I get up to speed on TOTP 😄

To which extend does it make sense to allow editing the key entries? I don't see the use case to be able to edit keys for which Vault operates as generator. Maybe changing the account name makes sense but probably not the parameters for the generator. In provider mode editing could make sense but once the key is created the two modes cannot be distinguished anymore.

Good question -- let me take this back to my team for discussion. I don't think we have any other engines that don't allow edit at all, and according to the API it looks like any of the fields other than name could be updated (whether that's advisable is something different).

How to deal with the URL vs individual fields to create a new key In generator mode keys can be created either with the otpauth:// URL or by providing all the required information separately. In provider mode the URL option doesn't exists. One option would be parse the URL in the frontend, populate the individual fields with the extracted values and never send the url parameter to the backend. Or based on which fields are filled the request to the backend is build.

Another great question. Generally we prefer not to mess too much with the user input, and submit as-is.

For displaying the fields relevant to the mode, we do have some patterns for choosing a type of setup that maybe we could incorporate here. I will talk to our design team, but relying on the user to know what to fill out might be fine for now. Do you know, if you create a key and set url only, does the read response only return with the url value?

Is the key name equal to the TOTP account_name? The TOTP model has an account_name attribute which typically would be used as the name for the key. If we want the same we need to at least extract the account name from the otpauth:// URL chooses that option to create a new key.

It looks like the key's name is different than account_name according to the API docs. Are you saying that in practice, they are usually the same?

Support for provider mode? Do we want to support creating keys in provider mode at all? Doesn't make a big difference. Just after creating a new key the shared secret would need to be displayed. That step doesn't exist for the provider mode. Except for that the two modes cannot be distinguished later on (I think).

Your solution for provider mode seems reasonable, and I think it's good to support it with this work. If the key read response returns generate: true when that flag is set on create then we could distinguish later, but the docs don't seem to be explicit about that detail.

Thanks again for your work on this! I will pull down the latest changes and test, and let you know if there are any other things we should update.

[mpflanzer]

Thank you, this is excellent progress! Pardon me while I get up to speed on TOTP 😄
No worries, most of the details were new to me as well. No need to rush on this. I'll anyway mostly have time on the weekends to address the comments.

To which extend does it make sense to allow editing the key entries? I don't see the use case to be able to edit keys for which Vault operates as generator. Maybe changing the account name makes sense but probably not the parameters for the generator. In provider mode editing could make sense but once the key is created the two modes cannot be distinguished anymore.

Good question -- let me take this back to my team for discussion. I don't think we have any other engines that don't allow edit at all, and according to the API it looks like any of the fields other than name could be updated (whether that's advisable is something different).

I just tried to edit the issuer of a key but that failed with an error about missing information. Also by looking at the backend code (

vault/builtin/logical/totp/path_keys.go

Line 210 in 9af5c5c

func (b *backend) pathKeyCreate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {

) I think there is no real editing. I can just completely replace all the information under a given key name but not edit part of it. In HTTP terms I would describe it as a PUT but not a PATCH API. The problem is that the TOTP secret is not known by the frontend so it cannot send it along with the request to edit. I have the feeling that doesn't play nicely with how editing usually works for the secrets in Vault. If I didn't overlook something I would recommend to disable editing to avoid confusion about what it's doing. Especially since a "delete and create" would achieve the same.

How to deal with the URL vs individual fields to create a new key In generator mode keys can be created either with the otpauth:// URL or by providing all the required information separately. In provider mode the URL option doesn't exists. One option would be parse the URL in the frontend, populate the individual fields with the extracted values and never send the url parameter to the backend. Or based on which fields are filled the request to the backend is build.

Another great question. Generally we prefer not to mess too much with the user input, and submit as-is.

For displaying the fields relevant to the mode, we do have some patterns for choosing a type of setup that maybe we could incorporate here. I will talk to our design team, but relying on the user to know what to fill out might be fine for now. Do you know, if you create a key and set url only, does the read response only return with the url value?

I'm not sure which exact request you're talking about. So I'll just provide the answer for all of them. 😉

• In provider mode (generate=true) the POST request to generate a key returns url and optionally barcode.
• In generator mode (generate=false) the POST request to generate a key returns nothing
• Independent of the mode that was used to generate the key the GET request to read the details for a key never returns the url as this would contain the secret. Only basic information like account_name, alogrithm, digits are reported.

If generate is false if a URL is provided all information contained in it takes precedence over the other fields. But things missing from an incomplete URL will be used from the fields. So we could just say we have the field for the URL and all other fields and maybe add a short description for the user how to fill it.

Is the key name equal to the TOTP account_name? The TOTP model has an account_name attribute which typically would be used as the name for the key. If we want the same we need to at least extract the account name from the otpauth:// URL chooses that option to create a new key.

It looks like the key's name is different than account_name according to the API docs. Are you saying that in practice, they are usually the same?

Yes, I would say so. For example in the screenshot from FreeOTP the accounts are just identified by information like account name and issuer. I'm not sure if you could add a custom key name as well or edit some of the information.
I don't have a very strong opinion of how we should do it. The question is mostly for me to know how I should implement it. I could imagine the following scenarios:

• A separate and mandatory field for the key name
• A separate but optional field for the key name. If the user doesn't fill it the information from the other fields is used.
• No extra field for the key name and it is always derived from the other fields.

The last two options might come with some issues to transform the values into a valid key name. I'm not sure if all characters are allowed as part of the key name. And if we allow to edit the account_name later it would anyway diverge from the key name as that wouldn't change I assume.

Support for provider mode? Do we want to support creating keys in provider mode at all? Doesn't make a big difference. Just after creating a new key the shared secret would need to be displayed. That step doesn't exist for the provider mode. Except for that the two modes cannot be distinguished later on (I think).

Your solution for provider mode seems reasonable, and I think it's good to support it with this work. If the key read response returns generate: true when that flag is set on create then we could distinguish later, but the docs don't seem to be explicit about that detail.

Unfortunately that information is not stored. From a TOTP perspective it matters only during the setup and doesn't make any difference later which is I guess why it is not stored. But as you said, I think we can stick to what I currently implemented and that should be fine.

Thanks again for your work on this! I will pull down the latest changes and test, and let you know if there are any other things we should update.

You're welcome, thanks for the review and feedback. I'll address your new comments over the next days.

[mpflanzer]

@hashishaw do you have any update on this?

[e-scheer]

any ETA regarding that feature ?

[hashishaw]

Hey folks, sorry I was on vacation last month and then dropped the ball on this. Let me schedule a sync with our designer, and I'll post an update after we review based on our user experience standards. Thanks for your patience!

[hyzza]

thanks for working on this, so important feature for us..any eta on release?

[mpflanzer]

Is there still interest in merging this into Vault?