improve oauth2 exchange security

Author: Thomas Stromberg

assets/app.js

@@ -837,6 +837,21 @@ const App = (() => {
       return;
     }
 
+    // Handle OAuth callback with auth code
+    console.log('[App.init] Checking for OAuth auth code...');
+    const authCodeExchanged = await Auth.handleAuthCodeCallback();
+
+    // Re-check for authentication token (it might have been set after module load or auth code exchange)
+    state.accessToken = Auth.getStoredToken();
+    console.log('[App.init] Checked for access token:', state.accessToken ? 'found' : 'not found');
+
+    // If we just exchanged an auth code successfully, reload to start with fresh state
+    if (authCodeExchanged) {
+      console.log('[App.init] Auth code exchanged successfully, reloading page...');
+      window.location.reload();
+      return;
+    }
+
     // Check for authentication
     if (!state.accessToken) {
       updateSearchInputVisibility();

assets/auth.js

@@ -1,5 +1,17 @@
 // Authentication Module for Ready To Review
 console.log('[Auth Module] Loading...');
+
+// Log URL parameters on page load for debugging OAuth flow
+const urlParams = new URLSearchParams(window.location.search);
+if (urlParams.has('code') || urlParams.has('state') || urlParams.has('error')) {
+  console.log('[Auth] OAuth callback detected!');
+  console.log('[Auth] URL:', window.location.href);
+  console.log('[Auth] Code:', urlParams.get('code') ? 'present (length=' + urlParams.get('code').length + ')' : 'missing');
+  console.log('[Auth] State:', urlParams.get('state') ? 'present' : 'missing');
+  console.log('[Auth] Error:', urlParams.get('error'));
+  console.log('[Auth] Error description:', urlParams.get('error_description'));
+}
+
 export const Auth = (() => {
   "use strict";
   console.log('[Auth Module] Initializing...');
@@ -36,39 +48,38 @@ export const Auth = (() => {
   }
 
   const getStoredToken = () => {
-    // Check for domain-wide access_token cookie (set by server after OAuth)
-    const accessToken = getCookie('access_token');
-    if (accessToken) return accessToken;
+    // Check localStorage for OAuth token
+    const localToken = localStorage.getItem(CONFIG.STORAGE_KEY);
+    if (localToken) return localToken;
 
-    // Check cookie for PAT
+    // Check for PAT (user-entered token, stored in non-HttpOnly cookie)
     const cookieToken = getCookie(CONFIG.COOKIE_KEY);
     if (cookieToken) return cookieToken;
 
-    // Fall back to localStorage (for OAuth - legacy)
-    return localStorage.getItem(CONFIG.STORAGE_KEY);
+    return null;
   };
 
   const storeToken = (token, useCookie = false) => {
-    // For PAT, store in a non-HttpOnly cookie
     if (useCookie) {
+      // For PAT, store in cookie
       setCookie(CONFIG.COOKIE_KEY, token, 365); // 1 year
     } else {
-      // For OAuth, token is now set by server as HttpOnly cookie
-      // We don't store it in localStorage anymore
-      // This function is kept for backward compatibility
+      // For OAuth, store in localStorage
+      localStorage.setItem(CONFIG.STORAGE_KEY, token);
     }
   };
 
   const clearToken = () => {
+    // Clear localStorage
     localStorage.removeItem(CONFIG.STORAGE_KEY);
+    // Clear PAT cookie
     deleteCookie(CONFIG.COOKIE_KEY);
-    // Clear domain-wide cookies
-    deleteCookie('access_token');
-    deleteCookie('username');
   };
 
   const initiateOAuthLogin = () => {
     console.log('[Auth.initiateOAuthLogin] Starting OAuth flow...');
+    console.log('[Auth.initiateOAuthLogin] Current URL:', window.location.href);
+    console.log('[Auth.initiateOAuthLogin] Redirecting to:', window.location.origin + '/oauth/login');
 
     // Simply redirect to the backend OAuth endpoint
     // The backend will handle state generation and cookie management
@@ -179,8 +190,53 @@ export const Auth = (() => {
     }
   };
 
-  // OAuth callback is now fully handled by the backend
-  // The backend sets domain-wide cookies and redirects to the user's workspace
+  // Handle OAuth callback with auth code
+  const handleAuthCodeCallback = async () => {
+    // Auth code is in fragment (hash) not query parameter for security
+    // Fragments are not sent to server in Referer headers
+    const fragment = window.location.hash.substring(1); // Remove leading #
+    const fragmentParams = new URLSearchParams(fragment);
+    const authCode = fragmentParams.get('auth_code');
+
+    if (!authCode) {
+      return false; // No auth code, nothing to do
+    }
+
+    console.log('[Auth] Found auth_code in fragment, exchanging for token...');
+
+    try {
+      // Exchange auth code for token
+      const response = await fetch('/oauth/exchange', {
+        method: 'POST',
+        headers: {
+          'Content-Type': 'application/json',
+        },
+        body: JSON.stringify({ auth_code: authCode }),
+      });
+
+      if (!response.ok) {
+        console.error('[Auth] Failed to exchange auth code:', response.status);
+        return false;
+      }
+
+      const data = await response.json();
+
+      // Store token in localStorage
+      storeToken(data.token);
+
+      console.log('[Auth] Successfully exchanged auth code for token, user:', data.username);
+
+      // Remove auth_code from URL fragment
+      const url = new URL(window.location);
+      url.hash = ''; // Clear the fragment
+      window.history.replaceState({}, '', url);
+
+      return true;
+    } catch (error) {
+      console.error('[Auth] Error exchanging auth code:', error);
+      return false;
+    }
+  };
 
   const handleAuthError = () => {
     clearToken();
@@ -374,6 +430,7 @@ export const Auth = (() => {
     storeToken,
     clearToken,
     initiateOAuthLogin,
+    handleAuthCodeCallback,
     showGitHubAppModal,
     closeGitHubAppModal,
     proceedWithOAuth,

assets/user.js

@@ -110,7 +110,7 @@ export const User = (() => {
 
     try {
       const response = await fetch(
-        "https://turn.ready-to-review.dev/v1/validate",
+        "https://turn.github.codegroove.app/v1/validate",
         {
           method: "POST",
           headers,

index.html

@@ -8,16 +8,16 @@
             content="A modern dashboard for managing GitHub pull requests"
         />
         <title>Ready To Review - GitHub PR Dashboard</title>
-        <link rel="stylesheet" href="/assets/styles.css" />
+        <link rel="stylesheet" href="/assets/styles.css?v=BUILD_TIMESTAMP" />
         <link rel="icon" href="/favicon.ico" />
         <link rel="preconnect" href="https://api.github.com" />
         <link rel="dns-prefetch" href="https://api.github.com" />
         <link rel="preconnect" href="https://avatars.githubusercontent.com" />
         <link rel="dns-prefetch" href="https://avatars.githubusercontent.com" />
-        <link rel="modulepreload" href="/assets/app.js" />
-        <link rel="modulepreload" href="/assets/user.js" />
-        <link rel="modulepreload" href="/assets/utils.js" />
-        <link rel="modulepreload" href="/assets/workspace.js" />
+        <link rel="modulepreload" href="/assets/app.js?v=BUILD_TIMESTAMP" />
+        <link rel="modulepreload" href="/assets/user.js?v=BUILD_TIMESTAMP" />
+        <link rel="modulepreload" href="/assets/utils.js?v=BUILD_TIMESTAMP" />
+        <link rel="modulepreload" href="/assets/workspace.js?v=BUILD_TIMESTAMP" />
     </head>
     <body>
         <div id="app">
@@ -1286,7 +1286,7 @@ <h1 class="changelog-title" id="changelogTitleText">
             </footer>
         </div>
 
-        <script src="/assets/demo-data.js"></script>
-        <script type="module" src="/assets/app.js"></script>
+        <script src="/assets/demo-data.js?v=BUILD_TIMESTAMP"></script>
+        <script type="module" src="/assets/app.js?v=BUILD_TIMESTAMP"></script>
     </body>
 </html>