Skip to content

Commit

Permalink
Merge pull request #41187 from acwwat/b-aws_cognito_user_pool_client-…
Browse files Browse the repository at this point in the history 
…fix_utf8_str_len_checks

fix: Fix string length checks for UTF-8 fields for aws_cognito_identity_provider, aws_cognito_user_group, and aws_cognito_user_pool_client
  • Loading branch information
@ewbankkit
ewbankkit authored Feb 3, 2025
2 parents 0bb20e1 + a0c5787 commit 97d572b
Show file tree
Hide file tree
Showing 7 changed files with 205 additions and 21 deletions.
11 changes: 11 additions & 0 deletions .changelog/41187.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
```release-note:bug
resource/aws_cognito_identity_provider: Correct plan-time validation of `provider_name` to count UTF-8 characters properly
```

```release-note:bug
resource/aws_cognito_user_group: Correct plan-time validation of `name` to count UTF-8 characters properly
```

```release-note:bug
resource/aws_cognito_user_pool_client: Correct plan-time validation of `callback_urls, `default_redirect_uri`, `logout_urls`, and `supported_identity_providers` to count UTF-8 characters properly
```
11 changes: 4 additions & 7 deletions internal/service/cognitoidp/identity_provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,10 @@ func resourceIdentityProvider() *schema.Resource {
Elem: &schema.Schema{Type: schema.TypeString},
},
names.AttrProviderName: {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validation.All(
validation.StringLenBetween(1, 32),
validation.StringMatch(regexache.MustCompile(`^[^_][\p{L}\p{M}\p{S}\p{N}\p{P}][^_]+$`), "see https://docs.aws.amazon.com/cognito-user-identity-pools/latest/APIReference/API_CreateIdentityProvider.html#API_CreateIdentityProvider_RequestSyntax"),
),
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validIdentityProviderName,
},
"provider_type": {
Type: schema.TypeString,
Expand Down
30 changes: 24 additions & 6 deletions internal/service/cognitoidp/identity_provider_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package cognitoidp_test
import (
"context"
"fmt"
"strings"
"testing"

awstypes "github.com/aws/aws-sdk-go-v2/service/cognitoidentityprovider/types"
Expand Down Expand Up @@ -123,6 +124,7 @@ func TestAccCognitoIDPIdentityProvider_saml(t *testing.T) {
var identityProvider awstypes.IdentityProviderType
resourceName := "aws_cognito_identity_provider.test"
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
rNameUTF8 := strings.Repeat("あ", 32)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t); testAccPreCheckIdentityProvider(ctx, t) },
Expand All @@ -131,7 +133,7 @@ func TestAccCognitoIDPIdentityProvider_saml(t *testing.T) {
CheckDestroy: testAccCheckIdentityProviderDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccIdentityProviderConfig_saml(rName, acctest.CtFalse),
Config: testAccIdentityProviderConfig_saml(rName, rName, acctest.CtFalse),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckIdentityProviderExists(ctx, resourceName, &identityProvider),
resource.TestCheckResourceAttr(resourceName, "attribute_mapping.%", "1"),
Expand All @@ -152,7 +154,7 @@ func TestAccCognitoIDPIdentityProvider_saml(t *testing.T) {
ImportStateVerify: true,
},
{
Config: testAccIdentityProviderConfig_saml(rName, acctest.CtTrue),
Config: testAccIdentityProviderConfig_saml(rName, rName, acctest.CtTrue),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckIdentityProviderExists(ctx, resourceName, &identityProvider),
resource.TestCheckResourceAttr(resourceName, "attribute_mapping.%", "1"),
Expand All @@ -167,6 +169,22 @@ func TestAccCognitoIDPIdentityProvider_saml(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "provider_type", "SAML"),
),
},
{
Config: testAccIdentityProviderConfig_saml(rNameUTF8, rName, acctest.CtTrue),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckIdentityProviderExists(ctx, resourceName, &identityProvider),
resource.TestCheckResourceAttr(resourceName, "attribute_mapping.%", "1"),
resource.TestCheckResourceAttr(resourceName, "attribute_mapping.email", names.AttrEmail),
resource.TestCheckNoResourceAttr(resourceName, "idp_identifiers.#"),
resource.TestCheckResourceAttr(resourceName, "provider_details.%", "4"),
resource.TestCheckResourceAttrSet(resourceName, "provider_details.ActiveEncryptionCertificate"),
resource.TestCheckResourceAttr(resourceName, "provider_details.EncryptedResponses", acctest.CtTrue),
resource.TestCheckResourceAttrSet(resourceName, "provider_details.MetadataFile"),
resource.TestCheckResourceAttr(resourceName, "provider_details.SSORedirectBindingURI", "https://terraform-dev-ed.my.salesforce.com/idp/endpoint/HttpRedirect"),
resource.TestCheckResourceAttr(resourceName, names.AttrProviderName, rNameUTF8),
resource.TestCheckResourceAttr(resourceName, "provider_type", "SAML"),
),
},
},
})
}
Expand Down Expand Up @@ -354,10 +372,10 @@ resource "aws_cognito_identity_provider" "test" {
`, rName, attribute)
}

