From 4762ffd9485734fa7c21bdf29b9d6ccb1680585f Mon Sep 17 00:00:00 2001 From: Marc-Andrieu Date: Fri, 17 Oct 2025 09:17:37 +0200 Subject: [PATCH 1/9] Add should_change_password column --- app/core/users/cruds_users.py | 12 +++++ app/core/users/endpoints_users.py | 1 + app/core/users/factory_users.py | 2 + app/core/users/models_users.py | 1 + .../versions/43-password_invalidation.py | 53 +++++++++++++++++++ tests/commons.py | 1 + 6 files changed, 70 insertions(+) create mode 100644 migrations/versions/43-password_invalidation.py diff --git a/app/core/users/cruds_users.py b/app/core/users/cruds_users.py index 5863711723..009150d4d0 100644 --- a/app/core/users/cruds_users.py +++ b/app/core/users/cruds_users.py @@ -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, diff --git a/app/core/users/endpoints_users.py b/app/core/users/endpoints_users.py index 3fa1492edf..d9e4bcb98a 100644 --- a/app/core/users/endpoints_users.py +++ b/app/core/users/endpoints_users.py @@ -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, diff --git a/app/core/users/factory_users.py b/app/core/users/factory_users.py index 468dcfbe06..9e23daf8f6 100644 --- a/app/core/users/factory_users.py +++ b/app/core/users/factory_users.py @@ -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], @@ -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, diff --git a/app/core/users/models_users.py b/app/core/users/models_users.py index 3802326f72..4ac1d2c445 100644 --- a/app/core/users/models_users.py +++ b/app/core/users/models_users.py @@ -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 diff --git a/migrations/versions/43-password_invalidation.py b/migrations/versions/43-password_invalidation.py new file mode 100644 index 0000000000..aa02eb937f --- /dev/null +++ b/migrations/versions/43-password_invalidation.py @@ -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 = "c4812e1ab108" +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 diff --git a/tests/commons.py b/tests/commons.py index 6d187f4772..3ae2e40e23 100644 --- a/tests/commons.py +++ b/tests/commons.py @@ -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, From d9c39b0665e3458bb1fce603bb7b87d935102faa Mon Sep 17 00:00:00 2001 From: Marc-Andrieu Date: Fri, 17 Oct 2025 11:22:37 +0200 Subject: [PATCH 2/9] Feat: don't allow changing to the same password --- app/core/users/endpoints_users.py | 33 +++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/app/core/users/endpoints_users.py b/app/core/users/endpoints_users.py index d9e4bcb98a..0e35a449bf 100644 --- a/app/core/users/endpoints_users.py +++ b/app/core/users/endpoints_users.py @@ -641,12 +641,29 @@ async def reset_password( ), ) + user = await cruds_users.get_user_by_id(db=db, user_id=recover_request.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( @@ -823,12 +840,28 @@ async def change_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", + ) + 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 From bd51d02994de7cb793e1cfe300c35c6624675a3e Mon Sep 17 00:00:00 2001 From: Marc-Andrieu <146140470+Marc-Andrieu@users.noreply.github.com> Date: Fri, 17 Oct 2025 11:34:34 +0200 Subject: [PATCH 3/9] Fix: type-checking --- app/core/users/endpoints_users.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/core/users/endpoints_users.py b/app/core/users/endpoints_users.py index 0e35a449bf..5782ac086d 100644 --- a/app/core/users/endpoints_users.py +++ b/app/core/users/endpoints_users.py @@ -642,6 +642,8 @@ 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("Invalid user ID") if user.should_change_password: # we control whether we check if the new password is different if security.verify_password( From 28de3812b69a930537585a74a37cf57024e06762 Mon Sep 17 00:00:00 2001 From: Marc-Andrieu <146140470+Marc-Andrieu@users.noreply.github.com> Date: Fri, 17 Oct 2025 11:40:28 +0200 Subject: [PATCH 4/9] wtf did I write --- app/core/users/endpoints_users.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/core/users/endpoints_users.py b/app/core/users/endpoints_users.py index 5782ac086d..b704e44238 100644 --- a/app/core/users/endpoints_users.py +++ b/app/core/users/endpoints_users.py @@ -643,7 +643,7 @@ 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("Invalid user ID") + 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( From 7d0b6a4dff484e852704126e550f9f20e475a01f Mon Sep 17 00:00:00 2001 From: Marc-Andrieu Date: Fri, 17 Oct 2025 11:23:50 +0200 Subject: [PATCH 5/9] mv /users/change-password with sibling endpoints on setting password --- app/core/users/endpoints_users.py | 114 +++++++++++++++--------------- 1 file changed, 57 insertions(+), 57 deletions(-) diff --git a/app/core/users/endpoints_users.py b/app/core/users/endpoints_users.py index b704e44238..611f3b682f 100644 --- a/app/core/users/endpoints_users.py +++ b/app/core/users/endpoints_users.py @@ -684,6 +684,63 @@ 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", + ) + + 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/migrate-mail", status_code=204, @@ -819,63 +876,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") - - 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", - ) - - 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() - - # 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" From a00ece48dd6f0e22ad67ac597ea0566a4db4789b Mon Sep 17 00:00:00 2001 From: Marc-Andrieu Date: Fri, 17 Oct 2025 11:57:12 +0200 Subject: [PATCH 6/9] Feat: admin endpoint to invalidate one user's passwd --- app/core/users/endpoints_users.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/app/core/users/endpoints_users.py b/app/core/users/endpoints_users.py index 611f3b682f..da41f98794 100644 --- a/app/core/users/endpoints_users.py +++ b/app/core/users/endpoints_users.py @@ -741,6 +741,21 @@ async def change_password( 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. + 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) + + @router.post( "/users/migrate-mail", status_code=204, From 7abf71b59cf0c483b03f3343e0ff52d8704b3ce1 Mon Sep 17 00:00:00 2001 From: Marc-Andrieu Date: Mon, 19 Jan 2026 14:16:18 +0100 Subject: [PATCH 7/9] Fix migration rebase --- ...{43-password_invalidation.py => 54-password_invalidation.py} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename migrations/versions/{43-password_invalidation.py => 54-password_invalidation.py} (96%) diff --git a/migrations/versions/43-password_invalidation.py b/migrations/versions/54-password_invalidation.py similarity index 96% rename from migrations/versions/43-password_invalidation.py rename to migrations/versions/54-password_invalidation.py index aa02eb937f..4c3e2ad914 100644 --- a/migrations/versions/43-password_invalidation.py +++ b/migrations/versions/54-password_invalidation.py @@ -14,7 +14,7 @@ # revision identifiers, used by Alembic. revision: str = "cfba514689ed" -down_revision: str | None = "c4812e1ab108" +down_revision: str | None = "9fc3dc926600" branch_labels: str | Sequence[str] | None = None depends_on: str | Sequence[str] | None = None From 8a9bca34616c6a2a6dc1f18e45fcc290027a0275 Mon Sep 17 00:00:00 2001 From: Marc-Andrieu Date: Wed, 21 Jan 2026 20:17:19 +0100 Subject: [PATCH 8/9] Coordination with CalypSSO --- app/core/auth/endpoints_auth.py | 19 ++++++++++++++++++ app/core/users/endpoints_users.py | 32 +++++++++++++++++-------------- requirements.txt | 2 +- 3 files changed, 38 insertions(+), 15 deletions(-) diff --git a/app/core/auth/endpoints_auth.py b/app/core/auth/endpoints_auth.py index df63af2336..bb108552af 100644 --- a/app/core/auth/endpoints_auth.py +++ b/app/core/auth/endpoints_auth.py @@ -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) @@ -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 ( diff --git a/app/core/users/endpoints_users.py b/app/core/users/endpoints_users.py index da41f98794..79f617a18c 100644 --- a/app/core/users/endpoints_users.py +++ b/app/core/users/endpoints_users.py @@ -708,15 +708,19 @@ async def change_password( 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", - ) + # # 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( @@ -724,11 +728,11 @@ async def change_password( 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, - ) + # 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 diff --git a/requirements.txt b/requirements.txt index f09c236b9d..4a51bce6a7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -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 From 5b37bd1a8d7eed0b3dda309e65af809fa6e2dca3 Mon Sep 17 00:00:00 2001 From: Marc Andrieu <146140470+Marc-Andrieu@users.noreply.github.com> Date: Sat, 7 Mar 2026 18:19:00 +0100 Subject: [PATCH 9/9] Fix: revoke refresh tokens when invalidating one's passwd --- app/core/users/endpoints_users.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/core/users/endpoints_users.py b/app/core/users/endpoints_users.py index 79f617a18c..a46185b73a 100644 --- a/app/core/users/endpoints_users.py +++ b/app/core/users/endpoints_users.py @@ -753,11 +753,16 @@ async def invalidate_password( ): """ 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(