-
Notifications
You must be signed in to change notification settings - Fork 159
feat: add tenantId attribute to Cognito user pool #348
feat: add tenantId attribute to Cognito user pool #348
Conversation
Have we considered getting custom attributes from Cognito via api calls? I am not sure how common it is for providers to pass in Id token instead of access Auth0 is pretty opinionated about it: "Do not use ID tokens to gain access to an API." |
For APIGW + Cognito, using IdToken or AccessToken are both safe and equally valid. The AWS docs never give preference to either one, rather they mention that they work best for different use cases.
It's hard to tell how common it is to use IdTokens. As an example, AWS amplify(a popular tool for building client apps that integrate with AWS services) allows customers to use either one, but actually use idTokens in many of their examples. In general, apps/services that use custom claims favor using IdTokens. e.g. performance-dashboard-on-aws, AWS blog post about using custom claims for multi-tenancy
We could build such solution. It would be either a custom authorizer or additional code on the RBAC auth package. Use AccessToken + callback to Cognito to fetch custom claims
Cons:
Use IdToken
Cons:
I’m in favor of using IdTokens here, but we can discuss further. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Talked on a call approved as is!
* feat: add tenantId attribute to Cognito user pool (#348) * feat: remove unneeded scope checks in authorizer (#347) * feat: update lambda state machine to accommodate tenantId (#367) * feat: add "enableMultiTenancy" CFN parameter (#381) * test: add multi-tenancy integ tests (#387) * fix: remove _id, _tenantId from bulk export results (#384) * feat: Group export scripts (#389) * fix: add multi-tenant metadata route (#392) * fix: allow more concurrent export jobs for multi-tenant deployments (#397) * test: integ tests for Group export (#393) * feat: add ES hard delete config value (#398) * docs: update postman collection and docs to use Id token (#399) * docs: add multi-tenancy docs (#400) Co-authored-by: Yanyu Zheng <yz2690@columbia.edu> BREAKING CHANGE: The Cognito IdToken is now used instead of the accessToken to authorize requests.
Description of changes:
Adding tenantId as an attribute to user pool.
Also, minor updates to py scripts so that they print both the
AccessToken
and theIdToken
. Multitenancy requires the use of theIdToken
since it is impossible to add custom claims on theAccessToken
. Other than custom claims, both tokens are interchangeable in our case, sincecognito:groups
is present on both tokens and the Cognito user pools APIGW Authorizer accepts Id and access tokens indistinctively.Checklist:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.