Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CRUD of user groups #1738

Merged
merged 62 commits into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
ae8e63e
Remove useless `db` fixture from test
tcompa Sep 6, 2024
4a43abc
Extract test of MockCurrentUser into new module
tcompa Sep 6, 2024
44ba212
Aff first `test_auth_groups_api` draft
tcompa Sep 6, 2024
b8f7147
Update test_auth_groups_api
tcompa Sep 6, 2024
cd3722d
BROKEN add user groups to test_unit_schemas_no_version
tcompa Sep 6, 2024
08fcba6
BROKEN - Add more user-group tests
tcompa Sep 6, 2024
35caf28
Add new user-group tables and API - first version
tcompa Sep 6, 2024
a02eee2
Update group API and tests
tcompa Sep 6, 2024
8acbf21
Update user endpoints with group logic
tcompa Sep 6, 2024
fdf7d30
Some more fixes to user API
tcompa Sep 6, 2024
7527070
Also collect user groups in `GET /users` endpoint
tcompa Sep 9, 2024
f796b9d
Add on_after_register hook in `on_after_register`
tcompa Sep 9, 2024
fcfd386
fix missing await
mfranzon Sep 9, 2024
5bf80a4
Setup update-db-data script for 2.4.0 (ref #1734)
tcompa Sep 9, 2024
b2583bf
Merge branch '1731-crud-for-user-groups' of github.com:fractal-analyt…
tcompa Sep 9, 2024
949dca2
Manually set version to 2.4.0a0, to test update-db-data script
tcompa Sep 9, 2024
a24e03a
Fix test_unit_schemas_no_version
tcompa Sep 9, 2024
d803c23
move _create_first_user to set_db()
mfranzon Sep 9, 2024
ae620a5
Merge branch '1731-crud-for-user-groups' of github.com:fractal-analyt…
mfranzon Sep 9, 2024
335d49b
Adapt `scripts/oauth/oauth.sh` to new `UserRead` response model
tcompa Sep 9, 2024
99e2512
Merge branch '1731-crud-for-user-groups' of github.com:fractal-analyt…
tcompa Sep 9, 2024
5c5593d
Fix `scripts/oauth/oauth.sh`
tcompa Sep 9, 2024
c68ff10
Add docstrings
tcompa Sep 9, 2024
87a53ed
Update `benchmarks/populate_db/populate_db_script.py`.
tcompa Sep 9, 2024
44d7baa
Fix `test_app_with_lifespan`
tcompa Sep 9, 2024
9f936bb
Fix usergroup/linkusergroup migration script
tcompa Sep 9, 2024
2b2cbee
Revert "Fix usergroup/linkusergroup migration script"
tcompa Sep 9, 2024
cd5363c
add models to init to make them visible from alembic
mfranzon Sep 9, 2024
ef2941c
add linkusergroup
mfranzon Sep 9, 2024
69b39e2
fix import
mfranzon Sep 9, 2024
375f1ac
Merge branch '1731-crud-for-user-groups' of github.com:fractal-analyt…
mfranzon Sep 9, 2024
779dbc9
Add `.unique` for query statement
tcompa Sep 9, 2024
af6f65f
Merge branch '1731-crud-for-user-groups' of github.com:fractal-analyt…
tcompa Sep 9, 2024
1a51a22
Fix `test_edit_users_as_superuser` (ref #1737)
tcompa Sep 9, 2024
a9b2b94
add migration usergroup and linkusergroup
mfranzon Sep 9, 2024
a4ebc69
Merge branch '1731-crud-for-user-groups' of github.com:fractal-analyt…
mfranzon Sep 9, 2024
26a3706
Refactor routes folders (especially the `auth` one)
tcompa Sep 9, 2024
0298197
Fix test relying on listdir output order
tcompa Sep 9, 2024
6321d09
Merge branch '1731-crud-for-user-groups' of github.com:fractal-analyt…
tcompa Sep 9, 2024
cd4a79a
Update tests
tcompa Sep 9, 2024
af4fd49
Improve logging
tcompa Sep 9, 2024
507e942
Remove unsupported `UserUpdate.new_group_ids` attribute
tcompa Sep 9, 2024
726487f
Update __init__.py [skip ci] - cc @ychiucco @mfranzon
tcompa Sep 9, 2024
5d56f13
Remove comment form `models/security.py` [skip ci]
tcompa Sep 9, 2024
b97b3b1
Improve docstrings [skip ci]
tcompa Sep 9, 2024
7426af9
refactor first group creation
mfranzon Sep 9, 2024
9997c9a
Merge branch '1731-crud-for-user-groups' of github.com:fractal-analyt…
mfranzon Sep 9, 2024
b5c210d
Update __init__.py
tcompa Sep 9, 2024
ab535ce
Fix wrong argument in `HTTPException`
tcompa Sep 9, 2024
73ba5d8
BROKEN - initialize `tests/no_version/test_unit_auth.py` - cc @ychiuc…
tcompa Sep 9, 2024
65ab38c
Merge branch '1731-crud-for-user-groups' of github.com:fractal-analyt…
tcompa Sep 9, 2024
7468e72
fix return and test
ychiucco Sep 9, 2024
7a4fcfa
fix return and test
ychiucco Sep 9, 2024
44918aa
Add `test_get_user_optional_group_info`
tcompa Sep 10, 2024
ba449e9
Update `test_get_user_optional_group_info`
tcompa Sep 10, 2024
4ac8a47
Add missing return object for enpdoint
tcompa Sep 10, 2024
35bb539
Remove unreachable valid-password check
tcompa Sep 10, 2024
f77b27c
Add invalid-password coverage in `test_edit_users_as_superuser`
tcompa Sep 10, 2024
239506a
Merge branch '1731-crud-for-user-groups' of github.com:fractal-analyt…
tcompa Sep 10, 2024
bcae19b
Improve docstrings [skip ci]
tcompa Sep 10, 2024
91ef6c9
Improve logging and test for create-first-group function
tcompa Sep 10, 2024
2c08400
Update CHANGELOG [skip ci]
tcompa Sep 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,22 @@
**Note**: Numbers like (\#1234) point to closed Pull Requests on the fractal-server repository.

# 2.4.0

* App:
* Move creation of first user from application startup into `fractalctl set-db` command (\#1738).
* Add creation of default user group into `fractalctl set-db` command (\#1738).
* Create `update-db-script` for current version, that adds all users to default group (\#1738).
* API:
* Added `/auth/group/` and `/auth/group-names/` routers (\#1738).
* Implement `/auth/users/{id}/` POST/PATCH routes in `fractal-server` (\#1738).
* Add `UserManager.on_after_register` hook to add new users to default user group (\#1738).
* Database:
* Added new `usergroup` and `linkusergroup` tables (\#1738).
* Internal
* Refactored `fractal_server.app.auth` and `fractal_server.app.security` (\#1738)/
* Export all relevant modules in `app.models`, since it matters e.g. for `autogenerate`-ing migration scripts (\#1738).


# 2.3.11

* SSH runner:
Expand Down
25 changes: 0 additions & 25 deletions benchmarks/populate_db/populate_db_script.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,9 @@
from passlib.context import CryptContext

from fractal_server.app.db import get_sync_db
from fractal_server.app.models.security import UserOAuth
from fractal_server.app.schemas.user import UserCreate
from fractal_server.app.schemas.v2 import DatasetImportV2
from fractal_server.app.schemas.v2 import JobCreateV2
from fractal_server.app.schemas.v2 import ProjectCreateV2
from fractal_server.app.schemas.v2 import WorkflowCreateV2
from fractal_server.app.schemas.v2 import WorkflowTaskCreateV2
from scripts.client import DEFAULT_CREDENTIALS
from scripts.client import FractalClient


Expand Down Expand Up @@ -37,25 +32,6 @@ def create_image_list(n_images: int) -> list:
return images_list


def create_first_user() -> None:
context = CryptContext(schemes=["bcrypt"], deprecated="auto")
hashed_password = context.hash(DEFAULT_CREDENTIALS["password"])

user_db = UserOAuth(
email=DEFAULT_CREDENTIALS["username"],
hashed_password=hashed_password,
username="admin",
slurm_user="slurm",
is_superuser=True,
is_verified=True,
)

with next(get_sync_db()) as session:
session.add(user_db)
session.commit()
print("Admin created!")


def _create_user_client(
_admin: FractalClient, user_identifier: str
) -> FractalClient:
Expand Down Expand Up @@ -255,7 +231,6 @@ def _user_flow_job(


if __name__ == "__main__":
create_first_user()
admin = FractalClient()

working_task = admin.add_working_task()
Expand Down
27 changes: 25 additions & 2 deletions fractal_server/__main__.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import argparse as ap
import asyncio
import sys

import uvicorn


parser = ap.ArgumentParser(description="fractal-server commands")

subparsers = parser.add_subparsers(title="Commands", dest="cmd", required=True)
Expand Down Expand Up @@ -75,12 +77,33 @@ def set_db():
import alembic.config
from pathlib import Path
import fractal_server
from fractal_server.app.security import _create_first_user
from fractal_server.app.security import _create_first_group
from fractal_server.syringe import Inject
from fractal_server.config import get_settings

alembic_ini = Path(fractal_server.__file__).parent / "alembic.ini"
alembic_args = ["-c", alembic_ini.as_posix(), "upgrade", "head"]

print(f"Run alembic.config, with argv={alembic_args}")
print(f"START: Run alembic.config, with argv={alembic_args}")
alembic.config.main(argv=alembic_args)
print("END: alembic.config")
# Insert default group
print("START: Default group creation")
_create_first_group()
print("END: Default group creation")
# NOTE: It will be fixed with #1739
settings = Inject(get_settings)
print("START: First user creation")
asyncio.run(
_create_first_user(
email=settings.FRACTAL_DEFAULT_ADMIN_EMAIL,
password=settings.FRACTAL_DEFAULT_ADMIN_PASSWORD,
username=settings.FRACTAL_DEFAULT_ADMIN_USERNAME,
is_superuser=True,
is_verified=True,
)
)
print("END: First user creation")


def update_db_data():
Expand Down
16 changes: 11 additions & 5 deletions fractal_server/app/models/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
from .v1 import Project # noqa: F401
from .v2 import ProjectV2 # noqa: F401

# We include the project models to avoid issues with LinkUserProject
# (sometimes taking place in alembic autogenerate)
"""
Note that this module is imported from `fractal_server/migrations/env.py`,
thus we should always export all relevant database models from here or they
will not be picked up by alembic.
"""
from .linkusergroup import LinkUserGroup # noqa: F401
from .linkuserproject import LinkUserProject # noqa: F401
from .linkuserproject import LinkUserProjectV2 # noqa: F401
from .security import * # noqa
from .v1 import * # noqa
from .v2 import * # noqa
11 changes: 11 additions & 0 deletions fractal_server/app/models/linkusergroup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
from sqlmodel import Field
from sqlmodel import SQLModel


class LinkUserGroup(SQLModel, table=True):
"""
Crossing table between User and UserGroup
"""

group_id: int = Field(foreign_key="usergroup.id", primary_key=True)
user_id: int = Field(foreign_key="user_oauth.id", primary_key=True)
27 changes: 24 additions & 3 deletions fractal_server/app/models/security.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,23 @@
#
# Copyright 2022 (C) Friedrich Miescher Institute for Biomedical Research and
# University of Zurich
from datetime import datetime
from typing import Optional

from pydantic import EmailStr
from sqlalchemy import Column
from sqlalchemy.types import DateTime
from sqlalchemy.types import JSON
from sqlmodel import Field
from sqlmodel import Relationship
from sqlmodel import SQLModel

from fractal_server.utils import get_timestamp


class OAuthAccount(SQLModel, table=True):
"""
OAuth account model
ORM model for OAuth accounts (`oauthaccount` database table).

This class is based on fastapi_users_db_sqlmodel::SQLModelBaseOAuthAccount.
Original Copyright: 2021 François Voron, released under MIT licence.
Expand Down Expand Up @@ -56,7 +60,7 @@ class Config:

class UserOAuth(SQLModel, table=True):
"""
User model
ORM model for the `user_oauth` database table.

This class is a modification of SQLModelBaseUserDB from from
fastapi_users_db_sqlmodel. Original Copyright: 2022 François Voron,
Expand All @@ -74,7 +78,6 @@ class UserOAuth(SQLModel, table=True):
cache_dir:
username:
oauth_accounts:
project_list:
"""

__tablename__ = "user_oauth"
Expand Down Expand Up @@ -103,3 +106,21 @@ class UserOAuth(SQLModel, table=True):

class Config:
orm_mode = True


class UserGroup(SQLModel, table=True):
"""
ORM model for the `usergroup` database table.

Attributes:
id: ID of the group
name: Name of the group
timestamp_created: Time of creation
"""

id: Optional[int] = Field(default=None, primary_key=True)
name: str = Field(unique=True)
timestamp_created: datetime = Field(
default_factory=get_timestamp,
sa_column=Column(DateTime(timezone=True), nullable=False),
)
2 changes: 1 addition & 1 deletion fractal_server/app/models/v1/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from . import LinkUserProject
from ....utils import get_timestamp
from ...schemas.v1.project import _ProjectBaseV1
from ..security import UserOAuth
from fractal_server.app.models import UserOAuth


class Project(_ProjectBaseV1, SQLModel, table=True):
Expand Down
6 changes: 3 additions & 3 deletions fractal_server/app/models/v2/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
from sqlmodel import Relationship
from sqlmodel import SQLModel

from . import LinkUserProjectV2
from ....utils import get_timestamp
from ..security import UserOAuth
from .. import LinkUserProjectV2
from fractal_server.app.models import UserOAuth
from fractal_server.utils import get_timestamp


class ProjectV2(SQLModel, table=True):
Expand Down
28 changes: 14 additions & 14 deletions fractal_server/app/routes/admin/v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
from ....zip_tools import _zip_folder_to_byte_stream_iterator
from ...db import AsyncSession
from ...db import get_async_db
from ...models.security import UserOAuth as User
from ...models.v1 import ApplyWorkflow
from ...models.v1 import Dataset
from ...models.v1 import JobStatusTypeV1
Expand All @@ -33,9 +32,10 @@
from ...schemas.v1 import DatasetReadV1
from ...schemas.v1 import ProjectReadV1
from ...schemas.v1 import WorkflowReadV1
from ...security import current_active_superuser
from ..aux._job import _write_shutdown_file
from ..aux._runner import _check_shutdown_is_supported
from fractal_server.app.models import UserOAuth
from fractal_server.app.routes.auth import current_active_superuser

router_admin_v1 = APIRouter()

Expand Down Expand Up @@ -63,7 +63,7 @@ async def view_project(
user_id: Optional[int] = None,
timestamp_created_min: Optional[datetime] = None,
timestamp_created_max: Optional[datetime] = None,
user: User = Depends(current_active_superuser),
user: UserOAuth = Depends(current_active_superuser),
db: AsyncSession = Depends(get_async_db),
) -> list[ProjectReadV1]:
"""
Expand All @@ -80,7 +80,7 @@ async def view_project(
stm = stm.where(Project.id == id)

if user_id is not None:
stm = stm.where(Project.user_list.any(User.id == user_id))
stm = stm.where(Project.user_list.any(UserOAuth.id == user_id))
if timestamp_created_min is not None:
timestamp_created_min = _convert_to_db_timestamp(timestamp_created_min)
stm = stm.where(Project.timestamp_created >= timestamp_created_min)
Expand All @@ -103,7 +103,7 @@ async def view_workflow(
name_contains: Optional[str] = None,
timestamp_created_min: Optional[datetime] = None,
timestamp_created_max: Optional[datetime] = None,
user: User = Depends(current_active_superuser),
user: UserOAuth = Depends(current_active_superuser),
db: AsyncSession = Depends(get_async_db),
) -> list[WorkflowReadV1]:
"""
Expand All @@ -119,7 +119,7 @@ async def view_workflow(

if user_id is not None:
stm = stm.join(Project).where(
Project.user_list.any(User.id == user_id)
Project.user_list.any(UserOAuth.id == user_id)
)
if id is not None:
stm = stm.where(Workflow.id == id)
Expand Down Expand Up @@ -153,7 +153,7 @@ async def view_dataset(
type: Optional[str] = None,
timestamp_created_min: Optional[datetime] = None,
timestamp_created_max: Optional[datetime] = None,
user: User = Depends(current_active_superuser),
user: UserOAuth = Depends(current_active_superuser),
db: AsyncSession = Depends(get_async_db),
) -> list[DatasetReadV1]:
"""
Expand All @@ -170,7 +170,7 @@ async def view_dataset(

if user_id is not None:
stm = stm.join(Project).where(
Project.user_list.any(User.id == user_id)
Project.user_list.any(UserOAuth.id == user_id)
)
if id is not None:
stm = stm.where(Dataset.id == id)
Expand Down Expand Up @@ -211,7 +211,7 @@ async def view_job(
end_timestamp_min: Optional[datetime] = None,
end_timestamp_max: Optional[datetime] = None,
log: bool = True,
user: User = Depends(current_active_superuser),
user: UserOAuth = Depends(current_active_superuser),
db: AsyncSession = Depends(get_async_db),
) -> list[ApplyWorkflowReadV1]:
"""
Expand Down Expand Up @@ -243,7 +243,7 @@ async def view_job(
stm = stm.where(ApplyWorkflow.id == id)
if user_id is not None:
stm = stm.join(Project).where(
Project.user_list.any(User.id == user_id)
Project.user_list.any(UserOAuth.id == user_id)
)
if project_id is not None:
stm = stm.where(ApplyWorkflow.project_id == project_id)
Expand Down Expand Up @@ -282,7 +282,7 @@ async def view_job(
async def view_single_job(
job_id: int = None,
show_tmp_logs: bool = False,
user: User = Depends(current_active_superuser),
user: UserOAuth = Depends(current_active_superuser),
db: AsyncSession = Depends(get_async_db),
) -> ApplyWorkflowReadV1:

Expand Down Expand Up @@ -311,7 +311,7 @@ async def view_single_job(
async def update_job(
job_update: ApplyWorkflowUpdateV1,
job_id: int,
user: User = Depends(current_active_superuser),
user: UserOAuth = Depends(current_active_superuser),
db: AsyncSession = Depends(get_async_db),
) -> Optional[ApplyWorkflowReadV1]:
"""
Expand Down Expand Up @@ -344,7 +344,7 @@ async def update_job(
@router_admin_v1.get("/job/{job_id}/stop/", status_code=202)
async def stop_job(
job_id: int,
user: User = Depends(current_active_superuser),
user: UserOAuth = Depends(current_active_superuser),
db: AsyncSession = Depends(get_async_db),
) -> Response:
"""
Expand All @@ -371,7 +371,7 @@ async def stop_job(
)
async def download_job_logs(
job_id: int,
user: User = Depends(current_active_superuser),
user: UserOAuth = Depends(current_active_superuser),
db: AsyncSession = Depends(get_async_db),
) -> StreamingResponse:
"""
Expand Down
Loading