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

Create Invitation API [Validation] #35448

Merged
merged 45 commits into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
2de602f
create class to consolidate validation of web user invitation fields
Jtang-1 Nov 20, 2024
1f42d95
this validation is already done as part of django form from CustomDat…
Jtang-1 Nov 21, 2024
b4c260c
refactor for readability
Jtang-1 Nov 22, 2024
2873fcb
adds function to validate profile
Jtang-1 Nov 22, 2024
06e60e1
refactor validation of email to consolidate
Jtang-1 Nov 22, 2024
591245a
do same validation as done for bulk user import in preparation of val…
Jtang-1 Nov 22, 2024
3d99f35
refactor: move validating tableau role and tableau group header to c…
Jtang-1 Nov 22, 2024
9e801be
prefactor - consolidate validation to be used for invitation creation…
Jtang-1 Nov 22, 2024
a0119f6
refactor: extract required values from spec in higher level function …
Jtang-1 Nov 22, 2024
0fc4ddd
create validation for location in preparetion for create invitation/u…
Jtang-1 Nov 23, 2024
fc6ced8
refactor: location_ids is the equivalent return
Jtang-1 Nov 23, 2024
30a3214
refactor: consolidate validation for location having users
Jtang-1 Nov 25, 2024
2e1de11
refactor: consolidate validating that primary location is one of the …
Jtang-1 Nov 25, 2024
7584aea
refactor to extract out validation of tableau groups from the expecte…
Jtang-1 Nov 25, 2024
21fd7ac
add validation of tableau group to Admin Invites User Validator to be…
Jtang-1 Nov 25, 2024
ff73910
prefactor: extract out the validation of the tableau_role independent…
Jtang-1 Nov 26, 2024
cd79129
add validation for tableau role
Jtang-1 Nov 26, 2024
062dde5
add validation for custom data in preparation for web user creation API
Jtang-1 Nov 26, 2024
f0cd397
remove unnecessary caching
Jtang-1 Nov 26, 2024
ef2b0e6
refactor: DRY function
Jtang-1 Nov 26, 2024
64c904a
refactor: The initial purpose of the validation class was to be used …
Jtang-1 Nov 26, 2024
a043006
perform permission and privilege check only if the parameter exists
Jtang-1 Nov 26, 2024
cca4098
remove location fields from backend if location shouldn't be shown
Jtang-1 Nov 27, 2024
7f3807e
update test and adds test for validating if invitation email is an ex…
Jtang-1 Nov 27, 2024
8fe97bc
rename since only one error is returned, not multiple
Jtang-1 Nov 27, 2024
998a221
separate conditionally allowed headers since allowed_headers is globa…
Jtang-1 Nov 28, 2024
6366ebf
fix typo
Jtang-1 Nov 28, 2024
34e4dc7
update test
Jtang-1 Nov 29, 2024
8d30494
test TableauRoleValidation and TableauGroupsValidation
Jtang-1 Nov 29, 2024
eba62f3
fix import
Jtang-1 Nov 29, 2024
d9f55a1
test: new tests for validation of Web User Resourcec
Jtang-1 Nov 30, 2024
427420a
update test so that order doesn't matter for the listed "illegal colu…
Jtang-1 Nov 30, 2024
c2df916
update test so it doesn't matter the order of the arguments
Jtang-1 Dec 1, 2024
d3b2d3a
delete unused error message. It is now handled in validate_assigned_l…
Jtang-1 Dec 4, 2024
eff4a17
correct patch, path should be where function is used
Jtang-1 Dec 4, 2024
6f0ec77
For form validation, clean only occurs if request is post so the "is_…
Jtang-1 Dec 4, 2024
ab7e8a7
fix argument
Jtang-1 Dec 4, 2024
d698b22
fix test - property was moved out of the class
Jtang-1 Dec 4, 2024
11edd76
avoid regenerating list of roles on each call to reduce memory
Jtang-1 Dec 4, 2024
a5ee207
bug: needs to check user has access to existing locations when all lo…
Jtang-1 Dec 4, 2024
33159f5
rename function for clarity
Jtang-1 Dec 4, 2024
da73efa
remove unneccessary model deletion from test database
Jtang-1 Dec 4, 2024
06dd234
clarify naming to distinguish from the form to invite web users
Jtang-1 Dec 5, 2024
8fee5fe
correct bug where regardless if locations are being changed,
Jtang-1 Dec 5, 2024
3dfa157
test: tests that invitiation access is checked even if no locations a…
Jtang-1 Dec 5, 2024
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
105 changes: 105 additions & 0 deletions corehq/apps/api/tests/test_validation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
from django.test import TestCase
from unittest.mock import patch

