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

Cognito circular reference when setting lambda trigger permissions #7016

Open
markcarroll opened this issue Mar 25, 2020 · 33 comments
Open

Cognito circular reference when setting lambda trigger permissions #7016

markcarroll opened this issue Mar 25, 2020 · 33 comments
Labels
@aws-cdk/aws-lambda Related to AWS Lambda bug This issue is a bug. effort/large Large work item – several weeks of effort p1

Comments

@markcarroll
Copy link

markcarroll commented Mar 25, 2020

Create a lambda
Create a user pool
Assign the lambda to one of the user pool triggers
Set the permissions on the lambda to call Cognito APIs against the user pool
Get circular reference error in cdk deploy

Reproduction Steps

    const postAuthentication = new lambda.Function(this, "postAuthentication", {
      description: "Cognito Post Authentication Function",
      runtime: lambda.Runtime.NODEJS_12_X,
      handler: "postAuthentication.handler",
      code: lambda.Code.asset("dist/postAuthentication"),
      timeout: cdk.Duration.seconds(30),
      memorySize: 256,
      environment: {},
    });

    const userPool = new cognito.UserPool(this, userPoolName, {
     ....
      lambdaTriggers: {
        postAuthentication,
      },
    });

    const postAuthPermissionPolicy = new iam.PolicyStatement({
      actions: ["cognito-idp:AdminDeleteUserAttributes", "cognito-idp:AdminAddUserToGroup"],
      resources: [userPool.userPoolArn],
    });
   // now give the postAuthentication lambda permission to change things
    postAuthentication.addToRolePolicy(postAuthPermissionPolicy);

Error Log

Cognito failed: Error [ValidationError]: Circular dependency between resources

Environment

  • CLI Version : 1.31.0
  • Framework Version:
  • OS :
  • Language : Typescript

Other


This is 🐛 Bug Report

@markcarroll markcarroll added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 25, 2020
@mshober
Copy link
Contributor

mshober commented Mar 27, 2020

Your user pool requires your Lambda function to be created first, but your function's policy depends on on your user pool's ARN, causing the circular dependency. Removing resources: [userPool.userPoolArn] should fix this, although you would need to make sure that doesn't pose a security risk.

@SomayaB SomayaB added the @aws-cdk/aws-cognito Related to Amazon Cognito label Mar 30, 2020
@nija-at
Copy link
Contributor

nija-at commented Apr 1, 2020

Providing the functionName property to the lambda.Function should resolve this issue. Can you give that a shot?

@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 Apr 1, 2020
@markcarroll
Copy link
Author

Sorry, that didn't work. I added functionName to the lambda:

    const postAuthentication = new lambda.Function(this, "postAuthentication", {
      description: "Cognito Post Authentication Function",
      functionName: stackName + "-postAuthentication",
      runtime: lambda.Runtime.NODEJS_12_X,
      handler: "postAuthentication.handler",
      code: lambda.Code.asset("dist/postAuthentication"),
      timeout: cdk.Duration.seconds(30),
      memorySize: 256,
      environment: {},
    });

Then after the user pool is defined I added:

    const postAuthPermissionPolicy = new iam.PolicyStatement({
      actions: ["cognito-idp:AdminDeleteUserAttributes", "cognito-idp:AdminAddUserToGroup"],
    });
    postAuthPermissionPolicy.addResources(userPool.userPoolArn);
    postAuthenticationLambda.addToRolePolicy(postAuthPermissionPolicy);

As soon as I switched out postAuthPermissionPolicy.addAllResources to postAuthPermissionPolicy.addResources(userPool.userPoolArn) the circular reference error came back.

In other CDK areas there are grant* functions that seem to do this magic. Any other ideas of a work around until we get #7112 ?

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

nija-at commented Apr 8, 2020

The grant* functions do not solve this problem. However, I've classified this as a bug.

nija-at pushed a commit that referenced this issue Apr 8, 2020
… function role

Lambda functions can be used as [triggers] on Cognito user pools. It is
common for these triggers to query (callback) the user pool for more
information.

