diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f144439..62387979 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## 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) - 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..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, @@ -214,6 +213,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 @@ -322,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 8b0b7774..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) @@ -537,6 +708,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)