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

vault auth tune: unable to reset listing-visibility to default (empty string) #15209

Closed
candlerb opened this issue Apr 28, 2022 · 5 comments
Closed
Labels
bug Used to indicate a potential bug core/cli devex Developer Experience

Comments

@candlerb
Copy link
Contributor

Describe the bug
Having used vault auth tune -listing-visibility=unauth to make an auth method visible in the web UI, I cannot reset this to empty string. The command is accepted but does not make a change.

To Reproduce

$ vault read sys/auth/oidc
Key                        Value
---                        -----
accessor                   auth_oidc_43c2410e
config                     map[default_lease_ttl:0 force_no_cache:false listing_visibility:unauth max_lease_ttl:0 token_type:default-service]
description                Keycloak
external_entropy_access    false
local                      false
options                    <nil>
seal_wrap                  false
type                       oidc
uuid                       fe03f5d3-cecf-70e0-2aa4-f5a887ceb29b
$ vault auth tune -listing-visibility="" oidc/
Success! Tuned the auth method at: oidc/
$ vault read sys/auth/oidc
Key                        Value
---                        -----
accessor                   auth_oidc_43c2410e
config                     map[default_lease_ttl:0 force_no_cache:false listing_visibility:unauth max_lease_ttl:0 token_type:default-service]
description                Keycloak
external_entropy_access    false
local                      false
options                    <nil>
seal_wrap                  false
type                       oidc
uuid                       fe03f5d3-cecf-70e0-2aa4-f5a887ceb29b
  • Note that listing_visibility:unauth is set both before and after
  • The "oidc" method is still visible in the web UI

Expected behavior
It should be possible to change listing-visibility back to its default (empty string) state.

Environment:

  • Vault Server Version (retrieve with vault status): 1.10.1
  • Vault CLI Version (retrieve with vault version): 1.10.1
  • Server Operating System/Architecture: Ubuntu 20.04

Vault server configuration file(s): N/A

Additional context
N/A

@ccapurso
Copy link
Contributor

@candlerb, this looks to be an inconsistency between the CLI and hitting the API directly. I have reproduced the issue locally using your CLI example. Performing the same steps with the curl, however, I was able to reset the field as expected.

❯ curl -X POST -H "X-Vault-Token: $VAULT_TOKEN" -d '{"listing_visibility": "unauth"}' "$VAULT_ADDR/v1/sys/mounts/auth/oidc/tune"

❯ vault read sys/auth/oidc
Key                        Value
---                        -----
accessor                   auth_oidc_cf06863b
config                     map[default_lease_ttl:0 force_no_cache:false listing_visibility:unauth max_lease_ttl:0 token_type:default-service]
description                n/a
external_entropy_access    false
local                      false
options                    <nil>
seal_wrap                  false
type                       oidc
uuid

❯ curl -X POST -H "X-Vault-Token: $VAULT_TOKEN" -d '{"listing_visibility": ""}' "$VAULT_ADDR/v1/sys/mounts/auth/oidc/tune"

❯ vault read sys/auth/oidc
Key                        Value
---                        -----
accessor                   auth_oidc_cf06863b
config                     map[default_lease_ttl:0 force_no_cache:false max_lease_ttl:0 token_type:default-service]
description                n/a
external_entropy_access    false
local                      false
options                    <nil>
seal_wrap                  false
type                       oidc
uuid                       173e582d-6c00-bdca-d7c4-50b5eda200d9

I believe the fix is likely pretty straightforward and is caused by the omitempty JSON tag for the MountConfigInput struct which the CLI uses for marshaling the input/output. I will verify this with some tests to ensure that removing that tag is safe.

@ccapurso ccapurso added bug Used to indicate a potential bug core/cli labels Apr 28, 2022
@candlerb
Copy link
Contributor Author

Many thanks. I can confirm the following workaround is successful:

curl -X POST -H "X-Vault-Token: $(cat ~/.vault-token)" \
  -d '{"listing_visibility": ""}' "$VAULT_ADDR/v1/sys/mounts/auth/oidc/tune"

In the case of the CLI, I wonder if that string needs to become a string pointer? It needs to distinguish between "no change to listing_visibility" and "set listing_visibility to empty string".

@ccapurso
Copy link
Contributor

Ah, yes there is not currently a CLI flag type for a string pointer that allows for specifying between a deliberate empty string and the absence of the flag. It would be fairly straightforward to add but I think we would then be faced with a similar issue due to the fact that the data is provided as a struct that is then marshaled into JSON. The omitempty JSON tag would allow for the ability to ignore if not specified but not allow to override as an empty string.

The inconsistency between the API and the CLI is unfortunate and might exist elsewhere for string fields where omitempty JSON tags are used. For now, I would recommend using the workaround discussed. I will bring this up as a discussion topic with our team to determine what paths exist to mitigate such inconsistencies. Thank you for bringing it to our attention!

@ccapurso ccapurso added the devex Developer Experience label Apr 28, 2022
@cipherboy
Copy link
Contributor

Hi @candlerb I believe listing_visibility=hidden solves this. It was added in a past release. I think there might be some documentation missing though; feel free to add a PR for that if its not clear where that should be.

[cipherboy@xps15 vault]$ vault read sys/auth/cert-b
Key                        Value
---                        -----
accessor                   auth_cert_ee4ca3c3
config                     map[default_lease_ttl:0 force_no_cache:false listing_visibility:unauth max_lease_ttl:0 token_type:default-service]
description                n/a
external_entropy_access    false
local                      false
options                    <nil>
seal_wrap                  false
type                       cert
uuid                       0cf513c2-06b8-096b-d723-26748191f038
[cipherboy@xps15 vault]$ vault auth tune -listing-visibility="hidden" cert-b
Success! Tuned the auth method at: cert-b/
[cipherboy@xps15 vault]$ vault read sys/auth/cert-b
Key                        Value
---                        -----
accessor                   auth_cert_ee4ca3c3
config                     map[default_lease_ttl:0 force_no_cache:false listing_visibility:hidden max_lease_ttl:0 token_type:default-service]
description                n/a
external_entropy_access    false
local                      false
options                    <nil>
seal_wrap                  false
type                       cert
uuid                       0cf513c2-06b8-096b-d723-26748191f038

cipherboy added a commit that referenced this issue Jun 6, 2022
cipherboy added a commit that referenced this issue Jun 6, 2022
* Match listing_visibility in system/auth with system/mounts

See also: #15209

Signed-off-by: Alexander Scheel <[email protected]>

* Fix path-help for listing_visibility

Signed-off-by: Alexander Scheel <[email protected]>
@candlerb
Copy link
Contributor Author

candlerb commented Jun 6, 2022

I can confirm that "hidden" works as a value, thank you.
Ref: #15833 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to indicate a potential bug core/cli devex Developer Experience
Projects
None yet
Development

No branches or pull requests

3 participants