-
Notifications
You must be signed in to change notification settings - Fork 247
[FIXED JENKINS-36315] Make it easier to infer the context in form binding #62
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
Changes from all commits
5d985eb
b0444dd
e16af99
b616485
660aeef
a021baf
7ccf111
e4e6a08
cdcc1c5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| package com.cloudbees.plugins.credentials; | ||
|
|
||
| import hudson.model.ModelObject; | ||
| import java.lang.annotation.Documented; | ||
| import java.lang.annotation.Retention; | ||
| import java.lang.annotation.Target; | ||
| import javax.servlet.ServletException; | ||
| import org.apache.commons.lang.StringUtils; | ||
| import org.kohsuke.stapler.AnnotationHandler; | ||
| import org.kohsuke.stapler.InjectedParameter; | ||
| import org.kohsuke.stapler.StaplerRequest; | ||
|
|
||
| import static java.lang.annotation.ElementType.PARAMETER; | ||
| import static java.lang.annotation.RetentionPolicy.RUNTIME; | ||
|
|
||
| /** | ||
| * Indicates that this parameter is injected by evaluating | ||
| * {@link StaplerRequest#getAncestors()} and searching for a credentials context with the parameter type. | ||
| * You can enhance the lookup by ensuring that there are query parameters of {@code $provider} and {@code $token} | ||
| * that correspond to the context's {@link CredentialsSelectHelper.ContextResolver} FQCN and | ||
| * {@link CredentialsSelectHelper.ContextResolver#getToken(ModelObject)} respectively. | ||
| * | ||
| * @see CredentialsDescriptor#getCheckMethod(String) | ||
| * @since 2.1.5 | ||
| */ | ||
| @Retention(RUNTIME) | ||
| @Target(PARAMETER) | ||
| @Documented | ||
| @InjectedParameter(ContextInPath.HandlerImpl.class) | ||
| public @interface ContextInPath { | ||
| class HandlerImpl extends AnnotationHandler<ContextInPath> { | ||
| public Object parse(StaplerRequest request, ContextInPath contextInPath, Class type, String parameterName) | ||
| throws | ||
| ServletException { | ||
| String $provider = request.getParameter("$provider"); | ||
| String $token = request.getParameter("$token"); | ||
| if (StringUtils.isNotBlank($provider) && StringUtils.isNotBlank($token)) { | ||
| ModelObject context = CredentialsDescriptor.lookupContext($provider, $token); | ||
| if (type.isInstance(context)) { | ||
| return type.cast(context); | ||
| } | ||
| } | ||
| return CredentialsDescriptor.findContextInPath(request, type); | ||
| } | ||
| } | ||
| } | ||
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,27 +24,39 @@ | |
| package com.cloudbees.plugins.credentials.impl; | ||
|
|
||
| import com.cloudbees.plugins.credentials.BaseCredentials; | ||
| import com.cloudbees.plugins.credentials.ContextInPath; | ||
| import com.cloudbees.plugins.credentials.Credentials; | ||
| import com.cloudbees.plugins.credentials.CredentialsDescriptor; | ||
| import com.cloudbees.plugins.credentials.CredentialsMatcher; | ||
| import com.cloudbees.plugins.credentials.CredentialsMatchers; | ||
| import com.cloudbees.plugins.credentials.CredentialsProvider; | ||
| import com.cloudbees.plugins.credentials.CredentialsScope; | ||
| import com.cloudbees.plugins.credentials.CredentialsSelectHelper; | ||
| import com.cloudbees.plugins.credentials.CredentialsStore; | ||
| import com.cloudbees.plugins.credentials.CredentialsStoreAction; | ||
| import com.cloudbees.plugins.credentials.common.IdCredentials; | ||
| import com.cloudbees.plugins.credentials.common.StandardCredentials; | ||
| import com.cloudbees.plugins.credentials.domains.Domain; | ||
| import edu.umd.cs.findbugs.annotations.CheckForNull; | ||
| import edu.umd.cs.findbugs.annotations.NonNull; | ||
| import hudson.ExtensionList; | ||
| import hudson.Util; | ||
| import hudson.model.Item; | ||
| import hudson.model.ModelObject; | ||
| import hudson.model.User; | ||
| import hudson.util.FormValidation; | ||
| import java.io.UnsupportedEncodingException; | ||
| import java.net.URLEncoder; | ||
| import java.util.EnumSet; | ||
| import java.util.Set; | ||
| import jenkins.model.Jenkins; | ||
| import org.kohsuke.stapler.AncestorInPath; | ||
| import org.apache.commons.lang.StringUtils; | ||
| import org.kohsuke.stapler.QueryParameter; | ||
| import org.kohsuke.stapler.export.Exported; | ||
| import org.kohsuke.stapler.export.ExportedBean; | ||
|
|
||
| import static com.cloudbees.plugins.credentials.CredentialsSelectHelper.*; | ||
|
|
||
| /** | ||
| * Base class for {@link StandardCredentials}. | ||
| */ | ||
|
|
@@ -139,17 +151,22 @@ protected BaseStandardCredentialsDescriptor(Class<? extends BaseStandardCredenti | |
|
|
||
| @CheckForNull | ||
| private static FormValidation checkForDuplicates(String value, ModelObject context, ModelObject object) { | ||
| CredentialsMatcher withId = CredentialsMatchers.withId(value); | ||
| for (CredentialsStore store : CredentialsProvider.lookupStores(object)) { | ||
| if (!store.hasPermission(CredentialsProvider.VIEW)) { | ||
| continue; | ||
| } | ||
| ModelObject storeContext = store.getContext(); | ||
| for (Domain domain : store.getDomains()) { | ||
| if (CredentialsMatchers.firstOrNull(store.getCredentials(domain), CredentialsMatchers.withId(value)) | ||
| != null) { | ||
| for (Credentials match : CredentialsMatchers.filter(store.getCredentials(domain), withId)) { | ||
| if (storeContext == context) { | ||
| return FormValidation.error("This ID is already in use"); | ||
| } else { | ||
| CredentialsScope scope = match.getScope(); | ||
| if (scope != null && !scope.isVisible(context)) { | ||
| // scope is not exported to child contexts | ||
| continue; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🐛 Should it continue with other credentials in the domain then? firstOrNull skips others from what I see |
||
| } | ||
| return FormValidation.warning("The ID ‘%s’ is already in use in %s", value, | ||
| storeContext instanceof Item | ||
| ? ((Item) storeContext).getFullDisplayName() | ||
|
|
@@ -161,7 +178,28 @@ private static FormValidation checkForDuplicates(String value, ModelObject conte | |
| return null; | ||
| } | ||
|
|
||
| public final FormValidation doCheckId(@QueryParameter String value, @AncestorInPath ModelObject context) { | ||
| /** | ||
| * Gets the check id url for the specified store. | ||
| * | ||
| * @param store the store. | ||
| * @return the url of the id check endpoint. | ||
| * @throws UnsupportedEncodingException if the JVM does not implement the JLS. | ||
| */ | ||
| public String getCheckIdUrl(CredentialsStore store) throws UnsupportedEncodingException { | ||
| ModelObject context = store.getContext(); | ||
| for (ContextResolver r : ExtensionList.lookup(ContextResolver.class)) { | ||
| String token = r.getToken(context); | ||
| if (token != null) { | ||
| return Jenkins.getActiveInstance().getRootUrlFromRequest() + "/" + getDescriptorUrl() | ||
| + "/checkId?provider=" + r.getClass().getName() + "&token=" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🐜 class name escaping
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. talk to core! (but I'll switch to sharing core's bug so I can share in core's fix) |
||
| + URLEncoder.encode(token, "UTF-8"); | ||
| } | ||
| } | ||
| return Jenkins.getActiveInstance().getRootUrlFromRequest() + "/" + getDescriptorUrl() | ||
| + "/checkId?provider=null&token=null"; | ||
| } | ||
|
|
||
| public final FormValidation doCheckId(@ContextInPath ModelObject context, @QueryParameter String value) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Restricted? |
||
| if (value.isEmpty()) { | ||
| return FormValidation.ok(); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,21 +24,24 @@ | |
| --> | ||
| <?jelly escape-by-default='true'?> | ||
| <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:f="/lib/form" xmlns:l="/lib/layout"> | ||
| <!-- TODO revert to dropdownDescriptorSelector when baseline is 1.645+ which supports `capture` attribute --> | ||
| <j:set var="descriptor" value="${it.credentialDescriptor}"/> | ||
| <j:set var="instance" value="${null}"/> | ||
| <f:dropdownList name="credentials" title="${%Kind}" help="${descriptor.getHelpFile('credentials')}"> | ||
| <j:set var="current" value="${instance[credentials]}"/> | ||
| <j:set var="current" value="${current!=null ? current : null}"/> | ||
| <j:forEach var="descriptor" items="${descriptors}" varStatus="loop"> | ||
| <f:dropdownListBlock value="${loop.index}" title="${descriptor.displayName}" | ||
| selected="${current.descriptor==descriptor || (current==null and descriptor==attrs.default)}" staplerClass="${descriptor.clazz.name}" | ||
| lazy="descriptor,it"> | ||
| <l:ajax> | ||
| <j:set var="instance" value="${current.descriptor==descriptor ? current : null}" /> | ||
| <st:include from="${descriptor}" page="${descriptor.configPage}" /> | ||
| </l:ajax> | ||
| </f:dropdownListBlock> | ||
| </j:forEach> | ||
| </f:dropdownList> | ||
| <j:local> | ||
| <j:set var="it" value="${it.store}"/> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you really need
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm curious about this
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It shouldn’t, assuming the xml namespace is correctly defined
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see a <local> tag in the Jelly documentation
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ahh should be |
||
| <!-- TODO revert to dropdownDescriptorSelector when baseline is 1.645+ which supports `capture` attribute --> | ||
| <j:set var="descriptor" value="${it.credentialDescriptor}"/> | ||
| <j:set var="instance" value="${null}"/> | ||
| <f:dropdownList name="credentials" title="${%Kind}" help="${descriptor.getHelpFile('credentials')}"> | ||
| <j:set var="current" value="${instance[credentials]}"/> | ||
| <j:set var="current" value="${current!=null ? current : null}"/> | ||
| <j:forEach var="descriptor" items="${descriptors}" varStatus="loop"> | ||
| <f:dropdownListBlock value="${loop.index}" title="${descriptor.displayName}" | ||
| selected="${current.descriptor==descriptor || (current==null and descriptor==attrs.default)}" staplerClass="${descriptor.clazz.name}" | ||
| lazy="descriptor,it"> | ||
| <l:ajax> | ||
| <j:set var="instance" value="${current.descriptor==descriptor ? current : null}" /> | ||
| <st:include from="${descriptor}" page="${descriptor.configPage}" /> | ||
| </l:ajax> | ||
| </f:dropdownListBlock> | ||
| </j:forEach> | ||
| </f:dropdownList> | ||
| </j:local> | ||
| </j:jelly> | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jglick you may be interested in this as a less hacky approach to https://github.com/jenkinsci/workflow-step-api-plugin/blob/aabbdca1c3c1f1aacf346b01d95d3be467547195/src/main/java/org/jenkinsci/plugins/workflow/util/StaplerReferer.java#L43
I wonder could we introduce some extension points and make a more generic solution in core?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linked from JENKINS-19413.