feat(fdc): Add support for Data Connect Impersonation #2844
Conversation
jonathanedey
requested review from
lahirumaramba,
maneesht and
aashishpatil-g
| // @public | ||
| export interface ImpersonateAuthenticated { | ||
| // Warning: (ae-forgotten-export) The symbol "DecodedIdToken" needs to be exported by the entry point index.d.ts | ||
| authClaims: Partial<DecodedIdToken>; |
There was a problem hiding this comment.
Should this type be re-exported from src/data-connect/index.ts?
There was a problem hiding this comment.
Offline notes:
- Exporting
DecodedIdTokenhere would require more auth types to be exported insrc/data-connect/index.tsto satisfy api doc generation which isn't ideal. - Exporting an aliased type works around this and better documents the type in the data connect context.
Exported the aliased type AuthClaims.
| .should.eventually.be.rejected.and.has.property('code', 'data-connect/unauthenticated'); | ||
| }); | ||
|
|
||
| it('executeGraphql() successfully executes an impersonated query with non-existing authenticated claims', |
There was a problem hiding this comment.
nit: maybe reword this to: "should return an empty list for an impersonated query with non-existing authenticated claims"
…erver side SDK. (#2829) * add serialization method to server side remote config * update with latest changes * restructure as a class, add documentation * more tests for server config * run apidocs * Refine api and rerun docs * add back the newline * Apply suggestions from review * Make eTag optional in fetch response * rerun doc generator --------- Co-authored-by: Kevin Elko <kjelko@google.com>
src/data-connect/data-connect-api.ts
Outdated
| export interface ImpersonateAuthenticated { | ||
| /** | ||
| * Evaluate the auth policy with a customized JWT auth token. Should follow the Firebase Auth token format. | ||
| * https://firebase.google.com/docs/rules/rules-and-auth |
There was a problem hiding this comment.
Let's change this to https://firebase.google.com/docs/data-connect/cel-reference#auth-token-contents.
| @@ -6,6 +6,11 @@ | |||
|
|
|||
| import { Agent } from 'http'; | |||
|
|
|||
| // Warning: (ae-forgotten-export) The symbol "DecodedIdToken" needs to be exported by the entry point index.d.ts | |||
There was a problem hiding this comment.
Do we need to re-export the DecodedIdToken here?
There was a problem hiding this comment.
Offline notes:
- Exporting
DecodedIdTokenhere would require more auth types to be exported insrc/data-connect/index.tsto satisfy api doc generation which isn't ideal.- Exporting an aliased type works around this and better documents the type in the data connect context.
Exported the aliased type
AuthClaims.
From my understanding it isn't necessary and the AuthClaims export alias should provide the necessary types. @lahirumaramba Correct me if i'm mistaken here.
| expect(resp.data.users[0].name).to.be.not.undefined; | ||
| expect(resp.data.users[0].address).to.be.not.undefined; | ||
| expect(resp.data.users.length).to.be.greaterThan(1); | ||
| expect(resp.data.users[0].uid).to.not.be.undefined; |
There was a problem hiding this comment.
Should we be checking the values here? Unless we already have that in a test somewhere else
There was a problem hiding this comment.
Yeah, we should be checking the other indexes. Updated this and similar cases.
No description provided.