**COLLECTING BRANCH** Account management: passkeys, password change, account deletion #68

Manually merged
marvin8 merged 18 commits from feat/issue-67-account-management into feat/multiuser 2026-10-06 03:35:17 +00:00
Collaborator

Effort 3 of Phase 6 multi-user (final effort before the collecting branch lands on main). Implements the approved account-management design: CLI-only account creation (yunjin create-user, no admin role), self-service password change (current-password gate; current session survives, others revoked), passkey enrollment + username-less login via webauthn 3.0.1 (additive to passwords, ~12 h passkey sessions vs 30-day password sessions), credential management on a new Account card, and self-service account deletion with data cleanup (FK cascades + orphan-feed GC; last-user guard in web, CLI delete-user deliberately unrestricted). One Alembic migration (20261004_webauthn_credentials). Passkey features gate on new env vars YUNJIN_RP_ID + YUNJIN_ORIGIN; documented in docs/config.md, docs/cli.md, new docs/passkeys.md (HTTPS-on-stable-domain prerequisite).

Also closes a latent setup-gate gap this branch made reachable: POST /setup now enforces the recorded per-request gate (redirect once any user is credentialed), and _ensure_default_user no longer resurrects a passwordless default while other users survive.

Base: feat/multiuser (collecting branch).

Refs #25
Refs #67

Test suite: 622 passed (617 + fix-wave tests); full nox green on CI.

Effort 3 of Phase 6 multi-user (final effort before the collecting branch lands on `main`). Implements the approved account-management design: CLI-only account creation (`yunjin create-user`, no admin role), self-service password change (current-password gate; current session survives, others revoked), passkey enrollment + username-less login via `webauthn` 3.0.1 (additive to passwords, ~12 h passkey sessions vs 30-day password sessions), credential management on a new Account card, and self-service account deletion with data cleanup (FK cascades + orphan-feed GC; last-user guard in web, CLI `delete-user` deliberately unrestricted). One Alembic migration (`20261004_webauthn_credentials`). Passkey features gate on new env vars `YUNJIN_RP_ID` + `YUNJIN_ORIGIN`; documented in `docs/config.md`, `docs/cli.md`, new `docs/passkeys.md` (HTTPS-on-stable-domain prerequisite). Also closes a latent setup-gate gap this branch made reachable: `POST /setup` now enforces the recorded per-request gate (redirect once any user is credentialed), and `_ensure_default_user` no longer resurrects a passwordless `default` while other users survive. **Base:** `feat/multiuser` (collecting branch). Refs #25 Refs #67 Test suite: 622 passed (617 + fix-wave tests); full nox green on CI.
🐛 Close setup-gate bypass and harden registration errors
All checks were successful
/ gitleaks (pull_request) Successful in 27s
/ pr-review (pull_request) Successful in 1m22s
/ checks (pull_request) Successful in 21m11s
/ publish-container (pull_request) Has been skipped
/ deploy-docs (pull_request) Has been skipped
/ container (pull_request) Successful in 1m36s
5ee6dc5ea4
POST /setup honored the setup gate only on GET, so a passwordless
leftover account was claimable by an anonymous visitor once any other
user was credentialed. The POST branch now checks setup_required first,
and _ensure_default_user seeds the passwordless default user only when
no users exist at all — deleting user id 1 while other users survive no
longer resurrects a claimable account, while fresh installs and full
teardowns still re-seed default and reopen setup.

Registration verify now maps the UNIQUE-constraint sqlite3.IntegrityError
from a repeated credential id to the generic 400 instead of a 500, and
finish_registration stores transports only when the client JSON carries a
genuine list.
forgejo-actions left a comment

WuMing

Found 2 issue(s). See inline comments below.

