feat: redirect root path based on auth state #52

clawbot · 2026-03-17T12:58:37+01:00

clawbot commented

2026-03-17 12:58:37 +01:00

Closes #51

The root path / now checks for an authenticated session and redirects accordingly:

Authenticated users → 303 See Other redirect to /sources (the webhook dashboard)
Unauthenticated users → 303 See Other redirect to /pages/login

Changes

internal/handlers/index.go — Replaced the template-rendering HandleIndex() with a session-checking redirect handler. Removed formatUptime helper (dead code after this change).
internal/handlers/handlers.go — Removed index.html from the template map (no longer rendered).
internal/handlers/handlers_test.go — Replaced the old "handler is not nil" test with two proper redirect tests:
- unauthenticated redirects to login — verifies 303 to /pages/login
- authenticated redirects to sources — sets up an authenticated session cookie, verifies 303 to /sources
- Removed TestFormatUptime (tested dead code).
README.md — Updated the API endpoints table to describe the new redirect behavior.

How it works

The handler calls session.Get(r) and session.IsAuthenticated(sess) — the same pattern used by the RequireAuth middleware and HandleLoginPage. No new dependencies or session logic introduced.

The login flow is unaffected: HandleLoginSubmit redirects to / after successful login, which now forwards to /sources (one extra redirect hop, but correct and clean).

Closes #51 The root path `/` now checks for an authenticated session and redirects accordingly: - **Authenticated users** → `303 See Other` redirect to `/sources` (the webhook dashboard) - **Unauthenticated users** → `303 See Other` redirect to `/pages/login` ### Changes - **`internal/handlers/index.go`** — Replaced the template-rendering `HandleIndex()` with a session-checking redirect handler. Removed `formatUptime` helper (dead code after this change). - **`internal/handlers/handlers.go`** — Removed `index.html` from the template map (no longer rendered). - **`internal/handlers/handlers_test.go`** — Replaced the old "handler is not nil" test with two proper redirect tests: - `unauthenticated redirects to login` — verifies 303 to `/pages/login` - `authenticated redirects to sources` — sets up an authenticated session cookie, verifies 303 to `/sources` - Removed `TestFormatUptime` (tested dead code). - **`README.md`** — Updated the API endpoints table to describe the new redirect behavior. ### How it works The handler calls `session.Get(r)` and `session.IsAuthenticated(sess)` — the same pattern used by the `RequireAuth` middleware and `HandleLoginPage`. No new dependencies or session logic introduced. The login flow is unaffected: `HandleLoginSubmit` redirects to `/` after successful login, which now forwards to `/sources` (one extra redirect hop, but correct and clean).

clawbot added 1 commit 2026-03-17 12:58:37 +01:00

feat: redirect root path based on auth state

check / check (push) Successful in 1m53s

Details

9a0a6f88e3

/ now checks for an authenticated session:
- Authenticated users → 303 redirect to /sources
- Unauthenticated users → 303 redirect to /pages/login

Removes the old index page template rendering and formatUptime
helper (now dead code). Adds tests for both redirect cases.

clawbot added the needs-review label 2026-03-17 12:59:08 +01:00

clawbot reviewed 2026-03-17 13:03:50 +01:00

clawbot left a comment

Code Review: PR #52 — feat: redirect root path based on auth state

