Merge pull request #269 from readur/oidc-improvements

Fix(OIDC): redirect to frontend, jwt, and callback handling

docs/oidc-setup.md

@@ -56,7 +56,7 @@ Configure OIDC by setting these environment variables:
 | `OIDC_CLIENT_SECRET` | ✅ | OAuth2 client secret from your provider | `very-secret-key` |
 | `OIDC_ISSUER_URL` | ✅ | OIDC provider's issuer URL | `https://accounts.google.com` |
 | `OIDC_REDIRECT_URI` | ✅ | Callback URL for your Readur instance | `https://readur.company.com/api/auth/oidc/callback` |
-| `OIDC_AUTO_REGISTER` | ❌ | Allow new users to self-register (default: `true`) | `true` or `false` |
+| `OIDC_AUTO_REGISTER` | ❌ | Allow new users to self-register (default: `false`) | `true` or `false` |
 | `ALLOW_LOCAL_AUTH` | ❌ | Allow username/password authentication (default: `true`) | `true` or `false` |
 
 ### Example Configurations
@@ -476,19 +476,19 @@ Readur intelligently handles existing local users when they first log in via OID
 
 The `OIDC_AUTO_REGISTER` setting controls whether new users can self-register:
 
-**When `OIDC_AUTO_REGISTER=true` (default)**:
+**When `OIDC_AUTO_REGISTER=true`**:
 - New OIDC users are automatically created when they first log in
 - Perfect for open environments where any company employee should get access
 - Username is derived from OIDC claims (preferred_username or email)
 - Users get the default "user" role
 
-**When `OIDC_AUTO_REGISTER=false`**:
+**When `OIDC_AUTO_REGISTER=false` (default)**:
 - Only existing users (pre-created by admin or linked by email) can log in
 - OIDC login attempts by unregistered users are rejected with HTTP 403
 - Ideal for production environments requiring controlled access
 - Admin must pre-create users before they can use OIDC
 
-**Migration Strategy**: Set to `false` initially, have existing users log in to link accounts, then enable for new users.
+**Migration Strategy**: The default (`false`) is ideal for production. Have existing users log in to link accounts by email, then optionally enable `true` for new user auto-registration.
 
 ### Disabling Local Authentication
 

frontend/src/App.tsx

