Wrong group name validation if name provided with spaces [source code]

[bostonaqua]

Jenkins and plugins versions report

Environment

azure-ad:442.v355cca_6b_c169
role-strategy:689.v731678c3e0eb_

What Operating System are you using (both controller, and any agents involved in the problem)?

Jenkins deployed with official helm chart on Kubernetes cluster

Reproduction steps

Reproducing logic of this function via Jenkins Script Console:

import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import com.microsoft.graph.options.QueryOption;

def groupName = 'Group Name With Spaces';  // group that exists in EntraID
def encodedGroupName = URLEncoder.encode(groupName, StandardCharsets.UTF_8.name()); 

def query = String.format("displayName eq '%s'", encodedGroupName);
def queryNoEncode = String.format("displayName eq '%s'", groupName);
println("Query with enc: $query\nQuery without enc: $queryNoEncode");

def requestOptions = new LinkedList<>();
def requestOptionsNoEncode = new LinkedList<>();
requestOptions.add(new QueryOption('$filter', query));
        requestOptionsNoEncode.add(new QueryOption('$filter', queryNoEncode));

def secRealm = Jenkins.get().getSecurityRealm();

groupCollectionPage = secRealm.getAzureClient().groups()
         .buildRequest(requestOptions)
         .select("id,displayName")
         .get();
println groupCollectionPage.getCurrentPage().size();  // 0. group will be null.

groupCollectionPageNoEncode = secRealm.getAzureClient().groups()
         .buildRequest(requestOptionsNoEncode)
         .select("id,displayName")
         .get();
println groupCollectionPageNoEncode.getCurrentPage().size();  // 1
println groupCollectionPageNoEncode.getCurrentPage().get(0).displayName;  // Group Name With Spaces

So this validation wont work with groups names which contain spaces.

Expected Results

validateGroup function validates group with spaces in its name

Actual Results

validateGroup function does not validate group with spaces in its name

Anything else?

As a solution need to replace all '+' to '%20' after encoding (did not test it, maybe need to replace it back to space ' ') I think encoding may affect other special characters

Are you interested in contributing a fix?

No response

[dsrowell]

Actually I think just encoding the group name is the bug. The implicitly-invoked GroupCollectionRequest inside getAzureClient().group().buildRequest().select().get() is encoding the entire URL, so what you end up with is "Group%2B%Name%2BWith%2BSpaces" in the actual URL. If you switch out '+' with '%20', then you'd end up with "Group%2520Name%2520With%2520Spaces", also not what you want. The solution, I believe, is to leave the group name alone (i.e. not attempt to encode it) and let the request encode it instead. But I sense a Chesterton's fence here and I'm reluctant to remove it without knowing why it's there in the first place.

[timja]

check the git blame on that line it was added for a reason.

[stefansli]

@timja I checked the git blame and I can't find an explanation, why encoding is necessary. Since you did the original pull request, can you please enlighten us?

Regarding the original bug, I can reproduce it. Encoding the group name does not yield any results, not encoding yields the expected result.

[timja]

Looking at the git blame and linked issues I think it was for groups with a # at the start, see https://github.com/jenkinsci/azure-ad-plugin/issues/145#issuecomment-866736490