Skip to content

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

Merged
merged 4 commits into from
Aug 2, 2022

Conversation

richieforeman
Copy link
Contributor

This adds authTokenFactory to FirstPartyCredentialsSettings, which avoids a hard dependency on GAPI.JS when specified.

@changeset-bot
Copy link

changeset-bot bot commented Apr 12, 2021

⚠️ No Changeset found

Latest commit: 948e418

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Comment on lines +310 to +321
} 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']([]);
Copy link
Contributor

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.

Copy link
Contributor

@schmidt-sebastian schmidt-sebastian left a 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

@wu-hui wu-hui changed the base branch from master to wuandy/FirstPartyAuth August 2, 2022 14:27
@wu-hui wu-hui merged commit 54ce678 into firebase:wuandy/FirstPartyAuth Aug 2, 2022
wu-hui added a commit that referenced this pull request Aug 5, 2022
* 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]>
@firebase firebase locked and limited conversation to collaborators Sep 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants