セッション管理をJWTに移行し、ユーザー認証のミドルウェアを追加。関連するルートとコントローラーを更新。#49
Conversation
タスクに関しては一応切り出したけど、使わない想定。
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the authentication system from session-based to JWT (JSON Web Token) based authentication. The changes improve security and scalability by eliminating server-side session storage.
- Introduces JWT authentication middleware to protect routes
- Replaces session-based user retrieval with JWT token-based authentication
- Refactors initial task creation logic into a dedicated model function
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/middleware/authMiddleware.js | New authentication middleware that validates JWT tokens and attaches user info to requests |
| backend/routes/authRoutes.js | Added protect middleware to the /me endpoint |
| backend/controllers/authController.js | Removed session handling, simplified login/logout logic, and extracted initial task creation |
| backend/controllers/taskController.js | Updated to use req.user instead of req.session.user |
| backend/controllers/letterController.js | Updated to use req.user instead of req.session.user |
| backend/models/task.js | Added createInitialTasks function to handle initial task setup |
| backend/app.js | Removed express-session configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (req.headers.authorization && req.headers.authorization.startsWith('Bearer')) { | ||
| try { | ||
| // 'Bearer <token>' からトークン部分を抽出 | ||
| token = req.headers.authorization.split(' ')[1]; | ||
|
|
||
| // トークンを検証 | ||
| const decoded = jwt.verify(token, process.env.JWT_SECRET); | ||
|
|
||
| // ペイロードからユーザーIDを取得し、DBからユーザー情報を取得(パスワードは除く) | ||
| const [rows] = await pool.execute('SELECT user_id, username, email FROM users WHERE user_id = ?', [decoded.user_id]); | ||
|
|
||
| if (rows.length === 0) { | ||
| return res.status(401).json({ message: '認証に失敗しました: ユーザーが見つかりません' }); | ||
| } | ||
|
|
||
| // reqオブジェクトにユーザー情報を格納 | ||
| req.user = rows[0]; | ||
|
|
||
| next(); | ||
| } catch (error) { | ||
| console.error(error); | ||
| return res.status(401).json({ message: '認証に失敗しました: トークンが無効です' }); | ||
| } | ||
| } | ||
|
|
||
| if (!token) { | ||
| return res.status(401).json({ message: '認証に失敗しました: トークンがありません' }); |
There was a problem hiding this comment.
This token check is unreachable. If the authorization header is missing or doesn't start with 'Bearer', the code exits the outer if-block at line 30, but token remains undefined. This check should be moved outside the try-catch block but still within the outer if condition, or restructured to handle the case when the authorization header is absent before entering the if block at line 7.
| if (req.headers.authorization && req.headers.authorization.startsWith('Bearer')) { | |
| try { | |
| // 'Bearer <token>' からトークン部分を抽出 | |
| token = req.headers.authorization.split(' ')[1]; | |
| // トークンを検証 | |
| const decoded = jwt.verify(token, process.env.JWT_SECRET); | |
| // ペイロードからユーザーIDを取得し、DBからユーザー情報を取得(パスワードは除く) | |
| const [rows] = await pool.execute('SELECT user_id, username, email FROM users WHERE user_id = ?', [decoded.user_id]); | |
| if (rows.length === 0) { | |
| return res.status(401).json({ message: '認証に失敗しました: ユーザーが見つかりません' }); | |
| } | |
| // reqオブジェクトにユーザー情報を格納 | |
| req.user = rows[0]; | |
| next(); | |
| } catch (error) { | |
| console.error(error); | |
| return res.status(401).json({ message: '認証に失敗しました: トークンが無効です' }); | |
| } | |
| } | |
| if (!token) { | |
| return res.status(401).json({ message: '認証に失敗しました: トークンがありません' }); | |
| if (!(req.headers.authorization && req.headers.authorization.startsWith('Bearer'))) { | |
| return res.status(401).json({ message: '認証に失敗しました: トークンがありません' }); | |
| } | |
| try { | |
| // 'Bearer <token>' からトークン部分を抽出 | |
| token = req.headers.authorization.split(' ')[1]; | |
| // トークンを検証 | |
| const decoded = jwt.verify(token, process.env.JWT_SECRET); | |
| // ペイロードからユーザーIDを取得し、DBからユーザー情報を取得(パスワードは除く) | |
| const [rows] = await pool.execute('SELECT user_id, username, email FROM users WHERE user_id = ?', [decoded.user_id]); | |
| if (rows.length === 0) { | |
| return res.status(401).json({ message: '認証に失敗しました: ユーザーが見つかりません' }); | |
| } | |
| // reqオブジェクトにユーザー情報を格納 | |
| req.user = rows[0]; | |
| next(); | |
| } catch (error) { | |
| console.error(error); | |
| return res.status(401).json({ message: '認証に失敗しました: トークンが無効です' }); |
| const created_at = req.body.created_at; | ||
| const result = await Letter.selectLetter(user_id, created_at); | ||
|
|
||
| if (!letter) { |
There was a problem hiding this comment.
The variable letter is undefined. The query result is stored in result (line 43), so this condition should check if (!result) or if (result.length === 0) instead.
| if (!letter) { | |
| if (!result || result.length === 0) { |
|
|
||
| exports.createInitialTasks = async (userId) => { | ||
| const taskQuery = | ||
| "INSERT INTO tasks(task_title, task_name, task_type, status, user_id) VALUES ?"; |
There was a problem hiding this comment.
The SQL query uses INSERT INTO tasks (lowercase), but other queries in this file use Tasks (capitalized). Ensure table name casing is consistent with your database schema to avoid potential query failures on case-sensitive database systems.
| "INSERT INTO tasks(task_title, task_name, task_type, status, user_id) VALUES ?"; | |
| "INSERT INTO Tasks(task_title, task_name, task_type, status, user_id) VALUES ?"; |
| "https://homemax-frontend.onrender.com", | ||
| ], | ||
| credentials: true, // Cookie やセッションを許可 | ||
| credentials: true, // Cookie やセッションを許可 |
There was a problem hiding this comment.
The comment mentions 'Cookie やセッションを許可' (allowing cookies and sessions), but this PR removes session support. The comment should be updated to reflect that this is now only for allowing credentials like authorization headers.
| credentials: true, // Cookie やセッションを許可 | |
| credentials: true, // 認証ヘッダーなどのクレデンシャルを許可 |
|
|
||
| // //JSONの受け取り | ||
| // app.use(express.json()); | ||
| const Task = require("../models/task"); |
There was a problem hiding this comment.
Unused variable Task.
| const Task = require("../models/task"); |
…得時の条件を改善。認証ミドルウェアのトークン検証処理を整理し、タスク作成クエリのテーブル名を修正。
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ], | ||
| ]; | ||
| try { | ||
| await db.query(taskQuery, [initialTasks]); |
There was a problem hiding this comment.
The db.query method is being called with await, but based on the db configuration (mysql2/promise pool) and other model patterns in this file, it should use db.execute for promise-based queries. Other methods in this file use new Promise wrappers around db.query for callback-based queries. Change to await db.execute(taskQuery, [initialTasks]) for consistency with the promise-based API.
| await db.query(taskQuery, [initialTasks]); | |
| await db.execute(taskQuery, [initialTasks]); |
| @@ -1,63 +1,55 @@ | |||
| // サインアアップ、ログイン機能とか | |||
There was a problem hiding this comment.
Corrected spelling of 'サインアアップ' to 'サインアップ' (removed duplicate 'ア').
| // サインアアップ、ログイン機能とか | |
| // サインアップ、ログイン機能とか |
| @@ -1,136 +1,50 @@ | |||
| // サインアアップ、ログイン機能とか | |||
There was a problem hiding this comment.
Corrected spelling of 'サインアアップ' to 'サインアップ' (removed duplicate 'ア').
| // サインアアップ、ログイン機能とか | |
| // サインアップ、ログイン機能とか |
| addLetter: async function (req, res) { | ||
| try { | ||
| //ログインしていない場合ユーザーIDは0 | ||
| let user_id = req.user ? req.user.user_id : 1; |
There was a problem hiding this comment.
Defaulting to user_id = 1 when req.user is not present could assign letters to the wrong user. If authentication is required, this route should be protected with the protect middleware. If unauthenticated access is intentional, consider using a null/0 value or rejecting the request instead of defaulting to user ID 1.
| let user_id = req.user ? req.user.user_id : 1; | |
| let user_id = req.user ? req.user.user_id : 0; |
タスクに関しては一応切り出したけど、使わない想定。