Yair Zaslavsky has posted comments on this change. Change subject: aaa: using the new extensions API in InternalDirectory ......................................................................
Patch Set 5: (5 comments) http://gerrit.ovirt.org/#/c/26477/5/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/DirectoryProxy.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/DirectoryProxy.java: > please see previous patch comments. Done Line 1: package org.ovirt.engine.core.aaa; Line 2: Line 3: import java.util.ArrayList; Line 4: import java.util.Arrays; http://gerrit.ovirt.org/#/c/26477/5/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/internal/InternalDirectory.java File backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/internal/InternalDirectory.java: Line 29: Authz.InvokeCommands.QUERY_PRINCIPALS_BY_IDS_OPEN, Line 30: Authz.InvokeCommands.QUERY_GROUPS_BY_IDS_OPEN) Line 31: ); Line 32: Line 33: private static final Set<ExtUUID> OTHER_AUTHZ_COMMANDS = > and there is the conservative... i thought about this one, didn't like it that much. if it bothers u that much, i'll change. Line 34: new HashSet<>(Arrays.asList(Authz.InvokeCommands.QUERY_CLOSE, Authz.InvokeCommands.FETCH_PRINCIPAL_RECORD)); Line 35: private ExtMap context; Line 36: Line 37: private ExtMap adminUser; Line 64: } else { Line 65: output.put(Base.InvokeKeys.COMMAND, Base.InvokeResult.UNSUPPORTED); Line 66: } Line 67: if (QUERY_COMMANDS.contains(command) || OTHER_AUTHZ_COMMANDS.contains(command)) { Line 68: output.putIfAbsent(Authz.InvokeKeys.Status, Authz.Status.SUCCESS); > I think you can drop conditional and always put success in this key. Well, don't you think for init putting Authz.Status.SUCCESS is not that in place? init is not an authorization command, IMHO. Line 69: } Line 70: output.putIfAbsent(Base.InvokeKeys.RESULT, Base.InvokeResult.SUCCESS); Line 71: Line 72: } catch (Exception ex) { Line 89: users = Arrays.asList(adminUser); Line 90: } else { Line 91: users = null; Line 92: } Line 93: output.put(Authz.InvokeKeys.QUERY_RESULT, users); > I still think my previous comment applies... per reduce logic. done. Line 94: } Line 95: } Line 96: Line 97: private void fillOpaque(ExtMap output, ExtUUID queryType) { Line 104: String principal = input.<ExtMap>get(Authn.InvokeKeys.AUTH_RECORD).get(Authn.AuthRecord.PRINCIPAL); Line 105: if (principal.equals(adminUser.<String>get(Authz.PrincipalRecord.NAME))) { Line 106: output.put(Authz.InvokeKeys.PRINCIPAL_RECORD, adminUser); Line 107: } else { Line 108: output.put(Authz.InvokeKeys.Status, Authz.Status.CONFIGURATION_INVALID); > why configuration is invalid? it is like you searching ldap for user that i Done Line 109: } Line 110: } Line 111: Line 112: private void doInit(ExtMap input, ExtMap output) { -- To view, visit http://gerrit.ovirt.org/26477 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I958443292da0455e0a12039fac98eebb9b17dee2 Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@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
