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

aws-cognito-identitypool-alpha: Can't add rules for the default authenticated role #27411

Closed
kristianhalte opened this issue Oct 5, 2023 · 5 comments
Assignees
Labels
@aws-cdk/aws-cognito-identitypool effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 service-api This issue is due to a problem in a service API

Comments

@kristianhalte
Copy link

Describe the bug

I am trying to configure an IdentityPool with google as an authenticationProviders and add some custom rules through roleMappings.

It's neither possible directly through the initiation of the IdentityPool, nor later through the addRoleMappings() method.

Expected Behavior

To be able to add custom rules to the default roles during initiation of the IdentityPool, or add them later with the addRoleMappings() method

Current Behavior

Getting errors that the DefaultRoleAttachment failed to update, so can't create a new one

The following resource(s) failed to create: [identitypoolRoleMappingAttachmentXXXXXXXXX]. The following resource(s) failed to update: [identitypoolDefaultRoleAttachmentYYYYYYYY]. 

Reproduction Steps

Since this isn't possible

new IdentityPool(stack, 'identity-pool', {
  allowUnauthenticatedIdentities: false,
  authenticationProviders: {
    google: {
      clientId: '12345678012.apps.googleusercontent.com',
    },
  },
  roleMappings: [
    {
      providerUrl: IdentityPoolProviderUrl.GOOGLE,
      rules: [
        {
          claim: 'sub',
          claimValue: '12345678012',
          mappedRole: 'default-auth-role-reference', // 👈 it would be nice if we could reference the default roles directly during initiation of the identitypool
         },
      ],
    },
  ],
})

I instead tried this

const identityPool = new IdentityPool(stack, 'identity-pool', {
  allowUnauthenticatedIdentities: false,
  authenticationProviders: {
    google: {
      clientId: '12345678012.apps.googleusercontent.com',
    },
  },
})

identityPool.addRoleMappings({
  providerUrl: IdentityPoolProviderUrl.GOOGLE,
  rules: [
    {
      claim: 'sub',
      claimValue: '12345678012',
      mappedRole: identityPool.authenticatedRole,
    },
  ],
})

Possible Solution

I don't have a technical solution to solve this, but I managed to get it working in a not so pretty way

// 1. create a new admin role
const adminRole = new Role(stack, 'admin-role', {
  assumedBy: new WebIdentityPrincipal('cognito-identity.amazonaws.com', {
    StringEquals: {
      'cognito-identity.amazonaws.com:aud': 'something-silly', // 👈 we unfortunately don't know the poolId yet
    },
    'ForAnyValue:StringLike': {
      'cognito-identity.amazonaws.com:amr': 'authenticated',
    },
  }),
})

// 2. create the identity pool
const identityPool = new IdentityPool(stack, 'identity-pool', {
  allowUnauthenticatedIdentities: false,
  authenticationProviders: {
    google: {
      clientId: '12345678012.apps.googleusercontent.com',
    },
  },
  roleMappings: [
    {
      providerUrl: IdentityPoolProviderUrl.GOOGLE,
      rules: [
        {
          claim: 'sub',
          claimValue: '12345678012',
          mappedRole: adminRole, // 👈 it is possible to reference the adminRole during initiation of the identityPool
        },
      ],
    },
  ],
})

// 3. create a new policy statement to the admin role, to include the poolId
const adminRolePolicyStatement = new PolicyStatement({
  principals: [new WebIdentityPrincipal('cognito-identity.amazonaws.com')],
  actions: ['sts:AssumeRoleWithWebIdentity'],
  conditions: {
    StringEquals: {
      'cognito-identity.amazonaws.com:aud': identityPool.identityPoolId, // 👈 only now can we reference the poolId
    },
    'ForAnyValue:StringLike': {
      'cognito-identity.amazonaws.com:amr': 'authenticated',
    },
  },
})

// 4. attach the policy statement to the admin role
adminRole.assumeRolePolicy?.addStatements(adminRolePolicyStatement)

This approach works, but it's not very intuitive and it does have two undesirable outcomes

  1. we get an extra role (adminRole) when we could have just used the default AuthenticatedRole
  2. we get an unnecessary policy statement on the admin role
{
  "Version": "2012-10-17",
  "Statement": [
    // this statement is not needed
    {
      "Effect": "Allow",
      "Principal": {
        "Federated": "cognito-identity.amazonaws.com"
      },
      "Action": "sts:AssumeRoleWithWebIdentity",
      "Condition": {
        "StringEquals": {
          "cognito-identity.amazonaws.com:aud": "something-silly"
        },
        "ForAnyValue:StringLike": {
          "cognito-identity.amazonaws.com:amr": "authenticated"
        }
      }
    },
    {
      "Effect": "Allow",
      "Principal": {
        "Federated": "cognito-identity.amazonaws.com"
      },
      "Action": "sts:AssumeRoleWithWebIdentity",
      "Condition": {
        "StringEquals": {
          "cognito-identity.amazonaws.com:aud": "[AWS:REGION]:[POOLID]"
        },
        "ForAnyValue:StringLike": {
          "cognito-identity.amazonaws.com:amr": "authenticated"
        }
      }
    }
  ]
}

