Alon Bar-Lev has posted comments on this change. Change subject: aaa: Introducing usage of Acct ......................................................................
Patch Set 13: (8 comments) http://gerrit.ovirt.org/#/c/27070/13/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/BasicAuthenticationFilter.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/BasicAuthenticationFilter.java: Line 138: AcctUtils.reportRecords( Line 139: Acct.ReportReason.PRINCIPAL_LOGIN_FAILED, Line 140: null, Line 141: null, Line 142: userProfile.userName, hmmm.... just noticed we are missing USER_NAME within the invoke keys... this should go into USER_NAME and not PRINCIPAL. Line 143: String.format("Basic authentication failed for User %1$s.", userProfile.userName) Line 144: ); Line 145: } Line 146: if (outputMap.<Integer> get(Authn.InvokeKeys.RESULT) != Authn.AuthResult.SUCCESS) { Line 139: Acct.ReportReason.PRINCIPAL_LOGIN_FAILED, Line 140: null, Line 141: null, Line 142: userProfile.userName, Line 143: String.format("Basic authentication failed for User %1$s.", userProfile.userName) consider prototype of String... args and call String.format within the function. Line 144: ); Line 145: } Line 146: if (outputMap.<Integer> get(Authn.InvokeKeys.RESULT) != Authn.AuthResult.SUCCESS) { Line 147: AcctUtils.reportRecords( http://gerrit.ovirt.org/#/c/27070/13/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/FiltersHelper.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/FiltersHelper.java: Line 13: public static class Constants { Line 14: public final static String REQUEST_AUTH_RECORD_KEY = "ovirt_aaa_auth_record"; Line 15: public final static String REQUEST_SCHEMES_KEY = "ovirt_aaa_schemes"; Line 16: public final static String REQUEST_PROFILE_KEY = "ovirt_aaa_profile"; Line 17: public final static String REQUEST_AUTH_TYPE_KEY = "ovirt_aaa_request_auth_type"; no need for _request... :) Line 18: } Line 19: Line 20: public static BackendLocal getBackend(Context context) { Line 21: http://gerrit.ovirt.org/#/c/27070/13/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/LoginFilter.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/LoginFilter.java: Line 49: LoginUserParameters( Line 50: profileName, Line 51: authRecord, Line 52: loginAsAdmin ? VdcActionType.LoginAdminUser : VdcActionType.LoginUser, Line 53: (Integer)request.getAttribute(FiltersHelper.Constants.REQUEST_AUTH_TYPE_KEY) how can it be integer if it is enum? Line 54: ) Line 55: ); Line 56: if (returnValue.getSucceeded()) { Line 57: HttpSession session = req.getSession(true); http://gerrit.ovirt.org/#/c/27070/13/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginBaseCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginBaseCommand.java: Line 182: log.errorFormat( Line 183: "Can't login user because no login name has been provided." Line 184: ); Line 185: addCanDoActionMessage(VdcBllMessages.USER_FAILED_TO_AUTHENTICATE); Line 186: AcctUtils.reportRecords(Acct.ReportReason.PRINCIPAL_USER_INVALID, null, null, null, "Invalid principal user was passed. Credentials based authentication has failed"); decide... principal or user :))) user is what user enters principal is what authn returns Line 187: return false; Line 188: } Line 189: String password = getParameters().getPassword(); Line 190: if (password == null) { Line 282: boolean isAdmin = MultiLevelAdministrationHandler.isAdminUser(dbUser); Line 283: log.debugFormat("Checking if user {0} is an admin, result {1}", dbUser.getLoginName(), isAdmin); Line 284: dbUser.setAdmin(isAdmin); Line 285: setCurrentUser(dbUser); Line 286: AcctUtils.reportRecords(reportReason, stupid note... I suggest you get used to put first parameter at new line as well, just like the others :) Line 287: authRecord, Line 288: principalRecord, Line 289: loginName, Line 290: String.format("The user %1$s has performed successful authentication", principalRecord.<String>get(Authz.PrincipalRecord.NAME)) http://gerrit.ovirt.org/#/c/27070/13/backend/manager/modules/extensions-api-root/extensions-api/src/main/java/org/ovirt/engine/api/extensions/aaa/Acct.java File backend/manager/modules/extensions-api-root/extensions-api/src/main/java/org/ovirt/engine/api/extensions/aaa/Acct.java: Line 119: public static final int PRINCIPAL_CREDENTIALS_INVALID = 12; Line 120: /** Line 121: * Principal user is invalid. Line 122: */ Line 123: public static final int PRINCIPAL_USER_INVALID = 13; USER_INVALID and move this before PRINCIPAL_INVALID and add USER to the invoke keys to put when you know user and there is no principal. Line 124: Line 125: /** Line 126: * Access denied. Line 127: */ http://gerrit.ovirt.org/#/c/27070/13/frontend/webadmin/modules/userportal-gwtp/src/main/webapp/WEB-INF/web.xml File frontend/webadmin/modules/userportal-gwtp/src/main/webapp/WEB-INF/web.xml: Line 63 Line 64 Line 65 Line 66 Line 67 not part of this patch -- To view, visit http://gerrit.ovirt.org/27070 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ief13d233d11b7ab32b328735b4f58ec7cffff567 Gerrit-PatchSet: 13 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