## WuMing Found **2** issue(s). See inline comments below.
.gitleaks.toml Outdated
@ -9,0 +10,4 @@
[[allowlists]]
description = "Canned WebAuthn test vectors copied verbatim from the public webauthn 3.0.1 sdist test suite; not secrets"
paths = [
'''(?:^|/)tests/webauthn_fixtures\.py$''',

config [MEDIUM]

This allowlist suppresses ALL gitleaks rules for the entire tests/webauthn_fixtures.py file, not just the canned WebAuthn vectors. Any genuine credential later added to (or accidentally pasted into) that file will never be detected, silently widening the secret-scanning blind spot. Prefer scoping the exception to the known values instead of the whole path, e.g. use regexes = [...] that match the specific fixture keys/vectors (optionally with regexTarget = "match"), or mark only those lines in the test file with #gitleaks:allow.

**config** [MEDIUM] This allowlist suppresses ALL gitleaks rules for the entire tests/webauthn_fixtures.py file, not just the canned WebAuthn vectors. Any genuine credential later added to (or accidentally pasted into) that file will never be detected, silently widening the secret-scanning blind spot. Prefer scoping the exception to the known values instead of the whole path, e.g. use `regexes = [...]` that match the specific fixture keys/vectors (optionally with `regexTarget = "match"`), or mark only those lines in the test file with `#gitleaks:allow`. <!-- wuming:sha256:95d74db6fd0f8e036b2c92d84ca3ad5b472395d9b09a6f4c81a72c69e59b5be5 -->
@ -0,0 +53,4 @@
" VALUES (1, X'00', X'00', 0, 'Passkey again')"
)
raised = False
except Exception:

tests [MEDIUM]

This uniqueness test wraps the second INSERT in try/except Exception and asserts a boolean flag, so it passes if the statement fails for ANY reason (missing column, FK problem, wrong table, etc.), not just the intended credential_id UNIQUE constraint. It also reports nothing about which constraint fired, making future regressions hard to diagnose. Replace the flag pattern with a specific expectation, e.g. with pytest.raises(sqlite3.IntegrityError): conn.execute(...), so only the expected violation satisfies the test.

**tests** [MEDIUM] This uniqueness test wraps the second INSERT in `try/except Exception` and asserts a boolean flag, so it passes if the statement fails for ANY reason (missing column, FK problem, wrong table, etc.), not just the intended `credential_id` UNIQUE constraint. It also reports nothing about which constraint fired, making future regressions hard to diagnose. Replace the flag pattern with a specific expectation, e.g. `with pytest.raises(sqlite3.IntegrityError): conn.execute(...)`, so only the expected violation satisfies the test. <!-- wuming:sha256:bbbfe78e0572898c97e0fd5b98a5f9e5d5fe630e5050aa5f6acf07297e660f31 -->
✅ Expect IntegrityError in credential-id uniqueness test
All checks were successful
/ gitleaks (pull_request) Successful in 25s
/ pr-review (pull_request) Successful in 1m16s
/ checks (pull_request) Successful in 21m1s
/ publish-container (pull_request) Has been skipped
/ deploy-docs (pull_request) Has been skipped
/ container (pull_request) Successful in 3m27s
f05ef6a65b
Author
Collaborator

.gitleaks.toml

This allowlist suppresses ALL gitleaks rules for the entire tests/webauthn_fixtures.py file, not just the canned WebAuthn vectors.

✅ Fixed in commit bbc03b6 — the allowlist no longer uses paths at all. It is now a single line-exact regexes entry (regexTarget = "line") matching only the one public sdist vector line that actually trips the scanner, so any other finding in that file is still detected. Verified empirically with the gitleaks 8.30.1 binary: fixture-only scan → no leaks; planted GitHub PAT in a copy of the same file → 1 finding (detected); same file with an empty config → 2 findings (both would trip without the allowlist). An inline #gitleaks:allow was not possible: the flagged line lives inside a JSON string constant that tests compare byte-for-byte.

