-
Notifications
You must be signed in to change notification settings - Fork 929
Allow FirstPartyAuth to specify a token factory func. #4773
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
Allow FirstPartyAuth to specify a token factory func. #4773
Conversation
…used in lieu of direct GAPI.
|
} else { | ||
// Make sure this really is a Gapi client. | ||
hardAssert( | ||
!!( | ||
typeof this.gapi === 'object' && | ||
this.gapi !== null && | ||
this.gapi['auth'] && | ||
this.gapi['auth']['getAuthHeaderValueForFirstParty'] | ||
), | ||
'unexpected gapi interface' | ||
); | ||
return this.gapi['auth']['getAuthHeaderValueForFirstParty']([]); |
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.
Could we pass this block in as the default value for authTokenFactory()? We can then just invoke this.authTokenFactory for all callsites.
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.
LGTM, but one suggestion for a potential simplification
* Allow FirstPartyAuth to specify a token factory func. (#4773) * Allow firstparty credentials to specify an authToken factory that is used in lieu of direct GAPI. * Remove stray import * Add return type to private method Co-authored-by: wu-hui <[email protected]> * First Party Auth Factory * More fix * Nullable gapi * Fix Co-authored-by: Richie Foreman <[email protected]>
This adds
authTokenFactory
toFirstPartyCredentialsSettings
, which avoids a hard dependency on GAPI.JS when specified.