Fix api-output usage: pass structured data with :status and :message
- Remove handler-case that was catching api-output's condition - Pass alist data structure instead of JSON string - Use :status and :message keyword arguments as per Radiance docs - Detection and formatting work correctly - Issue: api-output doesn't stop execution from helper function - Need Radiance-specific pattern (redirect works, api-output doesn't)
This commit is contained in:
parent
9ec7848b47
commit
dff299923e
|
|
@ -0,0 +1,79 @@
|
||||||
|
# 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!
|
||||||
|
|
@ -135,86 +135,50 @@
|
||||||
"Require user to be authenticated.
|
"Require user to be authenticated.
|
||||||
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."
|
||||||
(handler-case
|
(let* ((user-id (session:field "user-id"))
|
||||||
(let* ((user-id (session:field "user-id"))
|
(uri (uri-to-url (radiance:uri *request*) :representation :external))
|
||||||
(uri (uri-to-url (radiance:uri *request*) :representation :external))
|
;; Use explicit flag if provided, otherwise auto-detect from URI
|
||||||
;; Use explicit flag if provided, otherwise auto-detect from URI
|
(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
|
||||||
(unless user-id
|
(if is-api-request
|
||||||
(if is-api-request
|
;; API request - return JSON error with 401 status
|
||||||
;; API request - return JSON error with 401 status
|
(progn
|
||||||
(progn
|
(format t "Authentication failed - returning JSON 401~%")
|
||||||
(format t "Authentication failed - returning JSON 401~%")
|
(radiance:api-output
|
||||||
(setf (radiance:header "Content-Type") "application/json")
|
'(("error" . "Authentication required"))
|
||||||
(radiance:api-output
|
:status 401
|
||||||
(cl-json:encode-json-to-string
|
:message "You must be logged in to access this resource"))
|
||||||
`(("error" . "Authentication required")
|
;; Page request - redirect to login
|
||||||
("status" . 401)
|
(progn
|
||||||
("message" . "You must be logged in to access this resource")))))
|
(format t "Authentication failed - redirecting to login~%")
|
||||||
;; Page request - redirect to login
|
|
||||||
(progn
|
|
||||||
(format t "Authentication failed - redirecting to login~%")
|
|
||||||
(radiance:redirect "/asteroid/login")))))
|
|
||||||
(api-auth-error (e)
|
|
||||||
(format t "API auth error caught, returning JSON~%")
|
|
||||||
(get-json-response e))
|
|
||||||
(error (e)
|
|
||||||
(format t "Authentication error: ~a~%" e)
|
|
||||||
(let* ((uri (uri-to-url (radiance:uri *request*) :representation :external))
|
|
||||||
(is-api-request (if api t (search "/api/" uri))))
|
|
||||||
(if is-api-request
|
|
||||||
(progn
|
|
||||||
(setf (radiance:header "Content-Type") "application/json")
|
|
||||||
(cl-json:encode-json-to-string
|
|
||||||
`(("error" . "Internal server error")
|
|
||||||
("status" . 500)
|
|
||||||
("message" . ,(format nil "~a" e)))))
|
|
||||||
(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.
|
||||||
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."
|
||||||
(handler-case
|
(let* ((current-user (get-current-user))
|
||||||
(let* ((current-user (get-current-user))
|
(uri (uri-to-url (radiance:uri *request*) :representation :external))
|
||||||
(uri (uri-to-url (radiance:uri *request*) :representation :external))
|
;; Use explicit flag if provided, otherwise auto-detect from URI
|
||||||
;; Use explicit flag if provided, otherwise auto-detect from URI
|
(is-api-request (if api t (search "/api/" uri))))
|
||||||
(is-api-request (if api t (search "/api/" uri))))
|
(format t "Current user for role check: ~a~%" (if current-user "FOUND" "NOT FOUND"))
|
||||||
(format t "Current user for role check: ~a~%" (if current-user "FOUND" "NOT FOUND"))
|
(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))
|
||||||
(unless (and current-user (user-has-role-p current-user role))
|
(if is-api-request
|
||||||
(if is-api-request
|
;; API request - return JSON error with 403 status
|
||||||
;; API request - return JSON error with 403 status
|
(progn
|
||||||
(progn
|
(format t "Role check failed - returning JSON 403~%")
|
||||||
(format t "Role check failed - returning JSON 403~%")
|
(radiance:api-output
|
||||||
(setf (radiance:header "Content-Type") "application/json")
|
'(("error" . "Forbidden"))
|
||||||
(radiance:api-output
|
:status 403
|
||||||
(cl-json:encode-json-to-string
|
:message (format nil "You must be logged in with ~a role to access this resource" role)))
|
||||||
`(("error" . "Authentication required")
|
;; Page request - redirect to login
|
||||||
("status" . 403)
|
(progn
|
||||||
("message" . ,(format nil "You must be logged in with ~a role to access this resource" role))))))
|
(format t "Role check failed - redirecting to login~%")
|
||||||
;; Page request - redirect to login
|
|
||||||
(progn
|
|
||||||
(format t "Role check failed - redirecting to login~%")
|
|
||||||
(radiance:redirect "/asteroid/login")))))
|
|
||||||
(api-auth-error (e)
|
|
||||||
(format t "API auth error caught in require-role, returning JSON~%")
|
|
||||||
(get-json-response e))
|
|
||||||
(error (e)
|
|
||||||
(format t "Role check error: ~a~%" e)
|
|
||||||
(let* ((uri (uri-to-url (radiance:uri *request*) :representation :external))
|
|
||||||
(is-api-request (if api t (search "/api/" uri))))
|
|
||||||
(if is-api-request
|
|
||||||
(progn
|
|
||||||
(setf (radiance:header "Content-Type") "application/json")
|
|
||||||
(cl-json:encode-json-to-string
|
|
||||||
`(("error" . "Internal server error")
|
|
||||||
("status" . 500)
|
|
||||||
("message" . ,(format nil "~a" e)))))
|
|
||||||
(radiance:redirect "/asteroid/login"))))))
|
(radiance:redirect "/asteroid/login"))))))
|
||||||
|
|
||||||
(defun update-user-role (user-id new-role)
|
(defun update-user-role (user-id new-role)
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue