From 94ac31a043467cb51a7fb00e37cd0a3bc60bb548 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Wed, 3 Jul 2024 15:39:09 -0700 Subject: [PATCH 1/2] Reorder authentication classes for consistent dev and production errors. Check user_id not id for forbidden redirects. --- kolibri/core/assets/src/core-app/client.js | 16 ++++++++++------ kolibri/deployment/default/settings/dev.py | 4 +++- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/kolibri/core/assets/src/core-app/client.js b/kolibri/core/assets/src/core-app/client.js index 3355258c4f5..bb589a797d4 100644 --- a/kolibri/core/assets/src/core-app/client.js +++ b/kolibri/core/assets/src/core-app/client.js @@ -36,15 +36,19 @@ baseClient.interceptors.response.use( // client code is still trying to access data that they would be allowed to see // if they were logged in. if (error.response) { - if (error.response.status === 403 || error.response.status === 401) { - if (!store.state.core.session.id) { - // Don't have any session information, so assume that this - // page has just been reopened and the session has expired. - // Redirect now! + if (error.response.status === 403) { + if (store.state.core.session.id && !store.state.core.session.user_id) { + // We have session information but no user_id, which means we are not logged in + // This is a sign that the user has been logged out due to inactivity heartbeat.signOutDueToInactivity(); } else { // In this case, we should check right now if they are still logged in - heartbeat.pollSessionEndPoint(); + heartbeat.pollSessionEndPoint().then(() => { + // If they are not, we should handle sign out + if (!store.state.core.session.user_id) { + heartbeat.signOutDueToInactivity(); + } + }); } } // On every error, check to see if the status code is one of our designated diff --git a/kolibri/deployment/default/settings/dev.py b/kolibri/deployment/default/settings/dev.py index d97e46cc3df..07d38b3455e 100644 --- a/kolibri/deployment/default/settings/dev.py +++ b/kolibri/deployment/default/settings/dev.py @@ -26,9 +26,11 @@ REST_FRAMEWORK = { "UNAUTHENTICATED_USER": "kolibri.core.auth.models.KolibriAnonymousUser", "DEFAULT_AUTHENTICATION_CLASSES": [ + # Always keep this first, so that we consistently return 403 responses + # when a request is unauthenticated. + "rest_framework.authentication.SessionAuthentication", # Activate basic auth for external API testing tools "rest_framework.authentication.BasicAuthentication", - "rest_framework.authentication.SessionAuthentication", ], "DEFAULT_RENDERER_CLASSES": ( "rest_framework.renderers.JSONRenderer", From d7608df5574584faacb9cf3ff4f105ab08db2446 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Wed, 3 Jul 2024 16:02:01 -0700 Subject: [PATCH 2/2] Redirect a logged out user back to the last page they were on. --- kolibri/core/assets/src/heartbeat.js | 2 +- kolibri/core/assets/src/utils/redirectBrowser.js | 9 +++++++-- kolibri/core/views.py | 11 +++++++++++ 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/kolibri/core/assets/src/heartbeat.js b/kolibri/core/assets/src/heartbeat.js index 9c78fe954c8..93eba89efee 100644 --- a/kolibri/core/assets/src/heartbeat.js +++ b/kolibri/core/assets/src/heartbeat.js @@ -266,7 +266,7 @@ export class HeartBeat { Lockr.set(SIGNED_OUT_DUE_TO_INACTIVITY, true); // Redirect the user to let the server sort out where they should // be now - redirectBrowser(); + redirectBrowser(null, true); } _sessionUrl(id) { return urls['kolibri:core:session-detail'](id); diff --git a/kolibri/core/assets/src/utils/redirectBrowser.js b/kolibri/core/assets/src/utils/redirectBrowser.js index 00ad647732c..9d7bc913e24 100644 --- a/kolibri/core/assets/src/utils/redirectBrowser.js +++ b/kolibri/core/assets/src/utils/redirectBrowser.js @@ -1,5 +1,10 @@ import urls from 'kolibri.urls'; -export default function redirectBrowser(url) { - window.location.href = url || urls['kolibri:core:redirect_user'](); +export default function redirectBrowser(url, next = false) { + url = url || urls['kolibri:core:redirect_user'](); + const urlObject = new URL(url, window.location.origin); + if (next) { + urlObject.searchParams.set('next', encodeURIComponent(window.location.href)); + } + window.location.href = urlObject.href; } diff --git a/kolibri/core/views.py b/kolibri/core/views.py index 4b59d063cc1..b810a27ed34 100644 --- a/kolibri/core/views.py +++ b/kolibri/core/views.py @@ -10,6 +10,7 @@ from django.urls import reverse from django.urls import translate_url from django.utils.decorators import method_decorator +from django.utils.http import url_has_allowed_host_and_scheme from django.utils.translation import check_for_language from django.utils.translation import gettext_lazy as _ from django.utils.translation import LANGUAGE_SESSION_KEY @@ -179,6 +180,16 @@ def get(self, request): else: url = get_url_by_role(user_kinds.ANONYMOUS) if url: + next_url = request.GET.get("next") + if next_url: + # Step 2: Validate the next_url + if url_has_allowed_host_and_scheme( + next_url, + allowed_hosts={request.get_host()}, + require_https=request.is_secure(), + ): + # Step 3: Append next_url to the base url if it's valid + url = f"{url}?next={next_url}" return HttpResponseRedirect(url) raise Http404( _(