Defines the TenantManager class and its underlying methods.#617
Defines the TenantManager class and its underlying methods.#617bojeil-google merged 4 commits intoauth-multi-tenancyfrom
Conversation
Adds unit tests for this new class. Unit tests were copied from the exising auth.spec.ts file. All deprecated methods will be removed from the Auth class in a follow up PR.
| const tenantId = 'tenant_id'; | ||
| const serverResponse: TenantServerResponse = { | ||
| name: 'projects/project_id/tenants/tenant_id', | ||
| displayName: 'TENANT_DISPLAY_NAME', |
There was a problem hiding this comment.
Use only letters, digits and hyphens to to be consistent with requirements
…ers, digits and hyphens.
hiranya911
left a comment
hiranya911
left a comment
There was a problem hiding this comment.
LGTM with a few nits.
src/auth/tenant-manager.ts
Outdated
| return new Tenant(response); | ||
| }); | ||
| } | ||
| } No newline at end of file |
| let rejectedPromiseAccessTokenTenantManager: TenantManager; | ||
|
|
||
|
|
||
| beforeEach(() => { |
There was a problem hiding this comment.
May be before/after is enough instead of beforeEach/afterEach?
| }); | ||
|
|
||
| it('should be rejected given an invalid tenant ID', () => { | ||
| const invalidTenantId = ''; |
There was a problem hiding this comment.
Should we test for other arguments? null, int, object etc?
| passwordRequired: true, | ||
| }, | ||
| }; | ||
| const serverResponse: TenantServerResponse = { |
There was a problem hiding this comment.
This seems to be duplicated in multiple describe blocks. Perhaps move it one level up and turn into a constant like GET_TENANT_RESPONSE so all tests can use the same without duplication.
| }); | ||
|
|
||
| it('should be rejected given an invalid max result', () => { | ||
| const invalidResults = 5000; |
There was a problem hiding this comment.
I suppose 1000 is the allowed max. I'd mention that in the test description or change the assignment to something like:
const moreThanMax = 1000 + 1;
There was a problem hiding this comment.
Both suggestions accepted.
|
Thanks for the quick review! |
|
|
||
| it('should return a TenantAwareAuth with read-only tenant ID', () => { | ||
| expect(() => { | ||
| (tenantManager.authForTenant(TENANT_ID) as any).tenantId = 'OTHER_TENANT_ID'; |
There was a problem hiding this comment.
Use only letters, digits and hyphens to to be consistent with real tenant id
| }); | ||
|
|
||
| describe('updateTenant()', () => { | ||
| const tenantId = 'tenant_id'; |
| }, | ||
| }; | ||
| const serverResponse: TenantServerResponse = { | ||
| name: 'projects/project_id/tenants/tenant_id', |
There was a problem hiding this comment.
Same here for both project_id and tenant_id
| }); | ||
|
|
||
| describe('createTenant()', () => { | ||
| const tenantId = 'tenant_id'; |
| describe('createTenant()', () => { | ||
| const tenantId = 'tenant_id'; | ||
| const tenantOptions: TenantOptions = { | ||
| displayName: 'TENANT_DISPLAY_NAME', |
| }; | ||
| const serverResponse: TenantServerResponse = { | ||
| name: 'projects/project_id/tenants/tenant_id', | ||
| displayName: 'TENANT_DISPLAY_NAME', |
3 participants
Adds unit tests for this new class. Unit tests were copied from the
exising auth.spec.ts file.
All deprecated methods will be removed from the Auth class in a follow
up PR.