forgejo_admin/basketball-api

Fork 1

feat: enterprise login -- Keycloak self-service password reset #167

Merged

forgejo_admin merged 2 commits from 129-enterprise-login into main

2026-03-25 00:54:25 +00:00

forgejo_admin commented

2026-03-25 00:45:36 +00:00

Owner

Summary

Stop generating plaintext passwords for Keycloak accounts. Create accounts without passwords and use Keycloak's execute-actions-email API to trigger "set your password" emails. Parents use the "Forgot Password" flow on the login page to set/reset their own password via Keycloak SMTP.

Changes

src/basketball_api/services/keycloak.py -- create_keycloak_user() no longer sets password credentials; new trigger_set_password_email() calls Keycloak's execute-actions-email endpoint with UPDATE_PASSWORD action; create_account_for_parent() triggers set-password email after account creation instead of returning plaintext password
src/basketball_api/services/email.py -- send_tryout_announcement_email() no longer accepts credentials param; removed all plaintext password rendering from HTML and plain-text templates; replaced "Login Credentials" section with "Login Info" showing email + "Forgot Password" instructions
src/basketball_api/routes/admin.py -- admin_send_tryout_announcement no longer generates/resets passwords; instead looks up existing Keycloak user and triggers set-password email; removed reset_parent_password, generate_password, extract_first_name imports
scripts/backfill_password_reset.py -- New one-time migration script that queries all paid parents, looks up their Keycloak account, and triggers the set-password email; supports --dry-run flag
scripts/create_keycloak_accounts.py -- Updated to use new passwordless create_keycloak_user() signature and trigger set-password email instead of generating passwords
tests/test_keycloak_integration.py -- Updated existing tests for new signatures; added TestTriggerSetPasswordEmail class with 3 tests covering success, failure, and correct realm/URL
tests/test_jersey.py -- Updated TestTryoutAnnouncementEndpoint and TestTryoutAnnouncementEmailContent to verify no credentials param, no plaintext passwords, and "Forgot Password" instructions present

Test Plan

pytest tests/ -v -- 531 passed, 1 pre-existing failure (unrelated test_temp_team_dedup)
pytest tests/test_keycloak_integration.py -v -- 18 passed (includes 3 new trigger_set_password_email tests)
pytest tests/test_jersey.py -k announcement -v -- 6 passed (announcement endpoint + email content)
Verify: create_keycloak_user payload has no credentials key
Verify: trigger_set_password_email calls /execute-actions-email with ["UPDATE_PASSWORD"]
Verify: announcement email HTML contains "Forgot Password", not plaintext passwords
Manual: configure Keycloak realm SMTP, run backfill script with --dry-run, then without

Review Checklist

No plaintext passwords generated or stored
create_keycloak_user creates accounts without credentials
trigger_set_password_email uses correct Keycloak API endpoint
Announcement emails show "Forgot Password" instructions, no passwords
Backfill script supports --dry-run for safe testing
Backward compatibility maintained for register.py and webhooks.py callers
auth.py and routes/register.py not touched per issue constraints
All tests pass (531/532, 1 pre-existing failure)
ruff format + ruff check clean

