fix(ui): redirect authenticated users from login page to dashboard by marekdano · Pull Request #3461 · IBM/mcp-context-forge

marekdano · 2026-03-04T10:50:51Z

🐛 Bug-fix PR

Closes: #3430

📌 Summary

Fixed issue where authenticated users could still access the login page instead of being redirected to the dashboard.

🔁 Reproduction Steps

Log in to the application with valid credential
Visit /admin/login by typing in to address bar
User can view the login page again

🐞 Root Cause

When a logged-in user navigated to /admin/login by typing it directly in the address bar, they would see the login form again instead of being redirected to the dashboard. This created a confusing user experience.

💡 Fix Description

Added authentication check in the admin_login_page endpoint to detect already-authenticated users and redirect them to the dashboard.

Changes

Core Fix (mcpgateway/admin.py)

Modified admin_login_page() function (line ~3681):

Added JWT token validation check before rendering login page
Checks for jwt_token or access_token cookies
Verifies token validity using verify_jwt_token_cached()
Redirects authenticated users to /admin dashboard (303 redirect)
Falls through to show login form if token is invalid/expired

Unit Tests (tests/unit/mcpgateway/test_admin.py)

Added two test cases to TestAdminLoginPage class:

test_admin_login_page_redirects_authenticated_user

Mocks valid JWT token in request cookies
Verifies redirect to dashboard (status 303)
Confirms location header points to /app/admin

test_admin_login_page_shows_form_for_invalid_token

Mocks invalid/expired JWT token
Verifies login form is displayed (no redirect)
Confirms CSRF token is set in response

E2E Test (tests/playwright/test_auth.py)

Added test_logged_in_user_redirected_from_login_page to TestAuthentication class:

Performs full login flow with valid credentials
Verifies JWT cookie is set
Navigates to /admin/login while authenticated
Asserts redirect to dashboard occurs
Confirms login form is not visible
Verifies admin interface elements are displayed

Behavior

Before:

Logged-in user visits /admin/login → sees login form

After:

Logged-in user visits /admin/login → redirected to /admin dashboard
User with invalid/expired token → sees login form (unchanged)
Unauthenticated user → sees login form (unchanged)

🧪 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

Lang-Akshay

Thanks for the PR @marekdano . Great work. Please address the following findings.

Findings Summary

#	File	Line	Severity	CWE	Description
1	`mcpgateway/admin.py`	3731–3733	Low	CWE-754	Bare `except Exception: pass` silently swallows all exceptions from JWT verification, including unexpected failures (key-loading errors, cryptographic faults). Masks operational issues.
2	`mcpgateway/admin.py`	3724	Low	CWE-287	Accepting tokens from two cookie names (`jwt_token` and `access_token`) widens the attack surface. If one cookie has weaker `SameSite`/`Secure`/`Path` attributes, it may be injectable via a different origin.
3	`mcpgateway/admin.py`	3729–3730	Informational	CWE-613	When a token is invalid/expired, the code falls through without clearing the stale cookie. The browser continues sending the bad token on every subsequent request.
4	`tests/unit/mcpgateway/test_admin.py`	13660	⚠️ Medium	CWE-1164	The assertion `assert "secure cookies enabled" in (context.get("secure_cookie_warning") or "").lower()` was deleted from `test_admin_login_page_secure_cookie_warning_in_development`. The test now grabs the context dict but never validates its content, eliminating coverage for the security warning feature.
5	`tests/unit/mcpgateway/test_admin.py`	13625–13657	Low	CWE-1164	Pre-existing tests `test_admin_login_page_email_auth_enabled` and `test_admin_login_page_secure_cookie_warning_in_development` do not set `request.cookies = {}`. With `MagicMock(spec=Request)`, `request.cookies.get("jwt_token")` returns a truthy MagicMock, unintentionally triggering the new JWT-check code path. Tests pass only because the MagicMock isn't a valid JWT and the exception is swallowed.

Suggestions :

