Skip to content

Commit

Permalink
Remove Email Auth (#664)
Browse files Browse the repository at this point in the history
* Remove email-based authentication endpoints and models

- Remove email router and CRUD operations
- Remove EmailSignUpToken model
- Remove email-specific utility functions
- Update imports and references

This change removes the email-based authentication system as we no longer allow email authentication.

Co-Authored-By: Benjamin Bolte <[email protected]>

* Remove remaining email/password management functions (send_change_email, send_reset_password_email)

Co-Authored-By: Benjamin Bolte <[email protected]>

* Remove remaining email functions: send_delete_email, send_waitlist_email

Co-Authored-By: Benjamin Bolte <[email protected]>

* Enhance email notifications: restore delete email and add signup notification with tests

Co-Authored-By: Benjamin Bolte <[email protected]>

* Fix OAuth signup test: Use unique mock emails for different providers

Co-Authored-By: Benjamin Bolte <[email protected]>

* Fix test mocks: Add Google OAuth mock and use consistent mock emails

Co-Authored-By: Benjamin Bolte <[email protected]>

* update requirements

* fix failing test

* fix small cloudfront signing logic bug

* remove publish workflow

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: Benjamin Bolte <[email protected]>
Co-authored-by: Benjamin Bolte <[email protected]>
  • Loading branch information
3 people authored Dec 27, 2024
1 parent 05e627e commit 0b09dbf
Show file tree
Hide file tree
Showing 18 changed files with 166 additions and 332 deletions.
47 changes: 0 additions & 47 deletions .github/workflows/publish.yml

This file was deleted.

3 changes: 3 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,7 @@
}
],
"editor.formatOnSave": true,
"python.testing.pytestArgs": [],
"python.testing.unittestEnabled": false,
"python.testing.pytestEnabled": true
}
11 changes: 9 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,19 @@ def mock_github_access_token(mocker: MockerFixture) -> MockType:
@pytest.fixture(autouse=True)
def mock_github(mocker: MockerFixture) -> MockType:
mock = mocker.patch("www.app.routers.auth.github.github_req")
mock.return_value = Response(status_code=200, json={"html_url": "https://github.com/chennisden"})
mock.return_value = Response(status_code=200, json={"html_url": "https://github.com/kscalelabs"})
return mock


@pytest.fixture(autouse=True)
def mock_github_email(mocker: MockerFixture) -> MockType:
mock = mocker.patch("www.app.routers.auth.github.github_email_req")
mock.return_value = Response(status_code=200, json=[{"email": "[email protected]", "primary": True}])
mock.return_value = Response(status_code=200, json=[{"email": "[email protected]", "primary": True}])
return mock


@pytest.fixture(autouse=True)
def mock_google_user_data(mocker: MockerFixture) -> MockType:
mock = mocker.patch("www.app.routers.auth.google.get_google_user_data")
mock.return_value = {"email": "[email protected]", "given_name": "Test", "family_name": "User"}
return mock
67 changes: 57 additions & 10 deletions tests/test_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@

from fastapi import status
from fastapi.testclient import TestClient
from httpx import AsyncClient
from pytest_mock.plugin import MockType

from www.app.db import create_tables


def test_user_auth_functions(test_client: TestClient) -> None:
def test_user_auth_functions(test_client: TestClient, mock_send_email: MockType) -> None:
# Checks that without the session token we get a 401 response.
response = test_client.get("/users/me")
assert response.status_code == status.HTTP_401_UNAUTHORIZED, response.json()
Expand Down Expand Up @@ -48,30 +46,79 @@ def test_user_auth_functions(test_client: TestClient) -> None:
response = test_client.delete("/users/me", headers=auth_headers)
assert response.status_code == status.HTTP_200_OK, response.json()
assert response.json() is True
# Verify delete email was sent
mock_send_email.assert_called_with(
subject="Account Deleted - K-Scale Labs",
body=mock_send_email.call_args[1]["body"], # Don't compare exact HTML
to="[email protected]", # Using consistent mock email from GitHub OAuth
)

# Tries deleting the user again, which should fail.
response = test_client.delete("/users/me", headers=auth_headers)
assert response.status_code == status.HTTP_401_UNAUTHORIZED, response.json()
assert response.json()["detail"] == "Not authenticated"


async def test_user_general_functions(app_client: AsyncClient) -> None:
await create_tables()

async def test_user_general_functions(test_client: TestClient) -> None:
# Because of the way we patched GitHub functions for mocking, it doesn't matter what token we pass in.
response = await app_client.post("/auth/github/code", json={"code": "test_code"})
response = test_client.post("/auth/github/code", json={"code": "test_code"})
assert response.status_code == status.HTTP_200_OK, response.json()
token = response.json()["api_key"]
auth_headers = {"Authorization": f"Bearer {token}"}