The function's IAM policy needs to updated with permissions to call the
Cognito IDP service with the resource restricted to the specific user
pool. Adding such an IAM policy creates a circular reference between the
user pool, which refers to the lambda function, and the function policy,
which refers to the user pool.

This fixes the problem by using the `functionName` initialization
property, when specified, to determine the `functionArn`, thereby,
removing its dependency on the function being created first.

fixes #7016

[triggers]: https://docs.aws.amazon.com/cognito/latest/developerguide/cognito-user-identity-pools-working-with-aws-lambda-triggers.html
@nija-at
Copy link
Contributor

nija-at commented Apr 23, 2020

@markcarroll -

The workaround for this issue is to not use the addToRolePolicy() but instead to attachInlinePolicy(). See code snippet below -

import { UserPool } from '@aws-cdk/aws-cognito';
import { Function, Code, Runtime } from '@aws-cdk/aws-lambda';
import { Policy, PolicyStatement } from '@aws-cdk/aws-iam';
import { App, Stack } from '@aws-cdk/core';

const app = new App();
const stack = new Stack(app, 'mystack');

const fn = new Function(stack, 'fn', {
  code: Code.fromInline('foo'),
  runtime: Runtime.NODEJS_12_X,
  handler: 'index.handler',
});

const userpool = new UserPool(stack, 'pool', {
  lambdaTriggers: {
    userMigration: fn
  }
});

fn.role!.attachInlinePolicy(new Policy(stack, 'userpool-policy', {
  statements: [ new PolicyStatement({
    actions: ['cognito-idp:DescribeUserPool'],
    resources: [userpool.userPoolArn],
  }) ]
}));

Can you check if this fixes this issue for you?

@nija-at nija-at added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. p2 and removed p1 labels Apr 23, 2020
@github-actions
Copy link

github-actions bot commented May 6, 2020

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label May 6, 2020
@nija-at nija-at removed closing-soon This issue will automatically close in 4 days unless further comments are made. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels May 7, 2020
@nija-at
Copy link
Contributor

nija-at commented May 7, 2020

Please ignore the message from the bot. I've removed the labels that triggered this.

@markcarroll
Copy link
Author

The attachInlinePolicy workaround seems to do the trick. Do you want to close this or keep it open?

@nija-at
Copy link
Contributor

nija-at commented May 11, 2020

Leave this open. I'd like to get a better solution for this in place. It's more complicated than I originally thought it would be.

@nija-at nija-at added effort/medium Medium work item – several days of effort effort/large Large work item – several weeks of effort labels Aug 6, 2020
@nija-at nija-at removed effort/medium Medium work item – several days of effort @aws-cdk/aws-cognito Related to Amazon Cognito labels Oct 16, 2020
@nija-at nija-at added the @aws-cdk/aws-lambda Related to AWS Lambda label Oct 16, 2020
@shivlaks
Copy link
Contributor

discussed this issue with @nija-at and @rix0rrr

deployed this sample application inspired by the OP

import { PolicyStatement } from '@aws-cdk/aws-iam';
import * as lambda from '@aws-cdk/aws-lambda';
import { App, Stack } from '@aws-cdk/core';
import { UserPool } from '../lib';

const app = new App();
const stack = new Stack(app, 'integ-user-pool-client-explicit-props');

const fn = new lambda.Function(stack, 'fn', {
  code: lambda.Code.fromInline('foo'),
  runtime: lambda.Runtime.NODEJS_12_X,
  handler: 'index.handler',
});

const userpool = new UserPool(stack, 'pool', {
  lambdaTriggers: {
    postAuthentication: fn,
  },
});

const postAuthPermissionPolicy = new PolicyStatement({
  actions: ['cognito:DescribeUserPool'],
  resources: [userpool.userPoolArn],
});
// now give the postAuthentication lambda permission to change things
fn.addToRolePolicy(postAuthPermissionPolicy);

Screen Shot 2020-10-19 at 11 38 33 PM

