Fix token verification and clean up comments by ordo-chao · Pull Request #101 · opencrafts-io/sherehe

ordo-chao · 2026-04-02T19:57:38Z

This pull request improves the verifyToken middleware in Middleware/jwt_token_verification.js by strengthening JWT verification, adding better error handling, and clarifying error messages. The changes make the authentication process more robust and informative for both developers and API consumers.

Error handling and validation improvements:

Added a check to ensure JWT_SECRET is configured, returning a 500 error if not set.
Improved error responses for missing or malformed authorization headers, missing tokens, and invalid token formats.
Switched from jwt.decode to jwt.verify for actual signature verification and restricted to the HS256 algorithm.
Added checks for the presence and structure of the decoded token payload, specifically requiring a sub property.
Added granular error handling for expired tokens, invalid tokens, and unexpected verification errors, with appropriate HTTP status codes and logging.

coderabbitai · 2026-04-02T19:57:45Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3c019171-5ce6-43e4-ad95-c78b22ffd66e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix-token-verifications

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Pull request overview

This pull request hardens the verifyToken Express middleware by performing real JWT signature verification and improving error handling around missing/malformed credentials.

Changes:

Add a server-side configuration guard to ensure JWT_SECRET is set before attempting verification.
Validate/parse the Authorization header and token more explicitly, with clearer failure messages.
Replace jwt.decode with jwt.verify (HS256-restricted) and validate the decoded payload contains sub.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-02T20:01:31Z

+    if (err.name === 'TokenExpiredError') {
+      return res.status(401).json('Token expired');
+    }
+


jsonwebtoken can throw NotBeforeError (token used before nbf). Currently that case would fall through to the generic 500 handler, making a client/token issue look like a server error. Consider handling NotBeforeError explicitly (likely as 401) alongside the other JWT error branches.

Suggested change

if (err.name === 'NotBeforeError') {

return res.status(401).json('Token not active');

}

Copilot · 2026-04-02T20:01:32Z

+    return res.status(401).json('Authorization header missing');
+  }
+
+  if (!authHeader.startsWith('Bearer ')) {
+    return res.status(401).json('Invalid authorization format');


Error responses in this middleware are returned as JSON strings (e.g., res.status(...).json('...')), but the rest of the API appears to consistently return objects like { message: ... } / { error: ... } (see e.g. Controllers/OrganizerDashboard.controller.js:47 and Middleware/paginate.js:6). Returning plain strings can make client-side error handling inconsistent; consider standardizing these responses to the same object shape used elsewhere.

Copilot · 2026-04-02T20:01:32Z

+  try {
+    const decoded = jwt.verify(token, process.env.JWT_SECRET, {
+      algorithms: ['HS256'],          // match how you sign tokens
+    });



There are existing middleware unit tests (e.g. tests/middlewares/paginate.test.js), but no coverage for this updated token-verification behavior. Adding tests for the main branches (missing header, malformed Bearer scheme, expired token, invalid signature, valid token payload w/ sub) would help prevent regressions.

Eugene600 · 2026-04-05T18:32:48Z

@ordo-chao please review comments from coderabbit

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-05T18:37:38Z

+  if (!process.env.JWT_SECRET) {
+    console.error('JWT_SECRET is not configured');
+    return res.status(500).json('Internal server error');
+  }


This middleware returns raw strings via res.json(...) (e.g., on 500 here). Elsewhere in the codebase responses are consistently JSON objects with message/error fields (e.g., Middleware/paginate.js:6, Middleware/upload.js:72). Consider returning a structured object here as well (and applying the same shape to the other error returns in this middleware) to keep the API response schema consistent for clients.

Copilot · 2026-04-05T18:37:38Z

+  const [scheme, token] = authHeader.trim().split(/\s+/, 2);
+
+  if (!scheme || scheme.toLowerCase() !== 'bearer') {
+    return res.status(401).json('Invalid authorization format');
  }
+  if (!token) {
+    return res.status(401).json('Token missing');
+  }


authHeader.trim().split(/\s+/, 2) will accept headers with extra segments (e.g., "Bearer <token> extra") by silently ignoring everything after the token. Consider parsing into all parts and requiring exactly 2 segments (Bearer + token) to avoid accepting malformed Authorization headers.

Copilot · 2026-04-05T18:37:38Z

+  // --- 1. Check server configuration ---
+  if (!process.env.JWT_SECRET) {
+    console.error('JWT_SECRET is not configured');
+    return res.status(500).json('Internal server error');
+  }


This will console.error on every request when JWT_SECRET is unset, which can flood logs under load. Consider failing fast at process startup (preferred) or at least reading/caching the secret at module load and logging once, while still returning 500 for requests.

Copilot · 2026-04-05T18:37:38Z

+  try {
+    const decoded = jwt.verify(token, process.env.JWT_SECRET, {
+      algorithms: ['HS256'],          // match how you sign tokens
+    });
+
+    if (!decoded || typeof decoded !== 'object' || !decoded.sub) {
+      return res.status(401).json('Invalid token payload');
+    }


The verification logic and expanded error cases are now more complex (config missing, malformed header, missing token, expired token, invalid token, invalid payload). The repo already has middleware unit tests (e.g., tests/middlewares/paginate.test.js), but there are no tests covering verifyToken. Consider adding unit tests for the main branches to prevent regressions.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ordo-chao added 2 commits April 2, 2026 22:54

Fix: verification of tokens

1642aee

Fix: removed comments

2a37260

ordo-chao requested review from Eugene600 and Copilot April 2, 2026 19:57

Copilot AI reviewed Apr 2, 2026

View reviewed changes

Update Middleware/jwt_token_verification.js

3e47888

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings April 5, 2026 18:34

Copilot started reviewing on behalf of ordo-chao April 5, 2026 18:34 View session

Copilot AI reviewed Apr 5, 2026

View reviewed changes

Update Middleware/jwt_token_verification.js

6f29384

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings April 5, 2026 18:54

Copilot started reviewing on behalf of ordo-chao April 5, 2026 18:55 View session

Copilot AI reviewed Apr 5, 2026

View reviewed changes

Eugene600 approved these changes Apr 5, 2026

View reviewed changes

Eugene600 merged commit 7c2d7ee into staging Apr 5, 2026
6 checks passed

Eugene600 mentioned this pull request Apr 5, 2026

Revert "Fix token verification and clean up comments" #102

Merged

+    if (err.name === 'NotBeforeError') {
+      return res.status(401).json('Token not active');
+    }

Conversation

ordo-chao commented Apr 2, 2026

Uh oh!

coderabbitai Bot commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Eugene600 commented Apr 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai Bot commented Apr 2, 2026 •

edited

Loading