exceptions: Extract json_unauths into MissingAuthenticationError.

We raise two types of json_unauthorized when
MissingAuthenticationError is raised. Raising the one
with www_authenticate let's the client know that user needs
to be logged in to access the requested content.

Sending `www_authenticate='session'` header with the response
also stops modern web-browsers from showing a login form to the
user and let's the client handle it completely.

Structurally, this moves the handling of common authentication errors
to a single shared middleware exception handler.
This commit is contained in:
Aman
2020-08-22 23:50:42 +05:30
committed by Tim Abbott
parent 81893c9dbb
commit fd5423a8f9
3 changed files with 40 additions and 17 deletions

View File

@@ -45,6 +45,7 @@ class ErrorCode(AbstractEnum):
REQUEST_CONFUSING_VAR = ()
INVALID_API_KEY = ()
INVALID_ZOOM_TOKEN = ()
UNAUTHENTICATED_USER = ()
class JsonableError(Exception):
'''A standardized error format we can turn into a nice JSON HTTP response.
@@ -266,3 +267,13 @@ class UnexpectedWebhookEventType(JsonableError):
@staticmethod
def msg_format() -> str:
return _("The '{event_type}' event isn't currently supported by the {webhook_name} webhook")
class MissingAuthenticationError(JsonableError):
code = ErrorCode.UNAUTHENTICATED_USER
http_status_code = 401
def __init__(self) -> None:
pass
# No msg_format is defined since this exception is caught and
# converted into json_unauthorized in Zulip's middleware.

View File

@@ -1,8 +1,7 @@
from functools import wraps
from typing import Any, Dict, cast
from django.conf import settings
from django.http import HttpRequest, HttpResponse, HttpResponseRedirect
from django.http import HttpRequest, HttpResponse
from django.utils.cache import add_never_cache_headers
from django.utils.module_loading import import_string
from django.views.decorators.csrf import csrf_exempt, csrf_protect
@@ -13,7 +12,8 @@ from zerver.decorator import (
authenticated_uploads_api_view,
process_as_post,
)
from zerver.lib.response import json_method_not_allowed, json_unauthorized
from zerver.lib.exceptions import MissingAuthenticationError
from zerver.lib.response import json_method_not_allowed
from zerver.lib.types import ViewFuncT
METHODS = ('GET', 'HEAD', 'POST', 'PUT', 'DELETE', 'PATCH')
@@ -142,22 +142,17 @@ def rest_dispatch(request: HttpRequest, **kwargs: Any) -> HttpResponse:
)(target_function)
# Pick a way to tell user they're not authed based on how the request was made
else:
# If this looks like a request from a top-level page in a
# browser, send the user to the login page
if 'text/html' in request.META.get('HTTP_ACCEPT', ''):
# TODO: It seems like the `?next=` part is unlikely to be helpful
return HttpResponseRedirect(f'{settings.HOME_NOT_LOGGED_IN}?next={request.path}')
# Ask for basic auth (email:apiKey)
elif request.path.startswith("/api"):
return json_unauthorized()
# Logged out user accessing an endpoint with anonymous user access on JSON; proceed.
elif request.path.startswith("/json") and 'allow_anonymous_user_web' in view_flags:
# `allow_anonymous_user_web` calls are only restricted to /json calls used
# by our webapp.
# TODO: Allow /api calls when this is stable enough.
if request.path.startswith("/json") and 'allow_anonymous_user_web' in view_flags:
auth_kwargs = dict(allow_unauthenticated=True)
target_function = csrf_protect(authenticated_json_view(
target_function, **auth_kwargs))
# Session cookie expired, notify the client
else:
return json_unauthorized(www_authenticate='session')
# Don't allow anonymous queries to endpoints witout `allow_anonymous_user_web` flag.
raise MissingAuthenticationError()
if request.method not in ["GET", "POST"]:
# process_as_post needs to be the outer decorator, because

View File

@@ -7,7 +7,7 @@ from typing import Any, AnyStr, Callable, Dict, Iterable, List, MutableMapping,
from django.conf import settings
from django.core.handlers.wsgi import WSGIRequest
from django.db import connection
from django.http import HttpRequest, HttpResponse, StreamingHttpResponse
from django.http import HttpRequest, HttpResponse, HttpResponseRedirect, StreamingHttpResponse
from django.middleware.common import CommonMiddleware
from django.shortcuts import render
from django.utils.deprecation import MiddlewareMixin
@@ -19,11 +19,11 @@ from sentry_sdk.integrations.logging import ignore_logger
from zerver.lib.cache import get_remote_cache_requests, get_remote_cache_time
from zerver.lib.db import reset_queries
from zerver.lib.debug import maybe_tracemalloc_listen
from zerver.lib.exceptions import ErrorCode, JsonableError, RateLimited
from zerver.lib.exceptions import ErrorCode, JsonableError, MissingAuthenticationError, RateLimited
from zerver.lib.html_to_text import get_content_description
from zerver.lib.markdown import get_markdown_requests, get_markdown_time
from zerver.lib.rate_limiter import RateLimitResult
from zerver.lib.response import json_error, json_response_from_error
from zerver.lib.response import json_error, json_response_from_error, json_unauthorized
from zerver.lib.subdomains import get_subdomain
from zerver.lib.types import ViewFuncT
from zerver.lib.utils import statsd
@@ -301,6 +301,23 @@ class JsonErrorHandler(MiddlewareMixin):
ignore_logger("zerver.middleware.json_error_handler")
def process_exception(self, request: HttpRequest, exception: Exception) -> Optional[HttpResponse]:
if isinstance(exception, MissingAuthenticationError):
if 'text/html' in request.META.get('HTTP_ACCEPT', ''):
# If this looks like a request from a top-level page in a
# browser, send the user to the login page.
#
# TODO: The next part is a bit questionable; it will
# execute the likely intent for intentionally visiting
# an API endpoint without authentication in a browser,
# but that's an unlikely to be done intentionally often.
return HttpResponseRedirect(f'{settings.HOME_NOT_LOGGED_IN}?next={request.path}')
if request.path.startswith("/api"):
# For API routes, ask for HTTP basic auth (email:apiKey).
return json_unauthorized()
else:
# For /json routes, ask for session authentication.
return json_unauthorized(www_authenticate='session')
if isinstance(exception, JsonableError):
return json_response_from_error(exception)
if request.error_format == "JSON":