From 87f270f8b6d81bfc073bed7160c12d422602635f Mon Sep 17 00:00:00 2001 From: Paul Sanders Date: Wed, 6 Jul 2022 23:29:21 -0400 Subject: [PATCH 1/4] Handle hashed password --- requirements.txt | 2 +- tests/api/v1/endpoints/test_user_endpoints.py | 52 ++++++++++--------- tests/models/test_client.py | 2 +- tests/scripts/test_create_superuser.py | 9 ++-- 4 files changed, 34 insertions(+), 31 deletions(-) diff --git a/requirements.txt b/requirements.txt index 3c6c2dfc2..0bc24bbd1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,7 +10,7 @@ fastapi-caching[redis] fastapi-pagination[sqlalchemy]~= 0.8.3 fastapi[all]==0.78.0 fideslang==1.0.0 -fideslib==2.1.1 +fideslib==2.2.1 fideslog==1.2.1 multidimensional_urlencode==0.0.4 pandas==1.3.3 diff --git a/tests/api/v1/endpoints/test_user_endpoints.py b/tests/api/v1/endpoints/test_user_endpoints.py index a27a86751..4a0600fe1 100644 --- a/tests/api/v1/endpoints/test_user_endpoints.py +++ b/tests/api/v1/endpoints/test_user_endpoints.py @@ -4,6 +4,7 @@ import pytest from fastapi_pagination import Params +from fideslib.cryptography.cryptographic_util import str_to_b64_str from fideslib.cryptography.schemas.jwt import ( JWE_ISSUED_AT, JWE_PAYLOAD_CLIENT_ID, @@ -70,7 +71,10 @@ def test_create_user_bad_username( url, ) -> None: auth_header = generate_auth_header([USER_CREATE]) - body = {"username": "spaces in name", "password": "TestP@ssword9"} + body = { + "username": "spaces in name", + "password": str_to_b64_str("TestP@ssword9"), + } response = api_client.post(url, headers=auth_header, json=body) assert HTTP_422_UNPROCESSABLE_ENTITY == response.status_code @@ -84,7 +88,7 @@ def test_username_exists( ) -> None: auth_header = generate_auth_header([USER_CREATE]) - body = {"username": "test_user", "password": "TestP@ssword9"} + body = {"username": "test_user", "password": str_to_b64_str("TestP@ssword9")} user = FidesUser.create(db=db, data=body) response = api_client.post(url, headers=auth_header, json=body) @@ -103,7 +107,7 @@ def test_create_user_bad_password( ) -> None: auth_header = generate_auth_header([USER_CREATE]) - body = {"username": "test_user", "password": "short"} + body = {"username": "test_user", "password": str_to_b64_str("short")} response = api_client.post(url, headers=auth_header, json=body) assert HTTP_422_UNPROCESSABLE_ENTITY == response.status_code assert ( @@ -111,7 +115,7 @@ def test_create_user_bad_password( == "Password must have at least eight characters." ) - body = {"username": "test_user", "password": "longerpassword"} + body = {"username": "test_user", "password": str_to_b64_str("longerpassword")} response = api_client.post(url, headers=auth_header, json=body) assert HTTP_422_UNPROCESSABLE_ENTITY == response.status_code assert ( @@ -119,7 +123,7 @@ def test_create_user_bad_password( == "Password must have at least one number." ) - body = {"username": "test_user", "password": "longer55password"} + body = {"username": "test_user", "password": str_to_b64_str("longer55password")} response = api_client.post(url, headers=auth_header, json=body) assert HTTP_422_UNPROCESSABLE_ENTITY == response.status_code assert ( @@ -127,7 +131,7 @@ def test_create_user_bad_password( == "Password must have at least one capital letter." ) - body = {"username": "test_user", "password": "LoNgEr55paSSworD"} + body = {"username": "test_user", "password": str_to_b64_str("LoNgEr55paSSworD")} response = api_client.post(url, headers=auth_header, json=body) assert HTTP_422_UNPROCESSABLE_ENTITY == response.status_code assert ( @@ -143,7 +147,7 @@ def test_create_user( url, ) -> None: auth_header = generate_auth_header([USER_CREATE]) - body = {"username": "test_user", "password": "TestP@ssword9"} + body = {"username": "test_user", "password": str_to_b64_str("TestP@ssword9")} response = api_client.post(url, headers=auth_header, json=body) @@ -164,7 +168,7 @@ def test_create_user_with_name( auth_header = generate_auth_header([USER_CREATE]) body = { "username": "test_user", - "password": "TestP@ssword9", + "password": str_to_b64_str("TestP@ssword9"), "first_name": "Test", "last_name": "User", } @@ -211,7 +215,7 @@ def test_delete_self(self, api_client, db, generate_auth_header): db=db, data={ "username": "test_delete_user", - "password": "TESTdcnG@wzJeu0&%3Qe2fGo7", + "password": str_to_b64_str("TESTdcnG@wzJeu0&%3Qe2fGo7"), }, ) saved_user_id = user.id @@ -264,7 +268,7 @@ def test_delete_user_as_root(self, api_client, db, generate_auth_header, user): db=db, data={ "username": "test_delete_user", - "password": "TESTdcnG@wzJeu0&%3Qe2fGo7", + "password": str_to_b64_str("TESTdcnG@wzJeu0&%3Qe2fGo7"), }, ) @@ -362,7 +366,7 @@ def test_get_users(self, api_client: TestClient, generate_auth_header, url, db): for i in range(total_users): body = { "username": f"user{i}@example.com", - "password": "Password123!", + "password": str_to_b64_str("Password123!"), "first_name": "Test", "last_name": "User", } @@ -399,7 +403,7 @@ def test_get_filtered_users( for i in range(total_users): body = { "username": f"user{i}@example.com", - "password": "Password123!", + "password": str_to_b64_str("Password123!"), } resp = api_client.post(url, headers=create_auth_header, json=body) assert resp.status_code == HTTP_201_CREATED @@ -577,8 +581,8 @@ def test_update_different_user_password( f"{url_no_id}/{user.id}/reset-password", headers=auth_header, json={ - "old_password": OLD_PASSWORD, - "new_password": NEW_PASSWORD, + "old_password": str_to_b64_str(OLD_PASSWORD), + "new_password": str_to_b64_str(NEW_PASSWORD), }, ) assert resp.status_code == HTTP_401_UNAUTHORIZED @@ -610,8 +614,8 @@ def test_update_user_password_invalid( f"{url_no_id}/{application_user.id}/reset-password", headers=auth_header, json={ - "old_password": "mismatching password", - "new_password": NEW_PASSWORD, + "old_password": str_to_b64_str("mismatching password"), + "new_password": str_to_b64_str(NEW_PASSWORD), }, ) assert resp.status_code == HTTP_401_UNAUTHORIZED @@ -631,7 +635,6 @@ def test_update_user_password( OLD_PASSWORD = "oldpassword" NEW_PASSWORD = "newpassword" application_user.update_password(db=db, new_password=OLD_PASSWORD) - auth_header = generate_auth_header_for_user( user=application_user, scopes=[USER_PASSWORD_RESET], @@ -640,12 +643,11 @@ def test_update_user_password( f"{url_no_id}/{application_user.id}/reset-password", headers=auth_header, json={ - "old_password": OLD_PASSWORD, - "new_password": NEW_PASSWORD, + "old_password": str_to_b64_str(OLD_PASSWORD), + "new_password": str_to_b64_str(NEW_PASSWORD), }, ) assert resp.status_code == HTTP_200_OK - db.expunge(application_user) application_user = application_user.refresh_from_db(db=db) assert application_user.credentials_valid(password=NEW_PASSWORD) @@ -659,7 +661,7 @@ def url(self, oauth_client: ClientDetail) -> str: def test_user_does_not_exist(self, db, url, api_client): body = { "username": "does not exist", - "password": "idonotknowmypassword", + "password": str_to_b64_str("idonotknowmypassword"), } response = api_client.post(url, headers={}, json=body) assert response.status_code == HTTP_404_NOT_FOUND @@ -667,7 +669,7 @@ def test_user_does_not_exist(self, db, url, api_client): def test_bad_login(self, db, url, user, api_client): body = { "username": user.username, - "password": "idonotknowmypassword", + "password": str_to_b64_str("idonotknowmypassword"), } response = api_client.post(url, headers={}, json=body) assert response.status_code == HTTP_403_FORBIDDEN @@ -677,7 +679,7 @@ def test_login_creates_client(self, db, url, user, api_client): user.client.delete(db) body = { "username": user.username, - "password": "TESTdcnG@wzJeu0&%3Qe2fGo7", + "password": str_to_b64_str("TESTdcnG@wzJeu0&%3Qe2fGo7"), } assert user.client is None # client does not exist @@ -705,7 +707,7 @@ def test_login_creates_client(self, db, url, user, api_client): def test_login_updates_last_login_date(self, db, url, user, api_client): body = { "username": user.username, - "password": "TESTdcnG@wzJeu0&%3Qe2fGo7", + "password": str_to_b64_str("TESTdcnG@wzJeu0&%3Qe2fGo7"), } response = api_client.post(url, headers={}, json=body) @@ -717,7 +719,7 @@ def test_login_updates_last_login_date(self, db, url, user, api_client): def test_login_uses_existing_client(self, db, url, user, api_client): body = { "username": user.username, - "password": "TESTdcnG@wzJeu0&%3Qe2fGo7", + "password": str_to_b64_str("TESTdcnG@wzJeu0&%3Qe2fGo7"), } existing_client_id = user.client.id diff --git a/tests/models/test_client.py b/tests/models/test_client.py index 5bd65b3fe..836c6f153 100644 --- a/tests/models/test_client.py +++ b/tests/models/test_client.py @@ -1,9 +1,9 @@ +from fideslib.cryptography.cryptographic_util import hash_with_salt from fideslib.models.client import ClientDetail from sqlalchemy.orm import Session from fidesops.api.v1.scope_registry import SCOPE_REGISTRY from fidesops.core.config import config -from fidesops.util.cryptographic_util import hash_with_salt class TestClientModel: diff --git a/tests/scripts/test_create_superuser.py b/tests/scripts/test_create_superuser.py index 5982ad89d..a759bf35b 100644 --- a/tests/scripts/test_create_superuser.py +++ b/tests/scripts/test_create_superuser.py @@ -2,6 +2,7 @@ import pytest from create_superuser import collect_username_and_password, create_user_and_client +from fideslib.cryptography.cryptographic_util import str_to_b64_str from fideslib.exceptions import KeyOrNameAlreadyExists from fideslib.models.client import ADMIN_UI_ROOT, ClientDetail from fideslib.models.fides_user import FidesUser @@ -23,7 +24,7 @@ def test_collect_username_and_password( db, ): GENERIC_INPUT = "some_input" - mock_pass.return_value = "TESTP@ssword9" + mock_pass.return_value = str_to_b64_str("TESTP@ssword9") mock_user.return_value = "test_user" mock_input.return_value = GENERIC_INPUT user: UserCreate = collect_username_and_password(db) @@ -47,7 +48,7 @@ def test_collect_username_and_password_user_exists( db=db, data={"username": "test_user", "password": "test_password"}, ) - mock_pass.return_value = "TESTP@ssword9" + mock_pass.return_value = str_to_b64_str("TESTP@ssword9") mock_user.return_value = "test_user" mock_input.return_value = "some_input" @@ -66,7 +67,7 @@ def test_collect_username_and_password_bad_data( mock_user, db, ): - mock_pass.return_value = "bad_password" + mock_pass.return_value = str_to_b64_str("bad_password") mock_user.return_value = "test_user" mock_input.return_value = "some_input" @@ -83,7 +84,7 @@ def test_create_user_and_client( mock_user, db, ): - mock_pass.return_value = "TESTP@ssword9" + mock_pass.return_value = str_to_b64_str("TESTP@ssword9") mock_user.return_value = "test_user" mock_input.return_value = "some_input" From dc0f2fb1a07e074cf1b1990e6f2cd518b695ff5e Mon Sep 17 00:00:00 2001 From: Paul Sanders Date: Wed, 6 Jul 2022 23:34:07 -0400 Subject: [PATCH 2/4] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a9f7af30b..3de0dfba8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ The types of changes are: * Resolve issue with MyPy seeing files in fidesops as missing imports [#719](https://github.com/ethyca/fidesops/pull/719) * Fixed `check-migrations` Make command [#806](https://github.com/ethyca/fidesops/pull/806) * Fix issue requiring separate install of snowflake-connector-python [#807](https://github.com/ethyca/fidesops/pull/807) +* Bump fideslib to handle base64 encoded password [#820](https://github.com/ethyca/fidesops/pull/820) ## [1.6.1](https://github.com/ethyca/fidesops/compare/1.6.0...1.6.1) From d3b4e9f7805ec910f6eb4447b4e934e760d4075a Mon Sep 17 00:00:00 2001 From: Paul Sanders Date: Thu, 7 Jul 2022 09:51:23 -0400 Subject: [PATCH 3/4] Fix failing test and use UserPasswordReset schema from fideslib --- fidesops.toml | 1 + src/fidesops/api/v1/endpoints/user_endpoints.py | 9 ++++++--- src/fidesops/schemas/user.py | 8 -------- 3 files changed, 7 insertions(+), 11 deletions(-) delete mode 100644 src/fidesops/schemas/user.py diff --git a/fidesops.toml b/fidesops.toml index d9c17c75c..faf4f2769 100644 --- a/fidesops.toml +++ b/fidesops.toml @@ -37,6 +37,7 @@ TASK_RETRY_BACKOFF = 1 [root_user] ANALYTICS_OPT_OUT = false +ANALYTICS_ID = "eaa09bfca9b196bafe17fd4e85ff72ef" [admin_ui] ENABLED = true diff --git a/src/fidesops/api/v1/endpoints/user_endpoints.py b/src/fidesops/api/v1/endpoints/user_endpoints.py index 92ca47651..150f295ad 100644 --- a/src/fidesops/api/v1/endpoints/user_endpoints.py +++ b/src/fidesops/api/v1/endpoints/user_endpoints.py @@ -6,6 +6,7 @@ from fastapi_pagination import Page, Params from fastapi_pagination.bases import AbstractPage from fastapi_pagination.ext.sqlalchemy import paginate +from fideslib.cryptography.cryptographic_util import b64_str_to_str from fideslib.models.client import ADMIN_UI_ROOT, ClientDetail from fideslib.models.fides_user import FidesUser from fideslib.models.fides_user_permissions import FidesUserPermissions @@ -15,6 +16,7 @@ UserCreateResponse, UserLogin, UserLoginResponse, + UserPasswordReset, UserResponse, UserUpdate, ) @@ -42,7 +44,6 @@ ) from fidesops.api.v1.urn_registry import V1_URL_PREFIX from fidesops.core.config import config -from fidesops.schemas.user import UserPasswordReset from fidesops.util.oauth_util import get_current_user, verify_oauth_client logger = logging.getLogger(__name__) @@ -155,12 +156,14 @@ def update_user_password( """ _validate_current_user(user_id, current_user) - if not current_user.credentials_valid(data.old_password): + if not current_user.credentials_valid( + b64_str_to_str(data.old_password), config.security.ENCODING + ): raise HTTPException( status_code=HTTP_401_UNAUTHORIZED, detail="Incorrect password." ) - current_user.update_password(db=db, new_password=data.new_password) + current_user.update_password(db=db, new_password=b64_str_to_str(data.new_password)) logger.info(f"Updated user with id: '{current_user.id}'.") return current_user diff --git a/src/fidesops/schemas/user.py b/src/fidesops/schemas/user.py deleted file mode 100644 index fb939fa1d..000000000 --- a/src/fidesops/schemas/user.py +++ /dev/null @@ -1,8 +0,0 @@ -from fideslib.schemas.base_class import BaseSchema - - -class UserPasswordReset(BaseSchema): - """Contains both old and new passwords when resetting a password""" - - old_password: str - new_password: str From c5471ff88871707d1000efcf186131a4fc726c29 Mon Sep 17 00:00:00 2001 From: Paul Sanders Date: Thu, 7 Jul 2022 10:01:36 -0400 Subject: [PATCH 4/4] Restore fidesops.toml --- fidesops.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/fidesops.toml b/fidesops.toml index 2347b0673..1ce87750c 100644 --- a/fidesops.toml +++ b/fidesops.toml @@ -38,7 +38,6 @@ WORKER_ENABLED = false [root_user] ANALYTICS_OPT_OUT = false -ANALYTICS_ID = "eaa09bfca9b196bafe17fd4e85ff72ef" [admin_ui] ENABLED = true