From 1d32137c546a576298adb1713a9862cc92a26c83 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Wed, 29 Nov 2023 14:26:15 -0500 Subject: [PATCH 1/4] [App Check] Install catch handler for promise in Deferred. (#7822) Install catch handlers for promises in the Derferred object used by in AppCheck. There were some cases where the promise was cancelled and this bubbled up error messages to our clients' applications despite this being expected behavior. This change is to alleviate the logging reported in issue #7805. --- .changeset/shiny-houses-stare.md | 5 +++++ packages/app-check/src/proactive-refresh.ts | 6 ++++++ 2 files changed, 11 insertions(+) create mode 100644 .changeset/shiny-houses-stare.md diff --git a/.changeset/shiny-houses-stare.md b/.changeset/shiny-houses-stare.md new file mode 100644 index 00000000000..ab8ffb04cbc --- /dev/null +++ b/.changeset/shiny-houses-stare.md @@ -0,0 +1,5 @@ +--- +'@firebase/app-check': patch +--- + +Prevent App Check from logging "uncaught" cancelled promises. The cancelled promises are part of App Check's expected behavior, and their cancellation wasn't intended to produce errors or warnings. See issue #7805. diff --git a/packages/app-check/src/proactive-refresh.ts b/packages/app-check/src/proactive-refresh.ts index 89bc046935b..20f7c61e356 100644 --- a/packages/app-check/src/proactive-refresh.ts +++ b/packages/app-check/src/proactive-refresh.ts @@ -64,6 +64,9 @@ export class Refresher { this.stop(); try { this.pending = new Deferred(); + this.pending.promise.catch(_e => { + /* ignore */ + }); await sleep(this.getNextRun(hasSucceeded)); // Why do we resolve a promise, then immediate wait for it? @@ -74,6 +77,9 @@ export class Refresher { this.pending.resolve(); await this.pending.promise; this.pending = new Deferred(); + this.pending.promise.catch(_e => { + /* ignore */ + }); await this.operation(); this.pending.resolve(); From 70e4cf6a6544c4ccfa609c3f2c320980e7122101 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Wed, 29 Nov 2023 15:46:42 -0500 Subject: [PATCH 2/4] [Auth] Detect if IndexedDB returns an empty array in platform_browser/persistence. (#7825) Protection from enumerating an empty list in Auth's reading of IndexedDB results, as this causes errors in some macOS and iOS browser runtimes. This change addresses Issue #7737 which reported an error of `TypeError undefined is not an object (evaluating 'o.fbase_key')`. --- .changeset/tiny-lobsters-own.md | 5 +++++ .../src/platform_browser/persistence/indexed_db.ts | 13 ++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) create mode 100644 .changeset/tiny-lobsters-own.md diff --git a/.changeset/tiny-lobsters-own.md b/.changeset/tiny-lobsters-own.md new file mode 100644 index 00000000000..88c32d2cf9e --- /dev/null +++ b/.changeset/tiny-lobsters-own.md @@ -0,0 +1,5 @@ +--- +'@firebase/auth': patch +--- + +Protection from enumerating an empty list in Auth's reading of IndexedDB results, as this causes errors in some macOS and iOS browser runtimes. diff --git a/packages/auth/src/platform_browser/persistence/indexed_db.ts b/packages/auth/src/platform_browser/persistence/indexed_db.ts index a9231c0870d..597b45d3ff4 100644 --- a/packages/auth/src/platform_browser/persistence/indexed_db.ts +++ b/packages/auth/src/platform_browser/persistence/indexed_db.ts @@ -369,13 +369,16 @@ class IndexedDBLocalPersistence implements InternalPersistence { const keys = []; const keysInResult = new Set(); - for (const { fbase_key: key, value } of result) { - keysInResult.add(key); - if (JSON.stringify(this.localCache[key]) !== JSON.stringify(value)) { - this.notifyListeners(key, value as PersistenceValue); - keys.push(key); + if (result.length !== 0) { + for (const { fbase_key: key, value } of result) { + keysInResult.add(key); + if (JSON.stringify(this.localCache[key]) !== JSON.stringify(value)) { + this.notifyListeners(key, value as PersistenceValue); + keys.push(key); + } } } + for (const localKey of Object.keys(this.localCache)) { if (this.localCache[localKey] && !keysInResult.has(localKey)) { // Deleted From 0ecaf6c9fa14b40d32c52215d9e1c912bcc52bef Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Thu, 30 Nov 2023 11:45:08 -0800 Subject: [PATCH 3/4] Add app-check-interop-types dependency to database (#7827) --- .changeset/wet-moons-wave.md | 5 +++++ packages/database/package.json | 1 + 2 files changed, 6 insertions(+) create mode 100644 .changeset/wet-moons-wave.md diff --git a/.changeset/wet-moons-wave.md b/.changeset/wet-moons-wave.md new file mode 100644 index 00000000000..24e400e16aa --- /dev/null +++ b/.changeset/wet-moons-wave.md @@ -0,0 +1,5 @@ +--- +'@firebase/database': patch +--- + +Add `@firebase/app-check-interop-types` dependency to `database` package diff --git a/packages/database/package.json b/packages/database/package.json index ded628034f8..81e36946930 100644 --- a/packages/database/package.json +++ b/packages/database/package.json @@ -52,6 +52,7 @@ "@firebase/logger": "0.4.0", "@firebase/util": "1.9.3", "@firebase/component": "0.6.4", + "@firebase/app-check-interop-types": "0.3.0", "@firebase/auth-interop-types": "0.2.1", "faye-websocket": "0.11.4", "tslib": "^2.1.0" From ac10cc3491772a3b8500852625029a1602cb2635 Mon Sep 17 00:00:00 2001 From: renkelvin Date: Fri, 1 Dec 2023 17:03:26 -0800 Subject: [PATCH 4/4] Re-enable Auth webdriver tests (#7797) --- .../webdriver/compat/firebaseui.test.ts | 6 +- .../integration/webdriver/redirect.test.ts | 64 +++---------------- .../integration/webdriver/util/test_server.ts | 6 +- 3 files changed, 16 insertions(+), 60 deletions(-) diff --git a/packages/auth/test/integration/webdriver/compat/firebaseui.test.ts b/packages/auth/test/integration/webdriver/compat/firebaseui.test.ts index 9a79a2a9edf..79fd3fca77b 100644 --- a/packages/auth/test/integration/webdriver/compat/firebaseui.test.ts +++ b/packages/auth/test/integration/webdriver/compat/firebaseui.test.ts @@ -51,11 +51,7 @@ browserDescribe('WebDriver integration with FirebaseUI', driver => { expect(snap.uid).to.be.a('string'); }); - it('allows google redirect sign in', async function () { - // Test is ignored for now as it fails. - // TODO: Investigate and unskip the test. - this.skip(); - + it('allows google redirect sign in', async () => { const page = await startUi(); await page.clickGoogleSignIn(); const widget = new IdPPage(driver.webDriver); diff --git a/packages/auth/test/integration/webdriver/redirect.test.ts b/packages/auth/test/integration/webdriver/redirect.test.ts index 284c79aa6c0..4dbd4ea0665 100644 --- a/packages/auth/test/integration/webdriver/redirect.test.ts +++ b/packages/auth/test/integration/webdriver/redirect.test.ts @@ -48,11 +48,7 @@ browserDescribe('WebDriver redirect IdP test', driver => { await driver.start('chrome'); }); - it('allows users to sign in', async function () { - // Test is ignored for now as it fails. - // TODO: Investigate and unskip the test. - this.skip(); - + it('allows users to sign in', async () => { await driver.callNoWait(RedirectFunction.IDP_REDIRECT); const widget = new IdPPage(driver.webDriver); @@ -84,10 +80,6 @@ browserDescribe('WebDriver redirect IdP test', driver => { // Redirect works with middleware for now it('is blocked by middleware', async function () { - // Test is ignored for now as it fails. - // TODO: Investigate and unskip the test. - this.skip(); - if (driver.isCompatLayer()) { console.warn('Skipping middleware tests in compat'); this.skip(); @@ -114,11 +106,7 @@ browserDescribe('WebDriver redirect IdP test', driver => { expect(await driver.getUserSnapshot()).to.be.null; }); - it('can link with another account account', async function () { - // Test is ignored for now as it fails. - // TODO: Investigate and unskip the test. - this.skip(); - + it('can link with another account account', async () => { // First, sign in anonymously const { user: anonUser }: UserCredential = await driver.call( AnonFunction.SIGN_IN_ANONYMOUSLY @@ -140,11 +128,7 @@ browserDescribe('WebDriver redirect IdP test', driver => { expect(user.email).to.eq('bob@bob.test'); }); - it('can be converted to a credential', async function () { - // Test is ignored for now as it fails. - // TODO: Investigate and unskip the test. - this.skip(); - + it('can be converted to a credential', async () => { // Start with redirect await driver.callNoWait(RedirectFunction.IDP_REDIRECT); const widget = new IdPPage(driver.webDriver); @@ -172,11 +156,7 @@ browserDescribe('WebDriver redirect IdP test', driver => { expect(second.providerData).to.eql(first.providerData); }); - it('handles account exists different credential errors', async function () { - // Test is ignored for now as it fails. - // TODO: Investigate and unskip the test. - this.skip(); - + it('handles account exists different credential errors', async () => { // Start with redirect and a verified account await driver.callNoWait(RedirectFunction.IDP_REDIRECT); const widget = new IdPPage(driver.webDriver); @@ -211,11 +191,7 @@ browserDescribe('WebDriver redirect IdP test', driver => { ]); }); - it('does not auto-upgrade anon accounts', async function () { - // Test is ignored for now as it fails. - // TODO: Investigate and unskip the test. - this.skip(); - + it('does not auto-upgrade anon accounts', async () => { const { user: anonUser }: UserCredential = await driver.call( AnonFunction.SIGN_IN_ANONYMOUSLY ); @@ -232,11 +208,7 @@ browserDescribe('WebDriver redirect IdP test', driver => { expect(curUser.uid).not.to.eq(anonUser.uid); }); - it('linking with anonymous user upgrades account', async function () { - // Test is ignored for now as it fails. - // TODO: Investigate and unskip the test. - this.skip(); - + it('linking with anonymous user upgrades account', async () => { const { user: anonUser }: UserCredential = await driver.call( AnonFunction.SIGN_IN_ANONYMOUSLY ); @@ -254,11 +226,7 @@ browserDescribe('WebDriver redirect IdP test', driver => { expect(curUser.isAnonymous).to.be.false; }); - it('is possible to link with different email', async function () { - // Test is ignored for now as it fails. - // TODO: Investigate and unskip the test. - this.skip(); - + it('is possible to link with different email', async () => { const { user: emailUser }: UserCredential = await driver.call( EmailFunction.CREATE_USER, 'user@test.test' @@ -281,11 +249,7 @@ browserDescribe('WebDriver redirect IdP test', driver => { expect(curUser.providerData.length).to.eq(2); }); - it('is possible to link with the same email', async function () { - // Test is ignored for now as it fails. - // TODO: Investigate and unskip the test. - this.skip(); - + it('is possible to link with the same email', async () => { const { user: emailUser }: UserCredential = await driver.call( EmailFunction.CREATE_USER, 'same@test.test' @@ -327,11 +291,7 @@ browserDescribe('WebDriver redirect IdP test', driver => { await driver.call(CoreFunction.SIGN_OUT); }); - it('a user can sign in again', async function () { - // Test is ignored for now as it fails. - // TODO: Investigate and unskip the test. - this.skip(); - + it('a user can sign in again', async () => { // Sign in using pre-poulated user await driver.callNoWait(RedirectFunction.IDP_REDIRECT); @@ -347,11 +307,7 @@ browserDescribe('WebDriver redirect IdP test', driver => { expect(user.email).to.eq(user1.email); }); - it('reauthenticate works for the correct user', async function () { - // Test is ignored for now as it fails. - // TODO: Investigate and unskip the test. - this.skip(); - + it('reauthenticate works for the correct user', async () => { // Sign in using pre-poulated user await driver.callNoWait(RedirectFunction.IDP_REDIRECT); diff --git a/packages/auth/test/integration/webdriver/util/test_server.ts b/packages/auth/test/integration/webdriver/util/test_server.ts index 0c76e680d0e..9702c450135 100644 --- a/packages/auth/test/integration/webdriver/util/test_server.ts +++ b/packages/auth/test/integration/webdriver/util/test_server.ts @@ -39,7 +39,11 @@ class AuthTestServer { } get address(): string { - return `http://localhost:${PORT_NUMBER}`; + /* + We use "127.0.0.1" rather than "localhost" since emulator uses the former, + and we want to be on the same origin (to access session storage for redirect-based tests). + */ + return `http://127.0.0.1:${PORT_NUMBER}`; } async start(): Promise {