Skip to content

Commit 609739c

Browse files
fix(passport,wallet): prevent login popup on page load and fix wallet address display after OAuth redirect (#2801)
1 parent ac2d8bd commit 609739c

File tree

6 files changed

+156
-16
lines changed

6 files changed

+156
-16
lines changed

packages/passport/sdk/src/Passport.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -257,9 +257,11 @@ export class Passport {
257257

258258
// Use connectWallet to create the provider (it will create WalletConfiguration internally)
259259
const provider = await connectWallet({
260-
getUser: (forceRefresh) => (forceRefresh
261-
? this.auth.forceUserRefresh()
262-
: this.auth.getUserOrLogin()),
260+
getUser: (forceRefresh, getUserOptions) => {
261+
if (forceRefresh) return this.auth.forceUserRefresh();
262+
if (getUserOptions?.silent) return this.auth.getUser();
263+
return this.auth.getUserOrLogin();
264+
},
263265
clientId: this.passportConfig.oidcConfiguration.clientId,
264266
chains: [chainConfig],
265267
crossSdkBridgeEnabled: this.passportConfig.crossSdkBridgeEnabled,

packages/wallet/src/connectWallet.test.ts

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,11 +128,38 @@ describe('connectWallet', () => {
128128
);
129129
});
130130

131-
it('uses getUserOrLogin from internal Auth', async () => {
131+
it('uses getUser from internal Auth when silent (avoids popup on page load)', async () => {
132132
await connectWallet({ chains: [zkEvmChain] });
133133

134-
// Internal Auth's getUserOrLogin should be called during setup
135-
expect(mockAuthInstance.getUserOrLogin).toHaveBeenCalled();
134+
// Internal Auth's getUser should be called during setup (silent mode to avoid popup)
135+
expect(mockAuthInstance.getUser).toHaveBeenCalled();
136+
});
137+
138+
it('does not call getUserOrLogin during setup (silent flow avoids popup)', async () => {
139+
await connectWallet({ chains: [zkEvmChain] });
140+
141+
// Silent flow uses getUser only; getUserOrLogin would trigger popup
142+
expect(mockAuthInstance.getUserOrLogin).not.toHaveBeenCalled();
143+
});
144+
145+
it('uses getUserOrLogin when non-silent (e.g. eth_requestAccounts triggers login)', async () => {
146+
// When external getUser is provided, it may use getUserOrLogin for explicit login
147+
const getUser = jest.fn()
148+
.mockResolvedValueOnce(null) // setup (silent) - not used when we provide getUser
149+
.mockResolvedValueOnce({ profile: { sub: 'user' }, accessToken: 'token' });
150+
const getUserOrLogin = jest.fn().mockResolvedValue({ profile: { sub: 'user' }, accessToken: 'token' });
151+
152+
await connectWallet({
153+
getUser: async (forceRefresh?, options?) => {
154+
if (options?.silent) return getUser(forceRefresh, options);
155+
return getUserOrLogin(forceRefresh, options);
156+
},
157+
chains: [zkEvmChain],
158+
});
159+
160+
// Setup calls getUser with silent: true, so getUser (not getUserOrLogin) is used
161+
expect(getUser).toHaveBeenCalledWith(undefined, { silent: true });
162+
expect(getUserOrLogin).not.toHaveBeenCalled();
136163
});
137164

138165
it('derives passportDomain from chain apiUrl', async () => {
@@ -491,7 +518,7 @@ describe('connectWallet', () => {
491518

492519
describe('error handling', () => {
493520
it('handles auth failure gracefully', async () => {
494-
mockAuthInstance.getUserOrLogin.mockRejectedValueOnce(new Error('Auth failed'));
521+
mockAuthInstance.getUser.mockRejectedValueOnce(new Error('Auth failed'));
495522

496523
const provider = await connectWallet({ chains: [zkEvmChain] });
497524

packages/wallet/src/connectWallet.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -148,9 +148,13 @@ function createDefaultGetUser(initialChain: ChainConfig, options: ConnectWalletO
148148
});
149149
}
150150

151-
// Return getUser function that wraps Auth.getUserOrLogin
151+
// Return getUser function that wraps Auth.getUserOrLogin/getUser based on options
152152
return {
153-
getUser: async () => auth.getUserOrLogin(),
153+
getUser: async (forceRefresh?: boolean, getUserOptions?: { silent?: boolean }) => {
154+
if (forceRefresh) return auth.forceUserRefresh();
155+
if (getUserOptions?.silent) return auth.getUser();
156+
return auth.getUserOrLogin();
157+
},
154158
clientId,
155159
};
156160
}
@@ -222,8 +226,9 @@ export async function connectWallet(
222226
clientId = defaultAuth.clientId;
223227
}
224228

225-
// 4. Get current user (may be null if not logged in)
226-
const user = await getUser().catch(() => null);
229+
// 4. Get current user (may be null if not logged in).
230+
// Use silent: true to avoid triggering login popup on page load.
231+
const user = await getUser(undefined, { silent: true }).catch(() => null);
227232

228233
// 5. Create wallet configuration with concrete URLs
229234
const passportDomain = initialChain.passportDomain || initialChain.apiUrl.replace('api.', 'passport.');

packages/wallet/src/types.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ export type { RollupType } from '@imtbl/auth';
2323
// Wallet events
2424
export enum WalletEvents {
2525
ACCOUNTS_REQUESTED = 'accountsRequested',
26+
LOGGED_IN = 'loggedIn',
2627
LOGGED_OUT = 'loggedOut',
2728
}
2829

@@ -37,6 +38,7 @@ export type AccountsRequestedEvent = {
3738
// WalletEventMap for internal wallet events
3839
export interface WalletEventMap extends Record<string, any> {
3940
[WalletEvents.ACCOUNTS_REQUESTED]: [AccountsRequestedEvent];
41+
[WalletEvents.LOGGED_IN]: [User];
4042
[WalletEvents.LOGGED_OUT]: [];
4143
}
4244

@@ -214,6 +216,17 @@ export interface PopupOverlayOptions {
214216
disableBlockedPopupOverlay?: boolean;
215217
}
216218

219+
/**
220+
* Options for GetUserFunction calls.
221+
*/
222+
export interface GetUserOptions {
223+
/**
224+
* When true, do not trigger login (e.g. open popup) if user is not authenticated.
225+
* Returns null instead. Use during page load to avoid unwanted popups.
226+
*/
227+
silent?: boolean;
228+
}
229+
217230
/**
218231
* Function type for getting the current user.
219232
* Used as an alternative to passing an Auth instance.
@@ -222,8 +235,13 @@ export interface PopupOverlayOptions {
222235
* @param forceRefresh - When true, the auth layer should trigger a server-side
223236
* token refresh to get updated claims (e.g., after zkEVM registration).
224237
* This ensures the returned user has the latest data from the identity provider.
238+
* @param options - Optional. When options.silent is true, return null if not
239+
* authenticated instead of triggering login (avoids popup on page load).
225240
*/
226-
export type GetUserFunction = (forceRefresh?: boolean) => Promise<User | null>;
241+
export type GetUserFunction = (
242+
forceRefresh?: boolean,
243+
options?: GetUserOptions,
244+
) => Promise<User | null>;
227245

228246
/**
229247
* Options for connecting a wallet via connectWallet()
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import { TypedEventEmitter } from '@imtbl/auth';
2+
import { WalletEvents, WalletEventMap } from '../types';
3+
import { ProviderEvent } from './types';
4+
import { ZkEvmProvider } from './zkEvmProvider';
5+
import { WalletConfiguration } from '../config';
6+
7+
jest.mock('viem', () => {
8+
const actual = jest.requireActual<typeof import('viem')>('viem');
9+
return {
10+
...actual,
11+
createPublicClient: jest.fn(() => ({})),
12+
http: jest.fn(() => ({})),
13+
};
14+
});
15+
16+
jest.mock('./relayerClient', () => ({
17+
RelayerClient: jest.fn().mockImplementation(() => ({})),
18+
}));
19+
20+
jest.mock('../guardian', () => jest.fn().mockImplementation(() => ({
21+
withConfirmationScreen: () => (fn: () => Promise<any>) => fn(),
22+
})));
23+
24+
jest.mock('./sessionActivity/sessionActivity', () => ({
25+
trackSessionActivity: jest.fn(),
26+
}));
27+
28+
const mockUserWithZkEvm = {
29+
profile: { sub: 'user-123' },
30+
accessToken: 'token',
31+
zkEvm: { ethAddress: '0x1234567890123456789012345678901234567890' },
32+
};
33+
34+
describe('ZkEvmProvider', () => {
35+
const walletEventEmitter = new TypedEventEmitter<WalletEventMap>();
36+
const config = new WalletConfiguration({
37+
passportDomain: 'https://passport.immutable.com',
38+
zkEvmRpcUrl: 'https://rpc.test.immutable.com',
39+
relayerUrl: 'https://relayer.test.immutable.com',
40+
indexerMrBasePath: 'https://api.test.immutable.com',
41+
});
42+
43+
const mockGetUser = jest.fn().mockResolvedValue(null);
44+
const mockEthSigner = {
45+
getAddress: jest.fn().mockResolvedValue('0xabc'),
46+
signMessage: jest.fn(),
47+
signTypedData: jest.fn(),
48+
};
49+
50+
beforeEach(() => {
51+
jest.clearAllMocks();
52+
});
53+
54+
it('emits accountsChanged when LOGGED_IN event is received with zkEvm user', () => {
55+
const provider = new ZkEvmProvider({
56+
getUser: mockGetUser,
57+
clientId: 'test-client',
58+
config,
59+
multiRollupApiClients: {} as any,
60+
walletEventEmitter,
61+
guardianClient: {} as any,
62+
ethSigner: mockEthSigner as any,
63+
user: null,
64+
sessionActivityApiUrl: null,
65+
});
66+
67+
const accountsChangedHandler = jest.fn();
68+
provider.on(ProviderEvent.ACCOUNTS_CHANGED, accountsChangedHandler);
69+
70+
walletEventEmitter.emit(WalletEvents.LOGGED_IN, mockUserWithZkEvm as any);
71+
72+
expect(accountsChangedHandler).toHaveBeenCalledWith([mockUserWithZkEvm.zkEvm.ethAddress]);
73+
});
74+
});

packages/wallet/src/zkEvm/zkEvmProvider.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,9 @@ export class ZkEvmProvider implements Provider {
121121
this.#callSessionActivity(user.zkEvm.ethAddress);
122122
}
123123

124-
// Listen for logout events
124+
// Listen for auth events
125125
walletEventEmitter.on(WalletEvents.LOGGED_OUT, this.#handleLogout);
126+
walletEventEmitter.on(WalletEvents.LOGGED_IN, this.#handleLoggedIn);
126127
walletEventEmitter.on(
127128
WalletEvents.ACCOUNTS_REQUESTED,
128129
trackSessionActivity,
@@ -133,11 +134,24 @@ export class ZkEvmProvider implements Provider {
133134
this.#providerEventEmitter.emit(ProviderEvent.ACCOUNTS_CHANGED, []);
134135
};
135136

137+
#handleLoggedIn = (user: User) => {
138+
if (user && isZkEvmUser(user)) {
139+
this.#providerEventEmitter.emit(ProviderEvent.ACCOUNTS_CHANGED, [
140+
user.zkEvm.ethAddress,
141+
]);
142+
}
143+
// If user doesn't have zkEvm yet, app must call eth_requestAccounts to register
144+
};
145+
136146
/**
137147
* Get the current user using getUser function.
148+
* @param silent - When true, use getUser(undefined, { silent: true }) to avoid
149+
* triggering login popup on read-only checks (e.g. eth_accounts).
138150
*/
139-
async #getCurrentUser(): Promise<User | null> {
140-
return this.#getUser();
151+
async #getCurrentUser(silent = false): Promise<User | null> {
152+
return silent
153+
? this.#getUser(undefined, { silent: true })
154+
: this.#getUser();
141155
}
142156

143157
async #callSessionActivity(zkEvmAddress: string) {
@@ -174,7 +188,7 @@ export class ZkEvmProvider implements Provider {
174188
// Used to get the registered zkEvm address from the User session
175189
async #getZkEvmAddress() {
176190
try {
177-
const user = await this.#getCurrentUser();
191+
const user = await this.#getCurrentUser(true); // silent: avoid popup on read-only checks
178192
if (user && isZkEvmUser(user)) {
179193
return user.zkEvm.ethAddress;
180194
}

0 commit comments

Comments
 (0)