(tooling-agents) branch main updated: ASVS audit: 10.5.2.md (redacted) [source: apache/steve/v3 @ d0aa7e9]

[as...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

asf-tooling pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tooling-agents.git


The following commit(s) were added to refs/heads/main by this push:
     new e2aa2d2  ASVS audit: 10.5.2.md (redacted) [source: apache/steve/v3 @ d0aa7e9]
e2aa2d2 is described below

commit e2aa2d239561f62a2968b3d14a3bab9f520e55f2
Author: ASF Tooling Team <ro...@apache.org>
AuthorDate: Tue May 5 22:51:28 2026 +0200

    ASVS audit: 10.5.2.md (redacted) [source: apache/steve/v3 @ d0aa7e9]
---
 ASVS/reports/steve/v3/d0aa7e9/all/10.5.2.md | 127 ++++++++++++++++++++++++++++
 1 file changed, 127 insertions(+)

diff --git a/ASVS/reports/steve/v3/d0aa7e9/all/10.5.2.md b/ASVS/reports/steve/v3/d0aa7e9/all/10.5.2.md
new file mode 100644
index 0000000..333d5ab
--- /dev/null
+++ b/ASVS/reports/steve/v3/d0aa7e9/all/10.5.2.md
@@ -0,0 +1,127 @@
+# Security Audit Report: ASVS 10.5.2
+
+## Executive Summary
+
+| Field | Value |
+|-------|-------|
+| **Repository** | apache/steve/v3 |
+| **Audit Date** | May 05, 2026 |
+| **Auditor** | Tooling Agents (bundled-pass mode) |
+| **ASVS Requirement** | ASVS 10.5.2 |
+| **Files Analyzed** | 3 relevant / 49 total |
+| **Files Skipped** | 12 |
+
+**Requirement description:** Verify that the client uniquely identifies the user from ID Token claims, usually the 'sub' claim, which cannot be reassigned to other users (for the scope of an identity provider).
+
+### Findings Overview
+
+| Severity | Count |
+|----------|-------|
+| 🔴 Critical | 0 |
+| 🟠 High | 0 |
+| 🟡 Medium | 0 |
+| 🟢 Low | 1 |
+
+---
+
+## ASVS-10.5.2: Unique User Identification from ID Token Claims
+
+### Findings for 10.5.2
+
+### [LOW]
+
+**Finding ID:** ASVS-1052-LOW-001
+
+**Title:** User identification relies on session `uid` without visible mapping to ID Token `sub` claim
+
+**File:** `v3/server/pages.py:70-73`
+
+**Code:**
+
+```python
+s = await asfquart.session.read()
+if s:
+    basic.update(
+        uid=s['uid'],
+        name=s['fullname'],
+        email=s['email'],
+    )
+```
+
+**Analysis:** The application uses `s['uid']` as the primary user identifier throughout the codebase (for database queries, authorization checks, vote association, etc.). This value is used consistently as the `pid` (Person ID) in:
+- `steve.election.Election.open_to_pid(DB_FNAME, result.uid)` (line 148)
+- `election.q_find_issues.perform(result.uid, election.eid)` (line 236)
+- `election.add_vote(result.uid, iid, votestring)` (line 404)
+- `pdb.get_person(result.uid)` (line 302)
+
+The `uid` is used as a stable, unique identifier and is stored/matched in the `person` table. However, we cannot verify from the provided code that:
+1. `s['uid']` maps to the `sub` claim from the ID Token
+2. The `sub` claim is not reassignable by the identity provider
+3. The combination of issuer + subject is used if multiple IdPs are supported
+
+**Impact:** If `uid` does not derive from the `sub` claim (e.g., if it uses `email` or `preferred_username` which can change), user confusion or account takeover could occur.
+
+**Note:** LOW severity because the code uses `uid` consistently (suggesting it IS the `sub` claim) and the system appears to use a single identity provider (ASF).
+
+### Security Controls Inventory for 10.5.2
+
+| Control | Location | Status |
+|---------|----------|--------|
+| Unique user identification | `pages.py:70` via `s['uid']` | Present — consistently used as primary identifier |
+| User storage | `queries.yaml` `person` table with `pid` as key | Present — `pid` appears to be the unique identifier |
+| User lookup | `persondb.get_person(pid)` | Present — enforces existence |
+
+### Positive Patterns for 10.5.2
+
+- The `uid` is used consistently throughout the application as the primary user identifier for all operations (voting, election ownership, authorization checks).
+- The `person` table uses `pid` as the primary identifier with an ON CONFLICT UPDATE clause in `c_add_person`, ensuring the identifier is stable and name/email are treated as mutable attributes:
+  ```yaml
+  c_add_person: |
+      INSERT INTO person VALUES (?, ?, ?)
+      ON CONFLICT DO UPDATE SET
+          name=excluded.name,
+          email=excluded.email
+  ```
+- This design correctly treats `name` and `email` as mutable while keeping `pid` (the `sub` equivalent) as the immutable identifier.
+
+---
+
+
+
+## Cross-cutting Architecture Observations
+
+1. **Authentication Delegation Pattern**: The entire OIDC client functionality is delegated to the `asfquart` library. While this is architecturally sound (centralized security controls), it means compliance with ASVS 10.5.x requirements cannot be verified from the application code alone. The security posture depends entirely on the correctness of `asfquart`.
+
+2. **Not an OAuth Authorization Server**: This application is purely an OIDC relying party / OAuth client. Requirements 10.4.8 and 10.4.9 are not applicable to this codebase as it does not issue, manage, or revoke tokens.
+
+3. **Single Identity Provider Architecture**: The application appears to use only the ASF identity provider, which simplifies OIDC security (single issuer, single trust relationship) but the lack of visible configuration makes it impossible to verify that issuer pinning is enforced.
+
+4. **Session-Based Security Model**: After initial OIDC authentication, the application operates on server-side sessions rather than tokens. This means token replay/audience attacks are limited to the initial authentication flow (handled by `asfquart`), not to subsequent requests.
+
+5. **Evolving Authentication**: Comments like `### need general solution` on auth decorators suggest the authentication/authorization system is still under development, which increases risk of security gaps.
+
+## Cross-cutting Recommendations
+
+### Immediate
+1. **Audit `asfquart` Library**: The provided code cannot satisfy ASVS 10.5.x verification without examining the `asfquart` library. Conduct a focused audit of `asfquart.auth` and `asfquart.session` for:
+   - Nonce generation and validation (10.5.1)
+   - `sub` claim usage for user identification (10.5.2)
+   - Issuer URL validation (10.5.3)
+   - Audience claim validation (10.5.4)
+
+2. **Document Security Assumptions**: Create explicit documentation stating which OIDC security controls are expected from `asfquart` vs. implemented in the application.
+
+### Short-term
+3. **Add Integration Tests**: Write tests that verify the OIDC flow end-to-end, including:
+   - Rejection of tokens with incorrect nonce
+   - Rejection of tokens with mismatched audience
+   - Rejection of tokens from unauthorized issuers
+   - Proper mapping of `sub` claim to `uid`/`pid`
+
+4. **Configuration Visibility**: Surface the OIDC client configuration (issuer URL, client_id) in the application configuration rather than burying it in the library, making security audits more straightforward.
+
+### Long-term
+5. **Resolve Auth TODOs**: Address all `### need general solution` and `### check authz` placeholders to ensure the authorization model is complete and enforceable.
+
+6. **Consider Multi-IdP Support**: If the application may support multiple identity providers in the future, implement issuer-scoped user identification (`issuer` + `sub` as the unique key) now to prevent account confusion attacks later.
+        
\ No newline at end of file


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@tooling.apache.org
For additional commands, e-mail: commits-help@tooling.apache.org