fix(auth): address PR feedback on security and reliability

- Implemented session expiry tracking (1 hour timeout).
- Secured cookie retrieval to include path-scoped CSRF tokens.
- Added 30s timeout to axios instance.
- Explicitly accept 302 redirects for login endpoint while rejecting other 3xx/4xx/5xx in auth flow.
- Clarified auth tool descriptions regarding session vs API key scope.
- Ensured authentication state is reset on failure.

Author: koke1997

src/common/auth.ts

@@ -17,6 +17,8 @@ export interface AuthResult {
 
 let axiosInstance: AxiosInstance | null = null;
 let isAuthenticated = false;
+let authenticationTime: number | null = null;
+const SESSION_TIMEOUT_MS = 3600000; // 1 hour
 
 debugLog(`[AUTH] Module loaded - PID: ${process.pid}`).catch(() => {});
 
@@ -28,7 +30,11 @@ export function getAxiosInstance(): AxiosInstance {
   if (!axiosInstance) {
     debugLog("[AUTH] Creating new axios instance with cookie jar").catch(() => {});
     const jar = new CookieJar();
-    axiosInstance = wrapper(axios.create({ jar, withCredentials: true }));
+    axiosInstance = wrapper(axios.create({
+      jar,
+      withCredentials: true,
+      timeout: 30000 // 30 second timeout
+    }));
   } else {
     debugLog("[AUTH] Reusing existing axios instance").catch(() => {});
   }
@@ -64,9 +70,16 @@ export async function authenticateWithPassword(
     await debugLog(`[AUTH] Host URL: ${host}`);
 
     // Step 1: Get CSRF token (stored in cookie jar automatically)
-    const csrfResponse = await instance.get(`${host}auth/get-csrf-token/`);
+    // Use explicit path to ensure we capture path-scoped cookies
+    const csrfUrl = `${host}auth/get-csrf-token/`;
+    const csrfResponse = await instance.get(csrfUrl);
     await debugLog(`[AUTH] CSRF response status: ${csrfResponse.status}`);
-    await debugLog(`[AUTH] CSRF response headers: ${JSON.stringify(csrfResponse.headers)}`);
+    
+    // Only log headers if verbose debug is enabled to avoid leaking config details
+    if (process.env.PLANE_MCP_DEBUG === 'verbose') {
+      await debugLog(`[AUTH] CSRF response headers: ${JSON.stringify(csrfResponse.headers)}`);
+    }
+    
     await debugLog("[AUTH] CSRF token requested");
 
     // Step 2: Extract CSRF token from cookie jar for the request header
@@ -76,7 +89,8 @@ export async function authenticateWithPassword(
       return { success: false, error: "cookies", message: "Cookie jar not available for session authentication" };
     }
     const jar = maybeJar;
-    const cookies = await jar.getCookies(host);
+    // Check cookies on the specific CSRF URL to ensure we get path-scoped cookies
+    const cookies = await jar.getCookies(csrfUrl);
     await debugLog(`[AUTH] Cookies after CSRF request: ${cookies.map(c => c.key).join(", ")}`);
 
     const csrfCookie = cookies.find((c) => ["csrftoken", "csrf", "XSRF-TOKEN"].includes(c.key));
@@ -106,7 +120,7 @@ export async function authenticateWithPassword(
           "Content-Type": "application/x-www-form-urlencoded",
         },
         maxRedirects: 0, // Don't follow redirects, we just need the cookies
-        validateStatus: (status) => status >= 200 && status < 400, // Accept redirects as success
+        validateStatus: (status) => (status >= 200 && status < 300) || status === 302, // Accept 2xx and 302 (redirect) as success
       }
     );
 
@@ -115,16 +129,17 @@ export async function authenticateWithPassword(
     const headerNames = Object.keys(loginResponse.headers ?? {});
     await debugLog(`[AUTH] Login response headers present: ${headerNames.join(", ")}`);
 
-    // Log ALL headers for debugging
-    await debugLog(`[AUTH] Login response headers FULL: ${JSON.stringify(loginResponse.headers)}`);
+    // Log ALL headers for debugging ONLY if verbose
+    if (process.env.PLANE_MCP_DEBUG === 'verbose') {
+      await debugLog(`[AUTH] Login response headers FULL: ${JSON.stringify(loginResponse.headers)}`);
+    }
 
     // Check if Set-Cookie headers are present
     const setCookieHeader = loginResponse.headers['set-cookie'];
     if (setCookieHeader) {
       await debugLog(`[AUTH] Set-Cookie headers received: ${Array.isArray(setCookieHeader) ? setCookieHeader.length : 1} cookie(s)`);
     } else {
       await debugLog(`[AUTH] WARNING: No Set-Cookie headers in login response! Checking cookie jar anyway...`);
-      // We don't return error here, we assume cookies might be in the jar (e.g. from redirects or axios processing)
     }
 
     // Verify cookies were stored in the jar
@@ -137,17 +152,16 @@ export async function authenticateWithPassword(
     const sessionCookie = loginCookies.find((c) => sessionCookieNames.includes(c.key));
     if (!sessionCookie) {
       await debugLog(`[AUTH] WARNING: No standard session cookie found (looked for: ${sessionCookieNames.join(", ")})`);
-      // We don't return error here anymore, we let the verification step decide
     }
 
-    // Log full cookie details for debugging
-    loginCookies.forEach(c => {
-      // Redacted logging of cookie values
-      debugLog(`[AUTH] Cookie detail - ${c.key}: domain=${c.domain}, path=${c.path}, httpOnly=${c.httpOnly}, secure=${c.secure}`).catch(() => {});
-    });
+    // Log full cookie details for debugging - gated
+    if (process.env.PLANE_MCP_DEBUG === 'verbose') {
+      loginCookies.forEach(c => {
+        debugLog(`[AUTH] Cookie detail - ${c.key}: domain=${c.domain}, path=${c.path}, httpOnly=${c.httpOnly}, secure=${c.secure}`).catch(() => {});
+      });
+    }
 
     // Verify the session works with a test API call
-    // Note: Use /api/ endpoint (not /api/v1/) since session cookies work with /api/ endpoints
     try {
       const verifyUrl = `${host}api/users/me/`;
       await debugLog(`[AUTH] Attempting to verify session with: ${verifyUrl}`);
@@ -174,9 +188,14 @@ export async function authenticateWithPassword(
     }
 
     isAuthenticated = true;
+    authenticationTime = Date.now();
     await debugLog("[AUTH] Authentication successful");
     return { success: true };
   } catch (error) {
+    // Reset auth state on failure to avoid stale state
+    isAuthenticated = false;
+    authenticationTime = null;
+    
     await debugLog(`[AUTH] Authentication failed: ${error}`);
 
     if (axios.isAxiosError(error)) {
@@ -198,6 +217,19 @@ export async function authenticateWithPassword(
  * @returns true if authenticated, false otherwise
  */
 export function isSessionAuthenticated(): boolean {
+  if (!isAuthenticated || !authenticationTime) {
+    debugLog(`[AUTH] isSessionAuthenticated() - not authenticated`).catch(() => {});
+    return false;
+  }
+
+  const isStale = Date.now() - authenticationTime > SESSION_TIMEOUT_MS;
+  if (isStale) {
+    debugLog(`[AUTH] Session expired, resetting authentication`).catch(() => {});
+    isAuthenticated = false;
+    authenticationTime = null;
+    return false;
+  }
+  
   debugLog(`[AUTH] isSessionAuthenticated() called - returning: ${isAuthenticated}`).catch(() => {});
   return isAuthenticated;
 }
@@ -230,6 +262,7 @@ export async function resetAuthentication(): Promise<void> {
   } finally {
     axiosInstance = null;
     isAuthenticated = false;
+    authenticationTime = null;
     await debugLog("[AUTH] Authentication reset");
   }
-}
+}

src/common/request-helper.ts

@@ -66,7 +66,8 @@ export async function makePlaneRequest<T>(method: string, path: string, body: an
 
         const jar = (sessionAxios.defaults as any).jar;
         if (jar) {
-          const cookies = await jar.getCookies(host);
+          // Use full URL to match path-scoped cookies
+          const cookies = await jar.getCookies(url);
           const csrfCookie = cookies.find((c: any) => ["csrftoken", "csrf", "XSRF-TOKEN"].includes(c.key));
           if (csrfCookie) {
             headers["X-CSRFToken"] = csrfCookie.value;

src/tools/auth.ts

@@ -33,7 +33,7 @@ export const registerAuthTools = (server: McpServer) => {
                 {
                   message: "Successfully authenticated with Plane",
                   authenticated: true,
-                  note: "Session authentication enabled for Pages and /api/ endpoints. Other endpoints (/api/v1/) use API key if configured.",
+                  note: "Session authentication enables access to Pages and /api/ endpoints. Standard /api/v1/ endpoints (Issues, Projects, etc.) still require an API key if configured.",
                 },
                 null,
                 2
@@ -81,10 +81,10 @@ export const registerAuthTools = (server: McpServer) => {
                 api_key_configured: hasApiKey,
                 current_mode: authenticated ? "session (Pages + /api/ endpoints)" : hasApiKey ? "api_key (/api/v1/ endpoints)" : "unauthenticated",
                 note: authenticated
-                  ? "Using session authentication - access to Pages and /api/ endpoints"
+                  ? "Session active: Access to Pages and /api/ endpoints enabled. (API key required for /api/v1/ endpoints)"
                   : hasApiKey
-                  ? "Using API key - access to /api/v1/ endpoints only"
-                  : "No authentication configured",
+                  ? "API Key active: Access to /api/v1/ endpoints only."
+                  : "No authentication configured.",
               },
               null,
               2