From 25cfbcbea9056721b4989769c07997051d8d08e3 Mon Sep 17 00:00:00 2001 From: Sam Holmes Date: Wed, 21 May 2025 17:57:05 -0700 Subject: [PATCH 1/2] Fix loginWithPassword duress mode edge-case This bug is where login with password from the login server does not persist duress mode for the account, but persists it for other accounts when attempting to login using a key. The expected behavior is that login with password or key while in duress mode should always bring you to the duress mode account/stash regardless of whether login was online or offline. This is because online and offline should always behave the same. --- CHANGELOG.md | 2 ++ src/core/context/context-api.ts | 5 ++++ test/core/login/login.test.ts | 44 +++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f144439..e150f7a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- fixed: `loginWithPassword` bug disabled duress mode when doing an online login. + ## 2.27.4 (2025-05-13) - fixed: Disable duress pin mistakenly disables pin-login for entire account. diff --git a/src/core/context/context-api.ts b/src/core/context/context-api.ts index f560d90d..a0dc18e5 100644 --- a/src/core/context/context-api.ts +++ b/src/core/context/context-api.ts @@ -214,6 +214,11 @@ export function makeContextApi(ai: ApiInput): EdgeContext { // Attempt to log into duress account if duress mode is enabled: if (ai.props.state.clientInfo.duressEnabled) { const duressAppId = appId + '.duress' + const stash = getStashByUsername(ai, username) + if (stash == null) { + // This should never happen. + throw new Error('Missing stash after login with password') + } const duressStash = searchTree( stash, stash => stash.appId === duressAppId diff --git a/test/core/login/login.test.ts b/test/core/login/login.test.ts index 8b0b7774..75af8112 100644 --- a/test/core/login/login.test.ts +++ b/test/core/login/login.test.ts @@ -537,6 +537,50 @@ describe('duress', function () { expect(topicAccount.isDuressAccount).equals(true) }) + it('persist duress mode when using loginWithPassword on a forgotten account', async function () { + const world = await makeFakeEdgeWorld([fakeUser], quiet) + const context = await world.makeEdgeContext(contextOptions) + // Setup first account's duress mode: + const account = await context.loginWithPIN(fakeUser.username, fakeUser.pin) + await account.changePin({ pin: '0001', forDuressAccount: true }) + await account.logout() + + // Setup other account with duress mode: + const otherAccount = await context.createAccount({ + username: 'other-account', + pin: '1111' + }) + await otherAccount.changePin({ pin: '0002', forDuressAccount: true }) + const loginKey = await otherAccount.getLoginKey() + await otherAccount.logout() + + // Enable duress mode: + const duressAccount = await context.loginWithPIN(fakeUser.username, '0001') + await duressAccount.logout() + + // Forget the first account: + await context.forgetAccount(account.rootLoginId) + + // Password login with the forgotten account: + let topicAccount = await context.loginWithPassword( + fakeUser.username, + fakeUser.password, + { otpKey: 'HELLO' } + ) + + expect(topicAccount.username).equals('js test 0') + expect(topicAccount.isDuressAccount).equals(true) + expect(topicAccount.appId).equals('.duress') + + await topicAccount.logout() + + // Make sure second account is not in duress mode: + topicAccount = await context.loginWithKey('other-account', loginKey) + expect(topicAccount.username).equals('other-account') + expect(topicAccount.isDuressAccount).equals(true) + expect(topicAccount.appId).equals('.duress') + }) + it('Avoid creating duress account when using loginWithPassword', async function () { const world = await makeFakeEdgeWorld([fakeUser], quiet) const context = await world.makeEdgeContext(contextOptions) From 97e61954459d2bf906ec1812e25904f777971dda Mon Sep 17 00:00:00 2001 From: Sam Holmes Date: Mon, 19 May 2025 17:17:35 -0700 Subject: [PATCH 2/2] Fix pin enable/disable while in duress mode Completely fake enable and disable pin settings stash by looking at the duress stash within `localUser` reducer and respecting PinDisabledError that is thrown in `loginWithPIN`. --- CHANGELOG.md | 1 + src/core/context/context-api.ts | 6 +- src/core/login/login-reducer.ts | 9 +- src/core/login/login.ts | 41 ------- src/core/login/pin2.ts | 48 +++++--- test/core/account/account.test.ts | 29 ++++- test/core/login/login.test.ts | 175 +++++++++++++++++++++++++++++- 7 files changed, 240 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e150f7a1..62387979 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- fixed: Disabling pin-login while in duress mode disables pin-login for the main login. - fixed: `loginWithPassword` bug disabled duress mode when doing an online login. ## 2.27.4 (2025-05-13) diff --git a/src/core/context/context-api.ts b/src/core/context/context-api.ts index a0dc18e5..72176441 100644 --- a/src/core/context/context-api.ts +++ b/src/core/context/context-api.ts @@ -4,7 +4,6 @@ import { checkPasswordRules, fixUsername } from '../../client-side' import { asChallengeErrorPayload, asMaybePasswordError, - asMaybePinDisabledError, EdgeAccount, EdgeAccountOptions, EdgeContext, @@ -327,10 +326,7 @@ export function makeContextApi(ai: ApiInput): EdgeContext { : await loginMainAccount(stashTree, mainStash) } catch (error) { // If the error is not a failed login, rethrow it: - if ( - asMaybePasswordError(error) == null && - asMaybePinDisabledError(error) == null - ) { + if (asMaybePasswordError(error) == null) { throw error } const account = inDuressMode diff --git a/src/core/login/login-reducer.ts b/src/core/login/login-reducer.ts index fe58b285..1c516298 100644 --- a/src/core/login/login-reducer.ts +++ b/src/core/login/login-reducer.ts @@ -8,7 +8,7 @@ import { RootState } from '../root-reducer' import { searchTree } from './login' import { LoginStash } from './login-stash' import { WalletInfoFullMap } from './login-types' -import { findPin2Stash } from './pin2' +import { findPin2Stash, findPin2StashDuress } from './pin2' export interface LoginState { readonly apiKey: string @@ -50,7 +50,12 @@ export const login = buildReducer({ const keyLoginEnabled = stash != null && (stash.passwordAuthBox != null || stash.loginAuthBox != null) - const pin2Stash = findPin2Stash(stashTree, appId) + + // This allows us to lie about PIN being enabled or disabled while in + // duress mode! + const pin2Stash = clientInfo.duressEnabled + ? findPin2StashDuress(stashTree, appId) + : findPin2Stash(stashTree, appId) return { keyLoginEnabled, diff --git a/src/core/login/login.ts b/src/core/login/login.ts index dcea445d..5b456d1e 100644 --- a/src/core/login/login.ts +++ b/src/core/login/login.ts @@ -436,34 +436,6 @@ export async function applyKit( return newStashTree } -/** - * Applies changes to a login, but only temporarily (locally). - * This is used for duress account changes which need to pretend to make real - * changes to a login, only for them to be reverted once the device synchronizes - * with the login server - */ -export async function applyKitTemporarily( - ai: ApiInput, - kit: LoginKit -): Promise { - const { stashTree } = getStashById(ai, kit.loginId) - - const newStashTree = updateTree( - stashTree, - stash => verifyData(stash.loginId, kit.loginId), - stash => ({ - ...stash, - ...kit.stash, - children: softCat(stash.children, kit.stash.children), - keyBoxes: softCat(stash.keyBoxes, kit.stash.keyBoxes) - }), - (stash, children) => ({ ...stash, children }) - ) - await saveStash(ai, newStashTree) - - return newStashTree -} - /** * Applies an array of kits to a login, one after another. * We can't use `Promise.all`, since `applyKit` doesn't handle @@ -481,19 +453,6 @@ export async function applyKits( } } -/** - * Applies an array of kits to a login, _temporarily_ (locally). - */ -export async function applyKitsTemporarily( - ai: ApiInput, - kits: Array -): Promise { - for (const kit of kits) { - if (kit == null) continue - await applyKitTemporarily(ai, kit) - } -} - /** * Refreshes a login with data from the server. */ diff --git a/src/core/login/pin2.ts b/src/core/login/pin2.ts index 40f9926d..9ddf71fe 100644 --- a/src/core/login/pin2.ts +++ b/src/core/login/pin2.ts @@ -12,12 +12,7 @@ import { decrypt, encrypt } from '../../util/crypto/crypto' import { hmacSha256 } from '../../util/crypto/hashes' import { utf8 } from '../../util/encoding' import { ApiInput } from '../root-pixie' -import { - applyKits, - applyKitsTemporarily, - searchTree, - serverLogin -} from './login' +import { applyKits, searchTree, serverLogin } from './login' import { loginFetch } from './login-fetch' import { getStashById } from './login-selectors' import { LoginStash } from './login-stash' @@ -48,6 +43,21 @@ export function findPin2Stash( if (stash?.pin2Key != null) return stash } +/** + * Returns a copy of the PIN login key if one exists on the local device and + * the app id matches the duress appId. + */ +export function findPin2StashDuress( + stashTree: LoginStash, + appId: string +): LoginStash | undefined { + const duressAppId = appId.endsWith('.duress') ? appId : appId + '.duress' + if (stashTree.pin2Key != null && duressAppId === stashTree.appId) + return stashTree + const stash = searchTree(stashTree, stash => stash.appId === duressAppId) + if (stash?.pin2Key != null) return stash +} + /** * Logs a user in using their PIN. * @return A `Promise` for the new root login. @@ -100,16 +110,24 @@ export async function changePin( // Deleting PIN logins while in duress account should delete PIN locally for // all nodes: - if (inDuressMode && !forDuressAccount && !enableLogin) { - if (pin == null) { - await applyKitsTemporarily(ai, makeDeletePin2Kits(loginTree)) + if (inDuressMode && !forDuressAccount) { + if (enableLogin) { + if (pin != null) { + await applyKits( + ai, + sessionKey, + makeChangePin2Kits(ai, loginTree, username, pin, enableLogin, true) + ) + } } else { - await applyKitsTemporarily(ai, [ - // Delete for other apps: - ...makeDeletePin2Kits(loginTree, false), - // Change PIN for duress app: - ...makeChangePin2Kits(ai, loginTree, username, pin, enableLogin, true) - ]) + if (pin != null) { + // Change and disable PIN for duress app for real: + await applyKits( + ai, + sessionKey, + makeChangePin2Kits(ai, loginTree, username, pin, enableLogin, true) + ) + } } return } diff --git a/test/core/account/account.test.ts b/test/core/account/account.test.ts index e08b8255..2f35dd28 100644 --- a/test/core/account/account.test.ts +++ b/test/core/account/account.test.ts @@ -368,7 +368,7 @@ describe('account', function () { ).deep.include.members([{ username: 'js test 0', pinLoginEnabled: false }]) }) - it('disable pin while in duress account is temporary', async function () { + it('disable pin while in duress account is not temporary', async function () { const world = await makeFakeEdgeWorld([fakeUser], quiet) const context = await world.makeEdgeContext(contextOptions) @@ -384,10 +384,31 @@ describe('account', function () { await duressAccount.logout() // Forget account: await context.forgetAccount(account.rootLoginId) + // Login with password: - await context.loginWithPassword(fakeUser.username, fakeUser.password, { - otpKey: 'HELLO' - }) + const topicAccount = await context.loginWithPassword( + fakeUser.username, + fakeUser.password, + { + otpKey: 'HELLO' + } + ) + // Pin should be disabled for account because it is still in duress mode: + expect( + context.localUsers.map(({ pinLoginEnabled, username }) => ({ + pinLoginEnabled, + username + })) + ).deep.include.members([ + { username: fakeUser.username.toLowerCase(), pinLoginEnabled: false } + ]) + + await topicAccount.changePin({ enableLogin: true }) + await topicAccount.logout() + + // Login with non-duress PIN: + await context.loginWithPIN(fakeUser.username, fakeUser.pin) + // Pin should be enabled for account: expect( context.localUsers.map(({ pinLoginEnabled, username }) => ({ diff --git a/test/core/login/login.test.ts b/test/core/login/login.test.ts index 75af8112..3144e30a 100644 --- a/test/core/login/login.test.ts +++ b/test/core/login/login.test.ts @@ -295,39 +295,210 @@ describe('pin', function () { expect(successAccount.id).equals(account.id) }) - it('enable / disable duress', async function () { + 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() + + // 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 () { + 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 }) + + // 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' + ) + }) + + 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() + + 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: + 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() + + // 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 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 () { + 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 }) + // Check the PIN should still work as expected: expect(await duressAccount.checkPin('0000')).equals(true) expect(await duressAccount.checkPin('1234')).equals(false) + }) + + it('change pin still works while in duress mode and pin-login is disabled', 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 }) // Change PIN, leaving it disabled: await duressAccount.changePin({ pin: '9999', enableLogin: false }) + // Pin is still disabled: await expectRejection( context.loginWithPIN(fakeUser.username, '9999'), 'PinDisabledError: PIN login is not enabled for this account on this device' ) + // Check pin still works expect(await duressAccount.checkPin('9999')).equals(true) expect(await duressAccount.checkPin('0000')).equals(false) + }) - // Enable PIN login: + it('re-enable pin-login while in duress mode re-enables pin-login for the duress 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 }) + + // Change PIN, leaving it disabled: + await duressAccount.changePin({ pin: '9999', enableLogin: false }) + + // Re-enable PIN login: await duressAccount.changePin({ enableLogin: true }) + + // Check the PIN login is enabled again: + expect( + context.localUsers.map(user => ({ + username: user.username, + pinLoginEnabled: user.pinLoginEnabled + })) + ).deep.include.members([ + { + username: 'js test 0', + pinLoginEnabled: true + } + ]) + + // Pin login should work on the duress account: const successAccount = await context.loginWithPIN(fakeUser.username, '9999') expect(successAccount.id).equals(duressAccount.id) }) + it('re-enable pin-login while in duress mode re-enables pin-login for the 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 }) + + // Re-enable PIN login: + await duressAccount.changePin({ enableLogin: true }) + + // Check the PIN login is enabled again: + expect( + context.localUsers.map(user => ({ + username: user.username, + pinLoginEnabled: user.pinLoginEnabled + })) + ).deep.include.members([ + { + username: 'js test 0', + pinLoginEnabled: true + } + ]) + + // Pin login should work on the duress account: + const successAccount = await context.loginWithPIN( + fakeUser.username, + fakeUser.pin + ) + 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)