From 381a9d67b96b79fbe6a05a135fd853b3e47e6e2d Mon Sep 17 00:00:00 2001 From: Nicholas Shine Date: Wed, 21 Feb 2024 23:38:10 -0600 Subject: [PATCH] update to allow for static groups --- Makefile | 5 +++- README.md | 29 ++++++++++++++++++++-- plugin/artifactory_client.go | 9 ++++++- plugin/path_role.go | 42 ++++++++++++++++++++++++-------- plugin/path_role_test.go | 23 ++++++++++++++--- plugin/role.go | 20 ++++++++++----- scripts/setup_dev_artifactory.sh | 9 +++++++ scripts/setup_dev_vault.sh | 13 ++++++---- 8 files changed, 121 insertions(+), 29 deletions(-) diff --git a/Makefile b/Makefile index 1b145f2..bd48ed3 100644 --- a/Makefile +++ b/Makefile @@ -40,6 +40,9 @@ vault-only: build dev: tools build-linux @./scripts/init_dev.sh +reload: tools build-linux + @./scripts/setup_dev_vault.sh + clean-dev: @cd scripts && docker compose down -v @@ -72,4 +75,4 @@ tools: .tools .tools/gocover-cobertura .tools/gocovmerge .tools/golangci-lint .t curl -so .tools/vault.zip -sSL https://releases.hashicorp.com/vault/$(VAULT_VERSION)/vault_$(VAULT_VERSION)_$(VAULT_PLATFORM)_amd64.zip (cd .tools && unzip -o vault.zip && rm vault.zip) -.PHONY: all get build build-linux publish lint test test-artacc test-vaultacc report vault-only dev clean-dev clean-all tools +.PHONY: all get build build-linux publish lint test test-artacc test-vaultacc report vault-only dev reload clean-dev clean-all tools diff --git a/README.md b/README.md index aa5f86d..77868fb 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,3 @@ - # vault-plugin-secrets-artifactory [![build-status-badge]][actions-page] @@ -10,6 +9,7 @@ This is a backend plugin to be used with Vault. This plugin generates one-time a [Design doc][design-doc] +- [Differences between JFrog's Vault Plugin](#differences-between-jfrogs-vault-plugin) - [Requirements](#requirements) - [Getting Started](#getting-started) - [Usage](#usage) @@ -19,9 +19,18 @@ This is a backend plugin to be used with Vault. This plugin generates one-time a - [Development](#development) - [Full dev environment](#full-dev-environment) - [Developing with an existing Artifactory instance](#developing-with-an-existing-artifactory-instance) + - [Reloading plugin](#reloading-plugin) - [Tests](#tests) - [License](#license) +## Differences between JFrog's Vault Plugin + +JFrog has their [own vault plugin](https://github.com/jfrog/vault-plugin-secrets-artifactory/). The +main difference between that plugin and this one is the dynamic group/permission target generation. + +This plugin generates permission targets and a group to link the token with the desired permissions, +in addition to being able to specify a pre-existing group. + ## Requirements - Go: 1.22 or above @@ -55,9 +64,15 @@ $ vault write artifactory/config base_url="https://artifactory.example.com/artif $ vault path-help artifactory/ $ vault path-help artifactory/config -# create a role +# create a role with permissions targets $ vault write artifactory/roles/ci-role token_ttl=600 permission_targets=@scripts/sample_permission_targets.json +# create a role with permission targets and additional pre-existing static groups +$ vault write artifactory/roles/ci-role token_ttl=600 permission_targets=@scripts/sample_permission_targets.json groups=group1,group2,group3 + +# create a role with pre-existing static groups only +$ vault write artifactory/roles/ci-role token_ttl=600 groups=group1,group2,group3 + # generate an ephemeral artifactory token $ vault write artifactory/token/ci-role ttl=60 Key Value @@ -192,6 +207,16 @@ export ARTIFACTORY_BEARER_TOKEN=TOKEN You can then create a role and issue a token following above usage. +### Reloading plugin + +To quickly test changes to the plugin (using the docker environment), run: + +``` +make reload +``` + +This will re-compile, re-register, and reload the plugin. + ### Tests ```sh diff --git a/plugin/artifactory_client.go b/plugin/artifactory_client.go index 76961ac..0619c3f 100644 --- a/plugin/artifactory_client.go +++ b/plugin/artifactory_client.go @@ -18,6 +18,7 @@ import ( "context" "fmt" "io" + "strings" "time" "github.com/jfrog/jfrog-client-go/access" @@ -185,9 +186,15 @@ func (ac *artifactoryClient) DeletePermissionTarget(ptName string) error { func (ac *artifactoryClient) CreateToken(tokenReq TokenCreateEntry, role *RoleStorageEntry) (auth.CreateTokenResponseData, error) { expiresIn := uint(tokenReq.TTL.Seconds()) + var groups []string + if len(role.PermissionTargets) > 0 { + groups = append(groups, groupName(role)) + } + groups = append(groups, role.Groups...) + params := accessservices.CreateTokenParams{ CommonTokenParams: auth.CommonTokenParams{ - Scope: fmt.Sprintf("applied-permissions/groups:%s", groupName(role)), + Scope: fmt.Sprintf("applied-permissions/groups:%s", strings.Join(groups, ",")), ExpiresIn: &expiresIn, TokenType: "access_token", Audience: "*@*", diff --git a/plugin/path_role.go b/plugin/path_role.go index 70e6415..c5fd4ef 100644 --- a/plugin/path_role.go +++ b/plugin/path_role.go @@ -45,6 +45,10 @@ var createRoleSchema = map[string]*framework.FieldSchema{ Type: framework.TypeString, Description: "List of permission target configurations", }, + "groups": { + Type: framework.TypeCommaStringSlice, + Description: "Optional comma-separated list of static, pre-existing groups to associate with the role", + }, } // remove the specified role from the storage @@ -104,6 +108,7 @@ func (backend *ArtifactoryBackend) pathRoleRead(ctx context.Context, req *logica "token_ttl": int64(role.TokenTTL / time.Second), "max_ttl": int64(role.MaxTTL / time.Second), "permission_targets": role.RawPermissionTargets, + "groups": role.Groups, }, }, nil } @@ -124,6 +129,7 @@ func (backend *ArtifactoryBackend) pathRoleCreateUpdate(ctx context.Context, req "role_id": role.RoleID, "role_name": role.Name, "permission_targets": role.RawPermissionTargets, + "groups": role.Groups, } } @@ -156,7 +162,18 @@ func (backend *ArtifactoryBackend) pathRoleCreateUpdate(ctx context.Context, req role.RoleID = roleID(roleName) } - isCreate := req.Operation == logical.CreateOperation + // Groups + var groups []string + groupsRaw, newGroups := data.GetOk("groups") + if newGroups { + var ok bool + groups, ok = groupsRaw.([]string) + if !ok { + return logical.ErrorResponse("groups is not a string slice"), nil + } + + role.Groups = groups + } // Permission Targets ptsRaw, newPermissionTargets := data.GetOk("permission_targets") @@ -165,13 +182,13 @@ func (backend *ArtifactoryBackend) pathRoleCreateUpdate(ctx context.Context, req if !ok { return logical.ErrorResponse("permission targets are not a string"), nil } - if pts == "" { - return logical.ErrorResponse("permission targets are empty"), nil + if pts == "" && !newGroups { + return logical.ErrorResponse("permission targets and groups are empty"), nil } } - if isCreate && !newPermissionTargets { - return logical.ErrorResponse("permission targets are required for new role"), nil + if !newPermissionTargets && !newGroups { + return logical.ErrorResponse("permission targets and/or groups are required to create or update a role"), nil } maxttlRaw, ok := data.GetOk("max_ttl") @@ -194,6 +211,7 @@ func (backend *ArtifactoryBackend) pathRoleCreateUpdate(ctx context.Context, req if role.TokenTTL > role.MaxTTL { return logical.ErrorResponse(fmt.Sprintf("role token ttl is greater than role max ttl '%d'", role.MaxTTL)), nil } + // If no new permission targets or new permission targets are exactly same as old permission targets, // just return without updating permission targets if !newPermissionTargets || role.permissionTargetsHash() == getStringHash(ptsRaw.(string)) { @@ -278,11 +296,15 @@ func pathRoleList(backend *ArtifactoryBackend) []*framework.Path { return paths } -const pathRoleHelpSyn = `Read/write sets of permission targets to be given to generated credentials for specified role.` +const pathRoleHelpSyn = `Read/write sets of permission targets and/or groups to generate an Artifactory access token for the specified role.` const pathRoleHelpDesc = ` -This path allows you to create roles, which bind sets of permission targets -of specific repositories with patterns and operations to a group. Secrets are -generated under a role and will have the given set of permission targets on group. +This path allows you to create roles, which bind sets of permission targets of +specific repositories with patterns and operations to a group. In addition, +optional pre-existing groups can be bound to the role. + +Access tokens are generated under a role and will be scoped to the generated +group associated with the permission targets, as well as the optional +additional pre-existing groups. The specified permission targets file accepts an JSON string with the following format: @@ -304,7 +326,7 @@ with the following format: } ] -At least one of repo or build is required +At least one of repo or build is required (if no pre-existing groups specified) | field | subfield | required | | ----- | ---------------- | -------- | diff --git a/plugin/path_role_test.go b/plugin/path_role_test.go index b45c569..f33082e 100644 --- a/plugin/path_role_test.go +++ b/plugin/path_role_test.go @@ -367,7 +367,7 @@ func TestArtAccPermissionTargets(t *testing.T) { }) } -func TestPathRoleFail(t *testing.T) { +func TestPathRole(t *testing.T) { t.Parallel() req, backend := newArtMockEnv(t) conf := map[string]interface{}{ @@ -448,11 +448,11 @@ func TestPathRoleFail(t *testing.T) { require.True(t, resp.IsError(), "expecting error") actualErr := resp.Data["error"].(string) - expected := "permission targets are required for new role" + expected := "permission targets and/or groups are required to create or update a role" assert.Contains(t, actualErr, expected) }) - t.Run("empty_permission_targets", func(t *testing.T) { + t.Run("empty permission targets and groups", func(t *testing.T) { data := make(map[string]interface{}) roleName := "test_role1" data["permission_targets"] = "" @@ -461,7 +461,7 @@ func TestPathRoleFail(t *testing.T) { require.True(t, resp.IsError(), "expecting error") actualErr := resp.Data["error"].(string) - expected := "permission targets are empty" + expected := "permission targets and groups are empty" assert.Contains(t, actualErr, expected) }) @@ -528,6 +528,21 @@ func TestPathRoleFail(t *testing.T) { expected := "operation 'invalidop' is not allowed" assert.Contains(t, actualErr, expected) }) + + t.Run("static group only role succeeds", func(t *testing.T) { + data := make(map[string]interface{}) + roleName := "test_role1" + data["groups"] = []string{"testgroup1", "testgroup2"} + data["name"] = roleName + resp, err := testRoleCreate(req, backend, t, roleName, data) + require.NoError(t, err) + require.False(t, resp.IsError(), "expecting no response error") + + rawGroups := resp.Data["groups"] + groups := rawGroups.([]string) + + assert.Len(t, groups, 2) + }) } // assertPermissionTarget inspects the actual PermissionTarget in Artifactory against the one in vault role. diff --git a/plugin/role.go b/plugin/role.go index 902de59..0bad5ea 100644 --- a/plugin/role.go +++ b/plugin/role.go @@ -30,7 +30,6 @@ const ( ) type RoleStorageEntry struct { - // `json:"" structs:"" mapstructure:""` // The UUID that defines this role RoleID string `json:"role_id" structs:"role_id" mapstructure:"role_id"` @@ -43,6 +42,13 @@ type RoleStorageEntry struct { // The provided name for the role Name string `json:"name" structs:"name" mapstructure:"name"` + // Groups are optional pre-existing static Artifactory groups to associate + // with the role and attach to the generated token scope. + // + // These groups are separate from the generated group that is created to + // accompany any configured permission targets. + Groups []string `json:"groups,omitempty" structs:"groups" mapstructure:"groups,omitempty"` + RawPermissionTargets string PermissionTargets []PermissionTarget } @@ -56,11 +62,13 @@ func (role RoleStorageEntry) validate() error { if role.RoleID == "" { err = multierror.Append(err, errors.New("role id is empty")) } - if role.RawPermissionTargets == "" { - err = multierror.Append(err, errors.New("raw permission targets are empty")) - } - if role.PermissionTargets == nil { - err = multierror.Append(err, errors.New("permission targets are empty")) + if len(role.Groups) == 0 { + if role.RawPermissionTargets == "" { + err = multierror.Append(err, errors.New("raw permission targets are empty")) + } + if role.PermissionTargets == nil { + err = multierror.Append(err, errors.New("permission targets are empty and groups are empty")) + } } return err.ErrorOrNil() } diff --git a/scripts/setup_dev_artifactory.sh b/scripts/setup_dev_artifactory.sh index 2e80b8b..b312fbc 100755 --- a/scripts/setup_dev_artifactory.sh +++ b/scripts/setup_dev_artifactory.sh @@ -2,6 +2,8 @@ set -euo pipefail +: "${ARTIFACTORY_LICENSE_KEY:?license key is required}" + ARTIFACTORY_URL="http://localhost:8081/artifactory/" ACCESS_URL="http://localhost:8082/access/" ARTIFACTORY_USER="admin" @@ -35,6 +37,12 @@ setup_artifactory() { echo done + # create some static groups to test with + for group in "group1" "group2" "group3"; do + payload=$(jq -n --arg name "$group" '{"name": $name, "description": "\( $name ) group", "auto_join": false}') + curl -sS -XPOST -H "$auth_header" -H "$content_header" "${ACCESS_URL}api/v2/groups" -d "$payload" || true + done + # # create a new admin user for UI use # password=$(openssl rand -base64 8) # payload=$(jq -n --arg pw "$password" '{userName: "dev", email: "dev@dev.net", password: $pw, admin: true}') @@ -53,4 +61,5 @@ setup_artifactory >&2 echo export ARTIFACTORY_USER=\"$ARTIFACTORY_USER\"\; echo export ARTIFACTORY_PASSWORD=\"$ARTIFACTORY_PASSWORD\"\; echo export ARTIFACTORY_URL=\"$ARTIFACTORY_URL\"\; +echo export ACCESS_URL=\"$ACCESS_URL\"\; echo export ARTIFACTORY_BEARER_TOKEN=\"$ARTIFACTORY_BEARER_TOKEN\"\; diff --git a/scripts/setup_dev_vault.sh b/scripts/setup_dev_vault.sh index 835f48c..8b1811d 100755 --- a/scripts/setup_dev_vault.sh +++ b/scripts/setup_dev_vault.sh @@ -1,8 +1,8 @@ #!/bin/bash -set -euo pipefail +set -eo pipefail -: ${ARTIFACTORY_URL:?unset} +: "${ARTIFACTORY_URL:?unset}" export VAULT_ADDR="http://localhost:8200" export VAULT_TOKEN=root @@ -19,7 +19,7 @@ setup_vault() { sha=$(shasum -a 256 plugins/$plugin | cut -d' ' -f1) # if plugin is missing, it is assumed this is a CI environment and vault is running in a container docker cp plugins/$plugin vault:/vault/plugins - vault plugin register -sha256=$sha secret $plugin + vault plugin register -sha256="$sha" secret "$plugin" fi echo "Enabling vault artifactory plugin..." @@ -29,13 +29,16 @@ setup_vault() { echo echo "Plugin enabled on path 'artifactory-cloud/':" echo "$existing" | jq + sha=$(shasum -a 256 plugins/$plugin | cut -d' ' -f1) + vault plugin register -sha256="$sha" secret "$plugin" + vault plugin reload -plugin=$plugin fi if [ -z "$ARTIFACTORY_BEARER_TOKEN" ]; then echo "ARTIFACTORY_BEARER_TOKEN unset, using username/password" - : ${ARTIFACTORY_USERNAME:?unset} + : ${ARTIFACTORY_USER:?unset} : ${ARTIFACTORY_PASSWORD:?unset} - vault write artifactory-cloud/config base_url=$ARTIFACTORY_URL username=$ARTIFACTORY_USERNAME password=$ARTIFACTORY_PASSWORD ttl=600 max_ttl=600 + vault write artifactory-cloud/config base_url=$ARTIFACTORY_URL username=$ARTIFACTORY_USER password=$ARTIFACTORY_PASSWORD ttl=600 max_ttl=600 else vault write artifactory-cloud/config base_url=$ARTIFACTORY_URL bearer_token=$ARTIFACTORY_BEARER_TOKEN ttl=600 max_ttl=24h fi