Yair Zaslavsky has uploaded a new change for review.
Change subject: aaa: Fix group name issues for legacy provider
......................................................................
aaa: Fix group name issues for legacy provider
Chagning format from: authz/a/b/c/d to: a/b/c/d@authz
to conform to the users format.
Removed specific code at rest-api that handled old format.
Change-Id: I8c1feed08e33156c794b230fa04f33f2ebd82240
Topic: AAA
Signed-off-by: Yair Zaslavsky <yzasl...@redhat.com>
---
M
backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/kerberosldap/KerberosLdapAuthz.java
M
backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/kerberosldap/LdapBrokerUtils.java
M
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/aaa/BackendGroupsResource.java
M
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/aaa/BackendUsersResource.java
M
backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/aaa/BackendGroupsResourceTest.java
M
backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/aaa/BackendUsersResourceTest.java
A
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/utils/aaa/AuthzUtils.java
7 files changed, 125 insertions(+), 58 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/79/32779/1
diff --git
a/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/kerberosldap/KerberosLdapAuthz.java
b/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/kerberosldap/KerberosLdapAuthz.java
index ea9cfa0..dae3df2 100644
---
a/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/kerberosldap/KerberosLdapAuthz.java
+++
b/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/kerberosldap/KerberosLdapAuthz.java
@@ -272,13 +272,13 @@
user.getSurName()
).mput(
Authz.PrincipalRecord.NAME,
- removeUpnSuffix(user.getUserName())
+ formatValue(user.getUserName(),
Authz.PrincipalRecord.NAME)
).mput(
Authz.PrincipalRecord.TITLE,
user.getTitle()
).mput(
Authz.PrincipalRecord.PRINCIPAL,
- removeUpnSuffix(user.getUserName())
+ formatValue(user.getUserName(),
Authz.PrincipalRecord.PRINCIPAL)
);
if (user.getGroups() != null) {
List<ExtMap> groups = new ArrayList<>();
@@ -327,7 +327,7 @@
String.format(
"(%1$s%2$s%3$s)", keysToAttributes.get(key),
operatorsToChars.get(queryFilterRecord.get(QueryFilterRecord.OPERATOR)),
-
removeUpnSuffix(queryFilterRecord.get(key).toString())
+ formatValue(queryFilterRecord.get(key).toString(),
key)
)
);
} else {
@@ -346,8 +346,14 @@
return query;
}
- private String removeUpnSuffix(String value) {
- return value.contains("@") ? value.substring(0, value.indexOf("@")) :
value;
+ private String formatValue(String value, ExtKey key) {
+ String result = value;
+ if (key.equals(Authz.PrincipalRecord.NAME) ||
key.equals(Authz.PrincipalRecord.PRINCIPAL)) {
+ result = value.contains("@") ? value.substring(0,
value.indexOf("@")) : value;
+ } else if (key.equals(Authz.GroupRecord.NAME)) {
+ result = value.contains("/") ?
value.substring(value.lastIndexOf('/') + 1) : value;
+ }
+ return result;
}
}
diff --git
a/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/kerberosldap/LdapBrokerUtils.java
b/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/kerberosldap/LdapBrokerUtils.java
index eec7e24..ea7e32b 100644
---
a/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/kerberosldap/LdapBrokerUtils.java
+++
b/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/kerberosldap/LdapBrokerUtils.java
@@ -44,19 +44,25 @@
}
StringBuilder sb = new StringBuilder();
+ StringBuilder domainName = new StringBuilder();
List<Rdn> rdns = name.getRdns();
for (Rdn rdn : rdns) {
String type = rdn.getType();
String val = (String) rdn.getValue();
if (type.equalsIgnoreCase("dc")) {
- sb.insert(0, "." + val);
+ if (domainName.length() > 0) {
+ domainName.insert(0, ".");
+ }
+ domainName.insert(0, val);
continue;
+ } else {
+ sb.append("/" + val);
}
- sb.append("/" + val);
}
// remove the first "." character.
sb.delete(0, 1);
+ sb.append("@").append(domainName);
return sb.toString();
}
diff --git
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/aaa/BackendGroupsResource.java
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/aaa/BackendGroupsResource.java
index d524491..cacf81e 100644
---
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/aaa/BackendGroupsResource.java
+++
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/aaa/BackendGroupsResource.java
@@ -1,6 +1,7 @@
package org.ovirt.engine.api.restapi.resource.aaa;
import java.text.MessageFormat;
+import java.util.Collection;
import java.util.List;
import javax.ws.rs.core.Response;
@@ -17,6 +18,7 @@
import org.ovirt.engine.api.restapi.resource.ResourceConstants;
import org.ovirt.engine.api.restapi.resource.SingleEntityResource;
import org.ovirt.engine.api.restapi.utils.DirectoryEntryIdUtils;
+import org.ovirt.engine.api.restapi.utils.aaa.AuthzUtils;
import org.ovirt.engine.core.aaa.DirectoryGroup;
import org.ovirt.engine.core.common.action.AddGroupParameters;
import org.ovirt.engine.core.common.action.IdParameters;
@@ -64,18 +66,16 @@
* model directly, or it can be embedded in the name.
*
* @param group the model of the group
+ * @param authzProvidersNames
+ * list of existing authz provider names, including the
returned provider name, if exists in the list
* @return the name of the directory or {@code null} if the group can't be
determined
*/
- private String getDirectoryName(Group group) {
+ private String getAuthzProviderName(Group group, Collection<String>
authzProvidersNames) {
if (group.isSetDomain() && group.getDomain().isSetName()) {
return group.getDomain().getName();
}
else if (group.isSetDomain() && group.getDomain().isSetId()) {
- List<String> domains = getBackendCollection(
- String.class,
- VdcQueryType.GetDomainList,
- new VdcQueryParametersBase());
- for (String domain :domains) {
+ for (String domain : authzProvidersNames) {
Guid domainId = new Guid(domain.getBytes(), true);
if (domainId.toString().equals(group.getDomain().getId())) {
return domain;
@@ -84,13 +84,9 @@
throw new WebFaultException(
null,
"Domain: '" + group.getDomain().getId().toString() + "' does
not exist.",
- Response.Status.BAD_REQUEST
- );
+ Response.Status.BAD_REQUEST);
}
- else if (group.isSetName() && group.getName().contains("/")) {
- return group.getName().substring(0, group.getName().indexOf("/"));
- }
- return null;
+ return AuthzUtils.getAuthzNameFromEntityName(group.getName(),
authzProvidersNames);
}
/**
@@ -150,11 +146,15 @@
@Override
public Response add(Group group) {
+ List<String> authzProvidersNames = getBackendCollection(
+ String.class,
+ VdcQueryType.GetDomainList,
+ new VdcQueryParametersBase());
validateParameters(group, "name");
- if (!isNameContainsDomain(group)) {
+ if (AuthzUtils.getAuthzNameFromEntityName(group.getName(),
authzProvidersNames) == null) {
validateParameters(group, "domain.id|name");
}
- String directoryName = getDirectoryName(group);
+ String directoryName = getAuthzProviderName(group,
authzProvidersNames);
DirectoryGroup directoryGroup = findDirectoryGroup(directoryName,
group);
if (directoryGroup == null) {
return Response.status(Status.BAD_REQUEST)
@@ -182,16 +182,11 @@
} else if (groupModel.isSetDomainEntryId()) {
return getGroupById(directoryName, namespace,
groupModel.getDomainEntryId());
} else if (groupModel.isSetName()) {
- String groupName = groupModel.getName();
- if (groupName.startsWith(directoryName + "/")) {
- int lastSlash = groupName.lastIndexOf("/");
- groupName = groupName.substring(lastSlash + 1);
- }
return getEntity(
- DirectoryGroup.class,
- SearchType.DirectoryGroup,
- getDirectoryGroupSearchPattern(groupName, directoryName)
- );
+ DirectoryGroup.class,
+ SearchType.DirectoryGroup,
+
getDirectoryGroupSearchPattern(AuthzUtils.getEntityNameWithoutAuthz(groupModel.getName(),
directoryName), directoryName)
+ );
}
return null;
@@ -210,11 +205,6 @@
@Override
public Response performRemove(String id) {
return performAction(VdcActionType.RemoveGroup, new
IdParameters(asGuid(id)));
- }
-
- private boolean isNameContainsDomain(Group group) {
- String name = group.getName();
- return name.contains("/") && name.indexOf('/') != name.length() - 1;
}
}
diff --git
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/aaa/BackendUsersResource.java
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/aaa/BackendUsersResource.java
index fe0a8dd..c3d738e 100644
---
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/aaa/BackendUsersResource.java
+++
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/aaa/BackendUsersResource.java
@@ -4,6 +4,7 @@
import java.nio.charset.Charset;
import java.text.MessageFormat;
+import java.util.Collection;
import java.util.List;
import javax.ws.rs.core.Response;
@@ -19,6 +20,7 @@
import org.ovirt.engine.api.restapi.resource.AbstractBackendCollectionResource;
import org.ovirt.engine.api.restapi.resource.ResourceConstants;
import org.ovirt.engine.api.restapi.resource.SingleEntityResource;
+import org.ovirt.engine.api.restapi.utils.aaa.AuthzUtils;
import org.ovirt.engine.api.restapi.utils.DirectoryEntryIdUtils;
import org.ovirt.engine.core.aaa.DirectoryUser;
import org.ovirt.engine.core.common.action.AddUserParameters;
@@ -76,16 +78,12 @@
user_defined_pattern + AND_SEARCH_PATTERN +
USERS_SEARCH_PATTERN;
}
- protected String getDomain(User user) {
+ protected String getAuthzProviderName(User user, Collection<String>
authzProvidersNames) {
if (user.isSetDomain() && user.getDomain().isSetName()) {
return user.getDomain().getName();
}
else if (user.isSetDomain() && user.getDomain().isSetId()) {
- List<String> domains = getBackendCollection(
- String.class,
- VdcQueryType.GetDomainList,
- new VdcQueryParametersBase());
- for (String domain :domains) {
+ for (String domain : authzProvidersNames) {
Guid domainId =
asGuid(domain.getBytes(Charset.forName("UTF-8")), true);
if (domainId.toString().equals(user.getDomain().getId())) {
return domain;
@@ -93,10 +91,7 @@
}
throw new WebFaultException(null, "Domain: '" +
user.getDomain().getId().toString() + "' does not exist.",
Response.Status.BAD_REQUEST);
}
- else if (isNameContainsDomain(user)) {
- return
user.getUserName().substring(user.getUserName().lastIndexOf("@") + 1);
- }
- return null;
+ return AuthzUtils.getAuthzNameFromEntityName(user.getUserName(),
authzProvidersNames);
}
/**
@@ -174,10 +169,14 @@
@Override
public Response add(User user) {
validateParameters(user, "userName");
- if (!isNameContainsDomain(user)) {// user-name may contain the domain
(e.g: ol...@xxx.yyy)
+ List<String> authzProvidersNames = getBackendCollection(
+ String.class,
+ VdcQueryType.GetDomainList,
+ new VdcQueryParametersBase());
+ if (AuthzUtils.getAuthzNameFromEntityName(user.getUserName(),
authzProvidersNames) == null) {// user-name may contain the domain (e.g:
ol...@xxx.yyy)
validateParameters(user, "domain.id|name");
}
- String domain = getDomain(user);
+ String domain = getAuthzProviderName(user, authzProvidersNames);
DirectoryUser directoryUser = findDirectoryUser(domain, user);
if (directoryUser == null) {
return Response.status(Status.BAD_REQUEST)
@@ -187,11 +186,6 @@
AddUserParameters parameters = new AddUserParameters(new
DbUser(directoryUser));
QueryIdResolver<Guid> resolver = new
QueryIdResolver<>(VdcQueryType.GetDbUserByUserId, IdQueryParameters.class);
return performCreate(VdcActionType.AddUser, parameters, resolver,
BaseResource.class);
- }
-
- private boolean isNameContainsDomain(User user) {
- return ((user.getUserName().contains("@")) &&
(user.getUserName().lastIndexOf('@') != user.getUserName()
- .length() - 1));
}
/**
@@ -217,7 +211,7 @@
DirectoryUser.class,
SearchType.DirectoryUser,
getDirectoryUserSearchPattern(
- isNameContainsDomain(user) ?
user.getUserName().substring(0, user.getUserName().lastIndexOf("@")) :
user.getUserName(),
+
AuthzUtils.getEntityNameWithoutAuthz(user.getUserName(), directoryName),
user.getNamespace(),
directoryName)
);
diff --git
a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/aaa/BackendGroupsResourceTest.java
b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/aaa/BackendGroupsResourceTest.java
index 7999513..6435f10 100644
---
a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/aaa/BackendGroupsResourceTest.java
+++
b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/aaa/BackendGroupsResourceTest.java
@@ -1,6 +1,7 @@
package org.ovirt.engine.api.restapi.resource.aaa;
import java.nio.charset.Charset;
+import java.util.LinkedList;
import java.util.List;
import javax.ws.rs.WebApplicationException;
@@ -21,6 +22,7 @@
import org.ovirt.engine.core.common.interfaces.SearchType;
import org.ovirt.engine.core.common.queries.DirectoryIdQueryParameters;
import org.ovirt.engine.core.common.queries.IdQueryParameters;
+import org.ovirt.engine.core.common.queries.VdcQueryParametersBase;
import org.ovirt.engine.core.common.queries.VdcQueryType;
import org.ovirt.engine.core.compat.Guid;
@@ -45,11 +47,14 @@
* searches, thus then must have the same format that we generate when
searching in directories.
*/
private static final String[] GROUP_NAMES;
+ private static final String[] GROUP_NAMES_WITH_NO_DOMAIN;
static {
GROUP_NAMES = new String[NAMES.length];
+ GROUP_NAMES_WITH_NO_DOMAIN = new String[NAMES.length];
for (int i = 0; i < NAMES.length; i++) {
- GROUP_NAMES[i] = DOMAIN + "/Groups/" + NAMES[i];
+ GROUP_NAMES_WITH_NO_DOMAIN[i] = "Groups/" + NAMES[i];
+ GROUP_NAMES[i] = GROUP_NAMES_WITH_NO_DOMAIN[i] + "@" + DOMAIN;
}
}
@@ -204,8 +209,13 @@
@Test
public void testAddGroupWithExplicitDirectoryName() throws Exception {
setUriInfo(setUpBasicUriExpectations());
+ setUpEntityQueryExpectations(VdcQueryType.GetDomainList,
+ VdcQueryParametersBase.class,
+ new String[] {},
+ new Object[] {},
+ setUpDomains());
setUpGetEntityExpectations(
- "ADGROUP@" + DOMAIN + ":: name=" + NAMES[0],
+ "ADGROUP@" + DOMAIN + ":: name=" +
GROUP_NAMES_WITH_NO_DOMAIN[0],
SearchType.DirectoryGroup,
getDirectoryGroup(0)
);
@@ -227,13 +237,20 @@
Domain domain = new Domain();
domain.setName(DOMAIN);
Group model = new Group();
- model.setName(NAMES[0]);
+ model.setName(GROUP_NAMES_WITH_NO_DOMAIN[0]);
model.setDomain(domain);
Response response = collection.add(model);
assertEquals(201, response.getStatus());
assertTrue(response.getEntity() instanceof Group);
verifyModel((Group) response.getEntity(), 0);
+ }
+
+ private List<String> setUpDomains() {
+ List<String> domains = new LinkedList<>();
+ domains.add("some.domain");
+ domains.add(DOMAIN);
+ return domains;
}
/**
@@ -243,8 +260,13 @@
@Test
public void testAddGroupWithImplicitDirectoryName() throws Exception {
setUriInfo(setUpBasicUriExpectations());
+ setUpEntityQueryExpectations(VdcQueryType.GetDomainList,
+ VdcQueryParametersBase.class,
+ new String[] {},
+ new Object[] {},
+ setUpDomains());
setUpGetEntityExpectations(
- "ADGROUP@" + DOMAIN + ":: name=" + NAMES[0],
+ "ADGROUP@" + DOMAIN + ":: name=" +
GROUP_NAMES_WITH_NO_DOMAIN[0],
SearchType.DirectoryGroup,
getDirectoryGroup(0)
);
@@ -278,10 +300,15 @@
@Test
public void testAddGroupWithoutDirectoryName() throws Exception {
setUriInfo(setUpBasicUriExpectations());
+ setUpEntityQueryExpectations(VdcQueryType.GetDomainList,
+ VdcQueryParametersBase.class,
+ new String[] {},
+ new Object[] {},
+ setUpDomains());
control.replay();
Group model = new Group();
- model.setName(NAMES[0]);
+ model.setName(GROUP_NAMES_WITH_NO_DOMAIN[0]);
try {
collection.add(model);
@@ -299,6 +326,11 @@
@Test
public void testAddGroupById() throws Exception {
setUriInfo(setUpBasicUriExpectations());
+ setUpEntityQueryExpectations(VdcQueryType.GetDomainList,
+ VdcQueryParametersBase.class,
+ new String[] {},
+ new Object[] {},
+ setUpDomains());
setUpGetEntityExpectations(
VdcQueryType.GetDirectoryGroupById,
DirectoryIdQueryParameters.class,
@@ -338,6 +370,11 @@
@Test
public void testAddGroupByIdFailure() throws Exception {
setUriInfo(setUpBasicUriExpectations());
+ setUpEntityQueryExpectations(VdcQueryType.GetDomainList,
+ VdcQueryParametersBase.class,
+ new String[] {},
+ new Object[] {},
+ setUpDomains());
setUpGetEntityExpectations(
VdcQueryType.GetDirectoryGroupById,
DirectoryIdQueryParameters.class,
diff --git
a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/aaa/BackendUsersResourceTest.java
b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/aaa/BackendUsersResourceTest.java
index baa8a15..de6633c 100644
---
a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/aaa/BackendUsersResourceTest.java
+++
b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/aaa/BackendUsersResourceTest.java
@@ -129,6 +129,11 @@
@Test
public void testAddUser_2() throws Exception {
+ setUpEntityQueryExpectations(VdcQueryType.GetDomainList,
+ VdcQueryParametersBase.class,
+ new String[] {},
+ new Object[] {},
+ setUpDomains());
setUpAddUserExpectations("ADUSER@" + DOMAIN + ":: username=" +
NAMES[0]);
User model = new User();
Domain domain = new Domain();
@@ -141,6 +146,11 @@
@Test
public void testAddUser_3() throws Exception {
+ setUpEntityQueryExpectations(VdcQueryType.GetDomainList,
+ VdcQueryParametersBase.class,
+ new String[] {},
+ new Object[] {},
+ setUpDomains());
setUpAddUserExpectations("ADUSER@" + DOMAIN + ":: username=" +
NAMES[0]);
User model = new User();
model.setUserName(NAMES[0] + "@" + DOMAIN);
diff --git
a/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/utils/aaa/AuthzUtils.java
b/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/utils/aaa/AuthzUtils.java
new file mode 100644
index 0000000..4b0c03e
--- /dev/null
+++
b/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/utils/aaa/AuthzUtils.java
@@ -0,0 +1,24 @@
+package org.ovirt.engine.api.restapi.utils.aaa;
+
+import java.util.Collection;
+
+public class AuthzUtils {
+
+ public static String getAuthzNameFromEntityName(String entityName,
Collection<String> authzProvidersNames) {
+ String result = null;
+ if (entityName != null && entityName.contains("@")) {
+ String lastPart = entityName.substring(entityName.lastIndexOf('@')
+ 1);
+ result = authzProvidersNames.contains(lastPart) ? lastPart : null;
+ }
+ return result;
+ }
+
+
+ public static String getEntityNameWithoutAuthz(String entityName, String
authzProviderName) {
+ String result = entityName;
+ if (entityName != null && entityName.contains("@") &&
entityName.lastIndexOf('@') < entityName.length() -1) {
+ result =
entityName.substring(entityName.lastIndexOf('@')+1).equals(authzProviderName) ?
entityName.substring(0, entityName.lastIndexOf('@')) : entityName;
+ }
+ return result;
+ }
+}
--
To view, visit http://gerrit.ovirt.org/32779
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8c1feed08e33156c794b230fa04f33f2ebd82240
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.5
Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
