Skip to content

Commit ca7ba9a

Browse files
authored
fix: selection count in toolbar and 2FA token on password change (#166)
* fix: selection count in toolbar and 2FA token on password change Closes #163: show "N selected" in the file-browser toolbar so users can keep track of how many items are in the current multi-selection. Closes #164: the change-password form prompts for a 2FA token but `changePassword()` in web/services/account.ts never forwarded it to the backend. With TFA enabled, the request always reached `verify_tfa(None)` and was rejected as `invalid_otp_token` — regardless of which credentials (current password or private key) the user supplied. Forward the token when present. E2E coverage: - web/e2e/change-password.spec.ts covers both the current-password and private-key flows against a TFA-enabled account. - web/e2e/files.spec.ts gets a Selection counter case that asserts the toolbar reflects check/uncheck transitions. * fix(fs): drop redundant into_iter() in S3 versioned copy `zip` already accepts `IntoIterator`, so the explicit `.into_iter()` on `dst_keys` is a useless conversion. clippy 1.95 enforces this; older toolchains silently accepted it. Caught when CI on Rust 1.95 failed the useless_conversion lint while a local 1.91 run did not. * fix(storage): use sort_by_key for pre-dedup file sort `sort_by(|a, b| a.id.cmp(&b.id))` is a textbook `sort_by_key` case; clippy 1.95 enforces `unnecessary_sort_by` where 1.91 didn't.
1 parent 92e0da3 commit ca7ba9a

6 files changed

Lines changed: 121 additions & 2 deletions

File tree

fs/src/providers/s3.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,7 @@ impl FsProviderContract for S3Provider {
515515
.collect();
516516

517517
let bucket = self.bucket.clone();
518-
futures::stream::iter(src_keys.into_iter().zip(dst_keys.into_iter()))
518+
futures::stream::iter(src_keys.into_iter().zip(dst_keys))
519519
.map(|(src_key, dst_key)| {
520520
let bucket = bucket.clone();
521521
async move {

storage/src/repository/manage.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ where
325325

326326
// Sort files by id and then run dedup_by to remove all duplicates,
327327
// without sorting, this won't remove all duplicates
328-
files.sort_by(|a, b| a.id.cmp(&b.id));
328+
files.sort_by_key(|a| a.id);
329329
files.dedup_by(|a, b| a.id == b.id);
330330

331331
let ids: Vec<Uuid> = files.iter().map(|f| f.id).collect();

web/e2e/change-password.spec.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
import { test, expect } from '@playwright/test'
2+
import { authenticator } from 'otplib'
3+
import {
4+
randomEmail,
5+
randomPassword,
6+
createUser,
7+
createUserWithTwoFactor,
8+
loginAsUser,
9+
loginWithTwoFactor,
10+
logout,
11+
} from './helpers/auth'
12+
13+
/**
14+
* Walk from the authenticated file browser to the change-password screen
15+
* via the in-app sidebar + "My account" link. A full `page.goto` would
16+
* reload the SPA and drop the in-memory keypair, bouncing the user back
17+
* to /auth/login.
18+
*/
19+
async function openChangePasswordViaSidebar(page: import('@playwright/test').Page) {
20+
await page.locator('aside').getByText('Account', { exact: true }).first().click()
21+
await page.waitForURL(/\/account(\/|$|\?)/)
22+
await page.getByRole('link', { name: 'Change', exact: true }).click()
23+
await page.waitForURL(/\/account\/change-password/)
24+
}
25+
26+
test.describe('Change password', () => {
27+
test('with no TFA, change password via current password succeeds', async ({ page }) => {
28+
const email = randomEmail()
29+
const password = randomPassword()
30+
await createUser(page, email, password)
31+
32+
await openChangePasswordViaSidebar(page)
33+
await page.locator('#current_password').fill(password)
34+
35+
const newPassword = randomPassword()
36+
await page.locator('#password').fill(newPassword)
37+
38+
const response = page.waitForResponse(
39+
(res) =>
40+
res.url().includes('/api/auth/account/change-password') &&
41+
res.request().method() === 'POST'
42+
)
43+
await page.getByRole('button', { name: 'Change password' }).click()
44+
expect((await response).status()).toBe(204)
45+
46+
await logout(page)
47+
await loginAsUser(page, email, newPassword)
48+
await expect(page).toHaveURL(/\/$/)
49+
})
50+
51+
// Regression test for https://github.com/hudikhq/hoodik/issues/164
52+
test('with TFA enabled, change password via private key + OTP succeeds', async ({ page }) => {
53+
const email = randomEmail()
54+
const password = randomPassword()
55+
const { privateKey, secret } = await createUserWithTwoFactor(page, email, password)
56+
57+
await openChangePasswordViaSidebar(page)
58+
await page.locator('#use_private_key').check()
59+
await page.locator('#private_key').fill(privateKey)
60+
await page.locator('#token').fill(authenticator.generate(secret))
61+
62+
const newPassword = randomPassword()
63+
await page.locator('#password').fill(newPassword)
64+
65+
const response = page.waitForResponse(
66+
(res) =>
67+
res.url().includes('/api/auth/account/change-password') &&
68+
res.request().method() === 'POST'
69+
)
70+
await page.getByRole('button', { name: 'Change password' }).click()
71+
72+
// The bug in #164 manifests as a 401 with body `invalid_otp_token`
73+
// because the frontend silently dropped the token before sending.
74+
// After the fix the backend returns 204.
75+
expect((await response).status()).toBe(204)
76+
77+
await logout(page)
78+
await loginWithTwoFactor(page, email, newPassword, secret)
79+
await expect(page).toHaveURL(/\/$/)
80+
})
81+
})

web/e2e/files.spec.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,34 @@ test.describe('Rename', () => {
9191
})
9292
})
9393

94+
test.describe('Selection counter', () => {
95+
test('shows the count of selected items in the toolbar and updates as selection changes', async ({ page }) => {
96+
await setup(page)
97+
98+
for (const name of ['Folder_A', 'Folder_B']) {
99+
await page.locator('[name="create-dir"]').click()
100+
await page.locator('#name').fill(name)
101+
await page.getByRole('button', { name: 'Create', exact: true }).click()
102+
await expect(page.getByTestId(`file-row-${name}`)).toBeVisible()
103+
}
104+
105+
const counter = page.getByTestId('files-selected-count')
106+
await expect(counter).toHaveCount(0)
107+
108+
await page.getByTestId('file-row-Folder_A').locator('input[type="checkbox"]').check()
109+
await expect(counter).toHaveText(/^1 selected$/)
110+
111+
await page.getByTestId('file-row-Folder_B').locator('input[type="checkbox"]').check()
112+
await expect(counter).toHaveText(/^2 selected$/)
113+
114+
await page.getByTestId('file-row-Folder_A').locator('input[type="checkbox"]').uncheck()
115+
await expect(counter).toHaveText(/^1 selected$/)
116+
117+
await page.getByTestId('file-row-Folder_B').locator('input[type="checkbox"]').uncheck()
118+
await expect(counter).toHaveCount(0)
119+
})
120+
})
121+
94122
test.describe('Delete', () => {
95123
test('can delete a file', async ({ page }) => {
96124
await setup(page)

web/services/account.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ export async function changePassword(payload: UnsecureChangePassword): Promise<v
2424
email: payload.email
2525
}
2626

27+
if (payload.token) {
28+
data.token = payload.token
29+
}
30+
2731
if (payload.current_password) {
2832
data.current_password = payload.current_password
2933
} else {

web/src/views/files/index-view/TableFiles.vue

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,12 @@ const sizes = {
175175
class="w-full p-2 mb-2 flex rounded-t-md bg-brownish-100 dark:bg-brownish-900 gap-4"
176176
v-if="showActions"
177177
>
178+
<span
179+
v-if="checkedRows.length"
180+
data-testid="files-selected-count"
181+
class="self-center text-sm text-brownish-700 dark:text-brownish-200"
182+
>{{ checkedRows.length }} selected</span>
183+
178184
<BaseButton
179185
title="Delete"
180186
:iconSize="20"

0 commit comments

Comments
 (0)