From 3e786da5537cc243097b4bedb8e6b78ad1525f95 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 22 Jan 2022 01:54:15 +0300 Subject: Add Helper Functions For Managing Roles Signed-off-by: Hassan Abouelela --- backend/discord.py | 38 +++++++++++++++++++++++++++++++------- backend/models/discord_role.py | 40 ++++++++++++++++++++++++++++++++++++++++ backend/models/discord_user.py | 25 +++++++++++++++++++++++-- backend/routes/roles.py | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 130 insertions(+), 9 deletions(-) create mode 100644 backend/models/discord_role.py create mode 100644 backend/routes/roles.py diff --git a/backend/discord.py b/backend/discord.py index e5c7f8f..cf80cf3 100644 --- a/backend/discord.py +++ b/backend/discord.py @@ -1,16 +1,15 @@ """Various utilities for working with the Discord API.""" import httpx -from backend.constants import ( - DISCORD_API_BASE_URL, OAUTH2_CLIENT_ID, OAUTH2_CLIENT_SECRET -) +from backend import constants +from backend.models import discord_role, discord_user async def fetch_bearer_token(code: str, redirect: str, *, refresh: bool) -> dict: async with httpx.AsyncClient() as client: data = { - "client_id": OAUTH2_CLIENT_ID, - "client_secret": OAUTH2_CLIENT_SECRET, + "client_id": constants.OAUTH2_CLIENT_ID, + "client_secret": constants.OAUTH2_CLIENT_SECRET, "redirect_uri": f"{redirect}/callback" } @@ -21,7 +20,7 @@ async def fetch_bearer_token(code: str, redirect: str, *, refresh: bool) -> dict data["grant_type"] = "authorization_code" data["code"] = code - r = await client.post(f"{DISCORD_API_BASE_URL}/oauth2/token", headers={ + r = await client.post(f"{constants.DISCORD_API_BASE_URL}/oauth2/token", headers={ "Content-Type": "application/x-www-form-urlencoded" }, data=data) @@ -32,10 +31,35 @@ async def fetch_bearer_token(code: str, redirect: str, *, refresh: bool) -> dict async def fetch_user_details(bearer_token: str) -> dict: async with httpx.AsyncClient() as client: - r = await client.get(f"{DISCORD_API_BASE_URL}/users/@me", headers={ + r = await client.get(f"{constants.DISCORD_API_BASE_URL}/users/@me", headers={ "Authorization": f"Bearer {bearer_token}" }) r.raise_for_status() return r.json() + + +async def get_role_info() -> list[discord_role.DiscordRole]: + """Get information about the roles in the configured guild.""" + async with httpx.AsyncClient() as client: + r = await client.get( + f"{constants.DISCORD_API_BASE_URL}/guilds/{constants.DISCORD_GUILD}/roles", + headers={"Authorization": f"Bot {constants.DISCORD_BOT_TOKEN}"} + ) + + r.raise_for_status() + return [discord_role.DiscordRole(**role) for role in r.json()] + + +async def get_member(member_id: str) -> discord_user.DiscordMember: + """Get a member by ID from the configured guild.""" + async with httpx.AsyncClient() as client: + r = await client.get( + f"{constants.DISCORD_API_BASE_URL}/guilds/{constants.DISCORD_GUILD}" + f"/members/{member_id}", + headers={"Authorization": f"Bot {constants.DISCORD_BOT_TOKEN}"} + ) + + r.raise_for_status() + return discord_user.DiscordMember(**r.json()) diff --git a/backend/models/discord_role.py b/backend/models/discord_role.py new file mode 100644 index 0000000..9f0b7dd --- /dev/null +++ b/backend/models/discord_role.py @@ -0,0 +1,40 @@ +import typing + +from pydantic import BaseModel + + +class RoleTags(BaseModel): + """Meta information about a discord role.""" + + bot_id: typing.Optional[str] + integration_id: typing.Optional[str] + premium_subscriber: bool + + def __init__(self, **data: typing.Any): + """ + Handle the terrible discord API. + + Discord only returns the premium_subscriber field if it's true, + meaning the typical validation process wouldn't work. + + We manually parse the raw data to determine if the field exists, and give it a useful + bool value. + """ + data["premium_subscriber"] = "premium_subscriber" in data.keys() + super().__init__(**data) + + +class DiscordRole(BaseModel): + """Schema model of Discord guild roles.""" + + id: str + name: str + color: int + hoist: bool + icon: typing.Optional[str] + unicode_emoji: typing.Optional[str] + position: int + permissions: str + managed: bool + mentionable: bool + tags: typing.Optional[RoleTags] diff --git a/backend/models/discord_user.py b/backend/models/discord_user.py index 9f246ba..3f4209d 100644 --- a/backend/models/discord_user.py +++ b/backend/models/discord_user.py @@ -1,10 +1,11 @@ +import datetime import typing as t from pydantic import BaseModel -class DiscordUser(BaseModel): - """Schema model of Discord user for form response.""" +class _User(BaseModel): + """Base for discord users and members.""" # Discord default fields. username: str @@ -20,5 +21,25 @@ class DiscordUser(BaseModel): premium_type: t.Optional[int] public_flags: t.Optional[int] + +class DiscordUser(_User): + """Schema model of Discord user for form response.""" + # Custom fields admin: bool + + +class DiscordMember(BaseModel): + """A discord guild member.""" + + user: _User + nick: t.Optional[str] + avatar: t.Optional[str] + roles: list[str] + joined_at: datetime.datetime + premium_since: t.Optional[datetime.datetime] + deaf: bool + mute: bool + pending: t.Optional[bool] + permissions: t.Optional[str] + communication_disabled_until: t.Optional[datetime.datetime] diff --git a/backend/routes/roles.py b/backend/routes/roles.py new file mode 100644 index 0000000..b18a04b --- /dev/null +++ b/backend/routes/roles.py @@ -0,0 +1,36 @@ +import starlette.background +from pymongo.database import Database +from spectree import Response +from starlette.authentication import requires +from starlette.responses import JSONResponse +from starlette.routing import Request + +from backend import discord, route +from backend.validation import OkayResponse, api + + +async def refresh_roles(database: Database) -> None: + """Connect to the discord API and refresh the roles database.""" + roles = await discord.get_role_info() + roles_collection = database.get_collection("roles") + roles_collection.drop() + roles_collection.insert_many([role.dict() for role in roles]) + + +class RolesRoute(route.Route): + """Refreshes the roles database.""" + + name = "roles" + path = "/roles" + + @requires(["authenticated", "admin"]) + @api.validate( + resp=Response(HTTP_200=OkayResponse), + tags=["roles"] + ) + async def patch(self, request: Request) -> JSONResponse: + """Refresh the roles database.""" + return JSONResponse( + {"status": "ok"}, + background=starlette.background.BackgroundTask(refresh_roles, request.state.db) + ) -- cgit v1.2.3 From 134b2f70e4cf947744f1b061766bb37fe616ad65 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 5 Feb 2022 16:50:11 +0400 Subject: Overhaul Scope System Adds discord role support to the pre-existing scopes system to power more complex access permissions. Signed-off-by: Hassan Abouelela --- backend/authentication/backend.py | 9 +++- backend/authentication/user.py | 45 +++++++++++++--- backend/discord.py | 105 ++++++++++++++++++++++++++++++++++---- backend/models/__init__.py | 5 +- backend/models/discord_user.py | 9 ++++ backend/routes/auth/authorize.py | 12 +++-- backend/routes/discord.py | 83 ++++++++++++++++++++++++++++++ backend/routes/forms/submit.py | 2 +- backend/routes/roles.py | 36 ------------- 9 files changed, 246 insertions(+), 60 deletions(-) create mode 100644 backend/routes/discord.py delete mode 100644 backend/routes/roles.py diff --git a/backend/authentication/backend.py b/backend/authentication/backend.py index c7590e9..54385e2 100644 --- a/backend/authentication/backend.py +++ b/backend/authentication/backend.py @@ -5,6 +5,7 @@ from starlette import authentication from starlette.requests import Request from backend import constants +from backend import discord # We must import user such way here to avoid circular imports from .user import User @@ -60,8 +61,12 @@ class JWTAuthenticationBackend(authentication.AuthenticationBackend): except Exception: raise authentication.AuthenticationError("Could not parse user details.") - user = User(token, user_details) - if await user.fetch_admin_status(request): + user = User( + token, user_details, await discord.get_member(request.state.db, user_details["id"]) + ) + if await user.fetch_admin_status(request.state.db): scopes.append("admin") + scopes.extend(await user.get_user_roles(request.state.db)) + return authentication.AuthCredentials(scopes), user diff --git a/backend/authentication/user.py b/backend/authentication/user.py index 857c2ed..0ec0188 100644 --- a/backend/authentication/user.py +++ b/backend/authentication/user.py @@ -1,20 +1,27 @@ +import typing import typing as t import jwt +from pymongo.database import Database from starlette.authentication import BaseUser -from starlette.requests import Request +from backend import discord, models from backend.constants import SECRET_KEY -from backend.discord import fetch_user_details class User(BaseUser): """Starlette BaseUser implementation for JWT authentication.""" - def __init__(self, token: str, payload: dict[str, t.Any]) -> None: + def __init__( + self, + token: str, + payload: dict[str, t.Any], + member: typing.Optional[models.DiscordMember], + ) -> None: self.token = token self.payload = payload self.admin = False + self.member = member @property def is_authenticated(self) -> bool: @@ -34,16 +41,40 @@ class User(BaseUser): def decoded_token(self) -> dict[str, any]: return jwt.decode(self.token, SECRET_KEY, algorithms=["HS256"]) - async def fetch_admin_status(self, request: Request) -> bool: - self.admin = await request.state.db.admins.find_one( + async def get_user_roles(self, database: Database) -> list[str]: + """Get a list of the user's discord roles.""" + if not self.member: + return [] + + server_roles = await discord.get_roles(database) + roles = [] + + for role in server_roles: + if role.id in self.member.roles: + roles.append(role.name) + + if "admin" in roles: + # Protect against collision with the forms admin role + roles.remove("admin") + roles.append("discord admin") + + return roles + + async def fetch_admin_status(self, database: Database) -> bool: + self.admin = await database.admins.find_one( {"_id": self.payload["id"]} ) is not None return self.admin - async def refresh_data(self) -> None: + async def refresh_data(self, database: Database) -> None: """Fetches user data from discord, and updates the instance.""" - self.payload = await fetch_user_details(self.decoded_token.get("token")) + self.member = await discord.get_member(database, self.payload["id"]) + + if self.member: + self.payload = self.member.user.dict() + else: + self.payload = await discord.fetch_user_details(self.decoded_token.get("token")) updated_info = self.decoded_token updated_info["user_details"] = self.payload diff --git a/backend/discord.py b/backend/discord.py index cf80cf3..51de26a 100644 --- a/backend/discord.py +++ b/backend/discord.py @@ -1,8 +1,13 @@ """Various utilities for working with the Discord API.""" + +import datetime +import json +import typing + import httpx +from pymongo.database import Database -from backend import constants -from backend.models import discord_role, discord_user +from backend import constants, models async def fetch_bearer_token(code: str, redirect: str, *, refresh: bool) -> dict: @@ -40,7 +45,7 @@ async def fetch_user_details(bearer_token: str) -> dict: return r.json() -async def get_role_info() -> list[discord_role.DiscordRole]: +async def _get_role_info() -> list[models.DiscordRole]: """Get information about the roles in the configured guild.""" async with httpx.AsyncClient() as client: r = await client.get( @@ -49,11 +54,50 @@ async def get_role_info() -> list[discord_role.DiscordRole]: ) r.raise_for_status() - return [discord_role.DiscordRole(**role) for role in r.json()] - - -async def get_member(member_id: str) -> discord_user.DiscordMember: - """Get a member by ID from the configured guild.""" + return [models.DiscordRole(**role) for role in r.json()] + + +async def get_roles( + database: Database, *, force_refresh: bool = False +) -> list[models.DiscordRole]: + """ + Get a list of all roles from the cache, or discord API if not available. + + If `force_refresh` is True, the cache is skipped and the roles are updated. + """ + collection = database.get_collection("roles") + + if force_refresh: + # Drop all values in the collection + await collection.delete_many({}) + + # `create_index` creates the index if it does not exist, or passes + # This handles TTL on role objects + await collection.create_index( + "inserted_at", + expireAfterSeconds=60 * 60 * 24, # 1 day + name="inserted_at", + ) + + roles = [] + async for role in collection.find(): + roles.append(models.DiscordRole(**json.loads(role["data"]))) + + if len(roles) == 0: + # Fetch roles from the API and insert into the database + roles = await _get_role_info() + await collection.insert_many({ + "name": role.name, + "id": role.id, + "data": role.json(), + "inserted_at": datetime.datetime.now(tz=datetime.timezone.utc), + } for role in roles) + + return roles + + +async def _fetch_member_api(member_id: str) -> typing.Optional[models.DiscordMember]: + """Get a member by ID from the configured guild using the discord API.""" async with httpx.AsyncClient() as client: r = await client.get( f"{constants.DISCORD_API_BASE_URL}/guilds/{constants.DISCORD_GUILD}" @@ -61,5 +105,48 @@ async def get_member(member_id: str) -> discord_user.DiscordMember: headers={"Authorization": f"Bot {constants.DISCORD_BOT_TOKEN}"} ) + if r.status_code == 404: + return None + r.raise_for_status() - return discord_user.DiscordMember(**r.json()) + return models.DiscordMember(**r.json()) + + +async def get_member( + database: Database, user_id: str, *, force_refresh: bool = False +) -> typing.Optional[models.DiscordMember]: + """ + Get a member from the cache, or from the discord API. + + If `force_refresh` is True, the cache is skipped and the entry is updated. + None may be returned if the member object does not exist. + """ + collection = database.get_collection("discord_members") + + if force_refresh: + await collection.delete_one({"user": user_id}) + + # `create_index` creates the index if it does not exist, or passes + # This handles TTL on member objects + await collection.create_index( + "inserted_at", + expireAfterSeconds=60 * 60, # 1 hour + name="inserted_at", + ) + + result = await collection.find_one({"user": user_id}) + + if result is not None: + return models.DiscordMember(**json.loads(result["data"])) + + member = await _fetch_member_api(user_id) + + if not member: + return None + + await collection.insert_one({ + "user": user_id, + "data": member.json(), + "inserted_at": datetime.datetime.now(tz=datetime.timezone.utc), + }) + return member diff --git a/backend/models/__init__.py b/backend/models/__init__.py index 8ad7f7f..a9f76e0 100644 --- a/backend/models/__init__.py +++ b/backend/models/__init__.py @@ -1,12 +1,15 @@ from .antispam import AntiSpam -from .discord_user import DiscordUser +from .discord_role import DiscordRole +from .discord_user import DiscordMember, DiscordUser from .form import Form, FormList from .form_response import FormResponse, ResponseList from .question import CodeQuestion, Question __all__ = [ "AntiSpam", + "DiscordRole", "DiscordUser", + "DiscordMember", "Form", "FormResponse", "CodeQuestion", diff --git a/backend/models/discord_user.py b/backend/models/discord_user.py index 3f4209d..0eca15b 100644 --- a/backend/models/discord_user.py +++ b/backend/models/discord_user.py @@ -43,3 +43,12 @@ class DiscordMember(BaseModel): pending: t.Optional[bool] permissions: t.Optional[str] communication_disabled_until: t.Optional[datetime.datetime] + + def dict(self, *args, **kwargs) -> dict[str, t.Any]: + """Convert the model to a python dict, and encode timestamps in a serializable format.""" + data = super().dict(*args, **kwargs) + for field, value in data.items(): + if isinstance(value, datetime.datetime): + data[field] = value.isoformat() + + return data diff --git a/backend/routes/auth/authorize.py b/backend/routes/auth/authorize.py index d4587f0..42fb3ec 100644 --- a/backend/routes/auth/authorize.py +++ b/backend/routes/auth/authorize.py @@ -17,7 +17,7 @@ from starlette.requests import Request from backend import constants from backend.authentication.user import User from backend.constants import SECRET_KEY -from backend.discord import fetch_bearer_token, fetch_user_details +from backend.discord import fetch_bearer_token, fetch_user_details, get_member from backend.route import Route from backend.validation import ErrorMessage, api @@ -34,8 +34,8 @@ class AuthorizeResponse(BaseModel): async def process_token( - bearer_token: dict, - request: Request + bearer_token: dict, + request: Request ) -> Union[AuthorizeResponse, AUTH_FAILURE]: """Post a bearer token to Discord, and return a JWT and username.""" interaction_start = datetime.datetime.now() @@ -46,6 +46,9 @@ async def process_token( AUTH_FAILURE.delete_cookie("token") return AUTH_FAILURE + user_id = user_details["id"] + member = await get_member(request.state.db, user_id, force_refresh=True) + max_age = datetime.timedelta(seconds=int(bearer_token["expires_in"])) token_expiry = interaction_start + max_age @@ -53,11 +56,12 @@ async def process_token( "token": bearer_token["access_token"], "refresh": bearer_token["refresh_token"], "user_details": user_details, + "in_guild": bool(member), "expiry": token_expiry.isoformat() } token = jwt.encode(data, SECRET_KEY, algorithm="HS256") - user = User(token, user_details) + user = User(token, user_details, member) response = responses.JSONResponse({ "username": user.display_name, diff --git a/backend/routes/discord.py b/backend/routes/discord.py new file mode 100644 index 0000000..a980d94 --- /dev/null +++ b/backend/routes/discord.py @@ -0,0 +1,83 @@ +"""Routes which directly interact with discord related data.""" + +import pydantic +from spectree import Response +from starlette.authentication import requires +from starlette.responses import JSONResponse +from starlette.routing import Request + +from backend import discord, models, route +from backend.validation import ErrorMessage, OkayResponse, api + +NOT_FOUND_EXCEPTION = JSONResponse( + {"error": "Could not find the requested resource in the guild or cache."}, status_code=404 +) + + +class RolesRoute(route.Route): + """Refreshes the roles database.""" + + name = "roles" + path = "/roles" + + class RolesResponse(pydantic.BaseModel): + """A list of all roles on the configured server.""" + + roles: list[models.DiscordRole] + + @requires(["authenticated", "admin"]) + @api.validate( + resp=Response(HTTP_200=OkayResponse), + tags=["roles"] + ) + async def patch(self, request: Request) -> JSONResponse: + """Refresh the roles database.""" + roles = await discord.get_roles(request.state.db, force_refresh=True) + + return JSONResponse( + {"status": "ok"}, + ) + + +class MemberRoute(route.Route): + """Retrieve information about a server member.""" + + name = "member" + path = "/member" + + class MemberRequest(pydantic.BaseModel): + """An ID of the member to update.""" + + user_id: str + + @requires(["authenticated", "admin"]) + @api.validate( + resp=Response(HTTP_200=models.DiscordMember, HTTP_400=ErrorMessage), + json=MemberRequest, + tags=["auth"] + ) + async def delete(self, request: Request): + """Force a resync of the cache for the given user.""" + body = await request.json() + member = await discord.get_member(request.state.db, body["user_id"], force_refresh=True) + + if member: + return JSONResponse(member.dict()) + else: + return NOT_FOUND_EXCEPTION + + @requires(["authenticated", "admin"]) + @api.validate( + resp=Response(HTTP_200=models.DiscordMember, HTTP_400=ErrorMessage), + json=MemberRequest, + tags=["auth"] + ) + async def get(self, request: Request): + """Get a user's roles on the configured server.""" + body = await request.json() + member = await discord.get_member(request.state.db, body["user_id"]) + + if member: + return JSONResponse(member.dict()) + else: + return NOT_FOUND_EXCEPTION diff --git a/backend/routes/forms/submit.py b/backend/routes/forms/submit.py index 95e30b0..baf403d 100644 --- a/backend/routes/forms/submit.py +++ b/backend/routes/forms/submit.py @@ -83,7 +83,7 @@ class SubmitForm(Route): try: if hasattr(request.user, User.refresh_data.__name__): old = request.user.token - await request.user.refresh_data() + await request.user.refresh_data(request.state.db) if old != request.user.token: try: diff --git a/backend/routes/roles.py b/backend/routes/roles.py deleted file mode 100644 index b18a04b..0000000 --- a/backend/routes/roles.py +++ /dev/null @@ -1,36 +0,0 @@ -import starlette.background -from pymongo.database import Database -from spectree import Response -from starlette.authentication import requires -from starlette.responses import JSONResponse -from starlette.routing import Request - -from backend import discord, route -from backend.validation import OkayResponse, api - - -async def refresh_roles(database: Database) -> None: - """Connect to the discord API and refresh the roles database.""" - roles = await discord.get_role_info() - roles_collection = database.get_collection("roles") - roles_collection.drop() - roles_collection.insert_many([role.dict() for role in roles]) - - -class RolesRoute(route.Route): - """Refreshes the roles database.""" - - name = "roles" - path = "/roles" - - @requires(["authenticated", "admin"]) - @api.validate( - resp=Response(HTTP_200=OkayResponse), - tags=["roles"] - ) - async def patch(self, request: Request) -> JSONResponse: - """Refresh the roles database.""" - return JSONResponse( - {"status": "ok"}, - background=starlette.background.BackgroundTask(refresh_roles, request.state.db) - ) -- cgit v1.2.3 From 513de6945d40b66368a061dff6a81646e8bda7a0 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 5 Feb 2022 17:39:33 +0400 Subject: Add Role Based Authorized Readers Adds a new property on forms to declare which roles are authorized to access form responses. Signed-off-by: Hassan Abouelela --- backend/discord.py | 38 ++++++++++++++++++++++++++++++++++++++ backend/models/form.py | 12 ++++++++++-- backend/routes/forms/discover.py | 2 +- backend/routes/forms/response.py | 18 ++++++++++++++---- backend/routes/forms/responses.py | 17 ++++++++++------- 5 files changed, 73 insertions(+), 14 deletions(-) diff --git a/backend/discord.py b/backend/discord.py index 51de26a..4e35216 100644 --- a/backend/discord.py +++ b/backend/discord.py @@ -5,6 +5,7 @@ import json import typing import httpx +import starlette.requests from pymongo.database import Database from backend import constants, models @@ -150,3 +151,40 @@ async def get_member( "inserted_at": datetime.datetime.now(tz=datetime.timezone.utc), }) return member + + +class FormNotFoundError(Exception): + """The requested form was not found.""" + + +async def _verify_access_helper( + form_id: str, request: starlette.requests.Request, attribute: str +) -> bool: + """A low level helper to validate access to a form resource based on the user's scopes.""" + # Short circuit all resources for admins + if "admin" in request.auth.scopes: + return True + + form = await request.state.db.forms.find_one({"id": form_id}) + + if not form: + raise FormNotFoundError() + + form = models.Form(**form) + + for role_id in getattr(form, attribute) or []: + role = await request.state.db.roles.find_one({"id": role_id}) + if not role: + continue + + role = models.DiscordRole(**json.loads(role["data"])) + + if role.name in request.auth.scopes: + return True + + return False + + +async def verify_response_access(form_id: str, request: starlette.requests.Request) -> bool: + """Ensure the user can access responses on the requested resource.""" + return await _verify_access_helper(form_id, request, "response_readers") diff --git a/backend/models/form.py b/backend/models/form.py index f19ed85..45a7e0b 100644 --- a/backend/models/form.py +++ b/backend/models/form.py @@ -1,10 +1,10 @@ import typing as t import httpx -from pydantic import constr, BaseModel, Field, root_validator, validator +from pydantic import BaseModel, Field, constr, root_validator, validator from pydantic.error_wrappers import ErrorWrapper, ValidationError -from backend.constants import FormFeatures, WebHook +from backend.constants import DISCORD_GUILD, FormFeatures, WebHook from .question import Question PUBLIC_FIELDS = [ @@ -43,6 +43,7 @@ class Form(BaseModel): submitted_text: t.Optional[str] = None webhook: _WebHook = None discord_role: t.Optional[str] + response_readers: t.Optional[list[str]] class Config: allow_population_by_field_name = True @@ -67,6 +68,13 @@ class Form(BaseModel): return value + @validator("response_readers") + def validate_role_scoping(cls, value: t.Optional[list[str]]): + """Ensure special role based permissions aren't granted to the @everyone role.""" + if value and str(DISCORD_GUILD) in value: + raise ValueError("You can not add the everyone role as an access scope.") + return value + @root_validator def validate_role(cls, values: dict[str, t.Any]) -> t.Optional[dict[str, t.Any]]: """Validates does Discord role provided when flag provided.""" diff --git a/backend/routes/forms/discover.py b/backend/routes/forms/discover.py index d7351d5..b993075 100644 --- a/backend/routes/forms/discover.py +++ b/backend/routes/forms/discover.py @@ -29,7 +29,7 @@ EMPTY_FORM = Form( features=__FEATURES, questions=[__QUESTION], name="Auth form", - description="An empty form to help you get a token." + description="An empty form to help you get a token.", ) diff --git a/backend/routes/forms/response.py b/backend/routes/forms/response.py index d8d8d17..fbf8e99 100644 --- a/backend/routes/forms/response.py +++ b/backend/routes/forms/response.py @@ -1,11 +1,13 @@ """ Returns or deletes form response by ID. """ + from spectree import Response as RouteResponse from starlette.authentication import requires from starlette.requests import Request from starlette.responses import JSONResponse +from backend import discord from backend.models import FormResponse from backend.route import Route from backend.validation import ErrorMessage, OkayResponse, api @@ -17,23 +19,31 @@ class Response(Route): name = "response" path = "/{form_id:str}/responses/{response_id:str}" - @requires(["authenticated", "admin"]) + @requires(["authenticated"]) @api.validate( - resp=RouteResponse(HTTP_200=FormResponse, HTTP_404=ErrorMessage), + resp=RouteResponse(HTTP_200=FormResponse, HTTP_401=ErrorMessage, HTTP_404=ErrorMessage), tags=["forms", "responses"] ) async def get(self, request: Request) -> JSONResponse: """Return a single form response by ID.""" + form_id = request.path_params["form_id"] + + try: + if not await discord.verify_response_access(form_id, request): + return JSONResponse({"error": "unauthorized"}, status_code=401) + except discord.FormNotFoundError: + return JSONResponse({"error": "form_not_found"}, status_code=404) + if raw_response := await request.state.db.responses.find_one( { "_id": request.path_params["response_id"], - "form_id": request.path_params["form_id"] + "form_id": form_id } ): response = FormResponse(**raw_response) return JSONResponse(response.dict()) else: - return JSONResponse({"error": "not_found"}, status_code=404) + return JSONResponse({"error": "response_not_found"}, status_code=404) @requires(["authenticated", "admin"]) @api.validate( diff --git a/backend/routes/forms/responses.py b/backend/routes/forms/responses.py index f3c4cd7..1c8ebe3 100644 --- a/backend/routes/forms/responses.py +++ b/backend/routes/forms/responses.py @@ -7,9 +7,10 @@ from starlette.authentication import requires from starlette.requests import Request from starlette.responses import JSONResponse +from backend import discord from backend.models import FormResponse, ResponseList from backend.route import Route -from backend.validation import api, ErrorMessage, OkayResponse +from backend.validation import ErrorMessage, OkayResponse, api class ResponseIdList(BaseModel): @@ -24,20 +25,22 @@ class Responses(Route): name = "form_responses" path = "/{form_id:str}/responses" - @requires(["authenticated", "admin"]) + @requires(["authenticated"]) @api.validate( - resp=Response(HTTP_200=ResponseList, HTTP_404=ErrorMessage), + resp=Response(HTTP_200=ResponseList, HTTP_401=ErrorMessage, HTTP_404=ErrorMessage), tags=["forms", "responses"] ) async def get(self, request: Request) -> JSONResponse: """Returns all form responses by form ID.""" - if not await request.state.db.forms.find_one( - {"_id": request.path_params["form_id"]} - ): + form_id = request.path_params["form_id"] + try: + if not await discord.verify_response_access(form_id, request): + return JSONResponse({"error": "unauthorized"}, 401) + except discord.FormNotFoundError: return JSONResponse({"error": "not_found"}, 404) cursor = request.state.db.responses.find( - {"form_id": request.path_params["form_id"]} + {"form_id": form_id} ) responses = [ FormResponse(**response) for response in await cursor.to_list(None) -- cgit v1.2.3 From 7ae2d8714a5fd11155ee3974c1b7cd1be20ac56d Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 5 Feb 2022 18:32:06 +0400 Subject: Add Role Based Editors To Forms Adds the ability to specify who can edit forms using discord roles. Signed-off-by: Hassan Abouelela --- backend/discord.py | 5 +++ backend/models/form.py | 3 +- backend/routes/forms/form.py | 71 +++++++++++++++++++++++++------------------ backend/routes/forms/index.py | 6 ++-- 4 files changed, 51 insertions(+), 34 deletions(-) diff --git a/backend/discord.py b/backend/discord.py index 4e35216..6c8eefe 100644 --- a/backend/discord.py +++ b/backend/discord.py @@ -188,3 +188,8 @@ async def _verify_access_helper( async def verify_response_access(form_id: str, request: starlette.requests.Request) -> bool: """Ensure the user can access responses on the requested resource.""" return await _verify_access_helper(form_id, request, "response_readers") + + +async def verify_edit_access(form_id: str, request: starlette.requests.Request) -> bool: + """Ensure the user can view and modify the requested resource.""" + return await _verify_access_helper(form_id, request, "editors") diff --git a/backend/models/form.py b/backend/models/form.py index 45a7e0b..4ee2804 100644 --- a/backend/models/form.py +++ b/backend/models/form.py @@ -44,6 +44,7 @@ class Form(BaseModel): webhook: _WebHook = None discord_role: t.Optional[str] response_readers: t.Optional[list[str]] + editors: t.Optional[list[str]] class Config: allow_population_by_field_name = True @@ -68,7 +69,7 @@ class Form(BaseModel): return value - @validator("response_readers") + @validator("response_readers", "editors") def validate_role_scoping(cls, value: t.Optional[list[str]]): """Ensure special role based permissions aren't granted to the @everyone role.""" if value and str(DISCORD_GUILD) in value: diff --git a/backend/routes/forms/form.py b/backend/routes/forms/form.py index 0f96b85..15ff9a6 100644 --- a/backend/routes/forms/form.py +++ b/backend/routes/forms/form.py @@ -10,13 +10,16 @@ from starlette.authentication import requires from starlette.requests import Request from starlette.responses import JSONResponse -from backend import constants +from backend import constants, discord from backend.models import Form from backend.route import Route from backend.routes.forms.discover import EMPTY_FORM from backend.routes.forms.unittesting import filter_unittests from backend.validation import ErrorMessage, OkayResponse, api +NOT_FOUND_ERROR = JSONResponse({"error": "not_found"}, status_code=404) +PUBLIC_FORM_FEATURES = (constants.FormFeatures.OPEN, constants.FormFeatures.DISCOVERABLE) + class SingleForm(Route): """ @@ -31,9 +34,17 @@ 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.admin if request.user.is_authenticated else False form_id = request.path_params["form_id"].lower() + try: + admin = await discord.verify_edit_access(form_id, request) + except discord.FormNotFoundError: + if not constants.PRODUCTION and form_id == EMPTY_FORM.id: + # Empty form to help with authentication in development. + return JSONResponse(EMPTY_FORM.dict(admin=False)) + else: + return NOT_FOUND_ERROR + filters = { "_id": form_id } @@ -41,25 +52,19 @@ class SingleForm(Route): if not admin: filters["features"] = {"$in": ["OPEN", "DISCOVERABLE"]} - if raw_form := await request.state.db.forms.find_one(filters): - form = Form(**raw_form) - if not admin: - form = filter_unittests(form) - - return JSONResponse(form.dict(admin=admin)) - - elif not constants.PRODUCTION and form_id == EMPTY_FORM.id: - # Empty form to help with authentication in development. - return JSONResponse(EMPTY_FORM.dict(admin=admin)) + form = Form(**await request.state.db.forms.find_one(filters)) + if not admin: + form = filter_unittests(form) - return JSONResponse({"error": "not_found"}, status_code=404) + return JSONResponse(form.dict(admin=admin)) - @requires(["authenticated", "admin"]) + @requires(["authenticated"]) @api.validate( resp=Response( HTTP_200=OkayResponse, HTTP_400=ErrorMessage, - HTTP_404=ErrorMessage + HTTP_401=ErrorMessage, + HTTP_404=ErrorMessage, ), tags=["forms"] ) @@ -70,10 +75,17 @@ class SingleForm(Route): except json.decoder.JSONDecodeError: return JSONResponse("Expected a JSON body.", 400) - form_id = {"_id": request.path_params["form_id"].lower()} - if raw_form := await request.state.db.forms.find_one(form_id): + form_id = request.path_params["form_id"].lower() + + try: + if not await discord.verify_edit_access(form_id, request): + return JSONResponse({"error": "unauthorized"}, status_code=401) + except discord.FormNotFoundError: + return NOT_FOUND_ERROR + + if raw_form := await request.state.db.forms.find_one({"id": form_id}): if "_id" in data or "id" in data: - if (data.get("id") or data.get("_id")) != form_id["_id"]: + if (data.get("id") or data.get("_id")) != form_id: return JSONResponse({"error": "locked_field"}, status_code=400) # Build Data Merger @@ -90,7 +102,7 @@ class SingleForm(Route): except ValidationError as e: return JSONResponse(e.errors(), status_code=422) - await request.state.db.forms.replace_one(form_id, form.dict()) + await request.state.db.forms.replace_one({"id": form_id}, form.dict()) return JSONResponse(form.dict()) else: @@ -98,21 +110,20 @@ class SingleForm(Route): @requires(["authenticated", "admin"]) @api.validate( - resp=Response(HTTP_200=OkayResponse, HTTP_404=ErrorMessage), + resp=Response(HTTP_200=OkayResponse, HTTP_401=ErrorMessage, HTTP_404=ErrorMessage), tags=["forms"] ) async def delete(self, request: Request) -> JSONResponse: """Deletes form by ID.""" - if not await request.state.db.forms.find_one( - {"_id": request.path_params["form_id"].lower()} - ): - return JSONResponse({"error": "not_found"}, status_code=404) + form_id = request.path_params["form_id"].lower() + + try: + if not await discord.verify_edit_access(form_id, request): + return JSONResponse({"error": "unauthorized"}, status_code=401) + except discord.FormNotFoundError: + return NOT_FOUND_ERROR - await request.state.db.forms.delete_one( - {"_id": request.path_params["form_id"].lower()} - ) - await request.state.db.responses.delete_many( - {"form_id": request.path_params["form_id"].lower()} - ) + await request.state.db.forms.delete_one({"_id": form_id}) + await request.state.db.responses.delete_many({"form_id": form_id}) return JSONResponse({"status": "ok"}) diff --git a/backend/routes/forms/index.py b/backend/routes/forms/index.py index 22171fa..38be693 100644 --- a/backend/routes/forms/index.py +++ b/backend/routes/forms/index.py @@ -15,13 +15,13 @@ from backend.validation import ErrorMessage, OkayResponse, api class FormsList(Route): """ - List all available forms for administrator viewing. + List all available forms for authorized viewers. """ name = "forms_list_create" path = "/" - @requires(["authenticated", "admin"]) + @requires(["authenticated", "Admins"]) @api.validate(resp=Response(HTTP_200=FormList), tags=["forms"]) async def get(self, request: Request) -> JSONResponse: """Return a list of all forms to authenticated users.""" @@ -38,7 +38,7 @@ class FormsList(Route): forms ) - @requires(["authenticated", "admin"]) + @requires(["authenticated", "Helpers"]) @api.validate( json=Form, resp=Response(HTTP_200=OkayResponse, HTTP_400=ErrorMessage), -- cgit v1.2.3 From 1f516d67dbdbcee9554e61b242bf24fa1e521788 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 5 Feb 2022 18:42:07 +0400 Subject: Add Docs For Editors & Response Readers Props Signed-off-by: Hassan Abouelela --- SCHEMA.md | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/SCHEMA.md b/SCHEMA.md index 93be830..0e1093f 100644 --- a/SCHEMA.md +++ b/SCHEMA.md @@ -12,21 +12,24 @@ In this document: ## Form -| Field | Type | Description | Example | -| ------------------- | ----------------------------------------- | ----------------------------------------------------------------------------------------- | ---------------------------------------- | -| `id` | Unique identifier | A user selected, unique, descriptive identifier (used in URL routes, so no spaces) | `"ban-appeals"` | -| `features` | List of [form features](#form-features) | A list of features to change the behaviour of the form, described in the features section | `["OPEN", "COLLECT_EMAIL"]` | -| `questions` | List of [form questions](#form-question) | The list of questions to render on a specific form | Too long! See below | -| `name` | String | Name of the form | `"Summer Code Jam 2100"` | -| `description` | String | Form description | `"This is my amazing form description."` | -| `webhook` | [Webhook object](#webhooks) | An optional discord webhook. | See webhook documentation. | -| `submitted_text` | Optional[String] | An optional string for the response upon submitting. | `"This is my amazing form response."` | -| `discord_role` | String (optional) | Discord role ID what will be assigned, required when `ASSIGN_ROLE` flag provided. | `784467518298259466` | +| Field | Type | Description | Example | +|--------------------|------------------------------------------|------------------------------------------------------------------------------------------------------------------|------------------------------------------------| +| `id` | Unique identifier | A user selected, unique, descriptive identifier (used in URL routes, so no spaces) | `"ban-appeals"` | +| `features` | List of [form features](#form-features) | A list of features to change the behaviour of the form, described in the features section | `["OPEN", "COLLECT_EMAIL"]` | +| `questions` | List of [form questions](#form-question) | The list of questions to render on a specific form | Too long! See below | +| `name` | String | Name of the form | `"Summer Code Jam 2100"` | +| `description` | String | Form description | `"This is my amazing form description."` | +| `webhook` | [Webhook object](#webhooks) | An optional discord webhook. | See webhook documentation. | +| `submitted_text` | Optional[String] | An optional string for the response upon submitting. | `"This is my amazing form response."` | +| `discord_role` | String (optional) | Discord role ID what will be assigned, required when `ASSIGN_ROLE` flag provided. | `784467518298259466` | +| `response_readers` | List[String] | Discord roles which can view the responses of the form. Can not be the everyone role. | `["267629731250176001", "825337057181696020"]` | +| `editors` | List[String] | Discord roles which have permission to edit, delete, or otherwise modify the form. Can not be the everyone role. | `["409416496733880320"]` | + ### Form features | Flag | Description | -| ------------------ | ----------------------------------------------------------------------------- | +|--------------------|-------------------------------------------------------------------------------| | `DISCOVERABLE` | The form should be displayed on the homepage of the forms application. | | `REQUIRES_LOGIN` | Requires the user to authenticate with Discord before completing the form. | | `OPEN` | The form is currently accepting responses. | @@ -39,7 +42,7 @@ In this document: Discord webhooks to send information upon form submission. | Field | Type | Description | -| ----------| ------ | --------------------------------------------------------------------------------------------------------- | +|-----------|--------|-----------------------------------------------------------------------------------------------------------| | `url` | String | Discord webhook URL. | | `message` | String | An optional message to include before the embed. Can use certain [context variables](#webhook-variables). | @@ -48,7 +51,7 @@ Discord webhooks to send information upon form submission. The following variables can be used in a webhook's message. The variables must be wrapped by braces (`{}`). | Name | Description | -| ------------- | ---------------------------------------------------------------------------- | +|---------------|------------------------------------------------------------------------------| | `user` | A discord mention of the user submitting the form, or "User" if unavailable. | | `response_id` | ID of the submitted response. | | `form` | Name of the submitted form. | @@ -59,7 +62,7 @@ The following variables can be used in a webhook's message. The variables must b ### Form question | Field | Type | Description | Example | -| ---------- | ---------------------------------------- | ------------------------------------------------ | -------------------- | +|------------|------------------------------------------|--------------------------------------------------|----------------------| | `id` | string | Unique identifier of the question | `"aabbcc"` | | `name` | string | Name of the question | `"What's the time?"` | | `type` | one of [Question types](#question-types) | The type of input for this question | `"radio"` | @@ -69,7 +72,7 @@ The following variables can be used in a webhook's message. The variables must b #### Question types | Name | Description | -| ------------ | --------------------------------------------------------- | +|--------------|-----------------------------------------------------------| | `radio` | Radio buttons | | `checkbox` | Checkbox toggle | | `select` | Dropdown list | @@ -165,7 +168,7 @@ Textareas require no additional configuration. ## Form response | Field | Type | Description | -| ----------- | ---------------------------------------------------- | --------------------------------------------------------------------------- | +|-------------|------------------------------------------------------|-----------------------------------------------------------------------------| | `_id`/`id` | MongoDB ObjectID | Random identifier used for the response | | `user` | Optional [user details object](#user-details-object) | An object describing the user that submitted if the form is not anonymous | | `antispam` | Optional [anti spam object](#anti-spam-object) | An object containing information about the anti-spam on the form submission | @@ -197,7 +200,7 @@ The user details contains the information returned by Discord alongside an `admi The anti-spam object contains information about the source of the form submission. | Field | Type | Description | -| ----------------- | ------- | ----------------------------------------------- | +|-------------------|---------|-------------------------------------------------| | `ip_hash` | String | hash of the submitting users IP address | | `user_agent_hash` | String | hash of the submitting users user agent | | `captcha_pass` | Boolean | Whether the user passsed the hCaptcha | -- cgit v1.2.3 From c4f816c576097d78c5645113115bbcaa29f7a1ee Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 5 Feb 2022 18:49:45 +0400 Subject: Fix Linting Errors Signed-off-by: Hassan Abouelela --- backend/models/discord_role.py | 2 +- backend/models/form.py | 2 +- backend/routes/discord.py | 10 +++++----- tox.ini | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/backend/models/discord_role.py b/backend/models/discord_role.py index 9f0b7dd..c05c9de 100644 --- a/backend/models/discord_role.py +++ b/backend/models/discord_role.py @@ -10,7 +10,7 @@ class RoleTags(BaseModel): integration_id: typing.Optional[str] premium_subscriber: bool - def __init__(self, **data: typing.Any): + def __init__(self, **data: typing.Any) -> None: """ Handle the terrible discord API. diff --git a/backend/models/form.py b/backend/models/form.py index 4ee2804..f888d6e 100644 --- a/backend/models/form.py +++ b/backend/models/form.py @@ -70,7 +70,7 @@ class Form(BaseModel): return value @validator("response_readers", "editors") - def validate_role_scoping(cls, value: t.Optional[list[str]]): + def validate_role_scoping(cls, value: t.Optional[list[str]]) -> t.Optional[list[str]]: """Ensure special role based permissions aren't granted to the @everyone role.""" if value and str(DISCORD_GUILD) in value: raise ValueError("You can not add the everyone role as an access scope.") diff --git a/backend/routes/discord.py b/backend/routes/discord.py index a980d94..bca1edb 100644 --- a/backend/routes/discord.py +++ b/backend/routes/discord.py @@ -7,7 +7,7 @@ from starlette.responses import JSONResponse from starlette.routing import Request from backend import discord, models, route -from backend.validation import ErrorMessage, OkayResponse, api +from backend.validation import ErrorMessage, api NOT_FOUND_EXCEPTION = JSONResponse( {"error": "Could not find the requested resource in the guild or cache."}, status_code=404 @@ -27,7 +27,7 @@ class RolesRoute(route.Route): @requires(["authenticated", "admin"]) @api.validate( - resp=Response(HTTP_200=OkayResponse), + resp=Response(HTTP_200=RolesResponse), tags=["roles"] ) async def patch(self, request: Request) -> JSONResponse: @@ -35,7 +35,7 @@ class RolesRoute(route.Route): roles = await discord.get_roles(request.state.db, force_refresh=True) return JSONResponse( - {"status": "ok"}, + {"roles": [role.dict() for role in roles]}, ) @@ -56,7 +56,7 @@ class MemberRoute(route.Route): json=MemberRequest, tags=["auth"] ) - async def delete(self, request: Request): + async def delete(self, request: Request) -> JSONResponse: """Force a resync of the cache for the given user.""" body = await request.json() member = await discord.get_member(request.state.db, body["user_id"], force_refresh=True) @@ -72,7 +72,7 @@ class MemberRoute(route.Route): json=MemberRequest, tags=["auth"] ) - async def get(self, request: Request): + async def get(self, request: Request) -> JSONResponse: """Get a user's roles on the configured server.""" body = await request.json() member = await discord.get_member(request.state.db, body["user_id"]) diff --git a/tox.ini b/tox.ini index afb3b34..451c3dd 100644 --- a/tox.ini +++ b/tox.ini @@ -5,6 +5,6 @@ docstring-convention=all import-order-style=pycharm ignore= # Type annotations - ANN101,ANN102 + ANN002,ANN003,ANN101,ANN102 # Line breaks W503 -- cgit v1.2.3 From 1e5cdeac8d28e798a144374d331bd137c9b1fd93 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 5 Feb 2022 17:35:57 +0200 Subject: Switch To List Comp Co-authored-by: Bluenix Signed-off-by: Hassan Abouelela --- backend/authentication/user.py | 6 +----- backend/discord.py | 4 +--- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/backend/authentication/user.py b/backend/authentication/user.py index 0ec0188..6256cae 100644 --- a/backend/authentication/user.py +++ b/backend/authentication/user.py @@ -47,11 +47,7 @@ class User(BaseUser): return [] server_roles = await discord.get_roles(database) - roles = [] - - for role in server_roles: - if role.id in self.member.roles: - roles.append(role.name) + roles = [role.name for role in server_roles if role.id in self.member.roles] if "admin" in roles: # Protect against collision with the forms admin role diff --git a/backend/discord.py b/backend/discord.py index 6c8eefe..f972f5f 100644 --- a/backend/discord.py +++ b/backend/discord.py @@ -80,9 +80,7 @@ async def get_roles( name="inserted_at", ) - roles = [] - async for role in collection.find(): - roles.append(models.DiscordRole(**json.loads(role["data"]))) + roles = [models.DiscordRole(**json.loads(role["data"])) async for role in collection.find()] if len(roles) == 0: # Fetch roles from the API and insert into the database -- cgit v1.2.3 From 0344d9a2137eb755f002398aac40533e8ea46776 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 5 Feb 2022 20:14:05 +0400 Subject: Use HTTPException To Propagate Access Failures Co-authored-by: Bluenix Signed-off-by: Hassan Abouelela --- backend/discord.py | 25 +++++++++++++++---------- backend/routes/forms/form.py | 24 +++++++----------------- backend/routes/forms/response.py | 9 ++------- backend/routes/forms/responses.py | 8 ++------ 4 files changed, 26 insertions(+), 40 deletions(-) diff --git a/backend/discord.py b/backend/discord.py index f972f5f..856e878 100644 --- a/backend/discord.py +++ b/backend/discord.py @@ -7,6 +7,7 @@ import typing import httpx import starlette.requests from pymongo.database import Database +from starlette import exceptions from backend import constants, models @@ -151,22 +152,26 @@ async def get_member( return member -class FormNotFoundError(Exception): +class FormNotFoundError(exceptions.HTTPException): """The requested form was not found.""" +class UnauthorizedError(exceptions.HTTPException): + """You are not authorized to use this resource.""" + + async def _verify_access_helper( form_id: str, request: starlette.requests.Request, attribute: str -) -> bool: +) -> None: """A low level helper to validate access to a form resource based on the user's scopes.""" # Short circuit all resources for admins if "admin" in request.auth.scopes: - return True + return form = await request.state.db.forms.find_one({"id": form_id}) if not form: - raise FormNotFoundError() + raise FormNotFoundError(status_code=404) form = models.Form(**form) @@ -178,16 +183,16 @@ async def _verify_access_helper( role = models.DiscordRole(**json.loads(role["data"])) if role.name in request.auth.scopes: - return True + return - return False + raise UnauthorizedError(status_code=401) -async def verify_response_access(form_id: str, request: starlette.requests.Request) -> bool: +async def verify_response_access(form_id: str, request: starlette.requests.Request) -> None: """Ensure the user can access responses on the requested resource.""" - return await _verify_access_helper(form_id, request, "response_readers") + await _verify_access_helper(form_id, request, "response_readers") -async def verify_edit_access(form_id: str, request: starlette.requests.Request) -> bool: +async def verify_edit_access(form_id: str, request: starlette.requests.Request) -> None: """Ensure the user can view and modify the requested resource.""" - return await _verify_access_helper(form_id, request, "editors") + await _verify_access_helper(form_id, request, "editors") diff --git a/backend/routes/forms/form.py b/backend/routes/forms/form.py index 15ff9a6..b6c6f1d 100644 --- a/backend/routes/forms/form.py +++ b/backend/routes/forms/form.py @@ -17,7 +17,6 @@ from backend.routes.forms.discover import EMPTY_FORM from backend.routes.forms.unittesting import filter_unittests from backend.validation import ErrorMessage, OkayResponse, api -NOT_FOUND_ERROR = JSONResponse({"error": "not_found"}, status_code=404) PUBLIC_FORM_FEATURES = (constants.FormFeatures.OPEN, constants.FormFeatures.DISCOVERABLE) @@ -37,13 +36,15 @@ class SingleForm(Route): form_id = request.path_params["form_id"].lower() try: - admin = await discord.verify_edit_access(form_id, request) + await discord.verify_edit_access(form_id, request) + admin = True except discord.FormNotFoundError: if not constants.PRODUCTION and form_id == EMPTY_FORM.id: # Empty form to help with authentication in development. return JSONResponse(EMPTY_FORM.dict(admin=False)) - else: - return NOT_FOUND_ERROR + raise + except discord.UnauthorizedError: + admin = False filters = { "_id": form_id @@ -63,7 +64,6 @@ class SingleForm(Route): resp=Response( HTTP_200=OkayResponse, HTTP_400=ErrorMessage, - HTTP_401=ErrorMessage, HTTP_404=ErrorMessage, ), tags=["forms"] @@ -76,12 +76,7 @@ class SingleForm(Route): return JSONResponse("Expected a JSON body.", 400) form_id = request.path_params["form_id"].lower() - - try: - if not await discord.verify_edit_access(form_id, request): - return JSONResponse({"error": "unauthorized"}, status_code=401) - except discord.FormNotFoundError: - return NOT_FOUND_ERROR + await discord.verify_edit_access(form_id, request) if raw_form := await request.state.db.forms.find_one({"id": form_id}): if "_id" in data or "id" in data: @@ -116,12 +111,7 @@ class SingleForm(Route): async def delete(self, request: Request) -> JSONResponse: """Deletes form by ID.""" form_id = request.path_params["form_id"].lower() - - try: - if not await discord.verify_edit_access(form_id, request): - return JSONResponse({"error": "unauthorized"}, status_code=401) - except discord.FormNotFoundError: - return NOT_FOUND_ERROR + await discord.verify_edit_access(form_id, request) await request.state.db.forms.delete_one({"_id": form_id}) await request.state.db.responses.delete_many({"form_id": form_id}) diff --git a/backend/routes/forms/response.py b/backend/routes/forms/response.py index fbf8e99..565701f 100644 --- a/backend/routes/forms/response.py +++ b/backend/routes/forms/response.py @@ -21,18 +21,13 @@ class Response(Route): @requires(["authenticated"]) @api.validate( - resp=RouteResponse(HTTP_200=FormResponse, HTTP_401=ErrorMessage, HTTP_404=ErrorMessage), + resp=RouteResponse(HTTP_200=FormResponse, HTTP_404=ErrorMessage), tags=["forms", "responses"] ) async def get(self, request: Request) -> JSONResponse: """Return a single form response by ID.""" form_id = request.path_params["form_id"] - - try: - if not await discord.verify_response_access(form_id, request): - return JSONResponse({"error": "unauthorized"}, status_code=401) - except discord.FormNotFoundError: - return JSONResponse({"error": "form_not_found"}, status_code=404) + await discord.verify_response_access(form_id, request) if raw_response := await request.state.db.responses.find_one( { diff --git a/backend/routes/forms/responses.py b/backend/routes/forms/responses.py index 1c8ebe3..818ebce 100644 --- a/backend/routes/forms/responses.py +++ b/backend/routes/forms/responses.py @@ -27,17 +27,13 @@ class Responses(Route): @requires(["authenticated"]) @api.validate( - resp=Response(HTTP_200=ResponseList, HTTP_401=ErrorMessage, HTTP_404=ErrorMessage), + resp=Response(HTTP_200=ResponseList), tags=["forms", "responses"] ) async def get(self, request: Request) -> JSONResponse: """Returns all form responses by form ID.""" form_id = request.path_params["form_id"] - try: - if not await discord.verify_response_access(form_id, request): - return JSONResponse({"error": "unauthorized"}, 401) - except discord.FormNotFoundError: - return JSONResponse({"error": "not_found"}, 404) + await discord.verify_response_access(form_id, request) cursor = request.state.db.responses.find( {"form_id": form_id} -- cgit v1.2.3 From 10080f8f30fc29fca259347a7747cd27ab4c04ff Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Fri, 11 Feb 2022 00:12:58 +0400 Subject: Ensure Form Existence Is Always Checked Signed-off-by: Hassan Abouelela --- backend/discord.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/discord.py b/backend/discord.py index 856e878..70532bf 100644 --- a/backend/discord.py +++ b/backend/discord.py @@ -164,15 +164,15 @@ async def _verify_access_helper( form_id: str, request: starlette.requests.Request, attribute: str ) -> None: """A low level helper to validate access to a form resource based on the user's scopes.""" - # Short circuit all resources for admins - if "admin" in request.auth.scopes: - return - form = await request.state.db.forms.find_one({"id": form_id}) if not form: raise FormNotFoundError(status_code=404) + # Short circuit all resources for admins + if "admin" in request.auth.scopes: + return + form = models.Form(**form) for role_id in getattr(form, attribute) or []: -- cgit v1.2.3 From 7c3afa8935eb2ace980b170db7b3692e92321fdd Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Mon, 14 Mar 2022 08:06:04 +0400 Subject: Add Default Attribute For Form Access Helper Signed-off-by: Hassan Abouelela --- backend/discord.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/discord.py b/backend/discord.py index 70532bf..5a734db 100644 --- a/backend/discord.py +++ b/backend/discord.py @@ -175,7 +175,7 @@ async def _verify_access_helper( form = models.Form(**form) - for role_id in getattr(form, attribute) or []: + for role_id in getattr(form, attribute, []): role = await request.state.db.roles.find_one({"id": role_id}) if not role: continue -- cgit v1.2.3 From 27b0c38694f6bdd557c58a6bd6c49e5c95d5cfa5 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Mon, 14 Mar 2022 07:14:49 +0300 Subject: Explicitly Use `_id` Over `id` Co-authored-by: Kieran Siek --- backend/discord.py | 4 ++-- backend/routes/forms/form.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/discord.py b/backend/discord.py index 5a734db..be12109 100644 --- a/backend/discord.py +++ b/backend/discord.py @@ -164,12 +164,12 @@ async def _verify_access_helper( form_id: str, request: starlette.requests.Request, attribute: str ) -> None: """A low level helper to validate access to a form resource based on the user's scopes.""" - form = await request.state.db.forms.find_one({"id": form_id}) + form = await request.state.db.forms.find_one({"_id": form_id}) if not form: raise FormNotFoundError(status_code=404) - # Short circuit all resources for admins + # Short circuit all resources for forms admins if "admin" in request.auth.scopes: return diff --git a/backend/routes/forms/form.py b/backend/routes/forms/form.py index b6c6f1d..567c197 100644 --- a/backend/routes/forms/form.py +++ b/backend/routes/forms/form.py @@ -78,7 +78,7 @@ class SingleForm(Route): form_id = request.path_params["form_id"].lower() await discord.verify_edit_access(form_id, request) - if raw_form := await request.state.db.forms.find_one({"id": form_id}): + if raw_form := await request.state.db.forms.find_one({"_id": form_id}): if "_id" in data or "id" in data: if (data.get("id") or data.get("_id")) != form_id: return JSONResponse({"error": "locked_field"}, status_code=400) @@ -97,7 +97,7 @@ class SingleForm(Route): except ValidationError as e: return JSONResponse(e.errors(), status_code=422) - await request.state.db.forms.replace_one({"id": form_id}, form.dict()) + await request.state.db.forms.replace_one({"_id": form_id}, form.dict()) return JSONResponse(form.dict()) else: -- cgit v1.2.3