Alon Bar-Lev has posted comments on this change. Change subject: aaa: add support for basic athentication ......................................................................
Patch Set 3: (7 comments) ok, you confused me... why did you add the jboss security realm usage? http://gerrit.ovirt.org/#/c/37299/3/backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/SSOServletBase.java File backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/SSOServletBase.java: Line 14: protected boolean enableBasicAuth; Line 15: Line 16: @Override Line 17: public void init() throws ServletException { Line 18: String strVal = getServletConfig().getInitParameter("enable-basic-auth"); we can have this in application context instead of the base, so there is no need for base. Line 19: if (StringUtils.isNotEmpty(strVal)) { Line 20: enableBasicAuth = Boolean.parseBoolean(strVal); Line 21: } Line 22: } Line 22: } Line 23: Line 24: protected boolean containsUserCredentialsInParams(Map<String, String[]> paramMap) { Line 25: return paramMap.containsKey(USERNAME) && paramMap.containsKey(PASSWORD) && paramMap.containsKey(PROFILE); Line 26: } this can be a utility not a base class method. http://gerrit.ovirt.org/#/c/37299/3/backend/manager/modules/enginesso/src/main/resources/roles.properties File backend/manager/modules/enginesso/src/main/resources/roles.properties: Line 1: admin@internal=ovirtuser why is this required? we do not use jboss features. http://gerrit.ovirt.org/#/c/37299/3/backend/manager/modules/enginesso/src/main/resources/users.properties File backend/manager/modules/enginesso/src/main/resources/users.properties: Line 1: admin@internal=1 same http://gerrit.ovirt.org/#/c/37299/3/backend/manager/modules/enginesso/src/main/webapp/WEB-INF/jboss-web.xml File backend/manager/modules/enginesso/src/main/webapp/WEB-INF/jboss-web.xml: Line 1: <jboss-web> Line 2: <security-domain>java:/jaas/ovirtBasic</security-domain> Line 3: </jboss-web> same http://gerrit.ovirt.org/#/c/37299/3/backend/manager/modules/enginesso/src/main/webapp/WEB-INF/web.xml File backend/manager/modules/enginesso/src/main/webapp/WEB-INF/web.xml: Line 173: </auth-constraint> Line 174: <user-data-constraint> Line 175: <transport-guarantee>NONE</transport-guarantee> Line 176: </user-data-constraint> Line 177: </security-constraint> nothing within our code should use any special feature of jboss, just like we do not need it in current implementation. Line 178: Line 179: <login-config> Line 180: <auth-method>BASIC</auth-method> Line 181: <realm-name>ovirtBasic</realm-name> http://gerrit.ovirt.org/#/c/37299/3/packaging/services/ovirt-engine/ovirt-engine.xml.in File packaging/services/ovirt-engine/ovirt-engine.xml.in: Line 310: <module-option name="usersProperties" value="users.properties"/> Line 311: <module-option name="rolesProperties" value="roles.properties"/> Line 312: </login-module> Line 313: </authentication> Line 314: </security-domain> same, we do not need this. Line 315: </security-domains> Line 316: </subsystem> Line 317: Line 318: <subsystem xmlns="urn:jboss:domain:transactions:1.1"> -- To view, visit http://gerrit.ovirt.org/37299 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If09285f0e6cd8909f21aa7e88ae1a3c1a30763c2 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Ravi Nori <rn...@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
