Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 19 additions & 0 deletions app/core/auth/endpoints_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,12 @@ async def login_for_access_token(
detail="Incorrect login or password",
headers={"WWW-Authenticate": "Bearer"},
)
if user.should_change_password:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail="Should change password",
headers={"WWW-Authenticate": "Bearer"},
)
# 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.auth)
Expand Down Expand Up @@ -314,6 +320,19 @@ async def authorize_validation(
status_code=status.HTTP_302_FOUND,
)

if user.should_change_password:
hyperion_access_logger.warning(
f"Authorize-validation: User {authorizereq.email} should change password ({request_id})",
)
return RedirectResponse(
settings.CLIENT_URL
+ calypsso.get_recover_password_relative_url( # ty:ignore[unresolved-attribute]
email=authorizereq.email,
should_change_password=True,
),
status_code=status.HTTP_302_FOUND,
)

# The auth_client may restrict the usage of the client to specific Hyperion permissions
if auth_client.permission is not None:
if not (
Expand Down
12 changes: 12 additions & 0 deletions app/core/users/cruds_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,18 @@ async def update_user_password_by_id(
await db.flush()


async def update_should_user_change_password_by_id(
db: AsyncSession,
user_id: str,
should_change_password: bool = True,
):
await db.execute(
update(models_users.CoreUser)
.where(models_users.CoreUser.id == user_id)
.values(should_change_password=should_change_password),
)


async def remove_users_from_school(
db: AsyncSession,
school_id: UUID,
Expand Down
142 changes: 101 additions & 41 deletions app/core/users/endpoints_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,7 @@ async def activate_user(
school_id=school_id,
account_type=account_type,
password_hash=password_hash,
should_change_password=False,
name=user.name,
firstname=user.firstname,
nickname=user.nickname,
Expand Down Expand Up @@ -640,12 +641,31 @@ async def reset_password(
),
)

user = await cruds_users.get_user_by_id(db=db, user_id=recover_request.user_id)
if user is None:
raise HTTPException(status_code=404, detail="Invalid user ID")
if user.should_change_password:
# we control whether we check if the new password is different
if security.verify_password(
reset_password_request.new_password,
user.password_hash,
):
raise HTTPException(
status_code=403,
detail="The new password should not be identical to the current password",
)

new_password_hash = security.get_password_hash(reset_password_request.new_password)
await cruds_users.update_user_password_by_id(
db=db,
user_id=recover_request.user_id,
new_password_hash=new_password_hash,
)
await cruds_users.update_should_user_change_password_by_id(
db=db,
user_id=recover_request.user_id,
should_change_password=False,
)

# As the user has reset its password, all other recovery requests can be deleted from the table
await cruds_users.delete_recover_request_by_email(
Expand All @@ -664,6 +684,87 @@ async def reset_password(
return standard_responses.Result()


@router.post(
"/users/change-password",
response_model=standard_responses.Result,
status_code=201,
)
async def change_password(
change_password_request: schemas_users.ChangePasswordRequest,
db: AsyncSession = Depends(get_db),
):
"""
Change a user password.

This endpoint will check the **old_password**, see also the `/users/reset-password` endpoint if the user forgot their password.
"""

user = await security.authenticate_user(
db=db,
email=change_password_request.email,
password=change_password_request.old_password,
)
if user is None:
raise HTTPException(status_code=403, detail="The old password is invalid")

if user.should_change_password:
# # we control whether we check if the new password is different
# if security.verify_password(
# change_password_request.new_password,
# user.password_hash,
# ):
# raise HTTPException(
# status_code=403,
# detail="The new password should not be identical to the current password",
# )
raise HTTPException(
status_code=403,
detail="User must reset password via email recovery",
)

new_password_hash = security.get_password_hash(change_password_request.new_password)
await cruds_users.update_user_password_by_id(
db=db,
user_id=user.id,
new_password_hash=new_password_hash,
)
# await cruds_users.update_should_user_change_password_by_id(
# db=db,
# user_id=user.id,
# should_change_password=False,
# )

# Revoke existing auth refresh tokens
# to force the user to reauthenticate on all services and devices
# when their token expire
await cruds_auth.revoke_refresh_token_by_user_id(
db=db,
user_id=user.id,
)

return standard_responses.Result()


@router.post("/users/invalidate-password/{user_id}", status_code=201)
async def invalidate_password(
user_id: str,
user: models_users.CoreUser = Depends(is_user_in(GroupType.admin)),
db: AsyncSession = Depends(get_db),
):
"""
Invalidate one user's password.
Also revoke their refresh tokens to disable API access as soon as possible.
The concerned user should change their password with a different one to use our services again.

**This endpoint is only usable by administrators**
"""
await cruds_users.update_should_user_change_password_by_id(db=db, user_id=user_id)
await cruds_auth.revoke_refresh_token_by_user_id(
db=db,
user_id=recover_request.user_id,
)


@router.post(
"/users/migrate-mail",
status_code=204,
Expand Down Expand Up @@ -799,47 +900,6 @@ async def migrate_mail_confirm(
return "The email address has been successfully updated"


@router.post(
"/users/change-password",
response_model=standard_responses.Result,
status_code=201,
)
async def change_password(
change_password_request: schemas_users.ChangePasswordRequest,
db: AsyncSession = Depends(get_db),
):
"""
Change a user password.

This endpoint will check the **old_password**, see also the `/users/reset-password` endpoint if the user forgot their password.
"""

user = await security.authenticate_user(
db=db,
email=change_password_request.email,
password=change_password_request.old_password,
)
if user is None:
raise HTTPException(status_code=403, detail="The old password is invalid")

new_password_hash = security.get_password_hash(change_password_request.new_password)
await cruds_users.update_user_password_by_id(
db=db,
user_id=user.id,
new_password_hash=new_password_hash,
)

# Revoke existing auth refresh tokens
# to force the user to reauthenticate on all services and devices
# when their token expire
await cruds_auth.revoke_refresh_token_by_user_id(
db=db,
user_id=user.id,
)

return standard_responses.Result()


# We put the following endpoints at the end of the file to prevent them
# from interacting with the previous endpoints
# Ex: /users/activate is interpreted as a user whose id is "activate"
Expand Down
2 changes: 2 additions & 0 deletions app/core/users/factory_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ async def create_core_users(cls, db: AsyncSession):
user = CoreUser(
id=cls.other_users_id[i],
password_hash=password[i],
should_change_password=False,
firstname=firstname[i],
nickname=nickname[i],
name=name[i],
Expand All @@ -115,6 +116,7 @@ async def create_core_users(cls, db: AsyncSession):
password_hash=security.get_password_hash(
user_info.password or faker.password(16, True, True, True, True),
),
should_change_password=False,
firstname=user_info.firstname,
nickname=user_info.nickname,
name=user_info.name,
Expand Down
1 change: 1 addition & 0 deletions app/core/users/models_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class CoreUser(Base):
email: Mapped[str] = mapped_column(unique=True, index=True)
school_id: Mapped[UUID] = mapped_column(ForeignKey("core_school.id"))
password_hash: Mapped[str]
should_change_password: Mapped[bool]
# Depending on the account type, the user may have different rights and access to different features
# External users may exist for:
# - accounts meant to be used by external services based on Hyperion SSO or Hyperion backend
Expand Down
53 changes: 53 additions & 0 deletions migrations/versions/54-password_invalidation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
"""password-invalidation

Create Date: 2025-10-03 11:24:35.720759
"""

from collections.abc import Sequence
from typing import TYPE_CHECKING

if TYPE_CHECKING:
from pytest_alembic import MigrationContext

import sqlalchemy as sa
from alembic import op

# revision identifiers, used by Alembic.
revision: str = "cfba514689ed"
down_revision: str | None = "9fc3dc926600"
branch_labels: str | Sequence[str] | None = None
depends_on: str | Sequence[str] | None = None


def upgrade() -> None:
# ### commands auto generated by Alembic - please adjust! ###
op.add_column(
"core_user",
sa.Column(
"should_change_password",
sa.Boolean(),
nullable=False,
server_default=sa.sql.false(),
),
)
# ### end Alembic commands ###


def downgrade() -> None:
# ### commands auto generated by Alembic - please adjust! ###
op.drop_column("core_user", "should_change_password")
# ### end Alembic commands ###


def pre_test_upgrade(
alembic_runner: "MigrationContext",
alembic_connection: sa.Connection,
) -> None:
pass


def test_upgrade(
alembic_runner: "MigrationContext",
alembic_connection: sa.Connection,
) -> None:
pass
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ authlib==1.6.5
bcrypt==4.1.3 # password hashing
boto3==1.38.23
broadcaster==0.3.1 # Working with websockets with multiple workers.
calypsso==2.7.0
calypsso==2.8.0
Faker==37.1.0
fastapi[standard]==0.122.0
firebase-admin==7.1.0 # Firebase is used for push notification
Expand Down
1 change: 1 addition & 0 deletions tests/commons.py
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ async def create_user_with_groups(
email=email or (get_random_string() + "@etu.ec-lyon.fr"),
school_id=school_id,
password_hash=password_hash,
should_change_password=False,
name=name or get_random_string(),
firstname=firstname or get_random_string(),
nickname=nickname,
Expand Down
Loading