Finding 1 — Narrow the exception handler to catch only expected types (HTTPException, jwt.PyJWTError) instead of blanket Exception. This preserves intent while allowing unexpected errors to propagate and be logged. Effort: trivial. Ref: CWE-754, OWASP Error Handling.
Finding 2 — Either gate access_token acceptance behind a feature flag (e.g., SSO flow), document that both cookies MUST share identical Secure/HttpOnly/SameSite attributes, or simplify to only read jwt_token (the cookie the gateway itself sets). Effort: low.
Finding 3 — Consider clearing the stale cookie on verification failure via response.delete_cookie(). Effort: low (requires minor flow restructuring).
Finding 4 — ⚠️ Restore the deleted assertion. Add request.cookies = {} to the test to prevent the new JWT-check path from interfering, then re-add: assert "secure cookies enabled" in context["secure_cookie_warning"].lower(). Effort: trivial. This is the highest-priority fix.
Finding 5 — Add explicit request.cookies = {} to all pre-existing tests that don't intend to test the JWT cookie path. Effort: trivial.

Deny-Path Test Coverage

Deny-path scenario	Covered?	Notes
No cookie present → show login form	Implicit	Works via existing test, but `request.cookies` should be explicitly set to `{}`
Valid JWT → redirect to dashboard	✅ Yes	`test_admin_login_page_redirects_authenticated_user()` (`tests/unit/mcpgateway/test_admin.py:13663`)
Invalid/expired JWT → show login form	✅ Yes	`test_admin_login_page_shows_form_for_invalid_token()` (`tests/unit/mcpgateway/test_admin.py:13686`)
`verify_jwt_token_cached()` (`mcpgateway/utils/verify_credentials.py:229`) returns falsy/None → show login form	❌ No	The `if payload:` branch (`mcpgateway/admin.py:3729`) has no test for `None`/`{}` return
`access_token` cookie with valid JWT → redirect	❌ No	Only `jwt_token` tested; the `or request.cookies.get("access_token")` branch (`mcpgateway/admin.py:3724`) has zero coverage
`access_token` cookie with invalid JWT → show login form	❌ No	Same gap as above (`mcpgateway/admin.py:3724`)

crivetimihai · 2026-03-05T13:51:20Z

Thanks @marekdano — good UX improvement for #3430. Checking JWT/access_token cookies before rendering the login page is the right approach. LGTM.

gcgoncalves

Code looks good. Tested and works as expected.

Signed-off-by: Marek Dano <Marek.Dano@ibm.com>

- Remove stray @pytest.mark.asyncio decorator that was separated from test_admin_login_handler_missing_fields by new test insertions - Add missing password_reset_enabled mock to access_token and None payload test variants for consistency - Add HTTPException test case (the real-world error path from verify_jwt_token_cached) - Simplify Playwright test: remove unnecessary try/except around not_to_be_visible assertion and redundant wait_for_timeout - Apply black/isort formatting Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

crivetimihai

Reviewed and rebased onto latest main (clean, no conflicts).

Feature review: The login page redirect logic is correct and well-designed — validates JWT cookies server-side via verify_jwt_token_cached, redirects authenticated users to the dashboard (303), and falls through to the login form for invalid/expired tokens. No security or performance concerns.

Review fixes applied (commit 281b734):

Fixed stray @pytest.mark.asyncio decorator that was separated from test_admin_login_handler_missing_fields by the new test insertions
Added missing password_reset_enabled mock to access_token and None-payload test variants for consistency with the jwt_token variant
Added HTTPException test case — verify_jwt_token_cached raises HTTPException (not jwt.PyJWTError) for expired/invalid tokens, so this covers the primary real-world error path
Simplified Playwright test: removed unnecessary try/except around not_to_be_visible() and redundant wait_for_timeout(1000)
Applied black/isort formatting

Solved

The LoginPage.email_input locator (input[name="email"]) matches multiple elements on the admin dashboard (create-user and invite-user forms), causing a strict mode violation. Use is_on_login_page() URL check instead, which is both more reliable and semantically correct. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