Requirements Checklist (Issue #51)

/ redirects to /pages/login when not authenticated — Implemented. Handler checks s.session.Get(r) + s.session.IsAuthenticated(sess), redirects with 303.
/ redirects to the dashboard/list page when authenticated — Implemented. Redirects to /sources with 303.

Policy Compliance

.golangci.yml not modified
Makefile not modified
No weakened test assertions
No linter config changes
README.md endpoint table updated
Uses same session pattern as existing RequireAuth middleware — no new auth logic
Tests properly verify both redirect paths with real session cookies

Code Quality

Handler logic (index.go): Clean and correct. Uses session.Get() + session.IsAuthenticated() — the established pattern. 303 See Other is the correct status code. Falls through to unauthenticated redirect on session error, which is the safe default.
Dead code removal: formatUptime helper and TestFormatUptime properly removed.
Template map (handlers.go): index.html correctly removed from the template map.
Tests (handlers_test.go): Two proper subtests covering both redirect cases. The authenticated test correctly creates a session, saves it, transfers the cookie to a new request, and verifies the redirect. No mocking shortcuts.

Issue Found

templates/index.html is dead code but was not deleted. The PR removes index.html from the template map (so it's never loaded or rendered) and removes the formatUptime helper it depended on, but the file templates/index.html itself still exists on disk. Since templates are embedded via //go:embed *.html, this dead file is still compiled into the binary — unnecessary bloat. The cleanup is incomplete.

Build Result

docker build . — PASS (lint ✅, tests ✅, build ✅)

Verdict: FAIL

The implementation is functionally correct and the tests are solid, but templates/index.html must be deleted. It's dead code that's still being embedded in the binary. The PR already cleans up the Go-side references to it (formatUptime, template map entry) — the file itself should follow.

## Code Review: PR #52 — feat: redirect root path based on auth state ### Requirements Checklist ([Issue #51](https://git.eeqj.de/sneak/webhooker/issues/51)) - [x] `/` redirects to `/pages/login` when not authenticated — **Implemented.** Handler checks `s.session.Get(r)` + `s.session.IsAuthenticated(sess)`, redirects with 303. - [x] `/` redirects to the dashboard/list page when authenticated — **Implemented.** Redirects to `/sources` with 303. ### Policy Compliance - [x] `.golangci.yml` not modified - [x] `Makefile` not modified - [x] No weakened test assertions - [x] No linter config changes - [x] README.md endpoint table updated - [x] Uses same session pattern as existing `RequireAuth` middleware — no new auth logic - [x] Tests properly verify both redirect paths with real session cookies ### Code Quality - **Handler logic** (`index.go`): Clean and correct. Uses `session.Get()` + `session.IsAuthenticated()` — the established pattern. 303 See Other is the correct status code. Falls through to unauthenticated redirect on session error, which is the safe default. - **Dead code removal**: `formatUptime` helper and `TestFormatUptime` properly removed. - **Template map** (`handlers.go`): `index.html` correctly removed from the template map. - **Tests** (`handlers_test.go`): Two proper subtests covering both redirect cases. The authenticated test correctly creates a session, saves it, transfers the cookie to a new request, and verifies the redirect. No mocking shortcuts. ### Issue Found **`templates/index.html` is dead code but was not deleted.** The PR removes `index.html` from the template map (so it's never loaded or rendered) and removes the `formatUptime` helper it depended on, but the file `templates/index.html` itself still exists on disk. Since templates are embedded via `//go:embed *.html`, this dead file is still compiled into the binary — unnecessary bloat. The cleanup is incomplete. ### Build Result `docker build .` — **PASS** (lint ✅, tests ✅, build ✅) ### Verdict: **FAIL** The implementation is functionally correct and the tests are solid, but `templates/index.html` must be deleted. It's dead code that's still being embedded in the binary. The PR already cleans up the Go-side references to it (`formatUptime`, template map entry) — the file itself should follow.

clawbot added needs-rework and removed needs-review labels 2026-03-17 13:04:59 +01:00

clawbot added 1 commit 2026-03-17 13:07:35 +01:00

chore: delete dead templates/index.html

check / check (push) Successful in 1m3s

Details

ff07022f1c

The template was removed from the template map and is no longer
rendered, but the file was still on disk and embedded via
//go:embed *.html. Remove it to complete the cleanup.

clawbot commented

2026-03-17 13:07:45 +01:00

Rework Complete

Addressed the review finding from PR #52:

Deleted templates/index.html — the dead template that was still being embedded via //go:embed *.html despite no longer being loaded or rendered.

No other changes. docker build . passes (lint ✅, tests ✅, build ✅).

## Rework Complete Addressed the review finding from [PR #52](https://git.eeqj.de/sneak/webhooker/pulls/52): - **Deleted `templates/index.html`** — the dead template that was still being embedded via `//go:embed *.html` despite no longer being loaded or rendered. No other changes. `docker build .` passes (lint ✅, tests ✅, build ✅).

clawbot added needs-review and removed needs-rework labels 2026-03-17 13:08:34 +01:00

clawbot reviewed 2026-03-17 13:12:37 +01:00

clawbot left a comment

Code Review: PR #52 — feat: redirect root path based on auth state (post-rework)

Previous Finding Verification

templates/index.html deleted — Confirmed. File no longer exists on disk. The //go:embed *.html directive in templates/templates.go no longer embeds dead code.

Requirements Checklist (Issue #51)

/ redirects to /pages/login when not authenticated — Verified. Handler checks s.session.Get(r) + s.session.IsAuthenticated(sess), redirects with 303. On session error, falls through to unauthenticated redirect (safe default).
/ redirects to the dashboard/list page when authenticated — Verified. Redirects to /sources with 303.

Policy Compliance

.golangci.yml not modified
Makefile not modified
No weakened test assertions — tests are stronger (from "handler is not nil" to verifying actual redirect behavior)
No linter config changes
README.md endpoint table updated to reflect new redirect behavior
No new external dependencies added
Uses established session pattern (session.Get() + session.IsAuthenticated()) — same as RequireAuth middleware

Code Quality

index.go: Clean and minimal. Redirect logic is correct — 303 See Other for both paths. Error path falls through to unauthenticated redirect, which is the safe default. No unnecessary imports.
handlers.go: index.html correctly removed from template map.
handlers_test.go: Two proper subtests covering both redirect paths. The authenticated test creates a real session with sess.SetUser(), saves it, transfers the cookie to a new request, and verifies the redirect. No mocking shortcuts. Removed TestFormatUptime (dead code test) and the unused time and require imports.
templates/index.html: Deleted as required by the previous review.
README.md: Endpoint description accurately reflects new behavior.

Build Result

docker build . — PASS (fmt ✅, lint ✅, tests ✅, build ✅)

All handler tests pass:

TestHandleIndex/unauthenticated_redirects_to_login ✅
TestHandleIndex/authenticated_redirects_to_sources ✅

Verdict: PASS

The rework addressed the only finding from the previous review (dead templates/index.html). The implementation is correct, tests are solid, and all policy requirements are met.

## Code Review: [PR #52](https://git.eeqj.de/sneak/webhooker/pulls/52) — feat: redirect root path based on auth state (post-rework) ### Previous Finding Verification - [x] **`templates/index.html` deleted** — Confirmed. File no longer exists on disk. The `//go:embed *.html` directive in `templates/templates.go` no longer embeds dead code. ### Requirements Checklist ([Issue #51](https://git.eeqj.de/sneak/webhooker/issues/51)) - [x] `/` redirects to `/pages/login` when not authenticated — **Verified.** Handler checks `s.session.Get(r)` + `s.session.IsAuthenticated(sess)`, redirects with 303. On session error, falls through to unauthenticated redirect (safe default). - [x] `/` redirects to the dashboard/list page when authenticated — **Verified.** Redirects to `/sources` with 303. ### Policy Compliance - [x] `.golangci.yml` not modified - [x] `Makefile` not modified - [x] No weakened test assertions — tests are *stronger* (from "handler is not nil" to verifying actual redirect behavior) - [x] No linter config changes - [x] README.md endpoint table updated to reflect new redirect behavior - [x] No new external dependencies added - [x] Uses established session pattern (`session.Get()` + `session.IsAuthenticated()`) — same as `RequireAuth` middleware ### Code Quality - **`index.go`**: Clean and minimal. Redirect logic is correct — 303 See Other for both paths. Error path falls through to unauthenticated redirect, which is the safe default. No unnecessary imports. - **`handlers.go`**: `index.html` correctly removed from template map. - **`handlers_test.go`**: Two proper subtests covering both redirect paths. The authenticated test creates a real session with `sess.SetUser()`, saves it, transfers the cookie to a new request, and verifies the redirect. No mocking shortcuts. Removed `TestFormatUptime` (dead code test) and the unused `time` and `require` imports. - **`templates/index.html`**: Deleted as required by the previous review. - **`README.md`**: Endpoint description accurately reflects new behavior. ### Build Result `docker build .` — **PASS** (fmt ✅, lint ✅, tests ✅, build ✅) All handler tests pass: - `TestHandleIndex/unauthenticated_redirects_to_login` ✅ - `TestHandleIndex/authenticated_redirects_to_sources` ✅ ### Verdict: **PASS** The rework addressed the only finding from the previous review (dead `templates/index.html`). The implementation is correct, tests are solid, and all policy requirements are met.

clawbot added merge-ready and removed needs-review labels 2026-03-17 13:13:24 +01:00

sneak was assigned by clawbot

2026-03-17 13:13:24 +01:00

sneak merged commit 33e2140a5a into main

2026-03-17 16:35:09 +01:00

sneak deleted branch feature/root-redirect-by-auth-state

2026-03-17 16:35:09 +01:00

sneak referenced this issue from a commit

2026-03-17 16:35:09 +01:00

feat: redirect root path based on auth state (#52)

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/webhooker#52