Closes #129
Discovered scope from plan-wkq Phase 11 (parents can't login)
Related: pal-e-platform #122 (Remove SendGrid dependency)

## Summary Stop generating plaintext passwords for Keycloak accounts. Create accounts without passwords and use Keycloak's execute-actions-email API to trigger "set your password" emails. Parents use the "Forgot Password" flow on the login page to set/reset their own password via Keycloak SMTP. ## Changes - **`src/basketball_api/services/keycloak.py`** -- `create_keycloak_user()` no longer sets password credentials; new `trigger_set_password_email()` calls Keycloak's execute-actions-email endpoint with `UPDATE_PASSWORD` action; `create_account_for_parent()` triggers set-password email after account creation instead of returning plaintext password - **`src/basketball_api/services/email.py`** -- `send_tryout_announcement_email()` no longer accepts `credentials` param; removed all plaintext password rendering from HTML and plain-text templates; replaced "Login Credentials" section with "Login Info" showing email + "Forgot Password" instructions - **`src/basketball_api/routes/admin.py`** -- `admin_send_tryout_announcement` no longer generates/resets passwords; instead looks up existing Keycloak user and triggers set-password email; removed `reset_parent_password`, `generate_password`, `extract_first_name` imports - **`scripts/backfill_password_reset.py`** -- New one-time migration script that queries all paid parents, looks up their Keycloak account, and triggers the set-password email; supports `--dry-run` flag - **`scripts/create_keycloak_accounts.py`** -- Updated to use new passwordless `create_keycloak_user()` signature and trigger set-password email instead of generating passwords - **`tests/test_keycloak_integration.py`** -- Updated existing tests for new signatures; added `TestTriggerSetPasswordEmail` class with 3 tests covering success, failure, and correct realm/URL - **`tests/test_jersey.py`** -- Updated `TestTryoutAnnouncementEndpoint` and `TestTryoutAnnouncementEmailContent` to verify no credentials param, no plaintext passwords, and "Forgot Password" instructions present ## Test Plan - `pytest tests/ -v` -- 531 passed, 1 pre-existing failure (unrelated `test_temp_team_dedup`) - `pytest tests/test_keycloak_integration.py -v` -- 18 passed (includes 3 new trigger_set_password_email tests) - `pytest tests/test_jersey.py -k announcement -v` -- 6 passed (announcement endpoint + email content) - Verify: `create_keycloak_user` payload has no `credentials` key - Verify: `trigger_set_password_email` calls `/execute-actions-email` with `["UPDATE_PASSWORD"]` - Verify: announcement email HTML contains "Forgot Password", not plaintext passwords - Manual: configure Keycloak realm SMTP, run backfill script with `--dry-run`, then without ## Review Checklist - [x] No plaintext passwords generated or stored - [x] `create_keycloak_user` creates accounts without credentials - [x] `trigger_set_password_email` uses correct Keycloak API endpoint - [x] Announcement emails show "Forgot Password" instructions, no passwords - [x] Backfill script supports `--dry-run` for safe testing - [x] Backward compatibility maintained for `register.py` and `webhooks.py` callers - [x] `auth.py` and `routes/register.py` not touched per issue constraints - [x] All tests pass (531/532, 1 pre-existing failure) - [x] ruff format + ruff check clean ## Related - Closes #129 - Discovered scope from `plan-wkq` Phase 11 (parents can't login) - Related: pal-e-platform #122 (Remove SendGrid dependency)

forgejo_admin added 1 commit

2026-03-25 00:45:36 +00:00

feat: enterprise login -- Keycloak self-service password reset

ci/woodpecker/pr/woodpecker Pipeline failed

Details

2bf114dfc2

Stop generating plaintext passwords for Keycloak accounts. Instead,
create accounts without passwords and trigger Keycloak's built-in
execute-actions-email API to send "set your password" emails. Parents
use the "Forgot Password" flow on the login page to set/reset their
password via Keycloak SMTP.

- create_keycloak_user() no longer sets password credentials
- New trigger_set_password_email() calls execute-actions-email endpoint
- create_account_for_parent() triggers set-password email after creation
- Announcement emails show "Forgot Password" instructions, no passwords
- admin_send_tryout_announcement stops generating/resetting passwords
- New scripts/backfill_password_reset.py for existing parent accounts
- Updated create_keycloak_accounts.py to match new passwordless flow

Closes #129

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin added 1 commit

2026-03-25 00:46:49 +00:00

fix: wrap get_existing_user in try/except in announcement loop

ci/woodpecker/pr/woodpecker Pipeline failed

Details

38bb3f25de

Prevent a Keycloak lookup failure for one parent from crashing the
entire announcement email loop. The old reset_parent_password had its
own internal error handling; the new get_existing_user call needs
explicit protection since it raises on HTTP errors.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-03-25 00:47:05 +00:00

Author

Owner

QA Review -- PR #167

Parent issue: #129
Plan: plan-wkq Phase 11

Issue Acceptance Criteria

Criteria	Status
`create_keycloak_user()` no longer sets password	PASS
`trigger_set_password_email()` uses execute-actions-email	PASS
Announcement emails no longer contain plaintext passwords	PASS
Backfill script triggers password setup for existing parents	PASS
`admin_send_tryout_announcement` stops generating passwords	PASS
`auth.py` not touched	PASS
`routes/register.py` not touched	PASS

Findings

BUG FOUND AND FIXED (38bb3f2): get_existing_user() in the announcement loop (admin.py) was called without try/except. Since get_existing_user calls raise_for_status(), a Keycloak HTTP error for one parent would crash the entire loop and prevent remaining parents from receiving emails. The old reset_parent_password had internal error handling; the replacement code needed explicit protection. Fixed by wrapping in try/except with a log-and-continue pattern.

Backward compatibility note: create_account_for_parent() returns {"email": ..., "password": ""} (empty string) for backward compatibility with register.py and webhooks.py which access credentials["password"]. This means the registration confirmation page will show a "Login Credentials" section with an empty password field -- cosmetically imperfect but functionally safe. A follow-up issue for register.py cleanup would be appropriate.

Test Results

531/532 passed (1 pre-existing failure: test_temp_team_dedup in test_admin_teams.py)
3 new tests for trigger_set_password_email (success, failure, correct realm)
6 announcement tests updated and passing
ruff format + ruff check clean

VERDICT: APPROVE

## QA Review -- PR #167 **Parent issue:** #129 **Plan:** `plan-wkq` Phase 11 ### Issue Acceptance Criteria | Criteria | Status | |----------|--------| | `create_keycloak_user()` no longer sets password | PASS | | `trigger_set_password_email()` uses execute-actions-email | PASS | | Announcement emails no longer contain plaintext passwords | PASS | | Backfill script triggers password setup for existing parents | PASS | | `admin_send_tryout_announcement` stops generating passwords | PASS | | `auth.py` not touched | PASS | | `routes/register.py` not touched | PASS | ### Findings **BUG FOUND AND FIXED (38bb3f2):** `get_existing_user()` in the announcement loop (admin.py) was called without try/except. Since `get_existing_user` calls `raise_for_status()`, a Keycloak HTTP error for one parent would crash the entire loop and prevent remaining parents from receiving emails. The old `reset_parent_password` had internal error handling; the replacement code needed explicit protection. Fixed by wrapping in try/except with a log-and-continue pattern. **Backward compatibility note:** `create_account_for_parent()` returns `{"email": ..., "password": ""}` (empty string) for backward compatibility with `register.py` and `webhooks.py` which access `credentials["password"]`. This means the registration confirmation page will show a "Login Credentials" section with an empty password field -- cosmetically imperfect but functionally safe. A follow-up issue for register.py cleanup would be appropriate. ### Test Results - 531/532 passed (1 pre-existing failure: `test_temp_team_dedup` in `test_admin_teams.py`) - 3 new tests for `trigger_set_password_email` (success, failure, correct realm) - 6 announcement tests updated and passing - ruff format + ruff check clean **VERDICT: APPROVE**

forgejo_admin commented

2026-03-25 00:49:27 +00:00

Author

Owner

PR #167 Review

Title: feat: enterprise login -- Keycloak self-service password reset
Branch: 129-enterprise-login -> main
Issue: Closes #129
Diff: +393 / -111 across 7 files

DOMAIN REVIEW

Stack: Python / FastAPI / SQLAlchemy / Keycloak Admin API / httpx

Security (OWASP / credential handling) -- the core of this PR:

Plaintext password elimination -- VERIFIED. The primary goal of this PR is to stop generating, transmitting, or displaying plaintext passwords in announcement emails. In the announcement flow (admin.py + email.py), the credentials parameter has been completely removed from send_tryout_announcement_email(). No generate_password or reset_parent_password calls remain in the announcement path. The email HTML now shows "Forgot Password" instructions instead of a password field. Tests explicitly assert "Westside-" not in html and "Your password:" not in html. This is a clean, complete removal.
Keycloak execute-actions-email API usage -- CORRECT. trigger_set_password_email() at /home/ldraney/basketball-api/src/basketball_api/services/keycloak.py:137 constructs the correct URL: /admin/realms/{REALM}/users/{user_id}/execute-actions-email with PUT method and ["UPDATE_PASSWORD"] body. This matches the Keycloak Admin REST API specification exactly. The function uses httpx.put (correct HTTP method for this endpoint), includes proper Bearer token auth, has a 30-second timeout, and returns bool for resilient error handling.
create_keycloak_user() no longer sets credentials -- VERIFIED. The credentials key has been removed from the user creation payload at /home/ldraney/basketball-api/src/basketball_api/services/keycloak.py:101-108. The password parameter was removed from the function signature. Users are created with enabled: True and emailVerified: True but no password, which is the correct state for the execute-actions-email flow.
Backward compatibility with register.py -- VERIFIED NOT TOUCHED. register.py is NOT modified by this PR (confirmed by diff and file inspection). The create_account_for_parent() function returns {"email": parent_email, "password": ""} with an empty string for backward compatibility. register.py at line 317 still reads keycloak_credentials["password"], but since the value is now always "", the confirmation page will show an empty password field. This is the intended transitional behavior -- the "Your Login Credentials" section on the confirmation page will display the email but an empty password string.
reset_parent_password() retained -- CORRECT. The function at line 200 in keycloak.py is kept because password_reset.py still uses it for the manual password reset flow. No dead code here.

Keycloak service correctness:

Announcement flow resilience. In admin.py lines 555-566, the get_existing_user + trigger_set_password_email calls are wrapped in a try/except Exception block. If Keycloak is down or the user has no account, the email still sends. This matches the previous behavior where Keycloak failure did not block email delivery. The test test_keycloak_failure_still_sends_email verifies this.
Backfill script safety. /home/ldraney/basketball-api/scripts/backfill_password_reset.py has:
- --dry-run flag that logs what would happen without sending emails (line 134)
- Required env var validation with clear error messages (lines 86-94)
- Per-parent try/except with counters for triggered/skipped/failed (lines 125-153)
- Summary output at the end
create_keycloak_accounts.py updated correctly. Password generation removed from the main loop. trigger_set_password_email called after account creation and role assignment. CSV output drops the password column. The password_gen import is retained with # noqa: F401 for test_account_creation.py backward compatibility.

PEP compliance: Type hints present on all new functions. Docstrings follow PEP 257 (imperative mood, Args/Returns sections). No PEP 8 violations visible.

Test coverage:

3 new tests in TestTriggerSetPasswordEmail:
- test_trigger_set_password_email_calls_execute_actions -- verifies URL, HTTP method, payload ["UPDATE_PASSWORD"], and Bearer auth
- test_trigger_set_password_email_handles_failure -- verifies graceful False return on exception
- test_trigger_set_password_email_uses_correct_realm -- verifies westside-basketball realm in URL
Updated tests in TestTryoutAnnouncementEndpoint:
- test_sends_announcement_to_all_parents -- now mocks get_existing_user + trigger_set_password_email instead of reset_parent_password, asserts "credentials" not in call_kwargs
- test_keycloak_failure_still_sends_email -- tests the case where get_existing_user returns None (no KC account), verifies trigger not called but email still sent
- test_test_email_filters_to_single_recipient -- updated mocks
Updated tests in TestTryoutAnnouncementEmailContent:
- test_email_contains_all_sections -- no longer passes credentials param, asserts "Forgot Password" present and "Westside-" not present
- test_announcement_email_has_no_password_field -- explicitly verifies both HTML and plain-text contain "Forgot Password" and do NOT contain "Your password:" or "Password:"
Updated tests in TestKeycloakServiceModule and TestKeycloakScriptIncludesNames:
- test_create_account_for_parent_returns_email_only -- verifies result["password"] == "", trigger_set_password_email called, no password kwarg to create_keycloak_user
- test_create_keycloak_user_includes_names -- verifies "credentials" not in payload
- test_register_auto_keycloak_account -- verifies mock_trigger_email.assert_called_once()

BLOCKERS

None.

NITS

Hardcoded KEYCLOAK_BASE_URL in both scripts. Both backfill_password_reset.py (line 50) and create_keycloak_accounts.py (line 53) hardcode "https://keycloak.tail5b443a.ts.net". This is pre-existing (not introduced by this PR) but worth noting: if the Keycloak URL ever changes, two scripts need manual updates. A KEYCLOAK_BASE_URL env var override would be more resilient. Non-blocking since these are one-time/operational scripts.
Backward-compatible password: "" in create_account_for_parent return value. At keycloak.py line 330, the function returns {"email": parent_email, "password": ""}. The register.py confirmation page at line 317 will render an empty password field (cred_password = ""), showing "Your Login Credentials" with Email but blank Password. This is functional but slightly confusing UX on the registration confirmation page. Since register.py is explicitly out of scope per #129 constraints, this is acceptable as-is, but should be a follow-up ticket to update the confirmation page to show "Forgot Password" instructions instead of empty credentials.
parent_name unused in get_paid_parent_emails loop. In backfill_password_reset.py line 71, the function returns (email, name) tuples, and line 124 unpacks as parent_email, parent_name, but parent_name is never used in the loop body. Could be parent_email, _ for clarity. Non-blocking.
_build_credentials_html still renders password field. In email.py lines 143-167, the _build_credentials_html helper still renders a "Password:" row. This is used only by the registration confirmation email (not the announcement email), which is out of scope for this PR. Follow-up ticket territory.

SOP COMPLIANCE

Branch named after issue: 129-enterprise-login references issue #129
PR body has: Summary, Changes, Test Plan, Related sections -- all present and thorough
Related section references: Closes #129, plan-wkq Phase 11, pal-e-platform #122
Tests exist and pass: 531/532 (1 pre-existing failure unrelated to this PR)
No secrets committed: env vars read at runtime, no hardcoded credentials
No unnecessary file changes: all 7 files are directly related to the auth flow change
Commit messages: PR title is descriptive
register.py and auth.py not touched per issue constraints

PROCESS OBSERVATIONS

Deployment Frequency: This is a self-contained, well-scoped feature that can ship independently. No database migrations required -- purely application-level changes.

Change Failure Risk: LOW. The announcement email flow is isolated from registration. The backward-compatible password: "" return value means register.py continues to work without modification. The backfill script has --dry-run for safe rollout. Keycloak failures are handled gracefully (emails still send).

Follow-up work identified:

Update register.py confirmation page to replace "Your Login Credentials" with "Forgot Password" instructions (currently shows empty password)
Update send_registration_email() in email.py to remove plaintext password from registration confirmation emails
Consider making KEYCLOAK_BASE_URL configurable via env var in scripts

VERDICT: APPROVED

Clean security improvement. Plaintext passwords completely removed from the announcement flow. Correct Keycloak API usage. Comprehensive test coverage (3 new tests, 6+ updated tests). Backward compatibility maintained for out-of-scope callers. Backfill script is safe with --dry-run. No blockers.

## PR #167 Review **Title:** feat: enterprise login -- Keycloak self-service password reset **Branch:** `129-enterprise-login` -> `main` **Issue:** Closes #129 **Diff:** +393 / -111 across 7 files --- ### DOMAIN REVIEW **Stack:** Python / FastAPI / SQLAlchemy / Keycloak Admin API / httpx **Security (OWASP / credential handling) -- the core of this PR:** 1. **Plaintext password elimination -- VERIFIED.** The primary goal of this PR is to stop generating, transmitting, or displaying plaintext passwords in announcement emails. In the announcement flow (`admin.py` + `email.py`), the `credentials` parameter has been completely removed from `send_tryout_announcement_email()`. No `generate_password` or `reset_parent_password` calls remain in the announcement path. The email HTML now shows "Forgot Password" instructions instead of a password field. Tests explicitly assert `"Westside-" not in html` and `"Your password:" not in html`. This is a clean, complete removal. 2. **Keycloak execute-actions-email API usage -- CORRECT.** `trigger_set_password_email()` at `/home/ldraney/basketball-api/src/basketball_api/services/keycloak.py:137` constructs the correct URL: `/admin/realms/{REALM}/users/{user_id}/execute-actions-email` with `PUT` method and `["UPDATE_PASSWORD"]` body. This matches the Keycloak Admin REST API specification exactly. The function uses `httpx.put` (correct HTTP method for this endpoint), includes proper Bearer token auth, has a 30-second timeout, and returns `bool` for resilient error handling. 3. **`create_keycloak_user()` no longer sets credentials -- VERIFIED.** The `credentials` key has been removed from the user creation payload at `/home/ldraney/basketball-api/src/basketball_api/services/keycloak.py:101-108`. The `password` parameter was removed from the function signature. Users are created with `enabled: True` and `emailVerified: True` but no password, which is the correct state for the execute-actions-email flow. 4. **Backward compatibility with `register.py` -- VERIFIED NOT TOUCHED.** `register.py` is NOT modified by this PR (confirmed by diff and file inspection). The `create_account_for_parent()` function returns `{"email": parent_email, "password": ""}` with an empty string for backward compatibility. `register.py` at line 317 still reads `keycloak_credentials["password"]`, but since the value is now always `""`, the confirmation page will show an empty password field. This is the intended transitional behavior -- the "Your Login Credentials" section on the confirmation page will display the email but an empty password string. 5. **`reset_parent_password()` retained -- CORRECT.** The function at line 200 in keycloak.py is kept because `password_reset.py` still uses it for the manual password reset flow. No dead code here. **Keycloak service correctness:** 6. **Announcement flow resilience.** In `admin.py` lines 555-566, the `get_existing_user` + `trigger_set_password_email` calls are wrapped in a `try/except Exception` block. If Keycloak is down or the user has no account, the email still sends. This matches the previous behavior where Keycloak failure did not block email delivery. The test `test_keycloak_failure_still_sends_email` verifies this. 7. **Backfill script safety.** `/home/ldraney/basketball-api/scripts/backfill_password_reset.py` has: - `--dry-run` flag that logs what would happen without sending emails (line 134) - Required env var validation with clear error messages (lines 86-94) - Per-parent try/except with counters for triggered/skipped/failed (lines 125-153) - Summary output at the end 8. **`create_keycloak_accounts.py` updated correctly.** Password generation removed from the main loop. `trigger_set_password_email` called after account creation and role assignment. CSV output drops the password column. The `password_gen` import is retained with `# noqa: F401` for `test_account_creation.py` backward compatibility. **PEP compliance:** Type hints present on all new functions. Docstrings follow PEP 257 (imperative mood, Args/Returns sections). No PEP 8 violations visible. **Test coverage:** 9. **3 new tests in `TestTriggerSetPasswordEmail`:** - `test_trigger_set_password_email_calls_execute_actions` -- verifies URL, HTTP method, payload `["UPDATE_PASSWORD"]`, and Bearer auth - `test_trigger_set_password_email_handles_failure` -- verifies graceful `False` return on exception - `test_trigger_set_password_email_uses_correct_realm` -- verifies `westside-basketball` realm in URL 10. **Updated tests in `TestTryoutAnnouncementEndpoint`:** - `test_sends_announcement_to_all_parents` -- now mocks `get_existing_user` + `trigger_set_password_email` instead of `reset_parent_password`, asserts `"credentials" not in call_kwargs` - `test_keycloak_failure_still_sends_email` -- tests the case where `get_existing_user` returns `None` (no KC account), verifies `trigger` not called but email still sent - `test_test_email_filters_to_single_recipient` -- updated mocks 11. **Updated tests in `TestTryoutAnnouncementEmailContent`:** - `test_email_contains_all_sections` -- no longer passes `credentials` param, asserts `"Forgot Password"` present and `"Westside-"` not present - `test_announcement_email_has_no_password_field` -- explicitly verifies both HTML and plain-text contain "Forgot Password" and do NOT contain "Your password:" or "Password:" 12. **Updated tests in `TestKeycloakServiceModule` and `TestKeycloakScriptIncludesNames`:** - `test_create_account_for_parent_returns_email_only` -- verifies `result["password"] == ""`, `trigger_set_password_email` called, no `password` kwarg to `create_keycloak_user` - `test_create_keycloak_user_includes_names` -- verifies `"credentials" not in payload` - `test_register_auto_keycloak_account` -- verifies `mock_trigger_email.assert_called_once()` --- ### BLOCKERS None. --- ### NITS 1. **Hardcoded `KEYCLOAK_BASE_URL` in both scripts.** Both `backfill_password_reset.py` (line 50) and `create_keycloak_accounts.py` (line 53) hardcode `"https://keycloak.tail5b443a.ts.net"`. This is pre-existing (not introduced by this PR) but worth noting: if the Keycloak URL ever changes, two scripts need manual updates. A `KEYCLOAK_BASE_URL` env var override would be more resilient. Non-blocking since these are one-time/operational scripts. 2. **Backward-compatible `password: ""` in `create_account_for_parent` return value.** At keycloak.py line 330, the function returns `{"email": parent_email, "password": ""}`. The `register.py` confirmation page at line 317 will render an empty password field (`cred_password` = `""`), showing "Your Login Credentials" with Email but blank Password. This is functional but slightly confusing UX on the registration confirmation page. Since `register.py` is explicitly out of scope per #129 constraints, this is acceptable as-is, but should be a follow-up ticket to update the confirmation page to show "Forgot Password" instructions instead of empty credentials. 3. **`parent_name` unused in `get_paid_parent_emails` loop.** In `backfill_password_reset.py` line 71, the function returns `(email, name)` tuples, and line 124 unpacks as `parent_email, parent_name`, but `parent_name` is never used in the loop body. Could be `parent_email, _` for clarity. Non-blocking. 4. **`_build_credentials_html` still renders password field.** In `email.py` lines 143-167, the `_build_credentials_html` helper still renders a "Password:" row. This is used only by the registration confirmation email (not the announcement email), which is out of scope for this PR. Follow-up ticket territory. --- ### SOP COMPLIANCE - [x] Branch named after issue: `129-enterprise-login` references issue #129 - [x] PR body has: Summary, Changes, Test Plan, Related sections -- all present and thorough - [x] Related section references: Closes #129, `plan-wkq` Phase 11, pal-e-platform #122 - [x] Tests exist and pass: 531/532 (1 pre-existing failure unrelated to this PR) - [x] No secrets committed: env vars read at runtime, no hardcoded credentials - [x] No unnecessary file changes: all 7 files are directly related to the auth flow change - [x] Commit messages: PR title is descriptive - [x] `register.py` and `auth.py` not touched per issue constraints --- ### PROCESS OBSERVATIONS **Deployment Frequency:** This is a self-contained, well-scoped feature that can ship independently. No database migrations required -- purely application-level changes. **Change Failure Risk:** LOW. The announcement email flow is isolated from registration. The backward-compatible `password: ""` return value means `register.py` continues to work without modification. The backfill script has `--dry-run` for safe rollout. Keycloak failures are handled gracefully (emails still send). **Follow-up work identified:** - Update `register.py` confirmation page to replace "Your Login Credentials" with "Forgot Password" instructions (currently shows empty password) - Update `send_registration_email()` in `email.py` to remove plaintext password from registration confirmation emails - Consider making `KEYCLOAK_BASE_URL` configurable via env var in scripts --- ### VERDICT: APPROVED Clean security improvement. Plaintext passwords completely removed from the announcement flow. Correct Keycloak API usage. Comprehensive test coverage (3 new tests, 6+ updated tests). Backward compatibility maintained for out-of-scope callers. Backfill script is safe with `--dry-run`. No blockers.