crivetimihai · 2026-03-11T12:17:51Z

Fixed the Playwright test failure (test_logged_in_user_redirected_from_login_page):

The LoginPage.email_input locator (input[name="email"]) matched multiple elements on the admin dashboard (create-user and invite-user forms), causing a Playwright strict mode violation. Replaced with is_on_login_page() URL check, which is more reliable and semantically correct.

Commit: c2405f6

The @lru_cache on get_jwt_public_key_or_secret() in jwt_config_helper causes intermittent doctest failures under pytest-xdist (-n 4). When another test runs first and caches the real settings key, subsequent doctests that assign DummySettings get a stale cached key, causing InvalidSignatureError. Adding jch.clear_jwt_caches() after each DummySettings assignment ensures cache isolation. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

…3461) * fix: redirecting authenticated users on /admin/login page to /admin one. Signed-off-by: Marek Dano <Marek.Dano@ibm.com> * fix: failing pytest in test_admin.py Signed-off-by: Marek Dano <Marek.Dano@ibm.com> * fix: pylint issues Signed-off-by: Marek Dano <Marek.Dano@ibm.com> * fix: cwe issues and tests Signed-off-by: Marek Dano <Marek.Dano@ibm.com> * fix: unit tests in test_admin.py Signed-off-by: Marek Dano <Marek.Dano@ibm.com> * fix: review fixes for login page redirect tests - Remove stray @pytest.mark.asyncio decorator that was separated from test_admin_login_handler_missing_fields by new test insertions - Add missing password_reset_enabled mock to access_token and None payload test variants for consistency - Add HTTPException test case (the real-world error path from verify_jwt_token_cached) - Simplify Playwright test: remove unnecessary try/except around not_to_be_visible assertion and redundant wait_for_timeout - Apply black/isort formatting Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: use URL check instead of email locator in redirect test The LoginPage.email_input locator (input[name="email"]) matches multiple elements on the admin dashboard (create-user and invite-user forms), causing a strict mode violation. Use is_on_login_page() URL check instead, which is both more reliable and semantically correct. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: clear JWT lru_cache in doctests to prevent intermittent failures The @lru_cache on get_jwt_public_key_or_secret() in jwt_config_helper causes intermittent doctest failures under pytest-xdist (-n 4). When another test runs first and caches the real settings key, subsequent doctests that assign DummySettings get a stale cached key, causing InvalidSignatureError. Adding jch.clear_jwt_caches() after each DummySettings assignment ensures cache isolation. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Marek Dano <Marek.Dano@ibm.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Marek Dano <Marek.Dano@ibm.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>

