Adds gsuite provider specific extension for fetching groups and user information #123
Conversation
|
Additional test writing is still in progress. This is ready for some feedback in the meantime. I'll be updating this PR as I add more tests. |
There was a problem hiding this comment.
Thanks for rewriting it!
I had a concern about provisioning files on instances (I now need to securely deliver files onto the Vault isntances), would it be possible to expose the JSON file in the request body (and mark it as sensitive)?
For example, the SSH certificate backend allows you to choose between Vault managing the private key generation, or the user can submit the full private key and let Vault store it.
| } | ||
|
|
||
| // Create the google JWT config from the service account key file | ||
| if g.jwtConfig, err = google.JWTConfigFromJSON(config.serviceAccountKeyJSON, |
There was a problem hiding this comment.
Admin providing credentials with only the required scopes could have issues here:
For example. with fetch_groups = true and fetch_user_info = false, the minimal permission is admin.AdminDirectoryGroupReadonlyScope (no need to read user data).
There was a problem hiding this comment.
While this may be the minimal set, I think it makes sense to assume (and state in the docs) that the SA must have readonly on Users and Groups since most orgs would use this plugin to scope Vault tokens to group claims and not directly on users. There doesn't seem to be much of a use case for just scoping to individual users and not groups.
| } | ||
|
|
||
| // Create the google JWT config from the service account key file | ||
| if g.jwtConfig, err = google.JWTConfigFromJSON(config.serviceAccountKeyJSON, |
There was a problem hiding this comment.
While this may be the minimal set, I think it makes sense to assume (and state in the docs) that the SA must have readonly on Users and Groups since most orgs would use this plugin to scope Vault tokens to group claims and not directly on users. There doesn't seem to be much of a use case for just scoping to individual users and not groups.
|
@kalafut @austingebauer Thanks for putting this all together. I'm not super familiar with the release cycle for this plugin or Vault in general, but once this is merged, approximately when will this be released? |
|
@onetwopunch We're approaching a Vault release soon™, though bundling of this update vs. the timing of that is a little unclear. Regardless of the Vault release/bundled version though, this will be tagged in the plugin probably after the merge (I don't think we'd wait on anything else), and can be used as an external plugin. Note too that the external plugin can be used with existing data, and then subsequently you can revert to the bundled version when available. So there is some flexibility there. |
I think it's a valid concern, @adongy. If we were to take in the @onetwopunch, any thoughts on this? |
|
@austingebauer From a security perspective, there's a couple reasons I like having the file on the host as opposed to a sensitive value:
In my opinion, there should be a clear delineation of path configuration and the secrets stored in them. Users should be able to safely configure Vault with Terraform without worrying about secrets finding their way into the tfstate. Obviously this is not always possible and isn't super consistent across the plugin ecosystem, but I think it's a goal that we should shoot for when possible such as in this case. All that said, and to @adongy point, there is a precedence for setting GCP credentials in the path config in the GCP secret backend, though I would argue the security of that implementation as well with the points above. |
|
Thanks for sharing your thoughts, @onetwopunch. I generally agree with the points you've made. Some of those reasons are why I chose to take in a file path to begin with for this PR. I am mostly convinced to keep things as is by taking the file path in for the service account key file. I'm happy to hear others' thoughts though (@adongy). |
|
I have been looking for this for a long time, and happy to help with testing. Thank you @austingebauer |
|
While I agree with most points listed by @onetwopunch, I would prefer having the choice of posting the entire file or securely introducing it to the Vault instances. One key difference with letting Vault manage the file and reading it from the local filesystem is the storage location: the storage backend would have everything needed, making the Vault instances slightly less specialized. Rotation of the service account is immediately propagated to all instances when it's handled by the storage backend. Implementation wise, it could be done by exposing the serviceAccountKeyJSON in the struct, loading it from ServiceAccountFilePath if empty and finally erroring if both are missing. |
|
@adongy - I like the idea of providing the choice of configuring this via a file path or by providing the JSON contents directly. That would make this more consistent with other Google related Vault plugins. See auth/gcp#credentials and secret/gcp#credentials. I'm thinking this can be done by either:
Looking to hear some thoughts or other approaches related to this. |
|
My personal opinion on this matter is that providing strong guarantees about the type of values makes it easier for the end user to understand what to provide without looking at some documentation and allows developers to write things like LSPs for autocompletion more easily. As I do not use the GCP part of Vault, I cannot chime in on whether using environment variables for paths is the preferred way or not. The AWS equivalent allows you to either hardcode the variable value for the access keys, default to environment variables, or specify a profile file as describe in the AWS CLI docs. |
|
@adongy FWIW I wouldn't oppose the option of passing this in it's entirety, however I definitely think the documentation should clearly suggest loading the service account on the filesystem using the same mechanism for TLS certificates. For something like a service account that has GSuite admin access, many organizations would not be comfortable letting an operator see that in clear text to load it into Vault, in fact they'd probably say by virtue of that operator seeing it, the key is already compromised. Since it would allow that operator to dump all user and group info from the entire organization, it would be a valuable target for an insider threat. Separating the duties of the Vault server to load the file from an encrypted source and the operator that configures it would solve this problem in a low friction way. In regards to your question of key rotation, this is a valid concern that I think could be addressed in code and the documentation. First, @austingebauer could we add a check on that Thoughts? |
|
@onetwopunch - I also don't oppose having the option to pass the credentials in as JSON. The way that option is provided in the API is what needs to be settled. I agree that the documentation can suggest providing them via a file path for reasons that you've mentioned. Regarding the key rotation discussion, the steps you've provided above are already how a rotation would work. The configuration supplied via |
|
FWIW the GCP terraform provider has an option similar to one that I proposed above. It allows for supplying the credentials as a JSON string or a file path (see: credentials). I could see All that said, I don't think we need to hold up this PR to decide. I'm happy to open an issue and continue the discussion. I can always add flexibility to the service account configuration in a separate PR. I want to make sure this is carefully considered. Thoughts? |
|
@austingebauer do we have a process for adding this change to the Terraform Vault provider now that it's merged? |
|
@onetwopunch - I've opened up an issue to track getting this added (hashicorp/terraform-provider-vault#828). We try to get new features into the provider quickly, but it can become a balance of priority and demand. Note that this G Suite PR has yet to be bundled into an official Vault release, so it's not mentioned in the issue. |
|
Hi @austingebauer, I'm a complete noob at the whole OAuth2.0/OIDC stuff and probably missing something fundamental here. |
|
@m1keil you are correct, however access tokens are short lived (1 hour) so Vault would need a service account key to regenerate them. Think of a service account key as an identity that allows robots to login with OAuth without having to interact with the browser or redirects. I believe it's called a two-legged flow or something. |
|
Hi, I'm a complete noob with OIDC as well, so hopefully someone could point me in the right direction. I'm trying to follow the documentation preview and retrieve g-suite group membership which would then map with Vault External group with additional policies (similarly as described in these instructions step 5 & 6). So far I have setup the OAuth2 app and service account with domain delegation with 2 required scopes as per docs. I can login via OIDC successfully, however, it looks like no additional group membership is retrieved or is somewhat incorrectly mapped via Identity group alias (which, btw, would be nice to have an additional example in this documentation for identity group alias configuration for mapping with the g-suite group): So I'd expect that if everything configured correctly, aliased external group policy would be listed in the identity_policies, however it is empty. So couple of questions:
Any help is highly appreciated! |
|
@asmontas-i |
|
@cloudowski Any idea how I can make group memeber have the name/email from Google? currently it's just a random string. |
|
@azelezni Just use |
|
@cloudowski Doesn't seem to work, I've tried all of the supported claim by Google https://accounts.google.com/.well-known/openid-configuration, but nothing worked, either getting I did manage to update the entities metadata, but only for the alias and it doesn't actually change the name of the entity |
|
@azelezni - See example below with |
Overview
This PR adds an OIDC provider specific extension for G Suite that allows for fetching of groups and user information during authentication.
Please see the rendered documentation preview and PR (hashicorp/vault#9454) for additional context and explanation of the configuration.
This work is a continuation of #87 and closes #83. Thanks to @adongy and @onetwopunch for the help getting this work started.
Example configuration:
Example role:
Design of Change
This change uses the provider specific extension handling introduced in #118.
Related Issues/Pull Requests
Contributor Checklist
Test output