@@ -58,7 +58,7 @@ function App(): React.ReactElement {
       <CssBaseline />
       <Routes>
         <Route path="/login" element={!user ? <Login /> : <Navigate to="/dashboard" />} />
-        <Route path="/auth/oidc/callback" element={<OidcCallback />} />
+        <Route path="/auth/callback" element={<OidcCallback />} />
         <Route
           path="/*"
           element={

frontend/src/components/Auth/OidcCallback.tsx

@@ -14,9 +14,8 @@ const OidcCallback: React.FC = () => {
   useEffect(() => {
     const handleCallback = async () => {
       try {
-        const code = searchParams.get('code');
+        const token = searchParams.get('token');
         const error = searchParams.get('error');
-        const state = searchParams.get('state');
 
         if (error) {
           setError(`Authentication failed: ${error}`);
@@ -24,26 +23,18 @@ const OidcCallback: React.FC = () => {
           return;
         }
 
-        if (!code) {
+        if (!token) {
-          setError('No authorization code received');
+          setError('No authentication token received from server');
           setProcessing(false);
           return;
         }
 
-        // Call the backend OIDC callback endpoint
+        // Store the token and set up API authorization
-        const response = await api.get(`/auth/oidc/callback?code=${code}&state=${state || ''}`);
+        localStorage.setItem('token', token);
+        api.defaults.headers.common['Authorization'] = `Bearer ${token}`;
 
-        if (response.data && response.data.token) {
+        // Redirect to dashboard - the page will reload and AuthContext will pick up the token
-          // Store the token and user data
-          localStorage.setItem('token', response.data.token);
-          api.defaults.headers.common['Authorization'] = `Bearer ${response.data.token}`;
-          
-          // Redirect to dashboard - the auth context will pick up the token on next page load
         window.location.href = '/dashboard';
-        } else {
-          setError('Invalid response from authentication server');
-          setProcessing(false);
-        }
       } catch (err: any) {
         console.error('OIDC callback error:', err);
 

src/routes/auth.rs

@@ -1,6 +1,6 @@
 use axum::{
     extract::{Query, State},
-    http::StatusCode,
+    http::{StatusCode, HeaderMap},
     response::{IntoResponse, Json, Response, Redirect},
     routing::{get, post},
     Router,
@@ -188,8 +188,9 @@ async fn oidc_login(State(state): State<Arc<AppState>>) -> Result<Redirect, Stat
 )]
 async fn oidc_callback(
     State(state): State<Arc<AppState>>,
+    headers: HeaderMap,
     Query(params): Query<OidcCallbackQuery>,
-) -> Result<Json<LoginResponse>, StatusCode> {
+) -> Result<Redirect, StatusCode> {
     tracing::info!("OIDC callback called with params: code={:?}, state={:?}, error={:?}", 
         params.code, params.state, params.error);
     
@@ -324,10 +325,29 @@ async fn oidc_callback(
             StatusCode::INTERNAL_SERVER_ERROR
         })?;
 
-    Ok(Json(LoginResponse {
+    // Redirect to frontend with token in URL fragment
-        token,
+    // The frontend should extract the token and store it
-        user: user.into(),
+    // Use absolute URL to ensure hash fragment is handled correctly by the browser
-    }))
+    let host = headers
+        .get("host")
+        .and_then(|h| h.to_str().ok())
+        .unwrap_or("localhost:8000");
+
+    // Check if behind a proxy (X-Forwarded-Proto header)
+    let protocol = headers
+        .get("x-forwarded-proto")
+        .and_then(|h| h.to_str().ok())
+        .unwrap_or("https");
+
+    let redirect_url = format!(
+        "{}://{}/auth/callback?token={}",
+        protocol,
+        host,
+        urlencoding::encode(&token)
+    );
+    tracing::info!("OIDC authentication successful for user: {}, redirecting to callback", user.username);
+
+    Ok(Redirect::to(&redirect_url))
 }
 
 // Helper function to create a new OIDC user

tests/integration_oidc_tests.rs

@@ -338,11 +338,14 @@ mod tests {
             .unwrap();
 
         let status = response.status();
+
+        // Extract headers before consuming response
+        let headers = response.headers().clone();
+
+        if status != StatusCode::SEE_OTHER {
             let body = axum::body::to_bytes(response.into_body(), usize::MAX)
                 .await
                 .unwrap();
-        
-        if status != StatusCode::OK {
             let error_text = String::from_utf8_lossy(&body);
             eprintln!("Response status: {}", status);
             eprintln!("Response body: {}", error_text);
@@ -360,13 +363,24 @@ mod tests {
             }
         }
 
-        assert_eq!(status, StatusCode::OK);
+        // Expect a redirect (303 See Other) instead of JSON response
+        assert_eq!(status, StatusCode::SEE_OTHER);
 
-        let login_response: serde_json::Value = serde_json::from_slice(&body).unwrap();
+        // Extract the token from the Location header
+        let location = headers.get("location").unwrap().to_str().unwrap();
+        assert!(location.contains("/auth/callback?token="));
 
-        assert!(login_response["token"].is_string());
+        // Extract token from URL
-        assert_eq!(login_response["user"]["username"], test_username);
+        let token_start = location.find("token=").unwrap() + 6;
-        assert_eq!(login_response["user"]["email"], test_email);
+        let token = urlencoding::decode(&location[token_start..]).unwrap();
+
+        // Verify token is not empty
+        assert!(!token.is_empty());
+
+        // Verify user was created by checking database
+        let user = db.get_user_by_username(&test_username).await.unwrap().unwrap();
+        assert_eq!(user.username, test_username);
+        assert_eq!(user.email, test_email);
     }
 
     #[tokio::test]