Fix two regressions from recent merges: 1. pagination_controls.html references pagination.page_items but the 5 inline pagination dicts in admin.html did not include it. Jinja2 rendered Undefined as empty string producing invalid JS: `pageItems: ,`. Add 'page_items': None to all inline pagination dicts. (from #3238) 2. admin_login_page() redirected any authenticated user to /admin without checking admin privileges, causing an infinite redirect loop for non-admin users. Now checks is_admin in JWT payload and skips the redirect when error query params are present. (from #3461) 3. MANIFEST.in excluded gitignored plugins_rust/Cargo.lock that was matched by recursive-include *.lock, fixing make verify. Closes #3238, closes #3461 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

…3645) Fix two regressions from recent merges: 1. pagination_controls.html references pagination.page_items but the 5 inline pagination dicts in admin.html did not include it. Jinja2 rendered Undefined as empty string producing invalid JS: `pageItems: ,`. Add 'page_items': None to all inline pagination dicts. (from #3238) 2. admin_login_page() redirected any authenticated user to /admin without checking admin privileges, causing an infinite redirect loop for non-admin users. Now checks is_admin in JWT payload and skips the redirect when error query params are present. (from #3461) 3. MANIFEST.in excluded gitignored plugins_rust/Cargo.lock that was matched by recursive-include *.lock, fixing make verify. Closes #3238, closes #3461 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

…3461) * fix: redirecting authenticated users on /admin/login page to /admin one. Signed-off-by: Marek Dano <Marek.Dano@ibm.com> * fix: failing pytest in test_admin.py Signed-off-by: Marek Dano <Marek.Dano@ibm.com> * fix: pylint issues Signed-off-by: Marek Dano <Marek.Dano@ibm.com> * fix: cwe issues and tests Signed-off-by: Marek Dano <Marek.Dano@ibm.com> * fix: unit tests in test_admin.py Signed-off-by: Marek Dano <Marek.Dano@ibm.com> * fix: review fixes for login page redirect tests - Remove stray @pytest.mark.asyncio decorator that was separated from test_admin_login_handler_missing_fields by new test insertions - Add missing password_reset_enabled mock to access_token and None payload test variants for consistency - Add HTTPException test case (the real-world error path from verify_jwt_token_cached) - Simplify Playwright test: remove unnecessary try/except around not_to_be_visible assertion and redundant wait_for_timeout - Apply black/isort formatting Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: use URL check instead of email locator in redirect test The LoginPage.email_input locator (input[name="email"]) matches multiple elements on the admin dashboard (create-user and invite-user forms), causing a strict mode violation. Use is_on_login_page() URL check instead, which is both more reliable and semantically correct. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: clear JWT lru_cache in doctests to prevent intermittent failures The @lru_cache on get_jwt_public_key_or_secret() in jwt_config_helper causes intermittent doctest failures under pytest-xdist (-n 4). When another test runs first and caches the real settings key, subsequent doctests that assign DummySettings get a stale cached key, causing InvalidSignatureError. Adding jch.clear_jwt_caches() after each DummySettings assignment ensures cache isolation. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Marek Dano <Marek.Dano@ibm.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Marek Dano <Marek.Dano@ibm.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com> Signed-off-by: Yosief Eyob <yosiefogbazion@gmail.com>

