From 3144f7c0e8ef14003f721ab0c972ef117a7686a5 Mon Sep 17 00:00:00 2001 From: Sam Holmes Date: Fri, 6 Jun 2025 14:05:45 -0700 Subject: [PATCH 1/2] Combine related tests --- test/core/login/login.test.ts | 96 ++++++++++++++++++++--------------- 1 file changed, 55 insertions(+), 41 deletions(-) diff --git a/test/core/login/login.test.ts b/test/core/login/login.test.ts index b5ee0fa4..9917c90b 100644 --- a/test/core/login/login.test.ts +++ b/test/core/login/login.test.ts @@ -295,52 +295,66 @@ describe('pin', function () { expect(successAccount.id).equals(account.id) }) - it('disable pin in duress mode disables pin-login for duress mode', async function () { - const world = await makeFakeEdgeWorld([fakeUser], quiet) - const context = await world.makeEdgeContext(contextOptions) - const account = await context.loginWithPIN(fakeUser.username, fakeUser.pin) - await account.changePin({ pin: '0000', forDuressAccount: true }) - await account.logout() + describe('disable pin in duress mode', function () { + it('will disable pin-login for duress account', async function () { + const world = await makeFakeEdgeWorld([fakeUser], quiet) + const context = await world.makeEdgeContext(contextOptions) + const account = await context.loginWithPIN( + fakeUser.username, + fakeUser.pin + ) + await account.changePin({ pin: '0000', forDuressAccount: true }) + await account.logout() - // Disable PIN login: - const duressAccount = await context.loginWithPIN(fakeUser.username, '0000') - await duressAccount.changePin({ enableLogin: false }) - await expectRejection( - context.loginWithPIN(fakeUser.username, '0000'), - 'PinDisabledError: PIN login is not enabled for this account on this device' - ) - }) + // Disable PIN login: + const duressAccount = await context.loginWithPIN( + fakeUser.username, + '0000' + ) + await duressAccount.changePin({ enableLogin: false }) + await expectRejection( + context.loginWithPIN(fakeUser.username, '0000'), + 'PinDisabledError: PIN login is not enabled for this account on this device' + ) + }) - it('disable pin in duress mode disables pin-login for local user and disables pin-login for main account', async function () { - this.timeout(30000) - const world = await makeFakeEdgeWorld([fakeUser], quiet) - const context = await world.makeEdgeContext(contextOptions) - const account = await context.loginWithPIN(fakeUser.username, fakeUser.pin) - await account.changePin({ pin: '0000', forDuressAccount: true }) - await account.logout() + it('will disable pin-login for local user and main account', async function () { + this.timeout(30000) + const world = await makeFakeEdgeWorld([fakeUser], quiet) + const context = await world.makeEdgeContext(contextOptions) + const account = await context.loginWithPIN( + fakeUser.username, + fakeUser.pin + ) + await account.changePin({ pin: '0000', forDuressAccount: true }) + await account.logout() - // Disable PIN login: - const duressAccount = await context.loginWithPIN(fakeUser.username, '0000') - await duressAccount.changePin({ enableLogin: false }) + // Disable PIN login: + const duressAccount = await context.loginWithPIN( + fakeUser.username, + '0000' + ) + await duressAccount.changePin({ enableLogin: false }) - // Check the PIN login is disabled: - expect( - context.localUsers.map(user => ({ - username: user.username, - pinLoginEnabled: user.pinLoginEnabled - })) - ).deep.include.members([ - { - username: 'js test 0', - pinLoginEnabled: false - } - ]) + // Check the PIN login is disabled: + expect( + context.localUsers.map(user => ({ + username: user.username, + pinLoginEnabled: user.pinLoginEnabled + })) + ).deep.include.members([ + { + username: 'js test 0', + pinLoginEnabled: false + } + ]) - // Pin is disabled for main account: - await expectRejection( - context.loginWithPIN(fakeUser.username, fakeUser.pin), - 'PinDisabledError: PIN login is not enabled for this account on this device' - ) + // Pin is disabled for main account: + await expectRejection( + context.loginWithPIN(fakeUser.username, fakeUser.pin), + 'PinDisabledError: PIN login is not enabled for this account on this device' + ) + }) }) describe('disable pin in duress mode disables pin-login for _only_ its local user', function () { From 6758f8f5ad4a970514834ffb1959fd2486544b11 Mon Sep 17 00:00:00 2001 From: Sam Holmes Date: Fri, 6 Jun 2025 14:18:32 -0700 Subject: [PATCH 2/2] Show original error if pin-login is disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the pin-login is disabled for the second try within loginWithPIN, then we should show the original error message. This is important for when you disable duress mode that we show the invalid pin for the main account and also show the timeout message instead of “pin not enabled” error. --- CHANGELOG.md | 1 + src/core/context/context-api.ts | 33 +++++--- test/core/login/login.test.ts | 138 ++++++++++++++++++++------------ 3 files changed, 109 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bee962a4..2debb435 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased - fixed: Fix change-server reliability issues by fallback to pull-based syncing as a trade-off. +- fixed: Show the correct login errors when duress account is disabled. ## 2.31.0 (2025-06-05) diff --git a/src/core/context/context-api.ts b/src/core/context/context-api.ts index 72176441..6335ab55 100644 --- a/src/core/context/context-api.ts +++ b/src/core/context/context-api.ts @@ -4,6 +4,7 @@ import { checkPasswordRules, fixUsername } from '../../client-side' import { asChallengeErrorPayload, asMaybePasswordError, + asMaybePinDisabledError, EdgeAccount, EdgeAccountOptions, EdgeContext, @@ -324,21 +325,29 @@ export function makeContextApi(ai: ApiInput): EdgeContext { return inDuressMode ? await loginDuressAccount(stashTree, duressStash) : await loginMainAccount(stashTree, mainStash) - } catch (error) { + } catch (originalError) { // If the error is not a failed login, rethrow it: - if (asMaybePasswordError(error) == null) { - throw error + if (asMaybePasswordError(originalError) == null) { + throw originalError } - const account = inDuressMode - ? await loginMainAccount(stashTree, mainStash) - : await loginDuressAccount(stashTree, duressStash) - // Only Enable/Disable duress mode if account creation was success. - if (inDuressMode) { - await disableDuressMode() - } else { - await enableDuressMode() + try { + const account = inDuressMode + ? await loginMainAccount(stashTree, mainStash) + : await loginDuressAccount(stashTree, duressStash) + // Only Enable/Disable duress mode if account creation was success. + if (inDuressMode) { + await disableDuressMode() + } else { + await enableDuressMode() + } + return account + } catch (error) { + // Throw the original error if pin-login is disabled: + if (asMaybePinDisabledError(error) != null) { + throw originalError + } + throw error } - return account } }, diff --git a/test/core/login/login.test.ts b/test/core/login/login.test.ts index 9917c90b..8be79655 100644 --- a/test/core/login/login.test.ts +++ b/test/core/login/login.test.ts @@ -562,51 +562,59 @@ describe('pin', function () { ]) }) - it('exiting duress mode with a disable pin-login in duress account re-enables pin-login for local user but not for the duress account', async function () { - const world = await makeFakeEdgeWorld([fakeUser], quiet) - const context = await world.makeEdgeContext(contextOptions) - // Create a second account with duress setup: - const otherAccount = await context.createAccount({ - username: 'other-account', - pin: '1111' - }) - await otherAccount.changePin({ pin: '0000', forDuressAccount: true }) - await otherAccount.logout() + describe('when exiting duress mode from a duress account that has disabled pin-login', function () { + it('will re-enable pin-login for local user but not for the duress account', async function () { + const world = await makeFakeEdgeWorld([fakeUser], quiet) + const context = await world.makeEdgeContext(contextOptions) + // Create a second account with duress setup: + const otherAccount = await context.createAccount({ + username: 'other-account', + pin: '1111' + }) + await otherAccount.changePin({ pin: '0000', forDuressAccount: true }) + await otherAccount.logout() - // Setup duress on main account: - const account = await context.loginWithPIN(fakeUser.username, fakeUser.pin) - await account.changePin({ pin: '0000', forDuressAccount: true }) - await account.logout() + // Setup duress on main account: + const account = await context.loginWithPIN( + fakeUser.username, + fakeUser.pin + ) + await account.changePin({ pin: '0000', forDuressAccount: true }) + await account.logout() - // Disable PIN login for duress account on the main account: - const duressAccount = await context.loginWithPIN(fakeUser.username, '0000') - await duressAccount.changePin({ enableLogin: false }) - await duressAccount.logout() + // Disable PIN login for duress account on the main account: + const duressAccount = await context.loginWithPIN( + fakeUser.username, + '0000' + ) + await duressAccount.changePin({ enableLogin: false }) + await duressAccount.logout() - // Login/logout to other account in non-duress mode to disable duress mode: - await (await context.loginWithPIN('other-account', '1111')).logout() + // Login/logout to other account in non-duress mode to disable duress mode: + await (await context.loginWithPIN('other-account', '1111')).logout() - // Check the PIN login is enabled for local user: - expect( - context.localUsers.map(user => ({ - username: user.username, - pinLoginEnabled: user.pinLoginEnabled - })) - ).deep.include.members([ - { - username: 'js test 0', - pinLoginEnabled: true - } - ]) + // Check the PIN login is enabled for local user: + expect( + context.localUsers.map(user => ({ + username: user.username, + pinLoginEnabled: user.pinLoginEnabled + })) + ).deep.include.members([ + { + username: 'js test 0', + pinLoginEnabled: true + } + ]) - // Pin is disabled for duress account: - await expectRejection( - context.loginWithPIN(fakeUser.username, '0000'), - 'PinDisabledError: PIN login is not enabled for this account on this device' - ) + // Pin is disabled for duress account: + await expectRejection( + context.loginWithPIN(fakeUser.username, '0000'), + 'PasswordError: Invalid password' + ) - // Pin is enabled for main account: - await context.loginWithPIN(fakeUser.username, fakeUser.pin) + // Pin is enabled for main account: + await context.loginWithPIN(fakeUser.username, fakeUser.pin) + }) }) it('checkPin still works after disabling pin-login while in duress mode', async function () { @@ -720,19 +728,47 @@ describe('pin', function () { expect(successAccount.id).equals(account.id) }) - it('disable duress does not disable pin-login', async function () { - const world = await makeFakeEdgeWorld([fakeUser], quiet) - const context = await world.makeEdgeContext(contextOptions) - expect(context.localUsers[0].pinLoginEnabled).equals(true) - // Setup duress mode: - const account = await context.loginWithPIN(fakeUser.username, fakeUser.pin) - await account.changePin({ pin: '0000', forDuressAccount: true }) - // Disable duress mode: - await account.changePin({ - enableLogin: false, - forDuressAccount: true + describe('disable duress mode', function () { + it('will not disable pin-login for main account', async function () { + const world = await makeFakeEdgeWorld([fakeUser], quiet) + const context = await world.makeEdgeContext(contextOptions) + expect(context.localUsers[0].pinLoginEnabled).equals(true) + // Setup duress mode: + const account = await context.loginWithPIN( + fakeUser.username, + fakeUser.pin + ) + await account.changePin({ pin: '0000', forDuressAccount: true }) + // Disable duress mode: + await account.changePin({ + enableLogin: false, + forDuressAccount: true + }) + expect(context.localUsers[0].pinLoginEnabled).equals(true) + }) + + it('will not show duress-account pin errors', async function () { + const world = await makeFakeEdgeWorld([fakeUser], quiet) + const context = await world.makeEdgeContext(contextOptions) + expect(context.localUsers[0].pinLoginEnabled).equals(true) + // Setup duress mode: + const account = await context.loginWithPIN( + fakeUser.username, + fakeUser.pin + ) + await account.changePin({ pin: '0000', forDuressAccount: true }) + // Disable duress mode: + await account.changePin({ + enableLogin: false, + forDuressAccount: true + }) + await account.logout() + + await expectRejection( + context.loginWithPIN(fakeUser.username, '4444'), + 'PasswordError: Invalid password' + ) }) - expect(context.localUsers[0].pinLoginEnabled).equals(true) }) it('check', async function () {