Skip to content

Sam/rest o duress #659

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
11 changes: 6 additions & 5 deletions src/core/context/context-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { checkPasswordRules, fixUsername } from '../../client-side'
import {
asChallengeErrorPayload,
asMaybePasswordError,
asMaybePinDisabledError,
EdgeAccount,
EdgeAccountOptions,
EdgeContext,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
9 changes: 7 additions & 2 deletions src/core/login/login-reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -50,7 +50,12 @@ export const login = buildReducer<LoginState, RootAction, RootState>({
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,
Expand Down
41 changes: 0 additions & 41 deletions src/core/login/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<LoginStash> {
const { stashTree } = getStashById(ai, kit.loginId)

const newStashTree = updateTree<LoginStash, LoginStash>(
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
Expand All @@ -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<LoginKit | undefined>
): Promise<void> {
for (const kit of kits) {
if (kit == null) continue
await applyKitTemporarily(ai, kit)
}
}

/**
* Refreshes a login with data from the server.
*/
Expand Down
48 changes: 33 additions & 15 deletions src/core/login/pin2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect! This function makes sense.

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.
Expand Down Expand Up @@ -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
}
Expand Down
29 changes: 25 additions & 4 deletions test/core/account/account.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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 }) => ({
Expand Down
Loading
Loading