[`.gitleaks.toml`](https://forge.marvin8.zone/marvin8/yunjin/pulls/68#issuecomment-4267) > This allowlist suppresses ALL gitleaks rules for the entire tests/webauthn_fixtures.py file, not just the canned WebAuthn vectors. ✅ Fixed in commit `bbc03b6` — the allowlist no longer uses `paths` at all. It is now a single line-exact `regexes` entry (`regexTarget = "line"`) matching only the one public sdist vector line that actually trips the scanner, so any other finding in that file is still detected. Verified empirically with the gitleaks 8.30.1 binary: fixture-only scan → no leaks; planted GitHub PAT in a copy of the same file → 1 finding (detected); same file with an empty config → 2 findings (both would trip without the allowlist). An inline `#gitleaks:allow` was not possible: the flagged line lives inside a JSON string constant that tests compare byte-for-byte.
Author
Collaborator

tests/test_migration_webauthn.py

This uniqueness test wraps the second INSERT in try/except Exception and asserts a boolean flag, so it passes if the statement fails for ANY reason

✅ Fixed in commit f05ef6a — the flag pattern is replaced with with pytest.raises(sqlite3.IntegrityError): so only the intended UNIQUE-constraint violation satisfies the test.

[`tests/test_migration_webauthn.py`](https://forge.marvin8.zone/marvin8/yunjin/pulls/68#issuecomment-4268) > This uniqueness test wraps the second INSERT in try/except Exception and asserts a boolean flag, so it passes if the statement fails for ANY reason ✅ Fixed in commit `f05ef6a` — the flag pattern is replaced with `with pytest.raises(sqlite3.IntegrityError):` so only the intended UNIQUE-constraint violation satisfies the test.
forgejo-actions left a comment

WuMing

Found 1 issue(s). See inline comments below.

## WuMing Found **1** issue(s). See inline comments below.
@ -54,0 +86,4 @@
```
Answering anything but `y` or `yes` aborts with no changes. A deletion removes
the account's subscriptions, aggregates, read state, settings, and passkeys;

docs [LOW]

Inconsistent terminology between the two docs describing the same deletion. delete-user says it removes the account's "subscriptions, aggregates, read state, settings, and passkeys", while the new docs/passkeys.md (line 91) describes the same web-UI deletion as removing "subscriptions, groups, read state, settings, and passkeys". If "aggregates" and "groups" are the same concept, pick one term and use it in both places; if they are different concepts, each list is incomplete and should be reconciled so readers know exactly what data is removed.

**docs** [LOW] Inconsistent terminology between the two docs describing the same deletion. `delete-user` says it removes the account's "subscriptions, aggregates, read state, settings, and passkeys", while the new docs/passkeys.md (line 91) describes the same web-UI deletion as removing "subscriptions, groups, read state, settings, and passkeys". If "aggregates" and "groups" are the same concept, pick one term and use it in both places; if they are different concepts, each list is incomplete and should be reconciled so readers know exactly what data is removed. <!-- wuming:sha256:21bc6817e396a8f92210f2698b0798d058f0d0b5f13b54bf43b90edb0ff7ae28 -->
marvin8 marked this conversation as resolved
📝 Say aggregates, not groups, in the passkey deletion docs
All checks were successful
/ gitleaks (pull_request) Successful in 23s
/ pr-review (pull_request) Successful in 1m22s
/ checks (pull_request) Successful in 21m38s
/ publish-container (pull_request) Has been skipped
/ deploy-docs (pull_request) Has been skipped
/ container (pull_request) Successful in 1m39s
f94e6ca018
Author
Collaborator

docs/passkeys.md line 91

docs [LOW] Inconsistent terminology between the two docs describing the same deletion...

✅ Fixed in commit f94e6ca — passkeys.md now says "aggregates", matching docs/cli.md. ("Groups" was stale wording; the domain concept is aggregates throughout.)

[`docs/passkeys.md` line 91](https://forge.marvin8.zone/marvin8/yunjin/pulls/68#issuecomment-4361) > **docs** [LOW] Inconsistent terminology between the two docs describing the same deletion... ✅ Fixed in commit `f94e6ca` — passkeys.md now says "aggregates", matching docs/cli.md. ("Groups" was stale wording; the domain concept is aggregates throughout.)
forgejo-actions left a comment

WuMing

Found 4 issue(s). See inline comments below.

## WuMing Found **4** issue(s). See inline comments below.
@ -9,0 +9,4 @@
[[allowlists]]
description = "Canned WebAuthn assertion vector copied verbatim from the public webauthn 3.0.1 sdist test suite; not a secret. Line-exact: any other finding in this file still trips."
regexTarget = "line"

config [MEDIUM]

This second allowlist entry has no paths restriction and its regex is not anchored, so it is not "line-exact" as the description claims: gitleaks applies allowlist regexes as unanchored substring matches against the target, and an entry without paths applies repo-wide. A real secret that happens to sit on a line containing this substring (or a different file that ever embeds this vector) would be silently suppressed. Scope the exception to where the vendored test vector lives and anchor the pattern, e.g. add paths = ['''tests/data/webauthn_assertion.json'''] and use ^\s*'''"authenticatorData": "SZYN...Tg",'''$ so any other finding on that line still trips.

**config** [MEDIUM] This second allowlist entry has no `paths` restriction and its regex is not anchored, so it is not "line-exact" as the description claims: gitleaks applies allowlist `regexes` as unanchored substring matches against the target, and an entry without `paths` applies repo-wide. A real secret that happens to sit on a line containing this substring (or a different file that ever embeds this vector) would be silently suppressed. Scope the exception to where the vendored test vector lives and anchor the pattern, e.g. add `paths = ['''tests/data/webauthn_assertion.json''']` and use `^\s*'''"authenticatorData": "SZYN...Tg",'''$` so any other finding on that line still trips. <!-- wuming:sha256:aeb43d1f73049ac1f35f6ebfa2f9cc4b2693983902799ee93a3c2319d409385a -->
marvin8 marked this conversation as resolved
README.md Outdated
@ -31,1 +31,4 @@
keyboard shortcuts, next-unread navigation
- **Passkeys** — optional WebAuthn sign-in with self-service enrollment and
credential management (passwords remain the fallback)
- **Account management** — password change, self-service account deletion,

docs [LOW]

This bullet advertises self-service password change, but no documentation in this diff explains it. The only page describing the Settings → Account card (docs/passkeys.md, lines 54–95) covers passkey enrollment and account deletion, and docs/cli.md only covers create-user/delete-user; nowhere is it stated how to change a password, that the current password is required, or that the current session survives while other sessions are revoked. Add a short "Changing your password" section where the Account card is documented and link this bullet to it.

**docs** [LOW] This bullet advertises self-service password change, but no documentation in this diff explains it. The only page describing the Settings → Account card (docs/passkeys.md, lines 54–95) covers passkey enrollment and account deletion, and docs/cli.md only covers `create-user`/`delete-user`; nowhere is it stated how to change a password, that the current password is required, or that the current session survives while other sessions are revoked. Add a short "Changing your password" section where the Account card is documented and link this bullet to it. <!-- wuming:sha256:e94727db94581255825d4a18dd558aac77575dbee5016a8f17611199e7d41dfb -->
docs/cli.md Outdated
@ -53,1 +53,4 @@
### `create-user`
Create a new account with a password.

docs [LOW]

The section never states that this command is the only way to create an account. Since there is no web sign-up and no admin role, an administrator reading the docs may otherwise hunt for a registration page or an add-user-style flow in the UI. Add one sentence, e.g. "Accounts can only be created from the command line; the web interface has no sign-up page and there is no admin role."

**docs** [LOW] The section never states that this command is the *only* way to create an account. Since there is no web sign-up and no admin role, an administrator reading the docs may otherwise hunt for a registration page or an `add-user`-style flow in the UI. Add one sentence, e.g. "Accounts can only be created from the command line; the web interface has no sign-up page and there is no admin role." <!-- wuming:sha256:af36715b0bb9d389befa587e4b55026b38988551120330f13a062e25665e8f98 -->
docs/passkeys.md Outdated
@ -0,0 +85,4 @@
deleting a passkey in Yunjin does not remove the credential from the device's
passkey provider; remove it there too if you no longer want it on the device.
## Account deletion

docs [MEDIUM]

Account deletion is not a passkey feature, yet it is documented only on this page. The README advertises "self-service account deletion" as a headline capability (README line 34), and docs/cli.md documents the matching delete-user command, so a reader looking for the web deletion flow will never think to look under "Passkeys". Move this section to a general account/settings page (e.g. a new docs/account.md, or the settings section of docs/reader.md), and cross-link it from the README bullet and from docs/cli.md's delete-user section (which currently does not link back).

**docs** [MEDIUM] Account deletion is not a passkey feature, yet it is documented only on this page. The README advertises "self-service account deletion" as a headline capability (README line 34), and docs/cli.md documents the matching `delete-user` command, so a reader looking for the web deletion flow will never think to look under "Passkeys". Move this section to a general account/settings page (e.g. a new docs/account.md, or the settings section of docs/reader.md), and cross-link it from the README bullet and from docs/cli.md's `delete-user` section (which currently does not link back). <!-- wuming:sha256:fa0e62e8857621108755c83227e423ab0b663d59679ff20d8172195194ba59df -->
📝 Anchor the gitleaks vector regex and round out account docs
Some checks failed
/ gitleaks (pull_request) Successful in 20s
/ pr-review (pull_request) Successful in 1m20s
/ container (pull_request) Has been cancelled
/ publish-container (pull_request) Has been cancelled
/ deploy-docs (pull_request) Has been cancelled
/ checks (pull_request) Has been cancelled
e1a8fb2068
Author
Collaborator

.gitleaks.toml

config [MEDIUM] This second allowlist entry has no paths restriction and its regex is not anchored...

✅ Partially — commit e1a8fb2 anchors the pattern (^\s*...$), making it literally line-exact. The repo-wide claim, though, was already disproven empirically during wave 1 (comment 4271): a GitHub PAT planted in the same file is detected (1 finding) because regexTarget = "line" evaluates the allowlist regex per line — the same-file negative control and the suggested tests/data/webauthn_assertion.json layout (which does not exist in this repo) were re-checked before anchoring. Note paths+regexes in one entry was the wave-1 bug: gitleaks ORs the conditions within a single [[allowlists]] block, so a path-scoped entry re-widens the exemption to the whole file.

[`.gitleaks.toml`](https://forge.marvin8.zone/marvin8/yunjin/pulls/68#issuecomment-4365) > **config** [MEDIUM] This second allowlist entry has no `paths` restriction and its regex is not anchored... ✅ Partially — commit `e1a8fb2` anchors the pattern (`^\s*...$`), making it literally line-exact. The repo-wide claim, though, was already disproven empirically during wave 1 (comment 4271): a GitHub PAT planted in the same file is detected (1 finding) because `regexTarget = "line"` evaluates the allowlist regex per line — the same-file negative control and the suggested `tests/data/webauthn_assertion.json` layout (which does not exist in this repo) were re-checked before anchoring. Note `paths`+`regexes` in one entry was the wave-1 bug: gitleaks ORs the conditions within a single `[[allowlists]]` block, so a path-scoped entry re-widens the exemption to the whole file.
Author
Collaborator

docs/passkeys.md line 91

docs [MEDIUM] Account deletion is not a passkey feature, yet it is documented only on this page...

🔴 Declined for now — restructuring the docs (new docs/account.md, cross-links from README/cli.md) is an information-architecture decision for Marvin8, not a review-wave fix. The current placement is deliberate: the approved implementation plan pins the deletion documentation inside docs/passkeys.md (Task 9's content list), alongside the passkey-management and password-change sections of the same Settings → Account card. Raised with Marvin8; happy to do the move if he wants it.

[`docs/passkeys.md` line 91](https://forge.marvin8.zone/marvin8/yunjin/pulls/68#issuecomment-4366) > **docs** [MEDIUM] Account deletion is not a passkey feature, yet it is documented only on this page... 🔴 Declined for now — restructuring the docs (new `docs/account.md`, cross-links from README/cli.md) is an information-architecture decision for Marvin8, not a review-wave fix. The current placement is deliberate: the approved implementation plan pins the deletion documentation inside `docs/passkeys.md` (Task 9's content list), alongside the passkey-management and password-change sections of the same Settings → Account card. Raised with Marvin8; happy to do the move if he wants it.
Author
Collaborator

README.md

docs [LOW] This bullet advertises self-service password change, but no documentation in this diff explains it...

✅ Fixed in commit e1a8fb2 — passkeys.md (the page documenting the Settings → Account card) gains a "Changing your password" section: current password required, current session survives, other sessions revoked, and yunjin set-password as the lost-password path, cross-linked to the CLI docs.

[`README.md`](https://forge.marvin8.zone/marvin8/yunjin/pulls/68#issuecomment-4367) > **docs** [LOW] This bullet advertises self-service password change, but no documentation in this diff explains it... ✅ Fixed in commit `e1a8fb2` — passkeys.md (the page documenting the Settings → Account card) gains a "Changing your password" section: current password required, current session survives, other sessions revoked, and `yunjin set-password` as the lost-password path, cross-linked to the CLI docs.
Author
Collaborator

docs/cli.md

docs [LOW] The section never states that this command is the only way to create an account...

✅ Fixed in commit e1a8fb2 — the create-user section now states accounts can only be created from the command line: the web interface has no sign-up page and there is no admin role.

[`docs/cli.md`](https://forge.marvin8.zone/marvin8/yunjin/pulls/68#issuecomment-4368) > **docs** [LOW] The section never states that this command is the *only* way to create an account... ✅ Fixed in commit `e1a8fb2` — the `create-user` section now states accounts can only be created from the command line: the web interface has no sign-up page and there is no admin role.
📝 Move account docs to a dedicated page
All checks were successful
/ gitleaks (pull_request) Successful in 26s
/ pr-review (pull_request) Successful in 1m22s
/ checks (pull_request) Successful in 20m58s
/ publish-container (pull_request) Has been skipped
/ deploy-docs (pull_request) Has been skipped
/ container (pull_request) Successful in 1m47s
ab3edcdc4b
Author
Collaborator

docs/passkeys.md line 91

docs [MEDIUM] Account deletion is not a passkey feature, yet it is documented only on this page...

✅ Done in commit ab3edcd (Marvin8 approved the restructure): new docs/account.md — Account management, holding password change and account deletion, cross-linked from the passkeys page intro and footer, nav (after Passkeys), README's Account-management bullet, and docs/cli.md's delete-user section.

[`docs/passkeys.md` line 91](https://forge.marvin8.zone/marvin8/yunjin/pulls/68#issuecomment-4366) > **docs** [MEDIUM] Account deletion is not a passkey feature, yet it is documented only on this page... ✅ Done in commit `ab3edcd` (Marvin8 approved the restructure): new [docs/account.md](https://forge.marvin8.zone/marvin8/yunjin/blob/feat/issue-67-account-management/docs/account.md) — *Account management*, holding password change and account deletion, cross-linked from the passkeys page intro and footer, nav (after Passkeys), README's Account-management bullet, and `docs/cli.md`'s `delete-user` section.
forgejo-actions left a comment

WuMing

Found 3 issue(s). See inline comments below.

## WuMing Found **3** issue(s). See inline comments below.
docs/account.md Outdated
@ -0,0 +1,24 @@
# Account management
Your account is managed from **Settings → Account** in the web interface:
that card holds password change, passkey enrollment and management (see

docs [LOW]

This sentence presents passkey enrollment and management as permanently available on the Account card, but per docs/passkeys.md the Passkeys card only appears when passkey support is enabled (both YUNJIN_RP_ID and YUNJIN_ORIGIN set). Add a qualifier, e.g. 'passkey enrollment and management (only when passkey support is enabled — see Passkeys)'.

**docs** [LOW] This sentence presents passkey enrollment and management as permanently available on the Account card, but per docs/passkeys.md the Passkeys card only appears when passkey support is enabled (both YUNJIN_RP_ID and YUNJIN_ORIGIN set). Add a qualifier, e.g. 'passkey enrollment and management (only when passkey support is enabled — see [Passkeys](passkeys.md))'. <!-- wuming:sha256:5aa5732436d5b3be1cbbdc9f03368d9675d1037499b96e2e720b2862b912dc3d -->
docs/account.md Outdated
@ -0,0 +10,4 @@
Open **Settings → Account**, enter your current password and the new password
twice, and save. Your current session stays signed in; every other signed-in
device is signed out. If you have lost your password, an administrator on the

docs [LOW]

Line 6 of this same page states 'there is no admin role', so referring to 'an administrator on the server' here is inconsistent and may confuse readers looking for an admin UI. Reword to something role-free, e.g. 'anyone with access to the server's command line can reset it with yunjin set-password'.

**docs** [LOW] Line 6 of this same page states 'there is no admin role', so referring to 'an administrator on the server' here is inconsistent and may confuse readers looking for an admin UI. Reword to something role-free, e.g. 'anyone with access to the server\'s command line can reset it with `yunjin set-password`'. <!-- wuming:sha256:71403e6ecc6c8de7ffbea4aa3fdc120d62ea616deac6f2bbf570cf46a5132a13 -->
docs/cli.md Outdated
@ -53,1 +53,4 @@
### `create-user`
Create a new account with a password. This is the only way to create an

docs [LOW]

This claim conflicts with the rest of the page: the web interface auto-seeds a passwordless default account on first start (see the set-password section above and the delete-user note at line 96, which says a fresh start 're-seeds the passwordless default account'). Qualify it, e.g. 'This is the only way to explicitly create an account (the app also seeds a passwordless default account on first run)'.

**docs** [LOW] This claim conflicts with the rest of the page: the web interface auto-seeds a passwordless `default` account on first start (see the `set-password` section above and the `delete-user` note at line 96, which says a fresh start 're-seeds the passwordless `default` account'). Qualify it, e.g. 'This is the only way to *explicitly* create an account (the app also seeds a passwordless `default` account on first run)'. <!-- wuming:sha256:b38bd78506885817426fcc076940a778bb493df32d98d80f04b4f5b2f5a207b5 -->
📝 Qualify passkey gating, operator wording, and seeded default in docs
All checks were successful
/ gitleaks (pull_request) Successful in 17s
/ pr-review (pull_request) Successful in 1m15s
/ checks (pull_request) Successful in 21m16s
/ publish-container (pull_request) Has been skipped
/ deploy-docs (pull_request) Has been skipped
/ container (pull_request) Successful in 1m38s
f7fba96d94
Author
Collaborator

docs/account.md

docs [LOW] This sentence presents passkey enrollment and management as permanently available on the Account card...

✅ Fixed in commit f7fba96 — the intro now reads "— when passkey support is enabled (see Passkeys) — passkey enrollment and management".

[`docs/account.md`](https://forge.marvin8.zone/marvin8/yunjin/pulls/68#issuecomment-4377) > **docs** [LOW] This sentence presents passkey enrollment and management as permanently available on the Account card... ✅ Fixed in commit `f7fba96` — the intro now reads "— when passkey support is enabled (see [Passkeys](passkeys.md)) — passkey enrollment and management".
Author
Collaborator

docs/account.md

docs [LOW] Line 6 of this same page states 'there is no admin role', so referring to 'an administrator on the server' here is inconsistent...

✅ Fixed in commit f7fba96 — reworded to "whoever operates the server can reset it with yunjin set-password".

[`docs/account.md`](https://forge.marvin8.zone/marvin8/yunjin/pulls/68#issuecomment-4378) > **docs** [LOW] Line 6 of this same page states 'there is no admin role', so referring to 'an administrator on the server' here is inconsistent... ✅ Fixed in commit `f7fba96` — reworded to "whoever operates the server can reset it with `yunjin set-password`".
Author
Collaborator

docs/cli.md

docs [LOW] This claim conflicts with the rest of the page: the web interface auto-seeds a passwordless default account on first start...

✅ Fixed in commit f7fba96 — the sentence now reads "This is the only way to explicitly create an account (the app also seeds a passwordless default account on first run)", keeping the no-sign-up/no-admin-role clause.

[`docs/cli.md`](https://forge.marvin8.zone/marvin8/yunjin/pulls/68#issuecomment-4379) > **docs** [LOW] This claim conflicts with the rest of the page: the web interface auto-seeds a passwordless `default` account on first start... ✅ Fixed in commit `f7fba96` — the sentence now reads "This is the only way to explicitly create an account (the app also seeds a passwordless `default` account on first run)", keeping the no-sign-up/no-admin-role clause.
forgejo-actions left a comment

WuMing

Found 1 issue(s). See inline comments below.

## WuMing Found **1** issue(s). See inline comments below.
@ -0,0 +60,4 @@
# Flask rejects any session cookie older than PERMANENT_SESSION_LIFETIME
# (open_session max_age), so a literal 30-day offset needs a ceiling above
# it; only the guard's cap mechanism is under test here.
app.config["PERMANENT_SESSION_LIFETIME"] = timedelta(days=60)

tests [LOW]

This test mutates the config of the app fixture (PERMANENT_SESSION_LIFETIME) partway through and never restores it. If the app fixture is broader than function scope, the 60-day lifetime leaks into other tests in the module; even with a function-scoped fixture the test silently depends on that scope for correctness. Prefer monkeypatch.setitem(app.config, "PERMANENT_SESSION_LIFETIME", timedelta(days=60)) (or snapshot/restore in a finally block) so the override is guaranteed to be undone.

**tests** [LOW] This test mutates the config of the `app` fixture (`PERMANENT_SESSION_LIFETIME`) partway through and never restores it. If the `app` fixture is broader than function scope, the 60-day lifetime leaks into other tests in the module; even with a function-scoped fixture the test silently depends on that scope for correctness. Prefer `monkeypatch.setitem(app.config, "PERMANENT_SESSION_LIFETIME", timedelta(days=60))` (or snapshot/restore in a finally block) so the override is guaranteed to be undone. <!-- wuming:sha256:5ff12e2fa75574021897e43e4125b0eeda45e506de2a48eee6ef82798d309d23 -->
✅ Restore session lifetime config via monkeypatch
All checks were successful
/ gitleaks (pull_request) Successful in 20s
/ pr-review (pull_request) Successful in 1m16s
/ checks (pull_request) Successful in 21m13s
/ publish-container (pull_request) Has been skipped
/ deploy-docs (pull_request) Has been skipped
/ container (pull_request) Successful in 1m36s
d6318639e0
Author
Collaborator

tests/test_session_lifetime.py

tests [LOW] This test mutates the config of the app fixture (PERMANENT_SESSION_LIFETIME) partway through and never restores it...

✅ Fixed in commit d631863 — the override now uses monkeypatch.setitem(app.config, "PERMANENT_SESSION_LIFETIME", timedelta(days=60)), so it is guaranteed to be restored regardless of fixture scope. (The app fixture is function-scoped today, so nothing was leaking — this makes the test independent of that scope.)

[`tests/test_session_lifetime.py`](https://forge.marvin8.zone/marvin8/yunjin/pulls/68#issuecomment-4385) > **tests** [LOW] This test mutates the config of the `app` fixture (`PERMANENT_SESSION_LIFETIME`) partway through and never restores it... ✅ Fixed in commit `d631863` — the override now uses `monkeypatch.setitem(app.config, "PERMANENT_SESSION_LIFETIME", timedelta(days=60))`, so it is guaranteed to be restored regardless of fixture scope. (The `app` fixture is function-scoped today, so nothing was leaking — this makes the test independent of that scope.)
marvin8 approved these changes 2026-10-05 10:01:42 +00:00
marvin8 manually merged commit cea51eb532 into feat/multiuser 2026-10-06 03:35:17 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
marvin8/yunjin!68
No description provided.