func testAccIdentityProviderConfig_saml(rName, encryptedResponses string) string {
func testAccIdentityProviderConfig_saml(rName, userPoolName, encryptedResponses string) string {
return fmt.Sprintf(`
resource "aws_cognito_user_pool" "test" {
name = %[1]q
name = %[2]q
auto_verified_attributes = ["email"]
}
Expand All @@ -367,7 +385,7 @@ resource "aws_cognito_identity_provider" "test" {
provider_type = "SAML"
provider_details = {
EncryptedResponses = %[2]q
EncryptedResponses = %[3]q
MetadataFile = file("./test-fixtures/saml-metadata.xml")
SSORedirectBindingURI = "https://terraform-dev-ed.my.salesforce.com/idp/endpoint/HttpRedirect"
}
Expand All @@ -380,5 +398,5 @@ resource "aws_cognito_identity_provider" "test" {
ignore_changes = [provider_details["ActiveEncryptionCertificate"]]
}
}
`, rName, encryptedResponses)
`, rName, userPoolName, encryptedResponses)
}
9 changes: 9 additions & 0 deletions internal/service/cognitoidp/user_group_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package cognitoidp_test
import (
"context"
"fmt"
"strings"
"testing"

sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest"
Expand All @@ -23,6 +24,7 @@ func TestAccCognitoIDPUserGroup_basic(t *testing.T) {
poolName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
groupName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
updatedGroupName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
updatedGroupNameUTF8 := strings.Repeat("あ", 128)
resourceName := "aws_cognito_user_group.test"

resource.ParallelTest(t, resource.TestCase{
Expand Down Expand Up @@ -50,6 +52,13 @@ func TestAccCognitoIDPUserGroup_basic(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, names.AttrName, updatedGroupName),
),
},
{
Config: testAccUserGroupConfig_basic(poolName, updatedGroupNameUTF8),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckUserGroupExists(ctx, resourceName),
resource.TestCheckResourceAttr(resourceName, names.AttrName, updatedGroupNameUTF8),
),
},
},
})
}
Expand Down
94 changes: 94 additions & 0 deletions internal/service/cognitoidp/user_pool_client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package cognitoidp_test
import (
"context"
"fmt"
"strings"
"testing"

"github.com/YakDriver/regexache"
Expand Down Expand Up @@ -1179,6 +1180,69 @@ func TestAccCognitoIDPUserPoolClient_frameworkMigration_emptySet(t *testing.T) {
})
}

func TestAccCognitoIDPUserPoolClient_supportedIdentityProviders_utf8(t *testing.T) {
ctx := acctest.Context(t)
var client awstypes.UserPoolClientType
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_cognito_user_pool_client.test"
identityProvider := strings.Repeat("あ", 32)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t); testAccPreCheckIdentityProvider(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, names.CognitoIDPServiceID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckUserPoolClientDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccUserPoolClientConfig_supportedIdentityProviders_utf8(rName, identityProvider),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckUserPoolClientExists(ctx, resourceName, &client),
resource.TestCheckResourceAttr(resourceName, "supported_identity_providers.0", identityProvider),
),
},
{
ResourceName: resourceName,
ImportStateIdFunc: testAccUserPoolClientImportStateIDFunc(ctx, resourceName),
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func TestAccCognitoIDPUserPoolClient_urls_utf8(t *testing.T) {
ctx := acctest.Context(t)
var client awstypes.UserPoolClientType
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_cognito_user_pool_client.test"
defaultRedirectURI := "https://www.example.com/redirect/" + strings.Repeat("あ", 991)
logoutURL := "https://www.example.com/logout/" + strings.Repeat("あ", 993)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t); testAccPreCheckIdentityProvider(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, names.CognitoIDPServiceID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckUserPoolClientDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccUserPoolClientConfig_urls_utf8(rName, defaultRedirectURI, logoutURL),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckUserPoolClientExists(ctx, resourceName, &client),
resource.TestCheckResourceAttr(resourceName, "callback_urls.0", defaultRedirectURI),
resource.TestCheckResourceAttr(resourceName, "default_redirect_uri", defaultRedirectURI),
resource.TestCheckResourceAttr(resourceName, "logout_urls.0", logoutURL),
),
},
{
ResourceName: resourceName,
ImportStateIdFunc: testAccUserPoolClientImportStateIDFunc(ctx, resourceName),
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func testAccUserPoolClientImportStateIDFunc(ctx context.Context, n string) resource.ImportStateIdFunc {
return func(s *terraform.State) (string, error) {
rs, ok := s.RootModule().Resources[n]
Expand Down Expand Up @@ -1641,3 +1705,33 @@ resource "aws_cognito_user_pool_client" "test" {
}
`, rName))
}

