chore: Add serverPassword param to endpoints (#2919) [skip e2e]

* chore: send server password param to delete account endpoint

* chore: send server password param to disable mfa endpoint

* chore: modify tests

* chore: force challenge prompt for mfa disable

* chore: fix eslint errors

* chore: add server passsword to get recovery codes

* chore: fix tests

* chore: pass server password as header
This commit is contained in:
Antonella Sgarlatta
2025-08-26 09:04:03 -03:00
committed by GitHub
parent cf4d2196de
commit 54af28aa04
29 changed files with 298 additions and 62 deletions

View File

@@ -1026,6 +1026,7 @@ export class Dependencies {
return new GetRecoveryCodes(
this.get<AuthManager>(TYPES.AuthManager),
this.get<SettingsService>(TYPES.SettingsService),
this.get<EncryptionService>(TYPES.EncryptionService),
)
})
@@ -1231,6 +1232,7 @@ export class Dependencies {
this.get<PureCryptoInterface>(TYPES.Crypto),
this.get<FeaturesService>(TYPES.FeaturesService),
this.get<ProtectionsClientInterface>(TYPES.ProtectionService),
this.get<EncryptionService>(TYPES.EncryptionService),
this.get<InternalEventBus>(TYPES.InternalEventBus),
)
})

View File

