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

Allow for scoped down group access tokens #147

Merged
merged 10 commits into from
Mar 12, 2024
74 changes: 68 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,18 @@ vault write artifactory/config/admin \
use_expiring_tokens=true
```

#### Enable Scoped down Tokens

[!WARNING]
In order to decouple Artifactory Group maintenance from Vault plugin configuration, you can configure a single role to request Access Tokens for specific groups. This option should be used with extreme care to ensure that your Vault policies are restricting which groups it can request tokens on behalf of.

alexhung marked this conversation as resolved.
Show resolved Hide resolved
```sh
vault write artifactory/config/admin \
url=https://artifactory.example.org \
access_token=$TOKEN \
allow_scope_override=true
```

## Usage

Create a role (scope for artifactory >= 7.21.1)
Expand Down Expand Up @@ -326,6 +338,56 @@ token_id 06d962b2-63e2-4279-a25d-d2a9cab6507f
username v-jenkins-x4mohTA8
```

### Scoped Access Tokens

[!IMPORTANT]
In order to use this functionality, you must enable `allow_scope_override` when configuring the plugin, see [Enable Scoped down Tokens](#Use-scoped-down-tokens)

Create a role (scope for artifactory >= 7.21.1)

```sh
vault write artifactory/roles/jenkins \
username="jenkins-vault"
scope="applied-permissions/groups:admin" \
default_ttl=1h max_ttl=3h
```

Request Access Token for `test-group`

```sh
vault read artifactory/token/jenkins scope=applied-permissions/groups:test-group
```

Example output (token truncated):

```console
Key Value
--- -----
lease_id artifactory/token/jenkins/9hHxV1NlyLzPgmNIzjssRCa9
lease_duration 1h
lease_renewable true
access_token eyJ2ZXIiOiIyIiw....
role jenkins
scope applied-permissions/groups:test-group
token_id 06d962b2-63e2-4279-a25d-d2a9cab6507f
username v-jenkins-b0ftbTAG
```

Example Vault Policy

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

### User Token Path

User tokens may be obtained from the `/artifactory/user_token/<user-name>` endpoint. This is useful in conjunction with [ACL Policy Path Templating](https://developer.hashicorp.com/vault/tutorials/policies/policy-templating) to allow users authenticated to Vault to obtain API tokens in Artfactory for their own account. Be careful to ensure that Vault authentication methods & policies align with user account names in Artifactory.
Expand Down Expand Up @@ -448,10 +510,10 @@ Configures default values for the `user_token/:user-name` path. The optional `us
* `access_token` (stirng) - Optional. User identity token to access Artifactory. If `username` is not set then this token will be used for *all* users.
* `refresh_token` (string) - Optional. Refresh token for the user access token. If `username` is not set then this token will be used for *all* users.
* `audience` (string) - Optional. See the JFrog Platform REST documentation on [Create Token](https://jfrog.com/help/r/jfrog-rest-apis/create-token) for a full and up to date description. Service ID must begin with valid JFrog service type. Options: jfrt, jfxr, jfpip, jfds, jfmc, jfac, jfevt, jfmd, jfcon, or *. For instructions to retrieve the Artifactory Service ID see this [documentation](https://jfrog.com/help/r/jfrog-rest-apis/get-service-id)
* `refreshable` (boolean) - Optional. A refreshable access token gets replaced by a new access token, which is not what a consumer of tokens from this backend would be expecting; instead they'd likely just request a new token periodically. Set this to `true` only if your usage requires this. See the JFrog Platform documentation on [Generating Refreshable Tokens](https://jfrog.com/help/r/jfrog-platform-administration-documentation/generating-refreshable-tokens) for a full and up to date description. Defaults to `false`.
* `include_reference_token` (boolean) - Optional. Generate a Reference Token (alias to Access Token) in addition to the full token (available from Artifactory 7.38.10). A reference token is a shorter, 64-character string, which can be used as a bearer token, a password, or with the `X-JFrog-Art-Api`header. Note: Using the reference token might have performance implications over a full length token. Defaults to `false`.
* `use_expiring_tokens` (boolean) - Optional. If Artifactory version >= 7.50.3, set `expires_in` to `ttl` and `force_revocable = true`. Defaults to `false`.
* `default_ttl` (int64) - Optional. Default TTL for issued user access tokens. If unset, uses the backend's `default_ttl`. Cannot exceed `max_ttl`.
* `refreshable` (boolean) - Optional. A refreshable access token gets replaced by a new access token, which is not what a consumer of tokens from this backend would be expecting; instead they'd likely just request a new token periodically. Set this to `true` only if your usage requires this. See the JFrog Platform documentation on [Generating Refreshable Tokens](https://jfrog.com/help/r/jfrog-platform-administration-documentation/generating-refreshable-tokens) for a full and up to date description. Defaults to `false`.
* `include_reference_token` (boolean) - Optional. Generate a Reference Token (alias to Access Token) in addition to the full token (available from Artifactory 7.38.10). A reference token is a shorter, 64-character string, which can be used as a bearer token, a password, or with the `X-JFrog-Art-Api`header. Note: Using the reference token might have performance implications over a full length token. Defaults to `false`.
* `use_expiring_tokens` (boolean) - Optional. If Artifactory version >= 7.50.3, set `expires_in` to `ttl` and `force_revocable = true`. Defaults to `false`.
* `default_ttl` (int64) - Optional. Default TTL for issued user access tokens. If unset, uses the backend's `default_ttl`. Cannot exceed `max_ttl`.
* `default_description` (string) - Optional. Default token description to set in Artifactory for issued user access tokens.

#### Examples
Expand Down Expand Up @@ -494,7 +556,7 @@ vault delete artifactory/config/user_token/myuser
* `audience` (string) - Optional. See the JFrog Platform REST documentation on [Create Token](https://jfrog.com/help/r/jfrog-rest-apis/create-token) for a full and up to date description. Service ID must begin with valid JFrog service type. Options: jfrt, jfxr, jfpip, jfds, jfmc, jfac, jfevt, jfmd, jfcon, or *. For instructions to retrieve the Artifactory Service ID see this [documentation](https://jfrog.com/help/r/jfrog-rest-apis/get-service-id)
* `include_reference_token` (boolean) - Optional. Generate a Reference Token (alias to Access Token) in addition to the full token (available from Artifactory 7.38.10). A reference token is a shorter, 64-character string, which can be used as a bearer token, a password, or with the `X-JFrog-Art-Api`header. Note: Using the reference token might have performance implications over a full length token. Defaults to `false`.
* `default_ttl` (int64) - Default TTL for issued user access tokens. If unset, uses the backend's `default_ttl`. Cannot exceed `max_ttl`.
* `max_ttl` (int64) - Maximum TTL that an access token can be renewed for. If unset, uses the backend's `max_ttl`. Cannot exceed backend's `max_ttl`.
* `max_ttl` (int64) - Maximum TTL that an access token can be renewed for. If unset, uses the backend's `max_ttl`. Cannot exceed backend's `max_ttl`.

#### Examples

Expand Down Expand Up @@ -557,7 +619,7 @@ Provides optional parameters to override default values for the user_token/:user
* `description` (string) - Optional. Override the token description to set in Artifactory for issued user access tokens.
* `refreshable` (boolean) - Optional. Override the `refreshable` for this access token. Defaults to `false`.
* `include_reference_token` (boolean) - Optional. Override the `include_reference_token` for this access token. Defaults to `false`.
* `use_expiring_tokens` (boolean) - Optional. Override the `use_expiring_tokens` for this access token. If Artifactory version >= 7.50.3, set `expires_in` to `ttl` and `force_revocable = true`. Defaults to `false`.
* `use_expiring_tokens` (boolean) - Optional. Override the `use_expiring_tokens` for this access token. If Artifactory version >= 7.50.3, set `expires_in` to `ttl` and `force_revocable = true`. Defaults to `false`.
* `ttl` (int64) - Optional. Override the default TTL when issuing this access token. Cannot exceed smallest (system, backend, role, this request) maximum TTL.
* `max_ttl` (int64) - Optional. Override the maximum TTL for this access token. Cannot exceed smallest (system, backend) maximum TTL.

Expand Down
12 changes: 12 additions & 0 deletions path_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ func (b *backend) pathConfig() *framework.Path {
Default: false,
Description: "Optional. Bypass certification verification for TLS connection with Artifactory. Default to `false`.",
},
"allow_scope_override": {
Type: framework.TypeBool,
Default: false,
Description: "Optional. Determine if scoped tokens should be allowed. This is an advanced configuration option. Default to `false`.",
"revoke_on_delete": {
Type: framework.TypeBool,
Default: false,
Expand Down Expand Up @@ -77,6 +81,9 @@ usernames if a static one is not provided.

An optional "bypass_artifactory_tls_verification" parameter will enable bypassing the TLS connection verification with Artifactory.

An optional "allow_scope_override" parameter will enable issuing scoped tokens with Artifactory. This is an advanced option that must
have more sophisticated Vault policies. Please see README for an example.

No renewals or new tokens will be issued if the backend configuration (config/admin) is deleted.
`,
}
Expand All @@ -86,6 +93,7 @@ type adminConfiguration struct {
baseConfiguration
UsernameTemplate string `json:"username_template,omitempty"`
BypassArtifactoryTLSVerification bool `json:"bypass_artifactory_tls_verification,omitempty"`
AllowScopeOverride bool `json:"allow_scope_override,omitempty"`
RevokeOnDelete bool `json:"revoke_on_delete,omitempty"`
}

Expand Down Expand Up @@ -149,6 +157,9 @@ func (b *backend) pathConfigUpdate(ctx context.Context, req *logical.Request, da
config.BypassArtifactoryTLSVerification = val.(bool)
}

if val, ok := data.GetOk("allow_scope_override"); ok {
config.AllowScopeOverride = val.(bool)

if val, ok := data.GetOk("revoke_on_delete"); ok {
config.RevokeOnDelete = val.(bool)
}
Expand Down Expand Up @@ -245,6 +256,7 @@ func (b *backend) pathConfigRead(ctx context.Context, req *logical.Request, _ *f
"version": b.version,
"use_expiring_tokens": config.UseExpiringTokens,
"bypass_artifactory_tls_verification": config.BypassArtifactoryTLSVerification,
"allow_scope_override": config.AllowScopeOverride,
"revoke_on_delete": config.RevokeOnDelete,
}

Expand Down
5 changes: 5 additions & 0 deletions path_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ func TestAcceptanceBackend_PathConfig(t *testing.T) {
t.Run("read", accTestEnv.ReadPathConfig)
t.Run("expiringTokens", accTestEnv.PathConfigUpdateExpiringTokens)
t.Run("bypassArtifactoryTLSVerification", accTestEnv.PathConfigUpdateBypassArtifactoryTLSVerification)
t.Run("allowScopedTokens", accTestEnv.PathConfigUpdateAllowScopeOverride)
t.Run("usernameTemplate", accTestEnv.PathConfigUpdateUsernameTemplate)
t.Run("delete", accTestEnv.DeletePathConfig)
t.Run("errors", accTestEnv.PathConfigUpdateErrors)
Expand All @@ -46,6 +47,10 @@ func (e *accTestEnv) PathConfigUpdateBypassArtifactoryTLSVerification(t *testing
e.pathConfigUpdateBooleanField(t, "bypass_artifactory_tls_verification")
}

func (e *accTestEnv) PathConfigUpdateAllowScopeOverride(t *testing.T) {
e.pathConfigUpdateBooleanField(t, "allow_scope_override")
}

func (e *accTestEnv) pathConfigUpdateBooleanField(t *testing.T, fieldName string) {
// Boolean
e.UpdateConfigAdmin(t, testData{
Expand Down
23 changes: 23 additions & 0 deletions path_token_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package artifactory

import (
"context"
"errors"
"regexp"
"time"

"github.com/hashicorp/vault/sdk/framework"
Expand All @@ -24,6 +26,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": {
Copy link
Contributor

@TJM TJM Feb 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@TJM TJM Feb 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'll work that into my PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexhung, @TJM any feedback on my updates?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the comments, I have addressed them

Type: framework.TypeString,
Description: `Override the scope for this access token.`,
alexhung marked this conversation as resolved.
Show resolved Hide resolved
},
},
Operations: map[logical.Operation]framework.OperationHandler{
logical.ReadOperation: &framework.PathOperation{
Expand Down Expand Up @@ -135,6 +141,23 @@ func (b *backend) pathTokenCreatePerform(ctx context.Context, req *logical.Reque
role.ExpiresIn = maxLeaseTTL
}

if config.AllowScopeOverride {
scope := data.Get("scope").(string)
if len(scope) != 0 {
re, err := regexp.Compile(`^applied-permissions\/groups:.+$`)
if err != nil {
return nil, err
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect indentation. I can fix this before release.

match := re.MatchString(scope)

if !match {
return logical.ErrorResponse("provided scope is invalid"), errors.New("provided scope is invalid")
alexhung marked this conversation as resolved.
Show resolved Hide resolved
}
//use the overridden scope rather than role default
role.Scope = scope
}
}

resp, err := b.CreateToken(config.baseConfiguration, *role)
if err != nil {
return nil, err
Expand Down
3 changes: 3 additions & 0 deletions path_token_create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ func TestAcceptanceBackend_PathTokenCreate(t *testing.T) {

t.Run("configure backend", accTestEnv.UpdatePathConfig)
t.Run("create role", accTestEnv.CreatePathRole)
t.Run("create admin role", accTestEnv.CreatePathAdminRole)
t.Run("create token for role", accTestEnv.CreatePathToken)
t.Run("create scoped down token for admin role", accTestEnv.CreatePathScopedDownToken)
t.Run("create scoped down token for admin role with bad scope", accTestEnv.CreatePathScopedDownTokenBadScope)
t.Run("delete role", accTestEnv.DeletePathRole)
t.Run("cleanup backend", accTestEnv.DeletePathConfig)
}
Expand Down
64 changes: 62 additions & 2 deletions test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,9 @@ func (e *accTestEnv) revokeTestToken(t *testing.T, accessToken string, tokenID s

func (e *accTestEnv) UpdatePathConfig(t *testing.T) {
e.UpdateConfigAdmin(t, testData{
"access_token": e.AccessToken,
"url": e.URL,
"access_token": e.AccessToken,
"url": e.URL,
"allow_scope_override": true,
})
}

Expand Down Expand Up @@ -229,6 +230,27 @@ func (e *accTestEnv) CreatePathRole(t *testing.T) {
assert.Nil(t, resp)
}

func (e *accTestEnv) CreatePathAdminRole(t *testing.T) {
roleData := map[string]interface{}{
"role": "admin-role",
"username": "admin",
"scope": "applied-permissions/groups:admin",
"audience": "*@*",
"default_ttl": 30 * time.Minute,
"max_ttl": 45 * time.Minute,
}

resp, err := e.Backend.HandleRequest(context.Background(), &logical.Request{
Operation: logical.UpdateOperation,
Path: "roles/admin-role",
Storage: e.Storage,
Data: roleData,
})

assert.NoError(t, err)
assert.Nil(t, resp)
}

func (e *accTestEnv) ReadPathRole(t *testing.T) {
resp, err := e.Backend.HandleRequest(context.Background(), &logical.Request{
Operation: logical.ReadOperation,
Expand Down Expand Up @@ -303,6 +325,44 @@ func (e *accTestEnv) CreatePathToken_overrides(t *testing.T) {
assert.Equal(t, 60, resp.Data["expires_in"])
}

func (e *accTestEnv) CreatePathScopedDownToken(t *testing.T) {
resp, err := e.Backend.HandleRequest(context.Background(), &logical.Request{
Operation: logical.ReadOperation,
Path: "token/admin-role",
Storage: e.Storage,
Data: map[string]interface{}{
"scope": "applied-permissions/groups:test-group",
},
})

assert.NoError(t, err)
assert.NotNil(t, resp)
assert.NotEmpty(t, resp.Data["access_token"])
assert.NotEmpty(t, resp.Data["token_id"])
assert.Equal(t, "admin", resp.Data["username"])
assert.Equal(t, "admin-role", resp.Data["role"])
assert.Equal(t, "applied-permissions/groups:test-group", resp.Data["scope"])
}

func (e *accTestEnv) CreatePathScopedDownTokenBadScope(t *testing.T) {
resp, err := e.Backend.HandleRequest(context.Background(), &logical.Request{
Operation: logical.ReadOperation,
Path: "token/admin-role",
Storage: e.Storage,
Data: map[string]interface{}{
"scope": "blueberries?pancakes",
},
})

assert.Error(t, err, "provided scope is invalid")
assert.NotNil(t, resp)
assert.Empty(t, resp.Data["access_token"])
assert.Empty(t, resp.Data["token_id"])
assert.NotEqual(t, "admin", resp.Data["username"])
assert.NotEqual(t, "admin-role", resp.Data["role"])
assert.NotEqual(t, "applied-permissions/groups:test-group", resp.Data["scope"])
}

func (e *accTestEnv) CreatePathUserToken(t *testing.T) {
resp, err := e.Backend.HandleRequest(context.Background(), &logical.Request{
Operation: logical.UpdateOperation,
Expand Down
Loading