Skip to content

fix: not authenticate API with Cookies. Returns response 401 instead 200#2680

Merged
crivetimihai merged 2 commits intomainfrom
fix-api-authentication-with-cookies
Feb 7, 2026
Merged

fix: not authenticate API with Cookies. Returns response 401 instead 200#2680
crivetimihai merged 2 commits intomainfrom
fix-api-authentication-with-cookies

Conversation

@marekdano
Copy link
Copy Markdown
Collaborator

@marekdano marekdano commented Feb 3, 2026

🐛 Bug-fix PR

Issue reported at https://github.ibm.com/contextforge-org/mcp-context-forge/issues/3

Relates #2409

📌 Summary

What problem does this PR fix and why?

The issue is that API requests with cookies only (no Authorization header) are returning 200 OK instead of 401 Unauthorized.

The problem is that cookies are being accepted as valid authentication. According to the task, we should reject cookie-only authentication for API requests.

The key is to distinguish between browser requests (which should use cookies) and API requests (which should use Authorization header only).

The fix should be:

  • When a token is found ONLY in cookies (not in Authorization header)
  • AND the request is an API request (not a browser/HTML request)
  • THEN reject with 401

🔁 Reproduction Steps

make compose-up

# generate token
docker compose exec gateway python3 -m mcpgateway.utils.create_jwt_token --username admin@example.com --exp 10080 --secret my-test-key

# command should return a token
# save the token
export TOKEN=<TOKEN>

# call APIs with cookie jwt_token
curl -s -w "\nStatus: %{http_code}\n" \
  -b "jwt_token=$TOKEN" \
  http://localhost:8080/gateways
  
curl -s -w "\nStatus: %{http_code}\n" \
  -b "jwt_token=$TOKEN" \
  http://localhost:8080/resources
  
curl -s -w "\nStatus: %{http_code}\n" \
  -b "jwt_token=$TOKEN" \
  http://localhost:8080/tools

All APIs return 200 OK

🐞 Root Cause

What was wrong and where?

The user wants API requests with cookies only to return 401 instead of 200. I've identified the issue in mcpgateway/middleware/rbac.py where the get_current_user_with_permissions function accepts tokens from cookies for all requests.

💡 Fix Description

How did you solve it? Key design points.

I've applied a fix that:

  1. Prioritizes Authorization header over cookies
  2. Tracks if token came from cookie
  3. Checks if request is a browser request (HTML accept header or HTMX)
  4. Rejects cookie-only authentication for API requests (non-browser requests)

🧪 Verification

Check Command Status
Lint suite make lint ✅
Unit tests make test ✅
Coverage ≥ 80 % make coverage
Manual regression no longer fails steps / screenshots

📐 MCP Compliance (if relevant)

  • Matches current MCP spec
  • No breaking change to MCP clients

✅ Checklist

  • Code formatted (make black isort pre-commit)
  • No secrets/credentials committed

@crivetimihai
Copy link
Copy Markdown
Member

crivetimihai commented Feb 4, 2026

Clean security fix — the approach of tracking token source and rejecting cookie-only auth for non-browser requests is sound. CI is all green, tests are updated correctly.

One minor note: a client could technically bypass the check by including Accept: text/html in the header alongside a cookie, but that's indistinguishable from a legitimate browser request, so this seems acceptable.

LGTM — ready to merge.

@crivetimihai crivetimihai self-assigned this Feb 4, 2026
marekdano and others added 2 commits February 7, 2026 11:50
@marekdano @crivetimihai
fix: not authenticate API with Cookies. Returns response 401 instead 200
b40d4c8 
Signed-off-by: Marek Dano <mk.dano@gmail.com>
@crivetimihai
test: add test for cookie rejection on API requests
cc5db05 
Verify that cookie-only authentication returns 401 for non-browser
API requests (Accept: application/json), ensuring the security fix
is explicitly covered by tests.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
@crivetimihai crivetimihai force-pushed the fix-api-authentication-with-cookies branch from f127f14 to cc5db05 Compare February 7, 2026 12:08
crivetimihai
crivetimihai reviewed Feb 7, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@crivetimihai crivetimihai left a comment

Choose a reason for hiding this comment

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

Review Notes

Rebased onto current main (clean, no conflicts) and added a missing test.

Changes made during review

  • Rebased onto main (26f1f3e)
  • Added test test_get_current_user_with_permissions_cookie_rejected_for_api_request — verifies that an API request (Accept: application/json) with cookie-only auth and no Authorization header correctly returns 401 with the expected error message. The original PR updated existing tests to accommodate the new behavior but didn't include a positive assertion for the rejection itself.

Review findings

Logic & correctness: Sound. The token source priority is correctly reordered (Authorization header → cookies → FastAPI Cookie dependency), token_from_cookie tracking is accurate, and the browser detection heuristic (Accept: text/html or hx-request) is consistent with the rest of the codebase.

