aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.36k stars 3.77k forks source link

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

Open kristianhalte opened 9 months ago

kristianhalte commented 9 months ago

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_
peterwoodworth commented 9 months ago

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 commented 9 months ago

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.