Additional Information/Context

No response

CDK CLI Version

2.99.1

Framework Version

No response

Node.js Version

v20.8.0

OS

MacOS Ventura 13.5.2

Language

Typescript

Language Version

5.2.2

Other information

No response

@kristianhalte kristianhalte added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 5, 2023
@peterwoodworth peterwoodworth added p1 feature-request A feature should be added or improved. effort/medium Medium work item – several days of effort and removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 5, 2023
@peterwoodworth
Copy link
Contributor

Thanks for the detailed description of this issue, and for posting a workaround.

I was curious to look into this further because i'm honestly not familiar with cognito personally, and I'm getting a different error when using this code snippet:

const identityPool = new IdentityPool(stack, 'identity-pool', {
  allowUnauthenticatedIdentities: false,
  authenticationProviders: {
    google: {
      clientId: '12345678012.apps.googleusercontent.com',
    },
  },
})

identityPool.addRoleMappings({
  providerUrl: IdentityPoolProviderUrl.GOOGLE,
  rules: [
    {
      claim: 'sub',
      claimValue: '12345678012',
      mappedRole: identityPool.authenticatedRole,
    },
  ],
})

us-east-1:5a7d74f5-93d8-4586-b59c-8f2a25d9f696 already exists in stack arn:aws:cloudformation:us-east-1:676158502875:stack/TagVisitStack/5e666910-63be-11ee-b122-124bf6bff145

Could you help me understand why the cloudformation template generated by this snippet is failing in the first place?

@kristianhalte
Copy link
Author

Hi Peter

Thanks for looking into this.

I get the same error message, which I should have probably posted in the first place.

In fact, if you look at your CloudFormation console, you should see two error messages.

First, the one you posted, with the Logical ID of identitypoolRoleMappingAttachmentXXXXXXXXX and Status CREATE_FAILED

us-east-1:5a7d74f5-93d8-4586-b59c-8f2a25d9f696 already exists in stack arn:aws:cloudformation:us-east-1:676158502875:stack/TagVisitStack/5e666910-63be-11ee-b122-124bf6bff145

Note

I think the reason we see the identityPoolId in this error message, is because that's the return value of a AWS::Cognito::IdentityPoolRoleAttachment. So it's not the actual AWS::Cognito::IdentityPool that already exist, but the AWS::Cognito::IdentityPoolRoleAttachment (I think)

From the AWS::Cognito::IdentityPoolRoleAttachment official docs

When you pass the logical ID of this resource to the intrinsic Ref function, Ref returns the IdentityPoolId, such as us-east-2:0d01f4d7-1305-4408-b437-12345EXAMPLE.

And the second error, which I originally posted, with the Logical ID of TagVisitStack and Status UPDATE_ROLLBACK_IN_PROGRESS

The following resource(s) failed to create: [identitypoolRoleMappingAttachmentXXXXXXXXX]. The following resource(s) failed to update: [identitypoolDefaultRoleAttachmentYYYYYYYY].

I believe the addRoleMappings() is failing, because it wants to create a new AWS::Cognito::IdentityPoolRoleAttachment (the one named identitypoolRoleMappingAttachmentXXXXXXXXX), instead of updating the existing identitypoolDefaultRoleAttachmentYYYYYYYY (which was created during the initiation new IdentityPool())

I don't know enough about AWS::Cognito::IdentityPoolRoleAttachment to say this for sure, but it seems like it's not possible to associate more than one of them to a AWS::Cognito::IdentityPool.

@Leo10Gama
Copy link
Member

I've found that the issue seems to be stemming from the way the service API is creating the ID for the IdentityPoolRoleAttachment. Currently they are investigating the issue on their end.

@Leo10Gama Leo10Gama added the service-api This issue is due to a problem in a service API label Aug 28, 2024
@Leo10Gama
Copy link
Member

Since the root cause of this issue seems to be the same as #23449, I will close this one in favour of tracking there. If this problem persists after the other issue has been resolved, please feel free to open a new issue.

Copy link

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
@aws-cdk/aws-cognito-identitypool effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 service-api This issue is due to a problem in a service API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants