Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
bostonaqua opened this issue Apr 9, 2024 · 2 comments
Open
Labels

Comments

@bostonaqua
Copy link

bostonaqua commented Apr 9, 2024

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

@bostonaqua bostonaqua added the bug label Apr 9, 2024
@dsrowell
Copy link

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
Copy link
Member

timja commented Apr 26, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants