Skip to content

Maintain persistence for firebaseToken in AuthImpl object #9119

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

Open
wants to merge 11 commits into
base: gcip-byociam-web
Choose a base branch
from
2 changes: 2 additions & 0 deletions common/api-review/auth.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ export interface Auth {
languageCode: string | null;
readonly name: string;
onAuthStateChanged(nextOrObserver: NextOrObserver<User | null>, error?: ErrorFn, completed?: CompleteFn): Unsubscribe;
// (undocumented)
onFirebaseTokenChanged(nextOrObserver: NextOrObserver<FirebaseToken | null>, error?: ErrorFn, completed?: CompleteFn): Unsubscribe;
onIdTokenChanged(nextOrObserver: NextOrObserver<User | null>, error?: ErrorFn, completed?: CompleteFn): Unsubscribe;
setPersistence(persistence: Persistence): Promise<void>;
readonly settings: AuthSettings;
Expand Down
21 changes: 21 additions & 0 deletions docs-devsite/auth.auth.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export interface Auth
| [authStateReady()](./auth.auth.md#authauthstateready) | returns a promise that resolves immediately when the initial auth state is settled. When the promise resolves, the current user might be a valid user or <code>null</code> if the user signed out. |
| [beforeAuthStateChanged(callback, onAbort)](./auth.auth.md#authbeforeauthstatechanged) | Adds a blocking callback that runs before an auth state change sets a new user. |
| [onAuthStateChanged(nextOrObserver, error, completed)](./auth.auth.md#authonauthstatechanged) | Adds an observer for changes to the user's sign-in state. |
| [onFirebaseTokenChanged(nextOrObserver, error, completed)](./auth.auth.md#authonfirebasetokenchanged) | |
| [onIdTokenChanged(nextOrObserver, error, completed)](./auth.auth.md#authonidtokenchanged) | Adds an observer for changes to the signed-in user's ID token. |
| [setPersistence(persistence)](./auth.auth.md#authsetpersistence) | Changes the type of persistence on the <code>Auth</code> instance. |
| [signOut()](./auth.auth.md#authsignout) | Signs out the current user. This does not automatically revoke the user's ID token. |
Expand Down Expand Up @@ -227,6 +228,26 @@ onAuthStateChanged(nextOrObserver: NextOrObserver<User | null>, error?: ErrorFn,

[Unsubscribe](./util.md#unsubscribe)

## Auth.onFirebaseTokenChanged()

<b>Signature:</b>

```typescript
onFirebaseTokenChanged(nextOrObserver: NextOrObserver<FirebaseToken | null>, error?: ErrorFn, completed?: CompleteFn): Unsubscribe;
```

#### Parameters

| Parameter | Type | Description |
| --- | --- | --- |
| nextOrObserver | [NextOrObserver](./auth.md#nextorobserver)<!-- -->&lt;[FirebaseToken](./auth.firebasetoken.md#firebasetoken_interface) \| null&gt; | |
| error | [ErrorFn](./util.md#errorfn) | |
| completed | [CompleteFn](./util.md#completefn) | |

<b>Returns:</b>

[Unsubscribe](./util.md#unsubscribe)

## Auth.onIdTokenChanged()

Adds an observer for changes to the signed-in user's ID token.
Expand Down
37 changes: 37 additions & 0 deletions packages/auth/src/core/auth/auth_impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ export class AuthImpl implements AuthInternal, _FirebaseService {
private redirectPersistenceManager?: PersistenceUserManager;
private authStateSubscription = new Subscription<User>(this);
private idTokenSubscription = new Subscription<User>(this);
private firebaseTokenSubscription = new Subscription<FirebaseToken>(this);
private readonly beforeStateQueue = new AuthMiddlewareQueue(this);
private redirectUser: UserInternal | null = null;
private isProactiveRefreshEnabled = false;
Expand Down Expand Up @@ -195,6 +196,7 @@ export class AuthImpl implements AuthInternal, _FirebaseService {
}

await this.initializeCurrentUser(popupRedirectResolver);
await this.initializeFirebaseToken();

this.lastNotifiedUid = this.currentUser?.uid || null;

Expand Down Expand Up @@ -403,6 +405,12 @@ export class AuthImpl implements AuthInternal, _FirebaseService {
return this.directlySetCurrentUser(user);
}

private async initializeFirebaseToken(): Promise<void> {
this.firebaseToken =
(await this.persistenceManager?.getFirebaseToken()) ?? null;
this.firebaseTokenSubscription.next(this.firebaseToken);
}

useDeviceLanguage(): void {
this.languageCode = _getUserLanguage();
}
Expand Down Expand Up @@ -461,6 +469,12 @@ export class AuthImpl implements AuthInternal, _FirebaseService {
firebaseToken: FirebaseToken | null
): Promise<void> {
this.firebaseToken = firebaseToken;
this.firebaseTokenSubscription.next(firebaseToken);
if (firebaseToken) {
await this.assertedPersistence.setFirebaseToken(firebaseToken);
} else {
await this.assertedPersistence.removeFirebaseToken();
Copy link
Contributor

Choose a reason for hiding this comment

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

You will have to do same for signOut as well right?

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. I plan to create a separate CL for signOut logic. Please let me know if you see any concerns.

}
}

async signOut(): Promise<void> {
Expand Down Expand Up @@ -577,6 +591,29 @@ export class AuthImpl implements AuthInternal, _FirebaseService {
);
}

onFirebaseTokenChanged(
nextOrObserver: NextOrObserver<FirebaseToken | null>,
error?: ErrorFn,
completed?: CompleteFn
): Unsubscribe {
if (typeof nextOrObserver === 'function') {
const unsubscribe = this.firebaseTokenSubscription.addObserver(
nextOrObserver,
error,
completed
);
return () => {
unsubscribe();
};
} else {
const unsubscribe =
this.firebaseTokenSubscription.addObserver(nextOrObserver);
return () => {
unsubscribe();
};
}
}

beforeAuthStateChanged(
callback: (user: User | null) => void | Promise<void>,
onAbort?: () => void
Expand Down
36 changes: 35 additions & 1 deletion packages/auth/src/core/persistence/persistence_user_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import { getAccountInfo } from '../../api/account_management/account';
import { ApiKey, AppName, AuthInternal } from '../../model/auth';
import { FirebaseToken } from '../../model/public_types';
import { UserInternal } from '../../model/user';
import { PersistedBlob, PersistenceInternal } from '../persistence';
import { UserImpl } from '../user/user_impl';
Expand All @@ -27,7 +28,8 @@ export const enum KeyName {
AUTH_USER = 'authUser',
AUTH_EVENT = 'authEvent',
REDIRECT_USER = 'redirectUser',
PERSISTENCE_USER = 'persistence'
PERSISTENCE_USER = 'persistence',
PERSISTENCE_TOKEN = 'persistence-token'
}
export const enum Namespace {
PERSISTENCE = 'firebase'
Expand All @@ -44,6 +46,7 @@ export function _persistenceKeyName(
export class PersistenceUserManager {
private readonly fullUserKey: string;
private readonly fullPersistenceKey: string;
private readonly firebaseTokenPersistenceKey: string;
private readonly boundEventHandler: () => void;

private constructor(
Expand All @@ -58,6 +61,11 @@ export class PersistenceUserManager {
config.apiKey,
name
);
this.firebaseTokenPersistenceKey = _persistenceKeyName(
KeyName.PERSISTENCE_TOKEN,
config.apiKey,
name
);
this.boundEventHandler = auth._onStorageEvent.bind(auth);
this.persistence._addListener(this.fullUserKey, this.boundEventHandler);
}
Expand All @@ -66,6 +74,32 @@ export class PersistenceUserManager {
return this.persistence._set(this.fullUserKey, user.toJSON());
}

setFirebaseToken(firebaseToken: FirebaseToken): Promise<void> {
return this.persistence._set(this.firebaseTokenPersistenceKey, {
token: firebaseToken.token,
expirationTime: firebaseToken.expirationTime
});
}

async getFirebaseToken(): Promise<FirebaseToken | null> {
const blob = await this.persistence._get<PersistedBlob>(
this.firebaseTokenPersistenceKey
);
if (!blob) {
return null;
}
const token = blob.token as string;
const expirationTime = blob.expirationTime as number;
return {
token,
expirationTime
};
}

removeFirebaseToken(): Promise<void> {
return this.persistence._remove(this.firebaseTokenPersistenceKey);
}

async getCurrentUser(): Promise<UserInternal | null> {
const blob = await this.persistence._get<PersistedBlob | string>(
this.fullUserKey
Expand Down
6 changes: 6 additions & 0 deletions packages/auth/src/model/public_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,12 @@ export interface Auth {
callback: (user: User | null) => void | Promise<void>,
onAbort?: () => void
): Unsubscribe;

onFirebaseTokenChanged(
Copy link
Contributor

Choose a reason for hiding this comment

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

q: When do we call onFirebaseTokenChanged?

  • Please add jsdoc
  • Do we need to implement a onFirebaseTokenChanged function in packages/auth/src/core/index.ts ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey, I onFirebaseTokenChanged is a callback method that is triggered when firebase token is updated i.e. during the following scenarios -

  • On App initialization when FirebaseToken is fetched from the persistence.
  • When a getToken method is triggered and the token is expired. In this case the firebaseToken is updated to null.

The callback method was initially not planned. I had a discussion with pavanshankar@ and we decided on decoupling the persistence logic with the callback method.

Created a PR - #9138 for persisting the firebaseToken for BYO-CIAM.

Copy link
Contributor

Choose a reason for hiding this comment

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

Related: this method was not part of the API review. I'm concerned that onFirebaseTokenChanged (and the concept of a FirebaseToken) is very generic sounding. I'm worried it'll confuse users.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed.
The onFirebaseTokenChanged callback method was initially not planned during the API Review. Had a discussion with pavanshankar@ offline. Currently decoupling the callback from the persistence changes.

Created a PR - #9138 for persisting the firebaseToken for BYO-CIAM.

Copy link
Contributor

@sam-gc sam-gc Jul 1, 2025

Choose a reason for hiding this comment

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

We should go through another API review process surfacing this. "firebaseToken" is a really generic name -- "Firebase" is a huge suite of products that has many different types of tokens. Any and every change to the public API surface needs to go through review

nextOrObserver: NextOrObserver<FirebaseToken | null>,
error?: ErrorFn,
completed?: CompleteFn
): Unsubscribe;
/**
* Adds an observer for changes to the signed-in user's ID token.
*
Expand Down
Loading