this produced a stack with a circular dependency -> the trigger Lambda function has a dependency on the role policy, which has a dependency on the user pool, which has a dependency on the lambda function.

The problematic edge is the one between the function and the role policy - this should not be necessary as the role policy is already depending on the Lambda execution role (which the Lambda function also depends on). This problem will appear in any resource that has a Lambda function and a subsequent call to addToRolePolicy. It may not be possible to reliably remove that depends on with the current implementation in the lambda module.

As mentioned earlier by @nija-at, the workaround is to use attachInlinePolicy which does not create a dependency between the Lambda function and the inline policy.

import { Policy, PolicyStatement } from '@aws-cdk/aws-iam';
import * as lambda from '@aws-cdk/aws-lambda';
import { App, Stack } from '@aws-cdk/core';
import { UserPool } from '../lib';

const app = new App();
const stack = new Stack(app, 'integ-user-pool-client-explicit-props');

const fn = new lambda.Function(stack, 'fn', {
  code: lambda.Code.fromInline('foo'),
  runtime: lambda.Runtime.NODEJS_12_X,
  handler: 'index.handler',
});

const userpool = new UserPool(stack, 'pool', {
  lambdaTriggers: {
    postAuthentication: fn,
  },
});

fn.role?.attachInlinePolicy(new Policy(stack, 'userpool-policy', {
  statements: [new PolicyStatement({
    actions: ['cognito-idp:DescribeUserPool'],
    resources: [userpool.userPoolArn],
  })],
}));

Screen Shot 2020-10-19 at 11 42 11 PM

This issue is now being tracked under the lambda module

@arkyofficial
Copy link

arkyofficial commented Mar 14, 2021

For me even attachInlinePolicy creates an error
Do we know when the bug will be fixed?

I think trigger needs to be a new element like: new CognitoTrigger so the dependson can be added

@Townsheriff
Copy link

I want to mention that, I'm passing user pool id as the env variable and it has the same error effect. This just makes things weird, that I need to pass userPoolId in the parameter store to utilise it in the lambda.

@Townsheriff
Copy link

Today I noticed that userPoolId is inside event object for lambda.

@KennethVrb
Copy link

any further news on this? i'm havinga similar issue where a userpool depends on a lambda as it trigger for CustomMessage but the lambda needs the userpoolid as an environment variable

@Townsheriff
Copy link

any further news on this? i'm havinga similar issue where a userpool depends on a lambda as it trigger for CustomMessage but the lambda needs the userpoolid as an environment variable

Lambda receives userPoolId in the event argument.

@Selimmo
Copy link

Selimmo commented Jul 23, 2021

I had the same problem and i ended up doing something like this

const fn = new Function(stack, 'fn', {
  code: Code.fromInline('foo'),
  runtime: Runtime.NODEJS_12_X,
  handler: 'index.handler',
});

const userpool = new UserPool(stack, 'pool', {
  lambdaTriggers: {
    userMigration: fn
  }
}); 

const cognitoAdminPolicyStatement  = new PolicyStatement({
   effect: Effect.ALLOW,
   actions: 'cognito-idp:*',
   resources: [
       `arn:aws:cognito-idp:${region}:${account}:userpool/${userPoolId}`
   ]
 });

const lambdaRole = Role.fromRoleArn(this, 'lambdaRole', fn.role!.roleArn);

lambdaRole.addToPrincipalPolicy(cognitoAdminPolicyStatement);

