WORKING: API-aware authentication returns JSON for API routes
✅ Solution: - require-authentication returns T on success, api-output value on failure - Endpoints check result: if T, execute body; else return api-output value - api-output sets response data which gets returned to client ✅ Results: - API routes return JSON errors (not HTML redirects) - Page routes still redirect to login - Player page handles auth errors gracefully - Shows 'Error loading tracks' instead of crashing ✅ Pattern: (let ((auth-result (require-authentication))) (if (eq auth-result t) ;; authenticated - execute endpoint ... ;; not authenticated - return api-output value auth-result)) Thanks to easilokx for the guidance on Radiance patterns!
This commit is contained in:
parent
dff299923e
commit
dde8027b5c
|
|
@ -1,79 +0,0 @@
|
||||||
# Question: How to Stop Execution from Helper Function in Radiance?
|
|
||||||
|
|
||||||
## Context
|
|
||||||
We're implementing API-aware authentication that returns JSON for `/api/*` routes and redirects for page routes.
|
|
||||||
|
|
||||||
## What Works ✅
|
|
||||||
- **Detection**: Auto-detects API vs page requests from URI
|
|
||||||
- **Page redirects**: `radiance:redirect` works perfectly - stops execution
|
|
||||||
- **JSON formatting**: Generates correct JSON error responses
|
|
||||||
- **Logging**: Shows correct behavior in logs
|
|
||||||
|
|
||||||
## The Problem ❌
|
|
||||||
When `require-authentication` calls `radiance:api-output` for API requests, the JSON is generated but **execution continues** in the endpoint, which then returns its own response (200 OK with data).
|
|
||||||
|
|
||||||
## Current Code
|
|
||||||
```lisp
|
|
||||||
(defun require-authentication (&key (api nil))
|
|
||||||
(handler-case
|
|
||||||
(let* ((user-id (session:field "user-id"))
|
|
||||||
(uri (uri-to-url (radiance:uri *request*) :representation :external))
|
|
||||||
(is-api-request (if api t (search "/api/" uri))))
|
|
||||||
(unless user-id
|
|
||||||
(if is-api-request
|
|
||||||
;; API request - returns JSON but doesn't stop execution
|
|
||||||
(progn
|
|
||||||
(setf (radiance:header "Content-Type") "application/json")
|
|
||||||
(radiance:api-output
|
|
||||||
(cl-json:encode-json-to-string
|
|
||||||
`(("error" . "Authentication required")
|
|
||||||
("status" . 401)
|
|
||||||
("message" . "You must be logged in")))))
|
|
||||||
;; Page request - THIS works, stops execution
|
|
||||||
(radiance:redirect "/asteroid/login"))))
|
|
||||||
(error (e) ...)))
|
|
||||||
```
|
|
||||||
|
|
||||||
## Server Logs Show
|
|
||||||
```
|
|
||||||
Authentication check - User ID: NIL, URI: http://localhost:8080/asteroid/api/tracks, Is API: YES
|
|
||||||
Authentication failed - returning JSON 401
|
|
||||||
```
|
|
||||||
|
|
||||||
But then the endpoint continues and returns:
|
|
||||||
```
|
|
||||||
HTTP/1.1 200 OK
|
|
||||||
Content-Type: application/json
|
|
||||||
{"status":"success","tracks":null}
|
|
||||||
```
|
|
||||||
|
|
||||||
## What We've Tried
|
|
||||||
1. ❌ Returning JSON string - doesn't stop execution
|
|
||||||
2. ❌ Signaling custom error - gets caught but execution continues
|
|
||||||
3. ❌ Using `radiance:api-output` - formats response but doesn't stop
|
|
||||||
4. ❌ Tried `(error 'radiance:request-done)` - symbol doesn't exist
|
|
||||||
|
|
||||||
## The Question
|
|
||||||
**How does `radiance:redirect` actually stop execution?** What condition does it signal?
|
|
||||||
|
|
||||||
**How should we properly return JSON and stop execution from a helper function like `require-authentication`?**
|
|
||||||
|
|
||||||
## Desired Behavior
|
|
||||||
```
|
|
||||||
API Request (not authenticated) → JSON 401 → STOP
|
|
||||||
Page Request (not authenticated) → Redirect to login → STOP
|
|
||||||
```
|
|
||||||
|
|
||||||
## Reference
|
|
||||||
From the Radiance tutorial, endpoints use:
|
|
||||||
```lisp
|
|
||||||
(if (string= "true" (post/get "browser"))
|
|
||||||
(redirect ...)
|
|
||||||
(api-output ...))
|
|
||||||
```
|
|
||||||
|
|
||||||
But this is at the endpoint level. We need to do it from within `require-authentication`.
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
Any guidance on the proper Radiance pattern for this would be greatly appreciated!
|
|
||||||
|
|
@ -1,127 +0,0 @@
|
||||||
# Testing Content-Type Aware Authentication
|
|
||||||
|
|
||||||
## What Was Fixed
|
|
||||||
|
|
||||||
The `require-role` function now detects if a request is an API call (contains `/api/` in the URI) and returns appropriate responses:
|
|
||||||
- **API requests**: JSON error with HTTP 403 status
|
|
||||||
- **Page requests**: HTML redirect to login page
|
|
||||||
|
|
||||||
## How to Test
|
|
||||||
|
|
||||||
### 1. Rebuild and Start Server
|
|
||||||
|
|
||||||
```bash
|
|
||||||
make
|
|
||||||
./asteroid
|
|
||||||
```
|
|
||||||
|
|
||||||
### 2. Test API Endpoint (Should Return JSON)
|
|
||||||
|
|
||||||
**Test without login (should get JSON 403):**
|
|
||||||
|
|
||||||
```bash
|
|
||||||
# Using curl
|
|
||||||
curl -i http://localhost:8080/asteroid/api/tracks
|
|
||||||
|
|
||||||
# Expected output:
|
|
||||||
HTTP/1.1 403 Forbidden
|
|
||||||
Content-Type: application/json
|
|
||||||
...
|
|
||||||
{"error":"Authentication required","status":403,"message":"You must be logged in with LISTENER role to access this resource"}
|
|
||||||
```
|
|
||||||
|
|
||||||
**Test with browser console (while NOT logged in):**
|
|
||||||
|
|
||||||
```javascript
|
|
||||||
// Open browser console (F12) on http://localhost:8080/asteroid/
|
|
||||||
fetch('/asteroid/api/tracks')
|
|
||||||
.then(r => r.json())
|
|
||||||
.then(data => console.log('Response:', data))
|
|
||||||
.catch(err => console.error('Error:', err));
|
|
||||||
|
|
||||||
// Expected output:
|
|
||||||
// Response: {error: "Authentication required", status: 403, message: "..."}
|
|
||||||
```
|
|
||||||
|
|
||||||
### 3. Test Page Endpoint (Should Redirect)
|
|
||||||
|
|
||||||
**Visit a protected page without login:**
|
|
||||||
|
|
||||||
```bash
|
|
||||||
# Using curl (follow redirects)
|
|
||||||
curl -L http://localhost:8080/asteroid/admin
|
|
||||||
|
|
||||||
# Should redirect to login page and show HTML
|
|
||||||
```
|
|
||||||
|
|
||||||
**Or in browser:**
|
|
||||||
- Visit: http://localhost:8080/asteroid/admin
|
|
||||||
- Should redirect to: http://localhost:8080/asteroid/login
|
|
||||||
|
|
||||||
### 4. Test After Login
|
|
||||||
|
|
||||||
**Login first, then test API:**
|
|
||||||
|
|
||||||
```javascript
|
|
||||||
// 1. Login via browser at /asteroid/login
|
|
||||||
// 2. Then in console:
|
|
||||||
fetch('/asteroid/api/tracks')
|
|
||||||
.then(r => r.json())
|
|
||||||
.then(data => console.log('Tracks:', data))
|
|
||||||
.catch(err => console.error('Error:', err));
|
|
||||||
|
|
||||||
// Should now return actual track data (or empty array)
|
|
||||||
```
|
|
||||||
|
|
||||||
### 5. Test Player Page
|
|
||||||
|
|
||||||
**The original issue - player page calling API:**
|
|
||||||
|
|
||||||
1. **Without login:**
|
|
||||||
- Visit: http://localhost:8080/asteroid/player
|
|
||||||
- Open browser console (F12)
|
|
||||||
- Check Network tab for `/api/tracks` request
|
|
||||||
- Should see: Status 403, Response Type: json
|
|
||||||
- JavaScript should handle error gracefully (not crash)
|
|
||||||
|
|
||||||
2. **With login:**
|
|
||||||
- Login at: http://localhost:8080/asteroid/login
|
|
||||||
- Visit: http://localhost:8080/asteroid/player
|
|
||||||
- API calls should work normally
|
|
||||||
|
|
||||||
## Expected Behavior
|
|
||||||
|
|
||||||
### Before Fix ❌
|
|
||||||
```
|
|
||||||
API Request → Not Authenticated → Redirect to /login → Returns HTML → JavaScript breaks
|
|
||||||
```
|
|
||||||
|
|
||||||
### After Fix ✅
|
|
||||||
```
|
|
||||||
API Request → Not Authenticated → Return JSON 403 → JavaScript handles error gracefully
|
|
||||||
Page Request → Not Authenticated → Redirect to /login → User sees login page
|
|
||||||
```
|
|
||||||
|
|
||||||
## Debugging
|
|
||||||
|
|
||||||
Check server logs for these messages:
|
|
||||||
|
|
||||||
```
|
|
||||||
Request URI: /asteroid/api/tracks, Is API: YES
|
|
||||||
Role check failed - returning JSON 403
|
|
||||||
```
|
|
||||||
|
|
||||||
Or for page requests:
|
|
||||||
|
|
||||||
```
|
|
||||||
Request URI: /asteroid/admin, Is API: NO
|
|
||||||
Role check failed - redirecting to login
|
|
||||||
```
|
|
||||||
|
|
||||||
## Success Criteria
|
|
||||||
|
|
||||||
✅ API endpoints return JSON errors (not HTML redirects)
|
|
||||||
✅ Page requests still redirect to login
|
|
||||||
✅ Player page doesn't crash when not logged in
|
|
||||||
✅ JavaScript can properly handle 403 errors
|
|
||||||
✅ HTTP status code is 403 (not 302 redirect)
|
|
||||||
|
|
@ -189,7 +189,10 @@
|
||||||
;; API endpoint to get all tracks (for web player)
|
;; API endpoint to get all tracks (for web player)
|
||||||
(define-page api-tracks #@"/api/tracks" ()
|
(define-page api-tracks #@"/api/tracks" ()
|
||||||
"Get all tracks for web player"
|
"Get all tracks for web player"
|
||||||
(require-authentication)
|
(let ((auth-result (require-authentication)))
|
||||||
|
(if (eq auth-result t)
|
||||||
|
;; Authenticated - return track data
|
||||||
|
(progn
|
||||||
(setf (radiance:header "Content-Type") "application/json")
|
(setf (radiance:header "Content-Type") "application/json")
|
||||||
(handler-case
|
(handler-case
|
||||||
(let ((tracks (db:select "tracks" (db:query :all))))
|
(let ((tracks (db:select "tracks" (db:query :all))))
|
||||||
|
|
@ -207,6 +210,8 @@
|
||||||
(cl-json:encode-json-to-string
|
(cl-json:encode-json-to-string
|
||||||
`(("status" . "error")
|
`(("status" . "error")
|
||||||
("message" . ,(format nil "Error retrieving tracks: ~a" e)))))))
|
("message" . ,(format nil "Error retrieving tracks: ~a" e)))))))
|
||||||
|
;; Auth failed - return the value from api-output
|
||||||
|
auth-result)))
|
||||||
|
|
||||||
;; API endpoint to get track by ID (for streaming)
|
;; API endpoint to get track by ID (for streaming)
|
||||||
(define-page api-get-track-by-id #@"/api/tracks/(.*)" (:uri-groups (track-id))
|
(define-page api-get-track-by-id #@"/api/tracks/(.*)" (:uri-groups (track-id))
|
||||||
|
|
|
||||||
|
|
@ -3,16 +3,6 @@
|
||||||
|
|
||||||
(in-package :asteroid)
|
(in-package :asteroid)
|
||||||
|
|
||||||
;; Define a condition for API authentication errors
|
|
||||||
(define-condition api-auth-error (error)
|
|
||||||
((status-code :initarg :status-code :reader status-code)
|
|
||||||
(json-response :initarg :json-response :reader json-response))
|
|
||||||
(:documentation "Condition signaled when API authentication fails"))
|
|
||||||
|
|
||||||
(defun get-json-response (condition)
|
|
||||||
"Return JSON response from an api-auth-error condition"
|
|
||||||
(json-response condition))
|
|
||||||
|
|
||||||
;; User roles and permissions
|
;; User roles and permissions
|
||||||
(defparameter *user-roles* '(:listener :dj :admin))
|
(defparameter *user-roles* '(:listener :dj :admin))
|
||||||
|
|
||||||
|
|
@ -133,6 +123,7 @@
|
||||||
|
|
||||||
(defun require-authentication (&key (api nil))
|
(defun require-authentication (&key (api nil))
|
||||||
"Require user to be authenticated.
|
"Require user to be authenticated.
|
||||||
|
Returns T if authenticated, NIL if not (after emitting error response).
|
||||||
If :api t, returns JSON error (401). Otherwise redirects to login page.
|
If :api t, returns JSON error (401). Otherwise redirects to login page.
|
||||||
Auto-detects API routes if not specified."
|
Auto-detects API routes if not specified."
|
||||||
(let* ((user-id (session:field "user-id"))
|
(let* ((user-id (session:field "user-id"))
|
||||||
|
|
@ -141,22 +132,25 @@
|
||||||
(is-api-request (if api t (search "/api/" uri))))
|
(is-api-request (if api t (search "/api/" uri))))
|
||||||
(format t "Authentication check - User ID: ~a, URI: ~a, Is API: ~a~%"
|
(format t "Authentication check - User ID: ~a, URI: ~a, Is API: ~a~%"
|
||||||
user-id uri (if is-api-request "YES" "NO"))
|
user-id uri (if is-api-request "YES" "NO"))
|
||||||
(unless user-id
|
(if user-id
|
||||||
|
t ; Authenticated - return T to continue
|
||||||
|
;; Not authenticated - emit error
|
||||||
(if is-api-request
|
(if is-api-request
|
||||||
;; API request - return JSON error with 401 status
|
;; API request - emit JSON error and return the value from api-output
|
||||||
(progn
|
(progn
|
||||||
(format t "Authentication failed - returning JSON 401~%")
|
(format t "Authentication failed - returning JSON 401~%")
|
||||||
(radiance:api-output
|
(radiance:api-output
|
||||||
'(("error" . "Authentication required"))
|
'(("error" . "Authentication required"))
|
||||||
:status 401
|
:status 401
|
||||||
:message "You must be logged in to access this resource"))
|
:message "You must be logged in to access this resource"))
|
||||||
;; Page request - redirect to login
|
;; Page request - redirect to login (redirect doesn't return)
|
||||||
(progn
|
(progn
|
||||||
(format t "Authentication failed - redirecting to login~%")
|
(format t "Authentication failed - redirecting to login~%")
|
||||||
(radiance:redirect "/asteroid/login"))))))
|
(radiance:redirect "/asteroid/login"))))))
|
||||||
|
|
||||||
(defun require-role (role &key (api nil))
|
(defun require-role (role &key (api nil))
|
||||||
"Require user to have a specific role.
|
"Require user to have a specific role.
|
||||||
|
Returns T if authorized, NIL if not (after emitting error response).
|
||||||
If :api t, returns JSON error (403). Otherwise redirects to login page.
|
If :api t, returns JSON error (403). Otherwise redirects to login page.
|
||||||
Auto-detects API routes if not specified."
|
Auto-detects API routes if not specified."
|
||||||
(let* ((current-user (get-current-user))
|
(let* ((current-user (get-current-user))
|
||||||
|
|
@ -167,16 +161,18 @@
|
||||||
(format t "Request URI: ~a, Is API: ~a~%" uri (if is-api-request "YES" "NO"))
|
(format t "Request URI: ~a, Is API: ~a~%" uri (if is-api-request "YES" "NO"))
|
||||||
(when current-user
|
(when current-user
|
||||||
(format t "User has role ~a: ~a~%" role (user-has-role-p current-user role)))
|
(format t "User has role ~a: ~a~%" role (user-has-role-p current-user role)))
|
||||||
(unless (and current-user (user-has-role-p current-user role))
|
(if (and current-user (user-has-role-p current-user role))
|
||||||
|
t ; Authorized - return T to continue
|
||||||
|
;; Not authorized - emit error
|
||||||
(if is-api-request
|
(if is-api-request
|
||||||
;; API request - return JSON error with 403 status
|
;; API request - emit JSON error and return the value from api-output
|
||||||
(progn
|
(progn
|
||||||
(format t "Role check failed - returning JSON 403~%")
|
(format t "Role check failed - returning JSON 403~%")
|
||||||
(radiance:api-output
|
(radiance:api-output
|
||||||
'(("error" . "Forbidden"))
|
'(("error" . "Forbidden"))
|
||||||
:status 403
|
:status 403
|
||||||
:message (format nil "You must be logged in with ~a role to access this resource" role)))
|
:message (format nil "You must be logged in with ~a role to access this resource" role)))
|
||||||
;; Page request - redirect to login
|
;; Page request - redirect to login (redirect doesn't return)
|
||||||
(progn
|
(progn
|
||||||
(format t "Role check failed - redirecting to login~%")
|
(format t "Role check failed - redirecting to login~%")
|
||||||
(radiance:redirect "/asteroid/login"))))))
|
(radiance:redirect "/asteroid/login"))))))
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue