Scoped down tokens from artifactory role

[kkronenb]

Is your feature request related to a problem? Please describe.
I am working on setting up this integration and would like to configure our CI role be able to create tokens for any scope, i.e. applied-permissions/groups:*. Currently when a token is created for a role, it uses the scope that is configured without the ability to set an explicitly reduced scope.

Describe the solution you'd like
When calling vault read artifactory/token/jenkins, I'd like to be able to specify a scope such as scope=readonly.

Describe alternatives you've considered
The current behavior is a blocker for our implementation.

Additional context
Add any other context or screenshots about the feature request here.

[alexhung]

@kkronenb If I understand your use case, I'm curious why creating multiple roles (vs explicitly setting reduced scope) are not desirable.

[kkronenb]

That is a possibility, but is undesirable from a maintenance stand point. When we new repo/group is added to artifactory, we must make a corresponding configuration update to Vault as well. If we have the ability to request a token with a specific scope, this dual configuration goes away.

We currently have 28 groups in artifactory for example. This quickly becomes an administrative hurdle in Vault.

[kkronenb]

My reasoning for wanting scoped down tokens is that it decouples artifactory administration from vault administration. When a new group is added to artifactory, there is no need to make a corresponding role in vault which requires additional overhead.

[kkronenb]

Turns out adding this functionality was pretty trivial.

diff --git a/path_token_create.go b/path_token_create.go
index abdb9a2..55c8e18 100644
--- a/path_token_create.go
+++ b/path_token_create.go
@@ -24,6 +24,10 @@ func (b *backend) pathTokenCreate() *framework.Path {
                                Type:        framework.TypeDurationSecond,
                                Description: `Override the maximum TTL for this access token. Cannot exceed smallest (system, backend) maximum TTL.`,
                        },
+                       "scope": {
+                               Type:        framework.TypeString,
+                               Description: `Override the scope for this access token.`,
+                       },
                },
                Operations: map[logical.Operation]framework.OperationHandler{
                        logical.ReadOperation: &framework.PathOperation{
@@ -88,6 +92,13 @@ func (b *backend) pathTokenCreatePerform(ctx context.Context, req *logical.Reque
                }
        }
 
+       scope := data.Get("scope").(string)
+
+       //use the overridden scope rather than role default
+       if len(scope) != 0 {
+               role.Scope = scope
+       }
+
        var ttl time.Duration
        if value, ok := data.GetOk("ttl"); ok {
                ttl = time.Second * time.Duration(value.(int))

Key                Value
---                -----
lease_id           artifactory/token/drone/FAf5YgaZKbKBEgRO0zit334F
lease_duration     1h
lease_renewable    true
access_token       REDACTED
role               drone
scope              applied-permissions/groups:*
token_id           62388d00-91e8-40a6-af74-95801ef7869d
username           v-drone-sibnkfl8

vault read artifactory/token/drone scope="applied-permissions/groups:readonly"
Key                Value
---                -----
lease_id           artifactory/token/drone/5RZesfUWFxRfq5vX5venXnLR
lease_duration     1h
lease_renewable    true
access_token       REDACTED
role               drone
scope              applied-permissions/groups:readonly
token_id           62f07108-ec3e-4a7b-a253-68861c3698e4
username           v-drone-CtuWW5k8```

[alexhung]

@kkronenb Yes, that's right. The coding part is straightforward, I'm thinking through the ramification of allowing scope being settable for the creation of the token. I feel that in theory we need to ensure we don't allow privilege escalation (expanding scope) but in practice it may be non-trivial.

This is provisionally in our plan for Q1 2024.

[TJM]

That is tricky, even if you limited the scope to applied-permissions.groups:, there are still groups you don't necessarily want tokens generated for (global-admins or whatever). I suppose that would be a Vault policy issue. You could just call the endpoint "group-token" (artifactory/group-token) and the requested name (artifactory/group-token/jenkins) becomes the group name? I think we (tried to do/did) something similar for user tokens a while back. You could certainly have an "allow/deny" list (supporting regex) for group-names, and of course need the ability to enable/disable it all together for people that want more control (creating the roles themselves). You could also just have a role for group-tokens that takes an argument (group=whatever) so it would be like: vault read artifactory/token/group group=jenkins, which has the advantage of not changing the endpoints, but from a vault policy perspective, it may be easier to have allow/deny based on path than options? (maybe?)

I definitely do not think you should allow requestors to specify whatever raw scope they want, because if you do that, you might as well just create an admin role and be done with it. :)

Tommy

[kkronenb]

First, thank you both for the conversation around this, I appreciate enumerating the issues.

@TJM I definitely do not think you should allow requestors to specify whatever raw scope they want, because if you do that, you might as well just create an admin role and be done with it. :)

This is exactly what I want. We use the same pattern with the github plugin where Vault is a vending machine for github PATs and can mint anything from org readonly to repo admin from a single plugin role. This is all controlled by Vault policies.

By definition in the instructions, the Vault Access Token is already an admin token. All this change is doing is moving from an implicit scope with multiple roles to an explicit scope via request. Both of these cases need to be addressed by Vault policies; either granting it based on path with the role name providing the implicit scope, or via path and allowed_parameters with the explicit scope.

This change could be an opt in when configuring the plugin where the existing behavior is the default.

[alexhung]

@kkronenb @TJM I'm considering transferring this to the Discussion section. This will allow better threading, etc.

What do you think?

[kkronenb]

Is there anything I can do to move this request forward? Right now, we are testing these changes in a development environment, but would rather not have to maintain this change in the long run.

[kkronenb]

Are there any other thoughts on incorporating this in the baseline? If not, we will have to fork this repo.