Yair Zaslavsky has posted comments on this change. Change subject: core : Engine IDP ......................................................................
Patch Set 1: (6 comments) http://gerrit.ovirt.org/#/c/34194/1/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/EnginePrincipal.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/EnginePrincipal.java: Line 8: private static final long serialVersionUID = 7701951188631723261L; Line 9: private final String name; Line 10: private String engineSessionId; Line 11: Line 12: public EnginePrincipal(String name) { Can an object of EnginePrincipal exist with a sessionId? if not, can you add this to the ctor? Line 13: this.name = name; Line 14: } Line 15: Line 16: @Override http://gerrit.ovirt.org/#/c/34194/1/backend/manager/modules/idp/src/main/java/org/ovirt/engine/core/idp/EngineAttributeHandler.java File backend/manager/modules/idp/src/main/java/org/ovirt/engine/core/idp/EngineAttributeHandler.java: Line 32: if (getType() == HANDLER_TYPE.SP) Line 33: return; Line 34: Line 35: HTTPContext httpContext = (HTTPContext) request.getContext(); Line 36: HttpSession session = httpContext.getRequest().getSession(false); Can a session here return null? Line 37: Line 38: Principal userPrincipal = (Principal) session.getAttribute(GeneralConstants.PRINCIPAL_ID); Line 39: Line 40: if (userPrincipal == null) http://gerrit.ovirt.org/#/c/34194/1/backend/manager/modules/idp/src/main/java/org/ovirt/engine/core/idp/EngineLoginModule.java File backend/manager/modules/idp/src/main/java/org/ovirt/engine/core/idp/EngineLoginModule.java: Line 202: String profileName = (String) authResult.get(FiltersHelper.Constants.REQUEST_PROFILE_KEY); Line 203: Line 204: ExtMap authRecord = (ExtMap) authResult.get(FiltersHelper.Constants.REQUEST_AUTH_RECORD_KEY); Line 205: if (authRecord != null) { Line 206: InitialContext context = new InitialContext(); Double try because of new InitialContext? IMHO Mike Kolesnik added some utility to have an Autoclosable wrapper - a. I suggest to move it to uutils b. You can use it here. Line 207: try { Line 208: VdcLoginReturnValueBase returnValue = (VdcLoginReturnValueBase) FiltersHelper.getBackend(context).login(new Line 209: LoginUserParameters( Line 210: profileName, http://gerrit.ovirt.org/#/c/34194/1/backend/manager/modules/idp/src/main/webapp/WEB-INF/login-error.jsp File backend/manager/modules/idp/src/main/webapp/WEB-INF/login-error.jsp: Line 28: Line 29: <div class="col-sm-12"> Line 30: <div style="width:250px;"> Line 31: <form id="login_form" name="login_form" method="post" Line 32: action="j_security_check" enctype="application/x-www-form-urlencoded"> Lots of trailing whitespaces, please fix. Line 33: <center> Line 34: <p> Line 35: Welcome to the <b> Identity Provider</b> Line 36: </p> http://gerrit.ovirt.org/#/c/34194/1/backend/manager/modules/idp/src/main/webapp/WEB-INF/login.jsp File backend/manager/modules/idp/src/main/webapp/WEB-INF/login.jsp: Line 28: Line 29: <div class="col-sm-12"> Line 30: <div style="width:250px;"> Line 31: <form id="login_form" name="login_form" method="post" Line 32: action="j_security_check" enctype="application/x-www-form-urlencoded"> same. Line 33: <center> Line 34: <p> Line 35: Welcome to the <b> Identity Provider</b> Line 36: </p> http://gerrit.ovirt.org/#/c/34194/1/backend/manager/modules/idpfilters/src/main/java/org/ovirt/engine/core/idp/filters/EngineIDPLoginFilter.java File backend/manager/modules/idpfilters/src/main/java/org/ovirt/engine/core/idp/filters/EngineIDPLoginFilter.java: Line 73: Line 74: private boolean runQuery(String sessionID) { Line 75: boolean returnValue = false; Line 76: Line 77: BackendInternal backend = (BackendInternal) EjbUtils.findBean(BeanType.BACKEND, BeanProxyType.LOCAL); Didn't you add some FilterUtils.getBackend? Line 78: VdcQueryParametersBase params = new VdcQueryParametersBase(); Line 79: params.setSessionId(sessionID); Line 80: Line 81: VdcQueryReturnValue queryReturnValue = backend.runInternalQuery(VdcQueryType.ValidateSession, params); -- To view, visit http://gerrit.ovirt.org/34194 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib549b3b563081a37b1fae3f437a73a208dd86db5 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@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
