diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 46b9573e6..48be79c22 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -28,6 +28,7 @@ Version v39.0.0 - Add ALTCHA verification in UI - Add API/ UI support for Patch/PackageCommitPatch - Fix failing V2 pipelinea +- Improve altcha challenge flow and reduce session timeout (https://github.com/aboutcode-org/vulnerablecode/pull/2348) Version v38.6.0 diff --git a/vulnerabilities/forms.py b/vulnerabilities/forms.py index 0f46cd871..122f8125a 100644 --- a/vulnerabilities/forms.py +++ b/vulnerabilities/forms.py @@ -136,4 +136,4 @@ class AdvisoryToDoForm(forms.Form): class AltchaForm(forms.Form): - altcha = AltchaField() + altcha = AltchaField(auto="onload") diff --git a/vulnerabilities/middleware/altcha_protection.py b/vulnerabilities/middleware/altcha_protection.py index 2bfcf6503..ed1db446e 100644 --- a/vulnerabilities/middleware/altcha_protection.py +++ b/vulnerabilities/middleware/altcha_protection.py @@ -8,35 +8,39 @@ # import time +from urllib.parse import urlencode from django.shortcuts import redirect from django.utils.deprecation import MiddlewareMixin +SESSION_TIMEOUT = 900 # 15 minutes + +ALTCHA_PROTECTED_PREFIXES = ( + "/packages/", + "/vulnerabilities/", + "/advisories/", + "/affected-by-advisories/v2/", + "/fixing-advisories/v2/", + "/pipelines/", +) -class AltchaProtectionMiddleware(MiddlewareMixin): - PROTECTED_PREFIXES = ( - "/packages/", - "/vulnerabilities/", - "/advisories/", - "/affected-by-advisories/v2/", - "/fixing-advisories/v2/", - ) - SESSION_TIMEOUT = 3600 # 1 hour +class AltchaProtectionMiddleware(MiddlewareMixin): def __call__(self, request): - protected = any(request.path.startswith(prefix) for prefix in self.PROTECTED_PREFIXES) + protected = any(request.path.startswith(prefix) for prefix in ALTCHA_PROTECTED_PREFIXES) if not protected: return self.get_response(request) verified_at = request.session.get("altcha_verified_at") + next_url = request.get_full_path() if not verified_at: - return redirect(f"/altcha/") + return redirect(f"/altcha/?{urlencode({'next': next_url})}") - if time.time() - verified_at > self.SESSION_TIMEOUT: + if time.time() - verified_at > SESSION_TIMEOUT: request.session.pop("altcha_verified_at", None) - return redirect(f"/altcha/") + return redirect(f"/altcha/?{urlencode({'next': next_url})}") return self.get_response(request) diff --git a/vulnerabilities/migrations/0137_alter_pipelineschedule_run_interval.py b/vulnerabilities/migrations/0137_alter_pipelineschedule_run_interval.py index 0f440ff8d..1b31e008a 100644 --- a/vulnerabilities/migrations/0137_alter_pipelineschedule_run_interval.py +++ b/vulnerabilities/migrations/0137_alter_pipelineschedule_run_interval.py @@ -26,7 +26,7 @@ def revert_convert_hours_to_minutes(apps, schema_editor): migrations.AlterField( model_name="pipelineschedule", name="run_interval", - field=models.PositiveSmallIntegerField( + field=models.IntegerField( default=1440, help_text="Number of minutes to wait between run of this pipeline.", validators=[ diff --git a/vulnerabilities/templates/altcha.html b/vulnerabilities/templates/altcha.html index 071f625e0..907926b47 100644 --- a/vulnerabilities/templates/altcha.html +++ b/vulnerabilities/templates/altcha.html @@ -1,5 +1,36 @@ -
- {% csrf_token %} - {{ form }} - -
\ No newline at end of file +{% extends "base.html" %} +{% load static %} + +{% block content %} +
+
+
+
+
+

+ Please verify that you are human to continue +

+ +
+ {% csrf_token %} + +
+
+ {{ form }} +
+
+ +
+
+ +
+
+
+
+
+
+
+
+{% endblock %} \ No newline at end of file diff --git a/vulnerabilities/tests/test_altcha_protection.py b/vulnerabilities/tests/test_altcha_protection.py index f7f209c95..0232c3511 100644 --- a/vulnerabilities/tests/test_altcha_protection.py +++ b/vulnerabilities/tests/test_altcha_protection.py @@ -22,7 +22,7 @@ def test_protected_url_redirects_without_session(self, client): response = client.get("/packages/search/") assert response.status_code == 302 - assert response.url == "/altcha/" + assert response.url == "/altcha/?next=%2Fpackages%2Fsearch%2F" def test_unprotected_url_is_accessible(self, client): response = client.get("/") @@ -46,7 +46,7 @@ def test_expired_session_redirects(self, client): response = client.get("/packages/search/") assert response.status_code == 302 - assert response.url == "/altcha/" + assert response.url == "/altcha/?next=%2Fpackages%2Fsearch%2F" def test_expired_session_is_removed(self, client): session = client.session diff --git a/vulnerabilities/utils.py b/vulnerabilities/utils.py index ff229a5f1..a4fe0274f 100644 --- a/vulnerabilities/utils.py +++ b/vulnerabilities/utils.py @@ -20,12 +20,11 @@ from functools import total_ordering from http import HTTPStatus from typing import List -from typing import NamedTuple from typing import Optional -from typing import Set from typing import Tuple from typing import Union from unittest.mock import MagicMock +from urllib.parse import unquote from urllib.parse import urljoin import dateparser @@ -36,6 +35,8 @@ from cwe2.database import Database from cwe2.database import InvalidCWEError from django.db.models import Prefetch +from django.shortcuts import redirect +from django.utils.http import url_has_allowed_host_and_scheme from packageurl import PackageURL from packageurl.contrib.django.utils import without_empty_values from univers.version_range import RANGE_CLASS_BY_SCHEMES @@ -44,6 +45,7 @@ from univers.version_range import VersionRange from aboutcode.hashid import build_vcid +from vulnerabilities.middleware.altcha_protection import ALTCHA_PROTECTED_PREFIXES logger = logging.getLogger(__name__) @@ -1114,3 +1116,16 @@ def build_alias_to_advisory_map(aliases_strs): ): alias_to_advisories[advisory.advisory_id].add(advisory) return alias_to_advisories + + +def safe_altcha_redirect(next_url: str) -> redirect: + """Safely redirect to Altcha protected URL, block external URLs.""" + is_safe = url_has_allowed_host_and_scheme(url=next_url, allowed_hosts=None, require_https=False) + + decoded_url = unquote(next_url) if next_url else "" + is_protected = any(decoded_url.startswith(prefix) for prefix in ALTCHA_PROTECTED_PREFIXES) + + if is_safe and is_protected: + return redirect(next_url) + + return redirect("/") diff --git a/vulnerabilities/views.py b/vulnerabilities/views.py index 3c11ea556..5d69b4a57 100644 --- a/vulnerabilities/views.py +++ b/vulnerabilities/views.py @@ -16,7 +16,6 @@ from cvss.exceptions import CVSS2MalformedError from cvss.exceptions import CVSS3MalformedError from cvss.exceptions import CVSS4MalformedError -from django import forms from django.contrib import messages from django.contrib.auth.views import LoginView from django.core.cache import cache @@ -29,7 +28,6 @@ from django.http import HttpResponse from django.http.response import Http404 from django.shortcuts import get_object_or_404 -from django.shortcuts import redirect from django.shortcuts import render from django.urls import reverse_lazy from django.views import View @@ -38,7 +36,6 @@ from django.views.generic.edit import FormMixin from django.views.generic.edit import FormView from django.views.generic.list import ListView -from django_altcha import AltchaField from vulnerabilities import models from vulnerabilities.forms import AdminLoginForm @@ -49,6 +46,7 @@ from vulnerabilities.forms import PackageSearchForm from vulnerabilities.forms import PipelineSchedulePackageForm from vulnerabilities.forms import VulnerabilitySearchForm +from vulnerabilities.middleware.altcha_protection import SESSION_TIMEOUT as ALTCHA_SESSION_TIMEOUT from vulnerabilities.models import ISSUE_TYPE_CHOICES from vulnerabilities.models import AdvisorySetMember from vulnerabilities.models import AdvisoryToDoV2 @@ -66,6 +64,7 @@ from vulnerabilities.throttling import AnonUserUIThrottle from vulnerabilities.utils import TYPES_WITH_MULTIPLE_IMPORTERS from vulnerabilities.utils import get_advisories_from_groups +from vulnerabilities.utils import safe_altcha_redirect from vulnerablecode import __version__ as VULNERABLECODE_VERSION from vulnerablecode.settings import env @@ -1165,6 +1164,19 @@ class AltchaView(FormView): template_name = "altcha.html" form_class = AltchaForm + def dispatch(self, request, *args, **kwargs): + """Do not show Altcha challenge to already validated user.""" + verified_at = request.session.get("altcha_verified_at") + + if verified_at: + if time.time() - verified_at < ALTCHA_SESSION_TIMEOUT: + next_url = request.GET.get("next", "/") + return safe_altcha_redirect(next_url) + + return super().dispatch(request, *args, **kwargs) + def form_valid(self, form): self.request.session["altcha_verified_at"] = time.time() - return redirect("/") + + next_url = self.request.GET.get("next", "/") + return safe_altcha_redirect(next_url)