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-cdk-aws-cognito-identitypool-alpha: IdentityPoolProviderUrl.user_pool cant handle imported userpools #30304

Open
SpielerNogard opened this issue May 22, 2024 · 6 comments · May be fixed by #30421
Labels
@aws-cdk/aws-cognito Related to Amazon Cognito @aws-cdk/aws-cognito-identitypool bug This issue is a bug. effort/medium Medium work item – several days of effort p2

Comments

@SpielerNogard
Copy link

SpielerNogard commented May 22, 2024

Describe the bug

Our Userpool and clients are created outside our application. We now want to attach a identity pool to this userpool. Today i updated from version aws-cdk-aws-cognito-identitypool-alpha==2.96.0a0 to aws-cdk-aws-cognito-identitypool-alpha == 2.141.0a0 and changed the role_mappings accordingly. While running cdk synth i get the error: TypeError: type of argument user_pool must be aws_cdk.aws_cognito.UserPool; got jsii._reference_map.InterfaceDynamicProxy instead

Expected Behavior

Since the UserPoolAuthenticationProvider is able to handle imported userpools and clients, the IdentityPoolProviderUrl should also be

Current Behavior

Traceback (most recent call last):
  File "app.py", line 9, in <module>
    pipeline = PipelineStack(
  File "/Users/spielernogard/Documents/Repositories/bestoraged-okean-control-backend/.venv/lib/python3.8/site-packages/jsii/_runtime.py", line 118, in __call__
    inst = super(JSIIMeta, cast(JSIIMeta, cls)).__call__(*args, **kwargs)
  File "/Users/spielernogard/Documents/Repositories/bestoraged-okean-control-backend/pipeline/pipeline_stack.py", line 34, in __init__
    self._add_stages(pipeline=pipeline, id_suffix="Prod", branch=branch)
  File "/Users/spielernogard/Documents/Repositories/bestoraged-okean-control-backend/pipeline/pipeline_stack.py", line 69, in _add_stages
    services = Services(
  File "/Users/spielernogard/Documents/Repositories/bestoraged-okean-control-backend/.venv/lib/python3.8/site-packages/jsii/_runtime.py", line 118, in __call__
    inst = super(JSIIMeta, cast(JSIIMeta, cls)).__call__(*args, **kwargs)
  File "/Users/spielernogard/Documents/Repositories/bestoraged-okean-control-backend/pipeline/deployment.py", line 68, in __init__
    gui_backend = GUIBackendStack(
  File "/Users/spielernogard/Documents/Repositories/bestoraged-okean-control-backend/.venv/lib/python3.8/site-packages/jsii/_runtime.py", line 118, in __call__
    inst = super(JSIIMeta, cast(JSIIMeta, cls)).__call__(*args, **kwargs)
  File "/Users/spielernogard/Documents/Repositories/bestoraged-okean-control-backend/services/gui_backend/stack_gui_backend.py", line 18, in __init__
    ControlUserPool(scope=self, id="UserPool")
  File "/Users/spielernogard/Documents/Repositories/bestoraged-okean-control-backend/.venv/lib/python3.8/site-packages/jsii/_runtime.py", line 118, in __call__
    inst = super(JSIIMeta, cast(JSIIMeta, cls)).__call__(*args, **kwargs)
  File "/Users/spielernogard/Documents/Repositories/bestoraged-okean-control-backend/services/gui_backend/user_pool/infrastructure.py", line 106, in __init__
    provider_url=IdentityPoolProviderUrl.user_pool(
  File "/Users/spielernogard/Documents/Repositories/bestoraged-okean-control-backend/.venv/lib/python3.8/site-packages/aws_cdk/aws_cognito_identitypool_alpha/__init__.py", line 1411, in user_pool
    check_type(argname="argument user_pool", value=user_pool, expected_type=type_hints["user_pool"])
  File "/Users/spielernogard/Documents/Repositories/bestoraged-okean-control-backend/.venv/lib/python3.8/site-packages/typeguard/__init__.py", line 785, in check_type
    raise TypeError(
TypeError: type of argument user_pool must be aws_cdk.aws_cognito.UserPool; got jsii._reference_map.InterfaceDynamicProxy instead

Reproduction Steps

import os  
  
import aws_cdk as cdk  
import aws_cdk.aws_cognito as cognito  
from aws_cdk import aws_iam as iam  
from aws_cdk.aws_cognito_identitypool_alpha import (  
    IdentityPool,  
    IdentityPoolAuthenticationProviders,  
    IdentityPoolRoleMapping,  
    IdentityPoolProviderUrl,  
    UserPoolAuthenticationProvider,  
)    
from constructs import Construct  

  
USER_POOL_ARN = 'ARN_HERE'
USER_POOL_CLIENT_ID = "CLIENT_ID_HERE"
  
class ControlUserPool(Construct):  
    """Construct"""  
  
    def __init__(self, scope: Construct, id: str):  
        super().__init__(scope, id)   
        this_dir = os.path.dirname(__file__)  
  
        # import userpool
        self.user_pool = cognito.UserPool.from_user_pool_arn(  
	        scope=self, id="CognitoUserPool", user_pool_arn=USER_POOL_ARN  
        )  
        
        # import userpool client
        self.user_pool_client = cognito.UserPoolClient.from_user_pool_client_id(  
	        scope=self,  
	        id="UserPoolClientId",  
	        user_pool_client_id=USER_POOL_CLIENT_ID,  
        )  
        
        # create and attach identity pool
        self.identity_pool = IdentityPool(  
	        scope=self,  
	        id="IdentityPool",  
	        identity_pool_name=resource_name(  
		        self, name="IdentityPool"  
	        ),  
	        authentication_providers=IdentityPoolAuthenticationProviders(  
		        user_pools=[  
			        UserPoolAuthenticationProvider(  
				        user_pool=self.user_pool,  
				        user_pool_client=self.user_pool_client,  
			        )  
		        ]  
	        ),  
	        role_mappings=[  
		        IdentityPoolRoleMapping(  
			        mapping_key="cognito",  
			        provider_url=IdentityPoolProviderUrl.user_pool(  
				        user_pool=self.user_pool,  
				        user_pool_client=self.user_pool_client,  
			        ),  
			        use_token=True,  
		        )  
	        ],  
	        allow_unauthenticated_identities=False,  
        )

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.142.1 (build ed4e152)

Framework Version

No response

Node.js Version

v21.1.0

OS

Mac OS 14.5 (23F79)

Language

Python

Language Version

3.8.18 3.12.0 3.11.6

Other information

No response

@SpielerNogard SpielerNogard added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels May 22, 2024
@ashishdhingra ashishdhingra self-assigned this May 23, 2024
@ashishdhingra ashishdhingra added needs-reproduction This issue needs reproduction. and removed needs-triage This issue or PR still needs to be triaged. labels May 23, 2024
@ashishdhingra
Copy link
Contributor

ashishdhingra commented May 23, 2024

@SpielerNogard Good morning. I tried to reproduce the issue in TypeScript (even though the issue was reported in Python) to compare the behavior.

import * as cdk from 'aws-cdk-lib';
import { Construct } from 'constructs';
import * as cognito from 'aws-cdk-lib/aws-cognito';
import * as cognitoidp from '@aws-cdk/aws-cognito-identitypool-alpha';

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

    const userPool = cognito.UserPool.fromUserPoolArn(this, 'CognitoUserPool', 'arn:aws:cognito-idp:<<REGION>>:<<ACCOUNT-ID>>:userpool/<<REGION>>_<<SOME-ID>>');

    const userPoolClient = cognito.UserPoolClient.fromUserPoolClientId(this, 'UserPoolClientId', '<<APP-CLIENT-ID>>');
    
    const identityPool = new cognitoidp.IdentityPool(this, 'IdentityPool', {
      identityPoolName: 'IdentityPool',
      authenticationProviders: {
        userPools: [
          new cognitoidp.UserPoolAuthenticationProvider({
            userPool: userPool,
            userPoolClient: userPoolClient
          })
        ]
      },
      roleMappings: [
        {
          mappingKey: 'cognito', 
          providerUrl: cognitoidp.IdentityPoolProviderUrl.userPool(userPool, userPoolClient),
          useToken: true
        }
      ],
      allowUnauthenticatedIdentities: false
    });
  }
}

The Visual Studio Code IDE itself flagged the error Argument of type 'IUserPool' is not assignable to parameter of type 'UserPool'. Type 'IUserPool' is missing the following properties from type 'UserPool': userPoolProviderName, userPoolProviderUrl, triggers, addTrigger, and 17 more.ts(2345) at line providerUrl: cognitoidp.IdentityPoolProviderUrl.userPool(userPool, userPoolClient),.

Upon investigating further:

  • IdentityPoolProviderUrl.userPool() expects a strongly type class parameter UserPool.

  • However, the object returned from cognito.UserPool.fromUserPoolArn() is typed to interface IUserPool.

  • Per inheritance rules, it is possible to assign parent class object implementing an interface, to interface object type; but not vice versa, due to obvious reasons.

    Hence, the error. The CDK Python code is translated to TypeScript code via JSII, hence a different error message. But I'm assuming the reasoning is same.

I will discuss this with the team for any workarounds.

Thanks,
Ashish

@ashishdhingra ashishdhingra added p1 effort/medium Medium work item – several days of effort and removed needs-reproduction This issue needs reproduction. labels May 23, 2024
@ashishdhingra ashishdhingra removed their assignment May 23, 2024
@SpielerNogard
Copy link
Author

Thanks for the answer.

Thats bad, since we cant recreate the userpool in the stack, because we have too much users, and this error interrupts us also with the plan to go to python3.12

Hope its possible to find a workaround 🤞

@pahud
Copy link
Contributor

pahud commented May 23, 2024

That's correct!

But if we look at its implementation:

public static userPool(userPool: UserPool, userPoolClient: UserPoolClient): IdentityPoolProviderUrl {
const url = `${userPool.userPoolProviderName}:${userPoolClient.userPoolClientId}`;
return new IdentityPoolProviderUrl(IdentityPoolProviderType.USER_POOL, url);
}

we just need the userPoolProviderName attribute of the userPool, which is not available in IUserPool.

export interface IUserPool extends IResource {
/**
* The physical ID of this user pool resource
* @attribute
*/
readonly userPoolId: string;
/**
* The ARN of this user pool resource
* @attribute
*/
readonly userPoolArn: string;
/**
* Get all identity providers registered with this user pool.
*/
readonly identityProviders: IUserPoolIdentityProvider[];
/**
* Add a new app client to this user pool.
* @see https://docs.aws.amazon.com/cognito/latest/developerguide/user-pool-settings-client-apps.html
*/
addClient(id: string, options?: UserPoolClientOptions): UserPoolClient;
/**
* Associate a domain to this user pool.
* @see https://docs.aws.amazon.com/cognito/latest/developerguide/cognito-user-pools-assign-domain.html
*/
addDomain(id: string, options: UserPoolDomainOptions): UserPoolDomain;
/**
* Add a new resource server to this user pool.
* @see https://docs.aws.amazon.com/cognito/latest/developerguide/cognito-user-pools-resource-servers.html
*/
addResourceServer(id: string, options: UserPoolResourceServerOptions): UserPoolResourceServer;
/**
* Register an identity provider with this user pool.
*/
registerIdentityProvider(provider: IUserPoolIdentityProvider): void;
/**
* Adds an IAM policy statement associated with this user pool to an
* IAM principal's policy.
*/
grant(grantee: IGrantable, ...actions: string[]): Grant;
}

To fix this bug:

  1. We need to add userPoolProviderName in IUserPool
  2. We need to add a fromUserPoolAttributes() method that allows user to pass userPoolProviderName for an existing user pool.
  3. After all above, we can change userPool(userPool: UserPool to userPool(userPool: IUserPool

@sakurai-ryo
Copy link
Contributor

I'll take this.

@pahud
Copy link
Contributor

pahud commented May 29, 2024

Hi @sakurai-ryo

Please note adding new attributes to IUserPool would be a breaking change and we should avoid that as aws-cognito is a stable module. I am requesting input from our maintainers for this issue so we'll know how to address that.

@pahud pahud added the @aws-cdk/aws-cognito Related to Amazon Cognito label May 29, 2024
@TheRealAmazonKendra
Copy link
Contributor

Just one point of clarification: adding attributes is not a breaking change. The proposed change is OK on our end.

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 @aws-cdk/aws-cognito-identitypool bug This issue is a bug. effort/medium Medium work item – several days of effort p2
Projects
None yet
5 participants