Skip to content

Commit

Permalink
OpenConceptLab/ocl_issues#1167 | inactive user login | inactive user …
Browse files Browse the repository at this point in the history
…search and list
  • Loading branch information
snyaggarwal committed Dec 21, 2021
1 parent 3f08eb3 commit 3f51bc3
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 8 deletions.
1 change: 1 addition & 0 deletions core/common/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
INCLUDE_SUMMARY = 'includeSummary'
INCLUDE_VERIFICATION_TOKEN = 'includeVerificationToken'
INCLUDE_AUTH_GROUPS = 'includeAuthGroups'
INCLUDE_INACTIVE = 'includeInactive'
INCLUDE_SOURCE_VERSIONS = 'includeSourceVersions'
INCLUDE_COLLECTION_VERSIONS = 'includeCollectionVersions'
MAPPING_LOOKUP_CONCEPTS = 'lookupConcepts'
Expand Down
7 changes: 6 additions & 1 deletion core/common/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from core import __version__
from core.common.constants import SEARCH_PARAM, LIST_DEFAULT_LIMIT, CSV_DEFAULT_LIMIT, \
LIMIT_PARAM, NOT_FOUND, MUST_SPECIFY_EXTRA_PARAM_IN_BODY, INCLUDE_RETIRED_PARAM, VERBOSE_PARAM, HEAD, LATEST, \
BRIEF_PARAM, ES_REQUEST_TIMEOUT
BRIEF_PARAM, ES_REQUEST_TIMEOUT, INCLUDE_INACTIVE
from core.common.exceptions import Http400
from core.common.mixins import PathWalkerMixin, ListWithHeadersMixin
from core.common.serializers import RootSerializer
Expand Down Expand Up @@ -76,6 +76,9 @@ def _should_exclude_retired_from_search_results(self):
INCLUDE_RETIRED_PARAM, None) in [True, 'true']
return not include_retired

def should_include_inactive(self):
return self.request.query_params.get(INCLUDE_INACTIVE) in ['true', True]

def _should_include_private(self):
return self.is_user_document() or self.request.user.is_staff or self.is_user_scope()

Expand Down Expand Up @@ -457,6 +460,8 @@ def __search_results(self): # pylint: disable=too-many-branches,too-many-locals
if self.should_perform_es_search():
results = self.document_model.search()
default_filters = self.default_filters.copy()
if self.is_user_document() and self.should_include_inactive():
default_filters.pop('is_active', None)
if self.is_source_child_document_model() and self.__should_query_latest_version():
default_filters['is_latest_version'] = True

Expand Down
41 changes: 41 additions & 0 deletions core/integration_tests/tests_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,26 @@ def test_login(self):
self.assertEqual(response.status_code, 400)
self.assertEqual(response.data, dict(non_field_errors=["Unable to log in with provided credentials."]))

def test_login_inactive_user(self):
user = UserProfileFactory(username='marty', is_active=False)
user.set_password('boogeyman')
user.save()

response = self.client.post(
'/users/login/',
dict(username='marty', password='boogeyman'),
format='json'
)

self.assertEqual(response.status_code, 401)
self.assertEqual(
response.data,
dict(
detail='This account is deactivated. Please contact OCL Team to activate this account.',
email=user.email
)
)


class UserListViewTest(OCLAPITestCase):
def setUp(self):
Expand All @@ -271,6 +291,27 @@ def test_get_200(self):
self.assertEqual(response.data[0]['username'], 'ocladmin')
self.assertEqual(response.data[0]['email'], self.superuser.email)

def test_get_200_with_inactive_user(self):
UserProfileFactory(is_active=False, username='inactive')

response = self.client.get(
'/users/',
format='json'
)

self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.data), 1)
self.assertEqual(response.data[0]['username'], 'ocladmin')

response = self.client.get(
'/users/?includeInactive=true',
format='json'
)

self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.data), 2)
self.assertEqual(sorted(user['username'] for user in response.data), sorted(['ocladmin', 'inactive']))

def test_get_summary_200(self):
response = self.client.get(
'/users/?summary=true',
Expand Down
1 change: 1 addition & 0 deletions core/users/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
INVALID_RESET_PASSWORD_LINK = 'This link is invalid, possibly because it has already been used.'
VERIFY_EMAIL_MESSAGE = 'A verification email has been sent to the address on record. Verify your email address to ' \
'activate your account.'
INACTIVE_USER_MESSAGE = 'This account is deactivated. Please contact OCL Team to activate this account.'
OCL_SERVERS_GROUP = 'ocl_servers'
OCL_FHIR_SERVERS_GROUP = 'ocl_fhir_servers'
HAPI_FHIR_SERVERS_GROUP = 'hapi_fhir_servers'
Expand Down
9 changes: 9 additions & 0 deletions core/users/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@ def get_search_document():
from core.users.documents import UserProfileDocument
return UserProfileDocument

@property
def status(self):
if not self.is_active:
return 'deactivated'
if not self.verified:
return 'unverified'

return 'verified'

@property
def user(self):
return self.username
Expand Down
4 changes: 2 additions & 2 deletions core/users/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class Meta:
model = UserProfile
fields = (
'username', 'name', 'url', 'logo_url', 'sources', 'collections', 'organizations',
'is_superuser', 'is_staff', 'first_name', 'last_name'
'is_superuser', 'is_staff', 'first_name', 'last_name', 'status'
)


Expand Down Expand Up @@ -142,7 +142,7 @@ class Meta:
'public_collections', 'public_sources', 'created_on', 'updated_on', 'created_by', 'updated_by',
'url', 'organizations_url', 'extras', 'sources_url', 'collections_url', 'website', 'last_login',
'logo_url', 'subscribed_orgs', 'is_superuser', 'is_staff', 'first_name', 'last_name', 'verified',
'verification_token', 'date_joined', 'auth_groups'
'verification_token', 'date_joined', 'auth_groups', 'status'
)

def __init__(self, *args, **kwargs):
Expand Down
6 changes: 6 additions & 0 deletions core/users/tests/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ def test_user(self):
self.assertEqual(UserProfile().user, '')
self.assertEqual(UserProfile(username='foo').user, 'foo')

def test_status(self):
self.assertEqual(UserProfile(is_active=True, verified=True).status, 'verified')
self.assertEqual(UserProfile(is_active=True, verified=False).status, 'unverified')
self.assertEqual(UserProfile(is_active=False, verified=True).status, 'deactivated')
self.assertEqual(UserProfile(is_active=False, verified=False).status, 'deactivated')

@patch('core.users.models.UserProfile.source_set')
def test_public_sources(self, source_set_mock):
source_set_mock.exclude = Mock(return_value=Mock(filter=Mock(return_value=Mock(count=Mock(return_value=10)))))
Expand Down
25 changes: 20 additions & 5 deletions core/users/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from core.common.utils import parse_updated_since_param, parse_updated_since
from core.common.views import BaseAPIView, BaseLogoView
from core.orgs.models import Organization
from core.users.constants import VERIFICATION_TOKEN_MISMATCH, VERIFY_EMAIL_MESSAGE
from core.users.constants import VERIFICATION_TOKEN_MISMATCH, VERIFY_EMAIL_MESSAGE, INACTIVE_USER_MESSAGE
from core.users.documents import UserProfileDocument
from core.users.search import UserProfileSearch
from core.users.serializers import UserDetailSerializer, UserCreateSerializer, UserListSerializer, UserSummarySerializer
Expand All @@ -34,9 +34,17 @@ class TokenAuthenticationView(ObtainAuthToken):

@swagger_auto_schema(request_body=AuthTokenSerializer)
def post(self, request, *args, **kwargs):
user = UserProfile.objects.filter(username=request.data.get('username')).first()
if not user:
raise Http404()

if not user.is_active:
return Response(
{'detail': INACTIVE_USER_MESSAGE, 'email': user.email}, status=status.HTTP_401_UNAUTHORIZED
)

result = super().post(request, *args, **kwargs)
try:
user = UserProfile.objects.get(username=request.data['username'])
if not user.verified:
user.send_verification_email()
return Response(
Expand All @@ -53,7 +61,7 @@ class UserBaseView(BaseAPIView):
lookup_field = 'user'
pk_field = 'username'
model = UserProfile
queryset = UserProfile.objects.filter(is_active=True)
queryset = UserProfile.objects
es_fields = UserProfile.es_fields
document_model = UserProfileDocument
facet_class = UserProfileSearch
Expand All @@ -77,6 +85,8 @@ def get_queryset(self):
self.queryset = self.queryset.filter(created_at__gte=parse_updated_since(date_joined_since))
if date_joined_before:
self.queryset = self.queryset.filter(created_at__lt=parse_updated_since(date_joined_before))
if not self.should_include_inactive():
self.queryset = self.queryset.filter(is_active=True)
return self.queryset


Expand Down Expand Up @@ -122,8 +132,10 @@ def get(self, request, *args, **kwargs):

if not self.can_view(organization):
return Response(status=status.HTTP_403_FORBIDDEN)

self.queryset = organization.members.all()
queryset = organization.members
if not self.should_include_inactive():
queryset = queryset.filter(is_active=True)
self.queryset = queryset.all()
return self.list(request, *args, **kwargs)

def post(self, request, *args, **kwargs): # pylint: disable=unused-argument
Expand Down Expand Up @@ -271,6 +283,9 @@ class UserReactivateView(UserBaseView, UpdateAPIView):
permission_classes = (IsAdminUser, )
queryset = UserProfile.objects.filter(is_active=False)

def get_queryset(self):
return self.queryset

def update(self, request, *args, **kwargs):
profile = self.get_object()
profile.undelete()
Expand Down

0 comments on commit 3f51bc3

Please sign in to comment.