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

Disable account #344

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
"[python]": {
"editor.defaultFormatter": "ms-python.black-formatter"
},
"python.testing.pytestArgs": ["tests"],
"python.testing.pytestArgs": [
"."
],
"python.testing.unittestEnabled": false,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be changed?

"python.testing.pytestEnabled": true,
"black-formatter.path": [
Expand Down
2 changes: 2 additions & 0 deletions app/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
campaign,
cinema,
core,
external_account,
groups,
loan,
notification,
Expand All @@ -32,6 +33,7 @@
api_router.include_router(campaign.router)
api_router.include_router(cinema.router)
api_router.include_router(core.router)
api_router.include_router(external_account.router)
api_router.include_router(groups.router)
api_router.include_router(loan.router)
api_router.include_router(notification.router)
Expand Down
24 changes: 24 additions & 0 deletions app/cruds/cruds_external_account.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
from sqlalchemy import update
from sqlalchemy.exc import IntegrityError
from sqlalchemy.ext.asyncio import AsyncSession

from app.models import models_core
from app.utils.types.groups_type import GroupType


async def disable_external_accounts(db: AsyncSession):
try:
await db.execute(
update(models_core.CoreUser)
.where(
models_core.CoreUser.groups.any(
models_core.CoreGroup.id == GroupType.external.value
)
)
.values(enabled=False)
)
await db.commit()

except IntegrityError as error:
await db.rollback()
raise ValueError(error)

Check warning on line 24 in app/cruds/cruds_external_account.py

View check run for this annotation

Codecov / codecov/patch

app/cruds/cruds_external_account.py#L22-L24

Added lines #L22 - L24 were not covered by tests
5 changes: 5 additions & 0 deletions app/endpoints/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@
detail="Incorrect login or password",
headers={"WWW-Authenticate": "Bearer"},
)
if not user.enabled:
raise HTTPException(

Check warning on line 85 in app/endpoints/auth.py

View check run for this annotation

Codecov / codecov/patch

app/endpoints/auth.py#L84-L85

Added lines #L84 - L85 were not covered by tests
status_code=status.HTTP_401_UNAUTHORIZED,
detail="Disabled account. Contact eclair@myecl.fr for more informations.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to hard code an email in the API response?

)
# We put the user id in the subject field of the token.
# The subject `sub` is a JWT registered claim name, see https://datatracker.ietf.org/doc/html/rfc7519#section-4.1
data = schemas_auth.TokenData(sub=user.id, scopes=ScopeType.API)
Expand Down
22 changes: 22 additions & 0 deletions app/endpoints/external_account.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
from fastapi import APIRouter, Depends
from sqlalchemy.ext.asyncio import AsyncSession

from app.cruds import cruds_external_account
from app.dependencies import get_db, is_user_a_member_of
from app.models import models_core
from app.utils.types.groups_type import GroupType
from app.utils.types.tags import Tags

router = APIRouter()


@router.get(
"/external/",
status_code=200,
tags=[Tags.external_account],
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could put the endpoint in users.py

async def disable_external_users(
db: AsyncSession = Depends(get_db),
user: models_core.CoreUser = Depends(is_user_a_member_of(GroupType.admin)),
):
return await cruds_external_account.disable_external_accounts(db=db)
7 changes: 4 additions & 3 deletions app/endpoints/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,10 @@
# Its a former student email address
account_type = AccountType.formerstudent
else:
raise HTTPException(
status_code=400,
detail="Invalid ECL email address.",
# Its a external student email address
account_type = AccountType.external
hyperion_security_logger.info(

Check warning on line 183 in app/endpoints/users.py

View check run for this annotation

Codecov / codecov/patch

app/endpoints/users.py#L182-L183

Added lines #L182 - L183 were not covered by tests
"External account created. Will maybe be disabled."
)

# Make sure a confirmed account does not already exist
Expand Down
3 changes: 2 additions & 1 deletion app/models/models_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from datetime import date, datetime

from sqlalchemy import Date, DateTime, Enum, ForeignKey, Integer, String
from sqlalchemy import Boolean, Date, DateTime, Enum, ForeignKey, Integer, String
from sqlalchemy.orm import Mapped, mapped_column, relationship

from app.database import Base
Expand Down Expand Up @@ -35,6 +35,7 @@ class CoreUser(Base):
phone: Mapped[str | None] = mapped_column(String)
floor: Mapped[FloorsType] = mapped_column(Enum(FloorsType), nullable=False)
created_on: Mapped[datetime | None] = mapped_column(DateTime(timezone=True))
enabled: Mapped[bool] = mapped_column(Boolean, nullable=False)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think of disabled?


# We use list["CoreGroup"] with quotes as CoreGroup is only defined after this class
# Defining CoreUser after CoreGroup would cause a similar issue
Expand Down
1 change: 1 addition & 0 deletions app/schemas/schemas_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ class CoreUser(CoreUserSimple):
phone: str | None = None
created_on: datetime | None = None
groups: list[CoreGroupSimple] = []
enabled: bool


class CoreUserUpdate(BaseModel):
Expand Down
2 changes: 2 additions & 0 deletions app/utils/types/groups_type.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class GroupType(str, Enum):
formerstudent = "ab4c7503-41b3-11ee-8177-089798f1a4a5"
staff = "703056c4-be9d-475c-aa51-b7fc62a96aaa"
association = "29751438-103c-42f2-b09b-33fbb20758a7"
external = "e9f1085f-50e5-440c-80c7-b36c9ad4d6fb"

# Core groups
admin = "0a25cb76-4b63-4fd3-b939-da6d9feabf28"
Expand Down Expand Up @@ -43,6 +44,7 @@ class AccountType(str, Enum):
formerstudent = GroupType.formerstudent.value
staff = GroupType.staff.value
association = GroupType.association.value
external = GroupType.external.value

def __str__(self):
return f"{self.name}<{self.value}>"
1 change: 1 addition & 0 deletions app/utils/types/tags.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@ class Tags(str, Enum):
raffle = "Raffle"
advert = "Advert"
notifications = "Notifications"
external_account = "External_account"
1 change: 1 addition & 0 deletions tests/commons.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ async def create_user_with_groups(
name=name or get_random_string(),
firstname=firstname or get_random_string(),
floor=floor,
enabled=True
)

async with TestingSessionLocal() as db:
Expand Down
1 change: 1 addition & 0 deletions tests/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ async def init_objects():
birthday=date.fromisoformat("2000-01-01"),
floor=FloorsType.Autre,
created_on=date.fromisoformat("2000-01-01"),
enabled=True,
)
await add_object_to_db(user)

Expand Down
41 changes: 41 additions & 0 deletions tests/test_external_account.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import pytest_asyncio

from app.models import models_core
from app.utils.types.groups_type import GroupType

# We need to import event_loop for pytest-asyncio routine defined bellow
from tests.commons import event_loop # noqa
from tests.commons import client, create_api_access_token, create_user_with_groups

user: models_core.CoreUser

token_admin: str = ""


@pytest_asyncio.fixture(scope="module", autouse=True)
async def init_objects():
global user
user = await create_user_with_groups([GroupType.external])

global user_admin
user_admin = await create_user_with_groups([GroupType.admin])

global token_admin
token_admin = create_api_access_token(user_admin)


def test_disable_external_account():
global user
response = client.get(
"/external/",
follow_redirects=False,
headers={"Authorization": f"Bearer {token_admin}"},
)
response1 = client.get(
f"/users/{user.id}",
headers={"Authorization": f"Bearer {token_admin}"},
)
assert response.status_code == 200
assert response1.status_code == 200
data = response1.json()
assert not data["enabled"]