Basically the trick is retrieve the role as if it was created in another stack. (i know and i don't like it as well) but it seems to do the trick for now.

@MiguelNiblock
Copy link

Today I noticed that userPoolId is inside event object for lambda.

This saved me. I also got this error when trying to pass the pool.userPoolId to the lambda environment. But now I don't need to do that because it is included in the event, because it's a cognito post confirmation trigger. Thanks!

@shawnmclean
Copy link

I'm running into this exact circular dependency but from CloudFormation. How can I implement this delayed adding of the policy to the lambda role?

@Torsitano
Copy link

@shawnmclean Not sure if you're still running into that issue, but normally with regular CloudFormation you'd use the AWS::IAM::ManagedPolicy or AWS::IAM::Policy resources to get around a circular dependency between a resource and an IAM Role.

If you have a resource that needs a policy based on the Role, but the Role also needs a policy specifying the resource, the chain will go:
AWS::IAM::Role
AWS::Other::Resource
AWS::IAM::ManagedPolicy

Each depends on the one prior, but since the Managed Policy is created at a later point and attached to the IAM Role, it is not circular.

@teuteuguy
Copy link

I have the same issue with providing the lambda function with the list of app clientIds as environment variables upon creation.

Has this been looked into ?

@BartekCK
Copy link

BartekCK commented Mar 7, 2023

Problem still exist, but workaround with attachInlinePolicy helps in some cases.

@github-actions github-actions bot added p1 and removed p2 labels Apr 9, 2023
@github-actions
Copy link

github-actions bot commented Apr 9, 2023

This issue has received a significant amount of attention so we are automatically upgrading its priority. A member of the community will see the re-prioritization and provide an update on the issue.

@jamesleech
Copy link

I have the same issue with providing the lambda function with the list of app clientIds as environment variables upon creation.

Has this been looked into ?

I have two app clients, and want to do different things in cognito triggers based on the app client the user is coming in via... think mobile app client vs web.

if event.callerContext.callerContext.cliendId = appClientId { do stuff } else if event.callerContext.callerContext.cliendId = webClientId { do different stuff. }

I'm getting circular reference when creating cognito, added a second client, and then trying to bind the Ids of the clients into params and/or env vars.

Is there a workaround for this?

cognito depends on the trigger lambdas
trigger lambdas depend on cognito to be created to give me ids

I really don't want to use ssm at trigger runtime to get the ids.

@KomaGR
Copy link

KomaGR commented Jul 5, 2023

I am facing a similar problem because I'm trying to pass the userPoolId in the Lambda trigger. Is there a way to know the userPoolId without passing it as an env var?

        const postConfirmationLambda = new lambdaNodejs.NodejsFunction(this, 'postConfirmationLambda', {
            entry: 'lambda-functions/user-pool-trigger/post-confirmation.ts',
            environment: {
                "USER_POOL": cognitoUserPool.userPoolId, /* This causes a circular dependency while deploying */
            }
        });

@ignaciolarranaga
Copy link

Just to add to the discussion I resolved it using cloud trail events:

export function buildTestUsersHandlerFunction(
  scope: Construct,
  env: Environment,
  userPool: IUserPool
) {
  const lambda = new Function(scope, 'TestUsersHandler', {
    ...
  });

  userPool.grant(lambda, 'cognito-idp:AdminGetUser');
  userPool.grant(lambda, 'cognito-idp:AdminAddUserToGroup');

  addCognitoEventTarget(scope, env, lambda);
}

function addCognitoEventTarget(
  scope: Construct,
  env: Environment,
  lambda: IFunction
) {
  // WARNING: This function needs the Trail created by buildCloudTrailTrail to work
  const eventRule = Trail.onEvent(scope, 'CognitoSignUpUserEventRule', {
    target: new LambdaFunction(lambda),
  });

  eventRule.addEventPattern({
    source: ['aws.cognito-idp'],
    detailType: ['AWS API Call via CloudTrail'],
    detail: {
      eventSource: ['cognito-idp.amazonaws.com'],
      eventName: ['SignUp'],
    },
  });
}

This way each time an operation is made through the Cognito API I route an event to this lambda.
Tried the other suggested approaches but couldn't get rid of the circular dependency.

@ALFmachine
Copy link

for me everything works smashingly until cognitoPool.addTrigger(trigger, this.handler) which causes the circular dep trail...

@d-weg
Copy link

d-weg commented Oct 30, 2023

I'm facing a similar issue, I'm creating the lambdas after the UserPool and AppClients are created, I then use the app clients to populate a few env variables for the lambdas when creating them (I need a mapping so the event client id is not useful here), and then I just add them as triggers, so the issue is around the env variables, if I dont use the clients to populate it it works, otherwise just throws a circular reference

@flaxpanda
Copy link

this approach is the only way we have been able to solve for this currently

@viniciustrindade
Copy link

I am facing a similar problem because I'm trying to pass the userPoolId in the Lambda trigger. Is there a way to know the userPoolId without passing it as an env var?

        const postConfirmationLambda = new lambdaNodejs.NodejsFunction(this, 'postConfirmationLambda', {
            entry: 'lambda-functions/user-pool-trigger/post-confirmation.ts',
            environment: {
                "USER_POOL": cognitoUserPool.userPoolId, /* This causes a circular dependency while deploying */
            }
        });

You can use event.userPoolId inside of the lamba

@b-tin
Copy link

b-tin commented Feb 23, 2024

event.userPoolId

this save me to avoid the error when passing variable environment to lambda

@toddarmstrong
Copy link

I ran into this issue but regarding using a User Pool Id or User Pool Client Id as an environment variable for a Lambda function I was assigning as a trigger to the User Pool.
My fix for this was to use addTrigger() on the User Pool after setting everything up and using lambda.Function.fromFunctionArn() to reference the function.

const userPool = new cognito.UserPool(this, "UserPool", {
  ...
});

const client = new cognito.UserPoolClient(this, "UserPoolClient", {
  userPool,
  ...
});

const lambdaFunctionName = "function-name";
const lambdaFunction = new lambda.Function(this, "LambdaFunction", {
  functionName: lambdaFunctionName,
  environment: {
    USER_POOL_CLIENT_ID: client.userPoolClientId,
  },
  ...
});

userPool.addTrigger(
  cognito.UserPoolOperation.PRE_SIGN_UP,
  lambda.Function.fromFunctionArn(
    this,
    `PreSignUpLambdaFunction`,
    `arn:aws:lambda:${props.env.region}:${props.env.account}:function:${lambdaFunctionName}`
  )
);

@seanimamo
Copy link

This is still an issue in 2024. The attachInlinePolicy method works but are you guys planning to fix this or put it on the back burner for another 4 years? Or is this working as intended?

@ScArLeXiA
Copy link

I ran into this issue but regarding using a User Pool Id or User Pool Client Id as an environment variable for a Lambda function I was assigning as a trigger to the User Pool. My fix for this was to use addTrigger() on the User Pool after setting everything up and using lambda.Function.fromFunctionArn() to reference the function.

const userPool = new cognito.UserPool(this, "UserPool", {
  ...
});

const client = new cognito.UserPoolClient(this, "UserPoolClient", {
  userPool,
  ...
});

const lambdaFunctionName = "function-name";
const lambdaFunction = new lambda.Function(this, "LambdaFunction", {
  functionName: lambdaFunctionName,
  environment: {
    USER_POOL_CLIENT_ID: client.userPoolClientId,
  },
  ...
});

userPool.addTrigger(
  cognito.UserPoolOperation.PRE_SIGN_UP,
  lambda.Function.fromFunctionArn(
    this,
    `PreSignUpLambdaFunction`,
    `arn:aws:lambda:${props.env.region}:${props.env.account}:function:${lambdaFunctionName}`
  )
);

I encountered a similar situation, and this approach was very helpful. In my case, when I used userPool.addTrigger() within the same stack, I encountered a "Function not found" error. To resolve this, I initially commented out the userPool.addTrigger() line, deployed all dependencies, and then uncommented the line and deployed again. This worked for me.

For reference, here are the errors I encountered:

CREATE_FAILED        | AWS::Lambda::Permission      | userPoolPreSignUpCognitoXXXXXXXX
Resource handler returned message: "Function not found: arn:aws:lambda:{REGION}:{ACCOUNT_ID}:function:{FUNCTION_NAME} (Service: Lambda, Status Code: 404, Request ID: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX)" (Re
questToken: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX, HandlerErrorCode: NotFound)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-lambda Related to AWS Lambda bug This issue is a bug. effort/large Large work item – several weeks of effort p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.