Merge pull request #2348 from aboutcode-org/altcha-timeout

Improve altcha challenge flow and reduce session timeout

Author: keshav-space

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

vulnerabilities/forms.py

@@ -136,4 +136,4 @@ class AdvisoryToDoForm(forms.Form):
 
 
 class AltchaForm(forms.Form):
-    altcha = AltchaField()
+    altcha = AltchaField(auto="onload")

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)

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=[

vulnerabilities/templates/altcha.html

@@ -1,5 +1,36 @@
-<form method="post">
-    {% csrf_token %}
-    {{ form }}
-    <button type="submit">Continue</button>
-</form>
+{% extends "base.html" %}
+{% load static %}
+
+{% block content %}
+<section class="section">
+    <div class="columns is-centered mt-4 pt-4">
+        <div class="column is-4">
+            <div class="card">
+                <div class="card-content">
+                    <h1 class="title is-5 has-text-centered mt-4">
+                        Please verify that you are human to continue
+                    </h1>
+
+                    <form method="post">
+                        {% csrf_token %}
+
+                        <div class="has-text-centered ml-5 pt-4">
+                            <div style="display:inline-block; min-width: 304px;">
+                                {{ form }}
+                            </div>
+                        </div>
+
+                        <div class="field mt-4 pt-2">
+                            <div class="control">
+                                <button type="submit" class="button is-info is-fullwidth">
+                                    Continue
+                                </button>
+                            </div>
+                        </div>
+                    </form>
+                </div>
+            </div>
+        </div>
+    </div>
+</section>
+{% endblock %}

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

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("/")

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)