# Update the user's profile (e.g., change first_name).
update_data = {"first_name": "UpdatedFirstName", "last_name": "UpdatedLastName"}
response = await app_client.put("/users/me", headers=auth_headers, json=update_data)
response = test_client.put("/users/me", headers=auth_headers, json=update_data)
assert response.status_code == status.HTTP_200_OK, response.json()

# Verify that the user's profile has been updated.
response = await app_client.get("/users/me", headers=auth_headers)
response = test_client.get("/users/me", headers=auth_headers)
assert response.status_code == status.HTTP_200_OK, response.json()
updated_user_data = response.json()
assert updated_user_data["first_name"] == "UpdatedFirstName"
assert updated_user_data["last_name"] == "UpdatedLastName"

# Delete the user when finished.
response = test_client.delete("/users/me", headers=auth_headers)
assert response.status_code == status.HTTP_200_OK, response.json()
assert response.json() is True


async def test_oauth_signup_notifications(test_client: TestClient, mock_send_email: MockType) -> None:
"""Test that signup notification emails are sent when users sign up via OAuth."""
mock_send_email.reset_mock()

# Test GitHub signup
response = test_client.post("/auth/github/code", json={"code": "test_code"})
assert response.status_code == status.HTTP_200_OK, response.json()
mock_send_email.assert_called_with(
subject="Welcome to K-Scale Labs",
body=mock_send_email.call_args[1]["body"], # Don't compare exact HTML
to="[email protected]", # Mock GitHub user email
)

# Delete the user when finished.
token = response.json()["api_key"]
auth_headers = {"Authorization": f"Bearer {token}"}
response = test_client.delete("/users/me", headers=auth_headers)
assert response.status_code == status.HTTP_200_OK, response.json()
assert response.json() is True

# Reset mock for next test
mock_send_email.reset_mock()

# Test Google signup with different user
response = test_client.post("/auth/google/login", json={"token": "test_code"})
assert response.status_code == status.HTTP_200_OK, response.json()
mock_send_email.assert_called_with(
subject="Welcome to K-Scale Labs",
body=mock_send_email.call_args[1]["body"], # Don't compare exact HTML
to="[email protected]", # Mock Google user email
)

# Delete the user when finished.
token = response.json()["api_key"]
auth_headers = {"Authorization": f"Bearer {token}"}
response = test_client.delete("/users/me", headers=auth_headers)
assert response.status_code == status.HTTP_200_OK, response.json()
assert response.json() is True
24 changes: 14 additions & 10 deletions www/app/crud/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from aiobotocore.response import StreamingBody
from boto3.dynamodb.conditions import Attr, ComparisonCondition, Key
from botocore.exceptions import ClientError
from types_aiobotocore_dynamodb.service_resource import DynamoDBServiceResource
from types_aiobotocore_dynamodb.service_resource import DynamoDBServiceResource, Table
from types_aiobotocore_s3.service_resource import S3ServiceResource

from www.app.errors import InternalError, ItemNotFoundError
Expand Down Expand Up @@ -59,6 +59,10 @@ def s3(self) -> S3ServiceResource:
raise RuntimeError("Must call __aenter__ first!")
return self.__s3

@property
async def table(self) -> Table:
return await self.db.Table(TABLE_NAME)

@classmethod
def get_gsis(cls) -> set[str]:
return {"type"}
Expand All @@ -85,7 +89,7 @@ async def __aexit__(self, exc_type: Any, exc_val: Any, exc_tb: Any) -> None: #
await asyncio.gather(*(resource.__aexit__(exc_type, exc_val, exc_tb) for resource in to_close))

async def _add_item(self, item: StoreBaseModel, unique_fields: list[str] | None = None) -> None:
table = await self.db.Table(TABLE_NAME)
table = await self.table
item_data = item.model_dump()

# Ensure no empty strings are present
Expand Down Expand Up @@ -116,7 +120,7 @@ async def _add_item(self, item: StoreBaseModel, unique_fields: list[str] | None
raise

async def _delete_item(self, item: StoreBaseModel | str) -> None:
table = await self.db.Table(TABLE_NAME)
table = await self.table
await table.delete_item(Key={"id": item if isinstance(item, str) else item.id})

async def _list_items(
Expand All @@ -128,7 +132,7 @@ async def _list_items(
offset: int | None = None,
limit: int = DEFAULT_SCAN_LIMIT,
) -> list[T]:
table = await self.db.Table(TABLE_NAME)
table = await self.table

query_params = {
"IndexName": "type_index",
Expand Down Expand Up @@ -206,7 +210,7 @@ async def _list_me(
return sorted_items[(page - 1) * ITEMS_PER_PAGE : page * ITEMS_PER_PAGE], page * ITEMS_PER_PAGE < len(response)

async def _count_items(self, item_class: type[T]) -> int:
table = await self.db.Table(TABLE_NAME)
table = await self.table
item_dict = await table.scan(
IndexName="type_index",
Select="COUNT",
Expand Down Expand Up @@ -236,7 +240,7 @@ async def _get_item(
) -> T | None: ...

async def _get_item(self, item_id: str, item_class: type[T], throw_if_missing: bool = False) -> T | None:
table = await self.db.Table(TABLE_NAME)
table = await self.table
item_dict = await table.get_item(Key={"id": item_id})
if "Item" not in item_dict:
if throw_if_missing:
Expand All @@ -246,7 +250,7 @@ async def _get_item(self, item_id: str, item_class: type[T], throw_if_missing: b
return self._validate_item(item_data, item_class)

async def _item_exists(self, item_id: str) -> bool:
table = await self.db.Table(TABLE_NAME)
table = await self.table
item_dict = await table.get_item(Key={"id": item_id})
return "Item" in item_dict

Expand Down Expand Up @@ -283,7 +287,7 @@ async def _get_items_from_secondary_index(
filter_expression: ComparisonCondition = Key("type").eq(item_class.__name__)
if additional_filter_expression is not None:
filter_expression &= additional_filter_expression
table = await self.db.Table(TABLE_NAME)
table = await self.table
item_dict = await table.query(
IndexName=self.get_gsi_index_name(secondary_index_name),
KeyConditionExpression=Key(secondary_index_name).eq(secondary_index_value),
Expand All @@ -310,7 +314,7 @@ async def _get_items_from_secondary_index_batch(
chunk_size: int = DEFAULT_CHUNK_SIZE,
) -> list[list[T]]:
items: list[list[T]] = []
table = await self.db.Table(TABLE_NAME)
table = await self.table

for i in range(0, len(secondary_index_values), chunk_size):
chunk = secondary_index_values[i : i + chunk_size]
Expand Down Expand Up @@ -550,7 +554,7 @@ async def _delete_dynamodb_table(self, name: str) -> None:
logger.info("Table %s does not exist", name)

async def _get_by_known_id(self, record_id: str) -> dict[str, Any] | None:
table = await self.db.Table(TABLE_NAME)
table = await self.table
response = await table.get_item(Key={"id": record_id})
return response.get("Item")

Expand Down
30 changes: 0 additions & 30 deletions www/app/crud/email.py

This file was deleted.

20 changes: 19 additions & 1 deletion www/app/crud/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
User,
UserPermission,
)
from www.app.utils.email import send_signup_notification_email
from www.settings import settings
from www.utils import cache_async_result

Expand Down Expand Up @@ -121,13 +122,15 @@ async def _create_user_from_oauth(
elif provider == "google":
user.google_id = user_token
await self._add_item(user, unique_fields=["email", "username"])
await send_signup_notification_email(email)
elif provider == "github":
await self._update_item(user.id, User, {"github_id": user_token})
elif provider == "google":
await self._update_item(user.id, User, {"google_id": user_token})

oauth_key = OAuthKey.create(user_id=user.id, provider=provider, user_token=user_token)
await self._add_item(oauth_key, unique_fields=["user_token"])

return user
except Exception as e:
logger.exception("Error in _create_user_from_oauth: %s", e)
Expand Down Expand Up @@ -166,7 +169,10 @@ async def delete_github_token(self, github_id: str) -> None:
await self._delete_item(await self._get_oauth_key(github_auth_key(github_id), throw_if_missing=True))

async def get_user_from_google_token(
self, email: str, first_name: Optional[str] = None, last_name: Optional[str] = None
self,
email: str,
first_name: Optional[str] = None,
last_name: Optional[str] = None,
) -> User:
try:
auth_key = google_auth_key(email)
Expand All @@ -192,6 +198,18 @@ async def get_user_from_api_key(self, api_key_id: str) -> User:
return await self._get_item(api_key.user_id, User, throw_if_missing=True)

async def delete_user(self, id: str) -> None:
async def delete_api_keys() -> None:
api_keys = await self.list_api_keys(id)
await asyncio.gather(*[self.delete_api_key(key) for key in api_keys])

async def delete_oauth_keys() -> None:
oauth_keys = await self._get_items_from_secondary_index("user_id", id, OAuthKey)
await asyncio.gather(*[self._delete_item(key) for key in oauth_keys])

# Delete the user's API keys and OAuth keys.
await asyncio.gather(delete_api_keys(), delete_oauth_keys())

# Delete the user itself.
await self._delete_item(id)

async def list_users(self) -> list[User]:
Expand Down
2 changes: 0 additions & 2 deletions www/app/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

from www.app.crud.artifacts import ArtifactsCrud
from www.app.crud.base import TABLE_NAME, BaseCrud
from www.app.crud.email import EmailCrud
from www.app.crud.krecs import KRecsCrud
from www.app.crud.listings import ListingsCrud
from www.app.crud.onshape import OnshapeCrud
Expand All @@ -18,7 +17,6 @@

class Crud(
OnshapeCrud,
EmailCrud,
UserCrud,
ListingsCrud,
ArtifactsCrud,
Expand Down
Loading

0 comments on commit 0b09dbf

Please sign in to comment.