Skip to content

Commit

Permalink
Merge commit from fork
Browse files Browse the repository at this point in the history
* HJ-172 - Password Policy is now Enforced in Accept Invite API

* Update changelog

* HJ-172 - Password Policy is now Enforced in Accept Invite API

* QA
  • Loading branch information
andres-torres-marroquin authored Nov 20, 2024
1 parent 30c4464 commit ce664da
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 20 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ The types of changes are:
### Docs
- Added docs for PrivacyNoticeRegion type [#5488](https://github.com/ethyca/fides/pull/5488)

### Security
- Password Policy is now Enforced in Accept Invite API [CVE-2024-52008](https://github.com/ethyca/fides/security/advisories/GHSA-v7vm-rhmg-8j2r)

## [2.49.1](https://github.com/ethyca/fidesplus/compare/2.49.0...2.49.1)

### Added
Expand Down
4 changes: 2 additions & 2 deletions src/fides/api/api/v1/endpoints/user_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ def update_user_password(
status_code=HTTP_401_UNAUTHORIZED, detail="Incorrect password."
)

current_user.update_password(db=db, new_password=b64_str_to_str(data.new_password))
current_user.update_password(db=db, new_password=data.new_password)

logger.info("Updated user with id: '{}'.", current_user.id)
return current_user
Expand Down Expand Up @@ -202,7 +202,7 @@ def force_update_password(
detail=f"User with ID {user_id} does not exist.",
)

user.update_password(db=db, new_password=b64_str_to_str(data.new_password))
user.update_password(db=db, new_password=data.new_password)
logger.info("Updated user with id: '{}'.", user.id)
return user

Expand Down
39 changes: 32 additions & 7 deletions src/fides/api/schemas/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from fides.api.schemas.oauth import AccessToken



class PrivacyRequestReviewer(FidesSchema):
"""Data we can expose via the PrivacyRequest.reviewer relation"""

Expand Down Expand Up @@ -40,19 +41,29 @@ def validate_username(cls, username: str) -> str:
def validate_password(cls, password: str) -> str:
"""Add some password requirements"""
decoded_password = decode_password(password)

if len(decoded_password) < 8:
return UserCreate._validate_password(decoded_password)

@staticmethod
def _validate_password(password: str) -> str:
"""
Validate password requirements.
Raises:
ValueError: If password does not meet requirements
Returns:
str: password
"""
if len(password) < 8:
raise ValueError("Password must have at least eight characters.")
if re.search("[0-9]", decoded_password) is None:
if re.search("[\d]", password) is None:
raise ValueError("Password must have at least one number.")
if re.search("[A-Z]", decoded_password) is None:
if re.search("[A-Z]", password) is None:
raise ValueError("Password must have at least one capital letter.")
if re.search("[a-z]", decoded_password) is None:
if re.search("[a-z]", password) is None:
raise ValueError("Password must have at least one lowercase letter.")
if re.search(r"[\W_]", decoded_password) is None:
if re.search(r"[\W_]", password) is None:
raise ValueError("Password must have at least one symbol.")

return decoded_password
return password


class UserCreateResponse(FidesSchema):
Expand Down Expand Up @@ -102,12 +113,26 @@ class UserPasswordReset(FidesSchema):
old_password: str
new_password: str

@field_validator("new_password")
@classmethod
def validate_new_password(cls, password: str) -> str:
"""Add some password requirements"""
decoded_password = decode_password(password)
return UserCreate._validate_password(decoded_password)


class UserForcePasswordReset(FidesSchema):
"""Only a new password, for the case where the user does not remember their password"""

new_password: str

@field_validator("new_password")
@classmethod
def validate_new_password(cls, password: str) -> str:
"""Add some password requirements"""
decoded_password = decode_password(password)
return UserCreate._validate_password(decoded_password)


class UserUpdate(FidesSchema):
"""Data required to update a FidesUser"""
Expand Down
62 changes: 51 additions & 11 deletions tests/ops/api/v1/endpoints/test_user_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -852,7 +852,7 @@ def test_update_different_user_password(
application_user,
) -> None:
OLD_PASSWORD = "oldpassword"
NEW_PASSWORD = "newpassword"
NEW_PASSWORD = "Newpassword1!"
application_user.update_password(db=db, new_password=OLD_PASSWORD)

auth_header = generate_auth_header_for_user(user=application_user, scopes=[])
Expand All @@ -874,15 +874,15 @@ def test_update_different_user_password(
application_user = application_user.refresh_from_db(db=db)
assert application_user.credentials_valid(password=OLD_PASSWORD)

def test_update_user_password_invalid(
def test_update_user_password_invalid_old_password(
self,
api_client,
db,
url_no_id,
application_user,
) -> None:
OLD_PASSWORD = "oldpassword"
NEW_PASSWORD = "newpassword"
NEW_PASSWORD = "Newpassword1!"
application_user.update_password(db=db, new_password=OLD_PASSWORD)

auth_header = generate_auth_header_for_user(user=application_user, scopes=[])
Expand All @@ -909,7 +909,7 @@ def test_update_user_password(
application_user,
) -> None:
OLD_PASSWORD = "oldpassword"
NEW_PASSWORD = "newpassword"
NEW_PASSWORD = "Newpassword1!"
application_user.update_password(db=db, new_password=OLD_PASSWORD)
auth_header = generate_auth_header_for_user(user=application_user, scopes=[])
resp = api_client.post(
Expand All @@ -934,7 +934,7 @@ def test_force_update_different_user_password_without_scope(
application_user,
) -> None:
"""A user without the proper scope cannot change another user's password"""
NEW_PASSWORD = "newpassword"
NEW_PASSWORD = "Newpassword1!"
old_hashed_password = user.hashed_password

auth_header = generate_auth_header_for_user(user=application_user, scopes=[])
Expand Down Expand Up @@ -965,7 +965,7 @@ def test_force_update_different_user_password(
A user with the right scope should be able to set a new password
for another user.
"""
NEW_PASSWORD = "newpassword"
NEW_PASSWORD = "Newpassword1!"
auth_header = generate_auth_header_for_user(
user=application_user, scopes=[USER_PASSWORD_RESET]
)
Expand All @@ -982,6 +982,46 @@ def test_force_update_different_user_password(
user = user.refresh_from_db(db=db)
assert user.credentials_valid(password=NEW_PASSWORD)

@pytest.mark.parametrize(
"new_password, expected_error",
[
("short", "Value error, Password must have at least eight characters."),
("longerpassword", "Value error, Password must have at least one number."),
("longer55password", "Value error, Password must have at least one capital letter."),
("LONGER55PASSWORD", "Value error, Password must have at least one lowercase letter."),
("LoNgEr55paSSworD", "Value error, Password must have at least one symbol."),
],
)
def test_force_update_bad_password(
self,
api_client,
db,
url_no_id,
user,
application_user,
new_password,
expected_error,
) -> None:
"""
A user with the right scope should be able to set a new password
for another user.
"""
auth_header = generate_auth_header_for_user(
user=application_user, scopes=[USER_PASSWORD_RESET]
)

resp = api_client.post(
f"{url_no_id}/{user.id}/force-reset-password",
headers=auth_header,
json={
"new_password": str_to_b64_str(new_password),
},
)

assert resp.status_code == HTTP_422_UNPROCESSABLE_ENTITY
assert expected_error in resp.json()["detail"][0]["msg"]
db.expunge(user)

def test_force_update_non_existent_user(
self,
api_client,
Expand All @@ -991,7 +1031,7 @@ def test_force_update_non_existent_user(
"""
Resetting on a user that does not exist should 404
"""
NEW_PASSWORD = "newpassword"
NEW_PASSWORD = "Newpassword1!"
auth_header = generate_auth_header_for_user(
user=application_user, scopes=[USER_PASSWORD_RESET]
)
Expand Down Expand Up @@ -1902,7 +1942,7 @@ def test_accept_invite_valid(
response = api_client.post(
url,
params={"username": "valid_user", "invite_code": "valid_code"},
json={"username": "valid_user", "new_password": "pass"},
json={"username": "valid_user", "new_password": "Testpassword1!"},
)

assert response.status_code == HTTP_200_OK
Expand All @@ -1925,7 +1965,7 @@ def test_accept_invite_invalid_code(self, db, api_client, url):
response = api_client.post(
url,
params={"username": "valid_user", "invite_code": "invalid_code"},
json={"username": "valid_user", "new_password": "pass"},
json={"username": "valid_user", "new_password": "Testpassword1!"},
)
assert response.status_code == HTTP_400_BAD_REQUEST
assert response.json()["detail"] == "Invite code is invalid."
Expand All @@ -1943,7 +1983,7 @@ def test_accept_invite_expired_code(self, mock_get_by, api_client: TestClient, u
response = api_client.post(
url,
params={"username": "valid_user", "invite_code": "expired_code"},
json={"username": "valid_user", "new_password": "pass"},
json={"username": "valid_user", "new_password": "Testpassword1!"},
)
assert response.status_code == HTTP_400_BAD_REQUEST
assert response.json()["detail"] == "Invite code has expired."
Expand All @@ -1954,7 +1994,7 @@ def test_accept_invite_nonexistent_user(self, api_client, url):
params={"username": "nonexistent_user", "invite_code": "some_code"},
json={
"username": "nonexistent_user",
"new_password": "pass",
"new_password": "Testpassword1!",
},
)
assert response.status_code == HTTP_404_NOT_FOUND
Expand Down

0 comments on commit ce664da

Please sign in to comment.