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

[JEP-227][JENKINS-39324] Replace Acegi Security with Spring Security APIs #490

Merged
merged 13 commits into from
Oct 27, 2023
67 changes: 32 additions & 35 deletions docs/consumer.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ public FormValidation doCheckCredentialsId(
if (value.startsWith("${") && value.endsWith("}")) { // <5>
return FormValidation.warning("Cannot validate expression based credentials");
}
if (CredentialsProvider.listCredentials( // <6>
if (CredentialsProvider.listCredentialsInItem( // <6>
...,
CredentialsMatchers.withId(value) // <6>
).isEmpty()) {
Expand All @@ -187,29 +187,26 @@ Better yet would be to try and ping the remote service anonymously and report su
_You may want to cache the check result for a short time-span if the remote service has rate limits on anonymous access._
<5> If you have not enabled credentials parameter expressions on the select control then you do not need this test.
<6> This example checks that the credentials exist, but does not use them to connect.
Alternatively `CredentialsMatchers.firstOrNull(CredentialsProvider.lookupCredentials(...), withId(value))` can be used to retrieve the credentials, a `null` return value would indicate that the error that they cannot be found, while the non-`null` return value could be used to validate the credentials against the remote service.
Alternatively `CredentialsMatchers.firstOrNull(CredentialsProvider.lookupCredentialsInItem(...), withId(value))` can be used to retrieve the credentials, a `null` return value would indicate that the error that they cannot be found, while the non-`null` return value could be used to validate the credentials against the remote service.
_You may want to cache the check result for a short time-span if the remote service has rate limits._

=== Listing available credentials matching some specific set of criteria

We use the `CredentialsProvider.listCredentials()` overloads to list credentials.
An external credentials provider may be recording usage of the credential and as such the `listCredentials` methods are supposed to not access the secret information and hence should not trigger such usage records.
We use the `CredentialsProvider.listCredentialsInItem()` or `CredentialsProvider.listCredentialsInItemGroup()` methods to list credentials.
An external credentials provider may be recording usage of the credential and as such the `listCredentialsInItem`/`listCredentialsInItemGroup` methods are supposed to not access the secret information and hence should not trigger such usage records.

[TIP]
====
If you are listing available credentials in order to populate a drop-down list, then `StandardListBoxModel.includeMatchingAs()` may be a more convenient way to call `CredentialsProvider.listCredentials()`
If you are listing available credentials in order to populate a drop-down list, then `StandardListBoxModel.includeMatchingAs()` may be a more convenient way to call `CredentialsProvider.listCredentialsInItem()`/`CredentialsProvider.listCredentialsInItemGroup()`
====

There are currently two overloads, one taking `Item` as the context and the other taking `ItemGroup` as the context, the other parameters are otherwise identical.

NOTE: A current RFE https://issues.jenkins-ci.org/browse/JENKINS-39324[JENKINS-39324] is looking to replace overloaded methods with a single method taking the more generic `ModelObject`.
Copy link
Member

Choose a reason for hiding this comment

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

Completely forgot I filed this seven years ago. @AncestorInPath ModelObject context would be convenient sometimes I guess, though having explicit methods for distinct types of context as you have here is probably clearer.


The parameters are:

`type`::
The type of credentials to list.

`item` or `itemGroup`::
`item` (when using `CredentialsProvider.listCredentialsInItem`)::
`itemGroup` (when using `CredentialsProvider.listCredentialsInItemGroup`)::
The context within which to list available credentials.

`authentication`::
Expand All @@ -227,12 +224,12 @@ Here are some examples of usage:
+
[source,java]
----
CredentialsProvider.listCredentials(
CredentialsProvider.listCredentialsInItem(
StandardUsernamePasswordCredentials.class, // <1>
job, // <2>
job instanceof Queue.Task // <3>
? Tasks.getAuthenticationOf((Queue.Task)job)) // <4>
: ACL.SYSTEM, // <5>
? Tasks.getAuthenticationOf2((Queue.Task)job)) // <4>
: ACL.SYSTEM2, // <5>
URIRequirementBuilder.fromUri(scmUrl), // <6>
null // <7>
);
Expand All @@ -244,7 +241,7 @@ We need `UsernamePasswordCredentials` to ensure that they are username and passw
<3> For almost all implementations of `Job`, this will be `true`.
(Note: https://plugins.jenkins.io/external-monitor-job[external jobs] do *not* implement `Queue.Task`)
<4> This is important, we must use the authentication that the job is likely to run as.
<5> If not a `Queue.Task` then use `ACL.SYSTEM`
<5> If not a `Queue.Task` then use `ACL.SYSTEM2`
<6> We use the requirements builder most idiomatically appropriate to our use case.
In most cases, unless `URIRequirementBuilder` can be used to construct at least some domain requirements.
<7> We do not have any additional requirements to place, so we can specify `null` for the matcher.
Expand All @@ -253,10 +250,10 @@ In most cases, unless `URIRequirementBuilder` can be used to construct at least
+
[source,java]
----
CredentialsProvider.listCredentials(
CredentialsProvider.listCredentialsInItem(
StandardUsernamePasswordCredentials.class,
job,
Jenkins.getAuthentication(), // <1>
Jenkins.getAuthentication2(), // <1>
URIRequirementBuilder.fromUri(scmUrl),
null
)
Expand All @@ -267,12 +264,12 @@ CredentialsProvider.listCredentials(
+
[source,java]
----
CredentialsProvider.listCredentials(
CredentialsProvider.listCredentialsInItem(
StandardCredentials.class, // <1>
job,
job instanceof Queue.Task
? Tasks.getAuthenticationOf((Queue.Task)job))
: ACL.SYSTEM,
? Tasks.getAuthenticationOf2((Queue.Task)job))
: ACL.SYSTEM2,
URIRequirementBuilder.fromUri(issueTrackerUrl),
AuthenticationTokens.matcher(IssueTrackerAuthentication.class) // <2>
)
Expand All @@ -288,10 +285,10 @@ Alternatively, more complex conversion contexts can be handled with `Authenticat
+
[source,java]
----
CredentialsProvider.listCredentials(
CredentialsProvider.listCredentialsInItem(
StandardCredentials.class, // <1>
job,
Jenkins.getAuthentication(), // <2>
Jenkins.getAuthentication2(), // <2>
URIRequirementBuilder.fromUri(loadBalancerUrl),
CredentialsMatchers.allOf(
AuthenticationTokens.matcher(LoadBalancerAuthentication.class),
Expand All @@ -313,18 +310,18 @@ This drop down list would typically be displayed from one of the _Manage Jenkins
+
[source,java]
----
CredentialsProvider.listCredentials(
CredentialsProvider.listCredentialsInItemGroup(
StandardUsernameCredentials.class, // <1>
Jenkins.get(), // <2>
ACL.SYSTEM, // <2>
ACL.SYSTEM2, // <2>
URIRequirementBuilder.fromUri(scmUrl),
AuthenticationTokens.matcher(MySCMAuthentication.class) // <1>
)
----
<1> For this SCM, management of post commit hooks requires authentication that has specified a username, so even though there are other authentication mechanisms supported by `AuthenticationTokens.matcher(...)` we limit at the type level as that reduces the response that needs to be filtered.
The alternative would have been a matcher that combined `CredentialsMatchers.instanceOf(StandardUsernameCredentials.class)` but this reduces the ability of an external credentials provider to filter the query on the remote side.
<2> We are doing this operation outside of the context of a single job, rather this is being performed on behalf of the entire Jenkins instance.
Thus we should be performing this as `ACL.SYSTEM` and in the context of `Jenkins.get()`.
Thus we should be performing this as `ACL.SYSTEM2` and in the context of `Jenkins.get()`.
This has the additional benefit that the admin can restrict the high permission hook management credentials to `CredentialsScope.SYSTEM` which will prevent access by jobs.

=== Persist a reference to a specific credential instance
Expand Down Expand Up @@ -382,24 +379,24 @@ If we have a job, "foobar", and we configure a credentials parameter on that job

If you are working outside the context of a `Run` then you will not have to deal with the complexities of credentials expressions.

In most cases the retrieval will just be a call to one of the `CredentialsProvider.lookupCredentials(...)` wrapped within `CredentialsMatchers.firstOrNull(..., CredentialsMatchers.withId(...))`, for example:
In most cases the retrieval will just be a call to one of the `CredentialsProvider.lookupCredentialsInItem(...)`/`CredentialsProvider.lookupCredentialsInItemGroup(...)` wrapped within `CredentialsMatchers.firstOrNull(..., CredentialsMatchers.withId(...))`, for example:

[source,java]
----
StandardCredentials c = CredentialsMatchers.firstOrNull(
CredentialsProvider.lookupCredentials(
CredentialsProvider.lookupCredentialsInItem(
StandardCredentials.class, // <1>
job, // <1>
job instanceof Queue.Task // <1>
? Tasks.getAuthenticationOf((Queue.Task)job))
: ACL.SYSTEM,
: ACL.SYSTEM2,
URIRequirementBuilder.fromUri(...) // <1>
),
CredentialsMatchers.withId(credentialsId) // <2>
);
----
<1> These should be the same as your call to `CredentialsProvider.listCredentials(...)`/`StandardListBoxModel.includeMatchingAs(...)` in order to ensure that we get the same credential instance back.
<2> If you had additional `CredentialsMatcher` expressions in your call to `CredentialsProvider.listCredentials(...)`/`StandardListBoxModel.includeMatchingAs(...)` then you should merge them here with a `CredentialsMatchers.allOf(...)`
<1> These should be the same as your call to `CredentialsProvider.listCredentialsInItem(...)`/`CredentialsProvider.listCredentialsInItemGroup(...)`/`StandardListBoxModel.includeMatchingAs(...)` in order to ensure that we get the same credential instance back.
<2> If you had additional `CredentialsMatcher` expressions in your call to `CredentialsProvider.listCredentialsInItem(...)`/`CredentialsProvider.listCredentialsInItemGroup(...)`/`StandardListBoxModel.includeMatchingAs(...)` then you should merge them here with a `CredentialsMatchers.allOf(...)`

Once you have retrieved a non-null credentials instance, all non-secret properties can be assumed as eager-fetch immutable.

Expand All @@ -416,12 +413,12 @@ The recommended way to use a credential is through the https://plugins.jenkins.i
[source,java]
----
StandardCredentials c = CredentialsMatchers.firstOrNull( // <1>
CredentialsProvider.listCredentials(
CredentialsProvider.listCredentialsInItem(
StandardCredentials.class,
job,
job instanceof Queue.Task
? Tasks.getAuthenticationOf((Queue.Task)job))
: ACL.SYSTEM,
? Tasks.getAuthenticationOf2((Queue.Task)job))
: ACL.SYSTEM2,
URIRequirementBuilder.fromUri(issueTrackerUrl)
),
CredentialsMatchers.allOf(
Expand Down Expand Up @@ -461,12 +458,12 @@ IssueTrackerAuthentication auth = AuthenticationTokens.convert(
CredentialsProvider.track(
job,
CredentialsMatchers.firstOrNull(
CredentialsProvider.listCredentials(
CredentialsProvider.listCredentialsInItem(
StandardCredentials.class,
job,
job instanceof Queue.Task
? Tasks.getAuthenticationOf((Queue.Task)job))
: ACL.SYSTEM,
? Tasks.getAuthenticationOf2((Queue.Task)job))
: ACL.SYSTEM2,
URIRequirementBuilder.fromUri(issueTrackerUrl)
),
CredentialsMatchers.allOf(
Expand Down
6 changes: 3 additions & 3 deletions docs/implementation.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -742,9 +742,9 @@ If you have implemented that check before creating the proxy then you could be m
<2> Any consumer plugin that is transferring a credential to another JVM is supposed to call `CredentialsProvider.snapshot(credential)` and send the return value.
The `CredentialsSnapshotTaker` is supposed to fetch the secret as part of the snapshotting, so a proper consumer will never be at risk of this `IOException`.

* The `CredentialsProvider.getCredentials(...)` methods should instantiate the proxies, so these methods will operate from the cache while initiate background refresh. Where the cache is a miss or where the cache is stale, a short term block is acceptable.
* The `CredentialsProvider.getCredentialsInItem(...)` / `CredentialsProvider.getCredentialsInItemGroup(...)` methods should instantiate the proxies, so these methods will operate from the cache while initiate background refresh. Where the cache is a miss or where the cache is stale, a short term block is acceptable.

* The `CredentialsProvider.getCredentialIds(...)` methods are used to list credentials for drop-down list population, so these methods should use a live request with a fall-back to the cache where the live request takes too long.
* The `CredentialsProvider.getCredentialIdsInItem(...)` / `CredentialsProvider.getCredentialIdsInItemGroup(...)` methods are used to list credentials for drop-down list population, so these methods should use a live request with a fall-back to the cache where the live request takes too long.

[NOTE]
====
Expand All @@ -758,6 +758,6 @@ The main work in an implementation will be the mapping to `CredentialStore` inst
+
[NOTE]
====
Technically, the "read-only, implicitly exposed" style credentials provider implementation does not need to interact with the `CredentialsStore` portion of the API as it can expose credentials directly using just the `CredentialsProvider.getCredentials(...)` and `CredentialsProvider.getCredentialIds(...)`, however, implementing the `CredentialsStore` contract is required in order for the credentials to be visible to users via the Credentials side action on the different Jenkins context objects.
Technically, the "read-only, implicitly exposed" style credentials provider implementation does not need to interact with the `CredentialsStore` portion of the API as it can expose credentials directly using just the `CredentialsProvider.getCredentialsInItem(...)`/`CredentialsProvider.getCredentialsInItemGroup(...)` and `CredentialsProvider.getCredentialIdsInItem(...)`/`CredentialsProvider.getCredentialIdsInItemGroup(...)`, however, implementing the `CredentialsStore` contract is required in order for the credentials to be visible to users via the Credentials side action on the different Jenkins context objects.
====
* A "read-write, implicitly exposed" style implementation will need to semi-dynamically create `CredentialsStore` instances for each context in order to integrate with the Jenkins credentials management UI.
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@
import java.util.Set;
import jenkins.model.Jenkins;
import net.sf.json.JSONObject;
import org.acegisecurity.Authentication;
import org.apache.commons.lang.StringUtils;
import org.jenkinsci.Symbol;
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.StaplerRequest;
import org.springframework.security.core.Authentication;

/**
* A {@link ParameterDefinition} for a parameter that supplies a {@link Credentials}.
Expand Down Expand Up @@ -173,7 +173,7 @@ public StandardListBoxModel doFillDefaultValueItems(@AncestorInPath Item context
final StandardListBoxModel result = new StandardListBoxModel();
result.includeEmptyValue();
if (acl.hasPermission(CredentialsProvider.USE_ITEM)) {
result.includeAs(CredentialsProvider.getDefaultAuthenticationOf(context), context, typeClass, domainRequirements);
result.includeAs(CredentialsProvider.getDefaultAuthenticationOf2(context), context, typeClass, domainRequirements);
}
return result;
}
Expand All @@ -185,9 +185,9 @@ public StandardListBoxModel doFillValueItems(@AncestorInPath Item context,
@QueryParameter boolean includeUser) {
Jenkins jenkins = Jenkins.get();
final ACL acl = context == null ? jenkins.getACL() : context.getACL();
final Authentication authentication = Jenkins.getAuthentication();
final Authentication itemAuthentication = CredentialsProvider.getDefaultAuthenticationOf(context);
final boolean isSystem = ACL.SYSTEM.equals(authentication);
final Authentication authentication = Jenkins.getAuthentication2();
final Authentication itemAuthentication = CredentialsProvider.getDefaultAuthenticationOf2(context);
final boolean isSystem = ACL.SYSTEM2.equals(authentication);
final Class<? extends StandardCredentials> typeClass = decodeType(credentialType);
final List<DomainRequirement> domainRequirements = Collections.emptyList();
final StandardListBoxModel result = new StandardListBoxModel();
Expand Down
Loading
Loading