feat: migrate POST /api/users endpoint to POST /api/v1/users (#146)

# Description

This PR adds a new endpoint `POST /api/v1/users` allowing the creation
of a new user for API v1.

Changes of the new endpoint compared to the old one:
* `workspaces` attribute is not allowed when creating users with this
endpoint. If necessary subsequent requests should be done to an endpoint
that is still pending to be migrated. Specifically `POST
/api/v1/workspaces/:workspace_id/users/:user_id`.
* In the same way that is happening with previous PRs the returned
created user is not including `full_name` attribute.

Closes #145 

**Type of change**

(Please delete options that are not relevant. Remember to title the PR
according to the type of change)

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] Refactor (change restructuring the codebase without changing
functionality)
- [ ] Improvement (change adding some improvement to an existing
functionality)
- [ ] Documentation update

**How Has This Been Tested**

(Please describe the tests that you ran to verify your changes. And
ideally, reference `tests`)

- [x] Adding new tests and checking that the tests for API v0 are
working as expected if using API v1 endpoint (except the ones creating
workspaces along with the user).

**Checklist**

- [ ] I added relevant documentation
- [ ] follows the style guidelines of this project
- [ ] I did a self-review of my code
- [ ] I made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] I filled out [the contributor form](https://tally.so/r/n9XrxK)
(see text above)
- [ ] I have added relevant notes to the CHANGELOG.md file (See
https://keepachangelog.com/)

CHANGELOG.md

@@ -19,6 +19,7 @@ These are the section headers that we use:
 - Added `POST /api/v1/token` endpoint to generate a new API token for a user. ([#138](https://github.com/argilla-io/argilla-server/pull/138))
 - Added `GET /api/v1/me` endpoint to get the current user information. ([#140](https://github.com/argilla-io/argilla-server/pull/140))
 - Added `GET /api/v1/users` endpoint to get a list of all users. ([#142](https://github.com/argilla-io/argilla-server/pull/142))
+- Added `POST /api/v1/users` endpoint to create a new user. ([#146](https://github.com/argilla-io/argilla-server/pull/146))
 
 ## [Unreleased]()
 

src/argilla_server/apis/v0/handlers/users.py

@@ -23,6 +23,7 @@
 from argilla_server.contexts import accounts
 from argilla_server.database import get_async_db
 from argilla_server.errors import EntityAlreadyExistsError, EntityNotFoundError
+from argilla_server.errors.future import NotUniqueError
 from argilla_server.policies import UserPolicy, authorize
 from argilla_server.pydantic_v1 import parse_obj_as
 from argilla_server.schemas.v0.users import User, UserCreate
@@ -90,17 +91,17 @@ async def create_user(
 ):
     await authorize(current_user, UserPolicy.create)
 
-    user = await accounts.get_user_by_username(db, user_create.username)
-    if user is not None:
-        raise EntityAlreadyExistsError(name=user_create.username, type=User)
-
     try:
-        user = await accounts.create_user(db, user_create)
+        user = await accounts.create_user(db, user_create.dict(), user_create.workspaces)
+
         telemetry.track_user_created(user)
+    except NotUniqueError:
+        raise EntityAlreadyExistsError(name=user_create.username, type=User)
     except Exception as e:
         raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(e))
 
     await user.awaitable_attrs.workspaces
+
     return User.from_orm(user)
 
 

src/argilla_server/apis/v1/handlers/users.py

@@ -21,8 +21,9 @@
 from argilla_server import models, telemetry
 from argilla_server.contexts import accounts
 from argilla_server.database import get_async_db
+from argilla_server.errors.future import NotUniqueError
 from argilla_server.policies import UserPolicyV1, authorize
-from argilla_server.schemas.v1.users import User, Users
+from argilla_server.schemas.v1.users import User, UserCreate, Users
 from argilla_server.schemas.v1.workspaces import Workspaces
 from argilla_server.security import auth
 
@@ -49,6 +50,27 @@ async def list_users(
     return Users(items=users)
 
 
+@router.post("/users", status_code=status.HTTP_201_CREATED, response_model=User)
+async def create_user(
+    *,
+    db: AsyncSession = Depends(get_async_db),
+    user_create: UserCreate,
+    current_user: models.User = Security(auth.get_current_user),
+):
+    await authorize(current_user, UserPolicyV1.create)
+
+    try:
+        user = await accounts.create_user(db, user_create.dict())
+
+        telemetry.track_user_created(user)
+    except NotUniqueError as e:
+        raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(e))
+    except Exception as e:
+        raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(e))
+
+    return user
+
+
 @router.get("/users/{user_id}/workspaces", response_model=Workspaces)
 async def list_user_workspaces(
     *,

src/argilla_server/contexts/accounts.py

@@ -21,6 +21,7 @@
 from sqlalchemy.orm import Session, selectinload
 
 from argilla_server.enums import UserRole
+from argilla_server.errors.future import NotUniqueError
 from argilla_server.models import User, Workspace, WorkspaceUser
 from argilla_server.schemas.v0.users import UserCreate
 from argilla_server.schemas.v0.workspaces import WorkspaceCreate, WorkspaceUserCreate
@@ -121,23 +122,29 @@ async def list_users_by_ids(db: AsyncSession, ids: Iterable[UUID]) -> Sequence[U
     return result.scalars().all()
 
 
-async def create_user(db: "AsyncSession", user_create: UserCreate) -> User:
+# TODO: After removing API v0 implementation we can remove the workspaces attribute.
+# With API v1 the workspaces will be created doing additional requests to other endpoints for it.
+async def create_user(db: AsyncSession, user_attrs: dict, workspaces: Union[List[str], None] = None) -> User:
+    if (await get_user_by_username(db, user_attrs["username"])) is not None:
+        raise NotUniqueError(f"Username `{user_attrs['username']}` is not unique")
+
     async with db.begin_nested():
         user = await User.create(
             db,
-            first_name=user_create.first_name,
-            last_name=user_create.last_name,
-            username=user_create.username,
-            role=user_create.role,
-            password_hash=hash_password(user_create.password),
+            first_name=user_attrs["first_name"],
+            last_name=user_attrs["last_name"],
+            username=user_attrs["username"],
+            role=user_attrs["role"],
+            password_hash=hash_password(user_attrs["password"]),
             autocommit=False,
         )
 
-        if user_create.workspaces:
-            for workspace_name in user_create.workspaces:
+        if workspaces is not None:
+            for workspace_name in workspaces:
                 workspace = await get_workspace_by_name(db, workspace_name)
                 if not workspace:
                     raise ValueError(f"Workspace '{workspace_name}' does not exist")
+
                 await WorkspaceUser.create(
                     db,
                     workspace_id=workspace.id,
@@ -154,15 +161,18 @@ async def create_user_with_random_password(
     db,
     username: str,
     first_name: str,
-    workspaces: List[str] = None,
     role: UserRole = UserRole.annotator,
+    workspaces: Union[List[str], None] = None,
 ) -> User:
-    password = _generate_random_password()
-
-    user_create = UserCreate(
-        first_name=first_name, username=username, role=role, password=password, workspaces=workspaces
-    )
-    return await create_user(db, user_create)
+    user_attrs = {
+        "first_name": first_name,
+        "last_name": None,
+        "username": username,
+        "role": role,
+        "password": _generate_random_password(),
+    }
+
+    return await create_user(db, user_attrs, workspaces)
 
 
 async def delete_user(db: AsyncSession, user: User) -> User:

src/argilla_server/errors/future/base_errors.py

@@ -12,7 +12,7 @@
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
 
-__all__ = ["NotFoundError", "AuthenticationError"]
+__all__ = ["NotFoundError", "NotUniqueError", "AuthenticationError"]
 
 
 class NotFoundError(Exception):
@@ -21,6 +21,12 @@ class NotFoundError(Exception):
     pass
 
 
+class NotUniqueError(Exception):
+    """Custom Argilla not unique error. Use it for situations where an Argilla domain entity already exists violating a constraint."""
+
+    pass
+
+
 class AuthenticationError(Exception):
     """Custom Argilla unauthorized error. Use it for situations where an request is not authorized to perform an action."""
 

src/argilla_server/policies.py

@@ -133,6 +133,10 @@ class UserPolicyV1:
     async def list(cls, actor: User) -> bool:
         return actor.is_owner
 
+    @classmethod
+    async def create(cls, actor: User) -> bool:
+        return actor.is_owner
+
     @classmethod
     async def list_workspaces(cls, actor: User) -> bool:
         return actor.is_owner

src/argilla_server/schemas/v1/users.py

@@ -17,7 +17,11 @@
 from uuid import UUID
 
 from argilla_server.enums import UserRole
-from argilla_server.pydantic_v1 import BaseModel
+from argilla_server.pydantic_v1 import BaseModel, Field, constr
+
+USER_USERNAME_REGEX = "^(?!-|_)[A-za-z0-9-_]+$"
+USER_PASSWORD_MIN_LENGTH = 8
+USER_PASSWORD_MAX_LENGTH = 100
 
 
 class User(BaseModel):
@@ -34,5 +38,13 @@ class Config:
         orm_mode = True
 
 
+class UserCreate(BaseModel):
+    first_name: constr(min_length=1, strip_whitespace=True)
+    last_name: Optional[constr(min_length=1, strip_whitespace=True)]
+    username: str = Field(regex=USER_USERNAME_REGEX, min_length=1)
+    role: Optional[UserRole]
+    password: str = Field(min_length=USER_PASSWORD_MIN_LENGTH, max_length=USER_PASSWORD_MAX_LENGTH)
+
+
 class Users(BaseModel):
     items: List[User]

tests/unit/api/v0/test_users.py

@@ -217,6 +217,53 @@ async def test_create_user_with_non_default_role(
     assert response_body["role"] == UserRole.owner.value
 
 
+@pytest.mark.asyncio
+async def test_create_user_with_first_name_including_leading_and_trailing_spaces(
+    async_client: "AsyncClient", db: "AsyncSession", owner_auth_header: dict
+):
+    response = await async_client.post(
+        "/api/users",
+        headers=owner_auth_header,
+        json={
+            "first_name": "  First name  ",
+            "username": "username",
+            "password": "12345678",
+        },
+    )
+
+    assert response.status_code == 200
+
+    assert (await db.execute(select(func.count(User.id)))).scalar() == 2
+    user = (await db.execute(select(User).filter_by(username="username"))).scalar_one()
+
+    assert response.json()["first_name"] == "First name"
+    assert user.first_name == "First name"
+
+
+@pytest.mark.asyncio
+async def test_create_user_with_last_name_including_leading_and_trailing_spaces(
+    async_client: "AsyncClient", db: "AsyncSession", owner_auth_header: dict
+):
+    response = await async_client.post(
+        "/api/users",
+        headers=owner_auth_header,
+        json={
+            "first_name": "First name",
+            "last_name": "  Last name  ",
+            "username": "username",
+            "password": "12345678",
+        },
+    )
+
+    assert response.status_code == 200
+
+    assert (await db.execute(select(func.count(User.id)))).scalar() == 2
+    user = (await db.execute(select(User).filter_by(username="username"))).scalar_one()
+
+    assert response.json()["last_name"] == "Last name"
+    assert user.last_name == "Last name"
+
+
 @pytest.mark.asyncio
 async def test_create_user_without_authentication(async_client: "AsyncClient", db: "AsyncSession"):
     user = {"first_name": "first-name", "username": "username", "password": "12345678"}