Alon Bar-Lev has posted comments on this change. Change subject: aaa: Adding resolve groups ......................................................................
Patch Set 3: (11 comments) http://gerrit.ovirt.org/#/c/28368/3/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthzUtils.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthzUtils.java: Line 304: } Line 305: return directoryGroup; Line 306: } Line 307: Line 308: private static int queryFlagValue(boolean resolveGroups, boolean resolveGroupsRecursive) { I think this is insane.... but... Line 309: int result = 0; Line 310: if (resolveGroups) { Line 311: result |= Authz.QueryFlags.RESOLVE_GROUPS; Line 312: } http://gerrit.ovirt.org/#/c/28368/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/aaa/DirectoryUtils.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/aaa/DirectoryUtils.java: Line 19: StringBuilder sb = new StringBuilder(); Line 20: Set<DirectoryGroup> groupsSet = new HashSet<DirectoryGroup>(); Line 21: flatGroups(groupsSet, directoryUser.getGroups()); Line 22: List<DirectoryGroup> groupsList = new ArrayList<DirectoryGroup>(groupsSet); Line 23: Collections.sort(groupsList, new Comparator<DirectoryGroup>() { where do you use this groupsList? Line 24: @Override Line 25: public int compare(DirectoryGroup o1, DirectoryGroup o2) { Line 26: return o1.getId().compareTo(o2.getId()) != 0 ? o1.getId().compareTo(o2.getId()) : o1.getDirectoryName().compareTo(o2.getDirectoryName()); Line 27: } http://gerrit.ovirt.org/#/c/28368/3/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/kerberosldap/KerberosLdapAuthz.java File backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/kerberosldap/KerberosLdapAuthz.java: Line 116: new LdapSearchByQueryParameters(configuration, Line 117: null, Line 118: getDirectoryName(), Line 119: queryData, Line 120: input.<Integer> get(Authz.InvokeKeys.QUERY_FLAGS, Authz.QueryFlags.RESOLVE_GROUPS_NONE) != Authz.QueryFlags.RESOLVE_GROUPS_NONE no no no ... you should be ready for new flags. comparing to 0 means nothing. as the RESOLVE_GROUPS must be set even if recursive then you just need to check this flags in your case. Line 121: ) Line 122: ); Line 123: List<LdapUser> ldapUsers = (List<LdapUser>) ldapResult.getReturnValue(); Line 124: List<ExtMap> results = new ArrayList<>(); http://gerrit.ovirt.org/#/c/28368/3/backend/manager/modules/extensions-api-root/extensions-api/src/main/java/org/ovirt/engine/api/extensions/aaa/Authz.java File backend/manager/modules/extensions-api-root/extensions-api/src/main/java/org/ovirt/engine/api/extensions/aaa/Authz.java: Line 40: */ Line 41: public static final ExtKey PAGE_SIZE = new ExtKey("AAA_AUTHZ_PAGE_SIZE", Integer.class, "03197cd2-2d0f-4636-bd88-f65c4a543efe"); Line 42: /** Line 43: * Resolve groups. Line 44: * Resolve groups information. @see QueryFlags Line 45: * */ Line 46: public static final ExtKey QUERY_FLAGS = new ExtKey("AAA_AUTHZ_QUERY_FLAGS", Integer.class, "97d226e9-8d87-49a0-9a7f-af689320907b"); Line 47: /** Principal record. */ Line 48: public static final ExtKey PRINCIPAL_RECORD = new ExtKey("AAA_AUTHZ_PRINCIPAL_RECORD", ExtMap.class, "ebc0d5ca-f1ea-402c-86ae-a8ecbdadd6b5"); Line 87: Line 88: /** Line 89: * No groups resolving. Line 90: */ Line 91: public static final int RESOLVE_GROUPS_NONE = 0; you do not need none flag Line 92: /** Line 93: * Resolve groups recursively. Line 94: */ Line 95: public static final int RESOLVE_GROUPS_RECURSIVE = 1; Line 89: * No groups resolving. Line 90: */ Line 91: public static final int RESOLVE_GROUPS_NONE = 0; Line 92: /** Line 93: * Resolve groups recursively. please switch the two... and describe: /** Resolve groups. */ public static final int RESOLVE_GROUPS = (1<<0); /** Resolve groups recursive when resolving groups. */ public static final int RESOLVE_GROUPS_RECURSIVE = (1<<1); BTW: similar example is the Authn.Capabilities Line 94: */ Line 95: public static final int RESOLVE_GROUPS_RECURSIVE = 1; Line 96: /** Line 97: * Resolve only groups the entry is a direct member of. Line 91: public static final int RESOLVE_GROUPS_NONE = 0; Line 92: /** Line 93: * Resolve groups recursively. Line 94: */ Line 95: public static final int RESOLVE_GROUPS_RECURSIVE = 1; this should be 1<<0 Line 96: /** Line 97: * Resolve only groups the entry is a direct member of. Line 98: */ Line 99: public static final int RESOLVE_GROUPS = 2; Line 94: */ Line 95: public static final int RESOLVE_GROUPS_RECURSIVE = 1; Line 96: /** Line 97: * Resolve only groups the entry is a direct member of. Line 98: */ you can drop the long comments... /** xxxx */ Line 99: public static final int RESOLVE_GROUPS = 2; Line 100: } Line 101: Line 102: /** Line 95: public static final int RESOLVE_GROUPS_RECURSIVE = 1; Line 96: /** Line 97: * Resolve only groups the entry is a direct member of. Line 98: */ Line 99: public static final int RESOLVE_GROUPS = 2; this should be 1<<1 Line 100: } Line 101: Line 102: /** Line 103: * Invoke commands. Line 134: * <li>{@link InvokeKeys#NAMESPACE}[M]</li> Line 135: * <li>{@link InvokeKeys#QUERY_ENTITY}[M]</li> Line 136: * <li>{@link InvokeKeys#QUERY_FILTER}[M]</li> Line 137: * <li>{@link InvokeKeys#RESOLVE_GROUPS_RECURSIVE}[M] - resolve groups recursively.</li> Line 138: * <li>{@link InvokeKeys#RESOLVE_GROUPS}[M] - resolve groups infomration.</li> these are flags now Line 139: * </ul> Line 140: * </p> Line 141: * Line 142: * <p> Line 182: */ Line 183: public static final ExtUUID QUERY_CLOSE = new ExtUUID("AAA_AUTHZ_QUERY_CLOSE", "3e049bc0-055e-4789-a4e3-0ef51bfe6685"); Line 184: } Line 185: Line 186: - Line 187: /** Line 188: * Principal record. Line 189: */ Line 190: public static class PrincipalRecord { -- To view, visit http://gerrit.ovirt.org/28368 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3249b7f18c8bf609c9577e60aafa948a0aa55101 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
