Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .yarnrc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,4 @@ supportedArchitectures:
libc:
- current
- glibc
enableScripts: false
9 changes: 9 additions & 0 deletions RELEASE.rst
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
Release Notes
=============

Version 0.45.1
--------------

- allow multiple problem files (#2512)
- Celery task tweaks (#2545)
- feat: installs granian as a replacement process manager for production / hosted envs. (#2544)
- security: disable yarn postinstall scripts (#2543)
- Separate Login and Signup URLs (again) (#2535)

Version 0.45.0 (Released September 24, 2025)
--------------

Expand Down
38 changes: 21 additions & 17 deletions authentication/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,26 @@
log = logging.getLogger(__name__)


def get_redirect_url(request):
def get_redirect_url(request, param_names):
"""
Get the redirect URL from the request.

Args:
request: Django request object
param_names: Names of the GET parameter or cookie to look for the redirect URL;
first match will be used.

Returns:
str: Redirect URL
"""
next_url = request.GET.get("next") or request.COOKIES.get("next")
return (
next_url
if next_url
and url_has_allowed_host_and_scheme(
for param_name in param_names:
next_url = request.GET.get(param_name) or request.COOKIES.get(param_name)
if next_url and url_has_allowed_host_and_scheme(
next_url, allowed_hosts=settings.ALLOWED_REDIRECT_HOSTS
)
else "/app"
)
):
return next_url

return "/app"


class CustomLogoutView(View):
Expand All @@ -51,7 +52,7 @@ def get(
GET endpoint reached after logging a user out from Keycloak
"""
user = getattr(request, "user", None)
user_redirect_url = get_redirect_url(request)
user_redirect_url = get_redirect_url(request, ["next"])
if user and user.is_authenticated:
logout(request)
if request.META.get(ApisixUserMiddleware.header):
Expand All @@ -77,7 +78,8 @@ def get(
"""
GET endpoint for logging a user in.
"""
redirect_url = get_redirect_url(request)
redirect_url = get_redirect_url(request, ["next"])
signup_redirect_url = get_redirect_url(request, ["signup_next", "next"])
if not request.user.is_anonymous:
profile = request.user.profile

Expand All @@ -104,12 +106,14 @@ def get(
redirect_url = urljoin(
settings.APP_BASE_URL, f"/dashboard/organization/{org_slug}"
)
elif (
not profile.has_logged_in
and request.GET.get("skip_onboarding", "0") == "0"
):
params = urlencode({"next": redirect_url})
redirect_url = f"{settings.MITOL_NEW_USER_LOGIN_URL}?{params}"
# first-time non-org users
elif not profile.has_logged_in:
if request.GET.get("skip_onboarding", "0") == "0":
params = urlencode({"next": signup_redirect_url})
redirect_url = f"{settings.MITOL_NEW_USER_LOGIN_URL}?{params}"
profile.save()
else:
redirect_url = signup_redirect_url

if not profile.has_logged_in:
profile.has_logged_in = True
Expand Down
150 changes: 74 additions & 76 deletions authentication/views_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,44 @@

import pytest
from django.test import RequestFactory
from django.urls import reverse
from django.utils.http import urlencode

from authentication.views import CustomLoginView, get_redirect_url


@pytest.mark.parametrize(
("next_url", "allowed"),
("param_names", "expected_redirect"),
[
("/app", True),
("http://open.odl.local:8062/search", True),
("http://open.odl.local:8069/search", False),
("https://ocw.mit.edu", True),
("https://fake.fake.edu", False),
(["exists-a"], "/url-a"),
(["exists-b"], "/url-b"),
(["exists-a", "exists-b"], "/url-a"),
(["exists-b", "exists-a"], "/url-b"),
(["not-exists-x", "exists-a"], "/url-a"),
(["not-exists-x", "not-exists-y"], "/app"),
# With disallowed hosts in the params
(["disallowed-1"], "/app"),
(["not-exists-x", "disallowed-1"], "/app"),
(["disallowed-1", "exists-a"], "/url-a"),
(["allowed-2"], "https://good.com/url-2"),
],
)
def test_custom_login(mocker, next_url, allowed):
def test_get_redirect_url(mocker, param_names, expected_redirect):
"""Next url should be respected if host is allowed"""
mock_request = mocker.MagicMock(GET={"next": next_url})
assert get_redirect_url(mock_request) == (next_url if allowed else "/app")
GET = {
"exists-a": "/url-a",
"exists-b": "/url-b",
"exists-c": "/url-c",
"disallowed-a": "https://malicious.com/url-1",
"allowed-2": "https://good.com/url-2",
}
mocker.patch(
"authentication.views.settings.ALLOWED_REDIRECT_HOSTS",
["good.com"],
)

mock_request = mocker.MagicMock(GET=GET)
assert get_redirect_url(mock_request, param_names) == expected_redirect


@pytest.mark.parametrize(
Expand Down Expand Up @@ -120,23 +140,41 @@ def test_custom_logout_view(mocker, client, user, is_authenticated, has_next):
assert resp.url == (next_url if has_next else "/app")


def test_custom_login_view_authenticated_user_with_onboarding(mocker):
"""Test CustomLoginView for an authenticated user who has never logged in before"""
@pytest.mark.parametrize(
(
"req_data",
"expected_redirect",
),
[
(
{"next": "/irrelevant", "signup_next": "/this?after=signup"},
"/this?after=signup",
),
(
{"next": "/redirect?here=ok"}, # falls back to next
"/redirect?here=ok",
),
],
)
@pytest.mark.parametrize(
("skip_onboarding", "expect_onboarding"),
[
(None, True), # default behavior is to do onboarding
("0", True), # explicit skip_onboarding=0 means do onboarding
("1", False), # explicit skip_onboarding=1 means skip onboarding
],
)
def test_custom_login_view_authenticated_user_needs_onboarding(
mocker, req_data, expected_redirect, skip_onboarding, expect_onboarding
):
"""Test CustomLoginView for an authenticated user with incomplete onboarding"""
factory = RequestFactory()
request = factory.get("/login/", {"next": "/dashboard"})
if skip_onboarding is not None:
req_data["skip_onboarding"] = skip_onboarding
request = factory.get(reverse("login"), req_data)

request.user = MagicMock(is_anonymous=False)
request.user.profile = MagicMock(has_logged_in=False)

# Mock redirect to avoid URL resolution issues
mock_redirect = mocker.patch("authentication.views.redirect")
mock_redirect.return_value = MagicMock(
status_code=302, url="/onboarding?next=/search?resource=184"
)

mocker.patch("authentication.views.get_redirect_url", return_value="/dashboard")
mocker.patch(
"authentication.views.urlencode", return_value="next=/search?resource=184"
)
mocker.patch(
"authentication.views.settings.MITOL_NEW_USER_LOGIN_URL", "/onboarding"
)
Expand All @@ -145,72 +183,41 @@ def test_custom_login_view_authenticated_user_with_onboarding(mocker):
response = CustomLoginView().get(request)

assert response.status_code == 302
assert response.url == "/onboarding?next=/search?resource=184"

# Verify redirect was called with the onboarding URL
mock_redirect.assert_called_once_with("/onboarding?next=/search?resource=184")


def test_custom_login_view_authenticated_user_skip_onboarding(mocker):
"""Test skip_onboarding flag skips redirect to onboarding"""
factory = RequestFactory()
request = factory.get("/login/", {"next": "/dashboard", "skip_onboarding": "1"})
request.user = MagicMock(is_anonymous=False)
request.user.profile = MagicMock(has_logged_in=False)

# Mock redirect to avoid URL resolution issues
mock_redirect = mocker.patch("authentication.views.redirect")
mock_redirect.return_value = MagicMock(status_code=302, url="/dashboard")

mocker.patch("authentication.views.get_redirect_url", return_value="/dashboard")
mocker.patch("authentication.views.decode_apisix_headers", return_value={})

response = CustomLoginView().get(request)

# Verify has_logged_in was set to True and profile was saved
assert request.user.profile.has_logged_in is True
request.user.profile.save.assert_called_once()

assert response.status_code == 302
assert response.url == "/dashboard"
if expect_onboarding:
assert response.url == f"/onboarding?{urlencode({'next': expected_redirect})}"
else:
assert response.url == expected_redirect


def test_custom_login_view_authenticated_user_who_has_logged_in_before(mocker):
"""Test that user who has logged in before is redirected to next url"""
factory = RequestFactory()
request = factory.get("/login/", {"next": "/dashboard"})
request = factory.get(
reverse("login"),
{"next": "/should-be-redirect?foo", "signup_next": "/irrelevant"},
)
request.user = MagicMock(is_anonymous=False)
request.user.profile = MagicMock(has_logged_in=True)

# Mock redirect to avoid URL resolution issues
mock_redirect = mocker.patch("authentication.views.redirect")
mock_redirect.return_value = MagicMock(status_code=302, url="/dashboard")

mocker.patch("authentication.views.get_redirect_url", return_value="/dashboard")
mocker.patch("authentication.views.decode_apisix_headers", return_value={})

response = CustomLoginView().get(request)

assert response.status_code == 302
assert response.url == "/dashboard"
assert response.url == "/should-be-redirect?foo"


def test_custom_login_view_anonymous_user(mocker):
"""Test redirect for anonymous user"""
factory = RequestFactory()
request = factory.get("/login/", {"next": "/dashboard"})
request = factory.get(
reverse("login"), {"next": "/some-url", "signup_next": "/irrelevant"}
)
request.user = MagicMock(is_anonymous=True)

# Mock redirect to avoid URL resolution issues
mock_redirect = mocker.patch("authentication.views.redirect")
mock_redirect.return_value = MagicMock(status_code=302, url="/dashboard")

mocker.patch("authentication.views.get_redirect_url", return_value="/dashboard")

response = CustomLoginView().get(request)

assert response.status_code == 302
assert response.url == "/dashboard"
assert response.url == "/some-url"


def test_custom_login_view_first_time_login_sets_has_logged_in(mocker):
Expand All @@ -228,12 +235,6 @@ def test_custom_login_view_first_time_login_sets_has_logged_in(mocker):

request.user = mock_user

# Mock the redirect function to avoid URL resolution
mock_redirect = mocker.patch("authentication.views.redirect")
mock_redirect.return_value = MagicMock(status_code=302, url="/dashboard")
mocker.patch("authentication.views.get_redirect_url", return_value="/dashboard")
mocker.patch("authentication.views.decode_apisix_headers", return_value={})

response = CustomLoginView().get(request)

# Verify the response
Expand All @@ -243,9 +244,6 @@ def test_custom_login_view_first_time_login_sets_has_logged_in(mocker):
assert mock_profile.has_logged_in is True
mock_profile.save.assert_called_once()

# Verify redirect was called with the correct URL
mock_redirect.assert_called_once_with("/dashboard")


@pytest.mark.parametrize(
"test_case",
Expand Down
2 changes: 1 addition & 1 deletion docker-compose.apps.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ services:
command: >
/bin/bash -c '
sleep 3;
celery -A main.celery:app worker -E -Q default,edx_content -B --scheduler redbeat.RedBeatScheduler -l ${MITOL_LOG_LEVEL:-INFO}'
celery -A main.celery:app worker -E -Q default,edx_content,embeddings -B --scheduler redbeat.RedBeatScheduler -l ${MITOL_LOG_LEVEL:-INFO}'
depends_on:
db:
condition: service_healthy
Expand Down
17 changes: 13 additions & 4 deletions frontends/main/src/app-pages/B2BAttachPage/B2BAttachPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ import {
import * as commonUrls from "@/common/urls"
import { Permission } from "api/hooks/user"
import B2BAttachPage from "./B2BAttachPage"
import invariant from "tiny-invariant"

// Mock next-nprogress-bar for App Router
const mockPush = jest.fn()
const mockPush = jest.fn<void, [string]>()
jest.mock("next-nprogress-bar", () => ({
useRouter: () => ({
push: mockPush,
Expand All @@ -37,10 +38,18 @@ describe("B2BAttachPage", () => {
})

await waitFor(() => {
expect(mockPush).toHaveBeenCalledWith(
expect.stringMatching(/login.*next=.*skip_onboarding=1/),
)
expect(mockPush).toHaveBeenCalledOnce()
})

const url = new URL(mockPush.mock.calls[0][0])
expect(url.searchParams.get("skip_onboarding")).toBe("1")
const nextUrl = url.searchParams.get("next")
const signupNextUrl = url.searchParams.get("signup_next")
invariant(nextUrl)
invariant(signupNextUrl)
const attachView = commonUrls.b2bAttachView("test-code")
expect(new URL(nextUrl).pathname).toBe(attachView)
expect(new URL(signupNextUrl).pathname).toBe(attachView)
})

test("Renders when logged in", async () => {
Expand Down
13 changes: 10 additions & 3 deletions frontends/main/src/app-pages/B2BAttachPage/B2BAttachPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,16 @@ const B2BAttachPage: React.FC<B2BAttachPageProps> = ({ code }) => {
return
}
if (!user?.is_authenticated) {
const loginUrlString = urls.login({
pathname: urls.b2bAttachView(code),
searchParams: new URLSearchParams(),
const loginUrlString = urls.auth({
loginNext: {
pathname: urls.b2bAttachView(code),
searchParams: null,
},
// On signup, redirect to the attach page so attachment can occur.
signupNext: {
pathname: urls.b2bAttachView(code),
searchParams: null,
},
})
const loginUrl = new URL(loginUrlString)
loginUrl.searchParams.set("skip_onboarding", "1")
Expand Down
Loading
Loading