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

Add ability to store refresh token into a secrets plugin #107

Closed
wants to merge 2 commits into from

Conversation

DrDaveD
Copy link

@DrDaveD DrDaveD commented Apr 2, 2020

Overview

This PR adds configuration options to store a received refresh_token into the given path in vault. It has been tested with the Puppet Labs oauthapp vault plugin and the corresponding pull request that accepts the refresh token.

These options make it possible for a user to authenticate with vault once and from then on be able to get access tokens from the OIDC provider via the secrets plugin. This is needed by the High Energy Physics science community as they migrate their global computing from X.509 certificates to OIDC tokens.

Design of Change

The role configuration option refresh_store_path can be set to specify a vault path to write into. The path may contain any claim names from the id_token surrounded by double curly brackets (for example "{{email}}", which are replaced by the value of the claim. The role configuration option refresh_store_cred should contain a vault token with sufficient privileges to write to the path, or the token set in the environment variable VAULT_TOKEN.

Related Issues/Pull Requests

Addresses issue #101.

Contributor Checklist

I will write the docs if this PR is otherwise deemed acceptable.

[ ] Add relevant docs to upstream Vault repository, or sufficient reasoning why docs won’t be added yet
[ ] Add output for any tests not ran in CI to the PR description (eg, acceptance tests)
[x] Backwards compatible

@DrDaveD
Copy link
Author

DrDaveD commented Apr 3, 2020

I changed the single curly brackets around the claim in refresh_store_path to double curly brackets to match the usage in vault policies, and edited the description to reflect the change.

@DrDaveD
Copy link
Author

DrDaveD commented Apr 15, 2020

Is there any reaction to this by the plugin owners?

@DrDaveD
Copy link
Author

DrDaveD commented May 4, 2020

@kalafut @jefferai Do you have any thoughts on this?

@kalafut
Copy link
Contributor

kalafut commented May 7, 2020

@DrDaveD Thank you for your work on this and for the submission to Puppet's oauth secrets plugin (which I learned about through this issue). To date we've not allowed the pattern of plugins calling other Vault plugins or endpoints. More generally, the storage of a refresh token during Vault authentication is a fairly niche use case that we'll not be supporting at this time.

Maintaining a fork of the JWT/OIDC repo is the recommended path in this case. In #101 you had questions about replacing the internal JWT/OIDC mount with an external plugin, and that is definitely possible. If you have further questions about that process, the best venue would be to post to https://discuss.hashicorp.com/c/vault.

@jmls
Copy link

jmls commented May 10, 2020

dang, looks like I'm a niche case as well ... would have found this useful. @DrDaveD , could I ask how you distribute your custom version of this plugin ? Do you build a new vault image or deregister the official plugin and register yours the first time you start vault ? I have a change that I'd like to make (#109 ) and would want to include this PR intpo my version.

I just am not sure about the whole "replace a plugin with another" scenario ;)

@DrDaveD
Copy link
Author

DrDaveD commented May 11, 2020

@jmls I don't distribute this yet but if needed my plan would be to distribute the modified plugin only, and use it to replace the standard one. That's what I am doing for testing.

Note that I have also now proposed a simpler alternative to this PR in #113 which just makes the refresh token available to the client in metadata. If that's sufficient for your needs, I suggest that you comment on that PR to express your support.

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.

None yet

3 participants