You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Daniel Gergely <dg...@hortonworks.com> on 2016/03/08 14:36:43 UTC
Re: Review Request 44265: Basic Operational Audit Logging


> On márc. 7, 2016, 10:51 de, Sebastian Toader wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/api/services/BaseService.java, line 61
> > <https://reviews.apache.org/r/44265/diff/1/?file=1276699#file1276699line61>
> >
> >     I think this shouldn't be static but rather use guice to ensure that this object is a singleton.

This class is not guice controlled, nor its descendants are. The derived classes are created by jetty, so I did not find better way to access audit logger from BaseService.


> On márc. 7, 2016, 10:51 de, Sebastian Toader wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/api/services/LogoutService.java, line 44
> > <https://reviews.apache.org/r/44265/diff/1/?file=1276700#file1276700line44>
> >
> >     I think this shouldn't be static but rather use guice to ensure that this object is a singleton.

This class is not guice controlled, jetty initiate the object from it, I did not find better way to access audit logger from LogoutService.


> On márc. 7, 2016, 10:51 de, Sebastian Toader wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/DefaultEventCreator.java, line 65
> > <https://reviews.apache.org/r/44265/diff/1/?file=1276772#file1276772line65>
> >
> >     Usually its safer to use empty set than null.

At this point I want to express that "it is not specified" and not "it is empty". For me "null" shows that this is some special value, since I want to express "it is not restricted on resource types" and not "it is restricted to no rescource type"


> On márc. 7, 2016, 10:51 de, Sebastian Toader wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/GroupEventCreator.java, line 43
> > <https://reviews.apache.org/r/44265/diff/1/?file=1276773#file1276773line43>
> >
> >     Why is this not instantiated using DI similar to DefaultEventCreator?

I removed @Singleton from DefaultEventCreator, since it has single binding in guice. (It is not singleton because of the annotation, but because of guice)


- Daniel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44265/#review122279
-----------------------------------------------------------


