Skip to content

Commit

Permalink
Log when User/SA/Roles is added to a group (#1205)
Browse files Browse the repository at this point in the history 
* Log when a user/sa/role is added to a group

* Update format and add unit tests

* Update syntax error

* Adding filtering to better view the audit logs

* Update better syntax to description (thank you Petra again!)

* Adding changes based on comments from PR

* Update type to TYPE_SERVICE_ACCOUNT based on comments

* Remove migration for now from conflicts

* Adding changes based on comments

* Update according to recent rebase

* Update error type according to changes

* Update PR related to recent updates and comments

* Update pre-commit file with new revision 24.10.0

* Update PR with related comments

* Update changes to audit logs to fit the latest changes and after rebase

* Added changes to audit log based on comments

* Remove unnecessary function

---------

Co-authored-by: Libor Pichler <[email protected]>
  • Loading branch information
@Ellen-Yi-Dong @lpichler
Ellen-Yi-Dong and lpichler authored Feb 5, 2025
1 parent f3d3324 commit ee58a00
Show file tree
Hide file tree
Showing 4 changed files with 529 additions and 102 deletions.
18 changes: 15 additions & 3 deletions rbac/management/audit_log/model.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,8 @@ def get_resource_item(self, r_type, request, *args, **kwargs):
group_object_name = "group: " + group_object.name
return group_object_id, group_object_name

elif r_type == AuditLog.PERMISSION:
# TODO: update for permission related items
return None
else:
return ValueError("Wrong Resource Type")

def find_edited_field(self, resource, resource_name, request, object):
"""Add additional information when group/role is edited."""
Expand Down Expand Up @@ -138,3 +137,16 @@ def log_edit(self, request, resource, object):
self.action = AuditLog.EDIT
self.tenant_id = self.get_tenant_id(request)
super(AuditLog, self).save()

def log_group_assignment(self, request, resource_type, resource, values_for_description, assigned_resource_type):
"""Audit Log when a role, user/principal, or service account is added to a group."""
self.principal_username = request.user.username
self.resource_type = resource_type
self.resource_id = resource.id
resource_name = "group: " + resource.name

self.description = f"{assigned_resource_type} {values_for_description} added to {resource_name}"

self.action = AuditLog.ADD
self.tenant_id = self.get_tenant_id(request)
super(AuditLog, self).save()
6 changes: 6 additions & 0 deletions rbac/management/audit_log/view.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,13 @@ class AuditLogViewSet(mixins.ListModelMixin, viewsets.GenericViewSet):
serializer_class = AuditLogSerializer
permission_classes = (AuditLogAccessPermission,)

def order_by_id(self, queryset):
"""Order queryset by id."""
return queryset.order_by("-id").values()

def list(self, request, *args, **kwargs):
"""List all of the audit logs within database by tenant."""
self.queryset = filter_queryset_by_tenant(AuditLog.objects.all(), request.tenant)
if request.query_params.get("order_by") is not None:
self.queryset = self.order_by_id(self.queryset)
return super().list(request=request, args=args, kwargs=kwargs)
95 changes: 86 additions & 9 deletions rbac/management/group/view.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,12 @@
SERVICE_ACCOUNT_NAME_KEY = "service_account_name"
SERVICE_ACCOUNT_USERNAME_FORMAT = "service-account-{clientId}"
VALID_EXCLUDE_VALUES = ["true", "false"]
VALID_GROUP_ROLE_FILTERS = ["role_name", "role_description", "role_display_name", "role_system"]
VALID_GROUP_ROLE_FILTERS = [
"role_name",
"role_description",
"role_display_name",
"role_system",
]
VALID_GROUP_PRINCIPAL_FILTERS = ["principal_username"]
VALID_PRINCIPAL_ORDER_FIELDS = ["username"]
VALID_PRINCIPAL_TYPE_VALUE = ["service-account", "user"]
Expand Down Expand Up @@ -118,7 +123,10 @@ def roles_filter(self, queryset, field, values):
roles_list = [value.lower() for value in values.split(",")]

discriminator = validate_and_get_key(
self.request.query_params, ROLE_DISCRIMINATOR_KEY, VALID_ROLE_ROLE_DISCRIMINATOR, "any"
self.request.query_params,
ROLE_DISCRIMINATOR_KEY,
VALID_ROLE_ROLE_DISCRIMINATOR,
"any",
)

if discriminator == "any":
Expand Down Expand Up @@ -499,7 +507,13 @@ def validate_principals_in_proxy_request(self, principals, org_id=None):
if len(resp.get("data", [])) == 0:
return {
"status_code": status.HTTP_404_NOT_FOUND,
"errors": [{"detail": "User(s) {} not found.".format(users), "status": "404", "source": "principals"}],
"errors": [
{
"detail": "User(s) {} not found.".format(users),
"status": "404",
"source": "principals",
}
],
}
return resp

Expand Down Expand Up @@ -755,6 +769,14 @@ def principals(self, request: Request, uuid: Optional[UUID] = None):
self.ensure_id_for_service_accounts_exists(
user=request.user, service_accounts=service_accounts
)
auditlog = AuditLog()
auditlog.log_group_assignment(
request,
AuditLog.GROUP,
group,
service_accounts,
Principal.Types.SERVICE_ACCOUNT,
)
except InsufficientPrivilegesError as ipe:
return Response(
status=status.HTTP_403_FORBIDDEN,
Expand Down Expand Up @@ -788,6 +810,14 @@ def principals(self, request: Request, uuid: Optional[UUID] = None):
proxy_response = self.validate_principals_in_proxy_request(principals, org_id=org_id)
if len(proxy_response.get("data", [])) > 0:
principals_from_response = proxy_response.get("data", [])
auditlog = AuditLog()
auditlog.log_group_assignment(
request,
AuditLog.GROUP,
group,
principals,
Principal.Types.USER,
)
if isinstance(proxy_response, dict) and "errors" in proxy_response:
return Response(status=proxy_response["status_code"], data=proxy_response["errors"])

Expand All @@ -798,18 +828,34 @@ def principals(self, request: Request, uuid: Optional[UUID] = None):
service_accounts=service_accounts,
org_id=org_id,
)
auditlog = AuditLog()
auditlog.log_group_assignment(
request,
AuditLog.GROUP,
group,
service_accounts,
Principal.Types.SERVICE_ACCOUNT,
)
new_users = []
if len(principals) > 0:
group, new_users = self.add_users(group, principals_from_response, org_id=org_id)
auditlog = AuditLog()
auditlog.log_group_assignment(
request,
AuditLog.GROUP,
group,
principals,
Principal.Types.USER,
)

dual_write_handler = RelationApiDualWriteGroupHandler(
group, ReplicationEventType.ADD_PRINCIPALS_TO_GROUP
)
dual_write_handler.replicate_new_principals(new_users + new_service_accounts)

# Serialize the group...
output = GroupSerializer(group)
response = Response(status=status.HTTP_200_OK, data=output.data)

elif request.method == "GET":
group = self.get_object()
# Check if the request comes with a bunch of service account client IDs that we need to check. Since this
Expand All @@ -818,7 +864,11 @@ def principals(self, request: Request, uuid: Optional[UUID] = None):
if SERVICE_ACCOUNT_CLIENT_IDS_KEY in request.query_params:
# pagination is ignored in this case
for query_param in request.query_params:
if query_param not in [SERVICE_ACCOUNT_CLIENT_IDS_KEY, "limit", "offset"]:
if query_param not in [
SERVICE_ACCOUNT_CLIENT_IDS_KEY,
"limit",
"offset",
]:
return Response(
status=status.HTTP_400_BAD_REQUEST,
data={
Expand Down Expand Up @@ -897,7 +947,11 @@ def principals(self, request: Request, uuid: Optional[UUID] = None):

# Get the "username_only" query parameter.
username_only = validate_and_get_key(
request.query_params, USERNAME_ONLY_KEY, VALID_BOOLEAN_VALUE, "false", required=False
request.query_params,
USERNAME_ONLY_KEY,
VALID_BOOLEAN_VALUE,
"false",
required=False,
)

# Build the options dict.
Expand All @@ -907,7 +961,10 @@ def principals(self, request: Request, uuid: Optional[UUID] = None):
# parameter. It is important because we need to call BOP for
# the users, and IT for the service accounts.
principalType = validate_and_get_key(
request.query_params, PRINCIPAL_TYPE_KEY, VALID_PRINCIPAL_TYPE_VALUE, required=False
request.query_params,
PRINCIPAL_TYPE_KEY,
VALID_PRINCIPAL_TYPE_VALUE,
required=False,
)

# Store the principal type in the options dict.
Expand Down Expand Up @@ -936,7 +993,10 @@ def principals(self, request: Request, uuid: Optional[UUID] = None):
service_accounts = it_service.get_service_accounts_group(
group=group, user=request.user, options=options
)
except (requests.exceptions.ConnectionError, UnexpectedStatusCodeFromITError):
except (
requests.exceptions.ConnectionError,
UnexpectedStatusCodeFromITError,
):
return Response(
status=status.HTTP_500_INTERNAL_SERVER_ERROR,
data={
Expand Down Expand Up @@ -1122,10 +1182,24 @@ def roles(self, request, uuid=None, principals=None):
serializer = GroupRoleSerializerIn(data=request.data)
if serializer.is_valid(raise_exception=True):
roles = request.data.pop(ROLES_KEY, [])

with transaction.atomic():
group = set_system_flag_before_update(group, request.tenant, request.user)
add_roles(group, roles, request.tenant, user=request.user)

response_data = GroupRoleSerializerIn(group)
response = Response(status=status.HTTP_200_OK, data=response_data.data)
if status.is_success(response.status_code):
for role in response_data.data["data"]:
auditlog = AuditLog()
auditlog.log_group_assignment(
request,
AuditLog.GROUP,
group,
role["name"],
AuditLog.ROLE,
)

elif request.method == "GET":
serialized_roles = self.obtain_roles(request, group)
page = self.paginate_queryset(serialized_roles)
Expand Down Expand Up @@ -1226,7 +1300,10 @@ def remove_service_accounts(self, user: User, group: Group, service_accounts: It

# Get the group's service accounts that match the service accounts that the user specified.
valid_service_accounts = Principal.objects.filter(
group=group, tenant=tenant, type="service-account", service_account_id__in=service_accounts
group=group,
tenant=tenant,
type=Principal.Types.SERVICE_ACCOUNT,
service_account_id__in=service_accounts,
)

# Collect the service account IDs the user specified.
Expand Down
Loading

0 comments on commit ee58a00

Please sign in to comment.