from corehq.apps.api.validation import WebUserResourceValidator
from corehq.apps.domain.models import Domain
from corehq.apps.users.models import WebUser
from corehq.util.test_utils import flag_enabled, flag_disabled


class TestWebUserResourceValidator(TestCase):

@classmethod
def setUpClass(cls):
super().setUpClass()
cls.domain = Domain(name="test-domain", is_active=True)
cls.domain.save()
cls.addClassCleanup(cls.domain.delete)
cls.requesting_user = WebUser.create(cls.domain.name, "[email protected]", "123", None, None)
cls.validator = WebUserResourceValidator(cls.domain.name, cls.requesting_user)

@classmethod
def tearDownClass(cls):
cls.requesting_user.delete(None, None)
super().tearDownClass()

def test_validate_parameters(self):
params = {"email": "[email protected]", "role": "Admin"}
self.assertIsNone(self.validator.validate_parameters(params))

invalid_params = {"invalid_param": "value"}
self.assertEqual(self.validator.validate_parameters(invalid_params), "Invalid parameter(s): invalid_param")

@flag_enabled('TABLEAU_USER_SYNCING')
@patch('corehq.apps.users.models.WebUser.has_permission', return_value=True)
def test_validate_parameters_with_tableau_edit_permission(self, mock_has_permission):
params = {"email": "[email protected]", "role": "Admin", "tableau_role": "Viewer"}
self.assertIsNone(self.validator.validate_parameters(params))

@flag_disabled('TABLEAU_USER_SYNCING')
@patch('corehq.apps.users.models.WebUser.has_permission', return_value=False)
def test_validate_parameters_without_tableau_edit_permission(self, mock_has_permission):
params = {"email": "[email protected]", "role": "Admin", "tableau_role": "Viewer"}
self.assertEqual(self.validator.validate_parameters(params),
"You do not have permission to edit Tableau Configuration.")
Jtang-1 marked this conversation as resolved.
Show resolved Hide resolved
Jtang-1 marked this conversation as resolved.
Show resolved Hide resolved

@patch('corehq.apps.registration.validation.domain_has_privilege', return_value=True)
Jtang-1 marked this conversation as resolved.
Show resolved Hide resolved
def test_validate_parameters_with_profile_permission(self, mock_domain_has_privilege):
params = {"email": "[email protected]", "role": "Admin", "profile": "some_profile"}
self.assertIsNone(self.validator.validate_parameters(params))

@patch('corehq.apps.registration.validation.domain_has_privilege', return_value=False)
def test_validate_parameters_without_profile_permission(self, mock_domain_has_privilege):
params = {"email": "[email protected]", "role": "Admin", "profile": "some_profile"}
self.assertEqual(self.validator.validate_parameters(params),
"This domain does not have user profile privileges.")

@patch('corehq.apps.registration.validation.domain_has_privilege', return_value=True)
def test_validate_parameters_with_location_privilege(self, mock_domain_has_privilege):
params = {"email": "[email protected]", "role": "Admin", "primary_location": "some_location"}
self.assertIsNone(self.validator.validate_parameters(params))
params = {"email": "[email protected]", "role": "Admin", "assigned_locations": "some_location"}
self.assertIsNone(self.validator.validate_parameters(params))

@patch('corehq.apps.registration.validation.domain_has_privilege', return_value=False)
def test_validate_parameters_without_location_privilege(self, mock_domain_has_privilege):
params = {"email": "[email protected]", "role": "Admin", "primary_location": "some_location"}
self.assertEqual(self.validator.validate_parameters(params),
"This domain does not have locations privileges.")

params = {"email": "[email protected]", "role": "Admin", "assigned_locations": "some_location"}
self.assertEqual(self.validator.validate_parameters(params),
"This domain does not have locations privileges.")

def test_validate_email(self):
self.assertIsNone(self.validator.validate_email("[email protected]", True))

self.assertEqual(self.validator.validate_email("[email protected]", True),
"A user with this email address is already in "
"this project or has a pending invitation.")

deactivated_user = WebUser.create(self.domain.name, "[email protected]", "123", None, None)
deactivated_user.is_active = False
deactivated_user.save()
self.assertEqual(self.validator.validate_email("[email protected]", True),
"A user with this email address is deactivated. ")

def test_validate_locations(self):
with patch('corehq.apps.user_importer.validation.LocationValidator.validate_spec') as mock_validate_spec:
mock_validate_spec.return_value = None
self.assertIsNone(self.validator.validate_locations(self.requesting_user.username,
["loc1", "loc2"], "loc1"))

actual_spec = mock_validate_spec.call_args[0][0]
self.assertEqual(actual_spec['username'], self.requesting_user.username)
self.assertCountEqual(actual_spec['location_code'], ["loc1", "loc2"])