On márc. 3, 2016, 12:32 du, Daniel Gergely wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44265/
> -----------------------------------------------------------
> 
> (Updated márc. 3, 2016, 12:32 du)
> 
> 
> Review request for Ambari, Laszlo Puskas, Oliver Szabo, Robert Levas, Sandor Magyari, and Sebastian Toader.
> 
> 
> Bugs: AMBARI-15241
>     https://issues.apache.org/jira/browse/AMBARI-15241
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Entry points for logging events:
> - Login and logout: AmbariAuthenticationFilter, AmbariAuthorizationFilter, LogoutService
> - Operation and tasks: ActionDBAAccessorImpl
> - Kerberos related events: CreateKeytabFilesServerAction, CreatePrincipalServerAction, DestroyPrincipalsServerAction, FinalizeKerberosServerAction
> - REST api: BaseService
> 
> Architecture:
> AuditLogger injectable is created and wired in by Guice. The default implementation (AuditLoggerDefaultImpl) for this interface logs to a file. Considering performance impact, there is a buffered audit logger implementation (BufferedAuditLogger) that is a wrapper for an audit logger implementation and does the logging asynchronously.
> 
> Audit loggers have a single log method that accepts an AuditEvent object. At the entry points an AuditEvent is assembled and log method is called (except for the REST api, see below...)
> 
> REST Api:
> In BaseService, there is a RequestAuditLogger, that is responsible for handling and assembling AuditEvents. It also has a log method, but the parameters for that is the Request and Result objects.
> RequestAuditLogger is a small framework for that plugins can be implemented to handle different type of requests.
> Plugins implements RequestAuditEventCreator interface and can be set to handle one or multiple request types (PUT, POST, DELETE, etc...), resource types (HOST_COMPONENT, BLUEPRINT, etc..) and results statuses (200 OK, 202 ACCEPTED, 404 NOT_FOUND, etc). If null is returned by these getters, it means "all". If there are more than one RequestAuditEventCreators can handle a request, then the most specific is called. (Priority order: resourceType > resultStatus > requestType)
> For example it is possible to define a plugin for all the POST requests, but if there is an other plugin for POST request and for resource type HOST_COMPONENT, then this latter is used (because it is more specific). The method createAuditEvent is responsible for creating the AuditEvent object for the RequestAuditEventLogger. If null is returned, then request is not logged (can be used for ignoring events).
> Plugins are registered in ControllerModule and wired by guice to the RequestAuditLogger. If a new plugin is needed, then implementing the RequestAuditEventCreator interface and registering it in Guice is the only things to do. (open-closed principle)
> 
> 
> Diffs
> -----
> 
>   ambari-project/pom.xml ed94004 
>   ambari-server/conf/unix/log4j.properties 2ee32d4 
>   ambari-server/conf/windows/log4j.properties cc40f74 
>   ambari-server/pom.xml e3409b9 
>   ambari-server/src/main/conf/log4j.properties 8e6652d 
>   ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java 23686c3 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/BaseRequest.java a33e8a7 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/BaseService.java 7945599 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/LogoutService.java df8faaa 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/Request.java ff9b6c7 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/AuditLogger.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/AuditLoggerDefaultImpl.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/AuditLoggerModule.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/BufferedAuditLogger.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/AbstractAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/AbstractUserAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/AccessUnauthorizedAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/AuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/LoginAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/LogoutAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/OperationStatusAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/TaskStatusAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/kerberos/AbstractKerberosAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/kerberos/ChangeSecurityStateKerberosAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/kerberos/CreateKeyTabKerberosAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/kerberos/CreatePrincipalKerberosAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/kerberos/DestroyPrincipalKerberosAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/RequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/RequestAuditEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/RequestAuditLogger.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/RequestAuditLoggerImpl.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/ActivateUserRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/AddAlertGroupRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/AddAlertTargetRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/AddBlueprintRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/AddComponentToHostRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/AddCredentialRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/AddHostRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/AddRepositoryRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/AddRepositoryVersionRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/AddRequestRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/AddUpgradeRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/AddUserToGroupRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/AddViewInstanceRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/AdminUserRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/BlueprintExportRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/ChangeAlertGroupRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/ChangeAlertTargetRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/ChangeRepositoryVersionRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/ChangeViewInstanceRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/ClientConfigDownloadRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/ClusterNameChangeRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/ClusterPrivilegeChangeRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/ConfigurationChangeRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/CreateGroupRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/CreateUserRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/DeleteAlertGroupRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/DeleteAlertTargetRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/DeleteBlueprintRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/DeleteGroupRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/DeleteHostRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/DeleteRepositoryVersionRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/DeleteServiceRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/DeleteUserRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/DeleteViewInstanceRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/MembershipChangeRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/PrivilegeChangeRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/RemoveUserFromGroupRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/StartOperationRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/UpdateRepositoryRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/UpdateUpgradeItemRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/UserPasswordChangeRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/event/ViewPrivilegeChangeRequestAuditEvent.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/AlertGroupEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/AlertTargetEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/BlueprintEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/BlueprintExportEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/ComponentEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/ConfigurationChangeEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/CredentialEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/DefaultEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/GroupEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/HostEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/MemberEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/PrivilegeEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/RecommendationIgnoreEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/RepositoryEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/RepositoryVersionEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/RequestEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/ServiceConfigDownloadEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/ServiceEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/UnauthorizedEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/UpgradeEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/UpgradeItemEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/UserEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/ValidationIgnoreEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/ViewInstanceEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/eventcreator/ViewPrivilegeEventCreator.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java 882adb2 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 4695990 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java daca64d 
>   ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationFilter.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java e2a28d0 
>   ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AuthorizationHelper.java b136182 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/AbstractServerAction.java 33191bf 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreateKeytabFilesServerAction.java cadfe28 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreatePrincipalsServerAction.java 8009ae1 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/DestroyPrincipalsServerAction.java 93daae8 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/FinalizeKerberosServerAction.java c710b8e 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosServerAction.java 90d9414 
>   ambari-server/src/main/java/org/apache/ambari/server/utils/RequestUtils.java PRE-CREATION 
>   ambari-server/src/main/resources/webapp/WEB-INF/spring-security.xml 3bbc785 
>   ambari-server/src/test/java/org/apache/ambari/server/api/services/BaseServiceTest.java 19eeffb 
>   ambari-server/src/test/java/org/apache/ambari/server/audit/AccessUnauthorizedAuditEventTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/audit/BufferedAuditLoggerTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/audit/LoginAuditEventTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/audit/LogoutAuditEventTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/audit/OperationStatusAuditEventTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/audit/StartOperationRequestAuditEventTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/audit/request/AbstractBaseCreator.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/audit/request/AllGetCreator.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/audit/request/AllPostAndPutCreator.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/audit/request/DefaultEventCreatorTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/audit/request/PutHostComponentCreator.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/audit/request/RequestAuditLogModule.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/audit/request/RequestAuditLoggerTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/checks/UpgradeCheckOrderTest.java d4ff566 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java 498fddf 
>   ambari-server/src/test/java/org/apache/ambari/server/notifications/DispatchFactoryTest.java 6834d5c 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/InMemoryDefaultTestModule.java 3ecfe14 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/JdbcPropertyTest.java c07382b 
>   ambari-server/src/test/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationFilterTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java b30bff3 
>   ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLdapAuthenticationProviderForDNWithSpaceTest.java da8d9bc 
>   ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLdapAuthenticationProviderTest.java d48be85 
>   ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLocalUserDetailsServiceTest.java c410f5b 
>   ambari-server/src/test/java/org/apache/ambari/server/security/authorization/LdapServerPropertiesTest.java 0797239 
>   ambari-server/src/test/java/org/apache/ambari/server/utils/RequestUtilsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44265/diff/
> 
> 
> Testing
> -------
> 
> Unit tests are added to cover functionality. 
> 
> All tests passed on local machine.
> 
> 
> Thanks,
> 
> Daniel Gergely
> 
>