From 10a2afbf27b052ba3561709bcda1ae2924b90cd2 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Fri, 19 Feb 2021 09:10:38 +0300 Subject: Refreshes User Data On Form Submit Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- backend/routes/forms/submit.py | 46 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 8 deletions(-) (limited to 'backend/routes/forms/submit.py') diff --git a/backend/routes/forms/submit.py b/backend/routes/forms/submit.py index d8e6d35..ec9b24f 100644 --- a/backend/routes/forms/submit.py +++ b/backend/routes/forms/submit.py @@ -3,6 +3,7 @@ Submit a form. """ import binascii +import datetime import hashlib import uuid from typing import Any, Optional @@ -15,7 +16,8 @@ from starlette.background import BackgroundTask from starlette.requests import Request from starlette.responses import JSONResponse -from backend.constants import FRONTEND_URL, FormFeatures, HCAPTCHA_API_SECRET +from backend import constants +from backend.authentication.user import User from backend.models import Form, FormResponse from backend.route import Route from backend.validation import AuthorizationHeaders, ErrorMessage, api @@ -56,8 +58,36 @@ class SubmitForm(Route): ) async def post(self, request: Request) -> JSONResponse: """Submit a response to the form.""" - data = await request.json() + response = await self.submit(request) + + # Silently try to update user data + try: + if hasattr(request.user, User.refresh_data.__name__): + old = request.user.token + await request.user.refresh_data() + + if old != request.user.token: + try: + expiry = datetime.datetime.fromisoformat( + request.user.decoded_token.get("expiry") + ) + except ValueError: + expiry = None + + response.set_cookie( + "BackendToken", f"JWT {request.user.token}", + secure=constants.PRODUCTION, httponly=True, samesite="strict", + max_age=(expiry - datetime.datetime.now()).seconds + ) + except httpx.HTTPStatusError: + pass + + return response + + async def submit(self, request: Request) -> JSONResponse: + """Helper method for handling submission logic.""" + data = await request.json() data["timestamp"] = None if form := await request.state.db.forms.find_one( @@ -68,7 +98,7 @@ class SubmitForm(Route): response["id"] = str(uuid.uuid4()) response["form_id"] = form.id - if FormFeatures.DISABLE_ANTISPAM.value not in form.features: + if constants.FormFeatures.DISABLE_ANTISPAM.value not in form.features: ip_hash_ctx = hashlib.md5() ip_hash_ctx.update(request.client.host.encode()) ip_hash = binascii.hexlify(ip_hash_ctx.digest()) @@ -78,7 +108,7 @@ class SubmitForm(Route): async with httpx.AsyncClient() as client: query_params = { - "secret": HCAPTCHA_API_SECRET, + "secret": constants.HCAPTCHA_API_SECRET, "response": data.get("captcha") } r = await client.post( @@ -95,11 +125,11 @@ class SubmitForm(Route): "captcha_pass": captcha_data["success"] } - if FormFeatures.REQUIRES_LOGIN.value in form.features: + if constants.FormFeatures.REQUIRES_LOGIN.value in form.features: if request.user.is_authenticated: response["user"] = request.user.payload - if FormFeatures.COLLECT_EMAIL.value in form.features and "email" not in response["user"]: # noqa + if constants.FormFeatures.COLLECT_EMAIL.value in form.features and "email" not in response["user"]: # noqa return JSONResponse({ "error": "email_required" }, status_code=400) @@ -132,7 +162,7 @@ class SubmitForm(Route): ) send_webhook = None - if FormFeatures.WEBHOOK_ENABLED.value in form.features: + if constants.FormFeatures.WEBHOOK_ENABLED.value in form.features: send_webhook = BackgroundTask( self.send_submission_webhook, form=form, @@ -172,7 +202,7 @@ class SubmitForm(Route): embed = { "title": "New Form Response", "description": f"{mention} submitted a response to `{form.name}`.", - "url": f"{FRONTEND_URL}/path_to_view_form/{response.id}", # noqa # TODO: Enter Form View URL + "url": f"{constants.FRONTEND_URL}/path_to_view_form/{response.id}", # noqa # TODO: Enter Form View URL "timestamp": response.timestamp, "color": 7506394, } -- cgit v1.2.3 From 839525ef99ca353a107812cefb006e22a5f3f7f4 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Fri, 19 Feb 2021 09:49:09 +0300 Subject: Remove AuthorizationHeaders Class Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- backend/routes/forms/submit.py | 3 +-- backend/validation.py | 11 ----------- 2 files changed, 1 insertion(+), 13 deletions(-) (limited to 'backend/routes/forms/submit.py') diff --git a/backend/routes/forms/submit.py b/backend/routes/forms/submit.py index ec9b24f..55a4875 100644 --- a/backend/routes/forms/submit.py +++ b/backend/routes/forms/submit.py @@ -20,7 +20,7 @@ from backend import constants from backend.authentication.user import User from backend.models import Form, FormResponse from backend.route import Route -from backend.validation import AuthorizationHeaders, ErrorMessage, api +from backend.validation import ErrorMessage, api HCAPTCHA_VERIFY_URL = "https://hcaptcha.com/siteverify" HCAPTCHA_HEADERS = { @@ -53,7 +53,6 @@ class SubmitForm(Route): HTTP_404=ErrorMessage, HTTP_400=ErrorMessage ), - headers=AuthorizationHeaders, tags=["forms", "responses"] ) async def post(self, request: Request) -> JSONResponse: diff --git a/backend/validation.py b/backend/validation.py index e696683..8771924 100644 --- a/backend/validation.py +++ b/backend/validation.py @@ -1,6 +1,5 @@ """Utilities for providing API payload validation.""" -from typing import Optional from pydantic.fields import Field from pydantic.main import BaseModel from spectree import SpecTree @@ -18,13 +17,3 @@ class ErrorMessage(BaseModel): class OkayResponse(BaseModel): status: str = "ok" - - -class AuthorizationHeaders(BaseModel): - authorization: Optional[str] = Field( - title="Authorization", - description=( - "The Authorization JWT token received from the " - "authorize route in the format `JWT {token}`" - ) - ) -- cgit v1.2.3 From 3c4f7e71cb1ecdfd8d255b02cf44adcd90f32f01 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Sat, 20 Feb 2021 03:45:16 +0300 Subject: Centralizes Admin Authentication Sets admin authentication on authenticator to allow the addition and removal of admins without creating a new token. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- backend/authentication/backend.py | 9 ++------- backend/authentication/user.py | 9 +++++++++ backend/routes/forms/form.py | 2 +- backend/routes/forms/submit.py | 1 + 4 files changed, 13 insertions(+), 8 deletions(-) (limited to 'backend/routes/forms/submit.py') diff --git a/backend/authentication/backend.py b/backend/authentication/backend.py index abe7313..bdff796 100644 --- a/backend/authentication/backend.py +++ b/backend/authentication/backend.py @@ -60,13 +60,8 @@ class JWTAuthenticationBackend(authentication.AuthenticationBackend): except Exception: raise authentication.AuthenticationError("Could not parse user details.") - admin = await request.state.db.admins.find_one( - {"_id": user_details["id"]} - ) is not None - - if admin: - scopes.append("admin") - user = User(token, user_details) + if user.fetch_admin_status(request): + scopes.append("admin") return authentication.AuthCredentials(scopes), user diff --git a/backend/authentication/user.py b/backend/authentication/user.py index a1d78e5..52baa61 100644 --- a/backend/authentication/user.py +++ b/backend/authentication/user.py @@ -2,6 +2,7 @@ import typing as t import jwt from starlette.authentication import BaseUser +from starlette.requests import Request from backend.constants import SECRET_KEY from backend.discord import fetch_user_details @@ -13,6 +14,7 @@ class User(BaseUser): def __init__(self, token: str, payload: dict[str, t.Any]) -> None: self.token = token self.payload = payload + self.admin = False @property def is_authenticated(self) -> bool: @@ -32,6 +34,13 @@ class User(BaseUser): def decoded_token(self) -> dict[str, any]: return jwt.decode(self.token, SECRET_KEY, algorithms=["HS256"]) + def fetch_admin_status(self, request: Request) -> bool: + self.admin = request.state.db.admins.find_one( + {"_id": self.payload["id"]} + ) is not None + + return self.admin + async def refresh_data(self) -> None: """Fetches user data from discord, and updates the instance.""" self.payload = await fetch_user_details(self.decoded_token.get("token")) diff --git a/backend/routes/forms/form.py b/backend/routes/forms/form.py index b6b722e..e3360b1 100644 --- a/backend/routes/forms/form.py +++ b/backend/routes/forms/form.py @@ -26,7 +26,7 @@ class SingleForm(Route): @api.validate(resp=Response(HTTP_200=Form, HTTP_404=ErrorMessage), tags=["forms"]) async def get(self, request: Request) -> JSONResponse: """Returns single form information by ID.""" - admin = request.user.payload["admin"] if request.user.is_authenticated else False # noqa + admin = request.user.admin if request.user.is_authenticated else False filters = { "_id": request.path_params["form_id"] diff --git a/backend/routes/forms/submit.py b/backend/routes/forms/submit.py index 55a4875..8627a29 100644 --- a/backend/routes/forms/submit.py +++ b/backend/routes/forms/submit.py @@ -127,6 +127,7 @@ class SubmitForm(Route): if constants.FormFeatures.REQUIRES_LOGIN.value in form.features: if request.user.is_authenticated: response["user"] = request.user.payload + response["user"]["admin"] = request.user.admin if constants.FormFeatures.COLLECT_EMAIL.value in form.features and "email" not in response["user"]: # noqa return JSONResponse({ -- cgit v1.2.3 From 96c659fce17a5aca17fb913cf587765bac90481f Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Wed, 24 Feb 2021 12:08:11 +0100 Subject: Hook up unittesting in the submit protocol --- backend/routes/forms/submit.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'backend/routes/forms/submit.py') diff --git a/backend/routes/forms/submit.py b/backend/routes/forms/submit.py index d8e6d35..85a4226 100644 --- a/backend/routes/forms/submit.py +++ b/backend/routes/forms/submit.py @@ -18,6 +18,7 @@ from starlette.responses import JSONResponse from backend.constants import FRONTEND_URL, FormFeatures, HCAPTCHA_API_SECRET from backend.models import Form, FormResponse from backend.route import Route +from backend.routes.forms.unittesting import execute_unittest from backend.validation import AuthorizationHeaders, ErrorMessage, api HCAPTCHA_VERIFY_URL = "https://hcaptcha.com/siteverify" @@ -127,6 +128,19 @@ class SubmitForm(Route): except ValidationError as e: return JSONResponse(e.errors(), status_code=422) + has_unittests = any("unittests" in question.data for question in form.questions) + if has_unittests: + unittest_results = await execute_unittest(response_obj, form) + + was_successful = all(test.passed for test in unittest_results) + if not was_successful: + status_code = 500 if any(test.return_code == 99 for test in unittest_results) else 200 + + return JSONResponse({ + "error": "failed_tests", + "test_results": [test._asdict() for test in unittest_results if not test.passed] + }, status_code=status_code) + await request.state.db.responses.insert_one( response_obj.dict(by_alias=True) ) -- cgit v1.2.3 From da6b581185e8bbe37e561a05827c8517824c7d2c Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Wed, 24 Feb 2021 13:53:08 +0100 Subject: Switch to 100 chars line length and get rid of the noqas --- backend/constants.py | 8 ++++---- backend/models/form.py | 2 +- backend/routes/forms/form.py | 2 +- backend/routes/forms/submit.py | 15 +++++++++++---- backend/routes/forms/unittesting.py | 3 ++- tox.ini | 4 +++- 6 files changed, 22 insertions(+), 12 deletions(-) (limited to 'backend/routes/forms/submit.py') diff --git a/backend/constants.py b/backend/constants.py index cccf437..59b56e0 100644 --- a/backend/constants.py +++ b/backend/constants.py @@ -1,9 +1,9 @@ from dotenv import load_dotenv -load_dotenv() +import os +import binascii +from enum import Enum -import os # noqa -import binascii # noqa -from enum import Enum # noqa +load_dotenv() FRONTEND_URL = os.getenv("FRONTEND_URL", "https://forms.pythondiscord.com") diff --git a/backend/models/form.py b/backend/models/form.py index 8e59905..eac0b63 100644 --- a/backend/models/form.py +++ b/backend/models/form.py @@ -47,7 +47,7 @@ class Form(BaseModel): if any(v not in allowed_values for v in value): raise ValueError("Form features list contains one or more invalid values.") - if FormFeatures.COLLECT_EMAIL in value and FormFeatures.REQUIRES_LOGIN not in value: # noqa + if FormFeatures.COLLECT_EMAIL in value and FormFeatures.REQUIRES_LOGIN not in value: raise ValueError("COLLECT_EMAIL feature require REQUIRES_LOGIN feature.") return value diff --git a/backend/routes/forms/form.py b/backend/routes/forms/form.py index b6b722e..e5f7ec6 100644 --- a/backend/routes/forms/form.py +++ b/backend/routes/forms/form.py @@ -26,7 +26,7 @@ class SingleForm(Route): @api.validate(resp=Response(HTTP_200=Form, HTTP_404=ErrorMessage), tags=["forms"]) async def get(self, request: Request) -> JSONResponse: """Returns single form information by ID.""" - admin = request.user.payload["admin"] if request.user.is_authenticated else False # noqa + admin = request.user.payload["admin"] if request.user.is_authenticated else False filters = { "_id": request.path_params["form_id"] diff --git a/backend/routes/forms/submit.py b/backend/routes/forms/submit.py index 85a4226..c19fc2d 100644 --- a/backend/routes/forms/submit.py +++ b/backend/routes/forms/submit.py @@ -100,7 +100,10 @@ class SubmitForm(Route): if request.user.is_authenticated: response["user"] = request.user.payload - if FormFeatures.COLLECT_EMAIL.value in form.features and "email" not in response["user"]: # noqa + if ( + FormFeatures.COLLECT_EMAIL.value in form.features + and "email" not in response["user"] + ): return JSONResponse({ "error": "email_required" }, status_code=400) @@ -134,11 +137,15 @@ class SubmitForm(Route): was_successful = all(test.passed for test in unittest_results) if not was_successful: - status_code = 500 if any(test.return_code == 99 for test in unittest_results) else 200 + status_code = 500 if any( + test.return_code == 99 for test in unittest_results + ) else 200 return JSONResponse({ "error": "failed_tests", - "test_results": [test._asdict() for test in unittest_results if not test.passed] + "test_results": [ + test._asdict() for test in unittest_results if not test.passed + ] }, status_code=status_code) await request.state.db.responses.insert_one( @@ -186,7 +193,7 @@ class SubmitForm(Route): embed = { "title": "New Form Response", "description": f"{mention} submitted a response to `{form.name}`.", - "url": f"{FRONTEND_URL}/path_to_view_form/{response.id}", # noqa # TODO: Enter Form View URL + "url": f"{FRONTEND_URL}/path_to_view_form/{response.id}", # TODO: Enter Form View URL "timestamp": response.timestamp, "color": 7506394, } diff --git a/backend/routes/forms/unittesting.py b/backend/routes/forms/unittesting.py index 3e1d280..fe8320f 100644 --- a/backend/routes/forms/unittesting.py +++ b/backend/routes/forms/unittesting.py @@ -51,7 +51,8 @@ async def execute_unittest(form_response: FormResponse, form: Form) -> list[Unit unit_code = _make_unit_code(question.data["unittests"]) user_code = _make_user_code(form_response.response[question.id]) - code = TEST_TEMPLATE.replace("### USER CODE", user_code).replace("### UNIT CODE", unit_code) + code = TEST_TEMPLATE.replace("### USER CODE", user_code) + code = code.replace("### UNIT CODE", unit_code) # Make sure that the code is well formatted (we don't check for the user code) try: diff --git a/tox.ini b/tox.ini index 48a3da6..afb3b34 100644 --- a/tox.ini +++ b/tox.ini @@ -1,8 +1,10 @@ [flake8] -max-line-length=88 +max-line-length=100 exclude=.cache,.venv,.git docstring-convention=all import-order-style=pycharm ignore= # Type annotations ANN101,ANN102 + # Line breaks + W503 -- cgit v1.2.3 From d69f80e083ed1b9d91716609c7c063968aef22fa Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Wed, 24 Feb 2021 14:30:39 +0100 Subject: Return 403 on failed tests --- backend/routes/forms/submit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'backend/routes/forms/submit.py') diff --git a/backend/routes/forms/submit.py b/backend/routes/forms/submit.py index c19fc2d..7618a33 100644 --- a/backend/routes/forms/submit.py +++ b/backend/routes/forms/submit.py @@ -139,7 +139,7 @@ class SubmitForm(Route): if not was_successful: status_code = 500 if any( test.return_code == 99 for test in unittest_results - ) else 200 + ) else 403 return JSONResponse({ "error": "failed_tests", -- cgit v1.2.3 From 0a9026dcdd23eaf7c48256eb7da5af774892673b Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Wed, 24 Feb 2021 15:16:13 +0100 Subject: Document unittest code --- backend/routes/forms/submit.py | 2 ++ backend/routes/forms/unittesting.py | 20 ++++++++++++++------ resources/unittest_template.py | 13 +++++++------ 3 files changed, 23 insertions(+), 12 deletions(-) (limited to 'backend/routes/forms/submit.py') diff --git a/backend/routes/forms/submit.py b/backend/routes/forms/submit.py index 7618a33..d6b549e 100644 --- a/backend/routes/forms/submit.py +++ b/backend/routes/forms/submit.py @@ -131,12 +131,14 @@ class SubmitForm(Route): except ValidationError as e: return JSONResponse(e.errors(), status_code=422) + # Run unittests if needed has_unittests = any("unittests" in question.data for question in form.questions) if has_unittests: unittest_results = await execute_unittest(response_obj, form) was_successful = all(test.passed for test in unittest_results) if not was_successful: + # Return 500 if we encountered an internal error (code 99). status_code = 500 if any( test.return_code == 99 for test in unittest_results ) else 403 diff --git a/backend/routes/forms/unittesting.py b/backend/routes/forms/unittesting.py index 0cb7d8d..e038f3a 100644 --- a/backend/routes/forms/unittesting.py +++ b/backend/routes/forms/unittesting.py @@ -30,6 +30,7 @@ def filter_unittests(form: Form) -> Form: def _make_unit_code(units: dict[str, str]) -> str: + """Compose a dict mapping unit names to their code into an actual class body.""" result = "" for unit_name, unit_code in units.items(): @@ -39,14 +40,16 @@ def _make_unit_code(units: dict[str, str]) -> str: def _make_user_code(code: str) -> str: - # Make sure that we we escape triple quotes and backslashes in the user code - code = code.replace('"""', '\\"""').replace("\\", "\\\\") - return f'USER_CODE = """{code}"""' + """Compose the user code into an actual string variable.""" + # Make sure that we we escape triple quotes in the user code + code = code.replace('"""', '\\"""') + return f'USER_CODE = r"""{code}"""' async def _post_eval(code: str) -> Optional[dict[str, str]]: - data = {"input": code} + """Post the eval to snekbox and return the response.""" async with httpx.AsyncClient() as client: + data = {"input": code} response = await client.post(SNEKBOX_URL, json=data) if not response.status_code == 200: @@ -56,12 +59,14 @@ async def _post_eval(code: str) -> Optional[dict[str, str]]: async def execute_unittest(form_response: FormResponse, form: Form) -> list[UnittestResult]: + """Execute all the unittests in this form and return the results.""" unittest_results = [] for question in form.questions: if question.type == "code" and "unittests" in question.data: passed = False + # Tests starting with an hashtag should have censored names. hidden_test_counter = count(1) hidden_tests = { test.lstrip("#"): next(hidden_test_counter) @@ -69,19 +74,20 @@ async def execute_unittest(form_response: FormResponse, form: Form) -> list[Unit if test.startswith("#") } + # Compose runner code unit_code = _make_unit_code(question.data["unittests"]) user_code = _make_user_code(form_response.response[question.id]) code = TEST_TEMPLATE.replace("### USER CODE", user_code) code = code.replace("### UNIT CODE", unit_code) - # Make sure that the code is well formatted (we don't check for the user code) + # Make sure that the code is well formatted (we don't check for the user code). try: ast.parse(code) except SyntaxError: return_code = 99 result = "Invalid generated unit code." - + # The runner is correctly formatted, we can run it. else: response = await _post_eval(code) @@ -91,6 +97,7 @@ async def execute_unittest(form_response: FormResponse, form: Form) -> list[Unit else: return_code = int(response["returncode"]) + # Another code has been returned by CPython because of another failure. if return_code not in (0, 5, 99): return_code = 99 result = "Internal error." @@ -98,6 +105,7 @@ async def execute_unittest(form_response: FormResponse, form: Form) -> list[Unit stdout = response["stdout"] passed = bool(int(stdout[0])) + # If the test failed, we have to populate the result string. if not passed: failed_tests = stdout[1:].strip().split(";") diff --git a/resources/unittest_template.py b/resources/unittest_template.py index c792944..4c9b0bb 100644 --- a/resources/unittest_template.py +++ b/resources/unittest_template.py @@ -1,4 +1,5 @@ # flake8: noqa +"""This template is used inside snekbox to evaluate and test user code.""" import ast import io import os @@ -23,27 +24,26 @@ DEVNULL = SimpleNamespace(write=lambda *_: None, flush=lambda *_: None) RESULT = io.StringIO() ORIGINAL_STDOUT = sys.stdout +# stdout/err is patched in order to control what is outputted by the runner sys.stdout = DEVNULL sys.stderr = DEVNULL def _exit_sandbox(code: int) -> NoReturn: """ + Exit the sandbox by printing the result to the actual stdout and exit with the provided code. + Codes: - 0: Executed with success - 5: Syntax error while parsing user code - 99: Internal error """ - result_content = RESULT.getvalue() - - print( - f"{result_content}", - file=ORIGINAL_STDOUT - ) + print(RESULT.getvalue(), file=ORIGINAL_STDOUT, end="") sys.exit(code) def _load_user_module() -> ModuleType: + """Load the user code into a new module and return it.""" try: ast.parse(USER_CODE, "") except SyntaxError: @@ -74,6 +74,7 @@ def _main() -> None: try: + # Load the user code as a global module variable module = _load_user_module() _main() except Exception: -- cgit v1.2.3 From 06c01e78abcb0ab8713a3ad375218e98aab2882f Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Thu, 25 Feb 2021 14:44:26 +0100 Subject: Remove unneeded temp variable --- backend/routes/forms/submit.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'backend/routes/forms/submit.py') diff --git a/backend/routes/forms/submit.py b/backend/routes/forms/submit.py index d6b549e..b3a6afd 100644 --- a/backend/routes/forms/submit.py +++ b/backend/routes/forms/submit.py @@ -132,12 +132,10 @@ class SubmitForm(Route): return JSONResponse(e.errors(), status_code=422) # Run unittests if needed - has_unittests = any("unittests" in question.data for question in form.questions) - if has_unittests: + if any("unittests" in question.data for question in form.questions): unittest_results = await execute_unittest(response_obj, form) - was_successful = all(test.passed for test in unittest_results) - if not was_successful: + if not all(test.passed for test in unittest_results): # Return 500 if we encountered an internal error (code 99). status_code = 500 if any( test.return_code == 99 for test in unittest_results -- cgit v1.2.3 From 02154294da8b25bf7dae1b79f170aab888f92797 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Sat, 6 Mar 2021 22:42:52 +0300 Subject: Renames Token To `token` Changes the name for the token used to authorize with the backend. Co-authored-by: Joe Banks --- backend/authentication/backend.py | 2 +- backend/routes/auth/authorize.py | 4 ++-- backend/routes/forms/submit.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) (limited to 'backend/routes/forms/submit.py') diff --git a/backend/authentication/backend.py b/backend/authentication/backend.py index 206d1eb..c7590e9 100644 --- a/backend/authentication/backend.py +++ b/backend/authentication/backend.py @@ -33,7 +33,7 @@ class JWTAuthenticationBackend(authentication.AuthenticationBackend): self, request: Request ) -> t.Optional[tuple[authentication.AuthCredentials, authentication.BaseUser]]: """Handles JWT authentication process.""" - cookie = request.cookies.get("BackendToken") + cookie = request.cookies.get("token") if not cookie: return None diff --git a/backend/routes/auth/authorize.py b/backend/routes/auth/authorize.py index 65709ab..98f9887 100644 --- a/backend/routes/auth/authorize.py +++ b/backend/routes/auth/authorize.py @@ -41,7 +41,7 @@ async def process_token(bearer_token: dict) -> Union[AuthorizeResponse, AUTH_FAI try: user_details = await fetch_user_details(bearer_token["access_token"]) except httpx.HTTPStatusError: - AUTH_FAILURE.delete_cookie("BackendToken") + AUTH_FAILURE.delete_cookie("token") return AUTH_FAILURE max_age = datetime.timedelta(seconds=int(bearer_token["expires_in"])) @@ -63,7 +63,7 @@ async def process_token(bearer_token: dict) -> Union[AuthorizeResponse, AUTH_FAI }) response.set_cookie( - "BackendToken", f"JWT {token}", + "token", f"JWT {token}", secure=constants.PRODUCTION, httponly=True, samesite="strict", max_age=bearer_token["expires_in"] ) diff --git a/backend/routes/forms/submit.py b/backend/routes/forms/submit.py index 4224586..8680b2d 100644 --- a/backend/routes/forms/submit.py +++ b/backend/routes/forms/submit.py @@ -75,7 +75,7 @@ class SubmitForm(Route): expiry = None response.set_cookie( - "BackendToken", f"JWT {request.user.token}", + "token", f"JWT {request.user.token}", secure=constants.PRODUCTION, httponly=True, samesite="strict", max_age=(expiry - datetime.datetime.now()).seconds ) -- cgit v1.2.3 From 013ea9006352ed714cbbd561880770062ea3a0e9 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Sat, 6 Mar 2021 23:19:08 +0300 Subject: Sets Token Cookie To Same Site To Lax Sets the authorization token cookie's security policy to lax, to allow it to work on the site. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- backend/routes/auth/authorize.py | 2 +- backend/routes/forms/submit.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'backend/routes/forms/submit.py') diff --git a/backend/routes/auth/authorize.py b/backend/routes/auth/authorize.py index 26d8622..e00aef2 100644 --- a/backend/routes/auth/authorize.py +++ b/backend/routes/auth/authorize.py @@ -63,7 +63,7 @@ async def process_token(bearer_token: dict) -> Union[AuthorizeResponse, AUTH_FAI response.set_cookie( "token", f"JWT {token}", - secure=constants.PRODUCTION, httponly=True, samesite="strict", + secure=constants.PRODUCTION, httponly=True, samesite="lax", max_age=bearer_token["expires_in"] ) return response diff --git a/backend/routes/forms/submit.py b/backend/routes/forms/submit.py index 8680b2d..8803b7c 100644 --- a/backend/routes/forms/submit.py +++ b/backend/routes/forms/submit.py @@ -76,7 +76,7 @@ class SubmitForm(Route): response.set_cookie( "token", f"JWT {request.user.token}", - secure=constants.PRODUCTION, httponly=True, samesite="strict", + secure=constants.PRODUCTION, httponly=True, samesite="lax", max_age=(expiry - datetime.datetime.now()).seconds ) -- cgit v1.2.3 From b2ad14a87ab715eb403be68722914ed1c6b51d91 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Sat, 6 Mar 2021 23:32:21 +0300 Subject: Revert "Sets Token Cookie To Same Site To Lax" This reverts commit 013ea900 Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- backend/routes/auth/authorize.py | 2 +- backend/routes/forms/submit.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'backend/routes/forms/submit.py') diff --git a/backend/routes/auth/authorize.py b/backend/routes/auth/authorize.py index e00aef2..26d8622 100644 --- a/backend/routes/auth/authorize.py +++ b/backend/routes/auth/authorize.py @@ -63,7 +63,7 @@ async def process_token(bearer_token: dict) -> Union[AuthorizeResponse, AUTH_FAI response.set_cookie( "token", f"JWT {token}", - secure=constants.PRODUCTION, httponly=True, samesite="lax", + secure=constants.PRODUCTION, httponly=True, samesite="strict", max_age=bearer_token["expires_in"] ) return response diff --git a/backend/routes/forms/submit.py b/backend/routes/forms/submit.py index 8803b7c..8680b2d 100644 --- a/backend/routes/forms/submit.py +++ b/backend/routes/forms/submit.py @@ -76,7 +76,7 @@ class SubmitForm(Route): response.set_cookie( "token", f"JWT {request.user.token}", - secure=constants.PRODUCTION, httponly=True, samesite="lax", + secure=constants.PRODUCTION, httponly=True, samesite="strict", max_age=(expiry - datetime.datetime.now()).seconds ) -- cgit v1.2.3 From 5bab39126bb6b764595a4e21b454249c01628588 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Sun, 7 Mar 2021 00:07:19 +0300 Subject: Makes Helper To Handle Token SameSite Logic Adds a helper method to allow tokens to work on deploy previews. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- backend/constants.py | 6 +++-- backend/routes/auth/authorize.py | 49 ++++++++++++++++++++++++++++++---------- backend/routes/forms/submit.py | 9 ++++---- 3 files changed, 45 insertions(+), 19 deletions(-) (limited to 'backend/routes/forms/submit.py') diff --git a/backend/constants.py b/backend/constants.py index e1f4a5b..4bb7fd1 100644 --- a/backend/constants.py +++ b/backend/constants.py @@ -1,8 +1,9 @@ -from dotenv import load_dotenv -import os import binascii +import os from enum import Enum +from dotenv import load_dotenv + load_dotenv() @@ -12,6 +13,7 @@ MONGO_DATABASE = os.getenv("MONGO_DATABASE", "pydis_forms") SNEKBOX_URL = os.getenv("SNEKBOX_URL", "http://snekbox.default.svc.cluster.local/eval") PRODUCTION = os.getenv("PRODUCTION", "True").lower() != "false" +PRODUCTION_URL = "https://forms.pythondiscord.com/" OAUTH2_CLIENT_ID = os.getenv("OAUTH2_CLIENT_ID") OAUTH2_CLIENT_SECRET = os.getenv("OAUTH2_CLIENT_SECRET") diff --git a/backend/routes/auth/authorize.py b/backend/routes/auth/authorize.py index 26d8622..1e773d6 100644 --- a/backend/routes/auth/authorize.py +++ b/backend/routes/auth/authorize.py @@ -10,9 +10,9 @@ import jwt from pydantic.fields import Field from pydantic.main import BaseModel from spectree.response import Response +from starlette import responses from starlette.authentication import requires from starlette.requests import Request -from starlette.responses import JSONResponse from backend import constants from backend.authentication.user import User @@ -21,7 +21,7 @@ from backend.discord import fetch_bearer_token, fetch_user_details from backend.route import Route from backend.validation import ErrorMessage, api -AUTH_FAILURE = JSONResponse({"error": "auth_failure"}, status_code=400) +AUTH_FAILURE = responses.JSONResponse({"error": "auth_failure"}, status_code=400) class AuthorizeRequest(BaseModel): @@ -33,7 +33,7 @@ class AuthorizeResponse(BaseModel): expiry: str = Field("ISO formatted timestamp of expiry.") -async def process_token(bearer_token: dict) -> Union[AuthorizeResponse, AUTH_FAILURE]: +async def process_token(bearer_token: dict, origin: str) -> Union[AuthorizeResponse, AUTH_FAILURE]: """Post a bearer token to Discord, and return a JWT and username.""" interaction_start = datetime.datetime.now() @@ -56,17 +56,42 @@ async def process_token(bearer_token: dict) -> Union[AuthorizeResponse, AUTH_FAI token = jwt.encode(data, SECRET_KEY, algorithm="HS256") user = User(token, user_details) - response = JSONResponse({ + response = responses.JSONResponse({ "username": user.display_name, "expiry": token_expiry.isoformat() }) + await set_response_token(response, origin, token, bearer_token["expires_in"]) + return response + + +async def set_response_token( + response: responses.Response, + origin_url: str, + new_token: str, + expiry: int +) -> None: + """Helper that handles logic for updating a token in a set-cookie response.""" + if origin_url == constants.PRODUCTION_URL: + domain = constants.PRODUCTION_URL + samesite = "strict" + + elif not constants.PRODUCTION: + domain = None + samesite = "strict" + + else: + domain = origin_url + samesite = "None" + response.set_cookie( - "token", f"JWT {token}", - secure=constants.PRODUCTION, httponly=True, samesite="strict", - max_age=bearer_token["expires_in"] + "token", f"JWT {new_token}", + secure=constants.PRODUCTION, + httponly=True, + samesite=samesite, + domain=domain, + max_age=expiry ) - return response class AuthorizeRoute(Route): @@ -82,7 +107,7 @@ class AuthorizeRoute(Route): resp=Response(HTTP_200=AuthorizeResponse, HTTP_400=ErrorMessage), tags=["auth"] ) - async def post(self, request: Request) -> JSONResponse: + async def post(self, request: Request) -> responses.JSONResponse: """Generate an authorization token.""" data = await request.json() try: @@ -91,7 +116,7 @@ class AuthorizeRoute(Route): except httpx.HTTPStatusError: return AUTH_FAILURE - return await process_token(bearer_token) + return await process_token(bearer_token, url) class TokenRefreshRoute(Route): @@ -107,7 +132,7 @@ class TokenRefreshRoute(Route): resp=Response(HTTP_200=AuthorizeResponse, HTTP_400=ErrorMessage), tags=["auth"] ) - async def post(self, request: Request) -> JSONResponse: + async def post(self, request: Request) -> responses.JSONResponse: """Refresh an authorization token.""" try: token = request.user.decoded_token.get("refresh") @@ -116,4 +141,4 @@ class TokenRefreshRoute(Route): except httpx.HTTPStatusError: return AUTH_FAILURE - return await process_token(bearer_token) + return await process_token(bearer_token, url) diff --git a/backend/routes/forms/submit.py b/backend/routes/forms/submit.py index 8680b2d..975307b 100644 --- a/backend/routes/forms/submit.py +++ b/backend/routes/forms/submit.py @@ -20,6 +20,7 @@ from backend import constants from backend.authentication.user import User from backend.models import Form, FormResponse from backend.route import Route +from backend.routes.auth.authorize import set_response_token from backend.routes.forms.unittesting import execute_unittest from backend.validation import ErrorMessage, api @@ -74,11 +75,9 @@ class SubmitForm(Route): except ValueError: expiry = None - response.set_cookie( - "token", f"JWT {request.user.token}", - secure=constants.PRODUCTION, httponly=True, samesite="strict", - max_age=(expiry - datetime.datetime.now()).seconds - ) + origin = request.headers.get("origin") + expiry_seconds = (expiry - datetime.datetime.now()).seconds + await set_response_token(response, origin, request.user.token, expiry_seconds) except httpx.HTTPStatusError: pass -- cgit v1.2.3 From 8811959c6f13cdccb56d4fc72c1d9027e66d63d5 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Sun, 7 Mar 2021 00:37:42 +0300 Subject: Fixes Domain URL On Token Cookie Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- backend/routes/auth/authorize.py | 25 +++++++++++++++---------- backend/routes/forms/submit.py | 4 +++- 2 files changed, 18 insertions(+), 11 deletions(-) (limited to 'backend/routes/forms/submit.py') diff --git a/backend/routes/auth/authorize.py b/backend/routes/auth/authorize.py index 1e773d6..5742b9b 100644 --- a/backend/routes/auth/authorize.py +++ b/backend/routes/auth/authorize.py @@ -33,7 +33,11 @@ class AuthorizeResponse(BaseModel): expiry: str = Field("ISO formatted timestamp of expiry.") -async def process_token(bearer_token: dict, origin: str) -> Union[AuthorizeResponse, AUTH_FAILURE]: +async def process_token( + bearer_token: dict, + origin_url: str, + request_url: Request.url +) -> Union[AuthorizeResponse, AUTH_FAILURE]: """Post a bearer token to Discord, and return a JWT and username.""" interaction_start = datetime.datetime.now() @@ -61,19 +65,20 @@ async def process_token(bearer_token: dict, origin: str) -> Union[AuthorizeRespo "expiry": token_expiry.isoformat() }) - await set_response_token(response, origin, token, bearer_token["expires_in"]) + await set_response_token(response, origin_url, request_url, token, bearer_token["expires_in"]) return response async def set_response_token( - response: responses.Response, - origin_url: str, - new_token: str, - expiry: int + response: responses.Response, + origin_url: str, + request_url: Request.url, + new_token: str, + expiry: int ) -> None: """Helper that handles logic for updating a token in a set-cookie response.""" if origin_url == constants.PRODUCTION_URL: - domain = constants.PRODUCTION_URL + domain = request_url samesite = "strict" elif not constants.PRODUCTION: @@ -81,7 +86,7 @@ async def set_response_token( samesite = "strict" else: - domain = origin_url + domain = request_url samesite = "None" response.set_cookie( @@ -116,7 +121,7 @@ class AuthorizeRoute(Route): except httpx.HTTPStatusError: return AUTH_FAILURE - return await process_token(bearer_token, url) + return await process_token(bearer_token, url, request.url) class TokenRefreshRoute(Route): @@ -141,4 +146,4 @@ class TokenRefreshRoute(Route): except httpx.HTTPStatusError: return AUTH_FAILURE - return await process_token(bearer_token, url) + return await process_token(bearer_token, url, request.url) diff --git a/backend/routes/forms/submit.py b/backend/routes/forms/submit.py index 975307b..ae98cfb 100644 --- a/backend/routes/forms/submit.py +++ b/backend/routes/forms/submit.py @@ -77,7 +77,9 @@ class SubmitForm(Route): origin = request.headers.get("origin") expiry_seconds = (expiry - datetime.datetime.now()).seconds - await set_response_token(response, origin, request.user.token, expiry_seconds) + await set_response_token( + response, origin, request.url, request.user.token, expiry_seconds + ) except httpx.HTTPStatusError: pass -- cgit v1.2.3 From 8ef22e9bac402f12bb5f6e932ff67fd45b26433b Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Sun, 7 Mar 2021 00:55:31 +0300 Subject: Switches Forwarded Protocol Header Traefik forwards https traffic to http, which causes issues with the protocol in a request's URL. This switch uses the protocol header to correctly set the protocol. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- backend/routes/auth/authorize.py | 16 ++++++++-------- backend/routes/forms/submit.py | 5 +---- 2 files changed, 9 insertions(+), 12 deletions(-) (limited to 'backend/routes/forms/submit.py') diff --git a/backend/routes/auth/authorize.py b/backend/routes/auth/authorize.py index 6a27c65..e782bcc 100644 --- a/backend/routes/auth/authorize.py +++ b/backend/routes/auth/authorize.py @@ -35,8 +35,7 @@ class AuthorizeResponse(BaseModel): async def process_token( bearer_token: dict, - origin_url: str, - request_url: Request.url + request: Request ) -> Union[AuthorizeResponse, AUTH_FAILURE]: """Post a bearer token to Discord, and return a JWT and username.""" interaction_start = datetime.datetime.now() @@ -65,19 +64,20 @@ async def process_token( "expiry": token_expiry.isoformat() }) - await set_response_token(response, origin_url, request_url, token, bearer_token["expires_in"]) + await set_response_token(response, request, token, bearer_token["expires_in"]) return response async def set_response_token( response: responses.Response, - origin_url: str, - request_url: Request.url, + request: Request, new_token: str, expiry: int ) -> None: """Helper that handles logic for updating a token in a set-cookie response.""" - stripped_domain = f"{request_url.scheme}://{request_url.netloc}/" + origin_url = request.headers.get("origin") + protocol = request.headers.get("X-Forwarded-Proto") or "https" + stripped_domain = f"{protocol}://{request.url.netloc}/" if origin_url == constants.PRODUCTION_URL: domain = stripped_domain @@ -123,7 +123,7 @@ class AuthorizeRoute(Route): except httpx.HTTPStatusError: return AUTH_FAILURE - return await process_token(bearer_token, url, request.url) + return await process_token(bearer_token, request) class TokenRefreshRoute(Route): @@ -148,4 +148,4 @@ class TokenRefreshRoute(Route): except httpx.HTTPStatusError: return AUTH_FAILURE - return await process_token(bearer_token, url, request.url) + return await process_token(bearer_token, request) diff --git a/backend/routes/forms/submit.py b/backend/routes/forms/submit.py index ae98cfb..2624c98 100644 --- a/backend/routes/forms/submit.py +++ b/backend/routes/forms/submit.py @@ -75,11 +75,8 @@ class SubmitForm(Route): except ValueError: expiry = None - origin = request.headers.get("origin") expiry_seconds = (expiry - datetime.datetime.now()).seconds - await set_response_token( - response, origin, request.url, request.user.token, expiry_seconds - ) + await set_response_token(response, request, request.user.token, expiry_seconds) except httpx.HTTPStatusError: pass -- cgit v1.2.3