Omer Frenkel has posted comments on this change. Change subject: core, engine: servlet to support the console proxy ......................................................................
Patch Set 27: (5 comments) https://gerrit.ovirt.org/#/c/35887/27/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetAllUserProfilesQuery.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetAllUserProfilesQuery.java: Line 10: super(parameters); Line 11: } Line 12: Line 13: @Override Line 14: protected void executeQueryCommand() { since this query return information from the backend without any restrictions, i would like it to be blocked for non-internal calls. and since we don't have nice infrastructure for this, please return value only if (isInternalExecution()) please add a little documentation why we are doing it Line 15: getQueryReturnValue().setReturnValue( Line 16: DbFacade.getInstance().getUserProfileDao().getAll()); Line 17: } 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: } > I reviewed the flow, and indeed this query should run only if an user succe i think currently using the isInternal hack is ok, documented 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), > Will review the flow described in another comment to make sure the perimiss now im pretty sure this should not be user query, also the one below Line 13: GetUnregisteredVms, Line 14: GetUnregisteredVmTemplates, Line 15: GetVmsRunningOnOrMigratingToVds, Line 16: GetVmsByStorageDomain, 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 67: jsonUser.put("entity", "ssh-key"); Line 68: if (user != null) { Line 69: jsonUser.put("username", user.getLoginName()); Line 70: } Line 71: jsonUser.put("key", profile.getSshPublicKey()); afair. this can contain multiple ssh key, its ok to return like this, or need entry per-key? Line 72: Line 73: jsonUsers.add(jsonUser); Line 74: } Line 75: } Line 157: private Map<String, Object> produceContentFromParameters(Map<String, String> parameters) { Line 158: String command = parameters.get("command"); Line 159: String version = parameters.get("version"); Line 160: Line 161: BackendInternal backend = (BackendInternal) EjbUtils.findBean(BeanType.BACKEND, BeanProxyType.LOCAL); i think its possible to inject the backend instead of lookup also make it a class member and use it in all methods instead of passing it as parameter Line 162: Line 163: Map<String, Object> result = null; Line 164: Line 165: if ("1".equals(version)) { -- 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
