-
Notifications
You must be signed in to change notification settings - Fork 1
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: Improve new password scenario in lockscreen
- Loading branch information
Showing
9 changed files
with
448 additions
and
29 deletions.
There are no files selected for viewing
60 changes: 60 additions & 0 deletions
60
src/app/domain/authorization/services/LockScreenScenarios.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
In `validatePassword` function, there are several functional scenarios or permutations to consider, based on different outcomes at each step of the process. Here's a breakdown of these scenarios: | ||
|
||
### 1. Successful Validation with Cached Password | ||
|
||
- **Scenario:** The cached password is still valid (server confirms this). | ||
You are right. In the scenario where the cached password is confirmed as valid by the server, the next step is to check the input password against this cached password. Here's the corrected scenario: | ||
- **Flow:** | ||
- The server does not throw an error when the cached password is checked, indicating it's still valid. | ||
- The function then compares the user's input password hash with the cached password hash. | ||
- If the input hash matches the cached hash, the function calls `onSuccess`. | ||
- If the input hash does not match the cached hash, the function calls `onFailure` with an incorrect password error. | ||
|
||
### 2. Successful Validation with User Input Password | ||
|
||
- **Scenario:** The cached password is outdated or invalid, but the user's input password is correct. | ||
- **Flow:** | ||
- The server throws an error (403) when the cached password is checked. | ||
- The function then checks the user input password against the server. | ||
- The server confirms the input password is correct. | ||
- The local cache is updated with the new password hash. | ||
- The function calls `onSuccess`. | ||
|
||
### 3. Invalid Cached Password and Invalid User Input | ||
|
||
- **Scenario:** The cached password is invalid, and the user's input password is also incorrect. | ||
- **Flow:** | ||
- The server throws an error (403) when the cached password is checked. | ||
- The function checks the user input password against the server. | ||
- The server throws an error (403) indicating the input password is also incorrect. | ||
- The function calls `onFailure` with a bad password error message. | ||
|
||
### 4. Unexpected Server Error on Cached Password Check | ||
|
||
- **Scenario:** An unexpected error occurs when checking the cached password (not a 403 error). | ||
- **Flow:** | ||
- The server throws an error (not 403) when the cached password is checked. | ||
- The function calls `onFailure` with a generic server error message. | ||
|
||
### 5. Unexpected Server Error on User Input Password Check | ||
|
||
- **Scenario:** The cached password is invalid, and an unexpected error occurs when checking the user input password. | ||
- **Flow:** | ||
- The server throws an error (403) when the cached password is checked. | ||
- An unexpected error occurs (not 403) when checking the user input password. | ||
- The function calls `onFailure` with a generic server error message. | ||
|
||
### 6. Error in the Success Handler | ||
|
||
- **Scenario:** The password validation is successful, but an error occurs in the `onSuccess` handler. | ||
- **Flow:** | ||
- The password validation (either cached or user input) is successful. | ||
- An error occurs during the execution of `onSuccess`. | ||
- The function calls `onFailure` with an unknown error message. | ||
|
||
### 7. Network or Communication Errors | ||
|
||
- **Scenario:** Network or communication issues occur during the server validation process. | ||
- **Flow:** | ||
- A network or communication error occurs when attempting to check the cached or input password against the server. | ||
- The function calls `onFailure` with a server error or a specific network error message. |
258 changes: 258 additions & 0 deletions
258
src/app/domain/authorization/services/LockScreenService.spec.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,258 @@ | ||
import CozyClient from 'cozy-client' | ||
|
||
import { validatePassword } from '/app/domain/authorization/services/LockScreenService' | ||
|
||
const mockGetVaultInformation = jest.fn() | ||
const mockFetchJSON = jest.fn() | ||
const mockDoHashPassword = jest.fn() | ||
const mockSaveVaultInformation = jest.fn() | ||
|
||
jest.mock('/libs/functions/passwordHelpers', () => ({ | ||
__esModule: true, | ||
doHashPassword: (): { passwordHash: string } => | ||
mockDoHashPassword() as { passwordHash: string } | ||
})) | ||
|
||
jest.mock('/libs/client', () => ({ | ||
__esModule: true, | ||
getInstanceAndFqdnFromClient: jest.fn().mockReturnValue({ | ||
fqdn: 'http://test.fqdn' | ||
}) | ||
})) | ||
|
||
jest.mock('/libs/keychain', () => ({ | ||
getVaultInformation: (): Promise<string> => | ||
Promise.resolve(mockGetVaultInformation() as unknown as string), | ||
removeVaultInformation: jest.fn(), | ||
saveVaultInformation: (...args: unknown[]): Promise<void> => { | ||
mockSaveVaultInformation(...(args as [string, string])) | ||
return Promise.resolve() | ||
} | ||
})) | ||
|
||
jest.mock('cozy-client', () => ({ | ||
__esModule: true, | ||
default: jest.fn().mockImplementation(() => ({ | ||
getStackClient: jest.fn().mockReturnValue({ | ||
fetchJSON: mockFetchJSON, | ||
uri: 'http://test.cozy.tools:8080' | ||
}) | ||
})) | ||
})) | ||
|
||
describe('validatePassword', () => { | ||
// Mocks and common setup | ||
let client: CozyClient | ||
let onSuccessMock: jest.Mock | ||
let onFailureMock: jest.Mock | ||
|
||
beforeEach(() => { | ||
client = new CozyClient() | ||
onSuccessMock = jest.fn() | ||
onFailureMock = jest.fn() | ||
}) | ||
|
||
it('should succeed when cached password is correct and input is correct', async () => { | ||
// Mock the server response for the KdfIterations | ||
mockFetchJSON.mockResolvedValueOnce({ KdfIterations: 1337 }) | ||
|
||
// Mock the local password cache hash value | ||
mockGetVaultInformation.mockResolvedValue('1337') | ||
|
||
// Mock the input password hash value | ||
mockDoHashPassword.mockResolvedValueOnce({ | ||
passwordHash: '1337' | ||
}) | ||
|
||
// Mock the server response for successful cached password check | ||
mockFetchJSON.mockResolvedValueOnce({}) | ||
|
||
// Call the function | ||
await validatePassword({ | ||
client, | ||
input: 'password', | ||
onSuccess: onSuccessMock, | ||
onFailure: onFailureMock | ||
}) | ||
|
||
// Assertions | ||
expect(onSuccessMock).toHaveBeenCalled() | ||
expect(mockSaveVaultInformation).not.toHaveBeenCalled() | ||
expect(onFailureMock).not.toHaveBeenCalled() | ||
}) | ||
|
||
it('should fail when cached password is correct but user input is not correct', async () => { | ||
// Mock the server response for the KdfIterations | ||
mockFetchJSON.mockResolvedValueOnce({ KdfIterations: 1337 }) | ||
|
||
// Mock the local password cache hash value | ||
mockGetVaultInformation.mockResolvedValue('1337') | ||
|
||
// Mock the input password hash value | ||
mockDoHashPassword.mockResolvedValueOnce({ | ||
passwordHash: '7331' | ||
}) | ||
|
||
// Mock the server response for successful cached password check | ||
mockFetchJSON.mockResolvedValueOnce({}) | ||
|
||
// Call the function | ||
await validatePassword({ | ||
client, | ||
input: 'password', | ||
onSuccess: onSuccessMock, | ||
onFailure: onFailureMock | ||
}) | ||
|
||
// Assertions | ||
expect(onSuccessMock).not.toHaveBeenCalled() | ||
expect(mockSaveVaultInformation).not.toHaveBeenCalled() | ||
expect(onFailureMock).toHaveBeenCalledWith('errors.badUnlockPassword') | ||
}) | ||
|
||
it('should succeed when cached password is correct but user input is not correct', async () => { | ||
// Mock the server response for the KdfIterations | ||
mockFetchJSON.mockResolvedValueOnce({ KdfIterations: 1337 }) | ||
|
||
// Mock the local password cache hash value | ||
mockGetVaultInformation.mockResolvedValue('1337') | ||
|
||
// Mock the input password hash value | ||
mockDoHashPassword.mockResolvedValueOnce({ | ||
passwordHash: '1337' | ||
}) | ||
|
||
// Mock the server response for a failed cached password check | ||
mockFetchJSON.mockRejectedValueOnce({ status: 403 }) | ||
|
||
// Mock the server response for a correct input password check | ||
mockFetchJSON.mockResolvedValueOnce({}) | ||
|
||
// Call the function | ||
await validatePassword({ | ||
client, | ||
input: 'password', | ||
onSuccess: onSuccessMock, | ||
onFailure: onFailureMock | ||
}) | ||
|
||
// Assertions | ||
expect(onSuccessMock).toHaveBeenCalled() | ||
expect(mockSaveVaultInformation).toHaveBeenCalledWith( | ||
'passwordHash', | ||
'1337' | ||
) | ||
expect(onFailureMock).not.toHaveBeenCalled() | ||
}) | ||
|
||
it('should fail when both cached and user input passwords are not correct', async () => { | ||
// Mock the server response for the KdfIterations | ||
mockFetchJSON.mockResolvedValueOnce({ KdfIterations: 1337 }) | ||
|
||
// Mock the local password cache hash value | ||
mockGetVaultInformation.mockResolvedValue('1337') | ||
|
||
// Mock the input password hash value | ||
mockDoHashPassword.mockResolvedValueOnce({ | ||
passwordHash: '1337' | ||
}) | ||
|
||
// Cached password check | ||
mockFetchJSON.mockRejectedValueOnce({ status: 403 }) | ||
|
||
// User input password check | ||
mockFetchJSON.mockRejectedValueOnce({ status: 403 }) | ||
|
||
// Call the function | ||
await validatePassword({ | ||
client, | ||
input: 'password', | ||
onSuccess: onSuccessMock, | ||
onFailure: onFailureMock | ||
}) | ||
|
||
// Assertions | ||
expect(onFailureMock).toHaveBeenCalledWith('errors.badUnlockPassword') | ||
expect(mockSaveVaultInformation).not.toHaveBeenCalled() | ||
expect(onSuccessMock).not.toHaveBeenCalled() | ||
}) | ||
|
||
it('should handle unexpected server error during cached password check', async () => { | ||
// Mock the server response for the KdfIterations | ||
mockFetchJSON.mockResolvedValueOnce({ KdfIterations: 1337 }) | ||
|
||
// Mock the local password cache hash value | ||
mockGetVaultInformation.mockResolvedValue('1337') | ||
|
||
// Mock the input password hash value | ||
mockDoHashPassword.mockResolvedValueOnce({ | ||
passwordHash: '1337' | ||
}) | ||
|
||
// Mock server response to throw a non-403 error | ||
mockFetchJSON.mockRejectedValueOnce({ status: 500 }) // Cached password check | ||
|
||
// Call the function | ||
await validatePassword({ | ||
client, | ||
input: 'password', | ||
onSuccess: onSuccessMock, | ||
onFailure: onFailureMock | ||
}) | ||
|
||
// Assertions | ||
expect(onFailureMock).toHaveBeenCalledWith('errors.serverError') | ||
expect(mockSaveVaultInformation).not.toHaveBeenCalled() | ||
expect(onSuccessMock).not.toHaveBeenCalled() | ||
}) | ||
|
||
it('should handle unexpected server error during input password check', async () => { | ||
// Mock the server response for the KdfIterations | ||
mockFetchJSON.mockResolvedValueOnce({ KdfIterations: 1337 }) | ||
|
||
// Mock the local password cache hash value | ||
mockGetVaultInformation.mockResolvedValue('1337') | ||
|
||
// Mock the input password hash value | ||
mockDoHashPassword.mockResolvedValueOnce({ | ||
passwordHash: '1337' | ||
}) | ||
|
||
// Mock the server response for a failed cached password check | ||
mockFetchJSON.mockRejectedValueOnce({ status: 403 }) | ||
|
||
// Mock the server response for a correct input password check | ||
mockFetchJSON.mockRejectedValueOnce({ status: 500 }) | ||
|
||
// Call the function | ||
await validatePassword({ | ||
client, | ||
input: 'password', | ||
onSuccess: onSuccessMock, | ||
onFailure: onFailureMock | ||
}) | ||
|
||
// Assertions | ||
expect(onFailureMock).toHaveBeenCalledWith('errors.serverError') | ||
expect(mockSaveVaultInformation).not.toHaveBeenCalled() | ||
expect(onSuccessMock).not.toHaveBeenCalled() | ||
}) | ||
|
||
it('should handle network or communication errors', async () => { | ||
// Every fetchJSON call will throw a network error | ||
mockFetchJSON.mockRejectedValue(new Error('Unexpected error')) | ||
|
||
// Call the function | ||
await validatePassword({ | ||
client, | ||
input: 'any-input', | ||
onSuccess: onSuccessMock, | ||
onFailure: onFailureMock | ||
}) | ||
|
||
// Assertions | ||
expect(onFailureMock).toHaveBeenCalledWith('errors.unknown_error') | ||
expect(mockSaveVaultInformation).not.toHaveBeenCalled() | ||
expect(onSuccessMock).not.toHaveBeenCalled() | ||
}) | ||
}) |
Oops, something went wrong.