feat: Adds a safeguard check of permissions for modifications of repos/guidelines

README.md

@@ -85,6 +85,7 @@ This file will have to hold the following information:
 - `SUPERUSER_PWD`: the password of the initial admin access
 - `GH_OAUTH_ID`: the ID of the GitHub Oauth app
 - `GH_OAUTH_SECRET`: the secret of the GitHub Oauth app
+- `OPENAI_API_KEY`: your API key for Open AI
 
 Optionally, the following information can be added:
 - `SENTRY_DSN`: the URL of the [Sentry](https://sentry.io/) project, which monitors back-end errors and report them back.
@@ -102,6 +103,7 @@ SENTRY_DSN='https://replace.with.you.sentry.dsn/'
 SENTRY_SERVER_NAME=my_storage_bucket_name
 GH_OAUTH_ID=your_github_oauth_app_id
 GH_OAUTH_SECRET=your_github_oauth_app_secret
+OPENAI_API_KEY='you-openai-key'
 ```
 
 The file should be placed at the root folder of your local copy of the project.

scripts/test_e2e.py

@@ -51,13 +51,18 @@ def main(args):
         "Content-Type": "application/json",
     }
 
-    user_login = "my_user"
-    user_pwd = "my_pwd"  # nosec B105  # noqa S105
+    # Hardcoded info
+    user_login = "karpathy"
+    user_id = 241138
+    user_pwd = "my_pwd"  # noqa S105
+    repo_id = 582822129
+    # repo_name = "karpathy/nanoGPT"
 
     # create an access
-    payload = {"id": 1, "login": user_login, "password": user_pwd, "scope": "user"}
-    user_id = api_request("post", f"{args.endpoint}/users/", superuser_auth, payload)["id"]
-    {
+    api_request(
+        "post", f"{args.endpoint}/users/", superuser_auth, {"id": user_id, "password": user_pwd, "scope": "user"}
+    )
+    user_auth = {
         "Authorization": f"Bearer {get_token(args.endpoint, user_login, user_pwd)}",
         "Content-Type": "application/json",
     }
@@ -69,12 +74,11 @@ def main(args):
     api_request("put", f"{args.endpoint}/users/{user_id}/", superuser_auth, {"password": new_pwd})
 
     # Repos
-    payload = {"id": 1, "owner_id": user_id, "full_name": "frgfm/torch-cam"}
-    repo_id = api_request("post", f"{args.endpoint}/repos/", superuser_auth, payload)["id"]
-    api_request("get", f"{args.endpoint}/repos/{repo_id}/", superuser_auth)
-    api_request("get", f"{args.endpoint}/repos/", superuser_auth)
-    api_request("put", f"{args.endpoint}/repos/{repo_id}/disable", superuser_auth)
-    api_request("put", f"{args.endpoint}/repos/{repo_id}/enable", superuser_auth)
+    api_request("post", f"{args.endpoint}/repos/", user_auth, {"id": repo_id})
+    api_request("get", f"{args.endpoint}/repos/{repo_id}/", user_auth)
+    api_request("get", f"{args.endpoint}/repos/", user_auth)
+    api_request("put", f"{args.endpoint}/repos/{repo_id}/disable", user_auth, {})
+    api_request("put", f"{args.endpoint}/repos/{repo_id}/enable", user_auth, {})
 
     # Guidelines
     payload = {
@@ -83,41 +87,41 @@ def main(args):
         "repo_id": repo_id,
         "order": 0,
     }
-    guideline_id = api_request("post", f"{args.endpoint}/guidelines/", superuser_auth, payload)["id"]
+    guideline_id = api_request("post", f"{args.endpoint}/guidelines/", user_auth, payload)["id"]
     payload = {
         "title": "My second guideline",
         "details": "Always document public interface",
         "repo_id": repo_id,
         "order": 1,
     }
-    guideline_2 = api_request("post", f"{args.endpoint}/guidelines/", superuser_auth, payload)["id"]
-    guideline = api_request("get", f"{args.endpoint}/guidelines/{guideline_id}/", superuser_auth)
+    guideline_2 = api_request("post", f"{args.endpoint}/guidelines/", user_auth, payload)["id"]
+    guideline = api_request("get", f"{args.endpoint}/guidelines/{guideline_id}/", user_auth)
     api_request("get", f"{args.endpoint}/guidelines/", superuser_auth)
-    api_request("get", f"{args.endpoint}/repos/{repo_id}/guidelines", superuser_auth)
+    api_request("get", f"{args.endpoint}/repos/{repo_id}/guidelines", user_auth)
     api_request(
         "put",
         f"{args.endpoint}/guidelines/{guideline_id}",
-        superuser_auth,
+        user_auth,
         {"title": "Updated title", "details": "Updated details"},
     )
     api_request(
         "put",
-        f"{args.endpoint}/guidelines/{guideline_id}/order/1",
-        superuser_auth,
+        f"{args.endpoint}/guidelines/{guideline_2}/order/2",
+        user_auth,
         {},
     )
     api_request(
         "put",
         f"{args.endpoint}/repos/{repo_id}/guidelines/order",
-        superuser_auth,
+        user_auth,
         {"guideline_ids": [guideline_2, guideline_id]},
     )
 
     # Delete
     for guideline in api_request("get", f"{args.endpoint}/guidelines/", superuser_auth):
-        api_request("delete", f"{args.endpoint}/guidelines/{guideline['id']}/", superuser_auth)
+        api_request("delete", f"{args.endpoint}/guidelines/{guideline['id']}/", user_auth, {})
     for repo in api_request("get", f"{args.endpoint}/repos/", superuser_auth):
-        api_request("delete", f"{args.endpoint}/repos/{repo['id']}/", superuser_auth)
+        api_request("delete", f"{args.endpoint}/repos/{repo['id']}/", superuser_auth, {})
     api_request("delete", f"{args.endpoint}/users/{user_id}/", superuser_auth)
 
     print(f"SUCCESS in {time.time() - start_ts:.3}s")

src/app/api/api_v1/endpoints/guidelines.py

@@ -8,10 +8,12 @@
 
 from fastapi import APIRouter, Depends, Path, Security, status
 
-from app.api.dependencies import get_current_user, get_guideline_crud
-from app.crud import GuidelineCRUD
-from app.models import Guideline, UserScope
-from app.schemas.guidelines import ContentUpdate, GuidelineCreate, GuidelineEdit, OrderUpdate
+from app.api.dependencies import get_current_user, get_guideline_crud, get_repo_crud
+from app.crud import GuidelineCRUD, RepositoryCRUD
+from app.models import Guideline, Repository, UserScope
+from app.schemas.base import OptionalGHToken
+from app.schemas.guidelines import ContentUpdate, GuidelineCreate, GuidelineCreation, GuidelineEdit, OrderUpdate
+from app.services.github import gh_client
 from app.services.telemetry import telemetry_client
 
 router = APIRouter()
@@ -21,10 +23,14 @@
 async def create_guideline(
     payload: GuidelineCreate,
     guidelines: GuidelineCRUD = Depends(get_guideline_crud),
+    repos: RepositoryCRUD = Depends(get_repo_crud),
     user=Security(get_current_user, scopes=[UserScope.USER, UserScope.ADMIN]),
 ) -> Guideline:
     telemetry_client.capture(user.id, event="guideline-creation", properties={"repo_id": payload.repo_id})
-    return await guidelines.create(payload)
+    # Check if user is allowed
+    repo = cast(Repository, await repos.get(payload.repo_id, strict=True))
+    gh_client.check_user_permission(user, repo.full_name, repo.owner_id, payload.github_token)
+    return await guidelines.create(GuidelineCreation(**payload.dict()))
 
 
 @router.get("/{guideline_id}", status_code=status.HTTP_200_OK)
@@ -52,31 +58,45 @@
     payload: GuidelineEdit,
     guideline_id: int = Path(..., gt=0),
     guidelines: GuidelineCRUD = Depends(get_guideline_crud),
+    repos: RepositoryCRUD = Depends(get_repo_crud),
     user=Security(get_current_user, scopes=[UserScope.USER, UserScope.ADMIN]),
 ) -> Guideline:
     guideline = await guidelines.update(guideline_id, ContentUpdate(**payload.dict(), updated_at=datetime.utcnow()))
     telemetry_client.capture(user.id, event="guideline-content", properties={"repo_id": guideline.repo_id})
+    # Check if user is allowed
+    repo = cast(Repository, await repos.get(guideline.repo_id, strict=True))
+    gh_client.check_user_permission(user, repo.full_name, repo.owner_id, payload.github_token)
     return guideline
 
 
 @router.put("/{guideline_id}/order/{order_idx}", status_code=status.HTTP_200_OK)
 async def update_guideline_order(
+    payload: OptionalGHToken,
     guideline_id: int = Path(..., gt=0),
     order_idx: int = Path(..., gte=0),
     guidelines: GuidelineCRUD = Depends(get_guideline_crud),
+    repos: RepositoryCRUD = Depends(get_repo_crud),
     user=Security(get_current_user, scopes=[UserScope.USER, UserScope.ADMIN]),
 ) -> Guideline:
     guideline = await guidelines.update(guideline_id, OrderUpdate(order=order_idx, updated_at=datetime.utcnow()))
     telemetry_client.capture(user.id, event="guideline-order", properties={"repo_id": guideline.repo_id})
+    # Check if user is allowed
+    repo = cast(Repository, await repos.get(guideline.repo_id, strict=True))
+    gh_client.check_user_permission(user, repo.full_name, repo.owner_id, payload.github_token)
     return guideline
 
 
 @router.delete("/{guideline_id}", status_code=status.HTTP_200_OK)
 async def delete_guideline(
+    payload: OptionalGHToken,
     guideline_id: int = Path(..., gt=0),
     guidelines: GuidelineCRUD = Depends(get_guideline_crud),
+    repos: RepositoryCRUD = Depends(get_repo_crud),
     user=Security(get_current_user, scopes=[UserScope.USER, UserScope.ADMIN]),
 ) -> None:
     guideline = cast(Guideline, await guidelines.get(guideline_id, strict=True))
     telemetry_client.capture(user.id, event="guideline-deletion", properties={"repo_id": guideline.repo_id})
+    # Check if user is allowed
+    repo = cast(Repository, await repos.get(guideline.repo_id, strict=True))
+    gh_client.check_user_permission(user, repo.full_name, repo.owner_id, payload.github_token)
     await guidelines.delete(guideline_id)

src/app/api/api_v1/endpoints/login.py

@@ -87,6 +87,9 @@
             "twitter_username": gh_user["twitter_username"],
         },
     )
+    # Check that GH account is a user
+    if gh_user["type"] != "User":
+        raise HTTPException(status.HTTP_401_UNAUTHORIZED, "GitHub account is expected to be a user")
     # Verify credentials
     user = await users.get(gh_user["id"], strict=False)
     # Register if non existing

src/app/api/api_v1/endpoints/repos.py

@@ -11,6 +11,7 @@
 from app.api.dependencies import get_current_user, get_guideline_crud, get_repo_crud
 from app.crud import GuidelineCRUD, RepositoryCRUD
 from app.models import Guideline, Repository, User, UserScope
+from app.schemas.base import OptionalGHToken
 from app.schemas.guidelines import OrderUpdate
 from app.schemas.repos import GuidelineOrder, RepoCreate, RepoCreation, RepoUpdate
 from app.services.github import gh_client
@@ -31,7 +32,7 @@
         user.id, event="repo-creation", properties={"repo_id": payload.id, "full_name": gh_repo["full_name"]}
     )
     # Check if user is allowed
-    # gh_client.has_valid_permission(gh_repo["full_name"], user.login, payload.gh_token)
+    gh_client.check_user_permission(user, gh_repo["full_name"], gh_repo["owner"]["id"], payload.github_token)
     return await repos.create(
         RepoCreation(
             id=payload.id, full_name=gh_repo["full_name"], owner_id=gh_repo["owner"]["id"], installed_by=user.id
@@ -72,13 +73,15 @@
     if len(payload.guideline_ids) != len(set(payload.guideline_ids)):
         raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail="Duplicate IDs were passed.")
     # Check the repo
-    await repos.get(repo_id, strict=True)
+    repo = cast(Repository, await repos.get(repo_id, strict=True))
     # Ensure all IDs are valid
     guideline_ids = [elt.id for elt in await guidelines.fetch_all(("repo_id", repo_id))]
     if set(payload.guideline_ids) != set(guideline_ids):
         raise HTTPException(
             status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail="Guideline IDs for that repo don't match."
         )
+    # Check if user is allowed
+    gh_client.check_user_permission(user, repo.full_name, repo.owner_id, payload.github_token)
     # Update all order
     return [
         await guidelines.update(guideline_id, OrderUpdate(order=order_idx, updated_at=datetime.utcnow()))
@@ -88,21 +91,29 @@
 
 @router.put("/{repo_id}/disable", status_code=status.HTTP_200_OK)
 async def disable_repo(
+    payload: OptionalGHToken,
     repo_id: int = Path(..., gt=0),
     repos: RepositoryCRUD = Depends(get_repo_crud),
     user=Security(get_current_user, scopes=[UserScope.USER, UserScope.ADMIN]),
 ) -> Repository:
     telemetry_client.capture(user.id, event="repo-disable", properties={"repo_id": repo_id})
+    # Check if user is allowed
+    repo = cast(Repository, await repos.get(repo_id, strict=True))
+    gh_client.check_user_permission(user, repo.full_name, repo.owner_id, payload.github_token)
     return await repos.update(repo_id, RepoUpdate(is_active=False))
 
 
 @router.put("/{repo_id}/enable", status_code=status.HTTP_200_OK)
 async def enable_repo(
+    payload: OptionalGHToken,
     repo_id: int = Path(..., gt=0),
     repos: RepositoryCRUD = Depends(get_repo_crud),
     user=Security(get_current_user, scopes=[UserScope.USER, UserScope.ADMIN]),
 ) -> Repository:
     telemetry_client.capture(user.id, event="repo-enable", properties={"repo_id": repo_id})
+    # Check if user is allowed
+    repo = cast(Repository, await repos.get(repo_id, strict=True))
+    gh_client.check_user_permission(user, repo.full_name, repo.owner_id, payload.github_token)
     return await repos.update(repo_id, RepoUpdate(is_active=True))
 
 

src/app/api/api_v1/endpoints/users.py

@@ -5,13 +5,14 @@
 
 from typing import List, cast
 
-from fastapi import APIRouter, Depends, Path, Security, status
+from fastapi import APIRouter, Depends, HTTPException, Path, Security, status
 
 from app.api.dependencies import get_current_user, get_user_crud
 from app.core.security import hash_password
 from app.crud import UserCRUD
 from app.models import User, UserScope
 from app.schemas.users import Cred, CredHash, UserCreate, UserCreation
+from app.services.github import gh_client
 from app.services.telemetry import telemetry_client
 
 router = APIRouter()
@@ -21,16 +22,18 @@
 async def create_user(
     payload: UserCreate,
     users: UserCRUD = Depends(get_user_crud),
-    _=Security(get_current_user, scopes=[UserScope.ADMIN]),
+    user: User = Security(get_current_user, scopes=[UserScope.ADMIN]),
 ) -> User:
-    telemetry_client.capture(payload.id, event="user-creation", properties={"login": payload.login})
+    # Check that user exists on GitHub
+    gh_user = gh_client.get_user(payload.id)
+    if gh_user["type"] != "User":
+        raise HTTPException(status.HTTP_401_UNAUTHORIZED, "GitHub account is expected to be a user")
+    telemetry_client.capture(user.id, event="user-creation", properties={"login": gh_user["login"]})
     # Hash the password
     pwd = await hash_password(payload.password)
-
-    user = await users.create(
-        UserCreation(id=payload.id, login=payload.login, hashed_password=pwd, scope=payload.scope)
+    return await users.create(
+        UserCreation(id=payload.id, login=gh_user["login"], hashed_password=pwd, scope=payload.scope)
     )
-    return user
 
 
 @router.get("/{user_id}", status_code=status.HTTP_200_OK)

src/app/crud/crud_guideline.py

@@ -9,11 +9,11 @@
 
 from app.crud.base import BaseCRUD
 from app.models import Guideline
-from app.schemas.guidelines import ContentUpdate, GuidelineCreate, OrderUpdate
+from app.schemas.guidelines import ContentUpdate, GuidelineCreation, OrderUpdate
 
 __all__ = ["GuidelineCRUD"]
 
 
-class GuidelineCRUD(BaseCRUD[Guideline, GuidelineCreate, Union[ContentUpdate, OrderUpdate]]):
+class GuidelineCRUD(BaseCRUD[Guideline, GuidelineCreation, Union[ContentUpdate, OrderUpdate]]):
     def __init__(self, session: AsyncSession) -> None:
         super().__init__(session, Guideline)

src/app/models.py

@@ -5,8 +5,9 @@
 
 from datetime import datetime
 from enum import Enum
+from typing import List
 
-from sqlmodel import Field, SQLModel
+from sqlmodel import Field, Relationship, SQLModel
 
 __all__ = ["GHRole", "UserScope", "User", "Repository", "Guideline"]
 
@@ -39,6 +40,8 @@ class Repository(SQLModel, table=True):
     is_active: bool = Field(True, nullable=False)
     installed_by: int = Field(..., foreign_key="user.id")
 
+    guidelines: List["Guideline"] = Relationship(sa_relationship_kwargs={"cascade": "delete"})
+
 
 class Guideline(SQLModel, table=True):
     id: int = Field(None, primary_key=True)

src/app/schemas/base.py

@@ -4,6 +4,7 @@
 # Copying and/or distributing is strictly prohibited without the express permission of its copyright owner.
 
 from datetime import datetime
+from typing import Union
 
 from pydantic import BaseModel, Field
 
@@ -15,3 +16,7 @@ class _CreatedAt(BaseModel):
 
 class _Id(BaseModel):
     id: int = Field(..., gt=0)
+
+
+class OptionalGHToken(BaseModel):
+    github_token: Union[str, None] = None

src/app/schemas/guidelines.py

@@ -7,23 +7,37 @@
 
 from pydantic import BaseModel, Field
 
+from .base import OptionalGHToken
+
 __all__ = ["GuidelineCreate", "GuidelineEdit", "ContentUpdate", "OrderUpdate"]
 
 
-class GuidelineEdit(BaseModel):
+class GuidelineContent(BaseModel):
     title: str = Field(..., min_length=6, max_length=100)
     details: str = Field(..., min_length=6, max_length=1000)
 
 
-class GuidelineCreate(GuidelineEdit):
+class GuidelineLocation(BaseModel):
     repo_id: int = Field(..., gt=0)
-    order: int = Field(..., ge=0, nullable=False)
+    order: int = Field(0, ge=0, nullable=False)
+
+
+class GuidelineEdit(GuidelineContent, OptionalGHToken):
+    pass
+
+
+class GuidelineCreate(GuidelineEdit, GuidelineLocation):
+    pass
+
+
+class GuidelineCreation(GuidelineContent, GuidelineLocation):
+    pass
 
 
-class ContentUpdate(GuidelineEdit):
+class ContentUpdate(GuidelineContent):
     updated_at: datetime = Field(default_factory=datetime.utcnow, nullable=False)
 
 
-class OrderUpdate(BaseModel):
+class OrderUpdate(OptionalGHToken):
     order: int = Field(..., ge=0, nullable=False)
     updated_at: datetime = Field(default_factory=datetime.utcnow, nullable=False)