update with suggestions

hashicorp · Nov 21, 2023 · d53d87a · d53d87a
1 parent 0fb3774
commit d53d87a
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 8 deletions.
diff --git a/api/applications.go b/api/applications.go
@@ -124,9 +124,13 @@ func (c *MSGraphClient) ListApplications(ctx context.Context, filter string) ([]
 func (c *MSGraphClient) CreateApplication(ctx context.Context, displayName string, signInAudience string, tags []string) (Application, error) {
 	requestBody := models.NewApplication()
 	requestBody.SetDisplayName(&displayName)
-	requestBody.SetSignInAudience(&signInAudience)
 	requestBody.SetTags(tags)
 
+	// only set signInAudience if it's non-empty
+	if signInAudience != "" {
+		requestBody.SetSignInAudience(&signInAudience)
+	}
+
 	resp, err := c.client.Applications().Post(ctx, requestBody, nil)
 	if err != nil {
 		return Application{}, err

diff --git a/path_roles.go b/path_roles.go
@@ -91,7 +91,7 @@ func pathsRole(b *azureSecretBackend) []*framework.Path {
 				},
 				"sign_in_audience": {
 					Type:        framework.TypeString,
-					Description: "Specifies the security principal types that are allowed to sign in to the application.",
+					Description: "Specifies the security principal types that are allowed to sign in to the application. Valid values are: AzureADMyOrg, AzureADMultipleOrgs, AzureADandPersonalMicrosoftAccount, PersonalMicrosoftAccount",
 				},
 				"tags": {
 					Type:        framework.TypeCommaStringSlice,
@@ -218,13 +218,47 @@ func (b *azureSecretBackend) pathRoleUpdate(ctx context.Context, req *logical.Re
 		role.PermanentlyDelete = false
 	}
 
-	// update and verify SignInAudience if provided
+	// update and validate SignInAudience if provided
 	if signInAudience, ok := d.GetOk("sign_in_audience"); ok {
-		role.SignInAudience = signInAudience.(string)
+		signInAudienceValue, ok := signInAudience.(string)
+		if !ok {
+			return logical.ErrorResponse("Invalid type for sign_in_audience field. Expected string."), nil
+		}
+
+		validSignInAudiences := []string{"AzureADMyOrg", "AzureADMultipleOrgs", "AzureADandPersonalMicrosoftAccount", "PersonalMicrosoftAccount"}
+		valid := false
+		for _, validValue := range validSignInAudiences {
+			if signInAudienceValue == validValue {
+				valid = true
+				break
+			}
+		}
+		if !valid {
+			validValuesString := strings.Join(validSignInAudiences, ", ")
+			return logical.ErrorResponse("Invalid value for sign_in_audience field. Valid values are: %s", validValuesString), nil
+		}
+		role.SignInAudience = signInAudienceValue
 	}
 
+	// update and validate Tags if provided
 	if tags, ok := d.GetOk("tags"); ok {
 		if tagsList, ok := tags.([]string); ok {
+			uniqueTags := make(map[string]struct{})
+			for _, tag := range tagsList {
+				// Check individual tag size
+				if len(tag) < 1 || len(tag) > 256 {
+					return logical.ErrorResponse("individual tag size must be between 1 and 256 characters (inclusive)"), nil
+				}
+				// Check for whitespaces
+				if strings.Contains(tag, " ") {
+					return logical.ErrorResponse("whitespaces are not allowed in tags"), nil
+				}
+				// Check for duplicates
+				if _, exists := uniqueTags[tag]; exists {
+					return logical.ErrorResponse("duplicate tags are not allowed"), nil
+				}
+				uniqueTags[tag] = struct{}{}
+			}
 			role.Tags = tagsList
 		} else {
 			return logical.ErrorResponse("expected tags to be []string, but got %T", tags), nil

diff --git a/path_roles_test.go b/path_roles_test.go
@@ -197,8 +197,8 @@ func TestRoleCreate(t *testing.T) {
 			"max_ttl":               int64(3000),
 			"azure_roles":           "[]",
 			"azure_groups":          "[]",
-			"sign_in_audience":      "",
-			"tags":                  []string{""},
+			"sign_in_audience":      "PersonalMicrosoftAccount",
+			"tags":                  []string{"environment:production"},
 			"permanently_delete":    false,
 			"persist_app":           false,
 		}
@@ -223,8 +223,8 @@ func TestRoleCreate(t *testing.T) {
 				}]`,
 			),
 			"application_object_id": "",
-			"sign_in_audience":      "",
-			"tags":                  []string{""},
+			"sign_in_audience":      "AzureADandPersonalMicrosoftAccount",
+			"tags":                  []string{"team:engineering", "environment:development", "project:vault_testing"},
 			"azure_groups":          "[]",
 			"persist_app":           false,
 		}
@@ -547,6 +547,22 @@ func TestRoleCreateBad(t *testing.T) {
 	if !strings.Contains(resp.Error().Error(), msg) {
 		t.Fatalf("expected to find: %s, got: %s", msg, resp.Error().Error())
 	}
+
+	// invalid tags
+	role = map[string]interface{}{"tags": []string{"team:engineering", "team:engineering"}}
+	resp = testRoleCreateBasic(t, b, s, "test_role_1", role)
+	msg = "duplicate tags are not allowed"
+	if !strings.Contains(resp.Error().Error(), msg) {
+		t.Fatalf("expected to find: %s, got: %s", msg, resp.Error().Error())
+	}
+
+	// invalid signInAudience
+	role = map[string]interface{}{"sign_in_audience": "asdfg"}
+	resp = testRoleCreateBasic(t, b, s, "test_role_1", role)
+	msg = "Invalid value for sign_in_audience field. Valid values are: AzureADMyOrg, AzureADMultipleOrgs, AzureADandPersonalMicrosoftAccount, PersonalMicrosoftAccount"
+	if !strings.Contains(resp.Error().Error(), msg) {
+		t.Fatalf("expected to find: %s, got: %s", msg, resp.Error().Error())
+	}
 }
 
 func TestRoleUpdateError(t *testing.T) {