Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

[#446] Adds first_name, last_name, created_at to FidesopsUser response #465

Merged
merged 4 commits into from
May 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/fidesops/models/fidesops_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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"),
},
)

Expand Down
7 changes: 7 additions & 0 deletions src/fidesops/schemas/user.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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):
Expand Down
Original file line number Diff line number Diff line change
@@ -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 ###
104 changes: 87 additions & 17 deletions tests/api/v1/endpoints/test_user_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -38,6 +44,7 @@

page_size = Params().size


class TestCreateUser:
@pytest.fixture(scope="function")
def url(self, oauth_client: ClientDetail) -> str:
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be good to also assert that the users first_name and last_name are what they were set to when it was created. There are test cases below that check those fields so it is tested indirectly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've worked out why that's happening. We're explicitly running black /src in the Makefile. Will follow-up this change as part of a subsequent hotfix.

user.delete(db)


Expand Down Expand Up @@ -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
)
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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"])
Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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])
Expand Down
Loading