Skip to content

[JENKINS-19413] Use CredentialsDescriptor.findContextInPath rather than directly StaplerRequest2.findAncestorObject #651

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.

Sign up for GitHub

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

Merged
jtnord merged 1 commit into from
Sep 12, 2025
Merged

[JENKINS-19413] Use CredentialsDescriptor.findContextInPath rather than directly StaplerRequest2.findAncestorObject #651

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
188 changes: 94 additions & 94 deletions src/main/java/com/cloudbees/plugins/credentials/CredentialsDescriptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,111 +337,111 @@
o = ((CredentialsStoreAction.CredentialsWrapper) o).getStore().getContext();
} else if (o instanceof CredentialsStoreAction.DomainWrapper) {
o = ((CredentialsStoreAction.DomainWrapper) o).getStore().getContext();
} else if (o instanceof Descriptor && i == 1) { // URL is /descriptorByName/...
// TODO this is a https://issues.jenkins-ci.org/browse/JENKINS-19413 workaround

// we need to try an infer from the Referer as this is likely a doCheck or a doFill method
String referer = request.getReferer();
String rootPath = request.getRootPath();
if (referer != null && rootPath != null && referer.startsWith(rootPath)) {
// strip out any query portion of the referer URL.
String path = URI.create(referer.substring(rootPath.length())).getPath().substring(1);

// TODO have Stapler expose a method that can walk a path and produce the ancestors and use that

// what now follows is an example of a really evil hack, consequently this means...
//
// 7.. ,
// MMM. MMM.
// MMMMM .MMMMMM
// MMMM. MMMMM.
// OMMM MMZ
// MMM MM
// .MMMM $. . .MM,
// MMMMM MMM MM MMM
// .MMMMM. MMMMD 8MMM. MMMM
// MMMMMMM.MMMM MMMMM. MMMMMM
// MMMMMMMM.M . MMM. MMMMMMM
// MMMMMMMMM. MMMMMMMM
// MMMMMMMMM . .. MMMMMMMMMMM
// MMMMMMMM IMMMM Z.MMMMMMMMMM ,
// .MMMMMMM .M:M MMMMMMMMMMM M
// I MMMMMMM. MMMMMMMMMO M
// MMMM MMMMMMM .MMMMMMMMM. .
// :MMMMMM.MMMMMMM. MMMMMMMM .MMMM
// MMMMMMMMMMMMMMMM MMMMMMM MMMMMMMM
// MMMMMMMMM.MMMMMMM MMMMMMM MMMMMMMMMM.
// MMMMMMMMMMMMMMMM? MMMMMM MMMMMMMMMMM
// MMMMMMMMMM . . MMMMMIMMMMMMMMMMMM.
// MMMMMMMMMM .. :MMMMMMMMMMMM.
// DMMMMMMMMMM MMMMMMMMMMMMM.
// MMMMMMMMMM.M. MMMMMMMMMMMM.
// MMMMMM, ....
//
// I AM A SAD PANDA

List<String> pathSegments = new ArrayList<>(Arrays.asList(StringUtils.split(path, "/")));
// strip out any leading junk
while (!pathSegments.isEmpty() && StringUtils.isBlank(pathSegments.get(0))) {
pathSegments.remove(0);
}
if (type.isInstance(o) && o instanceof ModelObject && CredentialsProvider.hasStores((ModelObject) o)) {

Check warning on line 341 in src/main/java/com/cloudbees/plugins/credentials/CredentialsDescriptor.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 341 is only partially covered, one branch is missing
return type.cast(o);
}
}
// TODO this is a https://issues.jenkins-ci.org/browse/JENKINS-19413 workaround

Check warning on line 345 in src/main/java/com/cloudbees/plugins/credentials/CredentialsDescriptor.java

View check run for this annotation

ci.jenkins.io / Open Tasks Scanner

TODO 

NORMAL:
this is a https://issues.jenkins-ci.org/browse/JENKINS-19413 workaround
// we need to try an infer from the Referer as this is likely a doCheck or a doFill method or select.jelly from a lazy-load fragment
String referer = request.getReferer();
String rootPath = request.getRootPath();
if (referer != null && rootPath != null && referer.startsWith(rootPath)) {
// strip out any query portion of the referer URL.
String path = URI.create(referer.substring(rootPath.length())).getPath().substring(1);

// TODO have Stapler expose a method that can walk a path and produce the ancestors and use that

Check warning on line 353 in src/main/java/com/cloudbees/plugins/credentials/CredentialsDescriptor.java

View check run for this annotation

ci.jenkins.io / Open Tasks Scanner

TODO 

NORMAL:
have Stapler expose a method that can walk a path and produce the ancestors and use that

// what now follows is an example of a really evil hack, consequently this means...
//
// 7.. ,
// MMM. MMM.
// MMMMM .MMMMMM
// MMMM. MMMMM.
// OMMM MMZ
// MMM MM
// .MMMM $. . .MM,
// MMMMM MMM MM MMM
// .MMMMM. MMMMD 8MMM. MMMM
// MMMMMMM.MMMM MMMMM. MMMMMM
// MMMMMMMM.M . MMM. MMMMMMM
// MMMMMMMMM. MMMMMMMM
// MMMMMMMMM . .. MMMMMMMMMMM
// MMMMMMMM IMMMM Z.MMMMMMMMMM ,
// .MMMMMMM .M:M MMMMMMMMMMM M
// I MMMMMMM. MMMMMMMMMO M
// MMMM MMMMMMM .MMMMMMMMM. .
// :MMMMMM.MMMMMMM. MMMMMMMM .MMMM
// MMMMMMMMMMMMMMMM MMMMMMM MMMMMMMM
// MMMMMMMMM.MMMMMMM MMMMMMM MMMMMMMMMM.
// MMMMMMMMMMMMMMMM? MMMMMM MMMMMMMMMMM
// MMMMMMMMMM . . MMMMMIMMMMMMMMMMMM.
// MMMMMMMMMM .. :MMMMMMMMMMMM.
// DMMMMMMMMMM MMMMMMMMMMMMM.
// MMMMMMMMMM.M. MMMMMMMMMMMM.
// MMMMMM, ....
//
// I AM A SAD PANDA

List<String> pathSegments = new ArrayList<>(Arrays.asList(StringUtils.split(path, "/")));
// strip out any leading junk
while (!pathSegments.isEmpty() && StringUtils.isBlank(pathSegments.get(0))) {
pathSegments.remove(0);
}
if (pathSegments.size() >= 2) {
String firstSegment = pathSegments.get(0);
if ("user".equals(firstSegment)) {
User user = User.getById(pathSegments.get(1), true);
if (type.isInstance(user) && CredentialsProvider.hasStores(user)) {
// we have a winner
return type.cast(user);
}
if (pathSegments.size() >= 2) {
String firstSegment = pathSegments.get(0);
if ("user".equals(firstSegment)) {
User user = User.getById(pathSegments.get(1), true);
if (type.isInstance(user) && CredentialsProvider.hasStores(user)) {
// we have a winner
return type.cast(user);
} else if ("job".equals(firstSegment) || "item".equals(firstSegment) || "view"
.equals(firstSegment)) {
int index = 0;
while (index < pathSegments.size()) {
String segment = pathSegments.get(index);
if ("view".equals(segment)) {
// remove the /view/
pathSegments.remove(index);
if (index < pathSegments.size()) {
// remove the /view/{name}
pathSegments.remove(index);
}
} else if ("job".equals(firstSegment) || "item".equals(firstSegment) || "view"
.equals(firstSegment)) {
int index = 0;
} else if ("job".equals(segment) || "item".equals(segment)) {
// remove the /job/
pathSegments.remove(index);
// skip the name
index++;
} else {
// we have gone as far as we can parse the item path structure
while (index < pathSegments.size()) {
String segment = pathSegments.get(index);
if ("view".equals(segment)) {
// remove the /view/
pathSegments.remove(index);
if (index < pathSegments.size()) {
// remove the /view/{name}
pathSegments.remove(index);
}
} else if ("job".equals(segment) || "item".equals(segment)) {
// remove the /job/
pathSegments.remove(index);
// skip the name
index++;
} else {
// we have gone as far as we can parse the item path structure
while (index < pathSegments.size()) {
// remove the remainder
pathSegments.remove(index);
}
}
// remove the remainder
pathSegments.remove(index);
}
Jenkins jenkins = Jenkins.get();
while (!pathSegments.isEmpty()) {
String fullName = StringUtils.join(pathSegments, "/");
Item item = jenkins.getItemByFullName(fullName);
if (item != null) {
if (type.isInstance(item) && CredentialsProvider.hasStores(item)) {
// we have a winner
return type.cast(item);
}
}
// walk back up and try one level less deep
pathSegments.remove(pathSegments.size() - 1);
}
}
Jenkins jenkins = Jenkins.get();
while (!pathSegments.isEmpty()) {
String fullName = StringUtils.join(pathSegments, "/");
Item item = jenkins.getItemByFullName(fullName);
if (item != null) {
if (type.isInstance(item) && CredentialsProvider.hasStores(item)) {
// we have a winner
return type.cast(item);
}
}
// walk back up and try one level less deep
pathSegments.remove(pathSegments.size() - 1);
}
// ok we give up, we are not thirsty for more, we'll let "normal" ancestor in path logic continue
}
}
if (type.isInstance(o) && o instanceof ModelObject && CredentialsProvider.hasStores((ModelObject) o)) {
return type.cast(o);
}
// ok we give up, we are not thirsty for more
}
if (type.isAssignableFrom(Jenkins.class)) {
return type.cast(Jenkins.get());

Check warning on line 442 in src/main/java/com/cloudbees/plugins/credentials/CredentialsDescriptor.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines 

Lines 441-442 are not covered by tests
}
return null;

}

/**
Expand Down
20 changes: 3 additions & 17 deletions src/main/java/com/cloudbees/plugins/credentials/CredentialsSelectHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@
import org.kohsuke.args4j.Argument;
import org.kohsuke.args4j.CmdLineException;
import org.kohsuke.args4j.Localizable;
import org.kohsuke.stapler.Stapler;
import org.kohsuke.stapler.StaplerRequest2;
import org.kohsuke.stapler.StaplerResponse2;
import org.kohsuke.stapler.interceptor.RequirePOST;
Expand Down Expand Up @@ -119,14 +118,7 @@
@CheckForNull
@Restricted(NoExternalUse.class)
public ModelObject resolveContext(Object context) {
if (context instanceof ModelObject) {
return (ModelObject) context;
}
StaplerRequest2 request = Stapler.getCurrentRequest2();
if (request != null) {
return request.findAncestorObject(ModelObject.class);
}
return null;
Comment on lines -125 to -129
Copy link
Member

@jtnord jtnord Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing code had a check that we where in a Stapler request. if that was needed this will now throw a NullPointerException, did you check that calls to this are only web methods, or that null is never passed for any non web methods?

Copy link
Member Author

@jglick jglick Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#651 (comment) again

return context instanceof ModelObject mo ? mo : CredentialsDescriptor.findContextInPath(ModelObject.class);
}

/**
Expand All @@ -142,71 +134,65 @@
Set<String> urls = new HashSet<>();
List<StoreItem> result = new ArrayList<>();
if (context == null) {
StaplerRequest2 request = Stapler.getCurrentRequest2();
if (request != null) {
context = request.findAncestorObject(ModelObject.class);
}
context = CredentialsDescriptor.findContextInPath(ModelObject.class);
Copy link
Member

@jtnord jtnord Sep 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing code had a check that we are in a Stapler request. if that was needed this will now throw a NullPointerException, did you check that calls to this are only web methods?

Copy link
Member Author

@jglick jglick Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can double check but the other overload of this method passed in StaplerRequest2 already without checking. (It would make no sense outside of a request context.)

Copy link
Member Author

@jglick jglick Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CredentialsSelectHelper methods touched here are used from

credentials-plugin/src/main/resources/lib/credentials/select.jelly

Line 67 in f8963c3

<j:set var="context" value="${selectHelper.resolveContext(attrs.context ?: it)}"/>

credentials-plugin/src/main/resources/lib/credentials/select.jelly

Lines 115 to 117 in f8963c3

<j:set var="storeItems" value="${selectHelper.getStoreItems(context, includeUser)}"/>
<j:choose>
<j:when test="${selectHelper.hasCreatePermission(context, includeUser) and storeItems != null and !storeItems.isEmpty()}">

Whereas

credentials-plugin/src/main/java/com/cloudbees/plugins/credentials/CredentialsDescriptor.java

Line 328 in f8963c3

public static <T extends ModelObject> T findContextInPath(@NonNull StaplerRequest2 request, @NonNull Class<T> type) {
is used from

credentials-plugin/src/main/java/com/cloudbees/plugins/credentials/ContextInPath.java

Line 44 in f8963c3

return CredentialsDescriptor.findContextInPath(request, type);

}
if (context != null) {
for (CredentialsStore store : CredentialsProvider.lookupStores(context)) {
StoreItem item = new StoreItem(store);
String url = item.getUrl();
if (item.getUrl() != null && !urls.contains(url)) {
result.add(item);
urls.add(url);
}
}
}
if (includeUser) {
boolean hasPermission = false;
ModelObject current = context;
while (current != null) {
if (current instanceof AccessControlled) {
hasPermission = ((AccessControlled) current).hasPermission(CredentialsProvider.USE_OWN);
break;
} else if (current instanceof ComputerSet) {
current = Jenkins.get();
} else {
// fall back to Jenkins as the ultimate parent of everything else
current = Jenkins.get();
}
}
if (hasPermission) {
for (CredentialsStore store : CredentialsProvider.lookupStores(User.current())) {
StoreItem item = new StoreItem(store);
String url = item.getUrl();
if (item.getUrl() != null && !urls.contains(url)) {
result.add(item);
urls.add(url);
}
}
}
}
return result;
}

/**
* Checks if the current user has permission to create a credential.
*
* @param context the context.
* @param includeUser whether they can use their own credentials store.
* @return {@code true} if they can create a permission.
* @since FIXME
*/
@Restricted(NoExternalUse.class)
@SuppressWarnings("unused") // used via jelly
public boolean hasCreatePermission(ModelObject context, boolean includeUser) {
if (includeUser) {
User current = User.current();
if (current != null && current.hasPermission(CREATE)) {
return true;
}
}
if (context == null) {
StaplerRequest2 request = Stapler.getCurrentRequest2();
if (request != null) {
context = request.findAncestorObject(ModelObject.class);
}
context = CredentialsDescriptor.findContextInPath(ModelObject.class);

Check warning on line 195 in src/main/java/com/cloudbees/plugins/credentials/CredentialsSelectHelper.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines 

Lines 137-195 are not covered by tests
}
for (CredentialsStore store : CredentialsProvider.lookupStores(context)) {
if (store.hasPermission(CREATE)) {
Expand Down