mirror of
https://github.com/inventree/InvenTree.git
synced 2026-11-14 12:32:44 +00:00
fix(backend): preserve redirect target through /accounts/login/ (#13279)
AuthRequiredMiddleware redirects unauthenticated requests for server-rendered paths to /accounts/login/?next=<path>, but the account_login redirect view then bounced straight to the frontend root, dropping the target. Forward a safe relative 'next' parameter to the SPA's /logged-in page instead, where LoggedIn.tsx recovers it once the login flow completes. Absolute or protocol-relative targets are dropped to rule out open redirects.
This commit is contained in:
1 parent
ed0bbc3c75
commit
80ad822688
4 files changed
+95
-5
No files matched your search
@@ -97,4 +97,45 @@ class ViewTests(InvenTreeTestCase):
|
||||
f'/accounts/login/?next=/&login={self.username}&password={self.password}'
|
||||
)
|
||||
self.assertEqual(response.status_code, 302)
|
||||
self.assertEqual(response.url, '/web/logged-in/?next=%2F')
|
||||
|
||||
def test_account_login_redirect(self):
|
||||
"""Test that the login redirect preserves the 'next' parameter."""
|
||||
self.client.logout()
|
||||
|
||||
# AuthRequiredMiddleware -> /accounts/login/?next=<requested path>
|
||||
response = self.client.get('/part/42/')
|
||||
self.assertEqual(response.status_code, 302)
|
||||
self.assertEqual(response.url, '/accounts/login/?next=/part/42/')
|
||||
|
||||
# -> SPA /logged-in, with the target (and any query string) kept
|
||||
response = self.client.get(response.url)
|
||||
self.assertEqual(response.url, '/web/logged-in/?next=%2Fpart%2F42%2F')
|
||||
|
||||
response = self.client.get('/accounts/login/?next=/part/42/?search=abc')
|
||||
self.assertEqual(
|
||||
response.url, '/web/logged-in/?next=%2Fpart%2F42%2F%3Fsearch%3Dabc'
|
||||
)
|
||||
|
||||
# Old build order links are remapped to their SPA route
|
||||
for legacy in ['/build/42/', '/build/42']:
|
||||
response = self.client.get(f'/accounts/login/?next={legacy}')
|
||||
self.assertEqual(
|
||||
response.url,
|
||||
'/web/logged-in/?next=%2Fmanufacturing%2Fbuild-order%2F42%2F',
|
||||
f'next={legacy} was not remapped',
|
||||
)
|
||||
|
||||
# Absolute URLs must not be forwarded (open redirect prevention);
|
||||
# fall back to the plain frontend redirect
|
||||
for unsafe in [
|
||||
'https://example.org/evil/',
|
||||
'//example.org/evil/',
|
||||
'http://example.org/evil/',
|
||||
]:
|
||||
response = self.client.get(f'/accounts/login/?next={unsafe}')
|
||||
self.assertEqual(response.url, '/web', f'next={unsafe} must not be kept')
|
||||
|
||||
# No next parameter at all
|
||||
response = self.client.get(reverse('account_login'))
|
||||
self.assertEqual(response.url, '/web')
|
||||
@@ -45,7 +45,7 @@ from .api import (
|
||||
)
|
||||
from .config import get_setting
|
||||
from .magic_login import GetSimpleLoginView
|
||||
from .views import auth_request
|
||||
from .views import AccountLoginRedirectView, auth_request
|
||||
|
||||
# Set admin header from config or use default
|
||||
admin.site.site_header = get_setting(
|
||||
@@ -145,10 +145,8 @@ backendpatterns = [
|
||||
# SCIM 2 provisioning endpoint
|
||||
path('scim/v2/', include(scim.api)),
|
||||
path(
|
||||
'accounts/login/',
|
||||
RedirectView.as_view(url=f'/{settings.FRONTEND_URL_BASE}', permanent=False),
|
||||
name='account_login',
|
||||
), # Add a redirect for login views
|
||||
'accounts/login/', AccountLoginRedirectView.as_view(), name='account_login'
|
||||
), # Redirect for login views; keeps any 'next' parameter through to the SPA
|
||||
path('anymail/', include('anymail.urls')), # Emails
|
||||
]
|
||||
|
||||
|
||||
@@ -4,7 +4,35 @@ In particular these views provide base functionality for rendering Django forms
|
||||
as JSON objects and passing them to modal forms (using jQuery / bootstrap).
|
||||
"""
|
||||
|
||||
from urllib.parse import quote
|
||||
|
||||
from django.conf import settings
|
||||
from django.http import HttpResponse
|
||||
from django.views.generic import RedirectView
|
||||
|
||||
from InvenTree.helpers import pui_url
|
||||
|
||||
|
||||
class AccountLoginRedirectView(RedirectView):
|
||||
"""Redirect ``/accounts/login/`` into the SPA, keeping the ``next`` target.
|
||||
|
||||
The target is recovered by LoggedIn.tsx after login; only relative
|
||||
in-app paths are forwarded, to prevent open redirects.
|
||||
"""
|
||||
|
||||
permanent = False
|
||||
|
||||
def get_redirect_url(self, *args, **kwargs):
|
||||
"""Return the URL to redirect to."""
|
||||
next_url = self.request.GET.get('next', '')
|
||||
|
||||
if next_url.startswith('/') and not next_url.startswith('//'):
|
||||
parts = next_url.strip('/').split('/')
|
||||
if len(parts) == 2 and parts[0] == 'build' and parts[1].isdigit():
|
||||
next_url = f'/manufacturing/build-order/{parts[1]}/'
|
||||
return f'{pui_url("logged-in/")}?next={quote(next_url, safe="")}'
|
||||
|
||||
return f'/{settings.FRONTEND_URL_BASE}'
|
||||
|
||||
|
||||
def auth_request(request):
|
||||
|
||||
@@ -135,6 +135,29 @@ test('SSO - Complete Registration', async ({ page }) => {
|
||||
.waitFor();
|
||||
});
|
||||
|
||||
test('SSO - Deep Link Redirect Target', async ({ page }) => {
|
||||
// A CUI deep link visited while logged out must return the user to the
|
||||
// requested page after logging in via SSO
|
||||
await setSettingState({ setting: 'LOGIN_ENABLE_SSO', value: true });
|
||||
await setSettingState({ setting: 'LOGIN_ENABLE_SSO_REG', value: true });
|
||||
await setSettingState({ setting: 'LOGIN_SIGNUP_SSO_AUTO', value: true });
|
||||
|
||||
await navigate(page, logoutUrl, { waitUntil: 'load' });
|
||||
await page.waitForURL('**/web/login');
|
||||
|
||||
// The CUI compatibility redirect only exists on the backend
|
||||
const backendUrl = new URL('/', apiUrl).toString();
|
||||
|
||||
await page.goto(`${backendUrl}part/3/`, { waitUntil: 'load' });
|
||||
await page.waitForURL('**/web/login');
|
||||
|
||||
await page.getByRole('button', { name: 'Mock SSO' }).click();
|
||||
|
||||
// Back on the originally requested part
|
||||
await page.waitForURL(/\/web\/part\/3(\/|$)/);
|
||||
await page.getByRole('button', { name: 'navigation-menu' }).waitFor();
|
||||
});
|
||||
|
||||
test('SSO - Registration Disabled', async ({ page }) => {
|
||||
// Allow SSO login, but disable self-registration - a brand-new SSO
|
||||
// identity has no matching local account, and nothing should silently
|
||||
|
||||
Reference in new issue
Block a user