From 63b796db2386ce8debf7469d5bb43950cda4e06c Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Wed, 29 May 2024 14:44:34 -0700 Subject: [PATCH 01/11] Create Robots table and make all keys "id" More specifically each "id" key is a String and a primary key. No secondary keys are admitted. Note for the future: "columns" is a really bad name for create_dynamodb_table, maybe keys would be better. --- store/app/api/db.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/store/app/api/db.py b/store/app/api/db.py index 3c0b509f..426fdcc1 100644 --- a/store/app/api/db.py +++ b/store/app/api/db.py @@ -34,19 +34,21 @@ async def create_tables(crud: Crud | None = None) -> None: crud._create_dynamodb_table( name="Users", columns=[ - ("email", "S", "HASH"), - # ("banned", "B", "RANGE"), - # ("deleted", "B", "RANGE"), + ("id", "S", "HASH"), ], ), crud._create_dynamodb_table( name="Tokens", columns=[ - ("email", "S", "HASH"), - # ("issued", "N", "RANGE"), - # ("disabled", "B", "RANGE"), + ("id", "S", "HASH"), ], ), + crud._create_dynamodb_table( + name="Robots", + columns=[ + ("id", "S", "HASH"), + ] + ) ) From 15e61489f15c476d2202c5c28cf0b26489744f5b Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Wed, 29 May 2024 14:48:51 -0700 Subject: [PATCH 02/11] Rename "columns" in db to "keys" "Columns" is misleading because it implies that every field of the table ought to be specified, which is contrary to how NoSQL databases work. --- store/app/api/crud/base.py | 6 +++--- store/app/api/db.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/store/app/api/crud/base.py b/store/app/api/crud/base.py index 0dd09544..c4f13408 100644 --- a/store/app/api/crud/base.py +++ b/store/app/api/crud/base.py @@ -32,13 +32,13 @@ async def __aexit__(self, exc_type: Any, exc_val: Any, exc_tb: Any) -> None: # async def _create_dynamodb_table( self, name: str, - columns: list[tuple[str, Literal["S", "N", "B"], Literal["HASH", "RANGE"]]], + keys: list[tuple[str, Literal["S", "N", "B"], Literal["HASH", "RANGE"]]], deletion_protection: bool = False, ) -> None: table = await self.db.create_table( - AttributeDefinitions=[{"AttributeName": n, "AttributeType": t} for n, t, _ in columns], + AttributeDefinitions=[{"AttributeName": n, "AttributeType": t} for n, t, _ in keys], TableName=name, - KeySchema=[{"AttributeName": n, "KeyType": t} for n, _, t in columns], + KeySchema=[{"AttributeName": n, "KeyType": t} for n, _, t in keys], DeletionProtectionEnabled=deletion_protection, BillingMode="PAY_PER_REQUEST", ) diff --git a/store/app/api/db.py b/store/app/api/db.py index 426fdcc1..87ccf076 100644 --- a/store/app/api/db.py +++ b/store/app/api/db.py @@ -33,19 +33,19 @@ async def create_tables(crud: Crud | None = None) -> None: await asyncio.gather( crud._create_dynamodb_table( name="Users", - columns=[ + keys=[ ("id", "S", "HASH"), ], ), crud._create_dynamodb_table( name="Tokens", - columns=[ + keys=[ ("id", "S", "HASH"), ], ), crud._create_dynamodb_table( name="Robots", - columns=[ + keys=[ ("id", "S", "HASH"), ] ) From 7911ae2d2e5f1004772a58bd4e92ab91735200f0 Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Wed, 29 May 2024 15:08:03 -0700 Subject: [PATCH 03/11] reformatting --- store/app/api/db.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/store/app/api/db.py b/store/app/api/db.py index 87ccf076..b5fc9b57 100644 --- a/store/app/api/db.py +++ b/store/app/api/db.py @@ -47,8 +47,8 @@ async def create_tables(crud: Crud | None = None) -> None: name="Robots", keys=[ ("id", "S", "HASH"), - ] - ) + ], + ), ) From 7fc162001b34a44b690619e8b87baded80cfbb13 Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Wed, 29 May 2024 18:37:29 -0700 Subject: [PATCH 04/11] rewrite db table creation, typechecker doesnt work but the types are fine because the command runs --- store/app/api/crud/base.py | 14 +++++++++++++- store/app/api/db.py | 5 +++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/store/app/api/crud/base.py b/store/app/api/crud/base.py index c4f13408..cd25cd20 100644 --- a/store/app/api/crud/base.py +++ b/store/app/api/crud/base.py @@ -33,12 +33,24 @@ async def _create_dynamodb_table( self, name: str, keys: list[tuple[str, Literal["S", "N", "B"], Literal["HASH", "RANGE"]]], + # It turns out having HASH on a GSI does not actually enforce uniqueness. + # Instead, the difference is: you cannot query RANGE fields alone but you may query HASH fields + gsis: list[tuple[str, str, Literal["S", "N", "B"], Literal["HASH", "RANGE"]]] = [], deletion_protection: bool = False, ) -> None: table = await self.db.create_table( - AttributeDefinitions=[{"AttributeName": n, "AttributeType": t} for n, t, _ in keys], + AttributeDefinitions=[{"AttributeName": n, "AttributeType": t} for n, t, _ in keys] + + [{"AttributeName": n, "AttributeType": t} for _, n, t, _ in gsis], TableName=name, KeySchema=[{"AttributeName": n, "KeyType": t} for n, _, t in keys], + GlobalSecondaryIndexes=[ + { + "IndexName": i, + "KeySchema": [{"AttributeName": n, "KeyType": t}], + "Projection": {"ProjectionType": "ALL"}, + } + for i, n, _, t in gsis + ], DeletionProtectionEnabled=deletion_protection, BillingMode="PAY_PER_REQUEST", ) diff --git a/store/app/api/db.py b/store/app/api/db.py index b5fc9b57..9dafdfd7 100644 --- a/store/app/api/db.py +++ b/store/app/api/db.py @@ -36,18 +36,23 @@ async def create_tables(crud: Crud | None = None) -> None: keys=[ ("id", "S", "HASH"), ], + gsis=[ + ("emailIndex", "email", "S", "HASH"), + ], ), crud._create_dynamodb_table( name="Tokens", keys=[ ("id", "S", "HASH"), ], + gsis=[("emailIndex", "email", "S", "HASH")], ), crud._create_dynamodb_table( name="Robots", keys=[ ("id", "S", "HASH"), ], + gsis=[("ownerIndex", "owner", "S", "HASH")], ), ) From 92ad0eebf9dfd4131baac40e355f790e4db6e197 Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Wed, 29 May 2024 23:19:38 -0700 Subject: [PATCH 05/11] fix types for mypy --- store/app/api/crud/base.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/store/app/api/crud/base.py b/store/app/api/crud/base.py index cd25cd20..3d697393 100644 --- a/store/app/api/crud/base.py +++ b/store/app/api/crud/base.py @@ -4,6 +4,7 @@ import aioboto3 from types_aiobotocore_dynamodb.service_resource import DynamoDBServiceResource +import itertools class BaseCrud(AsyncContextManager["BaseCrud"]): @@ -39,8 +40,10 @@ async def _create_dynamodb_table( deletion_protection: bool = False, ) -> None: table = await self.db.create_table( - AttributeDefinitions=[{"AttributeName": n, "AttributeType": t} for n, t, _ in keys] - + [{"AttributeName": n, "AttributeType": t} for _, n, t, _ in gsis], + AttributeDefinitions=[ + {"AttributeName": n, "AttributeType": t} + for n, t in itertools.chain(((n, t) for (n, t, _) in keys), ((n, t) for _, n, t, _ in gsis)) + ], TableName=name, KeySchema=[{"AttributeName": n, "KeyType": t} for n, _, t in keys], GlobalSecondaryIndexes=[ From c3b2b76fe91a8f586860ef755afdc283bdadc252 Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Thu, 30 May 2024 00:07:34 -0700 Subject: [PATCH 06/11] SET SAME_SITE=LAX this is REALLY IMPORTANT!!! --- store/app/api/routers/users.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/store/app/api/routers/users.py b/store/app/api/routers/users.py index cec9c80f..07c2fe8e 100644 --- a/store/app/api/routers/users.py +++ b/store/app/api/routers/users.py @@ -33,8 +33,7 @@ def set_token_cookie(response: Response, token: str, key: str) -> None: value=token, httponly=True, secure=False, - # samesite="strict", - samesite="none", + samesite="Lax", ) From 48ddc20cc8ffd72be264a07e9b543e9f53bcf46d Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Thu, 30 May 2024 09:26:08 -0700 Subject: [PATCH 07/11] "eml" -> "email" This will reduce the mental overload when we inevitably need to deserialzie these tokens! --- store/app/api/token.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/store/app/api/token.py b/store/app/api/token.py index 8ce9ae18..c8fbbf16 100644 --- a/store/app/api/token.py +++ b/store/app/api/token.py @@ -74,7 +74,7 @@ async def create_refresh_token(email: str, crud: Crud) -> str: """ token = Token(email=email) await crud.add_token(token) - return create_token({"eml": email}) + return create_token({"email": email}) def load_refresh_token(payload: str) -> str: @@ -87,4 +87,4 @@ def load_refresh_token(payload: str) -> str: The decoded refresh token data. """ data = load_token(payload) - return data["eml"] + return data["email"] From bebe030a4a7675539119b6a16f43340f0c71022d Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Thu, 30 May 2024 10:02:43 -0700 Subject: [PATCH 08/11] finish db rearch we want unique ids on everything --- store/app/api/crud/users.py | 17 ++++++++++------- store/app/api/model.py | 4 ++++ store/app/api/routers/users.py | 4 +++- store/app/api/token.py | 4 +++- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/store/app/api/crud/users.py b/store/app/api/crud/users.py index 1752427c..8775c111 100644 --- a/store/app/api/crud/users.py +++ b/store/app/api/crud/users.py @@ -3,6 +3,9 @@ import asyncio import warnings +import boto3 +from boto3.dynamodb.conditions import Key + from store.app.api.crud.base import BaseCrud from store.app.api.model import Token, User @@ -14,15 +17,15 @@ async def add_user(self, user: User) -> None: async def get_user(self, email: str) -> User | None: table = await self.db.Table("Users") - user_dict = await table.get_item(Key={"email": email}) - if "Item" not in user_dict: + user_dict = await table.query(IndexName="emailIndex", KeyConditionExpression=Key("email").eq(email)) + if len(user_dict["Items"]) == 0: return None - user = User.model_validate(user_dict["Item"]) + user = User.model_validate(user_dict["Items"][0]) return user async def delete_user(self, user: User) -> None: table = await self.db.Table("Users") - await table.delete_item(Key={"email": user.email}) + await table.delete_item(Key={"id": user.id}) async def list_users(self) -> list[User]: warnings.warn("`list_users` probably shouldn't be called in production", ResourceWarning) @@ -40,10 +43,10 @@ async def add_token(self, token: Token) -> None: async def get_token(self, email: str) -> Token | None: table = await self.db.Table("Tokens") - token_dict = await table.get_item(Key={"email": email}) - if "Item" not in token_dict: + token_dict = await table.query(IndexName="emailIndex", KeyConditionExpression=Key("email").eq(email)) + if len(token_dict["Items"]) == 0: return None - token = Token.model_validate(token_dict["Item"]) + token = Token.model_validate(token_dict["Items"][0]) return token diff --git a/store/app/api/model.py b/store/app/api/model.py index 1cf6035d..88a5cae3 100644 --- a/store/app/api/model.py +++ b/store/app/api/model.py @@ -9,11 +9,15 @@ class User(BaseModel): email: str + id: str banned: bool = field(default=False) deleted: bool = field(default=False) class Token(BaseModel): + # Email of the user the token belongs to email: str + # Id of the token itself, not the user it belongs to. + id: str issued: Decimal = field(default_factory=lambda: Decimal(datetime.datetime.now().timestamp())) disabled: bool = field(default=False) diff --git a/store/app/api/routers/users.py b/store/app/api/routers/users.py index 07c2fe8e..4ea87dda 100644 --- a/store/app/api/routers/users.py +++ b/store/app/api/routers/users.py @@ -17,6 +17,8 @@ from store.app.api.token import create_refresh_token, create_token, load_refresh_token, load_token from store.settings import settings +import uuid + logger = logging.getLogger(__name__) users_router = APIRouter() @@ -106,7 +108,7 @@ async def create_or_get(email: str, crud: Crud) -> User: # Gets or creates the user object. user_obj = await crud.get_user(email) if user_obj is None: - await crud.add_user(User(email=email)) + await crud.add_user(User(id=str(uuid.uuid4()), email=email)) if (user_obj := await crud.get_user(email)) is None: raise RuntimeError("Failed to add user to the database") diff --git a/store/app/api/token.py b/store/app/api/token.py index c8fbbf16..2f89e497 100644 --- a/store/app/api/token.py +++ b/store/app/api/token.py @@ -11,6 +11,8 @@ from store.settings import settings from store.utils import server_time +import uuid + logger = logging.getLogger(__name__) TIME_FORMAT = "%Y-%m-%d %H:%M:%S" @@ -72,7 +74,7 @@ async def create_refresh_token(email: str, crud: Crud) -> str: Returns: The encoded JWT. """ - token = Token(email=email) + token = Token(id=str(uuid.uuid4()), email=email) await crud.add_token(token) return create_token({"email": email}) From 92a9e5896d9c2481522986f691e7fee1b21cea57 Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Thu, 30 May 2024 11:04:45 -0700 Subject: [PATCH 09/11] ruff fix --- store/app/api/crud/base.py | 2 +- store/app/api/crud/users.py | 1 - store/app/api/routers/users.py | 3 +-- store/app/api/token.py | 3 +-- 4 files changed, 3 insertions(+), 6 deletions(-) diff --git a/store/app/api/crud/base.py b/store/app/api/crud/base.py index 3d697393..08c90e0b 100644 --- a/store/app/api/crud/base.py +++ b/store/app/api/crud/base.py @@ -1,10 +1,10 @@ """Defines the base CRUD interface.""" +import itertools from typing import Any, AsyncContextManager, Literal, Self import aioboto3 from types_aiobotocore_dynamodb.service_resource import DynamoDBServiceResource -import itertools class BaseCrud(AsyncContextManager["BaseCrud"]): diff --git a/store/app/api/crud/users.py b/store/app/api/crud/users.py index 8775c111..3c74871b 100644 --- a/store/app/api/crud/users.py +++ b/store/app/api/crud/users.py @@ -3,7 +3,6 @@ import asyncio import warnings -import boto3 from boto3.dynamodb.conditions import Key from store.app.api.crud.base import BaseCrud diff --git a/store/app/api/routers/users.py b/store/app/api/routers/users.py index 4ea87dda..0e89227f 100644 --- a/store/app/api/routers/users.py +++ b/store/app/api/routers/users.py @@ -3,6 +3,7 @@ import asyncio import datetime import logging +import uuid from email.utils import parseaddr as parse_email_address from typing import Annotated @@ -17,8 +18,6 @@ from store.app.api.token import create_refresh_token, create_token, load_refresh_token, load_token from store.settings import settings -import uuid - logger = logging.getLogger(__name__) users_router = APIRouter() diff --git a/store/app/api/token.py b/store/app/api/token.py index 2f89e497..f65bee1b 100644 --- a/store/app/api/token.py +++ b/store/app/api/token.py @@ -2,6 +2,7 @@ import datetime import logging +import uuid import jwt from fastapi import HTTPException, status @@ -11,8 +12,6 @@ from store.settings import settings from store.utils import server_time -import uuid - logger = logging.getLogger(__name__) TIME_FORMAT = "%Y-%m-%d %H:%M:%S" From 0638c97b24aaefeabea06c62a3a0654d5066bbde Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Thu, 30 May 2024 11:33:09 -0700 Subject: [PATCH 10/11] fix some things without ids --- store/app/api/crud/users.py | 3 ++- store/app/api/routers/users.py | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/store/app/api/crud/users.py b/store/app/api/crud/users.py index 3c74871b..56491f45 100644 --- a/store/app/api/crud/users.py +++ b/store/app/api/crud/users.py @@ -1,6 +1,7 @@ """Defines CRUD interface for user API.""" import asyncio +import uuid import warnings from boto3.dynamodb.conditions import Key @@ -51,7 +52,7 @@ async def get_token(self, email: str) -> Token | None: async def test_adhoc() -> None: async with UserCrud() as crud: - await crud.add_user(User(email="ben@kscale.dev")) + await crud.add_user(User(id=str(uuid.uuid4()), email="ben@kscale.dev")) # print(await crud.get_user("ben")) # print(await crud.get_user_count()) # await crud.get_token("ben") diff --git a/store/app/api/routers/users.py b/store/app/api/routers/users.py index 0e89227f..ffe4a702 100644 --- a/store/app/api/routers/users.py +++ b/store/app/api/routers/users.py @@ -34,7 +34,7 @@ def set_token_cookie(response: Response, token: str, key: str) -> None: value=token, httponly=True, secure=False, - samesite="Lax", + samesite="lax", ) @@ -99,7 +99,7 @@ class UserLoginResponse(BaseModel): async def add_to_waitlist(email: str, crud: Crud) -> None: await asyncio.gather( send_waitlist_email(email), - crud.add_user(User(email=email, banned=True)), + crud.add_user(User(id=str(uuid.uuid4()), email=email, banned=True)), ) From 0de6e165788013b59833732d545ae53236136471 Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Thu, 30 May 2024 13:42:07 -0700 Subject: [PATCH 11/11] fix docs --- store/app/api/crud/base.py | 12 ++++++++++-- store/app/api/model.py | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/store/app/api/crud/base.py b/store/app/api/crud/base.py index 08c90e0b..bc7cf131 100644 --- a/store/app/api/crud/base.py +++ b/store/app/api/crud/base.py @@ -30,12 +30,20 @@ async def __aexit__(self, exc_type: Any, exc_val: Any, exc_tb: Any) -> None: # if self.__db is not None: await self.__db.__aexit__(exc_type, exc_val, exc_tb) + """Creates a table in the Dynamo database. + + Args: + name: Name of the table. + keys: Primary and secondary keys. Do not include non-key attributes. + gsis: Making an attribute a GSI is required in oredr to query against it. + Note HASH on a GSI does not actually enforce uniqueness. + Instead, the difference is: you cannot query RANGE fields alone but you may query HASH fields + deletion_protection: Whether the table is protected from being deleted. + """ async def _create_dynamodb_table( self, name: str, keys: list[tuple[str, Literal["S", "N", "B"], Literal["HASH", "RANGE"]]], - # It turns out having HASH on a GSI does not actually enforce uniqueness. - # Instead, the difference is: you cannot query RANGE fields alone but you may query HASH fields gsis: list[tuple[str, str, Literal["S", "N", "B"], Literal["HASH", "RANGE"]]] = [], deletion_protection: bool = False, ) -> None: diff --git a/store/app/api/model.py b/store/app/api/model.py index 88a5cae3..f3fc1004 100644 --- a/store/app/api/model.py +++ b/store/app/api/model.py @@ -17,7 +17,7 @@ class User(BaseModel): class Token(BaseModel): # Email of the user the token belongs to email: str - # Id of the token itself, not the user it belongs to. + # ID of the token itself, not the user it belongs to. id: str issued: Decimal = field(default_factory=lambda: Decimal(datetime.datetime.now().timestamp())) disabled: bool = field(default=False)