Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

feat: add tenantId attribute to Cognito user pool #348

Merged
merged 2 commits into from
Jun 17, 2021
Merged

feat: add tenantId attribute to Cognito user pool #348

merged 2 commits into from
Jun 17, 2021

Conversation

carvantes
Copy link
Contributor

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 the IdToken. Multitenancy requires the use of the IdToken since it is impossible to add custom claims on the AccessToken. Other than custom claims, both tokens are interchangeable in our case, since cognito:groups is present on both tokens and the Cognito user pools APIGW Authorizer accepts Id and access tokens indistinctively.

Checklist:

  • Have you successfully deployed to an AWS account with your changes?
  • [n/a ] Have you written new tests for your core changes, as applicable?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@rsmayda
Copy link
Contributor

rsmayda commented Jun 11, 2021

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."

https://auth0.com/docs/tokens

@carvantes
Copy link
Contributor Author

carvantes commented Jun 16, 2021

@rsmayda

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."

https://auth0.com/docs/tokens

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.

link - Control access to a REST API using Amazon Cognito user pools as authorizer - Amazon API Gateway

The identity token is used to authorize API calls based on identity claims of the signed-in user. The access token is used to authorize API calls based on the custom scopes of specified access-protected resources.

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.
https://docs.amplify.aws/lib/restapi/authz/q/platform/js#note-related-to-use-access-token-or-id-token

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

Have we considered getting custom attributes from Cognito via api calls?

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
Pros:

  • It is more OIDC compliant

Cons:

  • It is expensive/complex ( +1 Cognito call for ALL requests, need to build caching for performance),
  • It sort of defeats the point of using JWT
  • We are not authorizing using scopes anyways (scopes are the OIDC way of doing authZ).

Use IdToken
Pros:

  • Very simple. The built in Cognito Pools APIGW authorizer does the job.

Cons:

  • Does not strictly follow OIDC
  • It is a breaking change since we are asking customers to send IdToken in requests instead of AccessToken. Not a massive change, since any application calling FWoA would have access to both tokens(tokens have same lifecycle, refresh token works for both). They just need to switch to sending the other token as auth headers.
    Note: It is not possible to accept both IdTokens and AccessTokens on the same APIGW using the built-in Cognito Pools authorizer. To keep things simple we'd probably switch to IdTokens regardless of whether or not multi-tenancy is enabled.

I’m in favor of using IdTokens here, but we can discuss further.

Copy link
Contributor

@rsmayda rsmayda left a 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!

@carvantes carvantes merged commit 22711ae into awslabs:feat-multitenancy Jun 17, 2021
carvantes added a commit that referenced this pull request Aug 18, 2021
* 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.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants