Closed kkronenb closed 10 months ago
@kkronenb If I understand your use case, I'm curious why creating multiple roles (vs explicitly setting reduced scope) are not desirable.
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.
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.
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```
@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.
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
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.
@kkronenb @TJM I'm considering transferring this to the Discussion section. This will allow better threading, etc.
What do you think?
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 asscope=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.