# Authentication Redirect Loop Diagnosis - Phase 4

**Date**: 2025-11-18
**Status**: ROOT CAUSE IDENTIFIED
**Severity**: Critical - Blocking manual testing

## Executive Summary

The Phase 4 development authentication is experiencing a redirect loop between `/dev/login` and `/admin/`. The session cookie is being set correctly, but Flask's server-side session storage is failing, preventing the `@require_auth` decorator from storing the redirect URL properly.

**Root Cause**: Misuse of Flask's `session` object in the `require_auth` decorator without proper initialization.

## Problem Description

### User Experience
1. User clicks dev login at `/dev/login`
2. Browser redirects to `/admin/` (302)
3. Browser redirects back to `/admin/login` (302)
4. User lands on login page, unauthenticated

### Server Logs
```
[2025-11-18 21:55:03] WARNING in dev_auth: DEV MODE: Creating session for https://dev.example.com WITHOUT authentication.
[2025-11-18 21:55:03] INFO in auth: Session created for https://dev.example.com
127.0.0.1 - - [18/Nov/2025 21:55:03] "GET /dev/login HTTP/1.1" 302 -
127.0.0.1 - - [18/Nov/2025 21:55:03] "GET /admin/ HTTP/1.1" 302 -
127.0.0.1 - - [18/Nov/2025 21:55:03] "GET /admin/login HTTP/1.1" 200 -
```

## Root Cause Analysis

### The Critical Issue

In `starpunk/auth.py`, line 397, the `require_auth` decorator attempts to use Flask's server-side session:

```python
@wraps(f)
def decorated_function(*args, **kwargs):
    # Get session token from cookie
    session_token = request.cookies.get("session")

    # Verify session
    session_info = verify_session(session_token)

    if not session_info:
        # Store intended destination
        session["next"] = request.url  # ← THIS IS THE PROBLEM
        return redirect(url_for("auth.login_form"))
```

### Why This Causes the Redirect Loop

1. **Session Cookie Name Collision**:
   - Flask's server-side session uses a cookie named `session` by default
   - StarPunk's authentication uses a cookie named `session` for the session token
   - These are TWO DIFFERENT things being stored under the same name

2. **What Actually Happens**:
   - `/dev/login` sets `session` cookie with the authentication token (e.g., `"xyz123abc456..."`)
   - Browser sends this cookie to `/admin/`
   - `@require_auth` reads `request.cookies.get("session")` → Gets the auth token (correct)
   - `verify_session()` validates the token → Returns valid session info (correct)
   - BUT: If there's ANY code path that triggers Flask session access elsewhere, Flask tries to deserialize the auth token as a Flask session object
   - When `require_auth` tries to write `session["next"] = request.url`, Flask overwrites the `session` cookie with its own signed session data
   - On the next request, the auth token is gone, replaced by Flask session data
   - `verify_session()` fails because the cookie now contains Flask session JSON, not an auth token
   - User is redirected back to login

3. **The Timing Issue**:
   - The redirect happens so fast that the browser sees:
     1. Cookie set to auth token
     2. Redirect to `/admin/`
     3. Flask session middleware processes the request
     4. Cookie gets overwritten with Flask session data
     5. Auth check fails
     6. Redirect to `/admin/login`

### Secondary Issue: Flash Messages

The dev login route also uses `flash()` which relies on Flask's session:

```python
flash("DEV MODE: Logged in without authentication", "warning")
```

When `flash()` is called, Flask writes to the server-side session, which triggers the cookie overwrite.

## Why This Wasn't Caught Earlier

1. **Production IndieAuth Flow**: The production flow doesn't use `flash()` or `session["next"]` in the same request cycle as setting the auth cookie
2. **Test Coverage Gap**: Tests likely mock the session or don't test the full HTTP request/response cycle
3. **Cookie Name Collision**: Using `session` for both Flask's session and StarPunk's auth token is architecturally unsound

## The Fix

### Option 1: Rename StarPunk Session Cookie (RECOMMENDED)

**Rationale**: Flask owns the `session` cookie name. We should not conflict with framework conventions.

**Changes Required**:

#### 1. Update `starpunk/routes/dev_auth.py` (Line 74-81)

**Old Code**:
```python
response.set_cookie(
    "session",
    session_token,
    httponly=True,
    secure=False,
    samesite="Lax",
    max_age=30 * 24 * 60 * 60,
)
```

**New Code**:
```python
response.set_cookie(
    "starpunk_session",  # ← Changed from "session"
    session_token,
    httponly=True,
    secure=False,
    samesite="Lax",
    max_age=30 * 24 * 60 * 60,
)
```

#### 2. Update `starpunk/auth.py` (Line 390)

**Old Code**:
```python
session_token = request.cookies.get("session")
```

**New Code**:
```python
session_token = request.cookies.get("starpunk_session")  # ← Changed from "session"
```

#### 3. Update `starpunk/routes/auth.py` (IndieAuth callback)

Find where the session cookie is set after IndieAuth callback (likely similar to dev_auth) and change the cookie name there as well.

