Ravi Nori has posted comments on this change.

Change subject: aaa: Add support for rest api Basic auth
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.ovirt.org/#/c/37786/2/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/SSORestApiLoginFilter.java
File 
backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/SSORestApiLoginFilter.java:

Line 96:             try (ByteArrayOutputStream os = new 
ByteArrayOutputStream()) {
Line 97:                 try (InputStream input = connection.getInputStream()) {
Line 98:                     copy(input, os);
Line 99:                 }
Line 100:                 connection.connect();
> shouldn't the connect be before the copy?
Copied from ProxyServletBase

http://gerrit.ovirt.org/#/c/33479/31/backend/manager/modules/uutils/src/main/java/org/ovirt/engine/core/uutils/servlet/ProxyServletBase.java

If it needs fixing, we need to fix both places
Line 101:                 createUserSession(req, new String(os.toByteArray(), 
"UTF-8"));
Line 102:             }
Line 103:         } catch (Exception e) {
Line 104:             log.error("Exception obtaining ticket from sso", 
e.getMessage());


http://gerrit.ovirt.org/#/c/37786/2/backend/manager/modules/restapi/webapp/src/main/webapp/WEB-INF/jboss-deployment-structure.xml
File 
backend/manager/modules/restapi/webapp/src/main/webapp/WEB-INF/jboss-deployment-structure.xml:

Line 34:       <module name="org.ovirt.engine.api.restapi-definition" 
annotations="true" services="import"/>
Line 35:       <module name="org.ovirt.engine.api.restapi-jaxrs" 
annotations="true" services="import"/>
Line 36:       <module name="org.ovirt.engine.api.restapi-types" 
annotations="true" services="import"/>
Line 37:       <module name="org.ovirt.engine.core.aaa"/>
Line 38:       <module name="org.ovirt.engine.api.ovirt-engine-extensions-api"/>
> still unsure why this is needed.
If I don't have this, deserializing the payload throws exception stating ExtMap 
was not found
Line 39: 
Line 40:     </dependencies>
Line 41:   </deployment>
Line 42: 


-- 
To view, visit http://gerrit.ovirt.org/37786
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5f6975f2d306a4dc2d81b795ab4905e5d3281a1
Gerrit-PatchSet: 2
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

Reply via email to