Skip to content

Commit

Permalink
Remove custom token error handling (#49)
Browse files Browse the repository at this point in the history
Token errors now raise a PermissionDenied error and are handled by Django.
  • Loading branch information
hugorodgerbrown authored Mar 22, 2021
1 parent a43a79a commit 09843d2
Show file tree
Hide file tree
Showing 16 changed files with 67 additions and 221 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/tox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ jobs:
strategy:
matrix:
python: [3.8,3.9]
django: [31,master]
django: [31,main]

env:
TOXENV: py${{ matrix.python }}-django${{ matrix.django }}
Expand Down
7 changes: 7 additions & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@

All notable changes to this project will be documented in this file.

## [0.2] - 2021-03-22

### Removed

- Logging of token use errors - this has never worked
- Custom 403_TEMPLATE use (and setting)

## [0.12] - 2020-08-17

No functional changes - just updating classifiers and CI support.
Expand Down
6 changes: 3 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tool.poetry]
name = "django-request-token"
version = "0.13"
version = "0.2.b0"
description = "JWT-backed Django app for managing querystring tokens."
license = "MIT"
authors = ["YunoJuno <[email protected]>"]
Expand All @@ -9,6 +9,7 @@ readme = "README.rst"
homepage = "https://github.com/yunojuno/django-request-token"
repository = "https://github.com/yunojuno/django-request-token"
classifiers = [
"Development Status :: 4 - Beta",
"Environment :: Web Environment",
"Framework :: Django :: 2.2",
"Framework :: Django :: 3.0",
Expand All @@ -25,7 +26,6 @@ packages = [{ include = "request_token" }]
python = "^3.7"
django = "^2.2 || ^3.0"
pyjwt = "^2.0"
sqlparse = "*"

[tool.poetry.dev-dependencies]
tox = "*"
Expand All @@ -41,7 +41,7 @@ flake8-docstrings = "*"
isort = "*"
mypy = "*"
pre-commit = "*"
black = "==19.10b0"
black = { version="*", allow-prereleases=true }

[build-system]
requires = ["poetry>=0.12"]
Expand Down
3 changes: 1 addition & 2 deletions request_token/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from django.utils.safestring import mark_safe
from django.utils.timezone import now as tz_now

from .models import RequestToken, RequestTokenErrorLog, RequestTokenLog
from .models import RequestToken, RequestTokenLog


def pretty_print(data: Optional[dict]) -> Optional[str]:
Expand Down Expand Up @@ -108,5 +108,4 @@ class RequestTokenErrorLogAdmin(admin.ModelAdmin):


admin.site.register(RequestToken, RequestTokenAdmin)
admin.site.register(RequestTokenErrorLog, RequestTokenErrorLogAdmin)
admin.site.register(RequestTokenLog, RequestTokenLogAdmin)
20 changes: 0 additions & 20 deletions request_token/apps.py
Original file line number Diff line number Diff line change
@@ -1,30 +1,10 @@
from __future__ import annotations

from django.apps import AppConfig
from django.core.exceptions import ImproperlyConfigured
from django.template import TemplateDoesNotExist, loader

from .settings import FOUR03_TEMPLATE


class RequestTokenAppConfig(AppConfig):
"""AppConfig for request_token app."""

name = "request_token"
verbose_name = "JWT Request Tokens"

def ready(self) -> None:
"""Validate config and connect signals."""
super(RequestTokenAppConfig, self).ready()
if FOUR03_TEMPLATE:
check_template(FOUR03_TEMPLATE)


def check_template(template: str) -> None:
"""Check for the existance of the custom 403 page."""
try:
loader.get_template(template)
except TemplateDoesNotExist:
raise ImproperlyConfigured(
f"Custom request token template does not exist: '{template}'"
)
26 changes: 5 additions & 21 deletions request_token/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,16 @@

import json
import logging
import sys
from typing import Callable

from django.core.exceptions import ImproperlyConfigured
from django.http import HttpResponseForbidden
from django.core.exceptions import ImproperlyConfigured, PermissionDenied
from django.http.request import HttpRequest
from django.http.response import HttpResponse
from django.template import loader
from jwt.exceptions import InvalidTokenError

from .models import RequestToken
from .settings import FOUR03_TEMPLATE, JWT_QUERYSTRING_ARG
from .settings import JWT_QUERYSTRING_ARG
from .utils import decode

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -93,20 +92,5 @@ def process_exception(
) -> HttpResponse:
"""Handle all InvalidTokenErrors."""
if isinstance(exception, InvalidTokenError):
logger.exception("JWT request token error")
response = _403(request, exception)
if getattr(request, "token", None):
request.token.log(request, response, error=exception)
return response


def _403(request: HttpRequest, exception: Exception) -> HttpResponseForbidden:
"""Render HttpResponseForbidden for exception."""
if FOUR03_TEMPLATE:
html = loader.render_to_string(
template_name=FOUR03_TEMPLATE,
context={"token_error": str(exception), "exception": exception},
request=request,
)
return HttpResponseForbidden(html, reason=str(exception))
return HttpResponseForbidden(reason=str(exception))
logger.error("JWT request token error", exc_info=sys.exc_info())
raise PermissionDenied("Invalid request token.")
16 changes: 16 additions & 0 deletions request_token/migrations/0012_delete_requesttokenerrorlog.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Generated by Django 3.1.7 on 2021-03-22 18:56

from django.db import migrations


class Migration(migrations.Migration):

dependencies = [
("request_token", "0011_update_model_meta_options"),
]

operations = [
migrations.DeleteModel(
name="RequestTokenErrorLog",
),
]
90 changes: 8 additions & 82 deletions request_token/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@
from django.http import HttpRequest, HttpResponse
from django.utils.timezone import now as tz_now
from django.utils.translation import gettext_lazy as _lazy
from jwt.exceptions import InvalidAudienceError, InvalidTokenError
from jwt.exceptions import InvalidAudienceError

from .compat import JSONField
from .exceptions import MaxUseError
from .settings import DEFAULT_MAX_USES, JWT_SESSION_TOKEN_EXPIRY, LOG_TOKEN_ERRORS
from .settings import DEFAULT_MAX_USES, JWT_SESSION_TOKEN_EXPIRY
from .utils import encode, to_seconds

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -302,28 +302,8 @@ def authenticate(self, request: HttpRequest) -> HttpRequest:
return self._auth_is_authenticated(request)

@transaction.atomic
def log(
self,
request: HttpRequest,
response: HttpResponse,
error: Optional[InvalidTokenError] = None,
) -> RequestTokenLog:
"""
Record the use of a token.
This is used by the decorator to log each time someone uses the token,
or tries to. Used for reporting, diagnostics.
Args:
request: the HttpRequest object that used the token, from which the
user, ip and user-agenct are extracted.
response: the corresponding HttpResponse object, from which the status
code is extracted.
error: an InvalidTokenError that gets logged as a RequestTokenError.
Returns a RequestTokenUse object.
"""
def log(self, request: HttpRequest, response: HttpResponse) -> RequestTokenLog:
"""Record the use of a token."""

def rmg(key: str, default: Any = None) -> Any:
return request.META.get(key, default)
Expand All @@ -332,16 +312,12 @@ def rmg(key: str, default: Any = None) -> Any:
token=self,
user=None if request.user.is_anonymous else request.user,
user_agent=rmg("HTTP_USER_AGENT", "unknown"),
client_ip=parse_xff(rmg("HTTP_X_FORWARDED_FOR"))
or rmg("REMOTE_ADDR", None),
client_ip=(
parse_xff(rmg("HTTP_X_FORWARDED_FOR")) or rmg("REMOTE_ADDR", None)
),
status_code=response.status_code,
).save()
if error and LOG_TOKEN_ERRORS:
RequestTokenErrorLog.objects.create_error_log(log, error)
# NB this will include all error logs - which means that an error log
# may prohibit further use of the token. Is there a scenario in which
# this would be the wrong outcome?
self.used_to_date = self.logs.filter(error__isnull=True).count()
self.used_to_date = self.logs.count()
self.save()
return log

