Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cyrillic email domain in Cognito UserPool does not work #8473

Closed
oliora opened this issue Jun 10, 2020 · 13 comments · Fixed by #11099
Closed

Cyrillic email domain in Cognito UserPool does not work #8473

oliora opened this issue Jun 10, 2020 · 13 comments · Fixed by #11099
Assignees
Labels
@aws-cdk/aws-cognito Related to Amazon Cognito bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md in-progress This issue is being actively worked on. p2

Comments

@oliora
Copy link

oliora commented Jun 10, 2020

It's not possible to use Cyrillic domain name like домен.рф in Cognito UserPool email settings.

Reproduction Steps

const cyrillic_domain = 'пример.рф';

export class SomeInfraStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    cognito.UserPool(this, 'userpool', {
      userPoolName: 'someuserpool',
      emailSettings: { from: 'noreply@' + cyrillic_domain }
  });
});

Error Log

 3/4 | 4:06:29 PM | CREATE_FAILED        | AWS::Cognito::UserPool | userpool (userpool0AC4AA96) Provided From email address is invalid (Service: AWSCognitoIdentityProviderService; Status Code: 400; Error Code: InvalidParameterException; Request ID: 84fa010b-8029-42f4-bc13-07d5273f1322)

The generated CloudFormation template (as seen in AWS Console) has question marks instead of Cyrillic characters.

Environment

  • CLI Version : 1.45.0 (build 0cfab15)
  • Node.js Version: v13.8.0
  • OS : macOS Mojave 10.14.6 (18G4032)
  • Language (Version): TypeScript

This is 🐛 Bug Report

@oliora oliora added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 10, 2020
@oliora
Copy link
Author

oliora commented Jun 10, 2020

After all I've found that Cyrillic domain name can be specified in Punycode. But it still would be nice to get an error on a non-ASCII character instead of producing an invalid CloudFormation template.

@SomayaB SomayaB added the @aws-cdk/aws-cognito Related to Amazon Cognito label Jun 11, 2020
@SomayaB SomayaB added the feature-request A feature should be added or improved. label Jun 11, 2020
@nija-at
Copy link
Contributor

nija-at commented Jun 12, 2020

Can you paste the section with AWS::Cognito::UserPool from the CloudFormation template here?

@nija-at nija-at added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Jun 12, 2020
@oliora
Copy link
Author

oliora commented Jun 12, 2020

Sorry, @nija-at but I don't have it anymore. But that domain parameter was simply ??????.?? instead of пример.рф so each Cyrillic character has been replaced with question mark.

@nija-at
Copy link
Contributor

nija-at commented Jun 12, 2020

This worked just fine for me. Here's the generated CloudFormation template -

< -- snip -- >
"myuserpool01998219": {
  "Type": "AWS::Cognito::UserPool",
  "Properties": {
    "EmailConfiguration": {
      "From": "noreply@myawesomeapp.com",
      "ReplyToEmailAddress": "support@домен.рф"
    },
< -- snip -- >

You'll need to check the encoding settings in your environment or provide more details.

@nija-at nija-at added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. feature-request A feature should be added or improved. labels Jun 12, 2020
@oliora
Copy link
Author

oliora commented Jun 12, 2020

Is this what you see locally or in cloud formation console? The question marks I’ve seen are on CF console stack’s template tab

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jun 13, 2020
@nija-at
Copy link
Contributor

nija-at commented Jun 16, 2020

This is what is shown locally on CF template. It's possible that the the console web UI mangles the characters but is actually correctly set up.

Can you check whether an email sent via this user pool is actually correctly set up?

@nija-at nija-at added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jun 16, 2020
@oliora
Copy link
Author

oliora commented Jun 16, 2020

No it was not. As you can see in original post, there was CREATE_FAILED CloudFormation event.

@rix0rrr
Copy link
Contributor

rix0rrr commented Jun 16, 2020

I'm going to gander a guess and say this is where the confusion happened:

The generated CloudFormation template (as seen in AWS Console) has question marks instead of Cyrillic characters.

I'm guessing that Niranjan saw that remark, missed the error message, and thought the template had deployed successfully.

@nija-at as a reminder of one of those niggling CloudFormation gotchas, if a stack fails to create (so not fails to update, but fails to create, on the first attempt) then it will still show the template you were trying to deploy. So it is possible for a stack to NOT be successfully created an still have template for you to look at in the console.

At the same time, I wouldn't be suprised the CloudFormation console doesn't handle unicode characters properly and shows ?????.??? for them, but that is a different issue from the one that's reported here, which is that the service doesn't accept unicode and we could help the user out by somewhat more eagerly validating with a hint to punycode (or punycode-encode for them automically, if we dare to)

@rix0rrr
Copy link
Contributor

rix0rrr commented Jun 16, 2020

I mean, automatically encoding unicode -> punycode for domain names would totally be in the "do the right thing" charter for CDK to do. I'm just a little worried about the dependencies required to do it. Let me have a look-see.

@nija-at
Copy link
Contributor

nija-at commented Jun 16, 2020

The generated CloudFormation template (as seen in AWS Console) has question marks instead of Cyrillic characters.

I'm guessing that Niranjan saw that remark, missed the error message, and thought the template had deployed successfully.

Correct.

The deployment indeed fails.

@rix0rrr
Copy link
Contributor

rix0rrr commented Jun 16, 2020

Okay punycode package has 0 dependencies. That's not too shabby...

@nija-at nija-at added p2 and removed response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Jun 16, 2020
@nija-at
Copy link
Contributor

nija-at commented Jun 16, 2020

A validation in the CDK here would be a fine first step.

Open for anyone who wants to submit a PR.

@nija-at nija-at added the good first issue Related to contributions. See CONTRIBUTING.md label Jun 16, 2020
@nija-at nija-at added the effort/small Small work item – less than a day of effort label Aug 6, 2020
@SomayaB SomayaB added the in-progress This issue is being actively worked on. label Oct 26, 2020
@mergify mergify bot closed this as completed in #11099 Nov 9, 2020
mergify bot pushed a commit that referenced this issue Nov 9, 2020
If an email address for cognito email settings contains a non-ascii character, the cognito cdk package applies punycode encoding, which cloudformation will pick up properly.

This pull request adds the [`punycode`](https://github.com/bestiejs/punycode.js) package to the dependencies, as previously discussed in #8473. The package occupies 42KB in `node_modules` after installation. I am not sure if I configured the package correctly to be included in the cdk build. The project itself added `punycode` to `aws-cdk-lib` and `monocdk`, I had to re-run `yarn install` in these packages.

Unit- and integration tests are added, but notice that I could not manage a manual e2e test, I think I need to own a domain with non-ascii letters to do so, which I currently do not.

A similar approach as in this PR will also be interesting for other cdk packages which deal with email addresses and domains, like hosted zones.

According to the [Cfn Domain Name Format Docs](https://docs.aws.amazon.com/Route53/latest/DeveloperGuide/DomainNameFormat.html), characters with code point below 33, or in `[127, 255]` should be escaped with an octal code, which is not applied in this pull request.

Fixes #8473

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Nov 9, 2020

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-cognito Related to Amazon Cognito bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md in-progress This issue is being actively worked on. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants