98 lines
6.9 KiB
Markdown
98 lines
6.9 KiB
Markdown
# AUTH — Known Issues
|
|
|
|
Active known issues for user authentication (bearer tokens, JWT, login flow, hub authorization).
|
|
|
|
For planned/actionable work see [`AUTH_TODO.md`](AUTH_TODO.md).
|
|
For the architectural decision that scoped this topic, see [`../adr/0001-user-bearer-token-flow.md`](../adr/0001-user-bearer-token-flow.md).
|
|
|
|
## Active entries
|
|
|
|
## ACCORE-AUTH-I-B3X5: JWT signing key written to log (CRITICAL security)
|
|
|
|
**Severity:** 🛑 **Critical (security)** · **Status:** Closed (2026-04-25) · **Area:** `AyCode.Services.Server/Logins/AcLoginServiceServer.cs:192`
|
|
|
|
### Description
|
|
`GenerateAccessToken` writes the JWT signing key to the logger via `GlobalLogger.Detail`:
|
|
|
|
```csharp
|
|
GlobalLogger.Detail($"Key: {configuration["JWT:Key"]}");
|
|
```
|
|
|
|
The signing key is the cryptographic secret used to sign all JWT access tokens. Anyone with read access to the logs (file system, log aggregators, retention archives, operator dashboards, screenshots during incident response) can:
|
|
|
|
1. Forge arbitrary tokens for any user
|
|
2. Impersonate any role / claim
|
|
3. Bypass authentication entirely
|
|
|
|
Log storage is typically at a lower trust level than dedicated secret stores. This exposure path is a complete authentication bypass.
|
|
|
|
### Fix direction
|
|
Remove the `GlobalLogger.Detail($"Key: ...")` line entirely. The signing key must NEVER appear in any log output. If presence verification is needed for diagnostics, log only a hash prefix (e.g. `Key configured: SHA256[0..8] = abc12345`).
|
|
|
|
### Resolution
|
|
|
|
**What:** Wrapped the `GlobalLogger.Detail($"Key: ...")` call in `#if DEBUG` / `#endif` with a `// SECURITY:` comment block explaining the risk. Line preserved (not deleted) — user opted for DEBUG-gating over removal so local auth-debug retains the diagnostic.
|
|
|
|
**Where:** `AyCode.Services.Server/Logins/AcLoginServiceServer.cs:192` (line numbers shifted slightly due to the inserted comment).
|
|
|
|
**Why:** User-directed deviation from the originally proposed "remove entirely" fix direction (bearer-token ADR planning session, 2026-04-25). DEBUG-gating ensures production (`-c Release`) builds strip the line at compile time, while preserving dev-time diagnostics.
|
|
|
|
**Caveat:** DEBUG builds still write the key to log. CI/CD must build with `-c Release` for production deployments. The inline `// SECURITY:` comment in the code warns future maintainers.
|
|
|
|
### Related
|
|
- **Supersedes:** [`ACCORE-LOG-I-P5W3`](../../AyCode.Core/docs/LOGGING/LOGGING_ISSUES.md#accore-log-i-p5w3) — original LOG-topic home; relocated here per ADR-0001 "Status migration on AUTH topic creation" follow-up. The LOGGING entry retains its full body (immutable per `TOPIC_CODES.md` Status conventions) and now points back here as the canonical home.
|
|
- **Reference:** ADR [`0001-user-bearer-token-flow.md`](../adr/0001-user-bearer-token-flow.md) — formal architectural decision; this entry's Resolution is the pre-flight fix referenced in the ADR's Consequences.
|
|
- `ACCORE-AUTH-I-T8N2` (sibling — access token also leaked, same file, same code path)
|
|
- `LOGGING/README.md` may need a new "Never log secrets" guideline section (forthcoming TODO entry in `LOGGING_TODO.md`, no ID assigned yet).
|
|
- **Discovery context**: this issue and `ACCORE-AUTH-I-T8N2` emerged during the user-bearer-token ADR context-gathering phase (2026-04-25, while reading `AcLoginServiceServer.cs`). Originally tracked under LOG topic (`ACCORE-LOG-I-P5W3`); relocated here when AUTH topic was created (`LLMP-DEC-49`).
|
|
|
|
## ACCORE-AUTH-I-T8N2: JWT access token written to log (CRITICAL security)
|
|
|
|
**Severity:** 🛑 **Critical (security)** · **Status:** Closed (2026-04-25) · **Area:** `AyCode.Services.Server/Logins/AcLoginServiceServer.cs:212`
|
|
|
|
### Description
|
|
After `GenerateAccessToken` produces a token, the token text itself is written to the logger:
|
|
|
|
```csharp
|
|
GlobalLogger.Detail($"AccesToken: {writtenToken}");
|
|
```
|
|
|
|
(Side note: `AccesToken` is a typo for `AccessToken` — fix while at the line.)
|
|
|
|
Access tokens are bearer credentials — anyone holding the token can authenticate as the user until token expiry (currently 6h per `GenerateAccessToken` config). Logged tokens leak via the same channels as `ACCORE-AUTH-I-B3X5` (live streams, retention archives, screenshots, shared logs during ops).
|
|
|
|
### Fix direction
|
|
Remove the `GlobalLogger.Detail($"AccesToken: ...")` line entirely. If issuance verification is needed, log only **metadata** — user ID, expiry timestamp, issuer — never the token itself.
|
|
|
|
### Resolution
|
|
|
|
**What:** Two changes:
|
|
1. Wrapped the `GlobalLogger.Detail($"AccessToken: ...")` call in `#if DEBUG` / `#endif` with a `// SECURITY:` comment block.
|
|
2. Fixed the `AccesToken` → `AccessToken` typo (flagged in original Description as a side-note) in the log-message string literal.
|
|
|
|
**Where:** `AyCode.Services.Server/Logins/AcLoginServiceServer.cs:212` (line numbers shifted slightly due to the inserted comment).
|
|
|
|
**Why:** Same as `ACCORE-AUTH-I-B3X5` — user opted for DEBUG-gating over deletion (2026-04-25, bearer-token ADR session). Typo fix is opportunistic since the line was being touched.
|
|
|
|
**Caveat:** Same DEBUG-build leak constraint as `ACCORE-AUTH-I-B3X5`. CI/CD must build with `-c Release` for production.
|
|
|
|
### Related
|
|
- **Supersedes:** [`ACCORE-LOG-I-K1Z7`](../../AyCode.Core/docs/LOGGING/LOGGING_ISSUES.md#accore-log-i-k1z7) — original LOG-topic home; relocated here per ADR-0001 "Status migration on AUTH topic creation" follow-up.
|
|
- **Reference:** ADR [`0001-user-bearer-token-flow.md`](../adr/0001-user-bearer-token-flow.md) — formal architectural decision; this entry's Resolution is the pre-flight fix referenced in the ADR's Consequences.
|
|
- `ACCORE-AUTH-I-B3X5` (sibling — JWT signing key also leaked, same file)
|
|
- Same `LOGGING/README.md` "Never log secrets" guideline gap as `ACCORE-AUTH-I-B3X5`.
|
|
- **Discovery context**: emerged alongside `ACCORE-AUTH-I-B3X5` during the bearer-token ADR session. Originally tracked under LOG topic (`ACCORE-LOG-I-K1Z7`); relocated here when AUTH topic was created (`LLMP-DEC-49`).
|
|
|
|
## Entry format
|
|
|
|
When adding new entries, follow the standard `_ISSUES.md` shape used across topics (LOGGING, BINARY, SIGNALR), with `ACCORE-AUTH-I-<RAND>` ID format per [`TOPIC_CODES.md`](../../.github/skills/docs-check/references/TOPIC_CODES.md) and [`REPO_PREFIXES.md`](../../.github/REPO_PREFIXES.md):
|
|
|
|
- ID line with `**Severity:** ... · **Status:** ... · **Area:** ...`
|
|
- `### Description` — concrete problem with quotable evidence
|
|
- `### Root cause` — what's actually wrong (optional)
|
|
- `### Fix direction` — proposed approach
|
|
- `### Resolution` — when Status moves to `Closed` (per `TOPIC_CODES.md` Status conventions): what / where / why / caveat
|
|
- `### Related` — sibling entries, cross-topic refs, ADR refs (`**Reference:** ADR-NNNN` for ADR pre-flight cross-refs per `adr-author/SKILL.md` Step 8 #3)
|
|
|
|
Status vocabulary (per `TOPIC_CODES.md`): `Open`, `InProgress`, `Closed (YYYY-MM-DD)`. Three values only — distinct from ADR Status (Proposed/Accepted/Superseded/Rejected).
|