Expand Down Expand Up @@ -427,53 +403,3 @@ def save(self, *args: Any, **kwargs: Any) -> RequestToken:
self.timestamp = self.timestamp or tz_now()
super(RequestTokenLog, self).save(*args, **kwargs)
return self


class RequestTokenErrorLogQuerySet(models.query.QuerySet):
def create_error_log(
self, log: RequestTokenLog, error: Exception
) -> RequestTokenErrorLog:
return RequestTokenErrorLog(
token=log.token,
log=log,
error_type=type(error).__name__,
error_message=str(error),
)


class RequestTokenErrorLog(models.Model):
"""Used to log errors that occur with the use of a RequestToken."""

token = models.ForeignKey(
RequestToken,
related_name="errors",
on_delete=models.CASCADE,
help_text="The RequestToken that was used.",
db_index=True,
)
log = models.OneToOneField(
RequestTokenLog,
related_name="error",
on_delete=models.CASCADE,
help_text="The token use against which the error occurred.",
db_index=True,
)
error_type = models.CharField(
max_length=50, help_text="The underlying type of error raised."
)
error_message = models.CharField(
max_length=200, help_text="The error message supplied."
)

objects = RequestTokenErrorLogQuerySet().as_manager()

class Meta:
verbose_name = "Error"
verbose_name_plural = "Errors"

