Merge pull request #145 from splunk/add-static-groups

Allow for static groups

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

README.md

@@ -1,4 +1,3 @@
-<!-- omit in toc -->
 # 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

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:  "*@*",

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 |
 | ----- | ---------------- | -------- |

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.

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()
 }

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: "[email protected]", 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\"\;

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