> 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 > >
