Skip to content

Respect WithWrappingToken for all secret ID's in approle auth#13241

Merged
averche merged 5 commits intomainfrom
respect-wrapping-token
Nov 23, 2021
Merged

Respect WithWrappingToken for all secret ID's in approle auth#13241
averche merged 5 commits intomainfrom
respect-wrapping-token

Conversation

@averche
Copy link
Copy Markdown
Contributor

@averche averche commented Nov 22, 2021

Background

When authenticating with vault via AppRole, the secret ID can be specified in one of 3 ways:

  • FromFile
  • FromEnv
  • FromString

If the secret ID happens to be a wrapping token, the option auth.WithWrappingToken() will let the client know to treat it as such. However, the current implementation will only respect auth.WithWrappingToken() if the secret ID is specified from a file (FromFile) and silently fails otherwise, which could be surprising to callers.

The rationale behind this was that the wrapping token should be provided through a trusted orchestrator and is typically written to a file. However, it is conceivable that the trusted orchestrator would write it to an environment variable instead.

Solution

This PR will respect the auth.WithWrappingToken() for all secret ID's (whether specified via FromFile, FromEnv or FromString)

@hashicorp-cla
Copy link
Copy Markdown

hashicorp-cla commented Nov 22, 2021

CLA assistant check
All committers have signed the CLA.

@averche averche requested a review from digivava November 22, 2021 22:29
@vercel vercel bot temporarily deployed to Preview – vault-storybook November 22, 2021 23:30 Inactive
@vercel vercel bot temporarily deployed to Preview – vault November 22, 2021 23:30 Inactive
@vercel vercel bot temporarily deployed to Preview – vault November 22, 2021 23:31 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook November 22, 2021 23:31 Inactive
@vercel vercel bot temporarily deployed to Preview – vault November 23, 2021 16:35 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook November 23, 2021 16:35 Inactive
Copy link
Copy Markdown
Collaborator

@digivava digivava left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thank you! Just one comment with something optional, but looks good to go otherwise.

Comment thread api/auth/approle/approle.go
Comment thread api/auth/approle/approle.go
@averche averche marked this pull request as ready for review November 23, 2021 17:48
@averche averche merged commit 83f9186 into main Nov 23, 2021
@averche averche deleted the respect-wrapping-token branch November 23, 2021 23:53
@peaceofthepai peaceofthepai added this to the 1.10 milestone Feb 25, 2022
pull bot pushed a commit to NOUIY/vault that referenced this pull request Mar 23, 2026
…3309)

* Add LDAP secrets engine blackbox tests

* Format

* format

* cleanup environment

* Install ldap-utils in CI for LDAP domain provisioning

* wrap in eventually

* debugging

* fix ip issues

* Add LDAP root credential rollback test

* refactor rollback tests to fix race conditions and improve reliability

* removed AI comment

---------

Co-authored-by: shettykshitij <kshitij.shetty@hashicorp.com>
Co-authored-by: LT Carbonell <ltcarbonell@pm.me>
Co-authored-by: Luis (LT) Carbonell <lt.carbonell@hashicorp.com>
pbromb pushed a commit to pbromb/vault that referenced this pull request Apr 13, 2026
…3312) (hashicorp#13315)

* Add LDAP secrets engine blackbox tests

* Format

* format

* cleanup environment

* Install ldap-utils in CI for LDAP domain provisioning

* wrap in eventually

* debugging

* fix ip issues

* Add LDAP root credential rollback test

* refactor rollback tests to fix race conditions and improve reliability

* removed AI comment

---------

Co-authored-by: shettykshitij <kshitij.shetty@hashicorp.com>
Co-authored-by: LT Carbonell <ltcarbonell@pm.me>
Co-authored-by: Luis (LT) Carbonell <lt.carbonell@hashicorp.com>
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.

4 participants