Add provider config management operations. #433
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@kevinthecheung can you please review the API docs in following classes:
|
Adds OIDC provider config class and base class. This is part of adding multi-tenancy support (see issue #332).
This makes the base `CreateRequest` setters return the proper instance type, so that methods can be chained. This also makes it so that the provider ID can be parsed from the resource name. A package private `getProviderId()` method was also added, which will be needed by the `FirebaseUserManager` class when the provider config operations are added there. Also renamed `AuthProviderConfig` to `ProviderConfig` since "Auth" is redundant with the package name. This work is part of adding multi-tenancy support (see issue #332).
This adds an operation to create OIDC provider configs, as well as an operation to delete provider configs. These operations can be performed using either the tenant-aware or standard Firebase client. This work is part of adding multi-tenancy support (see issue #332).
This adds an operation to get OIDC provider configs (can be done using either the tenant-aware or standard Firebase client). This work is part of adding multi-tenancy support (see issue #332).
This adds an operation to update OIDC provider configs (can be done using either the tenant-aware or standard Firebase client). This work is part of adding multi-tenancy support (see issue #332).
This adds an operation to list OIDC provider configs (can be done using either the tenant-aware or standard Firebase client). This work is part of adding multi-tenancy support (see issue #332).
Move tenant-aware integration tests to separate class. This simplifies the setup and teardown required for these tests.
Each asynchronous method is supposed to have a synchronous counterpart. This was supposed to be included when I initially added the operation to list OIDC provider configs.
The OIDC Auth provider config ID must begin with "oidc.". This validation is being done in other APIs (e.g. Go), so we should do it here as well. I've moved assertValidUrl to the base class so that it can be reused for SamlProviderConfig, once that class is added.
* fix(auth): Ensuring test user account cleanup with a Rule * Updated copyright holder
I've renamed the delete operation since we need separate methods for deleting OIDC and SAML provider configs. I've also created a ProviderConfigTestUtils class to house shared logic between TenantAwareFirebaseAuthIT and FirebaseAuthIT, and I've added a TemporaryProviderConfig class to make it easier for provider configs to be cleaned up automatically. I've structured this class in such a way that we can easily tack on logic to cleanup SAML provider configs as well. I've also decided to remove testGetUserWithMultipleTenantIds. I initially wrote this test to understand how tenant-aware auths and tenant-agnostic auths interacted with one another. In its existing state, it appears to be leaking created users, and in order to resolve this with TemporaryUser, we would need to declare multiple TemporaryUser objects as rules. I think it's better to avoid this complexity and remove this test. After all, this integration test is mainly making assertions about the server's behavior and not our behavior.
…quest. (#418) This makes the formatting of the Javadoc comments more consistent, and it's now more clear when/why IllegalArgumentExceptions get thrown. This also adds validation to the provider ID in the CreateRequest to ensure that it has the correct prefix.' Improvements were also made to the tests. The JSON string is now more readable, the parameters are now in the correct order for assertEquals calls, and the IllegalArgumentException tests have been properly fleshed out.
Adds a class for the SAML provider config.
This adds operations to create and delete SAML provider configs.
Adds a list operation for SAML provider configs. I also set 'suppressLoadErrors' to true to avoid class information error. See https://stackoverflow.com/questions/27938039/unable-to-get-class-information-for-checkstyle for more context.
I've implemented addAllX509Certificates and removed the TODOs for the request signing, since we are not ready to expose that yet. I've also added unit tests for SamlProviderConfig.UpdateRequest, because those were missed in a previous PR. On an unrelated note, I've also added a comment describing why 'suppressLoadErrors' needs to be set.
kevinthecheung
requested changes
Jun 11, 2020
src/main/java/com/google/firebase/auth/AbstractFirebaseAuth.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/AbstractFirebaseAuth.java
Outdated
Show resolved
Hide resolved
src/main/java/com/google/firebase/auth/AbstractFirebaseAuth.java
Outdated
Show resolved
Hide resolved
|
Alright, thanks for the review! I've addressed your feedback, and I've also rebased this branch so that we don't have any more merge conflicts with |
kevinthecheung
approved these changes
Jun 17, 2020
hiranya911
pushed a commit
that referenced
this pull request
Jul 16, 2020
…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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds all of the OIDC and SAML provider config operations, as part of adding multi-tenancy support.