Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport of Auth method token_type possibleValues fix into release/1.11.x #19320

Conversation

hc-github-team-secure-vault-core
Copy link
Collaborator

Backport

This PR is auto-generated from #19290 to be assessed for backporting due to the inclusion of the label backport/1.11.x.

WARNING automatic cherry-pick of commits failed. Commits will require human attention.

merge conflict error: POST https://api.github.com/repos/hashicorp/vault/merges: 409 Merge conflict []

The below text is copied from the body of the original PR.


When configuring an Auth mount the UI was displaying the following three types: default, batch and service. However, the API lists the following four possible types: default-service, default-batch, batch and service. This caused the following problems:

  1. If the user didn't touch the Token Type dropdown we would send an empty string, which the api defaults to "default-service." However, if the user toggled this dropdown and then reselected default, and saved that value, the API returned with a not valid type error because it doesn't accept type default.

  2. The UI displayed the wrong values. I suspect default in the UI was supposed to generalize default-batch and default-service, however, they mean different things and confused the user.

I have fixed these errors by setting noDefault=true on the dropdown and changing the possibleValues to match the API. Ideally the API's default value would be default-service because this is the value all current auth methods return when you send token_type=''. I went back and forth on whether I should make the default value "default-service" in the UI. However, I settled on following what the API behavior is just in case for some reason in the future another Auth method does not return default-service as the token_type when passing an empty string.

With the fix:
image

When they go to tune their mount it will show default-service because even if they didn't send a token_type the API returns default-service
image

I also went ahead and fixed an active class error on the section tabs in this area by removing a surrounding li element.
Before:
image

After:
image

Notes:

  • the API doesn't list TokenType as a mount config option (it only shows that it's available for tuning). I'll make a ticket for this.
  • The UI has fallen behind on some properties like allowed_response_headers and plugin_version that the API lists but we don't have in the UI (we show allowed_response_headers on secret engine mounts but not on auth methods even though it's in the mount-config model?). I'll follow up with this as well, but thought I'd add a note in case anyone has some background here.

Overview of commits

@hc-github-team-secure-vault-core hc-github-team-secure-vault-core force-pushed the backport/ui/VAULT-7004/bug-fix-auth-token-type-2/currently-ultimate-caribou branch from 8299a94 to c9d663c Compare February 23, 2023 18:59
@hashicorp-cla
Copy link

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes


temp seems not to be a GitHub user.
You need a GitHub account to be able to sign the CLA. If you already have a GitHub account, please add the email address used for this commit to your account.

Have you signed the CLA already but the status is still pending? Recheck it.

@Monkeychip Monkeychip closed this Feb 23, 2023
@Monkeychip Monkeychip deleted the backport/ui/VAULT-7004/bug-fix-auth-token-type-2/currently-ultimate-caribou branch February 23, 2023 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants