Skip to content

Commit

Permalink
[JEP-227][JENKINS-39324] Replace Acegi Security with Spring Security …
Browse files Browse the repository at this point in the history
…APIs (#490)

* [JEP-227] Replace Acegi Security with Spring Security APIs

This is the implementation of
jenkinsci/jenkins#4848 for Credentials API. This
will allow consumers of the credentials API to remove references to
deprecated acegi APIs.

* Fix spotbugs issues

* Fix reviews

* Rename methods appropriately

* Fixup some javadocs.

* Remove unused method.

* Forgot to remove usages.

* Remove CredentialsProvider#getCredentials2(Class, ItemGroup, Authentication) in favor of CredentialsProvider#getCredentials2(Class, ItemGroup, Authentication, List)

* Fix a few null checks while we are here.

* Restore a method I deleted by mistake.

* Add tests for new signatures

* Rename *2 methods to *InItem/*InItemGroup to avoid ambiguous signatures

* Update docs
  • Loading branch information
Vlatombe committed Oct 27, 2023
1 parent 5ec13ee commit 04f5ec1
Show file tree
Hide file tree
Showing 19 changed files with 634 additions and 437 deletions.
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`.

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

0 comments on commit 04f5ec1

Please sign in to comment.