Security: Good defense-in-depth. The Accept header heuristic can be spoofed, but this is acceptable — the primary CSRF defense is the existing SameSite=Lax cookie attribute. This change prevents accidental cookie-based auth from API clients.

Consistency: No conflicts with other cookie-reading patterns in the codebase:

  • auth_middleware.py reads cookies for informational context (logging), not enforcement — unaffected
  • main.py cookie reads are for admin UI routes (HTML) or run after RBAC auth — unaffected
  • MCP transports (SSE, WebSocket, streamable HTTP) use Authorization headers exclusively — unaffected
  • llmchat_router.py cookie read is for forwarding credentials to downstream MCP servers, not user auth — unaffected

Tests: All 6520 unit tests pass. All 284 middleware tests pass (including the new one).

@crivetimihai crivetimihai merged commit da85027 into main Feb 7, 2026
46 checks passed
@crivetimihai crivetimihai deleted the fix-api-authentication-with-cookies branch February 7, 2026 12:09
crivetimihai added a commit that referenced this pull request Feb 7, 2026
@crivetimihai
fix: allow cookie auth for admin UI fetch requests
5ae03b6 
PR #2680 introduced a check that rejects cookie-only authentication for
API requests to prevent CSRF-style attacks from external callers.
However, the admin UI's JavaScript fetch() calls (fetchWithTimeout, etc.)
send Accept: */* or application/json — not text/html — so they were
incorrectly rejected with 401 Unauthorized.

Fix by also checking the Referer header for /admin to identify requests
originating from the admin UI. The browser automatically sets this header
on same-origin fetch calls, so admin UI AJAX requests are correctly
recognized as legitimate while external API callers without an admin
Referer are still blocked.

Also relax the unix socket throughput test threshold (50 → 10 calls/sec)
to prevent flaky failures in CI/slow environments.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
@crivetimihai crivetimihai mentioned this pull request Feb 7, 2026
Merged
crivetimihai added a commit that referenced this pull request Feb 7, 2026
@crivetimihai
fix: allow cookie auth for admin UI fetch requests (#2750)
bc8b764 
PR #2680 introduced a check that rejects cookie-only authentication for
API requests to prevent CSRF-style attacks from external callers.
However, the admin UI's JavaScript fetch() calls (fetchWithTimeout, etc.)
send Accept: */* or application/json — not text/html — so they were
incorrectly rejected with 401 Unauthorized.

Fix by also checking the Referer header for /admin to identify requests
originating from the admin UI. The browser automatically sets this header
on same-origin fetch calls, so admin UI AJAX requests are correctly
recognized as legitimate while external API callers without an admin
Referer are still blocked.

Also relax the unix socket throughput test threshold (50 → 10 calls/sec)
to prevent flaky failures in CI/slow environments.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
kcostell06 pushed a commit to kcostell06/mcp-context-forge that referenced this pull request Feb 24, 2026
@marekdano @crivetimihai
fix: not authenticate API with Cookies. Returns response 401 instead …
76669d5 
…200 (IBM#2680)

* fix: not authenticate API with Cookies. Returns response 401 instead 200

Signed-off-by: Marek Dano <mk.dano@gmail.com>

* test: add test for cookie rejection on API requests

Verify that cookie-only authentication returns 401 for non-browser
API requests (Accept: application/json), ensuring the security fix
is explicitly covered by tests.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

---------

Signed-off-by: Marek Dano <mk.dano@gmail.com>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
kcostell06 pushed a commit to kcostell06/mcp-context-forge that referenced this pull request Feb 24, 2026
@crivetimihai
fix: allow cookie auth for admin UI fetch requests (IBM#2750)
f7bb0a9 
PR IBM#2680 introduced a check that rejects cookie-only authentication for
API requests to prevent CSRF-style attacks from external callers.
However, the admin UI's JavaScript fetch() calls (fetchWithTimeout, etc.)
send Accept: */* or application/json — not text/html — so they were
incorrectly rejected with 401 Unauthorized.

Fix by also checking the Referer header for /admin to identify requests
originating from the admin UI. The browser automatically sets this header
on same-origin fetch calls, so admin UI AJAX requests are correctly
recognized as legitimate while external API callers without an admin
Referer are still blocked.

Also relax the unix socket throughput test threshold (50 → 10 calls/sec)
to prevent flaky failures in CI/slow environments.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@crivetimihai crivetimihai crivetimihai approved these changes

@kevalmahajan kevalmahajan Awaiting requested review from kevalmahajan kevalmahajan is a code owner

@madhav165 madhav165 Awaiting requested review from madhav165 madhav165 is a code owner

Assignees

@crivetimihai crivetimihai

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@marekdano @crivetimihai