From 7f8d22fdb4bb308099820864372768e8fea4a734 Mon Sep 17 00:00:00 2001 From: Deimos Date: Thu, 20 Dec 2018 16:33:43 -0700 Subject: [PATCH] Fix permissions as required by shortener I did a bad job of testing how this would work, and didn't account properly for people being considered not-logged-in while they're on tild.es. This should fix the issues, but it re-adds the minor "data leak" of people without an invite being able to determine some data, such as existing users and groups. Since the site is going to be publicly visible in the near future, I don't think this is a significant concern. --- tildes/tests/webtests/test_user_page.py | 19 ------------------- tildes/tildes/resources/__init__.py | 6 ------ tildes/tildes/routes.py | 4 +++- tildes/tildes/views/shortener.py | 7 ++++--- 4 files changed, 7 insertions(+), 29 deletions(-) delete mode 100644 tildes/tests/webtests/test_user_page.py diff --git a/tildes/tests/webtests/test_user_page.py b/tildes/tests/webtests/test_user_page.py deleted file mode 100644 index 6522ad3..0000000 --- a/tildes/tests/webtests/test_user_page.py +++ /dev/null @@ -1,19 +0,0 @@ -# Copyright (c) 2018 Tildes contributors -# SPDX-License-Identifier: AGPL-3.0-or-later - - -def test_loggedout_username_leak(webtest_loggedout, session_user): - """Ensure responses from existing and nonexistent users are the same. - - Since logged-out users are currently blocked from seeing user pages, this makes sure - that there isn't a data leak where it's possible to tell if a particular username - exists or not. - """ - existing_user = webtest_loggedout.get( - "/user/" + session_user.username, expect_errors=True - ) - nonexistent_user = webtest_loggedout.get( - "/user/thisdoesntexist", expect_errors=True - ) - - assert existing_user.status == nonexistent_user.status diff --git a/tildes/tildes/resources/__init__.py b/tildes/tildes/resources/__init__.py index 0406b95..d8472a1 100644 --- a/tildes/tildes/resources/__init__.py +++ b/tildes/tildes/resources/__init__.py @@ -11,12 +11,6 @@ from tildes.models import DatabaseModel, ModelQuery def get_resource(request: Request, base_query: ModelQuery) -> DatabaseModel: """Prepare and execute base query from a root factory, returning result.""" - # While the site is private, we don't want to leak information about which usernames - # or groups exist. So we should just always raise a 403 before doing a lookup and - # potentially raising a 404. - if not request.user: - raise HTTPForbidden - query = base_query.lock_based_on_request_method().join_all_relationships() if not request.is_safe_method: diff --git a/tildes/tildes/routes.py b/tildes/tildes/routes.py index 1b0d88d..1ad81d1 100644 --- a/tildes/tildes/routes.py +++ b/tildes/tildes/routes.py @@ -91,7 +91,9 @@ def includeme(config: Configurator) -> None: add_intercooler_routes(config) # Add routes for the link-shortener under the /shortener path - config.add_route("shortener", "/shortener") + # The trailing slash is required for the base /shortener/ path because of the way + # nginx's proxy_pass will forward the urls from the shortener + config.add_route("shortener", "/shortener/") with config.route_prefix_context("/shortener"): config.add_route("shortener_group", "/~{group_path}", factory=group_by_path) config.add_route("shortener_topic", "/{topic_id36}", factory=topic_by_id36) diff --git a/tildes/tildes/views/shortener.py b/tildes/tildes/views/shortener.py index a3de4cf..e6c363b 100644 --- a/tildes/tildes/views/shortener.py +++ b/tildes/tildes/views/shortener.py @@ -6,23 +6,24 @@ from pyramid.httpexceptions import HTTPFound from pyramid.request import Request from pyramid.response import Response +from pyramid.security import NO_PERMISSION_REQUIRED from pyramid.view import view_config -@view_config(route_name="shortener") +@view_config(route_name="shortener", permission=NO_PERMISSION_REQUIRED) def get_shortener(request: Request) -> Response: """Display a message if someone just visits the base shortener domain.""" # pylint: disable=unused-argument return Response("Link-shortener for tildes.net") -@view_config(route_name="shortener_group") +@view_config(route_name="shortener_group", permission=NO_PERMISSION_REQUIRED) def get_shortener_group(request: Request) -> HTTPFound: """Redirect to the base path of a group.""" raise HTTPFound(location=f"/~{request.context.path}") -@view_config(route_name="shortener_topic") +@view_config(route_name="shortener_topic", permission=NO_PERMISSION_REQUIRED) def get_shortener_topic(request: Request) -> HTTPFound: """Redirect to the full permalink for a topic.""" raise HTTPFound(location=request.context.permalink)