Allow for scoped down group access tokens

[kkronenb]

This enhancement will allow the plugin to create an access token of targeted scope and allows Vault to act as an Artifactory access token vending machine. This is desirable because it decouples the configuration the Artifactory Role configuration in Vault from the Artifactory Group/Repo configuration.

vault write artifactory/roles/ci scope="applied-permissions/groups:admin" default_ttl=1h max_ttl=3h

vault read artifactory/token/ci scope=applied-permissions/groups:test-group

The Vault policy looks like the following

path "artifactory/token/ci" {
  capabilities = ["read"],
  required_parameters = ["scope"],
  allowed_parameters = {
    "scope" = ["applied-permissions/groups:test-group"]
  }
  denied_parameters = {
    "scope" = ["applied-permissions/groups:admin"]
  }
}

[github-actions]

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

[kkronenb]

I have read the CLA Document and I hereby sign the CLA

[kkronenb]

recheck

[alexhung]

@kkronenb You're good to go with CLA. The checker doesn't work consistently 🤷🏼‍♂️

[alexhung]

Please also update the README to include the Vault policy example from this PR's description. Ideally as a separate section on how to use this new field, with plenty of cautions and recommendation.

[alexhung]

@TJM You may be interested in reviewing this too.

[TJM]

I agree that this creates a very significant security issue unless you have vault policies that are restricting the scope attribute's value. I would recommend heavy warnings, as you mention, and even vault_policy examples for how to restrict access to certain groups. It honestly looks to me like you are just moving the onus for which pipeline has access to which artifactory tokens to the vault policy layer. Perhaps you are doing some magic there (if I recall). Please post a working example policy so we can put it into the "acceptance" testing, and readme.

[kkronenb]

has access to which artifactory tokens to the vault policy layer.

Correct, each Artifactory role in Vault will need a policy to allow access to it in either scenario. For scoped tokens, the Vault policy will should have additional constraints as mentioned in the README.

[TJM]

has access to which artifactory tokens to the vault policy layer.

Correct, each Artifactory role in Vault will need a policy to allow access to it in either scenario. For scoped tokens, the Vault policy will should have additional constraints as mentioned in the README.

I was honestly hoping there was some sort of policy magic that you could restrict the group name to only groups that the logged in token was a member of. Maybe that is asking too much from vault? ;)

[TJM]

I may be reading this wrong, as it has been a while, but If this option is in pathTokenCreate, doesn't it apply to all token creations, or in other words, how do we prevent users from using this option, when it is not intended to be available. We can't just allow this for the current artifactory/token/:roleName requests. It would have to be a different path (such as artifactory/scopedToken/:roleName), that would have to be enabled through the artifactory/admin/config and/or it would have to have some sort of allowScopeOverride boolean for the role itself. Another idea would be if the scope on the role was set to some specific value, like REQUIRE_SCOPE_OVERRIDE, but that seems a bit janky. Maybe if it was set to admin, then a scope attribute could be required, but that would be a breaking change for anyone that intends to offer an admin token through a role. The case I see in the readme would create an admin token if the requestor did not specify a scope attribute.

If we just add this capability, an unsuspecting vault/artifactory admin might update and unknowingly grant everyone that they thought had restricted access by the scope parameter in their role the ability to gain admin access. We can't just change the functionality mid-stream and suddenly require a customized vault policy to be added that filters or disallows the scope attribute, where it had previously just been path based filtering.

[alexhung]

I agree with @TJM.

Since this PR intention is to expand the capability to allow minting of token for groups, why not create a new path for it (similar to user token)? e.g. artifactory/group_token/:name, along with corresponding config path artifactory/config/group_token/:name?

We still need the ample warnings for the scope field but the separate path means we won't be breaking existing paths.

[TJM]

This is not necessarily just for groups. It allows overriding scope, which is then sent to the API without validation, so it could be for any user, any group or anything else that scope can do. Now, it might be "safer" to require a group name (as a path parameter? like artifactory/groupToken/:groupName), which could have some input validation (no commas or spaces?), then pass in a token with group scope interpolated with that group name. That would allow for "groupToken" path to be "enabled" at the /config/admin layer, similar to the userToken stuff we were talking about before.

Allowing vault users to override the scope directly could be done as well, although it seems risky to me. It should just not be done on the current artifactory/token/:roleName path.

EDIT: with input validation, it would be best to just set a very limited set of allowed characters, including a length limit to prevent issues with people URL encoding or other "bypass" mechanisms.

[kkronenb]

From my read through the code, the user and role/group paths have separate token implementations. pathUserTokenCreate() vs. pathTokenCreate()

Please correct me if I am wrong in these assumptions. From what I can discern, there are 2 main paths to request tokens. One for users and one for roles/groups. I proposed this change to the role/group token path as this is described as the CI/automation path. The user path functionality would remain unchanged since those are tied to a specific entity/identity in Vault.

Rather than duplicating the code for the role/group into a new path, would a feasible work around be to add another configuration field to explicitly opt in to scoped role tokens. This would retain the existing behavior during an upgrade. Requests will be validated against Vault policies before they are passed onto Artifactory.

The proposed approach is very similar to how the Vault Github plugin works. There is a single endpoint to get a token and different parameters provide different token capabilities. See https://github.com/martinbaillie/vault-plugin-secrets-github

[alexhung]

In terms of validation, you can follow similar validation code from the Artifactory provider: https://github.com/jfrog/terraform-provider-artifactory/blob/master/pkg/artifactory/resource/security/resource_artifactory_scoped_token.go#L185 and https://github.com/jfrog/terraform-provider-artifactory/blob/master/pkg/artifactory/resource/security/resource_artifactory_scoped_token.go#L335

[kkronenb]

Thanks, I'll work that into my PR.

[kkronenb]

PR has been updated to allow this to be opt in during plugin configuration.

[kkronenb]

@alexhung, @TJM any feedback on my updates?

[alexhung]

@kkronenb Sorry about the delay. I'll take a look today.

[kkronenb]

thanks for the comments, I have addressed them

[alexhung]

The code is good to go, just need renaming of the field. Need minor documentation additions.

Generally this is almost there.

[alexhung]

Incorrect indentation. I can fix this before release.