feat: add Keycloak console links for players and admins #68
No reviewers
Labels
No labels
domain:backend
domain:devops
domain:frontend
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
forgejo_admin/westside-app!68
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "144-keycloak-console-links"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Add direct links to Keycloak consoles so players can manage their own account settings and admins can manage users without navigating to Keycloak manually.
Changes
src/routes/+layout.svelte— Added "Account" link in the player (default role) bottom nav bar, between Dashboard and Sign Out. Opens the Keycloak account console in a new tab.src/routes/admin/+page.svelte— Added "Manage Users (Keycloak)" button in the admin dashboard's "Manage" quick actions section. Opens the Keycloak admin console in a new tab.Both links use
target="_blank" rel="noopener noreferrer"for secure new-tab behavior.Test Plan
Review Checklist
target="_blank" rel="noopener noreferrer"btn btn-secondary btn-blockclasses for consistencysvelte-checkpasses with 0 errorsRelated
Self-review: LGTM
Reviewed the diff (7 additions, 0 deletions across 2 files). No issues found.
<a>+<span class="nav-icon">pattern, gear icon is appropriate.btn btn-secondary btn-blockclasses.target="_blank" rel="noopener noreferrer".svelte-checkpasses with 0 errors.PR #68 Review
DOMAIN REVIEW
Tech stack: SvelteKit (Svelte 5 runes), client-side Keycloak auth, CSS utility classes in
app.css.Correctness:
https://keycloak.tail5b443a.ts.net/realms/westside-basketball/account/is correct -- matches the Keycloak URL and realm name already established insrc/lib/keycloak.js(line 8:KEYCLOAK_URL, line 9:REALM = 'westside-basketball') andk8s/deployment.yaml(line 43).https://keycloak.tail5b443a.ts.net/admin/westside-basketball/console/follows the standard Keycloak admin console path pattern.target="_blank" rel="noopener noreferrer"-- correct secure new-tab behavior.actions-rowbelow the existing Players/Teams row.Scope / role visibility:
{:else}branch, line 124). It is NOT added to the admin nav or coach nav. Admins already get the Keycloak admin console link on their dashboard. Coaches get neither link. This may be intentional (coaches manage through their coach view), but worth confirming -- coaches might also want to manage their own Keycloak account.Accessibility:
class:activedirective. Every other bottom nav link usesclass:active={currentPath === '...'}for visual feedback. Since this is an external link it will never be "active" in the SPA sense, so functionally this is fine -- but it breaks the pattern. Not a blocker.aria-labelon the external links indicating they open in a new tab. Minor accessibility gap. Screen reader users would benefit from knowing the link leaves the app.CSS / Styling:
style="margin-top: 0.5rem;"on the newactions-row. The existingactions-rowclass usesgap: 0.5remvia CSS grid. The inline style is minor but inconsistent with the pattern of using CSS classes. Not a blocker.btn btn-secondary btn-block-- matches the Players/Teams buttons above it. Consistent.BLOCKERS
1. Hardcoded Keycloak URLs violate DRY (non-security path, but maintainability risk)
src/lib/keycloak.jsalready exports the Keycloak URL and realm as constants:The two new links hardcode these values inline in Svelte templates. If the Keycloak URL or realm name ever changes, these links will silently break while
keycloak.jsgets updated. There are now three independent locations defining the Keycloak base URL.This is not in a security/auth path so it does not meet the strict "DRY in auth paths" BLOCKER criteria. Downgrading to a strong nit -- but strongly recommended to export
KEYCLOAK_URLandREALMfromkeycloak.jsand construct these URLs from the constants.No formal BLOCKERS identified. This is a 7-line change adding two static links. No new functionality requiring test coverage (links are static HTML anchors). No user input. No secrets. No auth logic duplication.
NITS
DRY: Centralize Keycloak URLs. Export
KEYCLOAK_URLandREALMfromsrc/lib/keycloak.jsand use them to build the account/admin console URLs. Prevents silent breakage if the Keycloak host or realm changes. Three hardcoded occurrences across two files is the threshold where centralization pays off.Coach role missing Account link. The Account link is only in the default/parent nav. Coaches authenticated via Keycloak would also benefit from an Account link to manage their own profile/password. Confirm this omission is intentional.
Inline style on admin page.
style="margin-top: 0.5rem;"on the newactions-rowdiv (line 101 ofadmin/+page.svelte). Consider adding a CSS utility class or adjusting the existing.actions-rowspacing rule instead.Accessibility: external link indication. Consider adding
aria-label="Account settings (opens in new tab)"andaria-label="Manage Users in Keycloak (opens in new tab)"for screen reader users.PR body references non-existent issue. "Closes #144" -- the highest issue/PR number in this repo is #68 (this PR). Issue #144 does not exist. The branch name
144-keycloak-console-linksalso references this phantom issue number. This needs correction.SOP COMPLIANCE
144-keycloak-console-linksfollows the pattern, but issue #144 does not exist in this repo (see nit #5)feat: add Keycloak console links for players and adminsPROCESS OBSERVATIONS
VERDICT: APPROVED
Clean, minimal change. No blockers. The DRY concern with hardcoded URLs and the phantom issue reference are worth addressing but do not warrant blocking a 7-line link addition. The nits (especially #1 and #5) should be tracked as follow-up work.