self.assertEqual(
self.validator.validate_locations(self.requesting_user.username, ["loc1", "loc2"], "loc3"),
"Primary location must be one of the user's locations"
)

self.assertEqual(
self.validator.validate_locations(self.requesting_user.username, ["loc1", "loc2"], ""),
"Primary location can't be empty if the user has any locations set"
)
87 changes: 87 additions & 0 deletions corehq/apps/api/validation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
from memoized import memoized

from corehq.apps.custom_data_fields.models import CustomDataFieldsDefinition
from corehq.apps.reports.util import get_allowed_tableau_groups_for_domain
from corehq.apps.user_importer.importer import SiteCodeToLocationCache
from corehq.apps.user_importer.validation import (
RoleValidator,
ProfileValidator,
LocationValidator,
TableauGroupsValidator,
TableauRoleValidator,
CustomDataValidator,
EmailValidator,
)
from corehq.apps.users.validation import validate_primary_location_assignment
from corehq.apps.registration.validation import AdminInvitesUserFormValidator


class WebUserResourceValidator():
def __init__(self, domain, requesting_user):
self.domain = domain
self.requesting_user = requesting_user

@property
def roles_by_name(self):
from corehq.apps.users.views.utils import get_editable_role_choices
return {role[1]: role[0] for role in get_editable_role_choices(self.domain, self.requesting_user,
allow_admin_role=True)}

@property
@memoized
def profiles_by_name(self):
from corehq.apps.users.views.mobile.custom_data_fields import UserFieldsView
return CustomDataFieldsDefinition.get_profiles_by_name(self.domain, UserFieldsView.field_type)

@property
def location_cache(self):
return SiteCodeToLocationCache(self.domain)

def validate_parameters(self, parameters):
allowed_params = ['email', 'role', 'primary_location', 'assigned_locations',
'profile', 'custom_user_data', 'tableau_role', 'tableau_groups']
invalid_params = [param for param in parameters if param not in allowed_params]
if invalid_params:
return f"Invalid parameter(s): {', '.join(invalid_params)}"
return AdminInvitesUserFormValidator.validate_parameters(self.domain, self.requesting_user, parameters)

def validate_role(self, role):
spec = {'role': role}
return RoleValidator(self.domain, self.roles_by_name()).validate_spec(spec)

def validate_profile(self, new_profile_name):
profile_validator = ProfileValidator(self.domain, self.requesting_user, True, self.profiles_by_name())
spec = {'user_profile': new_profile_name}
return profile_validator.validate_spec(spec)

def validate_custom_data(self, custom_data, profile_name):
custom_data_validator = CustomDataValidator(self.domain, self.profiles_by_name())
spec = {'data': custom_data, 'user_profile': profile_name}
return custom_data_validator.validate_spec(spec)

def validate_email(self, email, is_post):
if is_post:
Jtang-1 marked this conversation as resolved.
Show resolved Hide resolved
error = AdminInvitesUserFormValidator.validate_email(self.domain, email)
if error:
return error
email_validator = EmailValidator(self.domain, 'email')
spec = {'email': email}
return email_validator.validate_spec(spec)

def validate_locations(self, editable_user, assigned_location_codes, primary_location_code):
error = validate_primary_location_assignment(primary_location_code, assigned_location_codes)
if error:
return error

location_validator = LocationValidator(self.domain, self.requesting_user, self.location_cache, True)
location_codes = list(set(assigned_location_codes + [primary_location_code]))
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't really apply to this PR but I'm just sort of curious... @jingcheng16 isn't there some thing with mobile worker location assignment where the primary location needs to come first in an import or in the API? I vaguely remember some discussion a while back about that.

Doesn't look like for web user import the primary needs to come first in the set of codes, and certainly there is no validation for this in bulk import at the moment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IIRC, for both mobile and web user bulk import, it assumes that the first location listed is the primary location. So there's no validation involves, it just assumes and updates/assigns the primary location as such.

spec = {'location_code': location_codes,
'username': editable_user}
return location_validator.validate_spec(spec)

def validate_tableau_group(self, tableau_groups):
allowed_groups_for_domain = get_allowed_tableau_groups_for_domain(self.domain) or []
return TableauGroupsValidator.validate_tableau_groups(allowed_groups_for_domain, tableau_groups)

def validate_tableau_role(self, tableau_role):
return TableauRoleValidator.validate_tableau_role(tableau_role)
12 changes: 12 additions & 0 deletions corehq/apps/custom_data_fields/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,18 @@ def get_profile_required_for_user_type_list(cls, domain, field_type):
return profile_required_for_user_type_list
return None

@classmethod
def get_profiles_by_name(cls, domain, field_type):
definition = cls.get(domain, field_type)
if definition:
profiles = definition.get_profiles()
return {
profile.name: profile
for profile in profiles
}
else:
return {}

class FieldFilterConfig:
def __init__(self, required_only=False, is_required_check_func=None):
self.required_only = required_only
Expand Down
43 changes: 19 additions & 24 deletions corehq/apps/registration/forms.py
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you link which part of CustomDataEditor form already did the validation you deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See my reply here. The validation is done here

Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
from corehq.apps.programs.models import Program
from corehq.toggles import WEB_USER_INVITE_ADDITIONAL_FIELDS
from corehq.apps.users.forms import SelectUserLocationForm, BaseTableauUserForm
from corehq.apps.users.models import CouchUser, WebUser
from corehq.apps.users.models import CouchUser


class RegisterWebUserForm(forms.Form):
Expand Down Expand Up @@ -433,7 +433,7 @@ def clean_eula_confirmed(self):
return data


class WebUserInvitationForm(BaseUserInvitationForm):
class AcceptedWebUserInvitationForm(BaseUserInvitationForm):
"""
Form for a brand new user, before they've created a domain or done anything on CommCare HQ.
"""
Expand Down Expand Up @@ -492,7 +492,7 @@ class AdminInvitesUserForm(SelectUserLocationForm):
max_length=User._meta.get_field('email').max_length)
role = forms.ChoiceField(choices=(), label="Project Role")

def __init__(self, data=None, excluded_emails=None, is_add_user=None,
def __init__(self, data=None, is_add_user=None,
role_choices=(), should_show_location=False, can_edit_tableau_config=False,
custom_data=None, *, domain, **kwargs):
self.custom_data = custom_data
Expand Down Expand Up @@ -520,8 +520,6 @@ def __init__(self, data=None, excluded_emails=None, is_add_user=None,
choices = [('', '')] + list((prog.get_id, prog.name) for prog in programs)
self.fields['program'].choices = choices

self.excluded_emails = [x.lower() for x in excluded_emails] if excluded_emails else []

if self.can_edit_tableau_config:
self._initialize_tableau_fields(data, domain)

Expand Down Expand Up @@ -554,6 +552,9 @@ def __init__(self, data=None, excluded_emails=None, is_add_user=None,
'primary_location',
)
)
else:
self.fields.pop('assigned_locations', None)
self.fields.pop('primary_location', None)
if self.can_edit_tableau_config:
fields.append(
crispy.Fieldset(
Expand Down Expand Up @@ -589,30 +590,17 @@ def __init__(self, data=None, excluded_emails=None, is_add_user=None,
),
)

def _validate_profile(self, profile_id):
valid_profile_ids = {choice[0] for choice in self.custom_data.form.fields[PROFILE_SLUG].widget.choices}
if profile_id and profile_id not in valid_profile_ids:
raise forms.ValidationError(
_('Invalid profile selected. Please select a valid profile.'),
)

def clean_email(self):
email = self.cleaned_data['email'].strip()
if email.lower() in self.excluded_emails:
raise forms.ValidationError(_("A user with this email address is already in "
"this project or has a pending invitation."))
web_user = WebUser.get_by_username(email)
if web_user and not web_user.is_active:
raise forms.ValidationError(_("A user with this email address is deactivated. "))

from corehq.apps.registration.validation import AdminInvitesUserFormValidator
error = AdminInvitesUserFormValidator.validate_email(self.domain, email)
if error:
raise forms.ValidationError(error)
return email

def clean(self):
cleaned_data = super(AdminInvitesUserForm, self).clean()

if (('tableau_role' in cleaned_data or 'tableau_group_indices' in cleaned_data)
and not self.can_edit_tableau_config):
raise forms.ValidationError(_("You do not have permission to edit Tableau Configuraion."))

if 'tableau_group_indices' in cleaned_data:
cleaned_data['tableau_group_ids'] = [
self.tableau_form.allowed_tableau_groups[int(i)].id
Expand All @@ -631,10 +619,17 @@ def clean(self):

if prefixed_profile_key in custom_user_data:
profile_id = custom_user_data.pop(prefixed_profile_key)
self._validate_profile(profile_id)
Jtang-1 marked this conversation as resolved.
Show resolved Hide resolved
cleaned_data['profile'] = profile_id
cleaned_data['custom_user_data'] = get_prefixed(custom_user_data, self.custom_data.prefix)

from corehq.apps.registration.validation import AdminInvitesUserFormValidator
error = AdminInvitesUserFormValidator.validate_parameters(
self.domain,
self.request.couch_user,
cleaned_data.keys()
)
if error:
raise forms.ValidationError(error)
return cleaned_data

def _initialize_tableau_fields(self, data, domain):
Expand Down
Loading
Loading