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

iam: SamlConsolePrincipal does not work in aws.amazon.com-regions #24243

Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management bug This issue is a bug. effort/small Small work item – less than a day of effort p0

Comments

@Wurstnase
Copy link
Contributor

Describe the bug

With #24034 the default SAML:aud changed from aws.amazon.com to "Ref": "AWS::URLSuffix" which is resolves to amazonaws.com.

Expected Behavior

When not in china, it should resolves to aws.amazon.com.

Current Behavior

it is "Ref": "AWS::URLSuffix", which resolves to amazonaws.com.

Reproduction Steps

install 2.65.0
create an iam.SamlConsolePrincipal

Possible Solution

https:/zorrofox/aws-cdk/blob/f8fe1d292feb3fc39a99687bf454a829302c4ff5/packages/%40aws-cdk/aws-iam/lib/principals.ts#L740

      StringEquals: {
-        'SAML:aud': cdk.Aws.PARTITION==='aws-cn'? 'https://signin.amazonaws.cn/saml': `https://signin.${cdk.Aws.URL_SUFFIX}/saml`,
+        'SAML:aud': cdk.Aws.PARTITION==='aws-cn'? 'https://signin.amazonaws.cn/saml': `https://signin.aws.amazon.com/saml`, 
      },

Additional Information/Context

No response

CDK CLI Version

2.65.0

Framework Version

No response

Node.js Version

OS

Language

Python

Language Version

No response

Other information

No response

@Wurstnase Wurstnase added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 21, 2023
@github-actions github-actions bot added the @aws-cdk/aws-iam Related to AWS Identity and Access Management label Feb 21, 2023
@Wurstnase
Copy link
Contributor Author

Wurstnase commented Feb 22, 2023

Here is the list of all possible endpoints: https://docs.aws.amazon.com/general/latest/gr/signin-service.html

I'm wondering, that i did not find amazonaws.cn in this list as in #24034 introduced.
Little bit hidden: https://docs.amazonaws.cn/en_us/IAM/latest/UserGuide/id_roles_providers_enable-console-saml.html

@pahud
Copy link
Contributor

pahud commented Feb 22, 2023

Yes I think you are right.

According to the doc, urlsuffix is typically amazonaws.com in non-china AWS commercial regions while aws.amazon.com should be use instead here.

@pahud pahud added p1 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Feb 22, 2023
@MrArnoldPalmer MrArnoldPalmer added p0 and removed p1 labels Feb 22, 2023
Naumel added a commit that referenced this issue Feb 22, 2023
@mergify mergify bot closed this as completed in #24277 Feb 22, 2023
mergify bot pushed a commit that referenced this issue Feb 22, 2023
Closes #24243.

----

*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

⚠️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.

Naumel added a commit that referenced this issue Feb 22, 2023
Closes #24243.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Naumel added a commit that referenced this issue Feb 22, 2023
Closes #24243.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Naumel added a commit that referenced this issue Feb 22, 2023
Closes #24243.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Naumel added a commit that referenced this issue Feb 23, 2023
Closes #24243.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Naumel added a commit that referenced this issue Feb 24, 2023
Closes #24243.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
beck3905 pushed a commit to beck3905/aws-cdk that referenced this issue Feb 28, 2023
Closes aws#24243.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
beck3905 pushed a commit to beck3905/aws-cdk that referenced this issue Feb 28, 2023
Closes aws#24243.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
homakk pushed a commit to homakk/aws-cdk that referenced this issue Mar 13, 2023
Closes aws#24243.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
homakk pushed a commit to homakk/aws-cdk that referenced this issue Mar 28, 2023
Closes aws#24243.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
homakk pushed a commit to homakk/aws-cdk that referenced this issue Mar 28, 2023
Closes aws#24243.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment