Skip to content

Conversation

@lodewiges
Copy link
Contributor

@lodewiges lodewiges commented Nov 2, 2025

We need CORS on amber-ui for proper authentication with Sofia.
This was not needed in the past because we did not use a proper AJAX request
https://developer.mozilla.org/en-US/docs/Web/HTTP/Guides/CORS

Summary by CodeRabbit

  • Chores
    • Added cross-origin request (CORS) support to enable communication from localhost:5000 to the server.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 2, 2025

Walkthrough

CORS middleware is integrated into the server by adding the cors package as a dev dependency and configuring it during server initialization to permit cross-origin requests from http://localhost:5000.

Changes

Cohort / File(s) Summary
CORS middleware integration
package.json, server/index.js
Added cors ^2.8.5 as dev dependency; imported and configured CORS middleware in server initialization with origin restricted to http://localhost:5000

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Verify CORS origin value (http://localhost:5000) matches intended deployment environment
  • Confirm cors package version ^2.8.5 is compatible with Node.js version used in this project

Poem

🐰 A rabbit hops through origins with glee,
CORS middleware now sets servers free,
From localhost:5000, requests may flow,
Cross-border bridges, put on a show! 🌉✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title "Add CORS for authentication" clearly and concisely summarizes the primary change in the pull request. It accurately reflects the main objective of adding CORS middleware support to enable authentication with Sofia, which is the core purpose of the changes made in both package.json (adding the cors dependency) and server/index.js (implementing the CORS middleware). The title is specific enough to inform developers scanning the history of the key change without being overly verbose.
Description Check ✅ Passed The pull request description provides meaningful context for the changes, explaining that CORS is needed for proper authentication with Sofia and clarifying why it wasn't necessary before (lack of proper AJAX requests). It includes a helpful reference to the MDN CORS guide. However, the description does not follow the template structure with required section headings ("### Summary" and "### Other information"), does not mention any related issues using the 'fixes #xyz' format, and does not explicitly detail the new dependency being added. Despite these formatting and completeness gaps, the description contains substantive information about the change's purpose and rationale.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/AddCORS

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.

@lodewiges lodewiges changed the title intial commit Add CORS for authentication Nov 2, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
server/index.js (1)

13-13: Consider making the CORS origin configurable.

The origin is hardcoded to http://localhost:5000. While this works for the specific use case with Sofia, consider using an environment variable for better flexibility during development.

Apply this diff to make the origin configurable:

-  const cors = require('cors');
+  const cors = require('cors');
+  const corsOrigin = process.env.CORS_ORIGIN || 'http://localhost:5000';
   const globSync = require('glob').sync;
   const mocks = globSync('./mocks/**/*.js', { cwd: __dirname }).map(require);
   const proxies = globSync('./proxies/**/*.js', { cwd: __dirname }).map(
     require
   );
 
-  app.use(cors({ origin: 'http://localhost:5000' }));
+  // Enable CORS for cross-origin authentication with Sofia
+  app.use(cors({ origin: corsOrigin }));

Also applies to: 20-20

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 050fab4 and 205301e.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (2)
  • package.json (1 hunks)
  • server/index.js (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build
🔇 Additional comments (2)
package.json (1)

43-43: No action required—cors version is current and secure.

The specified version ^2.8.5 is the latest stable release on npm, and no known security vulnerabilities exist for this package. The dependency is appropriately configured.

server/index.js (1)

20-20: Verify Sofia's authentication integration pattern with your backend.

The codebase analysis shows Sofia as an external service (linked in the UI and configurable via environment variables), but contains no evidence of credentialed HTTP communication between Sofia and this server. Specifically:

  • No credentials or withCredentials usage found in the codebase
  • No authentication middleware or endpoints handling Sofia requests
  • CORS origin is already correctly set to a specific origin ('http://localhost:5000'), not '*'

The suggested credentials: true flag is only necessary if Sofia (or requests from Sofia) actually send credentials (cookies, auth headers). If this authentication pattern exists in your backend services (outside this frontend codebase) or is a new requirement from the PR, then the suggestion is valid. Otherwise, it may be unnecessary.

Confirm whether:

  1. Sofia makes HTTP requests to this server with credentials
  2. Your backend sets cookies intended for Sofia
  3. This is a new requirement for the PR

@codecov
Copy link

codecov bot commented Nov 2, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 13.05%. Comparing base (0062397) to head (205301e).
⚠️ Report is 6 commits behind head on staging.

Additional details and impacted files
@@             Coverage Diff             @@
##           staging    #1047      +/-   ##
===========================================
- Coverage    13.07%   13.05%   -0.02%     
===========================================
  Files          450      450              
  Lines         3113     3117       +4     
===========================================
  Hits           407      407              
- Misses        2706     2710       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lodewiges lodewiges enabled auto-merge November 10, 2025 14:17
@lodewiges lodewiges disabled auto-merge November 10, 2025 14:17
@lodewiges lodewiges merged commit 165d4bd into staging Nov 10, 2025
5 of 6 checks passed
@lodewiges lodewiges deleted the feature/AddCORS branch November 10, 2025 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants