Skip to content

Commit

Permalink
Restrict actions on inactive user (#2111)
Browse files Browse the repository at this point in the history
  • Loading branch information
jadmsaadaot authored Sep 1, 2023
1 parent 74f7406 commit 70f77d5
Show file tree
Hide file tree
Showing 14 changed files with 91 additions and 37 deletions.
27 changes: 24 additions & 3 deletions met-api/src/met_api/models/staff_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
from sqlalchemy.sql import text
from sqlalchemy.sql.operators import ilike_op

from met_api.utils.enums import UserStatus

from .base_model import BaseModel
from .db import db
from .pagination_options import PaginationOptions
Expand All @@ -35,7 +37,7 @@ class StaffUser(BaseModel):
tenant_id = db.Column(db.Integer, db.ForeignKey('tenant.id'), nullable=True)

@classmethod
def get_all_paginated(cls, pagination_options: PaginationOptions, search_text=''):
def get_all_paginated(cls, pagination_options: PaginationOptions, search_text='', include_inactive=False):
"""Fetch list of users by access type."""
query = cls.query
query = cls._add_tenant_filter(query)
Expand All @@ -48,6 +50,9 @@ def get_all_paginated(cls, pagination_options: PaginationOptions, search_text=''
if search_text:
query = query.filter(ilike_op(StaffUser.full_name, '%' + search_text + '%'))

if not include_inactive:
query = query.filter(StaffUser.status_id == UserStatus.ACTIVE.value)

no_pagination_options = not pagination_options.page or not pagination_options.size
if no_pagination_options:
items = query.all()
Expand All @@ -57,9 +62,25 @@ def get_all_paginated(cls, pagination_options: PaginationOptions, search_text=''
return page.items, page.total

@classmethod
def get_user_by_external_id(cls, _external_id) -> StaffUser:
def get_by_id(cls, _id, include_inactive=False) -> Optional[StaffUser]:
"""Get a user by id."""
query = db.session.query(StaffUser) \
.filter(StaffUser.id == _id)

if not include_inactive:
query = query.filter(StaffUser.status_id == UserStatus.ACTIVE.value)
return query.first()

@classmethod
def get_user_by_external_id(cls, _external_id, include_inactive=False) -> Optional[StaffUser]:
"""Get a user with the provided external id."""
return cls.query.filter(func.lower(StaffUser.external_id) == func.lower(_external_id)).first()
query = db.session.query(StaffUser) \
.filter(func.lower(StaffUser.external_id) == func.lower(_external_id))

if not include_inactive:
query = query.filter(StaffUser.status_id == UserStatus.ACTIVE.value)

return query.first()

@classmethod
def create_user(cls, user) -> StaffUser:
Expand Down
6 changes: 4 additions & 2 deletions met-api/src/met_api/resources/staff_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ def get():
pagination_options=pagination_options,
search_text=args.get('search_text', '', str),
include_groups=args.get('include_groups', default=False, type=lambda v: v.lower() == 'true'),
include_inactive=args.get('include_inactive', default=False, type=lambda v: v.lower() == 'true')
)
return jsonify(users), HTTPStatus.OK

Expand All @@ -86,11 +87,12 @@ class StaffUser(Resource):
@cross_origin(origins=allowedorigins())
@require_role([Role.VIEW_USERS.value])
def get(user_id):
"""Return a set of users(staff only)."""
"""Fetch a user by id."""
args = request.args
user = StaffUserService.get_user_by_id(
user_id,
include_groups=args.get('include_groups', default=False, type=lambda v: v.lower() == 'true'),
include_inactive=True,
)
return user, HTTPStatus.OK

Expand All @@ -110,7 +112,7 @@ def patch(user_id):
if data.get('active', None) is None:
return {'message': 'active field is required'}, HTTPStatus.BAD_REQUEST

user = StaffUserService.toggle_user_active_status(
user = StaffUserMembershipService().toggle_user_active_status(
user_id,
active=data.get('active'),
)
Expand Down
4 changes: 3 additions & 1 deletion met-api/src/met_api/services/membership_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ def _validate_create_membership(engagement_id, user_details):
error='You cannot add yourself to an engagement.',
status_code=HTTPStatus.FORBIDDEN.value)

user_id = user_details.get('id')

groups = user_details.get('groups')
if KeycloakGroups.EAO_IT_ADMIN.value in groups:
raise BusinessException(
Expand All @@ -61,7 +63,7 @@ def _validate_create_membership(engagement_id, user_details):

existing_membership = MembershipModel.find_by_engagement_and_user_id(
engagement_id,
user_details.get('id'),
user_id,
status=MembershipStatus.ACTIVE.value
)

Expand Down
19 changes: 18 additions & 1 deletion met-api/src/met_api/services/staff_user_membership_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
from http import HTTPStatus

from met_api.exceptions.business_exception import BusinessException
from met_api.models.staff_user import StaffUser as StaffUserModel
from met_api.schemas.staff_user import StaffUserSchema
from met_api.services.membership_service import MembershipService
from met_api.services.staff_user_service import StaffUserService
from met_api.services.staff_user_service import KEYCLOAK_SERVICE, StaffUserService
from met_api.utils.constants import Groups
from met_api.utils.enums import UserStatus


class StaffUserMembershipService:
Expand Down Expand Up @@ -43,3 +45,18 @@ def reassign_user(cls, user_id, group_name):
MembershipService.revoke_memberships_bulk(user_id)
new_user = StaffUserService.get_user_by_id(user_id, include_groups=True)
return StaffUserSchema().dump(new_user)

@staticmethod
def toggle_user_active_status(user_external_id: str, active: bool):
"""Toggle user active status."""
user = StaffUserModel.get_user_by_external_id(user_external_id, include_inactive=True)
if user is None:
raise KeyError('User not found')

if not active:
MembershipService.revoke_memberships_bulk(user.id)

KEYCLOAK_SERVICE.toggle_user_enabled_status(user_id=user_external_id, enabled=active)
user.status_id = UserStatus.ACTIVE.value if active else UserStatus.INACTIVE.value
user.save()
return StaffUserSchema().dump(user)
27 changes: 8 additions & 19 deletions met-api/src/met_api/services/staff_user_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from met_api.services.keycloak import KeycloakService
from met_api.utils import notification
from met_api.utils.constants import GROUP_NAME_MAPPING, Groups
from met_api.utils.enums import KeycloakGroupName, UserStatus
from met_api.utils.enums import KeycloakGroupName
from met_api.utils.template import Template
from met_api.config import get_gc_notify_config

Expand All @@ -22,20 +22,20 @@ class StaffUserService:
"""User management service."""

@classmethod
def get_user_by_id(cls, _user_id, include_groups=False):
def get_user_by_id(cls, _user_id, include_groups=False, include_inactive=False):
"""Get user by id."""
user_schema = StaffUserSchema()
db_user = StaffUserModel.find_by_id(_user_id)
db_user = StaffUserModel.get_by_id(_user_id, include_inactive)
user = user_schema.dump(db_user)
if include_groups:
cls.attach_groups([user])
return user

@classmethod
def get_user_by_external_id(cls, _external_id):
def get_user_by_external_id(cls, _external_id, include_inactive=False):
"""Get user by external id."""
user_schema = StaffUserSchema()
db_user = StaffUserModel.get_user_by_external_id(_external_id)
db_user = StaffUserModel.get_user_by_external_id(_external_id, include_inactive)
return user_schema.dump(db_user)

def create_or_update_user(self, user: dict):
Expand Down Expand Up @@ -127,10 +127,11 @@ def find_users(
cls,
pagination_options: PaginationOptions = None,
search_text='',
include_groups=False
include_groups=False,
include_inactive=False
):
"""Return a list of users."""
users, total = StaffUserModel.get_all_paginated(pagination_options, search_text)
users, total = StaffUserModel.get_all_paginated(pagination_options, search_text, include_inactive)
user_collection = StaffUserSchema(many=True).dump(users)
if include_groups:
cls.attach_groups(user_collection)
Expand Down Expand Up @@ -188,15 +189,3 @@ def validate_user(db_user: StaffUserModel):
raise BusinessException(
error='This user is already a Superuser.',
status_code=HTTPStatus.CONFLICT.value)

@staticmethod
def toggle_user_active_status(user_external_id: str, active: bool):
"""Toggle user active status."""
db_user = StaffUserModel.get_user_by_external_id(user_external_id)
if db_user is None:
raise KeyError('User not found')

KEYCLOAK_SERVICE.toggle_user_enabled_status(user_id=user_external_id, enabled=active)
db_user.status_id = UserStatus.ACTIVE.value if active else UserStatus.INACTIVE.value
db_user.save()
return StaffUserSchema().dump(db_user)
15 changes: 11 additions & 4 deletions met-web/src/components/userManagement/listing/ActionsDropDown.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, { useMemo, useContext } from 'react';
import { MenuItem, Select } from '@mui/material';
import { User, USER_GROUP } from 'models/user';
import { User, USER_GROUP, USER_STATUS } from 'models/user';
import { Palette } from 'styles/Theme';
import { UserManagementContext } from './UserManagementContext';
import { useAppSelector } from 'hooks';
Expand Down Expand Up @@ -47,7 +47,10 @@ export const ActionsDropDown = ({ selectedUser }: { selectedUser: User }) => {
setUser(selectedUser);
setassignRoleModalOpen(true);
},
condition: hasNoRole() && roles.includes(USER_ROLES.EDIT_MEMBERS),
condition:
hasNoRole() &&
roles.includes(USER_ROLES.EDIT_MEMBERS) &&
selectedUser.status_id == USER_STATUS.ACTIVE.value,
},
{
value: 2,
Expand All @@ -56,7 +59,8 @@ export const ActionsDropDown = ({ selectedUser }: { selectedUser: User }) => {
setUser(selectedUser);
setAddUserModalOpen(true);
},
condition: !hasNoRole() && !isAdmin() && !isViewer(),
condition:
!hasNoRole() && !isAdmin() && !isViewer() && selectedUser.status_id == USER_STATUS.ACTIVE.value,
},
{
value: 3,
Expand All @@ -65,7 +69,10 @@ export const ActionsDropDown = ({ selectedUser }: { selectedUser: User }) => {
setUser(selectedUser);
setReassignRoleModalOpen(true);
},
condition: !hasNoRole() && roles.includes(USER_ROLES.UPDATE_USER_GROUP),
condition:
!hasNoRole() &&
roles.includes(USER_ROLES.UPDATE_USER_GROUP) &&
selectedUser.status_id == USER_STATUS.ACTIVE.value,
},
],
[selectedUser.id, selectedUser.main_group],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ export const UserManagementContextProvider = ({ children }: { children: JSX.Elem
sort_order,
include_groups: true,
search_text: searchText,
include_inactive: true,
});
setUsers(response.items);
setPageInfo({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { reinstateMembership, revokeMembership } from 'services/membershipServic
import { useAppDispatch } from 'hooks';
import { openNotification } from 'services/notificationService/notificationSlice';
import { UserDetailsContext } from './UserDetailsContext';
import { USER_STATUS } from 'models/user';

interface ActionDropDownItem {
value: number;
Expand All @@ -15,7 +16,7 @@ interface ActionDropDownItem {
}
export const ActionsDropDown = ({ membership }: { membership: EngagementTeamMember }) => {
const [loading, setLoading] = React.useState(false);
const { getUserMemberships } = React.useContext(UserDetailsContext);
const { getUserMemberships, savedUser } = React.useContext(UserDetailsContext);
const dispatch = useAppDispatch();

const handleRevoke = async () => {
Expand Down Expand Up @@ -59,13 +60,17 @@ export const ActionsDropDown = ({ membership }: { membership: EngagementTeamMemb
value: 1,
label: 'Revoke',
action: () => handleRevoke(),
condition: membership.status !== ENGAGEMENT_MEMBERSHIP_STATUS.Revoked,
condition:
membership.status !== ENGAGEMENT_MEMBERSHIP_STATUS.Revoked &&
savedUser?.status_id === USER_STATUS.ACTIVE.value,
},
{
value: 2,
label: 'Reinstate',
action: () => handleReinstate(),
condition: membership.status !== ENGAGEMENT_MEMBERSHIP_STATUS.Active,
condition:
membership.status !== ENGAGEMENT_MEMBERSHIP_STATUS.Active &&
savedUser?.status_id === USER_STATUS.ACTIVE.value,
},
],
[membership.id, membership.status],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { Engagement } from 'models/engagement';
import axios, { AxiosError } from 'axios';
import { Palette } from 'styles/Theme';
import ControlledRadioGroup from 'components/common/ControlledInputComponents/ControlledRadioGroup';
import { HTTP_STATUS_CODES } from 'constants/httpResponseCodes';

export const AddToEngagementModal = () => {
const { savedUser, addUserModalOpen, setAddUserModalOpen, getUserMemberships, getUserDetails } =
Expand Down Expand Up @@ -157,7 +158,7 @@ export const AddToEngagementModal = () => {
};

const setErrors = (error: AxiosError) => {
if (error.response?.status !== 409) {
if (error.response?.status !== HTTP_STATUS_CODES.CONFLICT) {
return;
}
setBackendError(error.response?.data.message || '');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { formatDate } from 'components/common/dateHelper';
import AssignedEngagementsListing from './AssignedEngagementsListing';
import UserStatusButton from './UserStatusButton';
import UserDetailsSkeleton from './UserDetailsSkeleton';
import { USER_GROUP } from 'models/user';
import { USER_GROUP, USER_STATUS } from 'models/user';

export const UserDetail = ({ label, value }: { label: string; value: JSX.Element }) => {
return (
Expand Down Expand Up @@ -80,7 +80,10 @@ export const UserDetails = () => {
<Grid container justifyContent={'flex-end'} alignItems={'flex-end'} item xs={6}>
<PrimaryButton
onClick={() => setAddUserModalOpen(true)}
disabled={savedUser?.main_group === USER_GROUP.VIEWER.label}
disabled={
savedUser?.main_group === USER_GROUP.VIEWER.label ||
savedUser?.status_id === USER_STATUS.INACTIVE.value
}
>
+ Add to an Engagement
</PrimaryButton>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export const UserDetailsContextProvider = ({ children }: { children: JSX.Element
};

const getUserDetails = async () => {
setUserLoading(true);
const fetchedUser = await getUser({ user_id: Number(userId), include_groups: true });
setSavedUser(fetchedUser);
setUserLoading(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { USER_GROUP } from 'models/user';

const UserStatusButton = () => {
const { roles } = useAppSelector((state) => state.user);
const { savedUser } = useContext(UserDetailsContext);
const { savedUser, getUserDetails } = useContext(UserDetailsContext);
const [userStatus, setUserStatus] = useState(false);
const [togglingUserStatus, setTogglingUserStatus] = useState(false);
const dispatch = useAppDispatch();
Expand All @@ -30,6 +30,7 @@ const UserStatusButton = () => {
setUserStatus(active);
setTogglingUserStatus(true);
await toggleUserStatus(savedUser.external_id, active);
getUserDetails();
dispatch(
openNotification({
severity: 'success',
Expand Down
3 changes: 3 additions & 0 deletions met-web/src/constants/httpResponseCodes.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
export const HTTP_STATUS_CODES = {
NOT_FOUND: 404,
FORBIDDEN: 403,
BAD_REQUEST: 400,
CONFLICT: 409,
};
1 change: 1 addition & 0 deletions met-web/src/services/userService/api/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ interface GetUserListParams {
search_text?: string;
// If yes, user groups will be fetched as well from keycloak
include_groups?: boolean;
include_inactive?: boolean;
}
export const getUserList = async (params: GetUserListParams = {}): Promise<Page<User>> => {
const responseData = await http.GetRequest<Page<User>>(Endpoints.User.GET_LIST, params);
Expand Down

0 comments on commit 70f77d5

Please sign in to comment.