Skip to content

Commit 7b8c409

Browse files
committed
Make origin validation happen asynchronously from the popup
1 parent 04b4986 commit 7b8c409

File tree

7 files changed

+37
-24
lines changed

7 files changed

+37
-24
lines changed

packages-exp/auth-exp/src/core/auth/initialize.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ describe('core/auth/initialize', () => {
110110
): void {
111111
cb(true);
112112
}
113+
async _originValidation(): Promise<void> {}
113114
async _completeRedirectFn(
114115
_auth: externs.Auth,
115116
_resolver: externs.PopupRedirectResolver,

packages-exp/auth-exp/src/model/popup_redirect.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ export interface PopupRedirectResolver extends externs.PopupRedirectResolver {
9292
cb: (support: boolean) => unknown
9393
): void;
9494
_redirectPersistence: externs.Persistence;
95+
_originValidation(auth: Auth): Promise<void>;
9596

9697
// This is needed so that auth does not have a hard dependency on redirect
9798
_completeRedirectFn: (

packages-exp/auth-exp/src/platform_browser/popup_redirect.test.ts

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -117,13 +117,6 @@ describe('platform_browser/popup_redirect', () => {
117117
);
118118
});
119119

120-
it('validates the origin', async () => {
121-
await resolver._initialize(auth);
122-
123-
await resolver._openPopup(auth, provider, event);
124-
expect(validateOrigin._validateOrigin).to.have.been.calledWith(auth);
125-
});
126-
127120
it('throws an error if apiKey is unspecified', async () => {
128121
delete (auth.config as Partial<Config>).apiKey;
129122
await resolver._initialize(auth);
@@ -132,17 +125,6 @@ describe('platform_browser/popup_redirect', () => {
132125
resolver._openPopup(auth, provider, event)
133126
).to.be.rejectedWith(FirebaseError, 'auth/invalid-api-key');
134127
});
135-
136-
it('rejects immediately if origin validation fails', async () => {
137-
await resolver._initialize(auth);
138-
(validateOrigin._validateOrigin as sinon.SinonStub).returns(
139-
Promise.reject(new Error('invalid-origin'))
140-
);
141-
142-
await expect(
143-
resolver._openPopup(auth, provider, event)
144-
).to.be.rejectedWith(Error, 'invalid-origin');
145-
});
146128
});
147129

148130
context('#_openRedirect', () => {
@@ -213,6 +195,26 @@ describe('platform_browser/popup_redirect', () => {
213195
});
214196
});
215197

198+
context('#_originValidation', () => {
199+
it('validates the origin', async () => {
200+
await resolver._initialize(auth);
201+
202+
await resolver._originValidation(auth);
203+
expect(validateOrigin._validateOrigin).to.have.been.calledWith(auth);
204+
});
205+
206+
it('rejects if origin validation fails', async () => {
207+
await resolver._initialize(auth);
208+
(validateOrigin._validateOrigin as sinon.SinonStub).returns(
209+
Promise.reject(new Error('invalid-origin'))
210+
);
211+
212+
await expect(
213+
resolver._originValidation(auth)
214+
).to.be.rejectedWith(Error, 'invalid-origin');
215+
});
216+
});
217+
216218
context('#_initialize', () => {
217219
it('returns different manager for a different auth', async () => {
218220
const manager = await resolver._initialize(auth);

packages-exp/auth-exp/src/platform_browser/popup_redirect.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,6 @@ class BrowserPopupRedirectResolver implements PopupRedirectResolver {
7373
'_initialize() not called before _openPopup()'
7474
);
7575

76-
await this.originValidation(auth);
7776
const url = _getRedirectUrl(
7877
auth,
7978
provider,
@@ -90,7 +89,7 @@ class BrowserPopupRedirectResolver implements PopupRedirectResolver {
9089
authType: AuthEventType,
9190
eventId?: string
9291
): Promise<never> {
93-
await this.originValidation(auth);
92+
await this._originValidation(auth);
9493
_setWindowLocation(
9594
_getRedirectUrl(auth, provider, authType, _getCurrentUrl(), eventId)
9695
);
@@ -154,7 +153,7 @@ class BrowserPopupRedirectResolver implements PopupRedirectResolver {
154153
);
155154
}
156155

157-
private originValidation(auth: Auth): Promise<void> {
156+
_originValidation(auth: Auth): Promise<void> {
158157
const key = auth._key();
159158
if (!this.originValidationPromises[key]) {
160159
this.originValidationPromises[key] = _validateOrigin(auth);

packages-exp/auth-exp/src/platform_browser/strategies/popup.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -235,13 +235,17 @@ class PopupOperation extends AbstractPopupRedirectOperation {
235235
);
236236
this.authWindow.associatedEvent = eventId;
237237

238-
// Check for web storage support _after_ the popup is loaded. Checking for
239-
// web storage is slow (on the order of a second or so). Rather than
240-
// waiting on that before opening the window, optimistically open the popup
238+
// Check for web storage support and origin validation _after_ the popup is
239+
// loaded. These operations are slow (~1 second or so) Rather than
240+
// waiting on them before opening the window, optimistically open the popup
241241
// and check for storage support at the same time. If storage support is
242242
// not available, this will cause the whole thing to reject properly. It
243243
// will also close the popup, but since the promise has already rejected,
244244
// the popup closed by user poll will reject into the void.
245+
this.resolver._originValidation(this.auth).catch(e => {
246+
this.reject(e);
247+
});
248+
245249
this.resolver._isIframeWebStorageSupported(this.auth, isSupported => {
246250
if (!isSupported) {
247251
this.reject(

packages-exp/auth-exp/src/platform_cordova/popup_redirect/popup_redirect.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,10 @@ class CordovaPopupRedirectResolver implements PopupRedirectResolver {
101101
throw new Error('Method not implemented.');
102102
}
103103

104+
_originValidation(): Promise<void> {
105+
return Promise.resolve();
106+
}
107+
104108
private attachCallbackListeners(auth: Auth, manager: AuthEventManager): void {
105109
const noEventTimeout = setTimeout(async () => {
106110
// We didn't see that initial event. Clear any pending object and

packages-exp/auth-exp/test/helpers/mock_popup_redirect_resolver.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,5 +52,7 @@ export function makeMockPopupRedirectResolver(
5252
_redirectPersistence?: Persistence;
5353

5454
async _completeRedirectFn(): Promise<void> {}
55+
56+
async _originValidation(): Promise<void> {}
5557
};
5658
}

0 commit comments

Comments
 (0)