func testAccUserPoolClientConfig_supportedIdentityProviders_utf8(rName, identityProvider string) string {
return acctest.ConfigCompose(
testAccIdentityProviderConfig_saml(identityProvider, rName, acctest.CtTrue),
fmt.Sprintf(`
resource "aws_cognito_user_pool_client" "test" {
name = %[1]q
user_pool_id = aws_cognito_user_pool.test.id
supported_identity_providers = [%[2]q]
depends_on = [aws_cognito_identity_provider.test]
}
`, rName, identityProvider))
}

func testAccUserPoolClientConfig_urls_utf8(rName, defaultRedirectUri, logoutUrl string) string {
return acctest.ConfigCompose(
testAccUserPoolClientConfig_base(rName),
fmt.Sprintf(`
resource "aws_cognito_user_pool_client" "test" {
name = %[1]q
user_pool_id = aws_cognito_user_pool.test.id
callback_urls = [%[2]q]
default_redirect_uri = %[2]q
logout_urls = [%[3]q]
}
`, rName, defaultRedirectUri, logoutUrl))
}
33 changes: 25 additions & 8 deletions internal/service/cognitoidp/validate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,30 @@ import (
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
)

func validIdentityProviderName(v interface{}, k string) (ws []string, es []error) {
value := v.(string)
count := utf8.RuneCountInString(value)
if count < 1 {
es = append(es, fmt.Errorf("%q cannot be less than 1 UTF-8 character", k))
}

if count > 32 {
es = append(es, fmt.Errorf("%q cannot be longer than 32 UTF-8 characters", k))
}
if !regexache.MustCompile(`[\p{L}\p{M}\p{S}\p{N}\p{P}\s]+`).MatchString(value) {
es = append(es, fmt.Errorf(`%q must satisfy regular expression pattern: [\p{L}\p{M}\p{S}\p{N}\p{P}\s]+`, k))
}
return
}

