From be642f5c1df3016f213cdd3218163b718d3970e2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 6 Jan 2026 07:20:24 +0000 Subject: [PATCH] Implement security improvements: enforce JWT secrets, remove client-side refresh token storage Co-authored-by: alexpolo1 <14327609+alexpolo1@users.noreply.github.com> --- apitest/FILE_GUIDE.md | 7 +++-- apitest/apikey.example | 2 ++ backend/.env.example | 11 ++++++++ backend/src/middleware/auth.js | 39 +++++++++++++++++++++++----- frontend/src/contexts/AuthContext.js | 32 ++++++++++++----------- 5 files changed, 67 insertions(+), 24 deletions(-) create mode 100644 apitest/apikey.example create mode 100644 backend/.env.example diff --git a/apitest/FILE_GUIDE.md b/apitest/FILE_GUIDE.md index 5cb0be7..0288b54 100644 --- a/apitest/FILE_GUIDE.md +++ b/apitest/FILE_GUIDE.md @@ -15,9 +15,12 @@ Quick reference for all files in this project. | File | Purpose | Security | |------|---------|----------| -| `apikey` | API authentication token | 🔒 SECRET | +| `apikey` | API authentication token | 🔒 SECRET - **DO NOT COMMIT** | +| `apikey.example` | Example template for apikey | Safe to commit | | `apiendpoint` | Production API endpoint URL | Public | +**⚠️ SECURITY WARNING:** The `apikey` file contains sensitive credentials and must NEVER be committed to version control. Use `apikey.example` as a template to create your local `apikey` file. + ## 🔧 Test Scripts (18 files) All scripts located in `examples/` directory: @@ -178,7 +181,7 @@ curl -X POST https://graphql.ordrestyring.dk/graphql \ ## ⚠️ Important Notes -1. **API Key:** Keep `apikey` file secure, never commit to version control +1. **API Key:** The `apikey` file contains sensitive credentials and must NEVER be committed to version control. Use `apikey.example` as a template to create your local `apikey` file. 2. **Production:** All tests run against production, be careful with writes 3. **Cleanup:** Write operations include cleanup steps 4. **Outputs:** Contain real data, may include sensitive information diff --git a/apitest/apikey.example b/apitest/apikey.example new file mode 100644 index 0000000..60498fe --- /dev/null +++ b/apitest/apikey.example @@ -0,0 +1,2 @@ +# Put your API key here +API_KEY=REPLACE_ME diff --git a/backend/.env.example b/backend/.env.example new file mode 100644 index 0000000..bc06dfa --- /dev/null +++ b/backend/.env.example @@ -0,0 +1,11 @@ +# JWT Secrets - REQUIRED +# Generate strong random secrets for production (e.g., using: openssl rand -base64 32) +JWT_ACCESS_SECRET=REPLACE_ME +JWT_REFRESH_SECRET=REPLACE_ME + +# OpenAI API Key - REQUIRED for AI features +OPENAI_ADMIN_KEY=REPLACE_ME + +# Optional: Token expiry configuration +# JWT_EXPIRY=24h +# REFRESH_TOKEN_EXPIRY=7d diff --git a/backend/src/middleware/auth.js b/backend/src/middleware/auth.js index 7b99bb2..4a8ccc1 100644 --- a/backend/src/middleware/auth.js +++ b/backend/src/middleware/auth.js @@ -1,10 +1,30 @@ const jwt = require('jsonwebtoken'); -// JWT secret - should be in .env in production -const JWT_SECRET = process.env.JWT_SECRET || 'dev-secret-change-in-production'; +// JWT secrets - REQUIRED in production, fail-fast if not set +const JWT_ACCESS_SECRET = process.env.JWT_ACCESS_SECRET; +const JWT_REFRESH_SECRET = process.env.JWT_REFRESH_SECRET; + +// Fail-fast: Require both secrets to be set at startup +if (!JWT_ACCESS_SECRET) { + throw new Error('FATAL: JWT_ACCESS_SECRET environment variable is required. Set it in your .env file.'); +} +if (!JWT_REFRESH_SECRET) { + throw new Error('FATAL: JWT_REFRESH_SECRET environment variable is required. Set it in your .env file.'); +} + +// Token expiry configuration const JWT_EXPIRY = process.env.JWT_EXPIRY || '24h'; const REFRESH_TOKEN_EXPIRY = process.env.REFRESH_TOKEN_EXPIRY || '7d'; +/** + * SECURITY NOTES: + * - Access tokens should be short-lived (default: 24h) + * - Refresh tokens should be stored server-side (database) or as HttpOnly cookies + * - Refresh tokens should be rotated on each use to prevent replay attacks + * - Never store refresh tokens in localStorage or any client-side storage + * - Consider implementing refresh token families for better security + */ + /** * Middleware to verify JWT token */ @@ -20,7 +40,7 @@ function verifyToken(req, res, next) { } try { - const decoded = jwt.verify(token, JWT_SECRET); + const decoded = jwt.verify(token, JWT_ACCESS_SECRET); req.user = decoded; next(); } catch (error) { @@ -47,13 +67,15 @@ function generateAccessToken(user) { username: user.username, id: user.id }, - JWT_SECRET, + JWT_ACCESS_SECRET, { expiresIn: JWT_EXPIRY } ); } /** * Generate refresh token + * NOTE: Refresh tokens MUST be stored server-side (in database) or as HttpOnly cookies. + * They should be rotated on each use to prevent replay attacks. */ function generateRefreshToken(user) { return jwt.sign( @@ -62,17 +84,19 @@ function generateRefreshToken(user) { id: user.id, type: 'refresh' }, - JWT_SECRET, + JWT_REFRESH_SECRET, { expiresIn: REFRESH_TOKEN_EXPIRY } ); } /** * Verify refresh token and return user data + * NOTE: In production, refresh tokens should be validated against a server-side store + * and rotated on each use. */ function verifyRefreshToken(token) { try { - const decoded = jwt.verify(token, JWT_SECRET); + const decoded = jwt.verify(token, JWT_REFRESH_SECRET); if (decoded.type !== 'refresh') { throw new Error('Invalid token type'); } @@ -87,5 +111,6 @@ module.exports = { generateAccessToken, generateRefreshToken, verifyRefreshToken, - JWT_SECRET + JWT_ACCESS_SECRET, + JWT_REFRESH_SECRET }; diff --git a/frontend/src/contexts/AuthContext.js b/frontend/src/contexts/AuthContext.js index 59abaf4..c104ed1 100644 --- a/frontend/src/contexts/AuthContext.js +++ b/frontend/src/contexts/AuthContext.js @@ -17,40 +17,42 @@ export const useAuth = () => { export const AuthProvider = ({ children }) => { const [user, setUser] = useState(null); const [accessToken, setAccessToken] = useState(null); - const [refreshToken, setRefreshToken] = useState(null); + // NOTE: refreshToken state removed - refresh tokens should be stored as HttpOnly cookies const [loading, setLoading] = useState(true); // Load tokens from localStorage on mount + // TODO: Remove refreshToken from localStorage - it should be stored as HttpOnly cookie useEffect(() => { const storedAccessToken = localStorage.getItem('accessToken'); - const storedRefreshToken = localStorage.getItem('refreshToken'); const storedUser = localStorage.getItem('user'); - if (storedAccessToken && storedRefreshToken && storedUser) { + if (storedAccessToken && storedUser) { setAccessToken(storedAccessToken); - setRefreshToken(storedRefreshToken); setUser(JSON.parse(storedUser)); } setLoading(false); }, []); // Set up axios interceptor for token refresh + // TODO: Backend must set refresh token as HttpOnly cookie on login/refresh + // TODO: Implement server-side refresh token persistence and rotation in follow-up PR useEffect(() => { const interceptor = axios.interceptors.response.use( (response) => response, async (error) => { const originalRequest = error.config; - // If 401 and we have a refresh token, try refreshing + // If 401 with expired flag, try refreshing using cookie-based flow if (error.response?.status === 401 && error.response?.data?.expired && - refreshToken && !originalRequest._retry) { originalRequest._retry = true; try { - const response = await axios.post(`${API_BASE_URL}/api/auth/refresh`, { - refreshToken + // Call refresh endpoint WITHOUT sending refreshToken in body + // Server should read refresh token from HttpOnly cookie + const response = await axios.post(`${API_BASE_URL}/api/auth/refresh`, {}, { + withCredentials: true // Important: send cookies with request }); const { accessToken: newAccessToken } = response.data; @@ -74,7 +76,7 @@ export const AuthProvider = ({ children }) => { return () => { axios.interceptors.response.eject(interceptor); }; - }, [refreshToken]); + }, []); // Removed refreshToken dependency const login = async (username, password) => { try { @@ -82,19 +84,20 @@ export const AuthProvider = ({ children }) => { username, password }, { - timeout: 10000 + timeout: 10000, + withCredentials: true // Important: receive HttpOnly cookie with refresh token }); if (response.data.success) { - const { accessToken: newAccessToken, refreshToken: newRefreshToken } = response.data; + // TODO: Backend should set refreshToken as HttpOnly cookie, not send it in response + const { accessToken: newAccessToken } = response.data; const userData = { username }; setAccessToken(newAccessToken); - setRefreshToken(newRefreshToken); setUser(userData); + // Only store access token in localStorage, NOT refresh token localStorage.setItem('accessToken', newAccessToken); - localStorage.setItem('refreshToken', newRefreshToken); localStorage.setItem('user', JSON.stringify(userData)); // Set default auth header @@ -116,9 +119,8 @@ export const AuthProvider = ({ children }) => { const logout = () => { setUser(null); setAccessToken(null); - setRefreshToken(null); + // No need to remove refreshToken from localStorage (it's not stored there anymore) localStorage.removeItem('accessToken'); - localStorage.removeItem('refreshToken'); localStorage.removeItem('user'); localStorage.removeItem('isAuthenticated'); // Legacy cleanup delete axios.defaults.headers.common['Authorization'];