25249 - Add/remove maximus staff group #3276
Conversation
| @@ -213,7 +214,7 @@ def update_membership(self, updated_fields, **kwargs): | |||
| # Ensure that this user is an COORDINATOR or ADMIN on the org associated with this membership | |||
| logger.debug("<update_membership") | |||
| user_from_context: UserContext = kwargs["user_context"] | |||
| check_auth(org_id=self._model.org_id, one_of_roles=(COORDINATOR, ADMIN, STAFF)) | |||
| check_auth(org_id=self._model.org_id, one_of_roles=(COORDINATOR, ADMIN, GOV_ACCOUNT_USER)) | |||
There was a problem hiding this comment.
this will break staff I think
| KeycloakService.remove_from_account_holders_group(model.user.keycloak_guid) | ||
| if len(MembershipModel.find_orgs_for_user(model.user.id)) == 0: | ||
| KeycloakService.remove_from_account_holders_group(model.user.keycloak_guid) | ||
| if model.org.type_code == OrgType.MAXIMUS_STAFF.value: |
There was a problem hiding this comment.
could we throw in the contact centre changes in here too?
| and len(MembershipModel.find_orgs_for_user(model.user.id)) == 0 | ||
| ): | ||
| if model.org.type_code == OrgType.MAXIMUS_STAFF.value: | ||
| KeycloakService.add_user_to_group(model.user.keycloak_guid, GROUP_MAXIMUS_STAFF) |
There was a problem hiding this comment.
should check for and remove staff group
| ): | ||
| if model.org.type_code in org_type_to_group_mapping: | ||
| KeycloakService.add_user_to_group(model.user.keycloak_guid, org_type_to_group_mapping[model.org.type_code]) | ||
| # remove staff group I'll need |
There was a problem hiding this comment.
should also deactivate any membership to the staff org here when removing the group
| if model.org.type_code in org_type_to_group_mapping: | ||
| KeycloakService.add_user_to_group( | ||
| model.user.keycloak_guid, org_type_to_group_mapping[model.org.type_code] | ||
| ) |
There was a problem hiding this comment.
After adding them to these new external staff groups, we still need to:
- check if they are part of the keycloak staff group and remove them
- check if they have membership in the staff org and deactivate their membership
There was a problem hiding this comment.
I wouldn't put it "after adding them", I don't think they should be coupled... I'd do that independently for the STAFF org
So they can
- Remove users from the staff org, it will remove their staff group / role
- When they invite into new org it just adds them the new group
There was a problem hiding this comment.
Do we need a check to see if people are joining multiple staff orgs though?
There was a problem hiding this comment.
yup, that is a good point. I agree. It will clean up some of the current logic so we don't need to manage the data by separating out the STAFF portion so it is evaluated on its own.
| ) | ||
| try: | ||
| membership = Membership.get_membership_for_org_and_user(model.org.id, model.user.id) | ||
| membership.deactivate_membership() |
There was a problem hiding this comment.
hmm deactivate_membership() calls add_or_remove_group, which calls deactivate_membership when the membership status is inactive. We probably should split out some of this logic.
There was a problem hiding this comment.
Should we just modify deactivate membership then? Doesn't that work as expected?
There was a problem hiding this comment.
Should we just modify deactivate membership then? Doesn't that work as expected?
Looks like a good spot to update to me.
| membership = Membership.get_membership_for_org_and_user(model.org.id, model.user.id) | ||
| membership.deactivate_membership() | ||
| except BusinessException as e: | ||
| logger.error(f"Error deactivating membership: {e}") |
There was a problem hiding this comment.
seems like a really bad state if this happens, we shouldn't catch and do nothing
|
We reviewed this and as discussed, I will dig into this a bit further as it will affect my permission scripts. I will review the backend code and adjust roles / permissions as needed. |
|
Issue #:
bcgov/entity#25249
Description of changes:
New team member added to Maximus staff account
Team member is removed from Maximus account
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the sbc-auth license (Apache 2.0).