Skip to content

Remove firebaseToken from persistence if tokenRefreshHandler is not s… #9298

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.

Sign up for GitHub

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
Oct 7, 2025
Merged

Remove firebaseToken from persistence if tokenRefreshHandler is not s… #9298

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
69e9995
Remove firebaseToken from persistence if tokenRefreshHandler is not s…
mansisampat Oct 7, 2025
8cba330
Update to call signOut instead
mansisampat Oct 7, 2025
bd6e331
update comment
mansisampat Oct 7, 2025
91b4d52
minnor changes
mansisampat Oct 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/auth/src/core/auth/auth_impl.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,9 @@ describe('core/auth/auth_impl', () => {
const token = await auth.getFirebaseAccessToken();
expect(token).to.be.null;
expect(exchangeTokenStub).not.to.have.been.called;
expect(persistenceStub._remove).to.have.been.calledWith(
'firebase:persistence-token:api-key:test-app'
);
});

it('should refresh the token if token is expiring in next 1 minute and a token refresh handler is set', async () => {
Expand Down Expand Up @@ -442,6 +445,9 @@ describe('core/auth/auth_impl', () => {
'Token refresh failed:',
sinon.match.instanceOf(Error)
);
expect(persistenceStub._remove).to.have.been.calledWith(
'firebase:persistence-token:api-key:test-app'
);
});

it('should return null and log an error if the refreshed token is invalid', async () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/auth/src/core/auth/auth_impl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,9 +266,9 @@ export class AuthImpl implements AuthInternal, _FirebaseService {
return firebaseAccessToken.token;
}

// Resets the Firebase Access Token to null i.e. logs out the user.
await this._updateFirebaseToken(null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we call signOut here? I see that signOut does few extra steps. Also probably this can go in the else part as if tokenRefreshHandler is present we should not be signing out.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, we can call the signOut method instead.

Also probably this can go in the else part as if tokenRefreshHandler is present we should not be signing out.

If tokenRefreshHandler is present and returns a successful response we are doing an exchangeToken that essentially logs in the user. Hence having signOut would have been a no-op.

That said I have moved the signOut towards the end of the function to avoid confusion.

if (firebaseAccessToken && this.tokenRefreshHandler) {
// Resets the Firebase Access Token to null i.e. logs out the user.
await this._updateFirebaseToken(null);
try {
// Awaits for the callback method to execute. The callback method
// is responsible for performing the exchangeToken(auth, valid3pIdpToken)
Expand Down
Loading