From b771f57f35cfd75bbed60ae677593e9c21034393 Mon Sep 17 00:00:00 2001 From: msramalho <19508417+msramalho@users.noreply.github.com> Date: Tue, 29 Oct 2024 12:44:01 +0000 Subject: [PATCH 1/7] all users are inactive unless in user-groups --- src/db/crud.py | 12 ++++++++---- src/db/models.py | 2 +- src/tests/db/test_crud.py | 11 +++++++++-- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/db/crud.py b/src/db/crud.py index ddd0412..a695b20 100644 --- a/src/db/crud.py +++ b/src/db/crud.py @@ -106,7 +106,7 @@ def create_tag(db: Session, tag: str): def is_active_user(db: Session, email: str) -> bool: email = email.lower() - return len(email) and db.query(models.User).filter(models.User.email == email).count() > 0 + return len(email) and db.query(models.User).filter(models.User.email == email, models.User.is_active == True).first() is not None def is_user_in_group(db: Session, group_name: str, email: str) -> models.Group: if email == ALLOW_ANY_EMAIL: return True @@ -129,11 +129,11 @@ def get_user_groups(db: Session, email: str): # --------------- INIT User-Groups -def create_or_get_user(db: Session, author_id: str): +def create_or_get_user(db: Session, author_id: str, is_active: bool = models.User.is_active.default.arg) -> models.User: if type(author_id) == str: author_id = author_id.lower() db_user = db.query(models.User).filter(models.User.email == author_id).first() if not db_user: - db_user = models.User(email=author_id) + db_user = models.User(email=author_id, is_active=is_active) db.add(db_user) db.commit() db.refresh(db_user) @@ -175,14 +175,18 @@ def upsert_user_groups(db: Session): logger.debug(f"Found {len(user_groups)} users.") db.query(models.association_table_user_groups).delete() + # set all users to inactive + db.query(models.User).update({models.User.is_active: False}) for user_email, groups in user_groups.items(): user_email = user_email.lower() assert '@' in user_email, f'Invalid user email {user_email}' logger.info(f"email='{user_email[0:3]}...{user_email[-8:]}', {groups=}") db_user = db.query(models.User).filter(models.User.email == user_email).first() if db_user is None: - db_user = models.User(email=user_email) + db_user = models.User(email=user_email, is_active=True) db.add(db_user) + else: + db_user.is_active = True if not groups: continue # avoid hanging in for x in None: for group in groups: db_group = create_or_get_group(db, group) diff --git a/src/db/models.py b/src/db/models.py index 29f0a05..2718687 100644 --- a/src/db/models.py +++ b/src/db/models.py @@ -65,7 +65,7 @@ class User(Base): __tablename__ = "users" email = Column(String, primary_key=True, index=True) - is_active = Column(Boolean, default=True) + is_active = Column(Boolean, default=False) archives = relationship("Archive", back_populates="author") groups = relationship("Group", back_populates="users", secondary=association_table_user_groups) diff --git a/src/tests/db/test_crud.py b/src/tests/db/test_crud.py index edd9abf..7b50447 100644 --- a/src/tests/db/test_crud.py +++ b/src/tests/db/test_crud.py @@ -342,15 +342,22 @@ def test_create_or_get_user(test_data, db_session): assert db_session.query(models.User).count() == 4 + # already exists assert (u1 := crud.create_or_get_user(db_session, "rick@example.com")) is not None assert u1.email == "rick@example.com" assert u1.is_active == True - assert (u2 := crud.create_or_get_user(db_session, "beth@example.com")) is not None + # new active + assert (u2 := crud.create_or_get_user(db_session, "beth@example.com", is_active=True)) is not None assert u2.email == "beth@example.com" assert u2.is_active == True - assert db_session.query(models.User).count() == 5 + # new not active + assert (u3 := crud.create_or_get_user(db_session, "not-active@example.com")) is not None + assert u3.email == "not-active@example.com" + assert u3.is_active == False + + assert db_session.query(models.User).count() == 6 def test_get_group(test_data, db_session): From aaada7d83f08253dd885b68795c8a0a7039f931d Mon Sep 17 00:00:00 2001 From: msramalho <19508417+msramalho@users.noreply.github.com> Date: Tue, 29 Oct 2024 12:57:07 +0000 Subject: [PATCH 2/7] users in domains are active --- src/core/config.py | 2 +- src/db/crud.py | 5 +++++ src/tests/db/test_crud.py | 3 ++- src/tests/endpoints/test_default.py | 12 +++++++----- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/core/config.py b/src/core/config.py index 7f6393e..d3cba7a 100644 --- a/src/core/config.py +++ b/src/core/config.py @@ -1,4 +1,4 @@ -VERSION = "0.7.1" +VERSION = "0.7.2" API_DESCRIPTION = """ #### API for the Auto-Archiver project, a tool to archive web pages and Google Sheets. diff --git a/src/db/crud.py b/src/db/crud.py index a695b20..746759c 100644 --- a/src/db/crud.py +++ b/src/db/crud.py @@ -106,6 +106,11 @@ def create_tag(db: Session, tag: str): def is_active_user(db: Session, email: str) -> bool: email = email.lower() + if "@" not in email: return False + global DOMAIN_GROUPS, DOMAIN_GROUPS_LOADED + if not DOMAIN_GROUPS_LOADED: upsert_user_groups(db) + domain = email.split('@')[1] + if domain in DOMAIN_GROUPS: return True return len(email) and db.query(models.User).filter(models.User.email == email, models.User.is_active == True).first() is not None def is_user_in_group(db: Session, group_name: str, email: str) -> models.Group: diff --git a/src/tests/db/test_crud.py b/src/tests/db/test_crud.py index 7b50447..3913548 100644 --- a/src/tests/db/test_crud.py +++ b/src/tests/db/test_crud.py @@ -298,9 +298,10 @@ def test_is_active_user(test_data, db_session): assert crud.is_active_user(db_session, "") == False assert crud.is_active_user(db_session, "example.com") == False - assert crud.is_active_user(db_session, "unknown@example.com") == False + assert crud.is_active_user(db_session, "unknown@example.com") == True assert crud.is_active_user(db_session, "rick@example.com") == True assert crud.is_active_user(db_session, "RICK@example.com") == True + assert crud.is_active_user(db_session, "rick@not-in-groups.com") == False def test_is_user_in_group(test_data, db_session): diff --git a/src/tests/endpoints/test_default.py b/src/tests/endpoints/test_default.py index b5e2e77..6a585e6 100644 --- a/src/tests/endpoints/test_default.py +++ b/src/tests/endpoints/test_default.py @@ -55,11 +55,13 @@ def test_endpoint_active_true_user(client_with_auth): assert r.status_code == 200 assert r.json() == {"active": True} -def test_endpoint_active_true_user(client_with_auth, db_session): - from db import models - db_session.query(models.User).delete() - db_session.commit() - r = client_with_auth.get("/user/active") +def test_endpoint_active_false_user(app): + from web.security import get_user_auth + + app.dependency_overrides[get_user_auth] = lambda: "morty@not-recognized-group.com" + client = TestClient(app) + r = client.get("/user/active") + assert r.status_code == 200 assert r.json() == {"active": False} From 8c658cdf52228b2b842d457ce53ef2366463d0ac Mon Sep 17 00:00:00 2001 From: msramalho <19508417+msramalho@users.noreply.github.com> Date: Tue, 29 Oct 2024 16:17:40 +0000 Subject: [PATCH 3/7] switching from optional response_model to mandatory return type --- src/db/crud.py | 6 +++--- src/db/schemas.py | 7 +++++++ src/endpoints/default.py | 8 ++++---- src/endpoints/sheet.py | 4 ++-- src/endpoints/task.py | 4 ++-- src/endpoints/url.py | 21 +++++++++++---------- src/tests/endpoints/test_url.py | 15 ++++++--------- 7 files changed, 35 insertions(+), 30 deletions(-) diff --git a/src/db/crud.py b/src/db/crud.py index 746759c..c3c069f 100644 --- a/src/db/crud.py +++ b/src/db/crud.py @@ -87,10 +87,10 @@ def count_by_user_since(db: Session, seconds_delta: int = 15): def base_query(db: Session): - # TODO: allow only some fields to be returned, for example author should remain hidden + #NOTE: load_only is for optimization and not obfuscation, use .with_entities() if needed return db.query(models.Archive)\ - .options(load_only(models.Archive.id, models.Archive.created_at, models.Archive.url, models.Archive.result))\ - .filter(models.Archive.deleted == False) + .filter(models.Archive.deleted == False)\ + .options(load_only(models.Archive.id, models.Archive.created_at, models.Archive.url, models.Archive.result)) # --------------- TAG diff --git a/src/db/schemas.py b/src/db/schemas.py index aa9abd7..538609a 100644 --- a/src/db/schemas.py +++ b/src/db/schemas.py @@ -45,6 +45,13 @@ class SubmitManual(BaseModel): group_id: str | None = None tags: set[str] | None = set() +# API RESPONSES BELOW +class ArchiveResult(BaseModel): + id: str + url: str + result: dict + created_at: datetime + class Task(BaseModel): id: str diff --git a/src/endpoints/default.py b/src/endpoints/default.py index a294fda..4569bd9 100644 --- a/src/endpoints/default.py +++ b/src/endpoints/default.py @@ -30,13 +30,13 @@ async def health(): return JSONResponse({"status": "ok"}) -@default_router.get("/user/active", summary="Check if the user is active and can use the tool.", response_model=schemas.ActiveUser) -async def active(db: Session = Depends(get_db_dependency), email=Depends(get_user_auth)): +@default_router.get("/user/active", summary="Check if the user is active and can use the tool.") +async def active(db: Session = Depends(get_db_dependency), email=Depends(get_user_auth)) -> schemas.ActiveUser: return {"active": crud.is_active_user(db, email)} -@default_router.get("/groups", response_model=list[str]) -def get_user_groups(db: Session = Depends(get_db_dependency), email=Depends(get_user_auth)): +@default_router.get("/groups") +def get_user_groups(db: Session = Depends(get_db_dependency), email=Depends(get_user_auth)) -> list[str]: return crud.get_user_groups(db, email) diff --git a/src/endpoints/sheet.py b/src/endpoints/sheet.py index 5a75c89..5a32d4b 100644 --- a/src/endpoints/sheet.py +++ b/src/endpoints/sheet.py @@ -12,8 +12,8 @@ from worker.main import create_sheet_task sheet_router = APIRouter(prefix="/sheet", tags=["Google Spreadsheet operations"]) -@sheet_router.post("/archive", status_code=201, summary="Submit a Google Sheet archive request, starts a sheet archiving task.", response_model=schemas.Task, response_description="task_id for the archiving task.") -def archive_sheet(sheet:schemas.SubmitSheet, email = Depends(get_token_or_user_auth)): +@sheet_router.post("/archive", status_code=201, summary="Submit a Google Sheet archive request, starts a sheet archiving task.", response_description="task_id for the archiving task.") +def archive_sheet(sheet:schemas.SubmitSheet, email = Depends(get_token_or_user_auth)) -> schemas.Task: logger.info(f"SHEET TASK for {sheet=}") if email == ALLOW_ANY_EMAIL: email = sheet.author_id or "api-endpoint" diff --git a/src/endpoints/task.py b/src/endpoints/task.py index 1a0c6ac..0887183 100644 --- a/src/endpoints/task.py +++ b/src/endpoints/task.py @@ -14,8 +14,8 @@ from worker.main import celery task_router = APIRouter(prefix="/task", tags=["Async task operations"]) -@task_router.get("/{task_id}", response_model=schemas.TaskResult, summary="Check the status of an async task by its id, works for URLs and Sheet tasks.") -def get_status(task_id, email=Depends(get_token_or_user_auth)): +@task_router.get("/{task_id}", summary="Check the status of an async task by its id, works for URLs and Sheet tasks.") +def get_status(task_id, email=Depends(get_token_or_user_auth)) -> schemas.TaskResult: logger.info(f"status check for user {email} task {task_id}") task = AsyncResult(task_id, app=celery) try: diff --git a/src/endpoints/url.py b/src/endpoints/url.py index 3932f73..b17b082 100644 --- a/src/endpoints/url.py +++ b/src/endpoints/url.py @@ -15,8 +15,8 @@ from worker.main import create_archive_task url_router = APIRouter(prefix="/url", tags=["Single URL operations"]) -@url_router.post("/archive", status_code=201, summary="Submit a single URL archive request, starts an archiving task.", response_model=schemas.Task, response_description="task_id for the archiving task, will match the archive id.") -def archive_url(archive: schemas.ArchiveCreate, email=Depends(get_token_or_user_auth)): +@url_router.post("/archive", status_code=201, summary="Submit a single URL archive request, starts an archiving task.", response_description="task_id for the archiving task, will match the archive id.") +def archive_url(archive: schemas.ArchiveCreate, email=Depends(get_token_or_user_auth)) -> schemas.Task: archive.author_id = email url = archive.url logger.info(f"new {archive.public=} task for {email=} and {archive.group_id=}: {url}") @@ -28,30 +28,31 @@ def archive_url(archive: schemas.ArchiveCreate, email=Depends(get_token_or_user_ return JSONResponse(task_response.model_dump(), status_code=201) -@url_router.get("/search", response_model=list[schemas.Archive], summary="Search for archive entries by URL.") +@url_router.get("/search", summary="Search for archive entries by URL.") def search_by_url( url: str, skip: int = 0, limit: int = 25, archived_after: datetime = None, archived_before: datetime = None, db: Session = Depends(get_db_dependency), - email=Depends(get_token_or_user_auth)): + email=Depends(get_token_or_user_auth) +) -> list[schemas.ArchiveResult]: return crud.search_archives_by_url(db, url.strip(), email, skip=skip, limit=limit, archived_after=archived_after, archived_before=archived_before) -@url_router.get("/latest", response_model=list[schemas.Archive], summary="Fetch latest URL archives for the authenticated user.") -def latest(skip: int = 0, limit: int = 25, db: Session = Depends(get_db_dependency), email=Depends(get_user_auth)): +@url_router.get("/latest", summary="Fetch latest URL archives for the authenticated user.") +def latest(skip: int = 0, limit: int = 25, db: Session = Depends(get_db_dependency), email=Depends(get_user_auth)) -> list[schemas.ArchiveResult]: return crud.search_archives_by_email(db, email, skip=skip, limit=limit) -@url_router.get("/{id}", response_model=schemas.Archive, summary="Fetch a single URL archive by the associated id.") -def lookup(id, db: Session = Depends(get_db_dependency), email=Depends(get_token_or_user_auth)): +@url_router.get("/{id}", summary="Fetch a single URL archive by the associated id.") +def lookup(id, db: Session = Depends(get_db_dependency), email=Depends(get_token_or_user_auth)) -> schemas.ArchiveResult: archive = crud.get_archive(db, id, email) if archive is None: raise HTTPException(status_code=404, detail="Archive not found") return archive -@url_router.delete("/{id}", response_model=schemas.TaskDelete, summary="Delete a single URL archive by id.") -def delete_task(id, db: Session = Depends(get_db_dependency), email=Depends(get_user_auth)): +@url_router.delete("/{id}", summary="Delete a single URL archive by id.") +def delete_task(id, db: Session = Depends(get_db_dependency), email=Depends(get_user_auth)) -> schemas.TaskDelete: logger.info(f"deleting url archive task {id} request by {email}") return JSONResponse({ "id": id, diff --git a/src/tests/endpoints/test_url.py b/src/tests/endpoints/test_url.py index 3d05434..4506c1c 100644 --- a/src/tests/endpoints/test_url.py +++ b/src/tests/endpoints/test_url.py @@ -38,7 +38,7 @@ def test_search_by_url(client_with_auth, db_session): assert response.status_code == 200 assert response.json() == [] - from db import crud + from db import crud, schemas for i in range(11): crud.create_task(db_session, ArchiveCreate(id=f"url-456-{i}", url="https://example.com" if i < 10 else "https://something-else.com", result={}, public=True, author_id="rick@example.com", group_id=None), [], []) #NB: this insertion is too fast for the ordering to be correct as they are within the same second @@ -49,6 +49,7 @@ def test_search_by_url(client_with_auth, db_session): assert "url-456-0" in [i["id"] for i in j] assert "url-456-9" in [i["id"] for i in j] assert "url-456-10" not in [i["id"] for i in j] + assert j[0].keys() == schemas.ArchiveResult.model_fields.keys() response = client_with_auth.get("/url/search?url=https://example.com&limit=5") assert response.status_code == 200 @@ -76,7 +77,7 @@ def test_latest(client_with_auth, db_session): assert response.status_code == 200 assert response.json() == [] - from db import crud + from db import crud, schemas for i in range(11): crud.create_task(db_session, ArchiveCreate(id=f"latest-456-{i}", url="https://example.com", result={}, public=True, author_id="morty@example.com" if i < 10 else "rick@example.com", group_id=None), [], []) #NB: this insertion is too fast for the ordering to be correct as they are within the same second @@ -90,6 +91,7 @@ def test_latest(client_with_auth, db_session): assert "latest-456-0" in [i["id"] for i in j] assert "latest-456-9" in [i["id"] for i in j] assert "latest-456-10" not in [i["id"] for i in j] + assert j[0].keys() == schemas.ArchiveResult.model_fields.keys() response = client_with_auth.get("/url/latest?limit=5") assert response.status_code == 200 @@ -109,21 +111,16 @@ def test_lookup(client_with_auth, db_session): assert response.status_code == 404 assert response.json() == {"detail": "Archive not found"} - from db import crud + from db import crud, schemas crud.create_task(db_session, ArchiveCreate(id="lookup-123-456-789", url="https://example.com", result={}, public=True, author_id="rick@example.com", group_id=None), [], []) response = client_with_auth.get("/url/lookup-123-456-789") assert response.status_code == 200 j = response.json() + assert j.keys() == schemas.ArchiveResult.model_fields.keys() assert j["id"] == "lookup-123-456-789" assert j["url"] == "https://example.com" assert j["result"] == {} - assert j["public"] == True - assert j["author_id"] == "rick@example.com" - assert j["group_id"] == None - assert j["tags"] == [] - assert j["updated_at"] == None - assert j["rearchive"] == True def test_delete_task_unauthenticated(client, test_no_auth): From 2a4c6444b0a5e386ff6a599dd83a065b97cc6095 Mon Sep 17 00:00:00 2001 From: msramalho <19508417+msramalho@users.noreply.github.com> Date: Tue, 29 Oct 2024 17:24:03 +0000 Subject: [PATCH 4/7] fixes bad input for limit --- src/db/crud.py | 6 ++++-- src/tests/db/test_crud.py | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/db/crud.py b/src/db/crud.py index c3c069f..4d03318 100644 --- a/src/db/crud.py +++ b/src/db/crud.py @@ -15,6 +15,8 @@ DATABASE_QUERY_LIMIT = get_settings().DATABASE_QUERY_LIMIT # --------------- TASK = Archive +def get_limit(user_limit:int): + return max(1, min(user_limit, DATABASE_QUERY_LIMIT)) def get_archive(db: Session, id: str, email: str): email = email.lower() @@ -40,12 +42,12 @@ def search_archives_by_url(db: Session, url: str, email: str, skip: int = 0, lim query = query.filter(models.Archive.created_at > archived_after) if archived_before: query = query.filter(models.Archive.created_at < archived_before) - return query.order_by(models.Archive.created_at.desc()).offset(skip).limit(min(limit, DATABASE_QUERY_LIMIT)).all() + return query.order_by(models.Archive.created_at.desc()).offset(skip).limit(get_limit(limit)).all() def search_archives_by_email(db: Session, email: str, skip: int = 0, limit: int = 100): email = email.lower() - return base_query(db).filter(models.Archive.author_id == email).order_by(models.Archive.created_at.desc()).offset(skip).limit(min(limit, DATABASE_QUERY_LIMIT)).all() + return base_query(db).filter(models.Archive.author_id == email).order_by(models.Archive.created_at.desc()).offset(skip).limit(get_limit(limit)).all() def create_task(db: Session, task: schemas.ArchiveCreate, tags: list[models.Tag], urls: list[models.ArchiveUrl]): diff --git a/src/tests/db/test_crud.py b/src/tests/db/test_crud.py index 3913548..f293cf8 100644 --- a/src/tests/db/test_crud.py +++ b/src/tests/db/test_crud.py @@ -127,6 +127,7 @@ def test_search_archives_by_url(test_data, db_session): # limit assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, limit=10)) == 10 + assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, limit=-1)) == 1 # skip assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, skip=10)) == 90 From 00e9df0ac342842d2b109499172baf714207ad91 Mon Sep 17 00:00:00 2001 From: msramalho <19508417+msramalho@users.noreply.github.com> Date: Wed, 30 Oct 2024 15:39:06 +0000 Subject: [PATCH 5/7] adds custom encoder --- src/endpoints/task.py | 3 ++- src/utils/mics.py | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 src/utils/mics.py diff --git a/src/endpoints/task.py b/src/endpoints/task.py index 0887183..f446d12 100644 --- a/src/endpoints/task.py +++ b/src/endpoints/task.py @@ -9,6 +9,7 @@ from web.security import get_token_or_user_auth from db import schemas from core.logging import log_error from worker.main import celery +from utils.mics import custom_jsonable_encoder task_router = APIRouter(prefix="/task", tags=["Async task operations"]) @@ -30,7 +31,7 @@ def get_status(task_id, email=Depends(get_token_or_user_auth)) -> schemas.TaskRe "status": task.status, "result": task.result } - return JSONResponse(jsonable_encoder(response, exclude_unset=True)) + return JSONResponse(jsonable_encoder(response, exclude_unset=True, custom_encoder={bytes: custom_jsonable_encoder})) except Exception as e: log_error(e) diff --git a/src/utils/mics.py b/src/utils/mics.py new file mode 100644 index 0000000..f3a9803 --- /dev/null +++ b/src/utils/mics.py @@ -0,0 +1,7 @@ +import base64 +from fastapi.encoders import jsonable_encoder + +def custom_jsonable_encoder(obj): + if isinstance(obj, bytes): + return base64.b64encode(obj).decode('utf-8') + return jsonable_encoder(obj) \ No newline at end of file From c8e1d441a48e56c70413ebb4cdc9d1a83d4dc523 Mon Sep 17 00:00:00 2001 From: msramalho <19508417+msramalho@users.noreply.github.com> Date: Thu, 31 Oct 2024 10:33:34 +0000 Subject: [PATCH 6/7] fixes uvicorn factory warning --- docker-compose.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker-compose.yml b/docker-compose.yml index 8764f02..1ffd164 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -18,7 +18,7 @@ services: <<: *base-setup ports: - "127.0.0.1:8004:8000" - command: uvicorn web:app --host 0.0.0.0 --reload + command: uvicorn web:app --factory --host 0.0.0.0 --reload volumes: - ./src:/app depends_on: From 10306eba8a17113566588306121909ed63b1cf7e Mon Sep 17 00:00:00 2001 From: msramalho <19508417+msramalho@users.noreply.github.com> Date: Thu, 31 Oct 2024 14:52:14 +0000 Subject: [PATCH 7/7] refactors user-groups setup and auth logic --- README.md | 5 +- src/.env.alembic | 5 + src/core/config.py | 2 +- src/core/events.py | 8 +- src/db/crud.py | 144 ++++++++++++------ src/db/models.py | 5 + ...a012ec405b8_add_columns_to_groups_table.py | 42 +++++ src/shared/settings.py | 1 + src/tests/db/test_crud.py | 30 +++- src/tests/user-groups.test.yaml | 36 ++++- 10 files changed, 216 insertions(+), 62 deletions(-) create mode 100644 src/.env.alembic create mode 100644 src/migrations/versions/fa012ec405b8_add_columns_to_groups_table.py diff --git a/README.md b/README.md index 102fd2d..5cd1ee6 100644 --- a/README.md +++ b/README.md @@ -46,8 +46,9 @@ orchestrators: check https://alembic.sqlalchemy.org/en/latest/tutorial.html#the-migration-environment * create migrations with `alembic revision -m "create account table"` -* migrate to most recent with `alembic upgrade head` -* downgrade with `alembic downgrade -1` +* if running in the normal pipenv environment use `PIPENV_DOTENV_LOCATION=.env.alembic pipenv run` followed by: + * migrate to most recent with `alembic upgrade head` + * downgrade with `alembic downgrade -1` ## Release Update `main.py:VERSION`. diff --git a/src/.env.alembic b/src/.env.alembic new file mode 100644 index 0000000..a53af2a --- /dev/null +++ b/src/.env.alembic @@ -0,0 +1,5 @@ +CHROME_APP_IDS='["1234567890"]' +ALLOWED_ORIGINS='["allowed"]' +BLOCKED_EMAILS='[]' +DATABASE_PATH="sqlite:///./auto-archiver.db" +API_BEARER_TOKEN=THIS_API_TOKEN_SHOULD_NEVER_BE_USED \ No newline at end of file diff --git a/src/core/config.py b/src/core/config.py index d3cba7a..dcfd135 100644 --- a/src/core/config.py +++ b/src/core/config.py @@ -1,4 +1,4 @@ -VERSION = "0.7.2" +VERSION = "0.8.0" API_DESCRIPTION = """ #### API for the Auto-Archiver project, a tool to archive web pages and Google Sheets. diff --git a/src/core/events.py b/src/core/events.py index c5cde2c..e9bbfbc 100644 --- a/src/core/events.py +++ b/src/core/events.py @@ -23,8 +23,9 @@ async def lifespan(app: FastAPI): # disabling uvicorn logger since we use loguru in logging_middleware logging.getLogger("uvicorn.access").disabled = True asyncio.create_task(redis_subscribe_worker_exceptions(get_settings().REDIS_EXCEPTIONS_CHANNEL, get_settings().CELERY_BROKER_URL)) - asyncio.create_task(refresh_user_groups()) asyncio.create_task(repeat_measure_regular_metrics()) + with get_db() as db: + crud.upsert_user_groups(db) yield # separates startup from shutdown instructions @@ -34,11 +35,6 @@ async def lifespan(app: FastAPI): # CRON JOBS -@repeat_every(seconds=60 * 60) # 1 hour -async def refresh_user_groups(): - with get_db() as db: - crud.upsert_user_groups(db) - @repeat_every(seconds=get_settings().REPEAT_COUNT_METRICS_SECONDS) async def repeat_measure_regular_metrics(): diff --git a/src/db/crud.py b/src/db/crud.py index 4d03318..7f9c67f 100644 --- a/src/db/crud.py +++ b/src/db/crud.py @@ -1,3 +1,4 @@ +from collections import defaultdict from functools import cache from sqlalchemy.orm import Session, load_only from sqlalchemy import Column, or_, func @@ -9,15 +10,15 @@ from shared.settings import get_settings from . import models, schemas import yaml -DOMAIN_GROUPS = {} -DOMAIN_GROUPS_LOADED = False DATABASE_QUERY_LIMIT = get_settings().DATABASE_QUERY_LIMIT # --------------- TASK = Archive -def get_limit(user_limit:int): + +def get_limit(user_limit: int): return max(1, min(user_limit, DATABASE_QUERY_LIMIT)) + def get_archive(db: Session, id: str, email: str): email = email.lower() query = base_query(db).filter(models.Archive.id == id) @@ -76,9 +77,11 @@ def count_archives(db: Session): def count_archive_urls(db: Session): return db.query(func.count(models.ArchiveUrl.url)).scalar() + def count_users(db: Session): return db.query(func.count(models.User.email)).scalar() + def count_by_user_since(db: Session, seconds_delta: int = 15): time_threshold = datetime.now() - timedelta(seconds=seconds_delta) return db.query(models.Archive.author_id, func.count().label('total'))\ @@ -89,7 +92,7 @@ def count_by_user_since(db: Session, seconds_delta: int = 15): def base_query(db: Session): - #NOTE: load_only is for optimization and not obfuscation, use .with_entities() if needed + # NOTE: load_only is for optimization and not obfuscation, use .with_entities() if needed return db.query(models.Archive)\ .filter(models.Archive.deleted == False)\ .options(load_only(models.Archive.id, models.Archive.created_at, models.Archive.url, models.Archive.result)) @@ -106,14 +109,17 @@ def create_tag(db: Session, tag: str): db.refresh(db_tag) return db_tag + def is_active_user(db: Session, email: str) -> bool: email = email.lower() - if "@" not in email: return False - global DOMAIN_GROUPS, DOMAIN_GROUPS_LOADED - if not DOMAIN_GROUPS_LOADED: upsert_user_groups(db) + if not email or not len(email) or "@" not in email: return False domain = email.split('@')[1] - if domain in DOMAIN_GROUPS: return True - return len(email) and db.query(models.User).filter(models.User.email == email, models.User.is_active == True).first() is not None + + explicitly_active = db.query(models.User).filter(models.User.email == email, models.User.is_active == True).first() is not None + if explicitly_active: return True + + return db.query(models.Group).filter(models.Group.domains.contains(domain)).first() is not None + def is_user_in_group(db: Session, group_name: str, email: str) -> models.Group: if email == ALLOW_ANY_EMAIL: return True @@ -121,16 +127,23 @@ def is_user_in_group(db: Session, group_name: str, email: str) -> models.Group: def get_user_groups(db: Session, email: str): + """ + given an email retrieves the user groups from the DB and then the email-domain groups from a global variable, the email does not need to belong to an existing user. User does not need to be active. + """ + if not email or not len(email) or "@" not in email: return [] email = email.lower() - if "@" not in email: return [] - global DOMAIN_GROUPS, DOMAIN_GROUPS_LOADED - if not DOMAIN_GROUPS_LOADED: upsert_user_groups(db) - # given an email retrieves the user groups from the DB and then the email-domain groups from a global variable - groups = db.query(models.association_table_user_groups).filter_by(user_id=email).with_entities(Column("group_id")).all() - user_level_groups = [g[0] for g in groups] - domain_level_groups = DOMAIN_GROUPS.get(email.split('@')[1], []) - logger.success(f"EMAIL {email} has {user_level_groups=} and {domain_level_groups=}") - return list(set(user_level_groups) | set(domain_level_groups)) + + # get user groups + user_groups = db.query(models.association_table_user_groups).filter_by(user_id=email).with_entities(Column("group_id")).all() + user_level_groups = [g[0] for g in user_groups] + + # get domain groups + domain = email.split('@')[1] + domain_level_groups = db.query(models.Group.id).filter(models.Group.domains.contains(domain)).with_entities(Column("id")).all() + domain_level_groups = [g[0] for g in domain_level_groups] + + # combine and return + return list(set(user_level_groups + domain_level_groups)) # --------------- INIT User-Groups @@ -147,19 +160,36 @@ def create_or_get_user(db: Session, author_id: str, is_active: bool = models.Use return db_user -@cache -def create_or_get_group(db: Session, group_name: str) -> models.Group: +def upsert_group(db: Session, group_name: str, description: str, orchestrator: str, orchestrator_sheet: str, permissions: dict, domains: list) -> models.Group: db_group = db.query(models.Group).filter(models.Group.id == group_name).first() if db_group is None: - db_group = models.Group(id=group_name) + db_group = models.Group(id=group_name, description=description, orchestrator=orchestrator, orchestrator_sheet=orchestrator_sheet, permissions=permissions, domains=domains) db.add(db_group) - db.commit() - db.refresh(db_group) + else: + db_group.description = description + db_group.orchestrator = orchestrator + db_group.orchestrator_sheet = orchestrator_sheet + db_group.permissions = permissions + db_group.domains = domains + db.commit() + db.refresh(db_group) return db_group +def upsert_user(db: Session, email: str, active: bool): + db_user = db.query(models.User).filter(models.User.email == email).first() + if db_user is None: + db_user = models.User(email=email, is_active=active) + db.add(db_user) + else: + db_user.is_active = active + db.commit() + return db_user + + def upsert_user_groups(db: Session): - global DOMAIN_GROUPS, DOMAIN_GROUPS_LOADED + def display_email_pii(email: str): + return f"'{email[0:3]}...@{email.split('@')[1]}'" """ reads the user_groups yaml file and inserts any new users, groups, along with new participation of users in groups @@ -174,32 +204,56 @@ def upsert_user_groups(db: Session): except Exception as e: logger.error(f"could not open user groups filename {filename}: {e}") raise e - # updating domain->groups access - DOMAIN_GROUPS = user_groups_yaml.get("domains", {}) - # upserting in DB - user_groups = user_groups_yaml.get("users", {}) - logger.debug(f"Found {len(user_groups)} users.") + # delete all user-groups relationships db.query(models.association_table_user_groups).delete() # set all users to inactive db.query(models.User).update({models.User.is_active: False}) - for user_email, groups in user_groups.items(): - user_email = user_email.lower() - assert '@' in user_email, f'Invalid user email {user_email}' - logger.info(f"email='{user_email[0:3]}...{user_email[-8:]}', {groups=}") - db_user = db.query(models.User).filter(models.User.email == user_email).first() - if db_user is None: - db_user = models.User(email=user_email, is_active=True) - db.add(db_user) - else: - db_user.is_active = True - if not groups: continue # avoid hanging in for x in None: - for group in groups: - db_group = create_or_get_group(db, group) - db_group.users.append(db_user) + + # create a map of group_id -> domains and another of domain -> groups + group_domains = defaultdict(set) + domain_groups = defaultdict(list) + for domain, explicit_groups in user_groups_yaml.get("domains", {}).items(): + domain_groups[domain] = list(set(explicit_groups)) + for group in explicit_groups: + group_domains[group].add(domain) + + # upsert groups and save a map of groupid -> dbobject + for group_id, g in user_groups_yaml.get("groups", {}).items(): + upsert_group(db, group_id, g["description"], g["orchestrator"], g["orchestrator_sheet"], g["permissions"], list(group_domains.get(group_id, []))) + db_groups: dict[str, models.Group] = {g.id: g for g in db.query(models.Group).all()} + + # integrity checks + for group_in_domains in group_domains: + if group_in_domains not in db_groups: + logger.error(f"[CONFIG] Group '{group_in_domains}' does not exist in the database: domains setting will not work.") + if group_in_domains not in user_groups_yaml.get("groups", {}): + logger.error(f"[CONFIG] Group '{group_in_domains}' does not exist in the config file: domains setting will not work.") + + # reinsert users in their EXPLICITLY DEFINED groups + # domain groups are check live, as there may be new users that are not explicitly registered but belong to a domain + for email, explicit_groups in user_groups_yaml.get("users", {}).items(): + explicit_groups = explicit_groups or [] + email = email.lower().strip() + if '@' not in email: + logger.error(f'[CONFIG] Invalid user email {email}, skipping.') + continue + + logger.info(f"{display_email_pii(email)} => {explicit_groups}") + + # upsert active user + db_user = upsert_user(db, email, active=True) + + # connect users to groups + for group_id in explicit_groups: + if group_id not in db_groups: + logger.error(f"[CONFIG] Group {group_id} does not exist in config file, skipping for email={display_email_pii(email)}.") + continue + db_groups[group_id].users.append(db_user) db.commit() count_user_groups = db.query(models.association_table_user_groups).count() - logger.success(f"Completed refresh, now: {count_user_groups} user-groups relationships.") - DOMAIN_GROUPS_LOADED = True + count_groups = db.query(func.count(models.Group.id)).scalar() + + logger.success(f"[CONFIG] DONE: [users={count_users(db)}, groups={count_groups}, explicit user groups={count_user_groups}].") diff --git a/src/db/models.py b/src/db/models.py index 2718687..193adba 100644 --- a/src/db/models.py +++ b/src/db/models.py @@ -74,6 +74,11 @@ class Group(Base): __tablename__ = "groups" id = Column(String, primary_key=True, index=True) + description = Column(String, default=None) + orchestrator = Column(String, default=None) + orchestrator_sheet = Column(String, default=None) + permissions = Column(JSON, default=None) + domains = Column(JSON, default=[]) archives = relationship("Archive", back_populates="group") users = relationship("User", back_populates="groups", secondary=association_table_user_groups) \ No newline at end of file diff --git a/src/migrations/versions/fa012ec405b8_add_columns_to_groups_table.py b/src/migrations/versions/fa012ec405b8_add_columns_to_groups_table.py new file mode 100644 index 0000000..da77d41 --- /dev/null +++ b/src/migrations/versions/fa012ec405b8_add_columns_to_groups_table.py @@ -0,0 +1,42 @@ +"""add columns to groups table + +Revision ID: fa012ec405b8 +Revises: 93a611e4c066 +Create Date: 2024-10-31 09:36:50.360710 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.engine.reflection import Inspector + + +# revision identifiers, used by Alembic. +revision = 'fa012ec405b8' +down_revision = '93a611e4c066' +branch_labels = None +depends_on = None + + +def upgrade() -> None: + conn = op.get_bind() + inspector = Inspector.from_engine(conn) + columns = [col['name'] for col in inspector.get_columns('groups')] + + if 'description' not in columns: + op.add_column('groups', sa.Column('description', sa.String(), nullable=True)) + if 'orchestrator' not in columns: + op.add_column('groups', sa.Column('orchestrator', sa.String(), nullable=True)) + if 'orchestrator_sheet' not in columns: + op.add_column('groups', sa.Column('orchestrator_sheet', sa.String(), nullable=True)) + if 'permissions' not in columns: + op.add_column('groups', sa.Column('permissions', sa.JSON(), nullable=True)) + if 'domains' not in columns: + op.add_column('groups', sa.Column('domains', sa.JSON(), nullable=True)) + + +def downgrade() -> None: + op.drop_column('groups', 'description') + op.drop_column('groups', 'orchestrator') + op.drop_column('groups', 'orchestrator_sheet') + op.drop_column('groups', 'permissions') + op.drop_column('groups', 'domains') diff --git a/src/shared/settings.py b/src/shared/settings.py index 56b9557..8fa5ae5 100644 --- a/src/shared/settings.py +++ b/src/shared/settings.py @@ -30,6 +30,7 @@ class Settings(BaseSettings): API_BEARER_TOKEN: Annotated[str, Len(min_length=20)] ALLOWED_ORIGINS: Annotated[set[str], Len(min_length=1)] CHROME_APP_IDS: Annotated[set[Annotated[str, Len(min_length=10)]], Len(min_length=1)] + #TODO: deprecate blocklist BLOCKED_EMAILS: Annotated[Set[str], Len(min_length=0)] = set() @lru_cache diff --git a/src/tests/db/test_crud.py b/src/tests/db/test_crud.py index f293cf8..6f838df 100644 --- a/src/tests/db/test_crud.py +++ b/src/tests/db/test_crud.py @@ -300,8 +300,11 @@ def test_is_active_user(test_data, db_session): assert crud.is_active_user(db_session, "") == False assert crud.is_active_user(db_session, "example.com") == False assert crud.is_active_user(db_session, "unknown@example.com") == True + assert crud.is_active_user(db_session, "ANYONE@example.com") == True + assert crud.is_active_user(db_session, "ANYONE@birdy.com") == True assert crud.is_active_user(db_session, "rick@example.com") == True assert crud.is_active_user(db_session, "RICK@example.com") == True + assert crud.is_active_user(db_session, "summer@herself.com") == True assert crud.is_active_user(db_session, "rick@not-in-groups.com") == False @@ -317,6 +320,7 @@ def test_is_user_in_group(test_data, db_session): ("rick@example.com", "spaceship", True), ("rick@example.com", "SPACESHIP", False), ("RICK@example.com", "interdimensional", True), + ("rick@example.com", "animated-characters", True), ("rick@example.com", "the-jerrys-club", False), ("morty@example.com", "spaceship", True), @@ -325,17 +329,22 @@ def test_is_user_in_group(test_data, db_session): ("jerry@example.com", "spaceship", False), ("jerry@example.com", "interdimensional", False), - ("jerry@example.com", "the-jerrys-club", True), + ("jerry@example.com", "the-jerrys-club", False), # group not in 'groups' ("rick@example.com", "animated-characters", True), ("morty@example.com", "animated-characters", True), ("jerry@example.com", "animated-characters", True), + ("ANYONE@example.com", "animated-characters", True), + ("ANYONE@birdy.com", "animated-characters", True), + + ("summer@herself.com", "animated-characters", False), ("rick@example.com", "", False), ("", "spaceship", False), ("BADEMAILexample.com", "spaceship", False), ] for email, group, expected in test_pairs: + print(f"{email} in {group} == {expected}") assert crud.is_user_in_group(db_session, group, email) == expected @@ -362,22 +371,29 @@ def test_create_or_get_user(test_data, db_session): assert db_session.query(models.User).count() == 6 -def test_get_group(test_data, db_session): +def test_upsert_group(test_data, db_session): from db import crud assert db_session.query(models.Group).count() == 3 - assert (g1 := crud.create_or_get_group(db_session, "spaceship")) is not None + repeatable_params = ["desc 1", "orch.yaml", "sheet.yaml", {"read": ["all"]}, ["example.com"]] + + assert (g1 := crud.upsert_group(db_session, "spaceship", *repeatable_params)) is not None assert g1.id == "spaceship" + assert g1.description == "desc 1" + assert g1.orchestrator == "orch.yaml" + assert g1.orchestrator_sheet == "sheet.yaml" + assert g1.permissions == {"read": ["all"]} + assert g1.domains == ["example.com"] assert len(g1.users) == 2 assert [u.email for u in g1.users] == ["rick@example.com", "morty@example.com"] - assert (g2 := crud.create_or_get_group(db_session, "the-jerrys-club")) is not None - assert g2.id == "the-jerrys-club" + assert (g2 := crud.upsert_group(db_session, "interdimensional", *repeatable_params)) is not None + assert g2.id == "interdimensional" assert len(g2.users) == 1 - assert g2.users[0].email == "jerry@example.com" + assert [u.email for u in g2.users] == ["rick@example.com"] - assert (g3 := crud.create_or_get_group(db_session, "this-is-a-new-group")) is not None + assert (g3 := crud.upsert_group(db_session, "this-is-a-new-group", *repeatable_params)) is not None assert g3.id == "this-is-a-new-group" assert len(g3.users) == 0 diff --git a/src/tests/user-groups.test.yaml b/src/tests/user-groups.test.yaml index 545b38c..612f09d 100644 --- a/src/tests/user-groups.test.yaml +++ b/src/tests/user-groups.test.yaml @@ -7,13 +7,47 @@ users: - spaceship jerry@example.com: - the-jerrys-club - birdman@example.com: + summer@herself.com: + badyemail.com: domains: example.com: - animated-characters + birdy.com: + - animated-characters + - this-does-not-exist + orchestrators: spaceship: tests/orchestration.test.yaml interdimensional: tests/orchestration.test.yaml default: tests/orchestration.test.yaml + +groups: + spaceship: + description: "The spaceship crew" + orchestrator: tests/orchestration.test.yaml + orchestrator_sheet: tests/orchestration.test.yaml + permissions: + read: ["all"] + active_sheets: -1 + monthly_urls: all + monthly_mbs: all + interdimensional: + description: "Interdimensional travelers" + orchestrator: tests/orchestration.test.yaml + orchestrator_sheet: tests/orchestration.test.yaml + permissions: + read: ["interdimensional", "animated-characters"] + active_sheets: 5 + monthly_urls: 1000 + monthly_mbs: 1000 + animated-characters: + description: "Animated characters" + orchestrator: tests/orchestration.test.yaml + orchestrator_sheet: tests/orchestration.test.yaml + permissions: + read: ["animated-characters"] + active_sheets: -1 + monthly_urls: all + monthly_mbs: all \ No newline at end of file