def __str__(self) -> str:
return self.error_message

def save(self, *args: Any, **kwargs: Any) -> RequestTokenErrorLog:
super(RequestTokenErrorLog, self).save(*args, **kwargs)
return self
28 changes: 3 additions & 25 deletions request_token/settings.py
Original file line number Diff line number Diff line change
@@ -1,31 +1,9 @@
from os import getenv
from typing import Any, Optional

from django.conf import settings


def _env_or_setting(key: str, default: Any) -> Any:
return getenv(key) or getattr(settings, key, default)


# the name of GET argument to contain the token
JWT_QUERYSTRING_ARG = _env_or_setting("REQUEST_TOKEN_QUERYSTRING", "rt") # type: str
JWT_QUERYSTRING_ARG: str = getattr(settings, "REQUEST_TOKEN_QUERYSTRING", "rt")

# the fixed expiration check on Session tokens
JWT_SESSION_TOKEN_EXPIRY = int(_env_or_setting("REQUEST_TOKEN_EXPIRY", 10)) # type: int

# Set the default 403 template value
FOUR03_TEMPLATE = _env_or_setting(
"REQUEST_TOKEN_403_TEMPLATE", None
) # type: Optional[str]

# log all InvalidTokenErrors
LOG_TOKEN_ERRORS = _env_or_setting(
"REQUEST_TOKEN_LOG_TOKEN_ERRORS", "True"
).lower() in (
"true",
"1",
) # noqa

JWT_SESSION_TOKEN_EXPIRY: int = getattr(settings, "REQUEST_TOKEN_EXPIRY", 10)

DEFAULT_MAX_USES = _env_or_setting("REQUEST_TOKEN_DEFAULT_MAX_USES", 10)
DEFAULT_MAX_USES: int = getattr(settings, "REQUEST_TOKEN_DEFAULT_MAX_USES", 10)
11 changes: 8 additions & 3 deletions tests/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,19 @@
import django

DJANGO_VERSION = StrictVersion(django.get_version())
assert DJANGO_VERSION >= StrictVersion("2.2")
if not DJANGO_VERSION >= StrictVersion("2.2"):
raise RuntimeError("Invalid Django version")

DEBUG = True

try:
from django.db.models import JSONField # noqa: F401

DATABASES = {
"default": {"ENGINE": "django.db.backends.sqlite3", "NAME": "test.db",}
"default": {
"ENGINE": "django.db.backends.sqlite3",
"NAME": "test.db",
}
}
except ImportError:
DATABASES = {
Expand Down Expand Up @@ -99,4 +103,5 @@
"loggers": {"request_token": {"handlers": ["console"], "level": "DEBUG"}},
}

assert DEBUG is True, "This project is only intended to be used for testing."
if not DEBUG:
raise Exception("This project is only intended to be used for testing.")
3 changes: 0 additions & 3 deletions tests/templates/403.html

This file was deleted.

16 changes: 0 additions & 16 deletions tests/test_apps.py

This file was deleted.

Loading

0 comments on commit 09843d2

Please sign in to comment.