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

feat(auth): Multi-tenancy support for Google Cloud Identity Platform #628

Merged
merged 18 commits into from
Sep 4, 2019

Conversation

bojeil-google
Copy link
Contributor

Merge all changes to master.

RELEASE NOTE: Added multi-tenancy support to the authentication service (Google Cloud Identity Platform project required). Tenant related APIs are exposed via tenantManager() on the admin.auth interface.
RELEASE NOTE: Added tenant management APIs authForTenant(), getTenant(), listTenants(), deleteTenant(), createTenant() and updateTenant() to the newly defined TenantManager.
RELEASE NOTE: Defined TenantAwareAuth interface retrieved via TenantManager#authForTenant() for managing users, configuring SAML/OIDC providers, generating email links for password reset, email verification, etc for specific tenants.

bojeil-google and others added 15 commits May 23, 2019 15:29
* Starts defining multi-tenancy APIs. This includes:
- Defining type definitions.
- Adding tenantId to UserRecord and UserImportBuilder.
- Adding new errors associated with tenant operations.
- Defines the Tenant object.

As the changes are quite large. This will be split into multiple PRs.

* Minor fixes and tweaks.

* Addresses comments from review.

* Addresses review comments.
* Defines BaseFirebaseAuthRequestHandler class for sending Auth requests related to user management APIs and SAML/OIDC config mgmt APIs, link generation, etc.

Defines FirebaseAuthRequestHandler which extends the base class for project level only calls and which will also be extended to include tenant mgmt APIs.
Defines FirebaseTenantRequestHandler which extends the base class for tenant level only calls.

Unit tests have been modified to run tests on both subclasses.

* Addresses review comments.

* Comment clean up.

* Address more comments.
* Defines TenantAwareAuth and its user management APIs, email action link APIs, OIDC/SAML provider config mgmt APIs.

* Throws error when tenantId is provided in createUser and updateUser Auth API requests.
Adds detailed tenant mismatch error for uploadAccount on tenant Id mismatch for TenantAwareAuth.

* Addresses comments.

* Added missing mapping to client error of MISSING_DISPLAY_NAME error.
* Defines tenant management API on AuthRequestHandler.

* Adds unit test for confirming an error is thrown on createTenant request with no type.

* Address review comments.
* Adds tenant management APIs to developer facing Auth instance.
This includes getTenant, deleteTenant, listTenants, createTenant and updateTenant.
This expects TenantServerResponse to be returned for createTenant and updateTenant.

Expected results have not been confirmed. A followup PR will add integration tests for the above.

* Addresses review comments.
Adds missing backend errors.

* Addresses review comments.
* Adds integration tests for tenant management APIs.
These tests are skipped by default as multi-tenancy is a paid feature on Google Cloud Identity Platform.
To run these tests, --testMultiTenancy flag has to be added.
Adds default email provider config when backend Auth server returns undefined for emailSignInConfig.

* Updates listTenants integration test.
* Defines Auth multi-tenancy references in index.d.ts.

* Addresses review comments.
…xt: (#584)

* Adds basic integration tests for the following in multi-tenancy context:
- User management
- Custom claims
- Import users
- List users
- Token revocation
- Email link generation (skipped due to backend bug)
- OIDC/SAML management APIs.

* Removes email link sign-in generation test for now.
All tenants are now created in lightweight state by default.
* Defines the TenantManager class and its underlying methods.
Adds unit tests for this new class. Unit tests were copied from the
exising auth.spec.ts file.
)

* Removes tenant management methods and `forTenant` from Auth class.
Defines Auth#tenantManager().
Updates unit and integration tests.
Updates index.d.ts type definitions.
- Enables email action link generation tests after backend bug was fixed.
- Adds email link sign-in test case.
- Fixes issues with nested multi-tenancy tests not being skipped when they are supposed to.
const serverRequestCopy: TenantServerResponse = deepCopy(serverRequest);
it('should return the expected object representation of a tenant', () => {
expect(new Tenant(serverRequestCopy).toJSON()).to.deep.equal({
tenantId: 'TENANT_ID',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TENANT-ID

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

it('should return the expected object representation of a tenant', () => {
expect(new Tenant(serverRequestCopy).toJSON()).to.deep.equal({
tenantId: 'TENANT_ID',
displayName: 'TENANT_DISPLAY_NAME',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TENANT-DISPLAY-NAME

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@@ -66,6 +66,7 @@ describe('UserImportBuilder', () => {
},
],
customClaims: {admin: true},
tenantId: 'TENANT_ID',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TENANT-ID

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

});

it('should return expected tenantId', () => {
const resp = deepCopy(getValidUserResponse('TENANT_ID'));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TENANT-ID

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

expect(() => {
const resp = deepCopy(getValidUserResponse('TENANT_ID'));
const tenantUserRecord = new UserRecord(resp);
(tenantUserRecord as any).tenantId = 'OTHER_TENANT_ID';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OTHER-TENANT-ID

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link

@wuyanna wuyanna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM. There are a few places that are still using underscore in tenant id and tenant display name in tests, it would be best if we can change them to dash to be consistent.

@bojeil-google bojeil-google merged commit 123538d into master Sep 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants