Francesco Romani has posted comments on this change. Change subject: core, engine: servlet to support the console proxy ......................................................................
Patch Set 27: (4 comments) https://gerrit.ovirt.org/#/c/35887/27/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetAllVmsForAnotherUserQuery.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetAllVmsForAnotherUserQuery.java: Line 19: protected void executeQueryCommand() { Line 20: List<VM> vmsList = getDbFacade().getVmDao().getAllForUser(getParameters().getId()); Line 21: Line 22: getQueryReturnValue().setReturnValue(vmsList); Line 23: } > can you explain where and how this is used, the implication of this is that Yes, I realized this is wrong for this exact reason you outlined. The purpose of this query is to make the VMConsoleProxyServlet able to answer the query "to which consoles can this user connect?" In a nutshell, the flow would be something like: - the user connect to the proxy, using ssh - the proxy, once the user was authenticated, asks Engine for a list of consoles to which the said user can connect - either the user provided a console id, then this id is checked to be present in the said list, or the user select interactively any of the consoles in that list This query will be used to implement the console_list hook documented here: http://git.engineering.redhat.com/git/users/abarlev/ovirt-vmconsole.git/tree/README.API I will doublecheck the above to make sure my memory serves us well, but that's should be the gist. https://gerrit.ovirt.org/#/c/35887/27/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java: Line 8: GetVmByVmId(VdcQueryAuthType.User), Line 9: GetVmByVmNameForDataCenter(VdcQueryAuthType.User), Line 10: GetAllVms(VdcQueryAuthType.User), Line 11: GetAllVmsForUser(VdcQueryAuthType.User), Line 12: GetAllVmsForAnotherUser(VdcQueryAuthType.User), > this doesnt look right.. Will review the flow described in another comment to make sure the perimissions are considered. Line 13: GetUnregisteredVms, Line 14: GetUnregisteredVmTemplates, Line 15: GetVmsRunningOnOrMigratingToVds, Line 16: GetVmsByStorageDomain, Line 177: GetDbUserByUserNameAndDomain(VdcQueryAuthType.User), Line 178: GetUserBySessionId(VdcQueryAuthType.User), Line 179: GetEngineSessionIdToken(VdcQueryAuthType.User), Line 180: GetUserProfile(VdcQueryAuthType.User), Line 181: GetAllUserProfiles(VdcQueryAuthType.User), > also questionable noted, same as above as my answer Line 182: Line 183: // Directory queries: Line 184: GetDirectoryUserById(VdcQueryAuthType.User), Line 185: GetDirectoryGroupById(VdcQueryAuthType.User), https://gerrit.ovirt.org/#/c/35887/27/backend/manager/modules/services/src/main/java/org/ovirt/engine/core/services/VMConsoleProxyServlet.java File backend/manager/modules/services/src/main/java/org/ovirt/engine/core/services/VMConsoleProxyServlet.java: Line 130: StringBuilder buffer = new StringBuilder(); Line 131: Line 132: int r; Line 133: while ((r = body.read()) != -1) { Line 134: buffer.append((char) r); > wouldnt it be easier to read lines and get it as strings already? This was the original implementation, but I was asked explicitely to read byte by byte, hence this change. Line 135: } Line 136: Line 137: body.close(); Line 138: -- To view, visit https://gerrit.ovirt.org/35887 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I53c721da21cefcf4069d14c7016b6f7d97f9eac9 Gerrit-PatchSet: 27 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vitor de Lima <vdel...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: Shahar Havivi <shav...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <wallaroo1...@gmail.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