**Search for**: `response.set_cookie("session"`
**Replace with**: `response.set_cookie("starpunk_session"`

#### 4. Update logout route to clear correct cookie

Find the logout route and ensure it clears `starpunk_session` instead of `session`.

### Option 2: Disable Flask Session (NOT RECOMMENDED)

We could disable Flask's session entirely by not setting `SECRET_KEY`, but this would:
- Break `flash()` messages
- Break `session["next"]` redirect tracking
- Require rewriting all flash message functionality

This adds complexity without benefit.

### Option 3: Use Query Parameter for Redirect (PARTIAL FIX)

Instead of `session["next"]`, use a query parameter:

```python
return redirect(url_for("auth.login_form", next=request.url))
```

This fixes the immediate issue but doesn't resolve the cookie name collision, which will cause problems elsewhere.

## Recommended Solution: Option 1

**Why**:
- Minimal code changes (4 locations)
- Follows Flask conventions (Flask owns `session`)
- Preserves all existing functionality
- Clear separation of concerns
- No security implications

**Implementation Steps**:
1. Search codebase for all instances of `"session"` cookie usage
2. Replace with `"starpunk_session"`
3. Update any logout functionality
4. Update any session validation code
5. Test full auth flow (dev and production)

## Files Requiring Changes

1. `/home/phil/Projects/starpunk/starpunk/routes/dev_auth.py` - Line 75
2. `/home/phil/Projects/starpunk/starpunk/auth.py` - Line 390
3. `/home/phil/Projects/starpunk/starpunk/routes/auth.py` - Find callback route cookie setting
4. `/home/phil/Projects/starpunk/starpunk/routes/auth.py` - Find logout route cookie clearing

## Testing Approach

### Manual Test Plan

1. **Dev Login Flow**:
   ```
   1. Visit http://localhost:5000/admin/
   2. Verify redirect to /admin/login
   3. Click dev login link
   4. Verify redirect to /admin/
   5. Verify dashboard loads (no redirect loop)
   6. Verify flash message appears
   7. Check browser DevTools → Application → Cookies
   8. Verify "starpunk_session" cookie exists with token value
   9. Verify "session" cookie exists with Flask session data (if flash used)
   ```

2. **Session Persistence**:
   ```
   1. After successful login, visit /admin/new
   2. Verify authentication persists
   3. Refresh page
   4. Verify still authenticated
   ```

3. **Logout**:
   ```
   1. While authenticated, click logout
   2. Verify redirect to login
   3. Verify "starpunk_session" cookie is cleared
   4. Try to visit /admin/
   5. Verify redirect to /admin/login
   ```

### Automated Test Requirements

Add tests for:
- Cookie name verification
- Session persistence across requests
- Flash message functionality with auth
- Redirect loop prevention

## Security Implications

**None**: This change is purely architectural cleanup. Both cookie names are:
- HttpOnly (prevents JavaScript access)
- SameSite=Lax (CSRF protection)
- Same security properties

The separation actually improves security by:
- Clear separation of concerns
- Easier to audit (two distinct cookies)
- Follows framework conventions

## Architecture Decision

This issue reveals a broader architectural concern: **Cookie Naming Strategy**.

### New Standard: Cookie Naming Convention

**Rule**: Never use generic names that conflict with framework conventions.

**StarPunk Cookie Names**:
- `starpunk_session` - Authentication session token
- `session` - Reserved for Flask framework use
- Future cookies should use `starpunk_*` prefix

**Document in**: `/docs/standards/api-design.md` under "Cookie Standards"

## Prevention

### Code Review Checklist Addition

Add to code review standards:
- [ ] No custom cookies named `session`, `csrf_token`, or other framework-reserved names
- [ ] All StarPunk cookies use `starpunk_` prefix
- [ ] Cookie security attributes verified (HttpOnly, Secure, SameSite)

### Configuration Validation

Consider adding startup validation:
```python
# In config.py validate_config()
if app.config.get("SESSION_COOKIE_NAME") == "session":
    app.logger.warning(
        "Using default Flask session cookie name. "
        "StarPunk auth uses 'starpunk_session' to avoid conflicts."
    )
```

## Timeline

**Estimated Fix Time**: 30 minutes
- 10 min: Search and replace cookie names
- 10 min: Manual testing
- 10 min: Update changelog and version

**Priority**: CRITICAL - Blocking Phase 4 manual testing

## Next Steps for Developer

1. Read this document completely
2. Search codebase for all `"session"` cookie references
3. Implement Option 1 changes systematically
4. Run manual test plan
5. Update `/docs/standards/api-design.md` with cookie naming convention
6. Update changelog
7. Increment version to 0.5.1 (bugfix)
8. Create git commit with proper message

## References

- Flask Documentation: https://flask.palletsprojects.com/en/3.0.x/api/#flask.session
- Cookie Security: https://owasp.org/www-community/controls/SecureFlag
- IndieWeb Session Spec: https://indieweb.org/session