-
Notifications
You must be signed in to change notification settings - Fork 286
Make user operations tenant-aware. #387
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
Conversation
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.
Looks mostly good. Just pointed out a few things that can be improved.
src/main/java/com/google/firebase/auth/FirebaseUserManager.java
Outdated
Show resolved
Hide resolved
src/main/java/com/google/firebase/auth/FirebaseUserManager.java
Outdated
Show resolved
Hide resolved
src/main/java/com/google/firebase/auth/AbstractFirebaseAuth.java
Outdated
Show resolved
Hide resolved
src/main/java/com/google/firebase/auth/TenantAwareFirebaseAuth.java
Outdated
Show resolved
Hide resolved
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.
Please fix the merge conflicts
I've fixed the merge conflicts. |
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.
Looks great. Just a couple of nits.
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.
Thanks. LGTM :+1
…provider config operations (#395) * Pull parts of FirebaseAuth into an abstract class. (#352) This moves parts of FirebaseAuth into an abstract class as part of adding multi-tenancy support. * Add Tenant class and its create and update request classes. (#344) This pull request adds the Tenant class (including it's create/update inner classes) as part of adding multi-tenancy support. * Add ListTenantsPage class. (#358) Add ListTenantsPage and some supporting code as part of adding multi-tenancy support. This code was very largely based off of ListUsersPage and ListUsersPageTest. * Add updateRequest method to Tenant class and add unit tests. (#361) Added some things to the Tenant class and added a few unit tests. This is part of the initiative to adding multi-tenancy support (see issue #332). * Create TenantManager class and wire through listTenants operation. (#369) Add the TenantManager class and wire through the listTenants operation. Also add unit tests to FirebaseUserManagerTest. * Add deleteTenant operation to TenantManager. (#372) This adds deleteTenant to the TenantManager class. I've added the relevant unit tests to FirebaseUserManagerTest. This is part of the initiative to adding multi-tenancy support (see issue #332). * Add getTenant operation to TenantManager. (#371) Added getTenant to the TenantManager class. Also added the relevant unit tests to FirebaseUserManagerTest. This is part of the initiative to adding multi-tenancy support (see issue #332). * Add createTenant and updateTenant operations. (#377) Added createTenant and updateTenant to the TenantManager class. Also added the relevant unit tests to FirebaseUserManagerTest. This is part of the initiative to adding multi-tenancy support (see issue #332). * Add integration tests for TenantManager operations. (#385) This adds some integration testing for all of the tenant operations in TenantManager. Several bugs were uncovered after running the tests, so these have been fixed. This is part of the initiative to adding multi-tenancy support (see issue #332). * Add firebase auth destroy check before tenant operations. (#386) This addresses some TODOs left as part of the initiative to add multi-tenancy support (see issue #332). * Make user operations tenant-aware. (#387) This makes user operations tenant-aware. I've added some integration tests to ensure that this is working correctly. This is part of the initiative to adding multi-tenancy support (see issue #332). * Remove unused AutoValue dependency. (#392) Remove unused AutoValue dependency (and remove Java 8 API dependency which was accidentally introduced). * Indicate how to get set up for the multitenancy integration tests. (#393) This documentation is based off of the instructions in https://github.com/firebase/firebase-admin-node/blob/master/CONTRIBUTING.md. * Add tenant-aware token generation and verification. (#391) This incorporates the tenant ID into the token generation and validation when using a tenant-aware client. This is part of the initiative to add multi-tenancy support (see issue #332). * Fix javadoc comment. * Trigger CI * Make several Op methods private. * Move createSessionCookie and verifySessionCookie back to FirebaseAuth. * Make verifySessionCookieOp private. * Fix a few javadoc comments. * Address Kevin's feedback. * Make TenantAwareFirebaseAuth final. * chore: Merging master into tenant-mgt (#422) * Fixed a bad merge * Add provider config management operations. (#433) Adds all of the OIDC and SAML provider config operations, related to adding multi-tenancy support. * Stop using deprecated MockHttpTransport.builder() method. * Moved tenant management code into a new package (#449) * Multi-tenancy refactor experiment * fix(auth): Completed tenant mgt refactor * Added license header to new class * Responding to code review comments: Consolidated error codes in AuthHttpClient * Improve unit test coverage of tenant/provider-related code (#453) I've improved the unit test coverage of tenant/provider-related code, and I've also removed a number of unused imports. * Fix integration tests.
This pull request make user operations tenant-aware. I've added some integration tests to ensure that this is correct correctly. This is part of the initiative to adding multi-tenancy support (see issue #332).