From d5764346fbe0551fe4099f889f0d5071ebb53313 Mon Sep 17 00:00:00 2001 From: Sean Preston Date: Thu, 5 May 2022 12:00:50 -0400 Subject: [PATCH] [#446] Adds `first_name`, `last_name`, `created_at` to `FidesopsUser` response (#465) * add fixture for user, add first_name and last_name to FidesopsUser, add fields to response * run autoformatter on migrations * make username unique, rename fixture * test multiple user get endpoint --- src/fidesops/models/fidesops_user.py | 4 + src/fidesops/schemas/user.py | 7 + ..._adds_first_name_and_last_name_to_user_.py | 30 ++++ tests/api/v1/endpoints/test_user_endpoints.py | 104 +++++++++-- .../test_user_permission_endpoints.py | 169 ++++++++++-------- tests/fixtures/application_fixtures.py | 20 ++- 6 files changed, 236 insertions(+), 98 deletions(-) create mode 100644 src/migrations/versions/29a7d707163a_adds_first_name_and_last_name_to_user_.py diff --git a/src/fidesops/models/fidesops_user.py b/src/fidesops/models/fidesops_user.py index 4a211f144..4f9d34559 100644 --- a/src/fidesops/models/fidesops_user.py +++ b/src/fidesops/models/fidesops_user.py @@ -13,6 +13,8 @@ class FidesopsUser(Base): """The DB ORM model for FidesopsUser""" username = Column(String, unique=True, index=True) + first_name = Column(String, nullable=True) + last_name = Column(String, nullable=True) hashed_password = Column(String, nullable=False) salt = Column(String, nullable=False) last_login_at = Column(DateTime(timezone=True), nullable=True) @@ -44,6 +46,8 @@ def create(cls, db: Session, data: Dict[str, Any]) -> "FidesopsUser": "salt": salt, "hashed_password": hashed_password, "username": data["username"], + "first_name": data.get("first_name"), + "last_name": data.get("last_name"), }, ) diff --git a/src/fidesops/schemas/user.py b/src/fidesops/schemas/user.py index 5e5f5289d..533471262 100644 --- a/src/fidesops/schemas/user.py +++ b/src/fidesops/schemas/user.py @@ -1,4 +1,6 @@ +from datetime import datetime import re +from typing import Optional from pydantic import validator from fidesops.schemas.base_class import BaseSchema @@ -9,6 +11,8 @@ class UserCreate(BaseSchema): username: str password: str + first_name: Optional[str] + last_name: Optional[str] @validator("username") def validate_username(cls, username: str) -> str: @@ -48,6 +52,9 @@ class UserResponse(BaseSchema): id: str username: str + created_at: datetime + first_name: Optional[str] + last_name: Optional[str] class UserCreateResponse(BaseSchema): diff --git a/src/migrations/versions/29a7d707163a_adds_first_name_and_last_name_to_user_.py b/src/migrations/versions/29a7d707163a_adds_first_name_and_last_name_to_user_.py new file mode 100644 index 000000000..d95205746 --- /dev/null +++ b/src/migrations/versions/29a7d707163a_adds_first_name_and_last_name_to_user_.py @@ -0,0 +1,30 @@ +"""adds first_name and last_name to user model + +Revision ID: 29a7d707163a +Revises: 90070db16d05 +Create Date: 2022-05-05 13:41:28.807920 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = "29a7d707163a" +down_revision = "90070db16d05" +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column("fidesopsuser", sa.Column("first_name", sa.String(), nullable=True)) + op.add_column("fidesopsuser", sa.Column("last_name", sa.String(), nullable=True)) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column("fidesopsuser", "last_name") + op.drop_column("fidesopsuser", "first_name") + # ### end Alembic commands ### diff --git a/tests/api/v1/endpoints/test_user_endpoints.py b/tests/api/v1/endpoints/test_user_endpoints.py index bc31b0030..727c595aa 100644 --- a/tests/api/v1/endpoints/test_user_endpoints.py +++ b/tests/api/v1/endpoints/test_user_endpoints.py @@ -14,10 +14,16 @@ HTTP_401_UNAUTHORIZED, HTTP_400_BAD_REQUEST, HTTP_422_UNPROCESSABLE_ENTITY, - HTTP_404_NOT_FOUND + HTTP_404_NOT_FOUND, ) -from fidesops.api.v1.urn_registry import V1_URL_PREFIX, USERS, LOGIN, LOGOUT, USER_DETAIL +from fidesops.api.v1.urn_registry import ( + V1_URL_PREFIX, + USERS, + LOGIN, + LOGOUT, + USER_DETAIL, +) from fidesops.models.client import ClientDetail, ADMIN_UI_ROOT from fidesops.api.v1.scope_registry import ( STORAGE_READ, @@ -38,6 +44,7 @@ page_size = Params().size + class TestCreateUser: @pytest.fixture(scope="function") def url(self, oauth_client: ClientDetail) -> str: @@ -142,8 +149,30 @@ def test_create_user( assert HTTP_201_CREATED == response.status_code assert response_body == {"id": user.id} assert user.permissions is not None + user.delete(db) + + def test_create_user_with_name( + self, + db, + api_client, + generate_auth_header, + url, + ) -> None: + auth_header = generate_auth_header([USER_CREATE]) + body = { + "username": "test_user", + "password": "TestP@ssword9", + "first_name": "Test", + "last_name": "User", + } + response = api_client.post(url, headers=auth_header, json=body) + user = FidesopsUser.get_by(db, field="username", value=body["username"]) + response_body = json.loads(response.text) + assert HTTP_201_CREATED == response.status_code + assert response_body == {"id": user.id} + assert user.permissions is not None user.delete(db) @@ -191,7 +220,6 @@ def test_delete_self(self, api_client, db, generate_auth_header): assert user.permissions is not None saved_permissions_id = user.permissions.id - client, _ = ClientDetail.create_client_and_secret( db, [USER_DELETE], user_id=user.id ) @@ -219,7 +247,9 @@ def test_delete_self(self, api_client, db, generate_auth_header): client_search = ClientDetail.get_by(db, field="id", value=saved_client_id) assert client_search is None - permissions_search = FidesopsUserPermissions.get_by(db, field="id", value=saved_permissions_id) + permissions_search = FidesopsUserPermissions.get_by( + db, field="id", value=saved_permissions_id + ) assert permissions_search is None def test_delete_user_as_root(self, api_client, db, generate_auth_header, user): @@ -269,7 +299,9 @@ def test_delete_user_as_root(self, api_client, db, generate_auth_header, user): client_search = ClientDetail.get_by(db, field="id", value=client_id) assert client_search is None - permissions_search = FidesopsUserPermissions.get_by(db, field="id", value=saved_permission_id) + permissions_search = FidesopsUserPermissions.get_by( + db, field="id", value=saved_permission_id + ) assert permissions_search is None # Deleted user's client is also deleted @@ -287,11 +319,15 @@ class TestGetUsers: def url(self, oauth_client: ClientDetail) -> str: return V1_URL_PREFIX + USERS - def test_get_users_not_authenticated(self, api_client: TestClient, url: str)-> None: + def test_get_users_not_authenticated( + self, api_client: TestClient, url: str + ) -> None: resp = api_client.get(url, headers={}) assert resp.status_code == HTTP_401_UNAUTHORIZED - def test_get_users_wrong_scope(self, api_client: TestClient, generate_auth_header, url): + def test_get_users_wrong_scope( + self, api_client: TestClient, generate_auth_header, url + ): auth_header = generate_auth_header(scopes=[USER_DELETE]) resp = api_client.get(url, headers=auth_header) assert resp.status_code == HTTP_403_FORBIDDEN @@ -308,13 +344,17 @@ def test_get_users_no_users( assert response_body["page"] == 1 assert response_body["size"] == page_size - - def test_get_users(self, api_client:TestClient, generate_auth_header, url, db): + def test_get_users(self, api_client: TestClient, generate_auth_header, url, db): create_auth_header = generate_auth_header(scopes=[USER_CREATE]) saved_users: List[FidesopsUser] = [] total_users = 25 for i in range(total_users): - body = {"username": f"user{i}@example.com", "password": "Password123!"} + body = { + "username": f"user{i}@example.com", + "password": "Password123!", + "first_name": "Test", + "last_name": "User", + } resp = api_client.post(url, headers=create_auth_header, json=body) assert resp.status_code == HTTP_201_CREATED user = FidesopsUser.get_by(db, field="username", value=body["username"]) @@ -329,11 +369,19 @@ def test_get_users(self, api_client:TestClient, generate_auth_header, url, db): assert response_body["page"] == 1 assert response_body["size"] == page_size + user_data = response_body["items"][0] + assert user_data["username"] + assert user_data["id"] + assert user_data["created_at"] + assert user_data["first_name"] + assert user_data["last_name"] + for i in range(total_users): saved_users[i].delete(db) - - def test_get_filtered_users(self, api_client:TestClient, generate_auth_header, url, db): + def test_get_filtered_users( + self, api_client: TestClient, generate_auth_header, url, db + ): create_auth_header = generate_auth_header(scopes=[USER_CREATE]) saved_users: List[FidesopsUser] = [] total_users = 50 @@ -370,12 +418,10 @@ def test_get_filtered_users(self, api_client:TestClient, generate_auth_header, u assert response_body["page"] == 1 assert response_body["size"] == page_size - for i in range(total_users): saved_users[i].delete(db) - class TestGetUser: @pytest.fixture(scope="function") def url(self, oauth_client: ClientDetail) -> str: @@ -389,7 +435,9 @@ def test_get_user_not_authenticated(self, api_client: TestClient, url: str) -> N resp = api_client.get(url, headers={}) assert resp.status_code == HTTP_401_UNAUTHORIZED - def test_get_user_wrong_scope(self, api_client: TestClient, generate_auth_header, url: str): + def test_get_user_wrong_scope( + self, api_client: TestClient, generate_auth_header, url: str + ): auth_header = generate_auth_header(scopes=[USER_DELETE]) resp = api_client.get(url, headers=auth_header) assert resp.status_code == HTTP_403_FORBIDDEN @@ -404,6 +452,27 @@ def test_get_user_does_not_exist( ) assert resp.status_code == HTTP_404_NOT_FOUND + def test_get_user( + self, + api_client: TestClient, + generate_auth_header, + url_no_id: str, + application_user, + ) -> None: + auth_header = generate_auth_header(scopes=[USER_READ]) + resp = api_client.get( + f"{url_no_id}/{application_user.id}", + headers=auth_header, + ) + assert resp.status_code == HTTP_200_OK + user_data = resp.json() + assert user_data["username"] == application_user.username + assert user_data["id"] == application_user.id + assert user_data["created_at"] == application_user.created_at.isoformat() + assert user_data["first_name"] == application_user.first_name + assert user_data["last_name"] == application_user.last_name + + class TestUserLogin: @pytest.fixture(scope="function") def url(self, oauth_client: ClientDetail) -> str: @@ -501,7 +570,9 @@ def test_user_not_deleted_on_logout(self, db, url, api_client, user): assert user_search is not None # Assert user permissions are not deleted - permission_search = FidesopsUserPermissions.get_by(db, field="user_id", value=user_id) + permission_search = FidesopsUserPermissions.get_by( + db, field="user_id", value=user_id + ) assert permission_search is not None # Assert user does not still have client reference @@ -518,7 +589,6 @@ def test_user_not_deleted_on_logout(self, db, url, api_client, user): response = api_client.post(url, headers=auth_header, json={}) assert HTTP_403_FORBIDDEN == response.status_code - def test_logout(self, db, url, api_client, generate_auth_header, oauth_client): oauth_client_id = oauth_client.id auth_header = generate_auth_header([STORAGE_READ]) diff --git a/tests/api/v1/endpoints/test_user_permission_endpoints.py b/tests/api/v1/endpoints/test_user_permission_endpoints.py index f1e3299b5..3f2ae09ea 100644 --- a/tests/api/v1/endpoints/test_user_permission_endpoints.py +++ b/tests/api/v1/endpoints/test_user_permission_endpoints.py @@ -1,11 +1,22 @@ import pytest import json -from starlette.status import HTTP_401_UNAUTHORIZED, HTTP_403_FORBIDDEN, HTTP_422_UNPROCESSABLE_ENTITY, HTTP_201_CREATED, \ - HTTP_404_NOT_FOUND, HTTP_200_OK +from starlette.status import ( + HTTP_401_UNAUTHORIZED, + HTTP_403_FORBIDDEN, + HTTP_422_UNPROCESSABLE_ENTITY, + HTTP_201_CREATED, + HTTP_404_NOT_FOUND, + HTTP_200_OK, +) from fidesops.api.v1.urn_registry import V1_URL_PREFIX, USER_PERMISSIONS -from fidesops.api.v1.scope_registry import USER_PERMISSION_CREATE, USER_PERMISSION_READ, USER_PERMISSION_UPDATE, \ - SAAS_CONFIG_READ, PRIVACY_REQUEST_READ +from fidesops.api.v1.scope_registry import ( + USER_PERMISSION_CREATE, + USER_PERMISSION_READ, + USER_PERMISSION_UPDATE, + SAAS_CONFIG_READ, + PRIVACY_REQUEST_READ, +) from fidesops.models.client import ClientDetail from fidesops.models.fidesops_user import FidesopsUser from fidesops.models.fidesops_user_permissions import FidesopsUserPermissions @@ -20,18 +31,20 @@ def test_create_user_permissions_not_authenticated(self, url, api_client): response = api_client.post(url, headers={}, json={}) assert HTTP_401_UNAUTHORIZED == response.status_code - def test_create_user_permissions_wrong_scope(self, url, api_client, generate_auth_header): + def test_create_user_permissions_wrong_scope( + self, url, api_client, generate_auth_header + ): auth_header = generate_auth_header([SAAS_CONFIG_READ]) response = api_client.post(url, headers=auth_header, json={}) assert HTTP_403_FORBIDDEN == response.status_code def test_create_user_permissions_invalid_scope( - self, - db, - api_client, - generate_auth_header, - user, - url, + self, + db, + api_client, + generate_auth_header, + user, + url, ) -> None: auth_header = generate_auth_header([USER_PERMISSION_CREATE]) user = FidesopsUser.create( @@ -39,31 +52,27 @@ def test_create_user_permissions_invalid_scope( data={"username": "user_1", "password": "test_password"}, ) - body = {"user_id": user.id, "scopes": ['not a real scope']} + body = {"user_id": user.id, "scopes": ["not a real scope"]} response = api_client.post(url, headers=auth_header, json=body) assert HTTP_422_UNPROCESSABLE_ENTITY == response.status_code user.delete(db) def test_create_user_permissions_invalid_user_id( - self, - db, - api_client, - generate_auth_header + self, db, api_client, generate_auth_header ) -> None: auth_header = generate_auth_header([USER_PERMISSION_CREATE]) user_id = "bogus_user_id" body = {"user_id": user_id, "scopes": [PRIVACY_REQUEST_READ]} - response = api_client.post(f"{V1_URL_PREFIX}/user/{user_id}/permission", headers=auth_header, json=body) + response = api_client.post( + f"{V1_URL_PREFIX}/user/{user_id}/permission", headers=auth_header, json=body + ) permissions = FidesopsUserPermissions.get_by(db, field="user_id", value=user_id) assert HTTP_404_NOT_FOUND == response.status_code assert permissions is None def test_create_user_permissions( - self, - db, - api_client, - generate_auth_header + self, db, api_client, generate_auth_header ) -> None: auth_header = generate_auth_header([USER_PERMISSION_CREATE]) user = FidesopsUser.create( @@ -72,11 +81,13 @@ def test_create_user_permissions( ) body = {"user_id": user.id, "scopes": [PRIVACY_REQUEST_READ]} - response = api_client.post(f"{V1_URL_PREFIX}/user/{user.id}/permission", headers=auth_header, json=body) + response = api_client.post( + f"{V1_URL_PREFIX}/user/{user.id}/permission", headers=auth_header, json=body + ) permissions = FidesopsUserPermissions.get_by(db, field="user_id", value=user.id) response_body = json.loads(response.text) assert HTTP_201_CREATED == response.status_code - assert response_body['id'] == permissions.id + assert response_body["id"] == permissions.id assert permissions.scopes == [PRIVACY_REQUEST_READ] user.delete(db) @@ -90,30 +101,29 @@ def test_edit_user_permissions_not_authenticated(self, url, api_client): response = api_client.put(url, headers={}, json={}) assert HTTP_401_UNAUTHORIZED == response.status_code - def test_edit_user_permissions_wrong_scope(self, url, api_client, generate_auth_header): + def test_edit_user_permissions_wrong_scope( + self, url, api_client, generate_auth_header + ): auth_header = generate_auth_header([SAAS_CONFIG_READ]) response = api_client.put(url, headers=auth_header, json={}) assert HTTP_403_FORBIDDEN == response.status_code def test_edit_user_permissions_invalid_scope( - self, - db, - api_client, - generate_auth_header, - url, + self, + db, + api_client, + generate_auth_header, + url, ) -> None: auth_header = generate_auth_header([USER_PERMISSION_UPDATE]) - body = {"user_id": "bogus_user_id", "scopes": ['not a real scope']} + body = {"user_id": "bogus_user_id", "scopes": ["not a real scope"]} response = api_client.put(url, headers=auth_header, json=body) assert HTTP_422_UNPROCESSABLE_ENTITY == response.status_code def test_edit_user_permissions_invalid_user_id( - self, - db, - api_client, - generate_auth_header + self, db, api_client, generate_auth_header ) -> None: auth_header = generate_auth_header([USER_PERMISSION_UPDATE]) invalid_user_id = "bogus_user_id" @@ -122,33 +132,32 @@ def test_edit_user_permissions_invalid_user_id( data={"username": "user_1", "password": "test_password"}, ) - permissions = FidesopsUserPermissions.create(db=db, data={ - "user_id": user.id, - "scopes": [PRIVACY_REQUEST_READ] - }) + permissions = FidesopsUserPermissions.create( + db=db, data={"user_id": user.id, "scopes": [PRIVACY_REQUEST_READ]} + ) body = {"id": permissions.id, "scopes": [PRIVACY_REQUEST_READ]} - response = api_client.put(f"{V1_URL_PREFIX}/user/{invalid_user_id}/permission", headers=auth_header, json=body) - permissions = FidesopsUserPermissions.get_by(db, field="user_id", value=invalid_user_id) + response = api_client.put( + f"{V1_URL_PREFIX}/user/{invalid_user_id}/permission", + headers=auth_header, + json=body, + ) + permissions = FidesopsUserPermissions.get_by( + db, field="user_id", value=invalid_user_id + ) assert HTTP_404_NOT_FOUND == response.status_code assert permissions is None user.delete(db) - def test_edit_user_permissions( - self, - db, - api_client, - generate_auth_header - ) -> None: + def test_edit_user_permissions(self, db, api_client, generate_auth_header) -> None: auth_header = generate_auth_header([USER_PERMISSION_UPDATE]) user = FidesopsUser.create( db=db, data={"username": "user_1", "password": "test_password"}, ) - permissions = FidesopsUserPermissions.create(db=db, data={ - "user_id": user.id, - "scopes": [PRIVACY_REQUEST_READ] - }) + permissions = FidesopsUserPermissions.create( + db=db, data={"user_id": user.id, "scopes": [PRIVACY_REQUEST_READ]} + ) ClientDetail.create_client_and_secret( db, [PRIVACY_REQUEST_READ], user_id=user.id @@ -156,12 +165,14 @@ def test_edit_user_permissions( updated_scopes = [PRIVACY_REQUEST_READ, SAAS_CONFIG_READ] body = {"id": permissions.id, "scopes": updated_scopes} - response = api_client.put(f"{V1_URL_PREFIX}/user/{user.id}/permission", headers=auth_header, json=body) + response = api_client.put( + f"{V1_URL_PREFIX}/user/{user.id}/permission", headers=auth_header, json=body + ) response_body = json.loads(response.text) - client: ClientDetail = ClientDetail.get_by(db, field='user_id', value=user.id) + client: ClientDetail = ClientDetail.get_by(db, field="user_id", value=user.id) assert HTTP_200_OK == response.status_code - assert response_body['id'] == permissions.id - assert response_body['scopes'] == updated_scopes + assert response_body["id"] == permissions.id + assert response_body["scopes"] == updated_scopes assert client.scopes == updated_scopes user.delete(db) @@ -176,16 +187,15 @@ def test_get_user_permissions_not_authenticated(self, url, api_client): response = api_client.get(url, headers={}, json={}) assert HTTP_401_UNAUTHORIZED == response.status_code - def test_get_user_permissions_wrong_scope(self, url, api_client, generate_auth_header): + def test_get_user_permissions_wrong_scope( + self, url, api_client, generate_auth_header + ): auth_header = generate_auth_header([SAAS_CONFIG_READ]) response = api_client.get(url, headers=auth_header, json={}) assert HTTP_403_FORBIDDEN == response.status_code def test_get_user_permissions_invalid_user_id( - self, - db, - api_client, - generate_auth_header + self, db, api_client, generate_auth_header ) -> None: auth_header = generate_auth_header([USER_PERMISSION_READ]) invalid_user_id = "bogus_user_id" @@ -194,37 +204,38 @@ def test_get_user_permissions_invalid_user_id( data={"username": "user_1", "password": "test_password"}, ) - permissions = FidesopsUserPermissions.create(db=db, data={ - "user_id": user.id, - "scopes": [PRIVACY_REQUEST_READ] - }) + permissions = FidesopsUserPermissions.create( + db=db, data={"user_id": user.id, "scopes": [PRIVACY_REQUEST_READ]} + ) body = {"id": permissions.id, "scopes": [PRIVACY_REQUEST_READ]} - response = api_client.get(f"{V1_URL_PREFIX}/user/{invalid_user_id}/permission", headers=auth_header, json=body) - permissions = FidesopsUserPermissions.get_by(db, field="user_id", value=invalid_user_id) + response = api_client.get( + f"{V1_URL_PREFIX}/user/{invalid_user_id}/permission", + headers=auth_header, + json=body, + ) + permissions = FidesopsUserPermissions.get_by( + db, field="user_id", value=invalid_user_id + ) assert HTTP_404_NOT_FOUND == response.status_code assert permissions is None user.delete(db) - def test_get_user_permissions( - self, - db, - api_client, - generate_auth_header - ) -> None: + def test_get_user_permissions(self, db, api_client, generate_auth_header) -> None: auth_header = generate_auth_header([USER_PERMISSION_READ]) user = FidesopsUser.create( db=db, data={"username": "user_1", "password": "test_password"}, ) - permissions = FidesopsUserPermissions.create(db=db, data={ - "user_id": user.id, - "scopes": [PRIVACY_REQUEST_READ] - }) + permissions = FidesopsUserPermissions.create( + db=db, data={"user_id": user.id, "scopes": [PRIVACY_REQUEST_READ]} + ) - response = api_client.get(f"{V1_URL_PREFIX}/user/{user.id}/permission", headers=auth_header) + response = api_client.get( + f"{V1_URL_PREFIX}/user/{user.id}/permission", headers=auth_header + ) response_body = json.loads(response.text) assert HTTP_200_OK == response.status_code - assert response_body['id'] == permissions.id - assert response_body['user_id'] == user.id - assert response_body['scopes'] == [PRIVACY_REQUEST_READ] + assert response_body["id"] == permissions.id + assert response_body["user_id"] == user.id + assert response_body["scopes"] == [PRIVACY_REQUEST_READ] user.delete(db) diff --git a/tests/fixtures/application_fixtures.py b/tests/fixtures/application_fixtures.py index dd4054705..4da811dfc 100644 --- a/tests/fixtures/application_fixtures.py +++ b/tests/fixtures/application_fixtures.py @@ -715,8 +715,8 @@ def user(db: Session): ) FidesopsUserPermissions.create( - db=db, data={"user_id": user.id, "scopes": [PRIVACY_REQUEST_READ]} - ) + db=db, data={"user_id": user.id, "scopes": [PRIVACY_REQUEST_READ]} + ) db.add(client) db.commit() @@ -929,3 +929,19 @@ def sample_data(): ["1", "a", [["z", "a", "a"]]], ], # Lists elems are different types, not officially supported } + + +@pytest.fixture(scope="function") +def application_user(db) -> FidesopsUser: + unique_username = f"user-{uuid4()}" + user = FidesopsUser.create( + db=db, + data={ + "username": unique_username, + "password": "test_password", + "first_name": "Test", + "last_name": "User", + }, + ) + yield user + user.delete(db=db)