@@ -1,4 +1,4 @@
import { AuthClientInterface } from '@standardnotes/services'
import { AuthClientInterface, EncryptionService } from '@standardnotes/services'
import { SettingsClientInterface } from '@Lib/Services/Settings/SettingsClientInterface'
import { GetRecoveryCodes } from './GetRecoveryCodes'
@@ -6,8 +6,9 @@ import { GetRecoveryCodes } from './GetRecoveryCodes'
describe('GetRecoveryCodes', () => {
let authClient: AuthClientInterface
let settingsClient: SettingsClientInterface
let encryption: EncryptionService
const createUseCase = () => new GetRecoveryCodes(authClient, settingsClient)
const createUseCase = () => new GetRecoveryCodes(authClient, settingsClient, encryption)
beforeEach(() => {
authClient = {} as jest.Mocked<AuthClientInterface>
@@ -15,12 +16,16 @@ describe('GetRecoveryCodes', () => {
settingsClient = {} as jest.Mocked<SettingsClientInterface>
settingsClient.getSetting = jest.fn().mockResolvedValue('existing-recovery-codes')
encryption = {} as jest.Mocked<EncryptionService>
encryption.computeRootKey = jest.fn().mockResolvedValue({ serverPassword: 'test-server-password' })
encryption.getRootKeyParams = jest.fn().mockReturnValue({ algorithm: 'test-algorithm' })
})
it('should return existing recovery code if they exist', async () => {
const useCase = createUseCase()
const result = await useCase.execute()
const result = await useCase.execute({ password: 'test-password' })
expect(result.getValue()).toBe('existing-recovery-codes')
})
@@ -30,7 +35,7 @@ describe('GetRecoveryCodes', () => {
const useCase = createUseCase()
const result = await useCase.execute()
const result = await useCase.execute({ password: 'test-password' })
expect(result.getValue()).toBe('recovery-codes')
})
@@ -41,7 +46,7 @@ describe('GetRecoveryCodes', () => {
const useCase = createUseCase()
const result = await useCase.execute()
const result = await useCase.execute({ password: 'test-password' })
expect(result.isFailed()).toBe(true)
})

View File

@@ -1,23 +1,40 @@
import { AuthClientInterface } from '@standardnotes/services'
import { AuthClientInterface, EncryptionService } from '@standardnotes/services'
import { Result, SettingName, UseCaseInterface } from '@standardnotes/domain-core'
import { SettingsClientInterface } from '@Lib/Services/Settings/SettingsClientInterface'
import { GetRecoveryCodesDTO } from './GetRecoveryCodesDTO'
import { SNRootKeyParams } from '@standardnotes/encryption'
export class GetRecoveryCodes implements UseCaseInterface<string> {
constructor(
private authClient: AuthClientInterface,
private settingsClient: SettingsClientInterface,
private encryption: EncryptionService,
) {}
async execute(): Promise<Result<string>> {
async execute(dto: GetRecoveryCodesDTO): Promise<Result<string>> {
if (!dto.password) {
return Result.fail('Password is required to get recovery code.')
}
const currentRootKey = await this.encryption.computeRootKey(
dto.password,
this.encryption.getRootKeyParams() as SNRootKeyParams,
)
const serverPassword = currentRootKey.serverPassword
if (!serverPassword) {
return Result.fail('Could not compute server password')
}
const existingRecoveryCodes = await this.settingsClient.getSetting(
SettingName.create(SettingName.NAMES.RecoveryCodes).getValue(),
serverPassword,
)
if (existingRecoveryCodes !== undefined) {
return Result.ok(existingRecoveryCodes)
}
const generatedRecoveryCodes = await this.authClient.generateRecoveryCodes()
const generatedRecoveryCodes = await this.authClient.generateRecoveryCodes({ serverPassword })
if (generatedRecoveryCodes === false) {
return Result.fail('Could not generate recovery code')
}

View File

@@ -0,0 +1,3 @@
export interface GetRecoveryCodesDTO {
password?: string
}

View File

@@ -578,12 +578,18 @@ export class LegacyApiService
})
}
async getSetting(userUuid: UuidString, settingName: string): Promise<HttpResponse<GetSettingResponse>> {
async getSetting(
userUuid: UuidString,
settingName: string,
serverPassword?: string,
): Promise<HttpResponse<GetSettingResponse>> {
const customHeaders = serverPassword ? [{ key: 'x-server-password', value: serverPassword }] : undefined
return await this.tokenRefreshableRequest<GetSettingResponse>({
verb: HttpVerb.Get,
url: joinPaths(this.host, Paths.v1.setting(userUuid, settingName.toLowerCase())),
authentication: this.getSessionAccessToken(),
fallbackErrorMessage: API_MESSAGE_FAILED_GET_SETTINGS,
customHeaders,
})
}
@@ -616,12 +622,18 @@ export class LegacyApiService
})
}
async deleteSetting(userUuid: UuidString, settingName: string): Promise<HttpResponse<DeleteSettingResponse>> {
async deleteSetting(
userUuid: UuidString,
settingName: string,
serverPassword?: string,
): Promise<HttpResponse<DeleteSettingResponse>> {
const customHeaders = serverPassword ? [{ key: 'x-server-password', value: serverPassword }] : undefined
return this.tokenRefreshableRequest<DeleteSettingResponse>({
verb: HttpVerb.Delete,
url: joinPaths(this.host, Paths.v1.setting(userUuid, settingName)),
authentication: this.getSessionAccessToken(),
fallbackErrorMessage: API_MESSAGE_FAILED_UPDATE_SETTINGS,
customHeaders,
})
}

View File

@@ -6,9 +6,12 @@ import {
InternalEventBusInterface,
MfaServiceInterface,
ProtectionsClientInterface,
EncryptionService,
SignInStrings,
ChallengeValidation,
} from '@standardnotes/services'
import { SettingName } from '@standardnotes/domain-core'
import { SNRootKeyParams } from '@standardnotes/encryption'
export class MfaService extends AbstractService implements MfaServiceInterface {
constructor(
@@ -16,6 +19,7 @@ export class MfaService extends AbstractService implements MfaServiceInterface {
private crypto: PureCryptoInterface,
private featuresService: FeaturesService,
private protections: ProtectionsClientInterface,
private encryption: EncryptionService,
protected override internalEventBus: InternalEventBusInterface,
) {
super(internalEventBus)
@@ -55,11 +59,23 @@ export class MfaService extends AbstractService implements MfaServiceInterface {
}
async disableMfa(): Promise<void> {
if (!(await this.protections.authorizeMfaDisable())) {
const { success, challengeResponse } = await this.protections.authorizeMfaDisable()
if (!success) {
return
}
return await this.settingsService.deleteSetting(SettingName.create(SettingName.NAMES.MfaSecret).getValue())
const password = challengeResponse?.getValueForType(ChallengeValidation.AccountPassword).value as string
const currentRootKey = await this.encryption.computeRootKey(
password,
this.encryption.getRootKeyParams() as SNRootKeyParams,
)
const serverPassword = currentRootKey.serverPassword
return await this.settingsService.deleteSetting(
SettingName.create(SettingName.NAMES.MfaSecret).getValue(),
serverPassword,
)
}
override deinit(): void {

View File

@@ -34,6 +34,7 @@ import {
import { ContentType } from '@standardnotes/domain-core'
import { isValidProtectionSessionLength } from './isValidProtectionSessionLength'
import { UnprotectedAccessSecondsDuration } from './UnprotectedAccessSecondsDuration'
import { ChallengeResponse } from '../Challenge'
/**
* Enforces certain actions to require extra authentication,
@@ -246,11 +247,11 @@ export class ProtectionService
})
}
async authorizeMfaDisable(): Promise<boolean> {
return this.authorizeAction(ChallengeReason.DisableMfa, {
async authorizeMfaDisable(): Promise<{ success: boolean; challengeResponse?: ChallengeResponse }> {
return this.authorizeActionWithChallengeResponse(ChallengeReason.DisableMfa, {
fallBackToAccountPassword: true,
requireAccountPassword: true,
forcePrompt: false,
forcePrompt: true,
})
}
@@ -278,6 +279,14 @@ export class ProtectionService
})
}
async authorizeAccountDeletion(): Promise<{ success: boolean; challengeResponse?: ChallengeResponse }> {
return this.authorizeActionWithChallengeResponse(ChallengeReason.DeleteAccount, {
fallBackToAccountPassword: true,
requireAccountPassword: true,
forcePrompt: true,
})
}
async authorizeAction(
reason: ChallengeReason,
dto: { fallBackToAccountPassword: boolean; requireAccountPassword: boolean; forcePrompt: boolean },
@@ -285,6 +294,13 @@ export class ProtectionService
return this.validateOrRenewSession(reason, dto)
}
async authorizeActionWithChallengeResponse(
reason: ChallengeReason,
dto: { fallBackToAccountPassword: boolean; requireAccountPassword: boolean; forcePrompt: boolean },
): Promise<{ success: boolean; challengeResponse?: ChallengeResponse }> {
return this.validateOrRenewSessionWithChallengeResponse(reason, dto)
}
getMobilePasscodeTimingOptions(): TimingDisplayOption[] {
return [
{
@@ -353,8 +369,20 @@ export class ProtectionService
reason: ChallengeReason,
{ fallBackToAccountPassword = true, requireAccountPassword = false, forcePrompt = false } = {},
): Promise<boolean> {
const response = await this.validateOrRenewSessionWithChallengeResponse(reason, {
fallBackToAccountPassword,
requireAccountPassword,
forcePrompt,
})
return response.success
}
private async validateOrRenewSessionWithChallengeResponse(
reason: ChallengeReason,
{ fallBackToAccountPassword = true, requireAccountPassword = false, forcePrompt = false } = {},
): Promise<{ success: boolean; challengeResponse?: ChallengeResponse }> {
if (this.getSessionExpiryDate() > new Date() && !forcePrompt) {
return true
return { success: true }
}
const prompts: ChallengePrompt[] = []
@@ -378,9 +406,10 @@ export class ProtectionService
if (fallBackToAccountPassword && this.encryption.hasAccount()) {
prompts.push(new ChallengePrompt(ChallengeValidation.AccountPassword))
} else {
return true
return { success: true }
}
}
const lastSessionLength = this.getLastSessionLength()
const chosenSessionLength = isValidProtectionSessionLength(lastSessionLength)
? lastSessionLength
@@ -407,9 +436,9 @@ export class ProtectionService
} else {
this.setSessionLength(length as UnprotectedAccessSecondsDuration)
}
return true
return { success: true, challengeResponse: response }
} else {
return false
return { success: false }
}
}

View File

@@ -30,8 +30,8 @@ export class SettingsService extends AbstractService implements SettingsClientIn
return this.provider.listSettings()
}
async getSetting(name: SettingName) {
return this.provider.getSetting(name)
async getSetting(name: SettingName, serverPassword?: string) {
return this.provider.getSetting(name, serverPassword)
}
async getSubscriptionSetting(name: SettingName) {
@@ -50,8 +50,8 @@ export class SettingsService extends AbstractService implements SettingsClientIn
return this.provider.getDoesSensitiveSettingExist(name)
}
async deleteSetting(name: SettingName) {
return this.provider.deleteSetting(name)
async deleteSetting(name: SettingName, serverPassword?: string) {
return this.provider.deleteSetting(name, serverPassword)
}
getEmailBackupFrequencyOptionLabel(frequency: EmailBackupFrequency): string {

View File

@@ -5,13 +5,13 @@ import { SettingName } from '@standardnotes/domain-core'
export interface SettingsClientInterface {
listSettings(): Promise<SettingsList>
getSetting(name: SettingName): Promise<string | undefined>
getSetting(name: SettingName, serverPassword?: string): Promise<string | undefined>
getDoesSensitiveSettingExist(name: SettingName): Promise<boolean>
updateSetting(name: SettingName, payload: string, sensitive?: boolean): Promise<void>
deleteSetting(name: SettingName): Promise<void>
deleteSetting(name: SettingName, serverPassword?: string): Promise<void>
getEmailBackupFrequencyOptionLabel(frequency: EmailBackupFrequency): string
}

View File

@@ -45,8 +45,8 @@ export class SettingsGateway {
return settings
}
async getSetting(name: SettingName): Promise<string | undefined> {
const response = await this.settingsApi.getSetting(this.userUuid, name.value)
async getSetting(name: SettingName, serverPassword?: string): Promise<string | undefined> {
const response = await this.settingsApi.getSetting(this.userUuid, name.value, serverPassword)
if (response.status === HttpStatusCode.BadRequest) {
return undefined
@@ -109,8 +109,8 @@ export class SettingsGateway {
}
}
async deleteSetting(name: SettingName): Promise<void> {
const response = await this.settingsApi.deleteSetting(this.userUuid, name.value)
async deleteSetting(name: SettingName, serverPassword?: string): Promise<void> {
const response = await this.settingsApi.deleteSetting(this.userUuid, name.value, serverPassword)
if (isErrorResponse(response)) {
throw new Error(getErrorFromErrorResponse(response).message)
}

View File

@@ -17,7 +17,11 @@ export interface SettingsServerInterface {
sensitive: boolean,
): Promise<HttpResponse<UpdateSettingResponse>>
getSetting(userUuid: UuidString, settingName: string): Promise<HttpResponse<GetSettingResponse>>
getSetting(
userUuid: UuidString,
settingName: string,
serverPassword?: string,
): Promise<HttpResponse<GetSettingResponse>>
getSubscriptionSetting(userUuid: UuidString, settingName: string): Promise<HttpResponse<GetSettingResponse>>
@@ -28,5 +32,9 @@ export interface SettingsServerInterface {
sensitive: boolean,
): Promise<HttpResponse<UpdateSettingResponse>>
deleteSetting(userUuid: UuidString, settingName: string): Promise<HttpResponse<DeleteSettingResponse>>
deleteSetting(
userUuid: UuidString,
settingName: string,
serverPassword?: string,
): Promise<HttpResponse<DeleteSettingResponse>>
}

View File

@@ -549,6 +549,24 @@ describe('basic auth', function () {
expect(sendChallengeSpy.callCount).to.equal(1)
}).timeout(Factory.TenSecondTimeout)
it('should send server password when deleting account', async function () {
Factory.handlePasswordChallenges(context.application, context.password)
const userApiService = context.application.dependencies.get(TYPES.UserApiService)
const deleteAccountSpy = sinon.spy(userApiService, 'deleteAccount')
await context.application.user.deleteAccount()
expect(deleteAccountSpy.callCount).to.equal(1)
const deleteAccountCall = deleteAccountSpy.getCall(0)
const callArgs = deleteAccountCall.args[0]
expect(callArgs).to.have.property('serverPassword')
expect(callArgs.serverPassword).to.not.be.undefined
expect(typeof callArgs.serverPassword).to.equal('string')
expect(callArgs.serverPassword.length).to.be.above(0)
}).timeout(Factory.TenSecondTimeout)
it('deleting account should sign out current user', async function () {
Factory.handlePasswordChallenges(context.application, context.password)
@@ -567,12 +585,40 @@ describe('basic auth', function () {
const response = await context.application.dependencies
.get(TYPES.UserApiService)
.deleteAccount(registerResponse.user.uuid)
.deleteAccount({
userUuid: registerResponse.user.uuid,
serverPassword: 'dummy-password'
})
expect(response.status).to.equal(401)
expect(response.data.error.message).to.equal('Operation not allowed.')
await secondContext.deinit()
})
it('should not allow deleting account if server password is not sent', async function () {
Factory.handlePasswordChallenges(context.application, context.password)
const response = await context.application.dependencies
.get(TYPES.UserApiService)
.deleteAccount({
userUuid: context.application.user.uuid,
})
expect(response.status).to.equal(400)
}).timeout(Factory.TenSecondTimeout)
it('should not allow deleting account if server password is incorrect', async function () {
Factory.handlePasswordChallenges(context.application, context.password)
const response = await context.application.dependencies
.get(TYPES.UserApiService)
.deleteAccount({
userUuid: context.application.user.uuid,
serverPassword: 'wrong-password'
})
expect(response.status).to.equal(400)
}).timeout(Factory.TenSecondTimeout)
})
})

View File

@@ -65,6 +65,7 @@ describe('mfa service', () => {
const token = await application.mfa.getOtpToken(secret)
sinon.spy(application.challenges, 'sendChallenge')
await application.mfa.enableMfa(secret, token)
await application.mfa.disableMfa()
@@ -73,4 +74,64 @@ describe('mfa service', () => {
expect(challenge.prompts).to.have.lengthOf(2)
expect(challenge.prompts[0].validation).to.equal(ChallengeValidation.AccountPassword)
}).timeout(Factory.TenSecondTimeout)
it('sends server password when disabling mfa', async () => {
await registerApp(application)
Factory.handlePasswordChallenges(application, accountPassword)
const secret = await application.mfa.generateMfaSecret()
const token = await application.mfa.getOtpToken(secret)
await application.mfa.enableMfa(secret, token)
sinon.spy(application.settings.settingsApi, 'deleteSetting')
await application.mfa.disableMfa()
const deleteSettingCall = application.settings.settingsApi.deleteSetting.getCall(0)
const [serverPassword] = deleteSettingCall.args
expect(typeof serverPassword).to.equal('string')
expect(serverPassword.length).to.be.above(0)
}).timeout(Factory.TenSecondTimeout)
it('should not allow disabling mfa if server password is not sent', async function () {
await registerApp(application)
Factory.handlePasswordChallenges(application, accountPassword)
const secret = await application.mfa.generateMfaSecret()
const token = await application.mfa.getOtpToken(secret)
await application.mfa.enableMfa(secret, token)
const response = await application.dependencies
.get(TYPES.SettingsApiService)
.deleteSetting({
userUuid: application.user.uuid,
settingName: 'MFA_SECRET',
})
expect(response.status).to.equal(400)
}).timeout(Factory.TenSecondTimeout)
it('should not allow disabling mfa if server password is incorrect', async function () {
await registerApp(application)
Factory.handlePasswordChallenges(application, accountPassword)
const secret = await application.mfa.generateMfaSecret()
const token = await application.mfa.getOtpToken(secret)
await application.mfa.enableMfa(secret, token)
const response = await application.dependencies
.get(TYPES.SettingsApiService)
.deleteSetting({
userUuid: application.user.uuid,
settingName: 'MFA_SECRET',
serverPassword: 'wrong-password'
})
expect(response.status).to.equal(400)
}).timeout(Factory.TenSecondTimeout)
})