Merge pull request #905 from jfrog/fix-scoped-token-not-allow-group-n…

…ame-with-spaces

Fix scoped token not allow group name with spaces

CHANGELOG.md

@@ -1,3 +1,12 @@
+## 10.3.1 (Mar 14, 2024)
+
+BUG FIXES:
+
+* resource/artifactory_scoped_token: Fix `scopes` attribute handling for scope with space character wraps in double quote (e.g. group name). Issue: [#903](https://github.com/jfrog/terraform-provider-artifactory/issues/903)
+* resource/artifactory_api_key: Add API key deprecation notice to documentation (deprecation message already displays when uses via Terraform CLI).
+
+PR: [#905](https://github.com/jfrog/terraform-provider-artifactory/pull/905)
+
 ## 10.3.0 (Mar 11, 2024)
 
 FEATURES:

docs/resources/api_key.md

@@ -7,6 +7,7 @@ Provides an Artifactory API key resource. This can be used to create and manage
 
 ~> **Note:** API keys will be stored in the raw state as plain-text. [Read more about sensitive data in state](https://www.terraform.io/docs/state/sensitive-data.html).
 
+!> As notified in [Artifactory 7.47.10](https://jfrog.com/help/r/jfrog-release-information/artifactory-7.47.10-cloud-self-hosted), support for API Key is slated to be removed in a future release. To ease customer migration to [reference tokens](https://jfrog.com/help/r/jfrog-platform-administration-documentation/user-profile), which replaces API key, we are disabling the ability to create new API keys at the end of Q3 2024. The ability to use API keys will be removed at the end of Q4 2024. For more information, see [JFrog API Key Deprecation Process](https://jfrog.com/help/r/jfrog-platform-administration-documentation/jfrog-api-key-deprecation-process).
 
 ## Example Usage
 
@@ -19,11 +20,7 @@ resource "artifactory_api_key" "ci" {}
 
 The following attributes are exported:
 
-* `api_key` - The API key. Deprecated. An upcoming version will support the option to block the usage/creation of API Keys (for admins to set on their platform).
-  In September 2022, the option to block the usage/creation of API Keys will be enabled by default, with the option for admins to change it back to enable API Keys.
-  In January 2023, API Keys will be deprecated all together and the option to use them will no longer be available.
-  It is recommended to use scoped tokens instead - `artifactory_scoped_token` resource.
-  Please check the [release notes](https://www.jfrog.com/confluence/display/JFROG/Artifactory+Release+Notes#ArtifactoryReleaseNotes-Artifactory7.38.4).
+* `api_key` - The API key. Deprecated.
 
 ## Import
 

go.mod

@@ -38,6 +38,7 @@ require (
 	github.com/bgentry/speakeasy v0.1.0 // indirect
 	github.com/cloudflare/circl v1.3.7 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
+	github.com/dlclark/regexp2 v1.11.0
 	github.com/fatih/color v1.13.0 // indirect
 	github.com/golang/protobuf v1.5.3 // indirect
 	github.com/google/go-cmp v0.6.0 // indirect

go.sum

@@ -34,6 +34,8 @@ github.com/cyphar/filepath-securejoin v0.2.4/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxG
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/dlclark/regexp2 v1.11.0 h1:G/nrcoOa7ZXlpoa/91N3X7mM3r8eIlMBBJZvsz/mxKI=
+github.com/dlclark/regexp2 v1.11.0/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8=
 github.com/emirpasic/gods v1.18.1 h1:FXtiHYKDGKCW2KzwZKx0iC0PQmdlorYgdFG9jPXJ1Bc=
 github.com/emirpasic/gods v1.18.1/go.mod h1:8tpGGwCnJ5H4r6BWwaV6OrWmMoPhUl5jm/FMNAnJvWQ=
 github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4=

pkg/artifactory/resource/security/resource_artifactory_api_key.go

@@ -34,9 +34,10 @@ func ResourceArtifactoryApiKey() *schema.Resource {
 
 		Schema: map[string]*schema.Schema{
 			"api_key": {
-				Type:      schema.TypeString,
-				Computed:  true,
-				Sensitive: true,
+				Type:       schema.TypeString,
+				Computed:   true,
+				Sensitive:  true,
+				Deprecated: "Deprecated in favor of \"artifactory_scoped_token\".",
 			},
 		},
 	}
@@ -48,7 +49,7 @@ func packApiKey(apiKey string, d *schema.ResourceData) diag.Diagnostics {
 
 	errors := setValue("api_key", apiKey)
 
-	if errors != nil && len(errors) > 0 {
+	if len(errors) > 0 {
 		return diag.Errorf("failed to pack api key %q", errors)
 	}
 

pkg/artifactory/resource/security/resource_artifactory_permission_target.go

@@ -156,7 +156,8 @@ func ResourceArtifactoryPermissionTarget() *schema.Resource {
 			StateContext: schema.ImportStatePassthroughContext,
 		},
 
-		Schema: BuildPermissionTargetSchema(),
+		Schema:             BuildPermissionTargetSchema(),
+		DeprecationMessage: `This resource has been deprecated in favour of "platform_permission" (https://registry.terraform.io/providers/jfrog/platform/latest/docs/resources/permission) resource.`,
 	}
 }
 
@@ -168,7 +169,7 @@ func hashPrincipal(o interface{}) int {
 	return part1 * part3
 }
 
-func unpackPermissionTarget(ctx context.Context, s *schema.ResourceData) *PermissionTargetParams {
+func unpackPermissionTarget(_ context.Context, s *schema.ResourceData) *PermissionTargetParams {
 	d := &utilsdk.ResourceData{ResourceData: s}
 
 	unpackPermission := func(rawPermissionData interface{}) *PermissionTargetSection {

pkg/artifactory/resource/security/resource_artifactory_scoped_token.go

@@ -7,6 +7,7 @@ import (
 	"regexp"
 	"strings"
 
+	regex2 "github.com/dlclark/regexp2"
 	"github.com/go-resty/resty/v2"
 	"github.com/hashicorp/terraform-plugin-framework-validators/int64validator"
 	"github.com/hashicorp/terraform-plugin-framework-validators/setvalidator"
@@ -23,8 +24,10 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/schema/validator"
 	"github.com/hashicorp/terraform-plugin-framework/types"
+	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"github.com/jfrog/terraform-provider-shared/util"
 	utilfw "github.com/jfrog/terraform-provider-shared/util/fw"
+	"golang.org/x/exp/slices"
 )
 
 func NewScopedTokenResource() resource.Resource {
@@ -488,17 +491,82 @@ func (r *ScopedTokenResource) ImportState(ctx context.Context, req resource.Impo
 	)
 }
 
+// splitScopes use positive lookahead regex to find the space character between scopes
+// but ignore group name with space wraps in double quotes
+func (r *ScopedTokenResourceModel) splitScopes(ctx context.Context, scopes string) []string {
+	if scopes == "" {
+		return []string{}
+	}
+
+	// regexp doesn't support lookahead so we have to import regex2 lib which does
+	re := regex2.MustCompile(`\s(?=(?:[^"]*"[^"]*")*[^"]*$)`, 0)
+	match, err := re.FindStringMatch(scopes)
+	if err != nil {
+		tflog.Warn(ctx, "fail to find scopes match", map[string]any{
+			"err": err,
+		})
+	}
+
+	// return the entire 'scopes' string if there's no space separator, i.e. only one item in the list
+	if match == nil {
+		return []string{scopes}
+	}
+
+	separatorIndices := []int{}
+	// collect all the indices for the space delimited scope string
+	for ok := true; ok; ok = (match != nil) { // mimic do...while loop
+		for _, g := range match.Groups() {
+			for _, c := range g.Captures {
+				separatorIndices = append(separatorIndices, c.Index)
+			}
+		}
+
+		match, err = re.FindNextMatch(match)
+		if err != nil {
+			tflog.Warn(ctx, "fail to find next scopes match", map[string]any{
+				"err": err,
+			})
+		}
+	}
+	tflog.Debug(ctx, "ScopedTokenResourceModel.splitScopes", map[string]any{
+		"separatorIndices": separatorIndices,
+	})
+
+	// insert a zero to the begining of the slice to represent the first index
+	separatorIndices = append([]int{0}, separatorIndices...)
+	// reverse the slice so the string splitting starts from the end
+	slices.Reverse(separatorIndices)
+
+	scopesCopy := scopes
+	scopesList := []string{}
+	for _, idx := range separatorIndices {
+		// pad the start index by 1 to take care of the space prefix character
+		startIdx := idx + 1
+		if idx == 0 {
+			startIdx = 0
+		}
+		scopesList = append(scopesList, scopesCopy[startIdx:])
+		// trim the end of string off for next iteration
+		scopesCopy = scopesCopy[:idx]
+	}
+	tflog.Debug(ctx, "ScopedTokenResourceModel.splitScopes", map[string]any{
+		"scopesList": scopesList,
+	})
+	return scopesList
+}
+
 func (r *ScopedTokenResourceModel) PostResponseToState(ctx context.Context,
 	accessTokenResp *AccessTokenPostResponseAPIModel, accessTokenPostBody *AccessTokenPostRequestAPIModel, getResult *AccessTokenGetAPIModel) diag.Diagnostics {
 
 	r.Id = types.StringValue(accessTokenResp.TokenId)
 
 	if len(accessTokenResp.Scope) > 0 {
-		scopesList := strings.Split(accessTokenResp.Scope, " ")
+		scopesList := r.splitScopes(ctx, accessTokenResp.Scope)
 		scopes, diags := types.SetValueFrom(ctx, types.StringType, scopesList)
 		if diags != nil {
 			return diags
 		}
+
 		r.Scopes = scopes
 	}
 

pkg/artifactory/resource/security/resource_artifactory_scoped_token_test.go

@@ -358,22 +358,24 @@ func TestAccScopedToken_WithAttributes(t *testing.T) {
 	})
 }
 
-func TestAccScopedToken_WithGroupScope(t *testing.T) {
+func TestAccScopedToken_WithSingleGroupScope(t *testing.T) {
 	_, fqrn, name := testutil.MkNames("test-access-token", "artifactory_scoped_token")
 
 	accessTokenConfig := util.ExecuteTemplate(
 		"TestAccScopedToken",
-		`resource "artifactory_group" "test-group" {
-			name = "{{ .groupName }}"
+		`resource "artifactory_group" "test-group-1" {
+			name = "{{ .groupName1 }}"
 		}
 
 		resource "artifactory_scoped_token" "{{ .name }}" {
-			username    = artifactory_group.test-group.name
-			scopes      = ["applied-permissions/groups:{{ .groupName }}"]
+			username    = artifactory_group.test-group-1.name
+			scopes      = [
+				"applied-permissions/groups:{{ .groupName1 }}",
+			]
 		}`,
 		map[string]interface{}{
-			"name":      name,
-			"groupName": "test-group",
+			"name":       name,
+			"groupName1": "test-group-1",
 		},
 	)
 
@@ -384,9 +386,65 @@ func TestAccScopedToken_WithGroupScope(t *testing.T) {
 			{
 				Config: accessTokenConfig,
 				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttr(fqrn, "username", "test-group"),
+					resource.TestCheckResourceAttr(fqrn, "username", "test-group-1"),
 					resource.TestCheckResourceAttr(fqrn, "scopes.#", "1"),
-					resource.TestCheckTypeSetElemAttr(fqrn, "scopes.*", "applied-permissions/groups:test-group"),
+					resource.TestCheckTypeSetElemAttr(fqrn, "scopes.*", "applied-permissions/groups:test-group-1"),
+				),
+			},
+			{
+				ResourceName: fqrn,
+				ImportState:  true,
+				ExpectError:  regexp.MustCompile("resource artifactory_scoped_token doesn't support import"),
+			},
+		},
+	})
+}
+
+func TestAccScopedToken_WithMultipleGroupScopes(t *testing.T) {
+	_, fqrn, name := testutil.MkNames("test-access-token", "artifactory_scoped_token")
+
+	accessTokenConfig := util.ExecuteTemplate(
+		"TestAccScopedToken",
+		`resource "artifactory_group" "test-group-1" {
+			name = "{{ .groupName1 }}"
+		}
+
+		resource "artifactory_group" "test-group-2" {
+			name = "{{ .groupName2 }}"
+		}
+
+		resource "artifactory_group" "test-group-3" {
+			name = "{{ .groupName3 }}"
+		}
+
+		resource "artifactory_scoped_token" "{{ .name }}" {
+			username    = artifactory_group.test-group-1.name
+			scopes      = [
+				"applied-permissions/groups:\"{{ .groupName1 }}\"",
+				"applied-permissions/groups:${artifactory_group.test-group-2.name}",
+				"applied-permissions/groups:${artifactory_group.test-group-3.name}",
+			]
+		}`,
+		map[string]interface{}{
+			"name":       name,
+			"groupName1": "test group 1",
+			"groupName2": "test-group-2",
+			"groupName3": "test-group-3",
+		},
+	)
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:                 func() { acctest.PreCheck(t) },
+		ProtoV6ProviderFactories: acctest.ProtoV6ProviderFactories,
+		Steps: []resource.TestStep{
+			{
+				Config: accessTokenConfig,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(fqrn, "username", "test group 1"),
+					resource.TestCheckResourceAttr(fqrn, "scopes.#", "3"),
+					resource.TestCheckTypeSetElemAttr(fqrn, "scopes.*", "applied-permissions/groups:\"test group 1\""),
+					resource.TestCheckTypeSetElemAttr(fqrn, "scopes.*", "applied-permissions/groups:test-group-2"),
+					resource.TestCheckTypeSetElemAttr(fqrn, "scopes.*", "applied-permissions/groups:test-group-3"),
 				),
 			},
 			{