This is an automated email from the ASF dual-hosted git repository. madhan pushed a commit to branch ranger-2.5 in repository https://gitbox.apache.org/repos/asf/ranger.git
The following commit(s) were added to refs/heads/ranger-2.5 by this push: new d4607772b RANGER-4851: fixed incorrect admin audit logs for user-group membership updates d4607772b is described below commit d4607772bfc71a1c00d125f1c58596e574aa3d3c Author: Dhaval.Rajpara <dhavalrajpara1...@gmail.com> AuthorDate: Tue Jul 23 14:09:12 2024 +0530 RANGER-4851: fixed incorrect admin audit logs for user-group membership updates Signed-off-by: Madhan Neethiraj <mad...@apache.org> --- .../java/org/apache/ranger/biz/RangerBizUtil.java | 2 +- .../main/java/org/apache/ranger/biz/XUserMgr.java | 5 +- .../service/AbstractAuditedResourceService.java | 19 +++++- .../ranger/service/XGroupUserServiceBase.java | 2 +- .../src/views/AuditEvent/AdminLogs.jsx | 17 ++++++ .../src/views/AuditEvent/AdminLogs/GroupLogs.jsx | 1 - .../src/views/AuditEvent/AdminLogs/RoleLogs.jsx | 1 - .../AdminLogs/UserAssociationWithGroupLogs.jsx | 70 ++++++++++++++++++++++ .../src/views/AuditEvent/AdminLogs/UserLogs.jsx | 1 - .../views/AuditEvent/AdminLogs/UserprofileLogs.jsx | 1 - .../src/views/AuditEvent/OperationAdminModal.jsx | 12 +++- .../webapp/scripts/views/reports/AuditLayout.js | 36 ++++++++++- .../apache/ranger/service/TestXPermMapService.java | 27 ++++++++- 13 files changed, 180 insertions(+), 14 deletions(-) diff --git a/security-admin/src/main/java/org/apache/ranger/biz/RangerBizUtil.java b/security-admin/src/main/java/org/apache/ranger/biz/RangerBizUtil.java index d5714e858..0d0102288 100644 --- a/security-admin/src/main/java/org/apache/ranger/biz/RangerBizUtil.java +++ b/security-admin/src/main/java/org/apache/ranger/biz/RangerBizUtil.java @@ -1135,7 +1135,7 @@ public class RangerBizUtil { } public void createTrxLog(List<XXTrxLogV2> trxLogList) { - if (trxLogList == null) { + if (trxLogList == null || trxLogList.size() == 0) { return; } diff --git a/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java b/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java index 4a2cd4535..c6062d21d 100755 --- a/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java +++ b/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java @@ -427,6 +427,7 @@ public class XUserMgr extends XUserMgrBase { vXPortalUser.setPassword(password); } Collection<Long> groupIdList = vXUser.getGroupIdList(); + VXUser existing = xUserService.readResource(vXUser.getId()); XXPortalUser xXPortalUser = new XXPortalUser(); xXPortalUser = userMgr.updateUserWithPass(vXPortalUser); //update permissions start @@ -486,7 +487,9 @@ public class XUserMgr extends XUserMgrBase { } } - VXUser existing = xUserService.readResource(vXUser.getId()); + if (password == null) { + vXUser.setPassword(hiddenPasswordString); //To stop Auditing Password transaction log, when it is not edited. + } List<XXTrxLogV2> trxLogList = xUserService.getTransactionLog(vXUser, existing, OPERATION_UPDATE_CONTEXT); vXUser.setPassword(hiddenPasswordString); diff --git a/security-admin/src/main/java/org/apache/ranger/service/AbstractAuditedResourceService.java b/security-admin/src/main/java/org/apache/ranger/service/AbstractAuditedResourceService.java index fd51724af..12146cbda 100644 --- a/security-admin/src/main/java/org/apache/ranger/service/AbstractAuditedResourceService.java +++ b/security-admin/src/main/java/org/apache/ranger/service/AbstractAuditedResourceService.java @@ -28,6 +28,7 @@ import org.apache.ranger.entity.XXTrxLogV2; import org.apache.ranger.plugin.util.JsonUtilsV2; import org.apache.ranger.util.RangerEnumUtil; import org.apache.ranger.view.VXDataObject; +import org.apache.ranger.view.VXTrxLogV2.AttributeChangeInfo; import org.apache.ranger.view.VXTrxLogV2.ObjectChangeInfo; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -86,6 +87,11 @@ public abstract class AbstractAuditedResourceService<T extends XXDBBase, V exten try { ObjectChangeInfo objChangeInfo = new ObjectChangeInfo(); + if ("Password".equalsIgnoreCase(attrName)) { + oldValue = hiddenPasswordString; + newValue = hiddenPasswordString; + } + objChangeInfo.addAttribute(attrName, oldValue, newValue); trxLog.setChangeInfo(JsonUtilsV2.objToJson(objChangeInfo)); @@ -132,7 +138,16 @@ public abstract class AbstractAuditedResourceService<T extends XXDBBase, V exten processFieldToCreateTrxLog(trxLog, obj, oldObj, action, objChangeInfo); } - trxLogList.add(new XXTrxLogV2(classType, obj.getId(), getObjectName(obj), getParentObjectType(obj, oldObj), getParentObjectId(obj, oldObj), getParentObjectName(obj, oldObj), toActionString(action), JsonUtilsV2.objToJson(objChangeInfo))); + if (objChangeInfo.getAttributes() != null) { + for (AttributeChangeInfo changeInfo : objChangeInfo.getAttributes()) { + if ("Password".equalsIgnoreCase(changeInfo.getAttributeName())) { + changeInfo.setNewValue(hiddenPasswordString); + changeInfo.setOldValue(hiddenPasswordString); + } + } + + trxLogList.add(new XXTrxLogV2(classType, obj.getId(), getObjectName(obj), getParentObjectType(obj, oldObj), getParentObjectId(obj, oldObj), getParentObjectName(obj, oldObj), toActionString(action), JsonUtilsV2.objToJson(objChangeInfo))); + } } catch (Exception e) { logger.warn("failed to get transaction log for object: type=" + obj.getClass().getName() + ", id=" + obj.getId(), e); } @@ -202,7 +217,7 @@ public abstract class AbstractAuditedResourceService<T extends XXDBBase, V exten newValue = null; } - if (StringUtils.equals(prevValue, newValue) || (StringUtils.isEmpty(prevValue) && StringUtils.isEmpty(newValue))) { + if ((StringUtils.isEmpty(prevValue) && StringUtils.isEmpty(newValue)) || StringUtils.equals(prevValue, newValue)) { return; } diff --git a/security-admin/src/main/java/org/apache/ranger/service/XGroupUserServiceBase.java b/security-admin/src/main/java/org/apache/ranger/service/XGroupUserServiceBase.java index 6776ab374..3f10abfde 100644 --- a/security-admin/src/main/java/org/apache/ranger/service/XGroupUserServiceBase.java +++ b/security-admin/src/main/java/org/apache/ranger/service/XGroupUserServiceBase.java @@ -42,7 +42,7 @@ public abstract class XGroupUserServiceBase<T extends XXGroupUser, V extends VXG public XGroupUserServiceBase() { super(AppConstants.CLASS_TYPE_XA_GROUP_USER, AppConstants.CLASS_TYPE_XA_GROUP); - trxLogAttrs.put("parentGroupId", new VTrxLogAttr("parentGroupId", "Group Name")); + trxLogAttrs.put("name", new VTrxLogAttr("name", "Group Name")); } @Override diff --git a/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs.jsx b/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs.jsx index 4f974097f..669266a33 100644 --- a/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs.jsx +++ b/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs.jsx @@ -221,6 +221,23 @@ function Admin() { Role {action}d <strong>{objectname}</strong> </span> ); + else if (classtype == ClassTypes.CLASS_TYPE_XA_GROUP_USER.value) + operation = + action == "create" ? ( + <span> + User <strong> {objectname} </strong>added to group{" "} + <strong> + {rawValue?.row?.original?.parentObjectName || " "} + </strong> + </span> + ) : ( + <span> + User <strong> {objectname} </strong>removed from group{" "} + <strong> + {rawValue?.row?.original?.parentObjectName || " "} + </strong> + </span> + ); return <div className="text-truncate">{operation}</div>; } }, diff --git a/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs/GroupLogs.jsx b/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs/GroupLogs.jsx index 62d71d4db..0d9b98d1e 100644 --- a/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs/GroupLogs.jsx +++ b/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs/GroupLogs.jsx @@ -164,7 +164,6 @@ export const GroupLogs = ({ data, reportdata }) => { <div> <div className="fw-bolder">Name : {objectName}</div> <div className="fw-bolder">Date: {currentTimeZone(createDate)}</div> - <div className="fw-bolder">Created By: {owner} </div> <div className="fw-bolder">Deleted By: {owner} </div> <br /> <h5 className="bold wrap-header m-t-sm">Group Details:</h5> diff --git a/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs/RoleLogs.jsx b/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs/RoleLogs.jsx index b2056209d..09226da8b 100644 --- a/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs/RoleLogs.jsx +++ b/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs/RoleLogs.jsx @@ -509,7 +509,6 @@ export const RoleLogs = ({ data, reportdata }) => { <div> <div className="fw-bolder">Name: {objectName}</div> <div className="fw-bolder">Date: {currentTimeZone(createDate)}</div> - <div className="fw-bolder">Created By: {owner} </div> <div className="fw-bolder">Deleted By: {owner} </div> <br /> <h5 className="bold wrap-header m-t-sm">Role Detail:</h5> diff --git a/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs/UserAssociationWithGroupLogs.jsx b/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs/UserAssociationWithGroupLogs.jsx new file mode 100644 index 000000000..6e66390fc --- /dev/null +++ b/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs/UserAssociationWithGroupLogs.jsx @@ -0,0 +1,70 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React from "react"; +import { Table } from "react-bootstrap"; +import { currentTimeZone } from "Utils/XAUtils"; +import { find } from "lodash"; + +export const UserAssociationWithGroupLogs = ({ data, reportdata }) => { + const { objectName, createDate, owner, action, parentObjectName } = data; + let actionType = action == "create" ? "Created By" : "Deleted By"; + let reportFinalData = find(reportdata, { + parentObjectName: parentObjectName + }); + let operation = + action == "create" ? ( + <span> + User <strong> {objectName} </strong>added to group{" "} + <strong>{reportFinalData?.newValue || " "}</strong> + </span> + ) : ( + <span> + User <strong> {objectName} </strong>removed from group{" "} + <strong>{reportFinalData?.previousValue || " "}</strong> + </span> + ); + return ( + <div> + <div className="fw-bolder">Name : {objectName}</div> + <div className="fw-bolder">Date: {currentTimeZone(createDate)}</div> + <div className="fw-bolder"> + {actionType}: {owner} + </div> + <br /> + <h5 className="bold wrap-header m-t-sm">User Details:</h5> + + <Table className="table table-bordered w-50"> + <thead className="thead-light"> + <tr> + <th>Fields</th> + <th>Value</th> + </tr> + </thead> + <tbody> + <tr> + <td className="table-warning">Group</td> + <td className="table-warning">{operation}</td> + </tr> + </tbody> + </Table> + </div> + ); +}; +export default UserAssociationWithGroupLogs; diff --git a/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs/UserLogs.jsx b/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs/UserLogs.jsx index 25f6f5d3f..95e88266e 100644 --- a/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs/UserLogs.jsx +++ b/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs/UserLogs.jsx @@ -393,7 +393,6 @@ export const UserLogs = ({ data, reportdata }) => { <div> <div className="fw-bolder">Name : {objectName}</div> <div className="fw-bolder">Date: {currentTimeZone(createDate)}</div> - <div className="fw-bolder">Created By: {owner} </div> <div className="fw-bolder">Deleted By: {owner} </div> <br /> <h5 className="bold wrap-header m-t-sm">User Details:</h5> diff --git a/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs/UserprofileLogs.jsx b/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs/UserprofileLogs.jsx index 72753b9d0..f2bce899a 100644 --- a/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs/UserprofileLogs.jsx +++ b/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/AdminLogs/UserprofileLogs.jsx @@ -106,7 +106,6 @@ export const UserprofileLogs = ({ data, reportdata }) => { <div> <div className="fw-bolder">Name : {objectName}</div> <div className="fw-bolder">Date: {currentTimeZone(createDate)}</div> - <div className="fw-bolder">Created By: {owner} </div> <div className="fw-bolder">Deleted By: {owner} </div> <br /> <h5 className="bold wrap-header m-t-sm">User Details:</h5> diff --git a/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/OperationAdminModal.jsx b/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/OperationAdminModal.jsx index d8378403a..ee67bc4c3 100644 --- a/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/OperationAdminModal.jsx +++ b/security-admin/src/main/webapp/react-webapp/src/views/AuditEvent/OperationAdminModal.jsx @@ -19,12 +19,13 @@ import React, { useEffect, useState } from "react"; import { ClassTypes } from "../../utils/XAEnums"; -import { Modal, Button } from "react-bootstrap"; +import { Modal, Button, Table } from "react-bootstrap"; import { fetchApi } from "Utils/fetchAPI"; import SecurityZonelogs from "./AdminLogs/SecurityZonelogs"; import UserLogs from "./AdminLogs/UserLogs"; import GroupLogs from "./AdminLogs/GroupLogs"; import RoleLogs from "./AdminLogs/RoleLogs"; +import UserAssociationWithGroupLogs from "./AdminLogs/UserAssociationWithGroupLogs"; import ServiceLogs from "./AdminLogs/ServiceLogs"; import PolicyLogs from "./AdminLogs/PolicyLogs"; import PasswordLogs from "./AdminLogs/PasswordLogs"; @@ -100,6 +101,15 @@ export const OperationAdminModal = ({ onHide, show, data = {} }) => { <UserLogs reportdata={reportdata} data={data} /> )} + {/* USER ADDED TO GROUP */} + + {objectClassType == ClassTypes.CLASS_TYPE_XA_GROUP_USER.value && ( + <UserAssociationWithGroupLogs + reportdata={reportdata} + data={data} + /> + )} + {/* GROUP */} {objectClassType == ClassTypes.CLASS_TYPE_XA_GROUP.value && ( diff --git a/security-admin/src/main/webapp/scripts/views/reports/AuditLayout.js b/security-admin/src/main/webapp/scripts/views/reports/AuditLayout.js index 8eea802f9..806fc7a35 100644 --- a/security-admin/src/main/webapp/scripts/views/reports/AuditLayout.js +++ b/security-admin/src/main/webapp/scripts/views/reports/AuditLayout.js @@ -927,6 +927,31 @@ define(function(require) { userName :self.model.get('owner'), action : action }); + } else if(self.model.get('objectClassType') == XAEnums.ClassTypes.CLASS_TYPE_XA_GROUP_USER.value){ + var actionType = action == "create" ? "Created" : "Deleted" + var fieldVal = action == "create" ? 'User <b>'+_.escape(self.model.get('objectName'))+'</b> added to group <b>'+_.escape(self.model.get('parentObjectName'))+ '</b>' + : 'User <b>'+_.escape(self.model.get('objectName'))+'</b> removed from group <b>'+ _.escape(self.model.get('parentObjectName')) + '</b>'; + + var view = '<div class="diff-content">\ + <div class="no-margin label-size13-weightbold">Name: '+self.model.get('objectName')+'</div>\ + <div class="no-margin label-size13-weightbold">Date: '+objectCreatedDate+'</div>\ + <div class="no-margin label-size13-weightbold">'+actionType+' By: '+self.model.get('owner')+'</div>\ + <h5 class="bold wrap-header m-t-sm">User Details:</h5>\ + <div class="diff">\ + <div class="diff-left">\ + <h3>Fields</h3>\ + <ol class="attr">\ + <li class="change-row">Group</li>\ + </ol>\ + </div>\ + <div class="diff-right">\ + <h3>Value</h3>\ + <ol class="list-unstyled data">\ + <li class="change-row">'+fieldVal+'</li>\ + </ol>\ + </div>\ + </div>\ + </div>' } else { var view = new vOperationDiffDetail({ collection : fullTrxLogListForTrxId, @@ -1025,7 +1050,8 @@ define(function(require) { rawValue = model.get('objectClassType'); var action = model.get('action'), name = _.escape(model.get('objectName')), label = XAUtils.enumValueToLabel(XAEnums.ClassTypes,rawValue), html = '', - hasAction = ["EXPORT JSON", "EXPORT EXCEL", "EXPORT CSV", "IMPORT START", "IMPORT END"]; + hasAction = ["EXPORT JSON", "EXPORT EXCEL", "EXPORT CSV", "IMPORT START", "IMPORT END"], + parentObjectName = _.escape(model.get('parentObjectName')); if($.inArray(action,hasAction)>=0){ if(action == "EXPORT JSON" || action == "EXPORT EXCEL" || action == "EXPORT CSV") return 'Exported policies'; @@ -1046,8 +1072,12 @@ define(function(require) { html = 'User profile '+action+'d '+'<b>'+name+'</b>'; else if(rawValue == XAEnums.ClassTypes.CLASS_TYPE_RANGER_SECURITY_ZONE.value) html = 'Security Zone '+action+'d '+'<b>'+name+'</b>'; - else if(rawValue == XAEnums.ClassTypes.CLASS_TYPE_RANGER_ROLE.value) - html = 'Role '+action+'d '+'<b>'+name+'</b>'; + else if(rawValue == XAEnums.ClassTypes.CLASS_TYPE_RANGER_ROLE.value) + html = 'Role '+action+'d '+'<b>'+name+'</b>'; + else if(rawValue == XAEnums.ClassTypes.CLASS_TYPE_XA_GROUP_USER.value){ + html = action == "create" ? 'User <b>'+name+'</b> added to group <b>'+ parentObjectName + '</b>' + : 'User <b>'+name+'</b> removed from group <b>'+ parentObjectName + '</b>'; + } return html; } } diff --git a/security-admin/src/test/java/org/apache/ranger/service/TestXPermMapService.java b/security-admin/src/test/java/org/apache/ranger/service/TestXPermMapService.java index b07223663..9e861eeae 100644 --- a/security-admin/src/test/java/org/apache/ranger/service/TestXPermMapService.java +++ b/security-admin/src/test/java/org/apache/ranger/service/TestXPermMapService.java @@ -141,10 +141,11 @@ public class TestXPermMapService { @Test public void test4GetTransactionLog() { VXPermMap vObj = createVXPermMap(); + VXPermMap vObj2 = createVXPermMap2(); Mockito.when(daoManager.getXXGroup()).thenReturn(xXGroupDao); XXGroup xGroup = createXXGroup(); Mockito.when(xXGroupDao.getById(1L)).thenReturn(xGroup); - xPermMapService.createTransactionLog(vObj, vObj, OPERATION_UPDATE_CONTEXT); + xPermMapService.createTransactionLog(vObj, vObj2, OPERATION_UPDATE_CONTEXT); } private VXPermMap createVXPermMap() { @@ -171,6 +172,30 @@ public class TestXPermMapService { return vXPermMap; } + private VXPermMap createVXPermMap2() { + VXPermMap vXPermMap = new VXPermMap(); + Date date = new Date(); + vXPermMap.setCreateDate(date); + vXPermMap.setGrantOrRevoke(false); + vXPermMap.setGroupId(1L); + vXPermMap.setGroupName("testGroupName1"); + vXPermMap.setId(1L); + vXPermMap.setIpAddress("123.45.678.91"); + vXPermMap.setIsRecursive(0); + vXPermMap.setIsWildCard(false); + vXPermMap.setMObj(gjObj); + vXPermMap.setOwner("admin"); + vXPermMap.setPermFor(0); + vXPermMap.setPermGroup(""); + vXPermMap.setPermType(0); + vXPermMap.setResourceId(1L); + vXPermMap.setUpdateDate(date); + vXPermMap.setUpdatedBy("admin"); + vXPermMap.setUserId(1L); + vXPermMap.setUserName("testUser"); + return vXPermMap; + } + public XXGroup createXXGroup() { XXGroup xGroup = new XXGroup(); xGroup.setAddedByUserId(1L);