…3645) Fix two regressions from recent merges: 1. pagination_controls.html references pagination.page_items but the 5 inline pagination dicts in admin.html did not include it. Jinja2 rendered Undefined as empty string producing invalid JS: `pageItems: ,`. Add 'page_items': None to all inline pagination dicts. (from #3238) 2. admin_login_page() redirected any authenticated user to /admin without checking admin privileges, causing an infinite redirect loop for non-admin users. Now checks is_admin in JWT payload and skips the redirect when error query params are present. (from #3461) 3. MANIFEST.in excluded gitignored plugins_rust/Cargo.lock that was matched by recursive-include *.lock, fixing make verify. Closes #3238, closes #3461 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Signed-off-by: Yosief Eyob <yosiefogbazion@gmail.com>

…BM#3645) Fix two regressions from recent merges: 1. pagination_controls.html references pagination.page_items but the 5 inline pagination dicts in admin.html did not include it. Jinja2 rendered Undefined as empty string producing invalid JS: `pageItems: ,`. Add 'page_items': None to all inline pagination dicts. (from IBM#3238) 2. admin_login_page() redirected any authenticated user to /admin without checking admin privileges, causing an infinite redirect loop for non-admin users. Now checks is_admin in JWT payload and skips the redirect when error query params are present. (from IBM#3461) 3. MANIFEST.in excluded gitignored plugins_rust/Cargo.lock that was matched by recursive-include *.lock, fixing make verify. Closes IBM#3238, closes IBM#3461 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Signed-off-by: KRISHNAN, SANTHANA <sk8069@exo.att.com>

marekdano requested review from crivetimihai, kevalmahajan and madhav165 as code owners March 4, 2026 10:50

Lang-Akshay self-requested a review March 4, 2026 14:07

Lang-Akshay previously requested changes Mar 4, 2026

View reviewed changes

marekdano requested a review from Lang-Akshay March 5, 2026 13:43

crivetimihai changed the title ~~fix: redirecting authenticated users on /admin/login page to /admin one.~~ fix(ui): redirect authenticated users from login page to dashboard Mar 5, 2026

crivetimihai added bug Something isn't working ui User Interface SHOULD P2: Important but not vital; high-value items that are not crucial for the immediate release labels Mar 5, 2026

crivetimihai added this to the Release 1.0.0-RC2 milestone Mar 5, 2026

marekdano force-pushed the 3430-fix-redirecting-to-dashboard-auth-users branch from 4c4b9e3 to c7860d0 Compare March 5, 2026 14:18

gcgoncalves previously approved these changes Mar 6, 2026

View reviewed changes

marekdano dismissed gcgoncalves’s stale review via cf5a923 March 6, 2026 15:06

marekdano force-pushed the 3430-fix-redirecting-to-dashboard-auth-users branch from 156ba79 to cf5a923 Compare March 6, 2026 15:06

marekdano requested a review from gcgoncalves March 6, 2026 15:07

crivetimihai modified the milestones: Release 1.0.0-RC2, Release 1.0.0 Mar 7, 2026

crivetimihai self-assigned this Mar 7, 2026

gcgoncalves previously approved these changes Mar 9, 2026

View reviewed changes

marekdano added the release-fix Critical bugfix required for the release label Mar 10, 2026

Marek Dano and others added 6 commits March 11, 2026 10:41

fix: redirecting authenticated users on /admin/login page to /admin one.

cd93cc4

Signed-off-by: Marek Dano <Marek.Dano@ibm.com>

fix: failing pytest in test_admin.py

5797178

Signed-off-by: Marek Dano <Marek.Dano@ibm.com>

fix: pylint issues

ad17b1f

Signed-off-by: Marek Dano <Marek.Dano@ibm.com>

fix: cwe issues and tests

f3100c0

Signed-off-by: Marek Dano <Marek.Dano@ibm.com>

fix: unit tests in test_admin.py

47fa4cb

Signed-off-by: Marek Dano <Marek.Dano@ibm.com>

crivetimihai dismissed gcgoncalves’s stale review via 281b734 March 11, 2026 12:07

crivetimihai force-pushed the 3430-fix-redirecting-to-dashboard-auth-users branch from a88047d to 281b734 Compare March 11, 2026 12:07

crivetimihai previously approved these changes Mar 11, 2026

View reviewed changes

crivetimihai dismissed their stale review via c2405f6 March 11, 2026 12:17

crivetimihai merged commit 767c495 into main Mar 11, 2026
39 checks passed

crivetimihai deleted the 3430-fix-redirecting-to-dashboard-auth-users branch March 11, 2026 12:50

crivetimihai mentioned this pull request Mar 12, 2026

fix: resolve JS syntax error in pagination and login redirect loop #3645

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(ui): redirect authenticated users from login page to dashboard#3461

fix(ui): redirect authenticated users from login page to dashboard#3461
crivetimihai merged 8 commits intomainfrom
3430-fix-redirecting-to-dashboard-auth-users

marekdano commented Mar 4, 2026

Uh oh!

Lang-Akshay left a comment

Uh oh!

crivetimihai commented Mar 5, 2026

Uh oh!

gcgoncalves left a comment

Uh oh!

crivetimihai left a comment

Uh oh!

crivetimihai commented Mar 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

marekdano commented Mar 4, 2026

🐛 Bug-fix PR

📌 Summary

🔁 Reproduction Steps

🐞 Root Cause

💡 Fix Description

Changes

Behavior

🧪 Verification

📐 MCP Compliance (if relevant)

✅ Checklist

Uh oh!

Lang-Akshay left a comment

Choose a reason for hiding this comment

Thanks for the PR @marekdano . Great work. Please address the following findings.

Findings Summary

Suggestions :

Deny-Path Test Coverage

Uh oh!

crivetimihai commented Mar 5, 2026

Uh oh!

gcgoncalves left a comment

Choose a reason for hiding this comment

Uh oh!

crivetimihai left a comment

Choose a reason for hiding this comment

Uh oh!

crivetimihai commented Mar 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants