Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Privacy Request Input Sanitization #2655

Merged
merged 18 commits into from
Mar 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ The types of changes are:
* Access and erasure for Jira SaaS Connector [#1871](https://github.com/ethyca/fides/issues/1871)
* Access and erasure support for Delighted [#2244](https://github.com/ethyca/fides/pull/2244)
* Improve "Upload a new dataset YAML" [#1531](https://github.com/ethyca/fides/pull/2258)
* Input validation and sanitization for Privacy Request fields [#2655](https://github.com/ethyca/fides/pull/2655)
* Access and erasure support for Yotpo [#2708](https://github.com/ethyca/fides/pull/2708)
* Custom Field Library Tab [#527](https://github.com/ethyca/fides/pull/2693)
* Allow SendGrid template usage [#2728](https://github.com/ethyca/fides/pull/2728)
Expand Down
16 changes: 16 additions & 0 deletions noxfiles/dev_nox.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,22 @@
from utils_nox import COMPOSE_DOWN_VOLUMES


@nox_session()
def shell(session: Session) -> None:
"""
Open a shell in an already-running Fides webesrver container.

If the container is not running, the command will fail.
"""
shell_command = (*EXEC_IT, "/bin/bash")
try:
session.run(*shell_command, external=True)
except CommandFailed:
session.error(
"Could not connect to the webserver container. Please confirm it is running and try again."
)


@nox_session()
def dev(session: Session) -> None:
"""
Expand Down
59 changes: 59 additions & 0 deletions src/fides/api/custom_types.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
"""Logic related to sanitizing and validating user application input."""
from html import escape
from re import compile as regex
from typing import Generator


class SafeStr(str):
"""
This class is designed to be used in place of the `str` type
any place where user input is expected.

The validation applied here does the typical sanitization/cleanup
that is required when dealing with user-supplied input.
"""

@classmethod
def __get_validators__(cls) -> Generator: # pragma: no cover
yield cls.validate

@classmethod
def validate(cls, value: str) -> str:

# HTML Escapes
value = escape(value)

if len(value) > 500:
raise ValueError("Value must be 500 characters or less.")

return value


class PhoneNumber(str):
"""
Format validated type for phone numbers.

Standard format can be found here: https://en.wikipedia.org/wiki/E.164
"""

@classmethod
def __get_validators__(cls) -> Generator:
yield cls.validate

@classmethod
def validate(cls, value: str) -> str:
# The front-end sends an empty string if the user doesn't input anything
if value == "":
return ""
max_length = 16 # Includes the +
min_length = 9
pattern = regex(r"^\+[1-9]\d{1,14}$")
if (
len(value) > max_length
or len(value) < min_length
or not pattern.search(value)
):
raise ValueError(
"Phone number must be formatted in E.164 format, i.e. '+15558675309'."
)
return value
11 changes: 5 additions & 6 deletions src/fides/api/ops/api/v1/endpoints/consent_request_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,19 +207,18 @@ def get_consent_preferences(
*, db: Session = Depends(get_db), data: Identity
) -> ConsentPreferences:
"""Gets the consent preferences for the specified user."""
if data.email:
lookup = data.email
elif data.phone_number:
lookup = data.phone_number
else:
if not data.email and not data.phone_number:
raise HTTPException(
status_code=HTTP_400_BAD_REQUEST, detail="No identity information provided"
)

# From the above check we know at least one exists
lookup = data.email if data.email else data.phone_number

identity = ProvidedIdentity.filter(
db,
conditions=(
(ProvidedIdentity.hashed_value == ProvidedIdentity.hash_value(lookup))
(ProvidedIdentity.hashed_value == ProvidedIdentity.hash_value(str(lookup)))
& (ProvidedIdentity.privacy_request_id.is_(None))
),
).first()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from typing import List, Optional

from pydantic import EmailStr
from pydantic.main import BaseModel

from fides.api.ops.schemas.base_class import NoValidationSchema
Expand All @@ -15,7 +16,7 @@ class KeyfileCreds(BaseModel):
project_id: str
private_key_id: Optional[str] = None
private_key: Optional[str] = None
client_email: Optional[str] = None
client_email: Optional[EmailStr] = None
client_id: Optional[str] = None
auth_uri: Optional[str] = None
token_uri: Optional[str] = None
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from typing import List, Optional

from pydantic import EmailStr

from fides.api.ops.schemas.base_class import NoValidationSchema
from fides.api.ops.schemas.connection_configuration.connection_secrets import (
ConnectionConfigSecretsSchema,
Expand All @@ -10,7 +12,7 @@ class EmailSchema(ConnectionConfigSecretsSchema):
"""Schema to validate the secrets needed for the EmailConnector"""

to_email: str
test_email: Optional[str] # Email to send a connection test email
test_email: Optional[EmailStr] # Email to send a connection test email

_required_components: List[str] = ["to_email"]

Expand Down
7 changes: 4 additions & 3 deletions src/fides/api/ops/schemas/drp_privacy_request.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
from enum import Enum
from typing import List, Optional

from pydantic import validator
from pydantic import EmailStr, validator

from fides.api.custom_types import PhoneNumber
from fides.api.ops.models.policy import DrpAction
from fides.api.ops.schemas.base_class import BaseSchema

Expand Down Expand Up @@ -50,9 +51,9 @@ class DrpIdentity(BaseSchema):
aud: Optional[str]
sub: Optional[str]
name: Optional[str]
email: Optional[str]
email: Optional[EmailStr]
email_verified: Optional[bool]
phone_number: Optional[str]
phone_number: Optional[PhoneNumber]
phone_number_verified: Optional[bool]
address: Optional[str]
address_verified: Optional[bool]
Expand Down
16 changes: 5 additions & 11 deletions src/fides/api/ops/schemas/messaging/messaging.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
from __future__ import annotations

from enum import Enum
from re import compile as regex
from typing import Any, Dict, List, Optional, Tuple, Type, Union

from fideslang import DEFAULT_TAXONOMY
from fideslang.validation import FidesKey
from pydantic import BaseModel, Extra, root_validator

from fides.api.custom_types import PhoneNumber, SafeStr
from fides.api.ops.models.privacy_request import CheckpointActionRequired
from fides.api.ops.schemas import Msg
from fides.api.ops.schemas.privacy_request import Consent
Expand Down Expand Up @@ -73,7 +73,7 @@ class ErrorNotificationBodyParams(BaseModel):
class SubjectIdentityVerificationBodyParams(BaseModel):
"""Body params required for subject identity verification email/sms template"""

verification_code: str
verification_code: SafeStr
verification_code_ttl_seconds: int

def get_verification_code_ttl_minutes(self) -> int:
Expand All @@ -86,7 +86,7 @@ def get_verification_code_ttl_minutes(self) -> int:
class RequestReceiptBodyParams(BaseModel):
"""Body params required for privacy request receipt template"""

request_types: List[str]
request_types: List[SafeStr]


class AccessRequestCompleteBodyParams(BaseModel):
Expand All @@ -99,7 +99,7 @@ class AccessRequestCompleteBodyParams(BaseModel):
class RequestReviewDenyBodyParams(BaseModel):
"""Body params required for privacy request review deny template"""

rejection_reason: Optional[str]
rejection_reason: Optional[SafeStr]


class ConsentPreferencesByUser(BaseModel):
Expand Down Expand Up @@ -262,7 +262,7 @@ class MessagingServiceSecretsTwilioSMS(BaseModel):
twilio_account_sid: str
twilio_auth_token: str
twilio_messaging_service_sid: Optional[str]
twilio_sender_phone_number: Optional[str]
twilio_sender_phone_number: Optional[PhoneNumber]

class Config:
"""Restrict adding other fields through this schema."""
Expand All @@ -276,12 +276,6 @@ def validate_fields(cls, values: Dict[str, Any]) -> Dict[str, Any]:
raise ValueError(
"Either the twilio_messaging_service_sid or the twilio_sender_phone_number should be supplied."
)
if sender_phone:
pattern = regex(r"^\+\d+$")
if not pattern.search(sender_phone):
raise ValueError(
"Sender phone number must include country code, formatted like +15558675309"
)
return values


Expand Down
3 changes: 2 additions & 1 deletion src/fides/api/ops/schemas/privacy_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from fideslang.validation import FidesKey
from pydantic import Field, validator
from fides.api.custom_types import SafeStr

from fides.api.ops.models.policy import ActionType
from fides.api.ops.models.privacy_request import (
Expand Down Expand Up @@ -222,7 +223,7 @@ class ReviewPrivacyRequestIds(BaseSchema):
class DenyPrivacyRequests(ReviewPrivacyRequestIds):
"""Pass in a list of privacy request ids and rejection reason"""

reason: Optional[str]
reason: Optional[SafeStr]


class BulkPostPrivacyRequests(BulkResponse):
Expand Down
19 changes: 4 additions & 15 deletions src/fides/api/ops/schemas/redis_cache.py
Original file line number Diff line number Diff line change
@@ -1,30 +1,19 @@
from re import compile as regex
from typing import Optional

from pydantic import Extra, validator
from pydantic import EmailStr, Extra

from fides.api.custom_types import PhoneNumber
from fides.api.ops.schemas.base_class import BaseSchema


class Identity(BaseSchema):
"""Some PII grouping pertaining to a human"""

phone_number: Optional[str] = None
email: Optional[str] = None
phone_number: Optional[PhoneNumber] = None
email: Optional[EmailStr] = None
ga_client_id: Optional[str] = None
ljt_readerID: Optional[str] = None

@validator("phone_number")
@classmethod
def validate_phone_number(cls, value: str) -> str:
if value:
pattern = regex(r"^\+[1-9]\d{1,14}$")
if not pattern.search(value):
raise ValueError(
"Identity phone number must be formatted in E.164 format. E.g +15558675309"
)
return value

class Config:
"""Only allow phone_number, email, and GA client id to be supplied"""

Expand Down
4 changes: 2 additions & 2 deletions src/fides/api/ops/schemas/registration.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from typing import Optional

from pydantic import Extra
from pydantic import EmailStr, Extra

from fides.api.ops.schemas.base_class import BaseSchema

Expand All @@ -25,5 +25,5 @@ class Registration(GetRegistrationStatusResponse):

analytics_id: str
opt_in: bool
user_email: Optional[str]
user_email: Optional[EmailStr]
user_organization: Optional[str]
Loading