func validResourceServerScopeName(v interface{}, k string) (ws []string, errors []error) {
value := v.(string)

if len(value) < 1 {
errors = append(errors, fmt.Errorf("%q cannot be less than 1 character", k))
}
if len(value) > 256 {
errors = append(errors, fmt.Errorf("%q cannot be longer than 256 character", k))
errors = append(errors, fmt.Errorf("%q cannot be longer than 256 characters", k))
}
if !regexache.MustCompile(`[\x21\x23-\x2E\x30-\x5B\x5D-\x7E]+`).MatchString(value) {
errors = append(errors, fmt.Errorf(`%q must satisfy regular expression pattern: [\x21\x23-\x2E\x30-\x5B\x5D-\x7E]+`, k))
Expand All @@ -29,12 +45,13 @@ func validResourceServerScopeName(v interface{}, k string) (ws []string, errors

func validUserGroupName(v interface{}, k string) (ws []string, es []error) {
value := v.(string)
if len(value) < 1 {
es = append(es, fmt.Errorf("%q cannot be less than 1 character", k))
count := utf8.RuneCountInString(value)
if count < 1 {
es = append(es, fmt.Errorf("%q cannot be less than 1 UTF-8 character", k))
}

if len(value) > 128 {
es = append(es, fmt.Errorf("%q cannot be longer than 128 character", k))
if count > 128 {
es = append(es, fmt.Errorf("%q cannot be longer than 128 UTF-8 characters", k))
}

if !regexache.MustCompile(`[\p{L}\p{M}\p{S}\p{N}\p{P}]+`).MatchString(value) {
Expand Down Expand Up @@ -134,7 +151,7 @@ func validUserPoolSchemaName(v interface{}, k string) (ws []string, es []error)
}

if len(value) > 20 {
es = append(es, fmt.Errorf("%q cannot be longer than 20 character", k))
es = append(es, fmt.Errorf("%q cannot be longer than 20 characters", k))
}

if !regexache.MustCompile(`[\p{L}\p{M}\p{S}\p{N}\p{P}]+`).MatchString(value) {
Expand Down Expand Up @@ -263,7 +280,7 @@ func validUserPoolTemplateSMSMessage(v interface{}, k string) (ws []string, es [
}

var userPoolClientIdentityProviderValidator = []validator.String{
stringvalidator.LengthBetween(1, 32),
stringvalidator.UTF8LengthBetween(1, 32),
stringValidatorpLpMpSpNpP,
}

Expand All @@ -273,7 +290,7 @@ var userPoolClientNameValidator = []validator.String{
}

var userPoolClientURLValidator = []validator.String{
stringvalidator.LengthBetween(1, 1024),
stringvalidator.UTF8LengthBetween(1, 1024),
stringValidatorpLpMpSpNpP,
}

Expand Down
38 changes: 38 additions & 0 deletions internal/service/cognitoidp/validate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,42 @@ import (
"github.com/hashicorp/terraform-provider-aws/names"
)

func TestValidIdentityProviderName(t *testing.T) {
t.Parallel()

validValues := []string{
"foo",
"7346241598935552",
"foo_bar",
"foo:bar",
"foo/bar",
"foo-bar",
"$foobar",
strings.Repeat("W", 32),
strings.Repeat("あ", 32), // UTF-8 (2 bytes/char)
}

for _, s := range validValues {
_, errors := validIdentityProviderName(s, names.AttrName)
if len(errors) > 0 {
t.Fatalf("%q should be a valid Cognito Identity Provider Name: %v", s, errors)
}
}

invalidValues := []string{
"",
strings.Repeat("W", 33), // > 32
strings.Repeat("あ", 33), // UTF-8 (2 bytes/char)
}

for _, s := range invalidValues {
_, errors := validIdentityProviderName(s, names.AttrName)
if len(errors) == 0 {
t.Fatalf("%q should not be a valid Cognito Identity Provider Name: %v", s, errors)
}
}
}

func TestValidUserGroupName(t *testing.T) {
t.Parallel()

Expand All @@ -22,6 +58,7 @@ func TestValidUserGroupName(t *testing.T) {
"foo-bar",
"$foobar",
strings.Repeat("W", 128),
strings.Repeat("あ", 128), // UTF-8 (2 bytes/char)
}

for _, s := range validValues {
Expand All @@ -34,6 +71,7 @@ func TestValidUserGroupName(t *testing.T) {
invalidValues := []string{
"",
strings.Repeat("W", 129), // > 128
strings.Repeat("あ", 129), // UTF-8 (2 bytes/char)
}

for _, s := range invalidValues {
Expand Down

0 comments on commit 97d572b

Please sign in to comment.