Skip to content

Commit

Permalink
fix: users should not be asked to reset password again right after th…
Browse files Browse the repository at this point in the history
…ey reset their password (#4672)
  • Loading branch information
RogerHYang authored Sep 19, 2024
1 parent ee3ef81 commit 7a67593
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 6 deletions.
9 changes: 9 additions & 0 deletions integration_tests/_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,15 @@ def _delete_api_key(
assert not headers.get("set-cookie")


def _will_be_asked_to_reset_password(
user: _User,
) -> bool:
query = "query($gid:GlobalID!){node(id:$gid){... on User{passwordNeedsReset}}}"
variables = dict(gid=user.gid)
resp_dict, _ = user.log_in().gql(query, variables)
return cast(bool, resp_dict["data"]["node"]["passwordNeedsReset"])


def _log_in(
password: _Password,
/,
Expand Down
26 changes: 20 additions & 6 deletions integration_tests/auth/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
_RoleOrUser,
_SpanExporterFactory,
_Username,
_will_be_asked_to_reset_password,
)

NOW = datetime.now(timezone.utc)
Expand Down Expand Up @@ -215,7 +216,9 @@ def test_initiate_password_reset_and_then_reset_password_using_token_from_email(
logged_in_user.create_api_key()
# new password should work
new_profile = replace(u.profile, password=new_password)
replace(u, profile=new_profile).log_in().create_api_key()
new_u = replace(u, profile=new_profile)
new_u.log_in()
assert not _will_be_asked_to_reset_password(new_u)

@pytest.mark.parametrize("role_or_user", [_MEMBER, _ADMIN])
def test_deleted_user_will_not_receive_email_after_initiating_password_reset(
Expand Down Expand Up @@ -394,6 +397,7 @@ def test_only_admin_can_create_user(
new_user = logged_in_user.create_user(role, profile=profile)
if not e:
new_user.log_in()
assert _will_be_asked_to_reset_password(new_user)

@pytest.mark.parametrize("role_or_user", [_ADMIN, _DEFAULT_ADMIN])
@pytest.mark.parametrize("role", list(UserRoleInput))
Expand Down Expand Up @@ -464,11 +468,17 @@ def test_change_password(
)
another_password = f"another_password_{next(_passwords)}"
with _EXPECTATION_401:
# old tokens should no longer work
_patch_viewer(old_token, new_password, new_password=another_password)
with _EXPECTATION_401:
_log_in(old_password, email=logged_in_user.email)
new_tokens = _log_in(new_password, email=logged_in_user.email)
# old password should no longer work
u.log_in()
new_profile = replace(u.profile, password=new_password)
new_u = replace(u, profile=new_profile)
new_tokens = new_u.log_in()
assert not _will_be_asked_to_reset_password(new_u)
with pytest.raises(Exception):
# old password should no longer work, even with new tokens
_patch_viewer(new_tokens, old_password, new_password=another_password)

@pytest.mark.parametrize("role", list(UserRoleInput))
Expand Down Expand Up @@ -559,12 +569,16 @@ def test_only_admin_can_change_password_for_non_self(
with expectation as e:
logged_in_user.patch_user(non_self, new_password=new_password)
if e:
# password should still work
non_self.log_in()
return
email = non_self.email
with _EXPECTATION_401:
_log_in(old_password, email=email)
_log_in(new_password, email=email)
# old password should no longer work
non_self.log_in()
new_profile = replace(non_self.profile, password=new_password)
new_non_self = replace(non_self, profile=new_profile)
new_non_self.log_in()
assert _will_be_asked_to_reset_password(new_non_self)

@pytest.mark.parametrize(
"role_or_user,expectation",
Expand Down
1 change: 1 addition & 0 deletions src/phoenix/server/api/routers/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ async def reset_password(request: Request) -> Response:
user.password_hash = await loop.run_in_executor(
None, partial(compute_password_hash, password=password, salt=user.password_salt)
)
user.reset_password = False
async with request.app.state.db() as session:
session.add(user)
await session.flush()
Expand Down

0 comments on commit 7a67593

Please sign in to comment.