From 90355fa2b0e015dfa0deba04747f45ce1c100e3b Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Wed, 22 Feb 2023 14:53:55 -0600 Subject: [PATCH 01/12] Optionally add the ability to grant roles directly to users instead of just scopes. Roles will be associated with a list of scopes. - Update the permission check in verify_oauth_client to see if you have the scope from one of two means: either a scope you were assigned directly OR if your role(s) are associated with that scope. - Add client.roles and fidesuserpermission.roles column. Make FidesUserPermissions.scopes and client.scopes nullable, as you can assign roles or scopes. - Add a new endpoint /oauth/role to get the possible roles and their associated scopes to which you can be assigned - Update POST /oauth/token to add an admin role if the client is the root client. - Update Patch user permissions to be able to update both roles and scopes - Update get user permissions to return both the scope registry and admin roles if this is the root user. - Update CLI command "fides user permissions" to return both roles and directly-assigned scopes - Update CLI command "fides user create" which creates a user with full permissions to also have the admin role. - Add ExecutionSettings.root_user_roles - Fix the default values on Client.scopes and FidesUserPermission.scopes - this did not appear to be working as intended - Try to ensure that if a client's roles or scopes are accessed, it returns an empty list instead of null if they don't exist. - Add first draft of role -> scope mapping. - Remove old, unused privileges code. We were once intending to group scopes into "privileges" but never finished that work. This replaces that effort. - On login, give the client both root_user_scopes and root_user_roles if it's the root user. Persist permission scopes and roles to the client if the client doesn't exist. --- scripts/create_test_data.py | 1 + scripts/setup/user.py | 8 +- .../985d3756f908_add_role_based_foundation.py | 50 +++ .../ops/api/v1/endpoints/oauth_endpoints.py | 30 +- .../v1/endpoints/user_permission_endpoints.py | 13 +- src/fides/api/ops/api/v1/urn_registry.py | 1 + src/fides/api/ops/schemas/user_permission.py | 15 +- src/fides/api/ops/util/oauth_util.py | 100 ++++-- src/fides/cli/commands/user.py | 2 +- src/fides/core/config/security_settings.py | 2 + src/fides/core/user.py | 35 ++- src/fides/lib/cryptography/schemas/jwt.py | 1 + src/fides/lib/models/client.py | 24 +- .../lib/models/fides_user_permissions.py | 21 +- .../lib/oauth/api/routes/user_endpoints.py | 8 +- src/fides/lib/oauth/privileges.py | 61 ---- src/fides/lib/oauth/roles.py | 100 ++++++ .../lib/oauth/schemas/user_permission.py | 9 +- tests/ctl/cli/test_cli.py | 55 ++++ tests/lib/conftest.py | 87 +++++- tests/lib/test_client_model.py | 165 +++++++++- tests/lib/test_fides_user_permissions.py | 36 --- tests/lib/test_oauth_util.py | 292 +++++++++++++++++- .../test_connection_config_endpoints.py | 30 ++ .../api/v1/endpoints/test_oauth_endpoints.py | 61 +++- .../test_privacy_request_endpoints.py | 29 ++ .../api/v1/endpoints/test_user_endpoints.py | 111 ++++++- .../test_user_permission_endpoints.py | 242 ++++++++++++++- tests/ops/conftest.py | 58 ++++ tests/ops/fixtures/application_fixtures.py | 57 ++++ .../test_consent_email_batch_send.py | 6 + 31 files changed, 1508 insertions(+), 202 deletions(-) create mode 100644 src/fides/api/ctl/migrations/versions/985d3756f908_add_role_based_foundation.py delete mode 100644 src/fides/lib/oauth/privileges.py create mode 100644 src/fides/lib/oauth/roles.py diff --git a/scripts/create_test_data.py b/scripts/create_test_data.py index ac1dc1b19a..70d7e3481e 100644 --- a/scripts/create_test_data.py +++ b/scripts/create_test_data.py @@ -161,6 +161,7 @@ def create_test_data(db: orm.Session) -> FidesUser: "hashed_secret": "autoseededdata", "salt": "autoseededdata", "scopes": [], + "roles": [], }, ) diff --git a/scripts/setup/user.py b/scripts/setup/user.py index 3b00c0fbae..0715270e4d 100644 --- a/scripts/setup/user.py +++ b/scripts/setup/user.py @@ -5,6 +5,7 @@ from fides.api.ops.api.v1 import urn_registry as urls from fides.api.ops.api.v1.scope_registry import SCOPE_REGISTRY +from fides.lib.oauth.roles import ADMIN from . import constants @@ -14,7 +15,7 @@ def create_user( username=constants.FIDES_USERNAME, password=constants.FIDES_PASSWORD, ): - """Adds a user with full permissions""" + """Adds a user with full permissions - all scopes and admin role""" login_response = requests.post( f"{constants.BASE_URL}{urls.LOGIN}", headers=auth_header, @@ -50,10 +51,7 @@ def create_user( response = requests.put( f"{constants.BASE_URL}{user_permissions_url}", headers=auth_header, - json={ - "id": user_id, - "scopes": SCOPE_REGISTRY, - }, + json={"id": user_id, "scopes": SCOPE_REGISTRY, "roles": [ADMIN]}, ) if not response.ok: diff --git a/src/fides/api/ctl/migrations/versions/985d3756f908_add_role_based_foundation.py b/src/fides/api/ctl/migrations/versions/985d3756f908_add_role_based_foundation.py new file mode 100644 index 0000000000..acde64046c --- /dev/null +++ b/src/fides/api/ctl/migrations/versions/985d3756f908_add_role_based_foundation.py @@ -0,0 +1,50 @@ +"""add_role_based_foundation + +Revision ID: 985d3756f908 +Revises: 8e198eb13802 +Create Date: 2023-02-17 21:04:20.113779 + +""" +import sqlalchemy as sa +from alembic import op +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = "985d3756f908" +down_revision = "8e198eb13802" +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column( + "client", + sa.Column("roles", sa.ARRAY(sa.String()), server_default="{}", nullable=True), + ) + op.alter_column( + "client", "scopes", existing_type=postgresql.ARRAY(sa.VARCHAR()), nullable=True + ) + op.add_column( + "fidesuserpermissions", sa.Column("roles", sa.ARRAY(sa.String()), nullable=True) + ) + op.alter_column( + "fidesuserpermissions", + "scopes", + existing_type=postgresql.ARRAY(sa.VARCHAR()), + nullable=True, + ) + + +def downgrade(): + op.alter_column( + "fidesuserpermissions", + "scopes", + existing_type=postgresql.ARRAY(sa.VARCHAR()), + nullable=False, + ) + op.drop_column("fidesuserpermissions", "roles") + op.alter_column( + "client", "scopes", existing_type=postgresql.ARRAY(sa.VARCHAR()), nullable=False + ) + op.drop_column("client", "roles") diff --git a/src/fides/api/ops/api/v1/endpoints/oauth_endpoints.py b/src/fides/api/ops/api/v1/endpoints/oauth_endpoints.py index 066dd32d16..3e461c2674 100644 --- a/src/fides/api/ops/api/v1/endpoints/oauth_endpoints.py +++ b/src/fides/api/ops/api/v1/endpoints/oauth_endpoints.py @@ -1,4 +1,4 @@ -from typing import List, Optional +from typing import Dict, List, Optional from fastapi import Body, Depends, HTTPException, Request, Security from fastapi.security import HTTPBasic @@ -27,6 +27,7 @@ CLIENT_BY_ID, CLIENT_SCOPE, OAUTH_CALLBACK, + ROLE, SCOPE, TOKEN, V1_URL_PREFIX, @@ -49,6 +50,7 @@ from fides.api.ops.util.oauth_util import verify_oauth_client from fides.core.config import get_config from fides.lib.models.client import ClientDetail +from fides.lib.oauth.roles import ADMIN, roles_to_scopes_mapping from fides.lib.oauth.schemas.oauth import ( AccessToken, OAuth2ClientCredentialsRequestForm, @@ -83,9 +85,9 @@ async def acquire_access_token( else: raise AuthenticationFailure(detail="Authentication Failure") - # scopes param is only used if client is root client, otherwise we use the client's associated scopes + # scopes/roles params are only used if client is root client, otherwise we use the client's associated scopes and/or roles client_detail = ClientDetail.get( - db, object_id=client_id, config=CONFIG, scopes=SCOPE_REGISTRY + db, object_id=client_id, config=CONFIG, scopes=SCOPE_REGISTRY, roles=[ADMIN] ) if client_detail is None: @@ -112,7 +114,7 @@ def create_client( db: Session = Depends(get_db), scopes: List[str] = Body([]), ) -> ClientCreatedResponse: - """Creates a new client and returns the credentials""" + """Creates a new client and returns the credentials. Only direct scopes can be added to the client via this endpoint.""" logger.info("Creating new client") if not all(scope in SCOPE_REGISTRY for scope in scopes): raise HTTPException( @@ -148,13 +150,15 @@ def delete_client(client_id: str, db: Session = Depends(get_db)) -> None: response_model=List[str], ) def get_client_scopes(client_id: str, db: Session = Depends(get_db)) -> List[str]: - """Returns a list of the scopes associated with the client. Returns an empty list if client does not exist.""" + """Returns a list of the directly-assigned scopes associated with the client. + Does not return roles associated with the client. + Returns an empty list if client does not exist.""" client = ClientDetail.get(db, object_id=client_id, config=CONFIG) if not client: return [] logger.info("Getting client scopes") - return client.scopes + return client.scopes or [] @router.put( @@ -167,7 +171,9 @@ def set_client_scopes( scopes: List[str], db: Session = Depends(get_db), ) -> None: - """Overwrites the client's scopes with those provided. Does nothing if the client doesn't exist""" + """Overwrites the client's directly-assigned scopes with those provided. + Roles cannot be edited via this endpoint. + Does nothing if the client doesn't exist""" client = ClientDetail.get(db, object_id=client_id, config=CONFIG) if not client: return @@ -193,6 +199,16 @@ def read_scopes() -> List[str]: return SCOPE_REGISTRY +@router.get( + ROLE, + dependencies=[Security(verify_oauth_client, scopes=[SCOPE_READ])], +) +def read_roles_to_scopes_mapping() -> Dict[str, List]: + """Returns a list of all roles and associated scopes available for assignment in the system""" + logger.info("Getting all available roles") + return roles_to_scopes_mapping + + @router.get(OAUTH_CALLBACK, response_model=None) def oauth_callback(code: str, state: str, db: Session = Depends(get_db)) -> None: """ diff --git a/src/fides/api/ops/api/v1/endpoints/user_permission_endpoints.py b/src/fides/api/ops/api/v1/endpoints/user_permission_endpoints.py index 88a725a78a..ac8c3e2370 100644 --- a/src/fides/api/ops/api/v1/endpoints/user_permission_endpoints.py +++ b/src/fides/api/ops/api/v1/endpoints/user_permission_endpoints.py @@ -29,6 +29,7 @@ from fides.core.config import get_config from fides.lib.models.fides_user import FidesUser from fides.lib.models.fides_user_permissions import FidesUserPermissions +from fides.lib.oauth.roles import ADMIN CONFIG = get_config() router = APIRouter(tags=["User Permissions"], prefix=V1_URL_PREFIX) @@ -62,6 +63,9 @@ def create_user_permissions( status_code=HTTP_400_BAD_REQUEST, detail="This user already has permissions set.", ) + + if user.client: + user.client.update(db=db, data=permissions.dict()) logger.info("Created FidesUserPermission record") return FidesUserPermissions.create( db=db, data={"user_id": user_id, **permissions.dict()} @@ -79,10 +83,16 @@ def update_user_permissions( user_id: str, permissions: UserPermissionsEdit, ) -> FidesUserPermissions: + """Update either a user's role(s) and/or scope(s). + + Typically we'll assign roles to a user and they'll inherit the associated scopes, + but we're still supporting assigning scopes directly as well. + """ user = validate_user_id(db, user_id) logger.info("Updated FidesUserPermission record") + if user.client: - user.client.update(db=db, data={"scopes": permissions.scopes}) + user.client.update(db=db, data=permissions.dict()) return user.permissions.update( # type: ignore[attr-defined] db=db, data={"id": user.permissions.id, "user_id": user_id, **permissions.dict()}, # type: ignore[attr-defined] @@ -109,6 +119,7 @@ async def get_user_permissions( id=CONFIG.security.oauth_root_client_id, user_id=CONFIG.security.oauth_root_client_id, scopes=SCOPE_REGISTRY, + roles=[ADMIN], ) logger.info("Retrieved FidesUserPermission record for current user") diff --git a/src/fides/api/ops/api/v1/urn_registry.py b/src/fides/api/ops/api/v1/urn_registry.py index 3fe83c662f..72ac28fdcc 100644 --- a/src/fides/api/ops/api/v1/urn_registry.py +++ b/src/fides/api/ops/api/v1/urn_registry.py @@ -18,6 +18,7 @@ TOKEN = "/oauth/token" CLIENT = "/oauth/client" SCOPE = "/oauth/scope" +ROLE = "/oauth/role" CLIENT_BY_ID = "/oauth/client/{client_id}" CLIENT_SCOPE = "/oauth/client/{client_id}/scope" OAUTH_CALLBACK = "/oauth/callback" diff --git a/src/fides/api/ops/schemas/user_permission.py b/src/fides/api/ops/schemas/user_permission.py index 76c52d5d52..af0018eedd 100644 --- a/src/fides/api/ops/schemas/user_permission.py +++ b/src/fides/api/ops/schemas/user_permission.py @@ -1,10 +1,11 @@ -from typing import List +from typing import List, Set from fastapi import HTTPException from pydantic import validator from starlette.status import HTTP_422_UNPROCESSABLE_ENTITY from fides.api.ops.api.v1.scope_registry import SCOPE_REGISTRY +from fides.lib.oauth.roles import ROLE_REGISTRY from fides.lib.oauth.schemas.user_permission import ( UserPermissionsCreate as UserPermissionsCreateLib, ) @@ -31,6 +32,18 @@ def validate_scopes(cls, scopes: List[str]) -> List[str]: ) return scopes + @validator("roles") + @classmethod + def validate_roles(cls, roles: List[str]) -> List[str]: + """Validates that all incoming roles are valid""" + diff: Set = set(roles).difference(set(ROLE_REGISTRY)) + if len(diff) > 0: + raise HTTPException( + status_code=HTTP_422_UNPROCESSABLE_ENTITY, + detail=f"Invalid Role(s) {diff}. Roles must be one of {ROLE_REGISTRY}.", + ) + return roles + class UserPermissionsEdit(UserPermissionsEditLib): """Data required to edit a FidesUserPermissions record""" diff --git a/src/fides/api/ops/util/oauth_util.py b/src/fides/api/ops/util/oauth_util.py index 893a797d05..c46089600a 100644 --- a/src/fides/api/ops/util/oauth_util.py +++ b/src/fides/api/ops/util/oauth_util.py @@ -4,7 +4,7 @@ from datetime import datetime from functools import update_wrapper from types import FunctionType -from typing import Callable +from typing import Any, Callable, Dict, List from fastapi import Depends, HTTPException, Security from fastapi.security import SecurityScopes @@ -24,12 +24,14 @@ from fides.lib.cryptography.schemas.jwt import ( JWE_ISSUED_AT, JWE_PAYLOAD_CLIENT_ID, + JWE_PAYLOAD_ROLES, JWE_PAYLOAD_SCOPES, ) from fides.lib.exceptions import AuthenticationError, AuthorizationError from fides.lib.models.client import ClientDetail from fides.lib.models.fides_user import FidesUser from fides.lib.oauth.oauth_util import extract_payload, is_token_expired +from fides.lib.oauth.roles import ADMIN, get_scopes_from_roles from fides.lib.oauth.schemas.oauth import OAuth2ClientCredentialsBearer CONFIG = get_config() @@ -141,7 +143,7 @@ async def get_root_client( This function is primarily used to let users bypass endpoint authorization """ client = ClientDetail.get( - db, object_id=client_id, config=CONFIG, scopes=SCOPE_REGISTRY + db, object_id=client_id, config=CONFIG, scopes=SCOPE_REGISTRY, roles=[ADMIN] ) if not client: logger.debug("Auth token belongs to an invalid client_id.") @@ -156,7 +158,7 @@ async def verify_oauth_client( ) -> ClientDetail: """ Verifies that the access token provided in the authorization header contains - the necessary scopes specified by the caller. Yields a 403 forbidden error + the necessary scopes or roles specified by the caller. Yields a 403 forbidden error if not. NOTE: This function may be overwritten in `main.py` when changing @@ -185,37 +187,97 @@ async def verify_oauth_client( ): raise AuthorizationError(detail="Not Authorized for this action") - assigned_scopes = token_data[JWE_PAYLOAD_SCOPES] - if not set(security_scopes.scopes).issubset(assigned_scopes): - scopes_required = ",".join(security_scopes.scopes) - scopes_provided = ",".join(assigned_scopes) - logger.debug( - "Auth token missing required scopes: {}. Scopes provided: {}.", - scopes_required, - scopes_provided, - ) - raise AuthorizationError(detail="Not Authorized for this action") - client_id = token_data.get(JWE_PAYLOAD_CLIENT_ID) if not client_id: logger.debug("No client_id included in auth token.") raise AuthorizationError(detail="Not Authorized for this action") - # scopes param is only used if client is root client, otherwise we use the client's associated scopes + # scopes/roles param is only used if client is root client, otherwise we use the client's associated scopes client = ClientDetail.get( - db, object_id=client_id, config=CONFIG, scopes=SCOPE_REGISTRY + db, object_id=client_id, config=CONFIG, scopes=SCOPE_REGISTRY, roles=[ADMIN] ) if not client: logger.debug("Auth token belongs to an invalid client_id.") raise AuthorizationError(detail="Not Authorized for this action") - if not set(assigned_scopes).issubset(set(client.scopes)): + if not has_permissions( + token_data=token_data, client=client, endpoint_scopes=security_scopes + ): + raise AuthorizationError(detail="Not Authorized for this action") + + return client + + +def has_permissions( + token_data: Dict[str, Any], client: ClientDetail, endpoint_scopes: SecurityScopes +) -> bool: + """Does the user have the necessary scopes, either via a scope they were assigned directly, + or a scope associated with their role(s)?""" + has_direct_scope: bool = _has_direct_scopes( + token_data=token_data, client=client, endpoint_scopes=endpoint_scopes + ) + has_role: bool = _has_scope_via_role( + token_data=token_data, client=client, endpoint_scopes=endpoint_scopes + ) + return has_direct_scope or has_role + + +def _has_scope_via_role( + token_data: Dict[str, Any], client: ClientDetail, endpoint_scopes: SecurityScopes +) -> bool: + """Does the user have the required scopes indirectly via a role and is the token valid?""" + assigned_roles: List[str] = token_data.get(JWE_PAYLOAD_ROLES, []) + associated_scopes: List[str] = get_scopes_from_roles(assigned_roles) + + if not _has_correct_scopes( + user_scopes=associated_scopes, endpoint_scopes=endpoint_scopes + ): + return False + + if not set(assigned_roles).issubset(set(client.roles or [])): + # If the roles on the token are not a subset of the roles available + # one the associated oauth client, this token is not valid + logger.debug("Client no longer allowed to issue these roles.") + return False + + return True + + +def _has_direct_scopes( + token_data: Dict[str, Any], client: ClientDetail, endpoint_scopes: SecurityScopes +) -> bool: + """Does the token have the required scopes directly and is the token still valid?""" + assigned_scopes: List[str] = token_data.get(JWE_PAYLOAD_SCOPES, []) + + if not _has_correct_scopes( + user_scopes=assigned_scopes, endpoint_scopes=endpoint_scopes + ): + return False + + if not set(assigned_scopes).issubset(set(client.scopes or [])): # If the scopes on the token are not a subset of the scopes available # to the associated oauth client, this token is not valid logger.debug("Client no longer allowed to issue these scopes.") - raise AuthorizationError(detail="Not Authorized for this action") - return client + return False + + return True + + +def _has_correct_scopes( + user_scopes: List[str], endpoint_scopes: SecurityScopes +) -> bool: + """Are the required scopes a subset of the scopes belonging to the user?""" + if not set(endpoint_scopes.scopes).issubset(user_scopes): + scopes_required = ",".join(endpoint_scopes.scopes) + scopes_provided = ",".join(user_scopes) + logger.debug( + "Auth token missing required scopes: {}. Scopes provided: {}.", + scopes_required, + scopes_provided, + ) + return False + return True # This is a workaround so that we can override CLI-related endpoints and diff --git a/src/fides/cli/commands/user.py b/src/fides/cli/commands/user.py index cae4f809e4..773712e547 100644 --- a/src/fides/cli/commands/user.py +++ b/src/fides/cli/commands/user.py @@ -59,7 +59,7 @@ def login(ctx: click.Context, username: str, password: str) -> None: @user.command(name="permissions") @click.pass_context def get_permissions(ctx: click.Context) -> None: - """List the scopes avaible to the current user.""" + """List the directly-assigned scopes and roles available to the current user.""" config = ctx.obj["CONFIG"] server_url = config.cli.server_url get_permissions_command(server_url=server_url) diff --git a/src/fides/core/config/security_settings.py b/src/fides/core/config/security_settings.py index 5dc2f09f0a..c2cb6e1e4a 100644 --- a/src/fides/core/config/security_settings.py +++ b/src/fides/core/config/security_settings.py @@ -11,6 +11,7 @@ from fides.api.ops.api.v1.scope_registry import SCOPE_REGISTRY from fides.lib.cryptography.cryptographic_util import generate_salt, hash_with_salt +from ...lib.oauth.roles import ADMIN from .fides_settings import FidesSettings ENV_PREFIX = "FIDES__SECURITY__" @@ -30,6 +31,7 @@ class SecuritySettings(FidesSettings): """Configuration settings for Security variables.""" root_user_scopes: Optional[List[str]] = SCOPE_REGISTRY + root_user_roles: Optional[List[str]] = [ADMIN] subject_request_download_link_ttl_seconds: int = 432000 # 5 days request_rate_limit: str = "1000/minute" rate_limit_prefix: str = "fides-" diff --git a/src/fides/core/user.py b/src/fides/core/user.py index fa927df904..f5551230db 100644 --- a/src/fides/core/user.py +++ b/src/fides/core/user.py @@ -4,6 +4,7 @@ import requests +from fides.api.ops.api.v1.scope_registry import SCOPE_REGISTRY from fides.cli.utils import handle_cli_response from fides.core.config import get_config from fides.core.utils import ( @@ -16,7 +17,7 @@ write_credentials_file, ) from fides.lib.cryptography.cryptographic_util import str_to_b64_str -from fides.lib.oauth.scopes import SCOPES +from fides.lib.oauth.roles import ADMIN config = get_config() CREATE_USER_PATH = "/api/v1/user" @@ -66,9 +67,9 @@ def create_user( def get_user_permissions( user_id: str, auth_header: Dict[str, str], server_url: str -) -> List[str]: +) -> Tuple[List[str], List[str]]: """ - List all of the user permissions for the provided user. + List all of the directly-assigned scopes for the provided user. """ get_permissions_path = USER_PERMISSIONS_PATH.format(user_id) response = requests.get( @@ -77,16 +78,20 @@ def get_user_permissions( ) handle_cli_response(response, verbose=False) - return response.json()["scopes"] + return response.json()["scopes"], response.json()["roles"] def update_user_permissions( - user_id: str, scopes: List[str], auth_header: Dict[str, str], server_url: str + user_id: str, + scopes: List[str], + auth_header: Dict[str, str], + server_url: str, + roles: List[str], ) -> requests.Response: """ Update user permissions for a given user. """ - request_data = {"scopes": scopes, "id": user_id} + request_data = {"scopes": scopes, "id": user_id, "roles": roles} set_permissions_path = USER_PERMISSIONS_PATH.format(user_id) response = requests.put( server_url + set_permissions_path, @@ -116,7 +121,11 @@ def create_command( ) user_id = user_response.json()["id"] update_user_permissions( - user_id=user_id, scopes=SCOPES, auth_header=auth_header, server_url=server_url + user_id=user_id, + scopes=SCOPE_REGISTRY, + auth_header=auth_header, + server_url=server_url, + roles=[ADMIN], ) echo_green(f"User: '{username}' created and assigned permissions.") @@ -152,8 +161,12 @@ def get_permissions_command(server_url: str) -> None: user_id = credentials.user_id auth_header = get_auth_header() - permissions: List[str] = get_user_permissions(user_id, auth_header, server_url) + scopes, roles = get_user_permissions(user_id, auth_header, server_url) + + print("Permissions (Direct Scopes):") + for scope in scopes: + print(f"\t{scope}") - print("Permissions:") - for permission in permissions: - print(f"\t{permission}") + print("Roles:") + for role in roles: + print(f"\t{role}") diff --git a/src/fides/lib/cryptography/schemas/jwt.py b/src/fides/lib/cryptography/schemas/jwt.py index 0b5aece904..07658d57de 100644 --- a/src/fides/lib/cryptography/schemas/jwt.py +++ b/src/fides/lib/cryptography/schemas/jwt.py @@ -1,3 +1,4 @@ JWE_PAYLOAD_CLIENT_ID = "client-id" JWE_PAYLOAD_SCOPES = "scopes" JWE_ISSUED_AT = "iat" +JWE_PAYLOAD_ROLES = "roles" diff --git a/src/fides/lib/models/client.py b/src/fides/lib/models/client.py index 8840756668..6211e785c2 100644 --- a/src/fides/lib/models/client.py +++ b/src/fides/lib/models/client.py @@ -17,6 +17,7 @@ from fides.lib.cryptography.schemas.jwt import ( JWE_ISSUED_AT, JWE_PAYLOAD_CLIENT_ID, + JWE_PAYLOAD_ROLES, JWE_PAYLOAD_SCOPES, ) from fides.lib.db.base_class import Base @@ -25,6 +26,7 @@ ADMIN_UI_ROOT = "admin_ui_root" DEFAULT_SCOPES: list[str] = [] +DEFAULT_ROLES: list[str] = [] class ClientDetail(Base): @@ -36,7 +38,8 @@ def __tablename__(self) -> str: hashed_secret = Column(String, nullable=False) salt = Column(String, nullable=False) - scopes = Column(ARRAY(String), nullable=False, default="{}") + scopes = Column(ARRAY(String), nullable=True, server_default="{}", default=dict) + roles = Column(ARRAY(String), nullable=True, server_default="{}", default=dict) fides_key = Column(String, index=True, unique=True, nullable=True) user_id = Column( String, ForeignKey(FidesUser.id_field_path), nullable=True, unique=True @@ -53,6 +56,7 @@ def create_client_and_secret( fides_key: str = None, user_id: str = None, encoding: str = "UTF-8", + roles: list[str] | None = None, ) -> tuple["ClientDetail", str]: """Creates a ClientDetail and returns that along with the unhashed secret so it can be returned to the user on create @@ -64,6 +68,9 @@ def create_client_and_secret( if not scopes: scopes = DEFAULT_SCOPES + if not roles: + roles = DEFAULT_ROLES + salt = generate_salt() hashed_secret = hash_with_salt( secret.encode(encoding), @@ -79,6 +86,7 @@ def create_client_and_secret( "scopes": scopes, "fides_key": fides_key, "user_id": user_id, + "roles": roles, }, ) return client, secret # type: ignore @@ -91,10 +99,11 @@ def get( # type: ignore object_id: Any, config: FidesConfig, scopes: list[str] | None = None, + roles: list[str] | None = None, ) -> ClientDetail | None: """Fetch a database record via a client_id""" if object_id == config.security.oauth_root_client_id: - return _get_root_client_detail(config, scopes) + return _get_root_client_detail(config, scopes=scopes, roles=roles) return super().get(db, object_id=object_id) def create_access_code_jwe(self, encryption_key: str) -> str: @@ -104,6 +113,7 @@ def create_access_code_jwe(self, encryption_key: str) -> str: JWE_PAYLOAD_CLIENT_ID: self.id, JWE_PAYLOAD_SCOPES: self.scopes, JWE_ISSUED_AT: datetime.now().isoformat(), + JWE_PAYLOAD_ROLES: self.roles, } return generate_jwe(json.dumps(payload), encryption_key) @@ -120,21 +130,29 @@ def credentials_valid(self, provided_secret: str, encoding: str = "UTF-8") -> bo def _get_root_client_detail( config: FidesConfig, scopes: list[str] | None, + roles: list[str] | None, encoding: str = "UTF-8", ) -> ClientDetail | None: + """ + Return a root ClientDetail + """ + if not config.security.oauth_root_client_secret_hash: raise ValueError("A root client hash is required") - if scopes: + if scopes or roles: return ClientDetail( id=config.security.oauth_root_client_id, hashed_secret=config.security.oauth_root_client_secret_hash[0], salt=config.security.oauth_root_client_secret_hash[1].decode(encoding), scopes=scopes, + roles=roles, ) return ClientDetail( id=config.security.oauth_root_client_id, hashed_secret=config.security.oauth_root_client_secret_hash[0], salt=config.security.oauth_root_client_secret_hash[1].decode(encoding), + scopes=DEFAULT_SCOPES, + roles=DEFAULT_ROLES, ) diff --git a/src/fides/lib/models/fides_user_permissions.py b/src/fides/lib/models/fides_user_permissions.py index 2cb28e8ac7..1c56badfbc 100644 --- a/src/fides/lib/models/fides_user_permissions.py +++ b/src/fides/lib/models/fides_user_permissions.py @@ -1,34 +1,17 @@ -from typing import List, Tuple - from sqlalchemy import ARRAY, Column, ForeignKey, String from sqlalchemy.orm import backref, relationship from fides.lib.db.base_class import Base from fides.lib.models.fides_user import FidesUser -from fides.lib.oauth.privileges import privileges -from fides.lib.oauth.scopes import PRIVACY_REQUEST_READ class FidesUserPermissions(Base): """The DB ORM model for FidesUserPermissions""" user_id = Column(String, ForeignKey(FidesUser.id), nullable=False, unique=True) - # escaping curly braces requires doubling them. Not a "\". So {{{test123}}} - # renders as {test123} - scopes = Column( - ARRAY(String), nullable=False, default=f"{{{PRIVACY_REQUEST_READ}}}" - ) + scopes = Column(ARRAY(String), nullable=True, server_default="{}", default=dict) + roles = Column(ARRAY(String), nullable=True, server_default="{}", default=dict) user = relationship( FidesUser, backref=backref("permissions", cascade="all,delete", uselist=False), ) - - @property - def privileges(self) -> Tuple[str, ...]: - """Return the big-picture privileges a user has based on their individual scopes""" - user_privileges: List[str] = [] - for privilege, required_scopes in privileges.items(): - if required_scopes.issubset(self.scopes): - user_privileges.append(privilege) - - return tuple(user_privileges) diff --git a/src/fides/lib/oauth/api/routes/user_endpoints.py b/src/fides/lib/oauth/api/routes/user_endpoints.py index 1df754cf00..ca0b58daa0 100644 --- a/src/fides/lib/oauth/api/routes/user_endpoints.py +++ b/src/fides/lib/oauth/api/routes/user_endpoints.py @@ -81,7 +81,11 @@ def create_user( user = FidesUser.create(db=db, data=user_data.dict()) logger.info("Created user with id: '{}'.", user.id) FidesUserPermissions.create( - db=db, data={"user_id": user.id, "scopes": [PRIVACY_REQUEST_READ]} + db=db, + data={ + "user_id": user.id, + "scopes": [PRIVACY_REQUEST_READ], + }, # TODO - Change this to Viewer Role by default? ) return user @@ -175,6 +179,7 @@ def user_login( object_id=config.security.oauth_root_client_id, config=config, scopes=config.security.root_user_scopes, + roles=config.security.root_user_roles, ) if not client_check: @@ -241,6 +246,7 @@ def perform_login( client_id_byte_length, client_secret_btye_length, scopes=user.permissions.scopes, # type: ignore + roles=user.permissions.roles, # type: ignore user_id=user.id, ) diff --git a/src/fides/lib/oauth/privileges.py b/src/fides/lib/oauth/privileges.py deleted file mode 100644 index de3ce9f573..0000000000 --- a/src/fides/lib/oauth/privileges.py +++ /dev/null @@ -1,61 +0,0 @@ -from fides.lib.oauth.scopes import ( - CONNECTION_AUTHORIZE, - CONNECTION_CREATE_OR_UPDATE, - CONNECTION_DELETE, - CONNECTION_READ, - DATASET_CREATE_OR_UPDATE, - DATASET_DELETE, - DATASET_READ, - PRIVACY_REQUEST_CALLBACK_RESUME, - PRIVACY_REQUEST_DELETE, - PRIVACY_REQUEST_READ, - PRIVACY_REQUEST_REVIEW, - SAAS_CONFIG_CREATE_OR_UPDATE, - SAAS_CONFIG_DELETE, - SAAS_CONFIG_READ, - USER_CREATE, - USER_DELETE, - USER_READ, -) - -VIEW = "view" -MANAGE = "manage" - -VIEW_SUBJECT_REQUESTS = f"{VIEW}_subject_requests" -MANAGE_SUBJECT_REQUESTS = f"{MANAGE}_subject_requests" - -VIEW_CONNECTIONS = f"{VIEW}_datastore_connections" -MANAGE_CONNECTIONS = f"{MANAGE}_datastore_connections" - -VIEW_USERS = f"{VIEW}_users" -MANAGE_USERS = f"{MANAGE}_users" - - -privileges = { - VIEW_SUBJECT_REQUESTS: {PRIVACY_REQUEST_READ}, - MANAGE_SUBJECT_REQUESTS: { - PRIVACY_REQUEST_CALLBACK_RESUME, - PRIVACY_REQUEST_DELETE, - PRIVACY_REQUEST_READ, - PRIVACY_REQUEST_REVIEW, - }, - VIEW_CONNECTIONS: { - CONNECTION_READ, - DATASET_READ, - SAAS_CONFIG_READ, - }, - MANAGE_CONNECTIONS: { - CONNECTION_AUTHORIZE, - CONNECTION_CREATE_OR_UPDATE, - CONNECTION_DELETE, - CONNECTION_READ, - DATASET_CREATE_OR_UPDATE, - DATASET_DELETE, - DATASET_READ, - SAAS_CONFIG_CREATE_OR_UPDATE, - SAAS_CONFIG_DELETE, - SAAS_CONFIG_READ, - }, - VIEW_USERS: {USER_READ}, - MANAGE_USERS: {USER_CREATE, USER_DELETE, USER_READ}, -} diff --git a/src/fides/lib/oauth/roles.py b/src/fides/lib/oauth/roles.py new file mode 100644 index 0000000000..e774d6acf6 --- /dev/null +++ b/src/fides/lib/oauth/roles.py @@ -0,0 +1,100 @@ +from typing import Dict, List, Optional + +from fides.api.ops.api.v1.scope_registry import ( + CLI_OBJECTS_READ, + CLIENT_READ, + CONFIG_READ, + CONNECTION_READ, + CONNECTION_TYPE_READ, + CONSENT_READ, + DATAMAP_READ, + DATASET_READ, + MESSAGING_READ, + POLICY_READ, + PRIVACY_REQUEST_CALLBACK_RESUME, + PRIVACY_REQUEST_CREATE, + PRIVACY_REQUEST_DELETE, + PRIVACY_REQUEST_NOTIFICATIONS_CREATE_OR_UPDATE, + PRIVACY_REQUEST_NOTIFICATIONS_READ, + PRIVACY_REQUEST_READ, + PRIVACY_REQUEST_REVIEW, + PRIVACY_REQUEST_TRANSFER, + PRIVACY_REQUEST_UPLOAD_DATA, + PRIVACY_REQUEST_VIEW_DATA, + RULE_READ, + SAAS_CONFIG_READ, + SCOPE_READ, + SCOPE_REGISTRY, + STORAGE_READ, + USER_READ, + WEBHOOK_READ, +) + +PRIVACY_REQUEST_MANAGER = "privacy_request_manager" +VIEWER = "viewer" +VIEWER_AND_PRIVACY_REQUEST_MANAGER = "viewer_and_privacy_request_manager" +ADMIN = "admin" + + +ROLE_REGISTRY = { + PRIVACY_REQUEST_MANAGER, + VIEWER, + VIEWER_AND_PRIVACY_REQUEST_MANAGER, + ADMIN, +} + + +privacy_request_manager_scopes = [ + PRIVACY_REQUEST_CREATE, + PRIVACY_REQUEST_REVIEW, + PRIVACY_REQUEST_READ, + PRIVACY_REQUEST_DELETE, + PRIVACY_REQUEST_CALLBACK_RESUME, + PRIVACY_REQUEST_NOTIFICATIONS_CREATE_OR_UPDATE, + PRIVACY_REQUEST_NOTIFICATIONS_READ, + PRIVACY_REQUEST_TRANSFER, + PRIVACY_REQUEST_UPLOAD_DATA, + PRIVACY_REQUEST_VIEW_DATA, +] + + +viewer_scopes = [ # Intentionally omitted USER_PERMISSION_READ + CLI_OBJECTS_READ, + CLIENT_READ, + CONFIG_READ, + CONNECTION_READ, + CONSENT_READ, + CONNECTION_TYPE_READ, + DATAMAP_READ, + DATASET_READ, + POLICY_READ, + PRIVACY_REQUEST_READ, + PRIVACY_REQUEST_NOTIFICATIONS_READ, + RULE_READ, + SCOPE_READ, + STORAGE_READ, + MESSAGING_READ, + WEBHOOK_READ, + SAAS_CONFIG_READ, + USER_READ, +] + +roles_to_scopes_mapping: Dict[str, List] = { + ADMIN: sorted(SCOPE_REGISTRY), + VIEWER_AND_PRIVACY_REQUEST_MANAGER: sorted( + list(set(viewer_scopes + privacy_request_manager_scopes)) + ), + VIEWER: sorted(viewer_scopes), + PRIVACY_REQUEST_MANAGER: sorted(privacy_request_manager_scopes), +} + + +def get_scopes_from_roles(roles: Optional[List[str]]) -> List[str]: + """Return a list of all the scopes the user has via their role(s)""" + if not roles: + return [] + + scope_list: List[str] = [] + for role in roles: + scope_list += roles_to_scopes_mapping.get(role, []) + return [*set(scope_list)] diff --git a/src/fides/lib/oauth/schemas/user_permission.py b/src/fides/lib/oauth/schemas/user_permission.py index 87f6d711ce..24e46ee0c9 100644 --- a/src/fides/lib/oauth/schemas/user_permission.py +++ b/src/fides/lib/oauth/schemas/user_permission.py @@ -4,9 +4,14 @@ class UserPermissionsCreate(BaseSchema): - """Data required to create a FidessUserPermissions record.""" + """Data required to create a FidesUserPermissions record. - scopes: List[str] + Users will generally be assigned role(s) directly which are associated with many scopes, + but we also will continue to support the ability to be assigned specific individual scopes. + """ + + scopes: List[str] = [] + roles: List[str] = [] class UserPermissionsEdit(UserPermissionsCreate): diff --git a/tests/ctl/cli/test_cli.py b/tests/ctl/cli/test_cli.py index 369bd733d0..29d83598f5 100644 --- a/tests/ctl/cli/test_cli.py +++ b/tests/ctl/cli/test_cli.py @@ -9,8 +9,14 @@ from git.repo import Repo from py._path.local import LocalPath +from fides.api.ops.api.v1.scope_registry import SCOPE_REGISTRY from fides.cli import cli +from fides.core.config import get_config +from fides.core.user import get_user_permissions +from fides.core.utils import get_auth_header, read_credentials_file +from fides.lib.oauth.roles import ADMIN +config = get_config() OKTA_URL = "https://dev-78908748.okta.com" @@ -916,6 +922,28 @@ def test_user_create( print(result.output) assert result.exit_code == 0 + test_cli_runner.invoke( + cli, + [ + "-f", + test_config_path, + "user", + "login", + "-u", + "newuser", + "-p", + "Newpassword1!", + ], + env={"FIDES_CREDENTIALS_PATH": credentials_path}, + ) + + credentials = read_credentials_file(credentials_path) + scopes, roles = get_user_permissions( + credentials.user_id, get_auth_header(), config.cli.server_url + ) + assert scopes == SCOPE_REGISTRY + assert roles == [ADMIN] + @pytest.mark.unit def test_user_permissions_valid( self, test_config_path: str, test_cli_runner: CliRunner, credentials_path: str @@ -930,6 +958,33 @@ def test_user_permissions_valid( print(result.output) assert result.exit_code == 0 + @pytest.mark.unit + def test_get_user_permissions( + self, test_config_path, test_cli_runner, credentials_path + ) -> None: + """Test getting user permissions""" + test_cli_runner.invoke( + cli, + [ + "-f", + test_config_path, + "user", + "login", + "-u", + "root_user", + "-p", + "Testpassword1!", + ], + env={"FIDES_CREDENTIALS_PATH": credentials_path}, + ) + scopes, roles = get_user_permissions( + config.security.oauth_root_client_id, + get_auth_header(), + config.cli.server_url, + ) + assert scopes == SCOPE_REGISTRY + assert roles == [ADMIN] + @pytest.mark.unit def test_user_permissions_not_found( self, test_config_path: str, test_cli_runner: CliRunner, credentials_path: str diff --git a/tests/lib/conftest.py b/tests/lib/conftest.py index 1e6fa5c6db..fe82085566 100644 --- a/tests/lib/conftest.py +++ b/tests/lib/conftest.py @@ -24,6 +24,7 @@ from fides.lib.models.fides_user_permissions import FidesUserPermissions from fides.lib.oauth.api.routes.user_endpoints import router from fides.lib.oauth.jwt import generate_jwe +from fides.lib.oauth.roles import ADMIN, VIEWER from fides.lib.oauth.scopes import PRIVACY_REQUEST_READ, SCOPES from tests.conftest import create_citext_extension @@ -84,9 +85,33 @@ def fides_toml_path(): def oauth_client(db): """Return a client for authentication purposes.""" client = ClientDetail( - hashed_secret="thisisatest", - salt="thisisstillatest", - scopes=SCOPES, + hashed_secret="thisisatest", salt="thisisstillatest", scopes=SCOPES, roles=[] + ) + db.add(client) + db.commit() + db.refresh(client) + yield client + client.delete(db) + + +@pytest.fixture +def admin_client(db): + """Return a client with an "admin" role for authentication purposes.""" + client = ClientDetail( + hashed_secret="thisisatest", salt="thisisstillatest", scopes=[], roles=[ADMIN] + ) + db.add(client) + db.commit() + db.refresh(client) + yield client + client.delete(db) + + +@pytest.fixture +def viewer_client(db): + """Return a client with a "viewer" role for authentication purposes.""" + client = ClientDetail( + hashed_secret="thisisatest", salt="thisisstillatest", scopes=[], roles=[VIEWER] ) db.add(client) db.commit() @@ -136,6 +161,62 @@ def user(db): user.delete(db) +@pytest.fixture +def admin_user(db): + user = FidesUser.create( + db=db, + data={ + "username": "test_fides_admin_user", + "password": "TESTdcnG@wzJeu0&%3Qe2fGo7", + }, + ) + client = ClientDetail( + hashed_secret="thisisatest", + salt="thisisstillatest", + scopes=[], + roles=[ADMIN], + user_id=user.id, + ) + + FidesUserPermissions.create( + db=db, data={"user_id": user.id, "scopes": [], "roles": [ADMIN]} + ) + + db.add(client) + db.commit() + db.refresh(client) + yield user + user.delete(db) + + +@pytest.fixture +def viewer_user(db): + user = FidesUser.create( + db=db, + data={ + "username": "test_fides_viewer_user", + "password": "TESTdcnG@wzJeu0&%3Qe2fGo7", + }, + ) + client = ClientDetail( + hashed_secret="thisisatest", + salt="thisisstillatest", + scopes=[], + roles=[VIEWER], + user_id=user.id, + ) + + FidesUserPermissions.create( + db=db, data={"user_id": user.id, "scopes": [], "roles": [VIEWER]} + ) + + db.add(client) + db.commit() + db.refresh(client) + yield user + user.delete(db) + + @pytest.fixture(scope="function") def application_user(db, oauth_client): unique_username = f"user-{uuid4()}" diff --git a/tests/lib/test_client_model.py b/tests/lib/test_client_model.py index 9cc4e25d67..3145c1a2d9 100644 --- a/tests/lib/test_client_model.py +++ b/tests/lib/test_client_model.py @@ -1,11 +1,24 @@ # pylint: disable=missing-function-docstring - +import json from copy import deepcopy import pytest -from fides.lib.cryptography.cryptographic_util import hash_with_salt +from fides.api.ops.api.v1.scope_registry import DATASET_CREATE_OR_UPDATE +from fides.lib.cryptography.cryptographic_util import ( + generate_salt, + generate_secure_random_string, + hash_with_salt, +) +from fides.lib.cryptography.schemas.jwt import ( + JWE_ISSUED_AT, + JWE_PAYLOAD_CLIENT_ID, + JWE_PAYLOAD_ROLES, + JWE_PAYLOAD_SCOPES, +) from fides.lib.models.client import ClientDetail, _get_root_client_detail +from fides.lib.oauth.oauth_util import extract_payload +from fides.lib.oauth.roles import ADMIN, VIEWER from fides.lib.oauth.scopes import SCOPES @@ -24,32 +37,127 @@ def test_create_client_and_secret(db, config): ) == new_client.hashed_secret ) + assert new_client.scopes == [] + assert new_client.roles == [] + + +def test_create_client_and_secret_no_roles(db, config): + new_client, secret = ClientDetail.create_client_and_secret( + db, + config.security.oauth_client_id_length_bytes, + config.security.oauth_client_secret_length_bytes, + scopes=["user:create", "user:read"], + ) + + assert new_client.hashed_secret is not None + assert ( + hash_with_salt( + secret.encode(config.security.encoding), + new_client.salt.encode(config.security.encoding), + ) + == new_client.hashed_secret + ) + assert new_client.scopes == ["user:create", "user:read"] + assert new_client.roles == [] + + +def test_create_client_and_secret_no_scopes(db, config): + new_client, secret = ClientDetail.create_client_and_secret( + db, + config.security.oauth_client_id_length_bytes, + config.security.oauth_client_secret_length_bytes, + roles=[VIEWER], + ) + + assert new_client.hashed_secret is not None + assert ( + hash_with_salt( + secret.encode(config.security.encoding), + new_client.salt.encode(config.security.encoding), + ) + == new_client.hashed_secret + ) + assert new_client.scopes == [] + assert new_client.roles == [VIEWER] -def test_get_client(db, oauth_client, config): +def test_create_client_and_secret_scopes_and_roles(db, config): + new_client, secret = ClientDetail.create_client_and_secret( + db, + config.security.oauth_client_id_length_bytes, + config.security.oauth_client_secret_length_bytes, + roles=[VIEWER], + scopes=[DATASET_CREATE_OR_UPDATE], + ) + + assert new_client.hashed_secret is not None + assert ( + hash_with_salt( + secret.encode(config.security.encoding), + new_client.salt.encode(config.security.encoding), + ) + == new_client.hashed_secret + ) + assert new_client.scopes == [DATASET_CREATE_OR_UPDATE] + assert new_client.roles == [VIEWER] + + +def test_create_client_defaults(db): + client_id = generate_secure_random_string(16) + secret = generate_secure_random_string(16) + + salt = generate_salt() + hashed_secret = hash_with_salt( + secret.encode("UTF-8"), + salt.encode("UTF-8"), + ) + client = ClientDetail( + id=client_id, + salt=salt, + hashed_secret=hashed_secret, + ) + db.add(client) + db.commit() + + assert client.scopes == [] + assert client.roles == [] + + client.delete(db) + + +def test_get_client_with_scopes(db, oauth_client, config): client = ClientDetail.get(db, object_id=oauth_client.id, config=config) assert client assert client.id == oauth_client.id assert client.scopes == SCOPES + assert client.roles == [] assert oauth_client.hashed_secret == "thisisatest" +def test_get_client_with_roles(db, admin_client, config): + client = ClientDetail.get(db, object_id=admin_client.id, config=config) + assert client + assert client.id == admin_client.id + assert client.scopes == [] + assert client.roles == [ADMIN] + assert admin_client.hashed_secret == "thisisatest" + + def test_get_client_root_client(db, config): client = ClientDetail.get( - db, - object_id="fidesadmin", - config=config, - scopes=SCOPES, + db, object_id="fidesadmin", config=config, scopes=SCOPES, roles=[ADMIN] ) assert client assert client.id == config.security.oauth_root_client_id assert client.scopes == SCOPES + assert client.roles == [ADMIN] -def test_get_client_root_client_no_scopes(db, config): +def test_get_root_client_no_scopes(db, config): client_detail = ClientDetail.get(db, object_id="fidesadmin", config=config) assert client_detail - assert client_detail.scopes is None + assert client_detail.scopes == [] + assert client_detail.roles == [] def test_credentials_valid(db, config): @@ -62,10 +170,47 @@ def test_credentials_valid(db, config): assert new_client.credentials_valid("this-is-not-the-right-secret") is False assert new_client.credentials_valid(secret) is True + assert new_client.scopes == SCOPES + assert new_client.roles == [] def test_get_root_client_detail_no_root_client_hash(config): test_config = deepcopy(config) test_config.security.oauth_root_client_secret_hash = None with pytest.raises(ValueError): - _get_root_client_detail(test_config, SCOPES) + _get_root_client_detail(test_config, SCOPES, []) + + +def test_client_create_access_code_jwe(oauth_client, config): + jwe = oauth_client.create_access_code_jwe(config.security.app_encryption_key) + + token_data = json.loads(extract_payload(jwe, config.security.app_encryption_key)) + + assert token_data[JWE_PAYLOAD_CLIENT_ID] == oauth_client.id + assert token_data[JWE_PAYLOAD_SCOPES] == oauth_client.scopes + assert token_data[JWE_ISSUED_AT] is not None + assert token_data[JWE_PAYLOAD_ROLES] == [] + + +def test_client_create_access_code_jwe_admin_client(admin_client, config): + + jwe = admin_client.create_access_code_jwe(config.security.app_encryption_key) + + token_data = json.loads(extract_payload(jwe, config.security.app_encryption_key)) + + assert token_data[JWE_PAYLOAD_CLIENT_ID] == admin_client.id + assert token_data[JWE_PAYLOAD_SCOPES] == [] + assert token_data[JWE_ISSUED_AT] is not None + assert token_data[JWE_PAYLOAD_ROLES] == [ADMIN] + + +def test_client_create_access_code_jwe_viewer_client(viewer_client, config): + + jwe = viewer_client.create_access_code_jwe(config.security.app_encryption_key) + + token_data = json.loads(extract_payload(jwe, config.security.app_encryption_key)) + + assert token_data[JWE_PAYLOAD_CLIENT_ID] == viewer_client.id + assert token_data[JWE_PAYLOAD_SCOPES] == [] + assert token_data[JWE_ISSUED_AT] is not None + assert token_data[JWE_PAYLOAD_ROLES] == [VIEWER] diff --git a/tests/lib/test_fides_user_permissions.py b/tests/lib/test_fides_user_permissions.py index 602575f267..aefed87538 100644 --- a/tests/lib/test_fides_user_permissions.py +++ b/tests/lib/test_fides_user_permissions.py @@ -14,39 +14,3 @@ USER_DELETE, USER_READ, ) - - -def test_create_user_permissions(): - permissions: FidesUserPermissions = FidesUserPermissions.create( # type: ignore - # Not using the `db` here allows us to omit PK and FK data - db=MagicMock(), - data={"user_id": "test", "scopes": [PRIVACY_REQUEST_READ]}, - ) - - assert permissions.user_id == "test" - assert permissions.scopes == [PRIVACY_REQUEST_READ] - assert permissions.privileges == ("view_subject_requests",) - - -def test_associated_privileges(): - permissions: FidesUserPermissions = FidesUserPermissions.create( # type: ignore - # Not using the `db` here allows us to omit PK and FK data - db=MagicMock(), - data={ - "user_id": "test", - "scopes": [USER_CREATE, USER_READ, USER_DELETE, PRIVACY_REQUEST_READ], - }, - ) - - assert permissions.user_id == "test" - assert permissions.scopes == [ - USER_CREATE, - USER_READ, - USER_DELETE, - PRIVACY_REQUEST_READ, - ] - assert permissions.privileges == ( - "view_subject_requests", - "view_users", - "manage_users", - ) diff --git a/tests/lib/test_oauth_util.py b/tests/lib/test_oauth_util.py index 338f49f7ee..85fdb01053 100644 --- a/tests/lib/test_oauth_util.py +++ b/tests/lib/test_oauth_util.py @@ -6,16 +6,26 @@ import pytest from fastapi.security import SecurityScopes -from fides.api.ops.util.oauth_util import verify_oauth_client +from fides.api.ops.api.v1.scope_registry import PRIVACY_REQUEST_READ +from fides.api.ops.util.oauth_util import ( + _has_correct_scopes, + _has_direct_scopes, + _has_scope_via_role, + get_root_client, + has_permissions, + verify_oauth_client, +) from fides.lib.cryptography.schemas.jwt import ( JWE_ISSUED_AT, JWE_PAYLOAD_CLIENT_ID, + JWE_PAYLOAD_ROLES, JWE_PAYLOAD_SCOPES, ) from fides.lib.exceptions import AuthorizationError from fides.lib.oauth.jwt import generate_jwe from fides.lib.oauth.oauth_util import extract_payload, is_token_expired -from fides.lib.oauth.scopes import USER_DELETE, USER_READ +from fides.lib.oauth.roles import ADMIN, VIEWER +from fides.lib.oauth.scopes import DATASET_CREATE_OR_UPDATE, USER_DELETE, USER_READ @pytest.fixture @@ -170,3 +180,281 @@ async def test_verify_oauth_client_wrong_client_scope(db, config, user): token, db=db, ) + + +class TestVerifyOauthClientRoles: + async def test_verify_oauth_client_roles(self, db, config, admin_user): + """Test token has the correct role and the client also has the matching role""" + payload = { + JWE_PAYLOAD_ROLES: [ADMIN], + JWE_PAYLOAD_CLIENT_ID: admin_user.client.id, + JWE_ISSUED_AT: datetime.now().isoformat(), + } + token = generate_jwe( + json.dumps(payload), + config.security.app_encryption_key, + ) + client = await verify_oauth_client( + SecurityScopes([USER_READ]), + token, + db=db, + ) + assert client == admin_user.client + + async def test_no_roles_on_client(self, db, config, user): + """Test token has the correct role but that role is not on the client""" + payload = { + JWE_PAYLOAD_ROLES: [ADMIN], + JWE_PAYLOAD_CLIENT_ID: user.client.id, + JWE_ISSUED_AT: datetime.now().isoformat(), + } + token = generate_jwe( + json.dumps(payload), + config.security.app_encryption_key, + ) + with pytest.raises(AuthorizationError): + await verify_oauth_client( + SecurityScopes([USER_READ]), + token, + db=db, + ) + + async def test_no_roles_on_client_but_has_scopes_coverage(self, db, config, user): + """Test roles on token are outdated but token still has scopes coverage""" + payload = { + JWE_PAYLOAD_ROLES: [ADMIN], + JWE_PAYLOAD_CLIENT_ID: user.client.id, + JWE_ISSUED_AT: datetime.now().isoformat(), + JWE_PAYLOAD_SCOPES: [USER_READ], + } + token = generate_jwe( + json.dumps(payload), + config.security.app_encryption_key, + ) + client = await verify_oauth_client( + SecurityScopes([USER_READ]), + token, + db=db, + ) + assert client == user.client + + async def test_token_does_not_have_role_with_coverage( + self, db, config, viewer_user + ): + """Test token only has a viewer role, which is not enough to view the particular endpoint + as it is missing DATASET_CREATE_OR_UPDATE scopes + """ + payload = { + JWE_PAYLOAD_ROLES: [VIEWER], + JWE_PAYLOAD_CLIENT_ID: viewer_user.client.id, + JWE_ISSUED_AT: datetime.now().isoformat(), + JWE_PAYLOAD_SCOPES: [USER_READ], + } + token = generate_jwe( + json.dumps(payload), + config.security.app_encryption_key, + ) + + with pytest.raises(AuthorizationError): + await verify_oauth_client( + SecurityScopes([DATASET_CREATE_OR_UPDATE]), + token, + db=db, + ) + + +class TestHasCorrectScopes: + def test_missing_scopes(self): + assert not _has_correct_scopes( + user_scopes=[DATASET_CREATE_OR_UPDATE, USER_READ], + endpoint_scopes=SecurityScopes([PRIVACY_REQUEST_READ]), + ) + + def test_has_correct_scopes(self): + assert _has_correct_scopes( + user_scopes=[DATASET_CREATE_OR_UPDATE, USER_READ, PRIVACY_REQUEST_READ], + endpoint_scopes=SecurityScopes([PRIVACY_REQUEST_READ]), + ) + + +class TestHasDirectScopes: + def test_token_does_not_have_any_scopes(self, viewer_client): + assert not _has_direct_scopes( + token_data={}, + client=viewer_client, + endpoint_scopes=SecurityScopes([PRIVACY_REQUEST_READ]), + ) + + def test_missing_scopes(self, oauth_client): + token_data = { + JWE_PAYLOAD_SCOPES: [DATASET_CREATE_OR_UPDATE, USER_READ], + } + assert not _has_direct_scopes( + token_data=token_data, + client=oauth_client, + endpoint_scopes=SecurityScopes([PRIVACY_REQUEST_READ]), + ) + + def test_has_direct_scopes_but_client_outdated(self, viewer_client): + token_data = { + JWE_PAYLOAD_SCOPES: [ + DATASET_CREATE_OR_UPDATE, + USER_READ, + PRIVACY_REQUEST_READ, + ], + } + assert not _has_direct_scopes( + token_data=token_data, + client=viewer_client, + endpoint_scopes=SecurityScopes([PRIVACY_REQUEST_READ]), + ) + + def test_has_direct_scopes_and_token_valid(self, oauth_client): + token_data = { + JWE_PAYLOAD_SCOPES: [ + DATASET_CREATE_OR_UPDATE, + USER_READ, + PRIVACY_REQUEST_READ, + ], + } + assert _has_direct_scopes( + token_data=token_data, + client=oauth_client, + endpoint_scopes=SecurityScopes([PRIVACY_REQUEST_READ]), + ) + + +class TestHasScopeViaRole: + def test_token_does_not_have_any_roles(self, viewer_client): + assert not _has_scope_via_role( + token_data={}, + client=viewer_client, + endpoint_scopes=SecurityScopes([PRIVACY_REQUEST_READ]), + ) + + def test_inadequate_roles(self, viewer_client): + token_data = { + JWE_PAYLOAD_ROLES: [VIEWER], + } + assert not _has_scope_via_role( + token_data=token_data, + client=viewer_client, + endpoint_scopes=SecurityScopes([DATASET_CREATE_OR_UPDATE]), + ) + + def test_has_adequate_role_but_client_outdated(self, viewer_client): + token_data = { + JWE_PAYLOAD_ROLES: [ADMIN], + } + assert not _has_scope_via_role( + token_data=token_data, + client=viewer_client, + endpoint_scopes=SecurityScopes([DATASET_CREATE_OR_UPDATE]), + ) + + def test_has_adequate_role_and_token_valid(self, admin_client): + token_data = { + JWE_PAYLOAD_ROLES: [ADMIN], + } + assert _has_scope_via_role( + token_data=token_data, + client=admin_client, + endpoint_scopes=SecurityScopes([DATASET_CREATE_OR_UPDATE]), + ) + + +class TestHasPermissions: + def test_has_no_permissions(self, oauth_client): + token_data = {} + assert not has_permissions( + token_data, + oauth_client, + endpoint_scopes=SecurityScopes([DATASET_CREATE_OR_UPDATE]), + ) + + def test_has_direct_scope(self, oauth_client): + token_data = { + JWE_PAYLOAD_SCOPES: [DATASET_CREATE_OR_UPDATE, USER_READ], + } + assert _has_direct_scopes( + token_data, + oauth_client, + endpoint_scopes=SecurityScopes([DATASET_CREATE_OR_UPDATE]), + ) + assert not _has_scope_via_role( + token_data, + oauth_client, + endpoint_scopes=SecurityScopes([DATASET_CREATE_OR_UPDATE]), + ) + assert has_permissions( + token_data, + oauth_client, + endpoint_scopes=SecurityScopes([DATASET_CREATE_OR_UPDATE]), + ) + + def test_has_scope_via_role(self, admin_client): + token_data = { + JWE_PAYLOAD_ROLES: [ADMIN], + } + assert not _has_direct_scopes( + token_data, + admin_client, + endpoint_scopes=SecurityScopes([DATASET_CREATE_OR_UPDATE]), + ) + assert _has_scope_via_role( + token_data, + admin_client, + endpoint_scopes=SecurityScopes([DATASET_CREATE_OR_UPDATE]), + ) + + assert has_permissions( + token_data, + admin_client, + endpoint_scopes=SecurityScopes([DATASET_CREATE_OR_UPDATE]), + ) + + async def test_has_scope_directly_and_via_role(self): + root_client = await get_root_client() + token_data = { + JWE_PAYLOAD_ROLES: [ADMIN], + JWE_PAYLOAD_SCOPES: [DATASET_CREATE_OR_UPDATE, USER_READ], + } + assert _has_direct_scopes( + token_data, + root_client, + endpoint_scopes=SecurityScopes([DATASET_CREATE_OR_UPDATE]), + ) + assert _has_scope_via_role( + token_data, + root_client, + endpoint_scopes=SecurityScopes([DATASET_CREATE_OR_UPDATE]), + ) + + assert has_permissions( + token_data, + root_client, + endpoint_scopes=SecurityScopes([DATASET_CREATE_OR_UPDATE]), + ) + + async def test_has_no_direct_scopes_or_via_role(self): + root_client = await get_root_client() + token_data = { + JWE_PAYLOAD_ROLES: [VIEWER], + JWE_PAYLOAD_SCOPES: [USER_READ], + } + assert not _has_direct_scopes( + token_data, + root_client, + endpoint_scopes=SecurityScopes([DATASET_CREATE_OR_UPDATE]), + ) + assert not _has_scope_via_role( + token_data, + root_client, + endpoint_scopes=SecurityScopes([DATASET_CREATE_OR_UPDATE]), + ) + + assert not has_permissions( + token_data, + root_client, + endpoint_scopes=SecurityScopes([DATASET_CREATE_OR_UPDATE]), + ) diff --git a/tests/ops/api/v1/endpoints/test_connection_config_endpoints.py b/tests/ops/api/v1/endpoints/test_connection_config_endpoints.py index 74b206316d..ca6b91bcb6 100644 --- a/tests/ops/api/v1/endpoints/test_connection_config_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_connection_config_endpoints.py @@ -33,6 +33,7 @@ from fides.api.ops.schemas.messaging.messaging import MessagingActionType from fides.api.ops.schemas.redis_cache import Identity from fides.lib.models.client import ClientDetail +from fides.lib.oauth.roles import ADMIN, PRIVACY_REQUEST_MANAGER, VIEWER from tests.ops.fixtures.application_fixtures import integration_secrets page_size = Params().size @@ -82,6 +83,35 @@ def test_patch_connections_incorrect_scope( response = api_client.patch(url, headers=auth_header, json=payload) assert 403 == response.status_code + def test_patch_connections_viewer_role( + self, api_client: TestClient, generate_role_header, url, payload + ) -> None: + auth_header = generate_role_header(roles=[VIEWER]) + response = api_client.patch(url, headers=auth_header, json=payload) + assert 403 == response.status_code + + def test_patch_connections_privacy_request_manager_role( + self, api_client: TestClient, generate_role_header, url, payload + ) -> None: + auth_header = generate_role_header(roles=[PRIVACY_REQUEST_MANAGER]) + response = api_client.patch(url, headers=auth_header, json=payload) + assert 403 == response.status_code + + def test_patch_connection_admin_role( + self, url, api_client, db: Session, generate_role_header + ): + auth_header = generate_role_header(roles=[ADMIN]) + payload = [ + { + "name": "My Post-Execution Webhook", + "key": "webhook_key", + "connection_type": "https", + "access": "read", + } + ] + response = api_client.patch(url, headers=auth_header, json=payload) + assert 200 == response.status_code + def test_patch_http_connection( self, url, api_client, db: Session, generate_auth_header ): diff --git a/tests/ops/api/v1/endpoints/test_oauth_endpoints.py b/tests/ops/api/v1/endpoints/test_oauth_endpoints.py index 27fd9bb959..0d5776bbd5 100644 --- a/tests/ops/api/v1/endpoints/test_oauth_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_oauth_endpoints.py @@ -20,6 +20,7 @@ CLIENT_BY_ID, CLIENT_SCOPE, OAUTH_CALLBACK, + ROLE, SCOPE, TOKEN, V1_URL_PREFIX, @@ -31,11 +32,13 @@ from fides.lib.cryptography.schemas.jwt import ( JWE_ISSUED_AT, JWE_PAYLOAD_CLIENT_ID, + JWE_PAYLOAD_ROLES, JWE_PAYLOAD_SCOPES, ) from fides.lib.models.client import ClientDetail from fides.lib.oauth.jwt import generate_jwe from fides.lib.oauth.oauth_util import extract_payload +from fides.lib.oauth.roles import ADMIN, ROLE_REGISTRY CONFIG = get_config() @@ -425,18 +428,14 @@ def test_get_access_token_root_client(self, url, api_client): response = api_client.post(url, data=data) jwt = json.loads(response.text).get("access_token") assert 200 == response.status_code - assert ( - data["client_id"] - == json.loads(extract_payload(jwt, CONFIG.security.app_encryption_key))[ - JWE_PAYLOAD_CLIENT_ID - ] - ) - assert ( - json.loads(extract_payload(jwt, CONFIG.security.app_encryption_key))[ - JWE_PAYLOAD_SCOPES - ] - == SCOPE_REGISTRY + + extracted_token = json.loads( + extract_payload(jwt, CONFIG.security.app_encryption_key) ) + assert data["client_id"] == extracted_token[JWE_PAYLOAD_CLIENT_ID] + assert extracted_token[JWE_PAYLOAD_SCOPES] == SCOPE_REGISTRY + + assert extracted_token[JWE_PAYLOAD_ROLES] == [ADMIN] def test_get_access_token(self, db, url, api_client): new_client, secret = ClientDetail.create_client_and_secret( @@ -538,3 +537,43 @@ def test_callback_for_valid_state_with_token_error( assert response.json() == {"detail": "Unable to retrieve access token."} authentication_request.delete(db) + + +class TestReadRoleMapping: + @pytest.fixture(scope="function") + def url(self, oauth_client) -> str: + return V1_URL_PREFIX + ROLE + + def test_get_roles_not_authenticated(self, api_client: TestClient, url): + response = api_client.get(url) + assert response.status_code == 401 + + def test_get_roles_wrong_scope( + self, + api_client: TestClient, + url, + generate_auth_header, + ) -> None: + auth_header = generate_auth_header([STORAGE_READ]) + response = api_client.get(url, headers=auth_header) + assert 403 == response.status_code + + def test_get_role_scope_mapping( + self, + api_client: TestClient, + url, + generate_auth_header, + ) -> None: + auth_header = generate_auth_header([SCOPE_READ]) + + response = api_client.get(url, headers=auth_header) + response_body = json.loads(response.text) + + assert 200 == response.status_code + assert set(response_body["admin"]) == set(SCOPE_REGISTRY) + assert set(response_body.keys()) == { + "admin", + "viewer_and_privacy_request_manager", + "privacy_request_manager", + "viewer", + } diff --git a/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py b/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py index 3301eeb70c..1c24905c5b 100644 --- a/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py @@ -93,6 +93,7 @@ from fides.lib.models.audit_log import AuditLog, AuditLogAction from fides.lib.models.client import ClientDetail from fides.lib.oauth.jwt import generate_jwe +from fides.lib.oauth.roles import PRIVACY_REQUEST_MANAGER, VIEWER CONFIG = get_config() page_size = Params().size @@ -593,6 +594,13 @@ def test_get_privacy_requests_wrong_scope( response = api_client.get(url, headers=auth_header) assert 403 == response.status_code + def test_get_privacy_requests_privacy_request_manager_role( + self, api_client: TestClient, generate_role_header, url + ): + auth_header = generate_role_header(roles=[PRIVACY_REQUEST_MANAGER]) + response = api_client.get(url, headers=auth_header) + assert 200 == response.status_code + def test_conflicting_query_params( self, api_client: TestClient, generate_auth_header, url ): @@ -1809,6 +1817,27 @@ def test_approve_privacy_request_bad_scopes( response = api_client.patch(url, headers=auth_header) assert response.status_code == 403 + def test_approve_privacy_request_viewer_role( + self, url, api_client, generate_role_header + ): + auth_header = generate_role_header(roles=[VIEWER]) + response = api_client.patch(url, headers=auth_header) + assert response.status_code == 403 + + @mock.patch( + "fides.api.ops.service.privacy_request.request_runner_service.run_privacy_request.delay" + ) + def test_approve_privacy_request_privacy_request_manager_role( + self, _, url, api_client, generate_role_header, privacy_request, db + ): + privacy_request.status = PrivacyRequestStatus.pending + privacy_request.save(db=db) + + auth_header = generate_role_header(roles=[PRIVACY_REQUEST_MANAGER]) + body = {"request_ids": [privacy_request.id]} + response = api_client.patch(url, headers=auth_header, json=body) + assert response.status_code == 200 + @mock.patch( "fides.api.ops.service.privacy_request.request_runner_service.run_privacy_request.delay" ) diff --git a/tests/ops/api/v1/endpoints/test_user_endpoints.py b/tests/ops/api/v1/endpoints/test_user_endpoints.py index f23ef4cb95..257bd7fad1 100644 --- a/tests/ops/api/v1/endpoints/test_user_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_user_endpoints.py @@ -44,6 +44,7 @@ from fides.lib.models.fides_user_permissions import FidesUserPermissions from fides.lib.oauth.jwt import generate_jwe from fides.lib.oauth.oauth_util import extract_payload +from fides.lib.oauth.roles import ADMIN, VIEWER from tests.ops.conftest import generate_auth_header_for_user CONFIG = get_config() @@ -831,7 +832,7 @@ class TestUserLogin: def url(self) -> str: return V1_URL_PREFIX + LOGIN - def test_user_does_not_exist(self, db, url, api_client): + def test_user_does_not_exist(self, url, api_client): body = { "username": "does not exist", "password": str_to_b64_str("idonotknowmypassword"), @@ -839,7 +840,7 @@ def test_user_does_not_exist(self, db, url, api_client): response = api_client.post(url, headers={}, json=body) assert response.status_code == HTTP_404_NOT_FOUND - def test_bad_login(self, db, url, user, api_client): + def test_bad_login(self, url, user, api_client): body = { "username": user.username, "password": str_to_b64_str("idonotknowmypassword"), @@ -875,6 +876,62 @@ def test_login_creates_client(self, db, url, user, api_client): assert "user_data" in list(response.json().keys()) assert response.json()["user_data"]["id"] == user.id + def test_login_with_scopes(self, db, url, user, api_client): + # Delete existing client for test purposes + user.client.delete(db) + body = { + "username": user.username, + "password": str_to_b64_str("TESTdcnG@wzJeu0&%3Qe2fGo7"), + } + + assert user.client is None # client does not exist + assert user.permissions is not None + response = api_client.post(url, headers={}, json=body) + assert response.status_code == HTTP_200_OK + + db.refresh(user) + assert user.client is not None + assert "token_data" in list(response.json().keys()) + token = response.json()["token_data"]["access_token"] + token_data = json.loads( + extract_payload(token, CONFIG.security.app_encryption_key) + ) + assert token_data["client-id"] == user.client.id + assert token_data["scopes"] == [ + PRIVACY_REQUEST_READ + ] # Uses scopes on existing client + assert token_data["roles"] == [] + + assert "user_data" in list(response.json().keys()) + assert response.json()["user_data"]["id"] == user.id + + def test_login_with_roles(self, db, url, viewer_user, api_client): + # Delete existing client for test purposes + viewer_user.client.delete(db) + body = { + "username": viewer_user.username, + "password": str_to_b64_str("TESTdcnG@wzJeu0&%3Qe2fGo7"), + } + + assert viewer_user.client is None # client does not exist + assert viewer_user.permissions is not None + response = api_client.post(url, headers={}, json=body) + assert response.status_code == HTTP_200_OK + + db.refresh(viewer_user) + assert viewer_user.client is not None + assert "token_data" in list(response.json().keys()) + token = response.json()["token_data"]["access_token"] + token_data = json.loads( + extract_payload(token, CONFIG.security.app_encryption_key) + ) + assert token_data["client-id"] == viewer_user.client.id + assert token_data["scopes"] == [] # Uses scopes on existing client + assert token_data["roles"] == [VIEWER] # Uses roles on existing client + + assert "user_data" in list(response.json().keys()) + assert response.json()["user_data"]["id"] == viewer_user.id + def test_login_updates_last_login_date(self, db, url, user, api_client): body = { "username": user.username, @@ -888,7 +945,7 @@ def test_login_updates_last_login_date(self, db, url, user, api_client): assert user.last_login_at is not None def test_login_is_case_insensitive( - self, db, url, user, api_client, generate_auth_header + self, url, user, api_client, generate_auth_header ): body = { "username": user.username.lower(), @@ -910,7 +967,7 @@ def test_login_is_case_insensitive( response = api_client.post(url, headers={}, json=body) assert response.status_code == HTTP_200_OK - def test_login_uses_existing_client(self, db, url, user, api_client): + def test_login_uses_existing_client_with_scopes(self, db, url, user, api_client): body = { "username": user.username, "password": str_to_b64_str("TESTdcnG@wzJeu0&%3Qe2fGo7"), @@ -937,6 +994,52 @@ def test_login_uses_existing_client(self, db, url, user, api_client): assert "user_data" in list(response.json().keys()) assert response.json()["user_data"]["id"] == user.id + def test_login_uses_existing_client_with_roles(self, url, admin_user, api_client): + body = { + "username": admin_user.username, + "password": str_to_b64_str("TESTdcnG@wzJeu0&%3Qe2fGo7"), + } + + existing_client_id = admin_user.client.id + response = api_client.post(url, headers={}, json=body) + assert response.status_code == HTTP_200_OK + + assert admin_user.client is not None + assert "token_data" in list(response.json().keys()) + token = response.json()["token_data"]["access_token"] + token_data = json.loads( + extract_payload(token, CONFIG.security.app_encryption_key) + ) + assert token_data["client-id"] == existing_client_id + assert token_data["scopes"] == [] + assert token_data["roles"] == [ADMIN] # Uses roles on existing client + + assert "user_data" in list(response.json().keys()) + assert response.json()["user_data"]["id"] == admin_user.id + + def test_login_as_root_user(self, api_client, url): + body = { + "username": CONFIG.security.root_username, + "password": str_to_b64_str(CONFIG.security.root_password), + } + + response = api_client.post(url, headers={}, json=body) + assert response.status_code == HTTP_200_OK + + assert "token_data" in list(response.json().keys()) + token = response.json()["token_data"]["access_token"] + token_data = json.loads( + extract_payload(token, CONFIG.security.app_encryption_key) + ) + assert token_data["client-id"] == CONFIG.security.oauth_root_client_id + assert token_data["scopes"] == SCOPE_REGISTRY # Uses all scopes + assert token_data["roles"] == [ADMIN] # Uses admin role + + assert "user_data" in list(response.json().keys()) + data = response.json()["user_data"] + assert data["username"] == CONFIG.security.root_username + assert data["id"] == CONFIG.security.oauth_root_client_id + class TestUserLogout: @pytest.fixture(scope="function") diff --git a/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py b/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py index 16058db17e..cb6999dc06 100644 --- a/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py @@ -23,7 +23,11 @@ from fides.lib.models.client import ClientDetail from fides.lib.models.fides_user import FidesUser from fides.lib.models.fides_user_permissions import FidesUserPermissions -from tests.ops.conftest import generate_auth_header_for_user +from fides.lib.oauth.roles import ADMIN, VIEWER +from tests.ops.conftest import ( + generate_auth_header_for_user, + generate_role_header_for_user, +) CONFIG = get_config() @@ -77,7 +81,7 @@ def test_create_user_permissions_invalid_user_id( assert HTTP_404_NOT_FOUND == response.status_code assert permissions is None - def test_create_user_permissions( + def test_create_user_permissions_add_scopes( self, db, api_client, generate_auth_header ) -> None: auth_header = generate_auth_header([USER_PERMISSION_CREATE]) @@ -85,6 +89,15 @@ def test_create_user_permissions( db=db, data={"username": "user_1", "password": "test_password"}, ) + client = ClientDetail( + hashed_secret="thisisatest", + salt="thisisstillatest", + scopes=[], + roles=[VIEWER], + user_id=user.id, + ) + db.add(client) + db.commit() body = {"user_id": user.id, "scopes": [PRIVACY_REQUEST_READ]} response = api_client.post( @@ -95,6 +108,89 @@ def test_create_user_permissions( assert HTTP_201_CREATED == response.status_code assert response_body["id"] == permissions.id assert permissions.scopes == [PRIVACY_REQUEST_READ] + assert ( + user.client.roles == [] + ), "Roles not specified in request so they were overridden" + user.delete(db) + + def test_create_user_permissions_add_bad_role( + self, db, api_client, generate_auth_header + ) -> None: + auth_header = generate_auth_header([USER_PERMISSION_CREATE]) + user = FidesUser.create( + db=db, + data={"username": "user_1", "password": "test_password"}, + ) + + body = {"user_id": user.id, "roles": ["nonexistent role"]} + response = api_client.post( + f"{V1_URL_PREFIX}/user/{user.id}/permission", headers=auth_header, json=body + ) + response_body = response.json() + assert HTTP_422_UNPROCESSABLE_ENTITY == response.status_code + assert "Invalid Role(s) {'nonexistent role'}." in response_body["detail"] + + def test_create_user_permissions_add_roles( + self, db, api_client, generate_auth_header + ) -> None: + auth_header = generate_auth_header([USER_PERMISSION_CREATE]) + user = FidesUser.create( + db=db, + data={"username": "user_1", "password": "test_password"}, + ) + client = ClientDetail( + hashed_secret="thisisatest", + salt="thisisstillatest", + scopes=[PRIVACY_REQUEST_READ], + roles=[], + user_id=user.id, + ) + db.add(client) + db.commit() + + body = {"user_id": user.id, "roles": [VIEWER]} + response = api_client.post( + f"{V1_URL_PREFIX}/user/{user.id}/permission", headers=auth_header, json=body + ) + permissions = FidesUserPermissions.get_by(db, field="user_id", value=user.id) + response_body = response.json() + assert HTTP_201_CREATED == response.status_code + assert response_body["id"] == permissions.id + assert permissions.scopes == [] + assert permissions.roles == [VIEWER] + assert client.scopes == [], "No scopes in body so they were overridden" + user.delete(db) + + def test_create_roles_on_permission_object_and_client( + self, db, api_client, generate_auth_header + ) -> None: + auth_header = generate_auth_header([USER_PERMISSION_CREATE]) + user = FidesUser.create( + db=db, + data={"username": "user_1", "password": "test_password"}, + ) + client = ClientDetail( + hashed_secret="thisisatest", + salt="thisisstillatest", + scopes=[], + roles=[VIEWER], + user_id=user.id, + ) + db.add(client) + db.commit() + + body = {"user_id": user.id, "roles": [ADMIN]} + response = api_client.post( + f"{V1_URL_PREFIX}/user/{user.id}/permission", headers=auth_header, json=body + ) + permissions = FidesUserPermissions.get_by(db, field="user_id", value=user.id) + response_body = response.json() + assert HTTP_201_CREATED == response.status_code + assert response_body["id"] == permissions.id + assert permissions.scopes == [] + assert permissions.roles == [ADMIN] + db.refresh(client) + assert client.roles == [ADMIN] user.delete(db) @@ -154,7 +250,7 @@ def test_edit_user_permissions_invalid_user_id( assert permissions is None user.delete(db) - def test_edit_user_permissions(self, db, api_client, generate_auth_header) -> None: + def test_edit_user_scopes(self, db, api_client, generate_auth_header) -> None: auth_header = generate_auth_header([USER_PERMISSION_UPDATE]) user = FidesUser.create( db=db, @@ -162,7 +258,12 @@ def test_edit_user_permissions(self, db, api_client, generate_auth_header) -> No ) permissions = FidesUserPermissions.create( - db=db, data={"user_id": user.id, "scopes": [PRIVACY_REQUEST_READ]} + db=db, + data={ + "user_id": user.id, + "scopes": [PRIVACY_REQUEST_READ], + "roles": [VIEWER], + }, ) ClientDetail.create_client_and_secret( @@ -170,6 +271,7 @@ def test_edit_user_permissions(self, db, api_client, generate_auth_header) -> No CONFIG.security.oauth_client_id_length_bytes, CONFIG.security.oauth_client_secret_length_bytes, scopes=[PRIVACY_REQUEST_READ], + roles=[VIEWER], user_id=user.id, ) @@ -179,11 +281,67 @@ def test_edit_user_permissions(self, db, api_client, generate_auth_header) -> No f"{V1_URL_PREFIX}/user/{user.id}/permission", headers=auth_header, json=body ) response_body = response.json() - 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["roles"] == [] + ), "Roles removed as they were not specified in the request" + + client: ClientDetail = ClientDetail.get_by(db, field="user_id", value=user.id) assert client.scopes == updated_scopes + assert client.roles == [] + + db.refresh(permissions) + assert permissions.scopes == updated_scopes + assert permissions.roles == [] + + user.delete(db) + + def test_edit_user_roles(self, db, api_client, generate_auth_header) -> None: + auth_header = generate_auth_header([USER_PERMISSION_UPDATE]) + user = FidesUser.create( + db=db, + data={"username": "user_1", "password": "test_password"}, + ) + + permissions = FidesUserPermissions.create( + db=db, + data={ + "user_id": user.id, + "scopes": [PRIVACY_REQUEST_READ], + "roles": [VIEWER], + }, + ) + + ClientDetail.create_client_and_secret( + db, + CONFIG.security.oauth_client_id_length_bytes, + CONFIG.security.oauth_client_secret_length_bytes, + scopes=[PRIVACY_REQUEST_READ], + roles=[VIEWER], + user_id=user.id, + ) + + body = {"id": permissions.id, "roles": [ADMIN]} + response = api_client.put( + f"{V1_URL_PREFIX}/user/{user.id}/permission", headers=auth_header, json=body + ) + response_body = response.json() + assert HTTP_200_OK == response.status_code + assert response_body["id"] == permissions.id + assert ( + response_body["scopes"] == [] + ), "Scopes removed as they were not specified in the request" + assert response_body["roles"] == [ADMIN] + + client: ClientDetail = ClientDetail.get_by(db, field="user_id", value=user.id) + assert client.scopes == [] + assert client.roles == [ADMIN] + + db.refresh(permissions) + assert permissions.scopes == [] + assert permissions.roles == [ADMIN] user.delete(db) @@ -278,6 +436,7 @@ def test_get_user_permissions( assert response_body["id"] == permissions.id assert response_body["user_id"] == user.id assert response_body["scopes"] == [PRIVACY_REQUEST_READ] + assert response_body["roles"] == [] def test_get_current_user_permissions(self, db, api_client, auth_user) -> None: # Note: Does not include USER_PERMISSION_READ. @@ -303,6 +462,7 @@ def test_get_current_user_permissions(self, db, api_client, auth_user) -> None: assert response_body["id"] == permissions.id assert response_body["user_id"] == auth_user.id assert response_body["scopes"] == scopes + assert response_body["roles"] == [] def test_get_current_root_user_permissions( self, api_client, oauth_root_client, root_auth_header @@ -316,6 +476,7 @@ def test_get_current_root_user_permissions( assert response_body["id"] == oauth_root_client.id assert response_body["user_id"] == oauth_root_client.id assert response_body["scopes"] == SCOPE_REGISTRY + assert response_body["roles"] == [ADMIN] def test_get_root_user_permissions_by_non_root_user( self, db, api_client, oauth_root_client, auth_user @@ -336,3 +497,74 @@ def test_get_root_user_permissions_by_non_root_user( headers=auth_header, ) assert HTTP_404_NOT_FOUND == response.status_code + + def test_get_own_user_roles(self, db, api_client, auth_user): + ClientDetail.create_client_and_secret( + db, + CONFIG.security.oauth_client_id_length_bytes, + CONFIG.security.oauth_client_secret_length_bytes, + roles=[VIEWER], + user_id=auth_user.id, + ) + FidesUserPermissions.create( + db=db, data={"user_id": auth_user.id, "roles": [VIEWER]} + ) + + auth_header = generate_role_header_for_user(auth_user, roles=[VIEWER]) + response = api_client.get( + f"{V1_URL_PREFIX}/user/{auth_user.id}/permission", + headers=auth_header, + ) + resp = response.json() + assert resp["scopes"] == [] + assert resp["roles"] == [VIEWER] + assert resp["user_id"] == auth_user.id + + def test_get_other_user_roles_as_root( + self, db, api_client, auth_user, root_auth_header + ): + + FidesUserPermissions.create( + db=db, data={"user_id": auth_user.id, "roles": [VIEWER]} + ) + + response = api_client.get( + f"{V1_URL_PREFIX}/user/{auth_user.id}/permission", + headers=root_auth_header, + ) + resp = response.json() + assert resp["scopes"] == [] + assert resp["roles"] == [VIEWER] + assert resp["user_id"] == auth_user.id + + def test_get_other_user_roles_as_viewer( + self, db, api_client, auth_user, viewer_user + ): + FidesUserPermissions.create( + db=db, data={"user_id": auth_user.id, "roles": [VIEWER]} + ) + + auth_header = generate_role_header_for_user(viewer_user, roles=[VIEWER]) + + response = api_client.get( + f"{V1_URL_PREFIX}/user/{auth_user.id}/permission", + headers=auth_header, + ) + assert response.status_code == 403 + + def test_get_other_user_roles_as_admin(self, db, api_client, auth_user, admin_user): + FidesUserPermissions.create( + db=db, data={"user_id": auth_user.id, "roles": [VIEWER]} + ) + + auth_header = generate_role_header_for_user(admin_user, roles=[ADMIN]) + + response = api_client.get( + f"{V1_URL_PREFIX}/user/{auth_user.id}/permission", + headers=auth_header, + ) + assert response.status_code == 200 + resp = response.json() + assert resp["scopes"] == [] + assert resp["roles"] == [VIEWER] + assert resp["user_id"] == auth_user.id diff --git a/tests/ops/conftest.py b/tests/ops/conftest.py index 1782bd6900..1ea94dc375 100644 --- a/tests/ops/conftest.py +++ b/tests/ops/conftest.py @@ -22,11 +22,16 @@ from fides.lib.cryptography.schemas.jwt import ( JWE_ISSUED_AT, JWE_PAYLOAD_CLIENT_ID, + JWE_PAYLOAD_ROLES, JWE_PAYLOAD_SCOPES, ) from fides.lib.db.session import Session, get_db_engine, get_db_session from fides.lib.models.client import ClientDetail from fides.lib.oauth.jwt import generate_jwe +from fides.lib.oauth.roles import ( + PRIVACY_REQUEST_MANAGER, + VIEWER_AND_PRIVACY_REQUEST_MANAGER, +) from tests.conftest import create_citext_extension from .fixtures.application_fixtures import * @@ -155,6 +160,25 @@ def oauth_client(db: Session) -> Generator: yield client +@pytest.fixture(scope="function") +def oauth_role_client(db: Session) -> Generator: + """Return a client that has all the roles for authentication purposes""" + client = ClientDetail( + hashed_secret="thisisatest", + salt="thisisstillatest", + roles=[ + ADMIN, + PRIVACY_REQUEST_MANAGER, + VIEWER, + VIEWER_AND_PRIVACY_REQUEST_MANAGER, + ], + ) + db.add(client) + db.commit() + db.refresh(client) + yield client + + @pytest.fixture(scope="function") def oauth_root_client(db: Session) -> ClientDetail: """Return the configured root client (never persisted)""" @@ -188,11 +212,28 @@ def generate_auth_header_for_user(user, scopes) -> Dict[str, str]: return {"Authorization": "Bearer " + jwe} +def generate_role_header_for_user(user, roles) -> Dict[str, str]: + payload = { + JWE_PAYLOAD_ROLES: roles, + JWE_PAYLOAD_CLIENT_ID: user.client.id, + JWE_ISSUED_AT: datetime.now().isoformat(), + } + jwe = generate_jwe(json.dumps(payload), CONFIG.security.app_encryption_key) + return {"Authorization": "Bearer " + jwe} + + @pytest.fixture(scope="function") def generate_auth_header(oauth_client) -> Callable[[Any], Dict[str, str]]: return _generate_auth_header(oauth_client, CONFIG.security.app_encryption_key) +@pytest.fixture(scope="function") +def generate_role_header(oauth_role_client) -> Callable[[Any], Dict[str, str]]: + return _generate_auth_role_header( + oauth_role_client, CONFIG.security.app_encryption_key + ) + + @pytest.fixture def generate_auth_header_ctl_config(oauth_client) -> Callable[[Any], Dict[str, str]]: return _generate_auth_header(oauth_client, CONFIG.security.app_encryption_key) @@ -215,6 +256,23 @@ def _build_jwt(scopes: List[str]) -> Dict[str, str]: return _build_jwt +def _generate_auth_role_header( + oauth_role_client, app_encryption_key +) -> Callable[[Any], Dict[str, str]]: + client_id = oauth_role_client.id + + def _build_jwt(roles: List[str]) -> Dict[str, str]: + payload = { + JWE_PAYLOAD_ROLES: roles, + JWE_PAYLOAD_CLIENT_ID: client_id, + JWE_ISSUED_AT: datetime.now().isoformat(), + } + jwe = generate_jwe(json.dumps(payload), app_encryption_key) + return {"Authorization": "Bearer " + jwe} + + return _build_jwt + + @pytest.fixture(scope="function") def generate_webhook_auth_header() -> Callable[[Any], Dict[str, str]]: def _build_jwt(webhook: PolicyPreWebhook) -> Dict[str, str]: diff --git a/tests/ops/fixtures/application_fixtures.py b/tests/ops/fixtures/application_fixtures.py index 6444ee67f5..9a4af3b928 100644 --- a/tests/ops/fixtures/application_fixtures.py +++ b/tests/ops/fixtures/application_fixtures.py @@ -70,6 +70,7 @@ from fides.lib.models.client import ClientDetail from fides.lib.models.fides_user import FidesUser from fides.lib.models.fides_user_permissions import FidesUserPermissions +from fides.lib.oauth.roles import ADMIN, VIEWER logging.getLogger("faker").setLevel(logging.ERROR) # disable verbose faker logging @@ -1601,3 +1602,59 @@ def system(db: Session) -> System: }, ) return system + + +@pytest.fixture +def admin_user(db): + user = FidesUser.create( + db=db, + data={ + "username": "test_fides_admin_user", + "password": "TESTdcnG@wzJeu0&%3Qe2fGo7", + }, + ) + client = ClientDetail( + hashed_secret="thisisatest", + salt="thisisstillatest", + scopes=[], + roles=[ADMIN], + user_id=user.id, + ) + + FidesUserPermissions.create( + db=db, data={"user_id": user.id, "scopes": [], "roles": [ADMIN]} + ) + + db.add(client) + db.commit() + db.refresh(client) + yield user + user.delete(db) + + +@pytest.fixture +def viewer_user(db): + user = FidesUser.create( + db=db, + data={ + "username": "test_fides_viewer_user", + "password": "TESTdcnG@wzJeu0&%3Qe2fGo7", + }, + ) + client = ClientDetail( + hashed_secret="thisisatest", + salt="thisisstillatest", + scopes=[], + roles=[VIEWER], + user_id=user.id, + ) + + FidesUserPermissions.create( + db=db, data={"user_id": user.id, "scopes": [], "roles": [VIEWER]} + ) + + db.add(client) + db.commit() + db.refresh(client) + yield user + user.delete(db) diff --git a/tests/ops/service/privacy_request/test_consent_email_batch_send.py b/tests/ops/service/privacy_request/test_consent_email_batch_send.py index 45fcc73e20..ccf1c8960e 100644 --- a/tests/ops/service/privacy_request/test_consent_email_batch_send.py +++ b/tests/ops/service/privacy_request/test_consent_email_batch_send.py @@ -272,6 +272,12 @@ def test_send_consent_email_multiple_users( call_kwargs = send_single_consent_email.call_args.kwargs + user_consent_preferences = call_kwargs["user_consent_preferences"] + assert {"12345", "abcde"} == { + consent_pref.identities["ljt_readerID"] + for consent_pref in user_consent_preferences + } + assert call_kwargs["user_consent_preferences"] == [ ConsentPreferencesByUser( identities={"ljt_readerID": "12345"}, From ecdcb7a066cfcccaac4d21b72b77c76ebe1267d9 Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Wed, 22 Feb 2023 17:35:21 -0600 Subject: [PATCH 02/12] Fix permissions id being inadvertently updated. --- .../ops/api/v1/endpoints/user_permission_endpoints.py | 6 ++++-- .../api/v1/endpoints/test_user_permission_endpoints.py | 9 ++++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/fides/api/ops/api/v1/endpoints/user_permission_endpoints.py b/src/fides/api/ops/api/v1/endpoints/user_permission_endpoints.py index ac8c3e2370..33538c6bcf 100644 --- a/src/fides/api/ops/api/v1/endpoints/user_permission_endpoints.py +++ b/src/fides/api/ops/api/v1/endpoints/user_permission_endpoints.py @@ -92,10 +92,12 @@ def update_user_permissions( logger.info("Updated FidesUserPermission record") if user.client: - user.client.update(db=db, data=permissions.dict()) + user.client.update( + db=db, data={"scopes": permissions.scopes, "roles": permissions.roles} + ) return user.permissions.update( # type: ignore[attr-defined] db=db, - data={"id": user.permissions.id, "user_id": user_id, **permissions.dict()}, # type: ignore[attr-defined] + data={"id": user.permissions.id, "user_id": user_id, "scopes": permissions.scopes, "roles": permissions.roles}, # type: ignore[attr-defined] ) diff --git a/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py b/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py index cb6999dc06..fbaa45f8ec 100644 --- a/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py @@ -265,6 +265,7 @@ def test_edit_user_scopes(self, db, api_client, generate_auth_header) -> None: "roles": [VIEWER], }, ) + permissions_id = permissions.id ClientDetail.create_client_and_secret( db, @@ -276,17 +277,19 @@ def test_edit_user_scopes(self, db, api_client, generate_auth_header) -> None: ) updated_scopes = [PRIVACY_REQUEST_READ, SAAS_CONFIG_READ] - body = {"id": permissions.id, "scopes": updated_scopes} + # Note: The permissions id should not be in the request body. + # Verify that we ignore this id + body = {"id": "invalid_id", "scopes": updated_scopes} response = api_client.put( f"{V1_URL_PREFIX}/user/{user.id}/permission", headers=auth_header, json=body ) response_body = response.json() assert HTTP_200_OK == response.status_code - assert response_body["id"] == permissions.id assert response_body["scopes"] == updated_scopes assert ( response_body["roles"] == [] ), "Roles removed as they were not specified in the request" + assert response_body["id"] == permissions_id client: ClientDetail = ClientDetail.get_by(db, field="user_id", value=user.id) assert client.scopes == updated_scopes @@ -295,6 +298,7 @@ def test_edit_user_scopes(self, db, api_client, generate_auth_header) -> None: db.refresh(permissions) assert permissions.scopes == updated_scopes assert permissions.roles == [] + assert permissions.id == permissions_id user.delete(db) @@ -329,7 +333,6 @@ def test_edit_user_roles(self, db, api_client, generate_auth_header) -> None: ) response_body = response.json() assert HTTP_200_OK == response.status_code - assert response_body["id"] == permissions.id assert ( response_body["scopes"] == [] ), "Scopes removed as they were not specified in the request" From 573e4afa5ac319781cae61012d7ea37fc2aa6fb4 Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Wed, 22 Feb 2023 20:23:47 -0600 Subject: [PATCH 03/12] Fix scopes validation on edit user permissions was not actually working - likely because we had two sets of user permissions schemas from the fideslib merge. - Make the "id" optional in the request to edit user permissions. The UI still passes this in but it's not used and we don't need it. - Annotate the db_dataset with the new fields. --- .fides/db_dataset.yml | 8 +++ src/fides/api/ops/schemas/user_permission.py | 37 ++++++----- .../lib/oauth/schemas/user_permission.py | 27 -------- .../test_user_permission_endpoints.py | 64 +++++++++++++++++-- 4 files changed, 89 insertions(+), 47 deletions(-) delete mode 100644 src/fides/lib/oauth/schemas/user_permission.py diff --git a/.fides/db_dataset.yml b/.fides/db_dataset.yml index ebe9c7a8fa..410900d6c5 100644 --- a/.fides/db_dataset.yml +++ b/.fides/db_dataset.yml @@ -123,6 +123,10 @@ dataset: data_categories: - system.operations data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified + - name: roles + data_categories: + - system.operations + data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified - name: updated_at data_categories: - system.operations @@ -615,6 +619,10 @@ dataset: data_categories: - system.operations data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified + - name: roles + data_categories: + - system.operations + data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified - name: updated_at data_categories: - system.operations diff --git a/src/fides/api/ops/schemas/user_permission.py b/src/fides/api/ops/schemas/user_permission.py index af0018eedd..1ad8e83eae 100644 --- a/src/fides/api/ops/schemas/user_permission.py +++ b/src/fides/api/ops/schemas/user_permission.py @@ -1,24 +1,23 @@ -from typing import List, Set +from typing import List, Optional, Set from fastapi import HTTPException from pydantic import validator from starlette.status import HTTP_422_UNPROCESSABLE_ENTITY from fides.api.ops.api.v1.scope_registry import SCOPE_REGISTRY +from fides.api.ops.schemas.base_class import BaseSchema from fides.lib.oauth.roles import ROLE_REGISTRY -from fides.lib.oauth.schemas.user_permission import ( - UserPermissionsCreate as UserPermissionsCreateLib, -) -from fides.lib.oauth.schemas.user_permission import ( - UserPermissionsEdit as UserPermissionsEditLib, -) -from fides.lib.oauth.schemas.user_permission import ( - UserPermissionsResponse as UserPermissionsResponseLib, -) -class UserPermissionsCreate(UserPermissionsCreateLib): - """Data required to create a FidesUserPermissions record""" +class UserPermissionsCreate(BaseSchema): + """Data required to create a FidesUserPermissions record + + Users will generally be assigned role(s) directly which are associated with many scopes, + but we also will continue to support the ability to be assigned specific individual scopes. + """ + + scopes: List[str] = [] + roles: List[str] = [] @validator("scopes") @classmethod @@ -45,9 +44,15 @@ def validate_roles(cls, roles: List[str]) -> List[str]: return roles -class UserPermissionsEdit(UserPermissionsEditLib): - """Data required to edit a FidesUserPermissions record""" +class UserPermissionsEdit(UserPermissionsCreate): + """Data required to edit a FidesUserPermissions record.""" + + id: Optional[str] # I don't think this should be in the request body, so making it optional. + + +class UserPermissionsResponse(UserPermissionsCreate): + """Response after creating, editing, or retrieving a FidesUserPermissions record.""" -class UserPermissionsResponse(UserPermissionsResponseLib): - """Response after creating, editing, or retrieving a FidesUserPermissions record""" + id: str + user_id: str diff --git a/src/fides/lib/oauth/schemas/user_permission.py b/src/fides/lib/oauth/schemas/user_permission.py deleted file mode 100644 index 24e46ee0c9..0000000000 --- a/src/fides/lib/oauth/schemas/user_permission.py +++ /dev/null @@ -1,27 +0,0 @@ -from typing import List - -from fides.lib.schemas.base_class import BaseSchema - - -class UserPermissionsCreate(BaseSchema): - """Data required to create a FidesUserPermissions record. - - Users will generally be assigned role(s) directly which are associated with many scopes, - but we also will continue to support the ability to be assigned specific individual scopes. - """ - - scopes: List[str] = [] - roles: List[str] = [] - - -class UserPermissionsEdit(UserPermissionsCreate): - """Data required to edit a FidesUserPermissions record.""" - - id: str - - -class UserPermissionsResponse(UserPermissionsCreate): - """Response after creating, editing, or retrieving a FidesUserPermissions record.""" - - id: str - user_id: str diff --git a/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py b/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py index fbaa45f8ec..22b83a9bed 100644 --- a/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py @@ -218,11 +218,17 @@ def test_edit_user_permissions_invalid_scope( url, ) -> None: auth_header = generate_auth_header([USER_PERMISSION_UPDATE]) + user = FidesUser.create( + db=db, + data={"username": "user_1", "password": "test_password"}, + ) - body = {"user_id": "bogus_user_id", "scopes": ["not a real scope"]} + body = {"scopes": ["not a real scope"]} - response = api_client.put(url, headers=auth_header, json=body) + response = api_client.put(f"{V1_URL_PREFIX}/user/{user.id}/permission", headers=auth_header, json=body) assert HTTP_422_UNPROCESSABLE_ENTITY == response.status_code + assert "Invalid Scope(s) {'not a real scope'}" in response.json()["detail"] + user.delete(db) def test_edit_user_permissions_invalid_user_id( self, db, api_client, generate_auth_header @@ -250,6 +256,56 @@ def test_edit_user_permissions_invalid_user_id( assert permissions is None user.delete(db) + def test_optional_permissions_id(self, db, api_client, generate_auth_header) -> None: + auth_header = generate_auth_header([USER_PERMISSION_UPDATE]) + user = FidesUser.create( + db=db, + data={"username": "user_1", "password": "test_password"}, + ) + + permissions = FidesUserPermissions.create( + db=db, + data={ + "user_id": user.id, + "scopes": [PRIVACY_REQUEST_READ], + "roles": [VIEWER], + }, + ) + permissions_id = permissions.id + + ClientDetail.create_client_and_secret( + db, + CONFIG.security.oauth_client_id_length_bytes, + CONFIG.security.oauth_client_secret_length_bytes, + scopes=[PRIVACY_REQUEST_READ], + roles=[VIEWER], + user_id=user.id, + ) + + updated_scopes = [PRIVACY_REQUEST_READ, SAAS_CONFIG_READ] + body = {"scopes": updated_scopes} + response = api_client.put( + f"{V1_URL_PREFIX}/user/{user.id}/permission", headers=auth_header, json=body + ) + response_body = response.json() + assert HTTP_200_OK == response.status_code + assert response_body["scopes"] == updated_scopes + assert ( + response_body["roles"] == [] + ), "Roles removed as they were not specified in the request" + assert response_body["id"] == permissions_id + + client: ClientDetail = ClientDetail.get_by(db, field="user_id", value=user.id) + assert client.scopes == updated_scopes + assert client.roles == [] + + db.refresh(permissions) + assert permissions.scopes == updated_scopes + assert permissions.roles == [] + assert permissions.id == permissions_id + + user.delete(db) + def test_edit_user_scopes(self, db, api_client, generate_auth_header) -> None: auth_header = generate_auth_header([USER_PERMISSION_UPDATE]) user = FidesUser.create( @@ -277,8 +333,8 @@ def test_edit_user_scopes(self, db, api_client, generate_auth_header) -> None: ) updated_scopes = [PRIVACY_REQUEST_READ, SAAS_CONFIG_READ] - # Note: The permissions id should not be in the request body. - # Verify that we ignore this id + # Note: It is odd that we have the permissions id in the request body. + # I've made it optional as the UI sends it. Verify we ignore it. body = {"id": "invalid_id", "scopes": updated_scopes} response = api_client.put( f"{V1_URL_PREFIX}/user/{user.id}/permission", headers=auth_header, json=body From 2420e09e0ff5091a354239f0ec680d677b557a16 Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Wed, 22 Feb 2023 22:03:15 -0600 Subject: [PATCH 04/12] Add clarifying comment to create user permissions. - Assert 422 when trying to create user perms w/ invalid scope. --- .../v1/endpoints/user_permission_endpoints.py | 2 ++ src/fides/api/ops/schemas/user_permission.py | 5 ++-- .../test_user_permission_endpoints.py | 30 +++++++++++++++++-- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/fides/api/ops/api/v1/endpoints/user_permission_endpoints.py b/src/fides/api/ops/api/v1/endpoints/user_permission_endpoints.py index 33538c6bcf..d3c1e68e4f 100644 --- a/src/fides/api/ops/api/v1/endpoints/user_permission_endpoints.py +++ b/src/fides/api/ops/api/v1/endpoints/user_permission_endpoints.py @@ -57,6 +57,7 @@ def create_user_permissions( user_id: str, permissions: UserPermissionsCreate, ) -> FidesUserPermissions: + """Create user permissions with big picture roles and/or scopes.""" user = validate_user_id(db, user_id) if user.permissions is not None: # type: ignore[attr-defined] raise HTTPException( @@ -65,6 +66,7 @@ def create_user_permissions( ) if user.client: + # Just in case - this shouldn't happen in practice. user.client.update(db=db, data=permissions.dict()) logger.info("Created FidesUserPermission record") return FidesUserPermissions.create( diff --git a/src/fides/api/ops/schemas/user_permission.py b/src/fides/api/ops/schemas/user_permission.py index 1ad8e83eae..50d5215c4f 100644 --- a/src/fides/api/ops/schemas/user_permission.py +++ b/src/fides/api/ops/schemas/user_permission.py @@ -44,11 +44,12 @@ def validate_roles(cls, roles: List[str]) -> List[str]: return roles - class UserPermissionsEdit(UserPermissionsCreate): """Data required to edit a FidesUserPermissions record.""" - id: Optional[str] # I don't think this should be in the request body, so making it optional. + id: Optional[ + str + ] # I don't think this should be in the request body, so making it optional. class UserPermissionsResponse(UserPermissionsCreate): diff --git a/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py b/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py index 22b83a9bed..8701b92110 100644 --- a/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py @@ -66,6 +66,7 @@ def test_create_user_permissions_invalid_scope( response = api_client.post(url, headers=auth_header, json=body) assert HTTP_422_UNPROCESSABLE_ENTITY == response.status_code + assert "Invalid Scope(s) {'not a real scope'}" in response.json()["detail"] user.delete(db) def test_create_user_permissions_invalid_user_id( @@ -81,6 +82,27 @@ def test_create_user_permissions_invalid_user_id( assert HTTP_404_NOT_FOUND == response.status_code assert permissions is None + def test_create_user_permissions_add_scopes_no_client_to_update( + self, db, api_client, generate_auth_header + ) -> None: + auth_header = generate_auth_header([USER_PERMISSION_CREATE]) + user = FidesUser.create( + db=db, + data={"username": "user_1", "password": "test_password"}, + ) + + 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 + ) + permissions = FidesUserPermissions.get_by(db, field="user_id", value=user.id) + response_body = response.json() + assert HTTP_201_CREATED == response.status_code + assert response_body["id"] == permissions.id + assert permissions.scopes == [PRIVACY_REQUEST_READ] + assert not user.client + user.delete(db) + def test_create_user_permissions_add_scopes( self, db, api_client, generate_auth_header ) -> None: @@ -225,7 +247,9 @@ def test_edit_user_permissions_invalid_scope( body = {"scopes": ["not a real scope"]} - 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 + ) assert HTTP_422_UNPROCESSABLE_ENTITY == response.status_code assert "Invalid Scope(s) {'not a real scope'}" in response.json()["detail"] user.delete(db) @@ -256,7 +280,9 @@ def test_edit_user_permissions_invalid_user_id( assert permissions is None user.delete(db) - def test_optional_permissions_id(self, db, api_client, generate_auth_header) -> None: + def test_optional_permissions_id( + self, db, api_client, generate_auth_header + ) -> None: auth_header = generate_auth_header([USER_PERMISSION_UPDATE]) user = FidesUser.create( db=db, From ae8355b51fbcc48d014029ad80d8b40ae96e5fa6 Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Thu, 23 Feb 2023 12:45:10 -0600 Subject: [PATCH 05/12] Everywhere that's assigning the entire scope registry and the admin role to the root user, pull this from the config settings instead. Permissions tests deleted (these were the "privileges" tests which is obsolete.) --- scripts/setup/user.py | 7 ++++++- .../ops/api/v1/endpoints/oauth_endpoints.py | 8 ++++++-- .../v1/endpoints/user_permission_endpoints.py | 6 ++---- src/fides/api/ops/util/oauth_util.py | 15 +++++++++++---- src/fides/core/user.py | 7 +++---- tests/lib/test_fides_user_permissions.py | 17 ----------------- tests/lib/test_oauth_util.py | 18 ++++++++++++++++++ 7 files changed, 46 insertions(+), 32 deletions(-) delete mode 100644 tests/lib/test_fides_user_permissions.py diff --git a/scripts/setup/user.py b/scripts/setup/user.py index 0715270e4d..c812dc5c55 100644 --- a/scripts/setup/user.py +++ b/scripts/setup/user.py @@ -5,6 +5,7 @@ from fides.api.ops.api.v1 import urn_registry as urls from fides.api.ops.api.v1.scope_registry import SCOPE_REGISTRY +from fides.core.config import CONFIG from fides.lib.oauth.roles import ADMIN from . import constants @@ -51,7 +52,11 @@ def create_user( response = requests.put( f"{constants.BASE_URL}{user_permissions_url}", headers=auth_header, - json={"id": user_id, "scopes": SCOPE_REGISTRY, "roles": [ADMIN]}, + json={ + "id": user_id, + "scopes": CONFIG.security.root_user_scopes, + "roles": CONFIG.security.root_user_roles, + }, ) if not response.ok: diff --git a/src/fides/api/ops/api/v1/endpoints/oauth_endpoints.py b/src/fides/api/ops/api/v1/endpoints/oauth_endpoints.py index aaabb4a135..0f1a80ee9f 100644 --- a/src/fides/api/ops/api/v1/endpoints/oauth_endpoints.py +++ b/src/fides/api/ops/api/v1/endpoints/oauth_endpoints.py @@ -50,7 +50,7 @@ from fides.api.ops.util.oauth_util import verify_oauth_client from fides.core.config import CONFIG from fides.lib.models.client import ClientDetail -from fides.lib.oauth.roles import ADMIN, roles_to_scopes_mapping +from fides.lib.oauth.roles import roles_to_scopes_mapping from fides.lib.oauth.schemas.oauth import ( AccessToken, OAuth2ClientCredentialsRequestForm, @@ -84,7 +84,11 @@ async def acquire_access_token( # scopes/roles params are only used if client is root client, otherwise we use the client's associated scopes and/or roles client_detail = ClientDetail.get( - db, object_id=client_id, config=CONFIG, scopes=SCOPE_REGISTRY, roles=[ADMIN] + db, + object_id=client_id, + config=CONFIG, + scopes=CONFIG.security.root_user_scopes, + roles=CONFIG.security.root_user_roles, ) if client_detail is None: diff --git a/src/fides/api/ops/api/v1/endpoints/user_permission_endpoints.py b/src/fides/api/ops/api/v1/endpoints/user_permission_endpoints.py index cf48758ca4..60de2b1cb6 100644 --- a/src/fides/api/ops/api/v1/endpoints/user_permission_endpoints.py +++ b/src/fides/api/ops/api/v1/endpoints/user_permission_endpoints.py @@ -9,7 +9,6 @@ from fides.api.ops.api import deps from fides.api.ops.api.v1 import urn_registry as urls from fides.api.ops.api.v1.scope_registry import ( - SCOPE_REGISTRY, USER_PERMISSION_CREATE, USER_PERMISSION_READ, USER_PERMISSION_UPDATE, @@ -29,7 +28,6 @@ from fides.core.config import CONFIG from fides.lib.models.fides_user import FidesUser from fides.lib.models.fides_user_permissions import FidesUserPermissions -from fides.lib.oauth.roles import ADMIN router = APIRouter(tags=["User Permissions"], prefix=V1_URL_PREFIX) @@ -121,8 +119,8 @@ async def get_user_permissions( return FidesUserPermissions( id=CONFIG.security.oauth_root_client_id, user_id=CONFIG.security.oauth_root_client_id, - scopes=SCOPE_REGISTRY, - roles=[ADMIN], + scopes=CONFIG.security.root_user_scopes, + roles=CONFIG.security.root_user_roles, ) logger.info("Retrieved FidesUserPermission record for current user") diff --git a/src/fides/api/ops/util/oauth_util.py b/src/fides/api/ops/util/oauth_util.py index b6062660e8..e1c59676cd 100644 --- a/src/fides/api/ops/util/oauth_util.py +++ b/src/fides/api/ops/util/oauth_util.py @@ -16,7 +16,6 @@ from starlette.status import HTTP_404_NOT_FOUND from fides.api.ops.api.deps import get_db -from fides.api.ops.api.v1.scope_registry import SCOPE_REGISTRY from fides.api.ops.api.v1.urn_registry import TOKEN, V1_URL_PREFIX from fides.api.ops.models.policy import PolicyPreWebhook from fides.api.ops.schemas.external_https import WebhookJWE @@ -31,7 +30,7 @@ from fides.lib.models.client import ClientDetail from fides.lib.models.fides_user import FidesUser from fides.lib.oauth.oauth_util import extract_payload, is_token_expired -from fides.lib.oauth.roles import ADMIN, get_scopes_from_roles +from fides.lib.oauth.roles import get_scopes_from_roles from fides.lib.oauth.schemas.oauth import OAuth2ClientCredentialsBearer JWT_ENCRYPTION_ALGORITHM = ALGORITHMS.A256GCM @@ -142,7 +141,11 @@ async def get_root_client( This function is primarily used to let users bypass endpoint authorization """ client = ClientDetail.get( - db, object_id=client_id, config=CONFIG, scopes=SCOPE_REGISTRY, roles=[ADMIN] + db, + object_id=client_id, + config=CONFIG, + scopes=CONFIG.security.root_user_scopes, + roles=CONFIG.security.root_user_roles, ) if not client: logger.debug("Auth token belongs to an invalid client_id.") @@ -193,7 +196,11 @@ async def verify_oauth_client( # scopes/roles param is only used if client is root client, otherwise we use the client's associated scopes client = ClientDetail.get( - db, object_id=client_id, config=CONFIG, scopes=SCOPE_REGISTRY, roles=[ADMIN] + db, + object_id=client_id, + config=CONFIG, + scopes=CONFIG.security.root_user_scopes, + roles=CONFIG.security.root_user_roles, ) if not client: diff --git a/src/fides/core/user.py b/src/fides/core/user.py index a3f61b1232..f9f82ce819 100644 --- a/src/fides/core/user.py +++ b/src/fides/core/user.py @@ -4,8 +4,8 @@ import requests -from fides.api.ops.api.v1.scope_registry import SCOPE_REGISTRY as SCOPES from fides.cli.utils import handle_cli_response +from fides.core.config import CONFIG from fides.core.utils import ( Credentials, echo_green, @@ -16,7 +16,6 @@ write_credentials_file, ) from fides.lib.cryptography.cryptographic_util import str_to_b64_str -from fides.lib.oauth.roles import ADMIN CREATE_USER_PATH = "/api/v1/user" LOGIN_PATH = "/api/v1/login" @@ -120,10 +119,10 @@ def create_command( user_id = user_response.json()["id"] update_user_permissions( user_id=user_id, - scopes=SCOPES, + scopes=CONFIG.security.root_user_scopes, auth_header=auth_header, server_url=server_url, - roles=[ADMIN], + roles=CONFIG.security.root_user_roles, ) echo_green(f"User: '{username}' created and assigned permissions.") diff --git a/tests/lib/test_fides_user_permissions.py b/tests/lib/test_fides_user_permissions.py deleted file mode 100644 index fba992923a..0000000000 --- a/tests/lib/test_fides_user_permissions.py +++ /dev/null @@ -1,17 +0,0 @@ -# pylint: disable=missing-function-docstring - -from unittest.mock import MagicMock - -from fides.api.ops.api.v1.scope_registry import ( - PRIVACY_REQUEST_READ, - USER_CREATE, - USER_DELETE, - USER_READ, -) - -# Included so that `AccessManualWebhook` can be located when -# `ConnectionConfig` is instantiated. -from fides.api.ops.models.manual_webhook import ( # pylint: disable=unused-import - AccessManualWebhook, -) -from fides.lib.models.fides_user_permissions import FidesUserPermissions diff --git a/tests/lib/test_oauth_util.py b/tests/lib/test_oauth_util.py index f109c25c2c..f75fd7cb0d 100644 --- a/tests/lib/test_oauth_util.py +++ b/tests/lib/test_oauth_util.py @@ -187,6 +187,24 @@ async def test_verify_oauth_client_wrong_client_scope(db, config, user): class TestVerifyOauthClientRoles: + async def test_token_does_not_have_roles(self, db, config, user): + """Test no error even if there are not roles on the token""" + payload = { + JWE_PAYLOAD_SCOPES: [USER_DELETE], + JWE_PAYLOAD_CLIENT_ID: user.client.id, + JWE_ISSUED_AT: datetime.now().isoformat(), + } + token = generate_jwe( + json.dumps(payload), + config.security.app_encryption_key, + ) + client = await verify_oauth_client( + SecurityScopes([USER_DELETE]), + token, + db=db, + ) + assert client == user.client + async def test_verify_oauth_client_roles(self, db, config, admin_user): """Test token has the correct role and the client also has the matching role""" payload = { From d381d2aa519d3ba7925ef16aeeb4f35a3af6bd8b Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Fri, 24 Feb 2023 12:48:45 -0600 Subject: [PATCH 06/12] Make roles_to_scopes_mapping uppercase to denote a constant. --- src/fides/api/ops/api/v1/endpoints/oauth_endpoints.py | 4 ++-- src/fides/lib/oauth/roles.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/fides/api/ops/api/v1/endpoints/oauth_endpoints.py b/src/fides/api/ops/api/v1/endpoints/oauth_endpoints.py index 0f1a80ee9f..4a7202ce86 100644 --- a/src/fides/api/ops/api/v1/endpoints/oauth_endpoints.py +++ b/src/fides/api/ops/api/v1/endpoints/oauth_endpoints.py @@ -50,7 +50,7 @@ from fides.api.ops.util.oauth_util import verify_oauth_client from fides.core.config import CONFIG from fides.lib.models.client import ClientDetail -from fides.lib.oauth.roles import roles_to_scopes_mapping +from fides.lib.oauth.roles import ROLES_TO_SCOPES_MAPPING from fides.lib.oauth.schemas.oauth import ( AccessToken, OAuth2ClientCredentialsRequestForm, @@ -207,7 +207,7 @@ def read_scopes() -> List[str]: def read_roles_to_scopes_mapping() -> Dict[str, List]: """Returns a list of all roles and associated scopes available for assignment in the system""" logger.info("Getting all available roles") - return roles_to_scopes_mapping + return ROLES_TO_SCOPES_MAPPING @router.get(OAUTH_CALLBACK, response_model=None) diff --git a/src/fides/lib/oauth/roles.py b/src/fides/lib/oauth/roles.py index e774d6acf6..343b377025 100644 --- a/src/fides/lib/oauth/roles.py +++ b/src/fides/lib/oauth/roles.py @@ -79,7 +79,7 @@ USER_READ, ] -roles_to_scopes_mapping: Dict[str, List] = { +ROLES_TO_SCOPES_MAPPING: Dict[str, List] = { ADMIN: sorted(SCOPE_REGISTRY), VIEWER_AND_PRIVACY_REQUEST_MANAGER: sorted( list(set(viewer_scopes + privacy_request_manager_scopes)) @@ -96,5 +96,5 @@ def get_scopes_from_roles(roles: Optional[List[str]]) -> List[str]: scope_list: List[str] = [] for role in roles: - scope_list += roles_to_scopes_mapping.get(role, []) + scope_list += ROLES_TO_SCOPES_MAPPING.get(role, []) return [*set(scope_list)] From cbf625192f708012c9e464d6c6d804a5070f3258 Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Fri, 24 Feb 2023 13:10:03 -0600 Subject: [PATCH 07/12] Fix relative import, make roles an enum. --- src/fides/api/ops/schemas/user_permission.py | 19 ++++++------------- src/fides/core/config/security_settings.py | 2 +- src/fides/lib/oauth/roles.py | 14 ++++++++------ .../test_user_permission_endpoints.py | 6 ++++-- 4 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/fides/api/ops/schemas/user_permission.py b/src/fides/api/ops/schemas/user_permission.py index 50d5215c4f..92768b0105 100644 --- a/src/fides/api/ops/schemas/user_permission.py +++ b/src/fides/api/ops/schemas/user_permission.py @@ -6,7 +6,7 @@ from fides.api.ops.api.v1.scope_registry import SCOPE_REGISTRY from fides.api.ops.schemas.base_class import BaseSchema -from fides.lib.oauth.roles import ROLE_REGISTRY +from fides.lib.oauth.roles import RoleRegistry class UserPermissionsCreate(BaseSchema): @@ -17,7 +17,7 @@ class UserPermissionsCreate(BaseSchema): """ scopes: List[str] = [] - roles: List[str] = [] + roles: List[RoleRegistry] = [] @validator("scopes") @classmethod @@ -31,17 +31,10 @@ def validate_scopes(cls, scopes: List[str]) -> List[str]: ) return scopes - @validator("roles") - @classmethod - def validate_roles(cls, roles: List[str]) -> List[str]: - """Validates that all incoming roles are valid""" - diff: Set = set(roles).difference(set(ROLE_REGISTRY)) - if len(diff) > 0: - raise HTTPException( - status_code=HTTP_422_UNPROCESSABLE_ENTITY, - detail=f"Invalid Role(s) {diff}. Roles must be one of {ROLE_REGISTRY}.", - ) - return roles + class Config: + """So roles are strings when we add to the db""" + + use_enum_values = True class UserPermissionsEdit(UserPermissionsCreate): diff --git a/src/fides/core/config/security_settings.py b/src/fides/core/config/security_settings.py index 248daea7fb..d33b578dfa 100644 --- a/src/fides/core/config/security_settings.py +++ b/src/fides/core/config/security_settings.py @@ -9,8 +9,8 @@ from fides.api.ops.api.v1.scope_registry import SCOPE_REGISTRY from fides.lib.cryptography.cryptographic_util import generate_salt, hash_with_salt +from fides.lib.oauth.roles import ADMIN -from ...lib.oauth.roles import ADMIN from .fides_settings import FidesSettings ENV_PREFIX = "FIDES__SECURITY__" diff --git a/src/fides/lib/oauth/roles.py b/src/fides/lib/oauth/roles.py index 343b377025..c91a23d051 100644 --- a/src/fides/lib/oauth/roles.py +++ b/src/fides/lib/oauth/roles.py @@ -1,3 +1,4 @@ +from enum import Enum from typing import Dict, List, Optional from fides.api.ops.api.v1.scope_registry import ( @@ -36,12 +37,13 @@ ADMIN = "admin" -ROLE_REGISTRY = { - PRIVACY_REQUEST_MANAGER, - VIEWER, - VIEWER_AND_PRIVACY_REQUEST_MANAGER, - ADMIN, -} +class RoleRegistry(Enum): + """Enum of available roles""" + + admin = ADMIN + viewer_and_privacy_request_manager = VIEWER_AND_PRIVACY_REQUEST_MANAGER + viewer = VIEWER + privacy_request_manager = PRIVACY_REQUEST_MANAGER privacy_request_manager_scopes = [ diff --git a/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py b/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py index cc365c83b4..9f0d655c2d 100644 --- a/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py @@ -138,14 +138,16 @@ def test_create_user_permissions_add_bad_role( db=db, data={"username": "user_1", "password": "test_password"}, ) - body = {"user_id": user.id, "roles": ["nonexistent role"]} response = api_client.post( f"{V1_URL_PREFIX}/user/{user.id}/permission", headers=auth_header, json=body ) response_body = response.json() assert HTTP_422_UNPROCESSABLE_ENTITY == response.status_code - assert "Invalid Role(s) {'nonexistent role'}." in response_body["detail"] + assert ( + "value is not a valid enumeration member" + in response_body["detail"][0]["msg"] + ) def test_create_user_permissions_add_roles( self, db, api_client, generate_auth_header From 608eb77dbfb06ec7e08abbccd2bf1d337eee924a Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Fri, 24 Feb 2023 14:12:35 -0600 Subject: [PATCH 08/12] Update migration so neither roles or scopes are nullable (scopes weren't nullable previously). Default to an empty list. --- ...b1e6ec39b83_add_role_based_permissions.py} | 43 ++++++++++++------- src/fides/api/ops/schemas/user_permission.py | 2 +- src/fides/lib/models/client.py | 12 +++--- .../lib/models/fides_user_permissions.py | 4 +- .../api/v1/endpoints/test_oauth_endpoints.py | 3 +- .../api/v1/endpoints/test_user_endpoints.py | 29 +++++++++++++ .../test_user_permission_endpoints.py | 16 +++++++ 7 files changed, 83 insertions(+), 26 deletions(-) rename src/fides/api/ctl/migrations/versions/{985d3756f908_add_role_based_foundation.py => eb1e6ec39b83_add_role_based_permissions.py} (56%) diff --git a/src/fides/api/ctl/migrations/versions/985d3756f908_add_role_based_foundation.py b/src/fides/api/ctl/migrations/versions/eb1e6ec39b83_add_role_based_permissions.py similarity index 56% rename from src/fides/api/ctl/migrations/versions/985d3756f908_add_role_based_foundation.py rename to src/fides/api/ctl/migrations/versions/eb1e6ec39b83_add_role_based_permissions.py index d4300e183d..c9b4f21bd2 100644 --- a/src/fides/api/ctl/migrations/versions/985d3756f908_add_role_based_foundation.py +++ b/src/fides/api/ctl/migrations/versions/eb1e6ec39b83_add_role_based_permissions.py @@ -1,50 +1,63 @@ -"""add_role_based_foundation +"""add role based permissions -Revision ID: 985d3756f908 +Revision ID: eb1e6ec39b83 Revises: d65bbc647083 -Create Date: 2023-02-17 21:04:20.113779 +Create Date: 2023-02-24 19:27:17.844231 """ import sqlalchemy as sa from alembic import op -from sqlalchemy.dialects import postgresql # revision identifiers, used by Alembic. -revision = "985d3756f908" +from sqlalchemy.dialects import postgresql + +revision = "eb1e6ec39b83" down_revision = "d65bbc647083" branch_labels = None depends_on = None def upgrade(): - # ### commands auto generated by Alembic - please adjust! ### op.add_column( "client", - sa.Column("roles", sa.ARRAY(sa.String()), server_default="{}", nullable=True), - ) - op.alter_column( - "client", "scopes", existing_type=postgresql.ARRAY(sa.VARCHAR()), nullable=True + sa.Column("roles", sa.ARRAY(sa.String()), server_default="{}", nullable=False), ) op.add_column( - "fidesuserpermissions", sa.Column("roles", sa.ARRAY(sa.String()), nullable=True) + "fidesuserpermissions", + sa.Column("roles", sa.ARRAY(sa.String()), server_default="{}", nullable=False), ) op.alter_column( "fidesuserpermissions", "scopes", existing_type=postgresql.ARRAY(sa.VARCHAR()), - nullable=True, + nullable=False, + server_default="{}", + ) + op.alter_column( + "client", + "scopes", + existing_type=postgresql.ARRAY(sa.VARCHAR()), + nullable=False, + server_default="{}", ) def downgrade(): + op.drop_column("fidesuserpermissions", "roles") + op.drop_column("client", "roles") + op.alter_column( "fidesuserpermissions", "scopes", existing_type=postgresql.ARRAY(sa.VARCHAR()), nullable=False, + server_default=None, ) - op.drop_column("fidesuserpermissions", "roles") + op.alter_column( - "client", "scopes", existing_type=postgresql.ARRAY(sa.VARCHAR()), nullable=False + "client", + "scopes", + existing_type=postgresql.ARRAY(sa.VARCHAR()), + nullable=False, + server_default=None, ) - op.drop_column("client", "roles") diff --git a/src/fides/api/ops/schemas/user_permission.py b/src/fides/api/ops/schemas/user_permission.py index 92768b0105..d6126df15d 100644 --- a/src/fides/api/ops/schemas/user_permission.py +++ b/src/fides/api/ops/schemas/user_permission.py @@ -1,4 +1,4 @@ -from typing import List, Optional, Set +from typing import List, Optional from fastapi import HTTPException from pydantic import validator diff --git a/src/fides/lib/models/client.py b/src/fides/lib/models/client.py index 6211e785c2..6a35f89460 100644 --- a/src/fides/lib/models/client.py +++ b/src/fides/lib/models/client.py @@ -38,8 +38,8 @@ def __tablename__(self) -> str: hashed_secret = Column(String, nullable=False) salt = Column(String, nullable=False) - scopes = Column(ARRAY(String), nullable=True, server_default="{}", default=dict) - roles = Column(ARRAY(String), nullable=True, server_default="{}", default=dict) + scopes = Column(ARRAY(String), nullable=False, server_default="{}", default=dict) + roles = Column(ARRAY(String), nullable=False, server_default="{}", default=dict) fides_key = Column(String, index=True, unique=True, nullable=True) user_id = Column( String, ForeignKey(FidesUser.id_field_path), nullable=True, unique=True @@ -98,8 +98,8 @@ def get( # type: ignore *, object_id: Any, config: FidesConfig, - scopes: list[str] | None = None, - roles: list[str] | None = None, + scopes: list[str] = [], + roles: list[str] = [], ) -> ClientDetail | None: """Fetch a database record via a client_id""" if object_id == config.security.oauth_root_client_id: @@ -129,8 +129,8 @@ def credentials_valid(self, provided_secret: str, encoding: str = "UTF-8") -> bo def _get_root_client_detail( config: FidesConfig, - scopes: list[str] | None, - roles: list[str] | None, + scopes: list[str], + roles: list[str], encoding: str = "UTF-8", ) -> ClientDetail | None: """ diff --git a/src/fides/lib/models/fides_user_permissions.py b/src/fides/lib/models/fides_user_permissions.py index 1c56badfbc..8c37bcc9db 100644 --- a/src/fides/lib/models/fides_user_permissions.py +++ b/src/fides/lib/models/fides_user_permissions.py @@ -9,8 +9,8 @@ class FidesUserPermissions(Base): """The DB ORM model for FidesUserPermissions""" user_id = Column(String, ForeignKey(FidesUser.id), nullable=False, unique=True) - scopes = Column(ARRAY(String), nullable=True, server_default="{}", default=dict) - roles = Column(ARRAY(String), nullable=True, server_default="{}", default=dict) + scopes = Column(ARRAY(String), nullable=False, server_default="{}", default=dict) + roles = Column(ARRAY(String), nullable=False, server_default="{}", default=dict) user = relationship( FidesUser, backref=backref("permissions", cascade="all,delete", uselist=False), diff --git a/tests/ops/api/v1/endpoints/test_oauth_endpoints.py b/tests/ops/api/v1/endpoints/test_oauth_endpoints.py index 4624091ba7..d69e9d4f39 100644 --- a/tests/ops/api/v1/endpoints/test_oauth_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_oauth_endpoints.py @@ -27,7 +27,6 @@ ) from fides.api.ops.common_exceptions import OAuth2TokenException from fides.api.ops.models.authentication_request import AuthenticationRequest -from fides.core.api import get from fides.core.config import CONFIG from fides.lib.cryptography.schemas.jwt import ( JWE_ISSUED_AT, @@ -38,7 +37,7 @@ from fides.lib.models.client import ClientDetail from fides.lib.oauth.jwt import generate_jwe from fides.lib.oauth.oauth_util import extract_payload -from fides.lib.oauth.roles import ADMIN, ROLE_REGISTRY +from fides.lib.oauth.roles import ADMIN class TestCreateClient: diff --git a/tests/ops/api/v1/endpoints/test_user_endpoints.py b/tests/ops/api/v1/endpoints/test_user_endpoints.py index 6f8cf0ac82..cee3948afd 100644 --- a/tests/ops/api/v1/endpoints/test_user_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_user_endpoints.py @@ -931,6 +931,35 @@ def test_login_with_roles(self, db, url, viewer_user, api_client): assert "user_data" in list(response.json().keys()) assert response.json()["user_data"]["id"] == viewer_user.id + def test_login_with_no_permissions(self, db, url, viewer_user, api_client): + # Delete existing client for test purposes + viewer_user.client.delete(db) + body = { + "username": viewer_user.username, + "password": str_to_b64_str("TESTdcnG@wzJeu0&%3Qe2fGo7"), + } + viewer_user.permissions.roles = [] + viewer_user.save(db) # Remove roles from user + + assert viewer_user.client is None # client does not exist + assert viewer_user.permissions is not None + response = api_client.post(url, headers={}, json=body) + assert response.status_code == HTTP_200_OK + + db.refresh(viewer_user) + assert viewer_user.client is not None + assert "token_data" in list(response.json().keys()) + token = response.json()["token_data"]["access_token"] + token_data = json.loads( + extract_payload(token, CONFIG.security.app_encryption_key) + ) + assert token_data["client-id"] == viewer_user.client.id + assert token_data["scopes"] == [] # Uses scopes on existing client + assert token_data["roles"] == [] # Uses roles on existing client + + assert "user_data" in list(response.json().keys()) + assert response.json()["user_data"]["id"] == viewer_user.id + def test_login_updates_last_login_date(self, db, url, user, api_client): body = { "username": user.username, diff --git a/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py b/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py index 9f0d655c2d..470f8745bc 100644 --- a/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py @@ -520,6 +520,22 @@ def test_get_user_permissions( assert response_body["scopes"] == [PRIVACY_REQUEST_READ] assert response_body["roles"] == [] + def test_get_user_with_no_permissions_as_root( + self, db, api_client, auth_user, root_auth_header + ): + FidesUserPermissions.create( + db=db, data={"user_id": auth_user.id, "scopes": None, "roles": None} + ) + + response = api_client.get( + f"{V1_URL_PREFIX}/user/{auth_user.id}/permission", + headers=root_auth_header, + ) + resp = response.json() + assert resp["scopes"] == [] + assert resp["roles"] == [] + assert resp["user_id"] == auth_user.id + def test_get_current_user_permissions(self, db, api_client, auth_user) -> None: # Note: Does not include USER_PERMISSION_READ. scopes = [PRIVACY_REQUEST_READ, SAAS_CONFIG_READ] From 71a81ae8399c5165f7aabf82b20f8de17a9205b3 Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Fri, 24 Feb 2023 14:41:03 -0600 Subject: [PATCH 09/12] Fix issue where I accidentally removed the other roles from the test client - need all roles for test purposes so the client has the same role as the token. --- tests/conftest.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index 4bfa5a2d55..69a3306ab6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -669,8 +669,12 @@ def oauth_role_client(db: Session) -> Generator: salt="thisisstillatest", roles=[ ADMIN, + PRIVACY_REQUEST_MANAGER, + VIEWER, + VIEWER_AND_PRIVACY_REQUEST_MANAGER, ], - ) + ) # Intentionally adding all roles here so the client will always + # have a role that matches a role on a token for testing db.add(client) db.commit() db.refresh(client) From 7e2e5bea9d938a81d591cdb75c0f6047b2678e56 Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Fri, 24 Feb 2023 15:03:30 -0600 Subject: [PATCH 10/12] In the off-chance a user doesn't have scopes or roles, don't allow them to login. --- .../lib/oauth/api/routes/user_endpoints.py | 5 ++++ .../api/v1/endpoints/test_user_endpoints.py | 27 +++++-------------- 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/src/fides/lib/oauth/api/routes/user_endpoints.py b/src/fides/lib/oauth/api/routes/user_endpoints.py index 9abb3bac04..bfb0f7682f 100644 --- a/src/fides/lib/oauth/api/routes/user_endpoints.py +++ b/src/fides/lib/oauth/api/routes/user_endpoints.py @@ -25,6 +25,7 @@ ) from fides.api.ops.util.oauth_util import verify_oauth_client from fides.core.config import FidesConfig, get_config +from fides.lib.exceptions import AuthorizationError from fides.lib.models.client import ClientDetail from fides.lib.models.fides_user import FidesUser from fides.lib.models.fides_user_permissions import FidesUserPermissions @@ -250,6 +251,10 @@ def perform_login( user_id=user.id, ) + if not user.permissions.scopes and not user.permissions.roles: # type: ignore + logger.info("User needs scopes and/or roles to login.") + raise AuthorizationError(detail="Not Authorized for this action") + user.last_login_at = datetime.utcnow() user.save(db) diff --git a/tests/ops/api/v1/endpoints/test_user_endpoints.py b/tests/ops/api/v1/endpoints/test_user_endpoints.py index cee3948afd..82f3918c05 100644 --- a/tests/ops/api/v1/endpoints/test_user_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_user_endpoints.py @@ -932,33 +932,18 @@ def test_login_with_roles(self, db, url, viewer_user, api_client): assert response.json()["user_data"]["id"] == viewer_user.id def test_login_with_no_permissions(self, db, url, viewer_user, api_client): - # Delete existing client for test purposes - viewer_user.client.delete(db) + viewer_user.permissions.roles = [] + viewer_user.permissions.scopes = [] + viewer_user.save(db) # Make sure user doesn't have roles or scopes + + assert viewer_user.permissions is not None body = { "username": viewer_user.username, "password": str_to_b64_str("TESTdcnG@wzJeu0&%3Qe2fGo7"), } - viewer_user.permissions.roles = [] - viewer_user.save(db) # Remove roles from user - assert viewer_user.client is None # client does not exist - assert viewer_user.permissions is not None response = api_client.post(url, headers={}, json=body) - assert response.status_code == HTTP_200_OK - - db.refresh(viewer_user) - assert viewer_user.client is not None - assert "token_data" in list(response.json().keys()) - token = response.json()["token_data"]["access_token"] - token_data = json.loads( - extract_payload(token, CONFIG.security.app_encryption_key) - ) - assert token_data["client-id"] == viewer_user.client.id - assert token_data["scopes"] == [] # Uses scopes on existing client - assert token_data["roles"] == [] # Uses roles on existing client - - assert "user_data" in list(response.json().keys()) - assert response.json()["user_data"]["id"] == viewer_user.id + assert response.status_code == HTTP_403_FORBIDDEN def test_login_updates_last_login_date(self, db, url, user, api_client): body = { From 03eb0784ffdcd6804c57cae459d0cdd8ecfb682a Mon Sep 17 00:00:00 2001 From: Thomas Date: Mon, 27 Feb 2023 11:05:01 +0800 Subject: [PATCH 11/12] fix: small nits --- src/fides/core/user.py | 2 +- tests/ctl/cli/test_cli.py | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/fides/core/user.py b/src/fides/core/user.py index f9f82ce819..58439a3e54 100644 --- a/src/fides/core/user.py +++ b/src/fides/core/user.py @@ -160,7 +160,7 @@ def get_permissions_command(server_url: str) -> None: auth_header = get_auth_header() scopes, roles = get_user_permissions(user_id, auth_header, server_url) - print("Permissions (Direct Scopes):") + print("Permissions (Directly-Assigned Scopes):") for scope in scopes: print(f"\t{scope}") diff --git a/tests/ctl/cli/test_cli.py b/tests/ctl/cli/test_cli.py index 29d83598f5..4916eddbcc 100644 --- a/tests/ctl/cli/test_cli.py +++ b/tests/ctl/cli/test_cli.py @@ -11,12 +11,11 @@ from fides.api.ops.api.v1.scope_registry import SCOPE_REGISTRY from fides.cli import cli -from fides.core.config import get_config +from fides.core.config import CONFIG from fides.core.user import get_user_permissions from fides.core.utils import get_auth_header, read_credentials_file from fides.lib.oauth.roles import ADMIN -config = get_config() OKTA_URL = "https://dev-78908748.okta.com" @@ -939,7 +938,7 @@ def test_user_create( credentials = read_credentials_file(credentials_path) scopes, roles = get_user_permissions( - credentials.user_id, get_auth_header(), config.cli.server_url + credentials.user_id, get_auth_header(), CONFIG.cli.server_url ) assert scopes == SCOPE_REGISTRY assert roles == [ADMIN] @@ -978,9 +977,9 @@ def test_get_user_permissions( env={"FIDES_CREDENTIALS_PATH": credentials_path}, ) scopes, roles = get_user_permissions( - config.security.oauth_root_client_id, + CONFIG.security.oauth_root_client_id, get_auth_header(), - config.cli.server_url, + CONFIG.cli.server_url, ) assert scopes == SCOPE_REGISTRY assert roles == [ADMIN] From 12a0c271327b956400c37998172de526cca90b14 Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Mon, 27 Feb 2023 08:31:25 -0600 Subject: [PATCH 12/12] Update changelog. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe417ba5c6..34b6dd760c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ The types of changes are: * Add API support for messaging config properties [#2551](https://github.com/ethyca/fides/pull/2551) * Access and erasure support for Kustomer [#2520](https://github.com/ethyca/fides/pull/2520) * Added the `erase_after` field on collections to be able to set the order for erasures [#2619](https://github.com/ethyca/fides/pull/2619) +- Added backend role-based permissions [#2671](https://github.com/ethyca/fides/pull/2671) ### Changed