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

Code improvements #228

Merged
merged 15 commits into from
Oct 22, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ public static <T extends ModelObject> T findContextInPath(@NonNull StaplerReques
if (pathSegments.size() >= 2) {
String firstSegment = pathSegments.get(0);
if ("user".equals(firstSegment)) {
User user = User.get(pathSegments.get(1));
User user = User.getById(pathSegments.get(1), true);
if (type.isInstance(user) && CredentialsProvider.hasStores(user)) {
// we have a winner
return type.cast(user);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.cloudbees.plugins.credentials.common.StandardCredentials;
import com.cloudbees.plugins.credentials.common.StandardListBoxModel;
import com.cloudbees.plugins.credentials.domains.DomainRequirement;
import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.Extension;
import hudson.model.Descriptor;
import hudson.model.Item;
Expand Down Expand Up @@ -117,6 +118,7 @@ public static class DescriptorImpl extends ParameterDescriptor {
/**
* {@inheritDoc}
*/
@NonNull
@Override
public String getDisplayName() {
return Messages.CredentialsParameterDefinition_DisplayName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -695,84 +695,80 @@ public static boolean hasStores(final ModelObject context) {
*/
public static Iterable<CredentialsStore> lookupStores(final ModelObject context) {
final ExtensionList<CredentialsProvider> providers = all();
return new Iterable<CredentialsStore>() {
public Iterator<CredentialsStore> iterator() {
return new Iterator<CredentialsStore>() {
private ModelObject current = context;
private Iterator<CredentialsProvider> iterator = providers.iterator();
private CredentialsStore next;

public boolean hasNext() {
return () -> new Iterator<CredentialsStore>() {
private ModelObject current = context;
private Iterator<CredentialsProvider> iterator = providers.iterator();
private CredentialsStore next;

public boolean hasNext() {
if (next != null) {
return true;
}
while (current != null) {
while (iterator.hasNext()) {
CredentialsProvider p = iterator.next();
if (!p.isEnabled(context)) {
continue;
}
next = p.getStore(current);
if (next != null) {
return true;
}
while (current != null) {
while (iterator.hasNext()) {
CredentialsProvider p = iterator.next();
if (!p.isEnabled(context)) {
continue;
}
next = p.getStore(current);
if (next != null) {
return true;
}
}
// now walk up the model object tree
// TODO make this an extension point perhaps ContextResolver could help
if (current instanceof Item) {
current = ((Item) current).getParent();
iterator = providers.iterator();
} else if (current instanceof User) {
Jenkins jenkins = Jenkins.get();
Authentication a;
if (jenkins.hasPermission(USE_ITEM) && current == User.current()) {
// this is the fast path for the 99% of cases
a = Jenkins.getAuthentication();
} else {
try {
a = ((User) current).impersonate();
} catch (UsernameNotFoundException e) {
a = null;
}
}
if (current == User.current() && jenkins.getACL().hasPermission(a, USE_ITEM)) {
current = jenkins;
iterator = providers.iterator();
} else {
current = null;
}
} else if (current instanceof Jenkins) {
// escape
current = null;
} else if (current instanceof ComputerSet) {
current = Jenkins.get();
iterator = providers.iterator();
} else if (current instanceof Computer) {
current = Jenkins.get();
iterator = providers.iterator();
} else if (current instanceof Node) {
current = Jenkins.get();
iterator = providers.iterator();
} else {
// fall back to Jenkins as the ultimate parent of everything else
current = Jenkins.get();
iterator = providers.iterator();
}
// now walk up the model object tree
// TODO make this an extension point perhaps ContextResolver could help
if (current instanceof Item) {
current = ((Item) current).getParent();
iterator = providers.iterator();
} else if (current instanceof User) {
Jenkins jenkins = Jenkins.get();
Authentication a;
if (jenkins.hasPermission(USE_ITEM) && current == User.current()) {
// this is the fast path for the 99% of cases
a = Jenkins.getAuthentication();
} else {
try {
a = ((User) current).impersonate();
} catch (UsernameNotFoundException e) {
a = null;
}
}
return false;
}

public CredentialsStore next() {
if (!hasNext()) {
throw new NoSuchElementException();
}
try {
return next;
} finally {
next = null;
if (current == User.current() && jenkins.getACL().hasPermission(a, USE_ITEM)) {
current = jenkins;
iterator = providers.iterator();
} else {
current = null;
}
} else if (current instanceof Jenkins) {
// escape
current = null;
} else if (current instanceof ComputerSet) {
current = Jenkins.get();
iterator = providers.iterator();
} else if (current instanceof Computer) {
current = Jenkins.get();
iterator = providers.iterator();
} else if (current instanceof Node) {
current = Jenkins.get();
iterator = providers.iterator();
} else {
// fall back to Jenkins as the ultimate parent of everything else
current = Jenkins.get();
iterator = providers.iterator();
}
};
}
return false;
}

public CredentialsStore next() {
if (!hasNext()) {
throw new NoSuchElementException();
}
try {
return next;
} finally {
next = null;
}
}
};
}
Expand Down Expand Up @@ -1068,6 +1064,7 @@ public boolean isEnabled(Object context) {
/**
* {@inheritDoc}
*/
@NonNull
@Override
public String getDisplayName() {
return StringUtils.join(StringUtils.splitByCharacterTypeCamelCase(getClass().getSimpleName()), ' ');
Expand Down Expand Up @@ -1658,19 +1655,16 @@ public static <C extends Credentials> List<C> trackAll(@NonNull Item item, @NonN
* {@link DoNotUse} in order to ensure that no plugin attempts to call this method. If invoking this method
* from an {@code init.d} Groovy script, ensure that the call is guarded by a marker file such that
*
* @throws IOException if things go wrong.
*/
@Restricted(DoNotUse.class) // Do not use from plugins
public static void saveAll() throws IOException {
public static void saveAll() {
LOGGER.entering(CredentialsProvider.class.getName(), "saveAll");
try {
Jenkins jenkins = Jenkins.get();
jenkins.checkPermission(Jenkins.ADMINISTER);
LOGGER.log(Level.INFO, "Forced save credentials stores: Requested by {0}",
StringUtils.defaultIfBlank(Jenkins.getAuthentication().getName(), "anonymous"));
Timer.get().execute(new Runnable() {
@Override
public void run() {
Timer.get().execute(() -> {
try (ACLContext ctx = ACL.as(ACL.SYSTEM)) {
if (jenkins.getInitLevel().compareTo(InitMilestone.JOB_LOADED) < 0) {
LOGGER.log(Level.INFO, "Forced save credentials stores: Initialization has not completed");
Expand Down Expand Up @@ -1722,7 +1716,7 @@ public void run() {
LOGGER.log(Level.INFO, "Forced save credentials stores: Processing Users ({0} processed)",
count);
}
// HACK ALERT we just want to access the user's stores so we do just enough impersonation
// HACK ALERT we just want to access the user's stores, so we do just enough impersonation
// to ensure that User.current() == user
// while we could use User.impersonate() that would force a query against the backing
// SecurityRealm to revalidate
Expand All @@ -1743,8 +1737,7 @@ public void run() {
} finally {
LOGGER.log(Level.INFO, "Forced save credentials stores: Completed");
}
}
});
});
} finally {
LOGGER.exiting(CredentialsProvider.class.getName(), "saveAll");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ public static class DescriptorImpl extends CredentialsProviderFilterDescriptor {
/**
* {@inheritDoc}
*/
@NonNull
@Override
public String getDisplayName() {
return Messages.CredentialsProviderFilter_None_DisplayName();
Expand Down Expand Up @@ -252,6 +253,7 @@ public static class DescriptorImpl extends CredentialsProviderFilterDescriptor {
/**
* {@inheritDoc}
*/
@NonNull
@Override
public String getDisplayName() {
return Messages.CredentialsProviderFilter_Includes_DisplayName();
Expand Down Expand Up @@ -364,6 +366,7 @@ public static class DescriptorImpl extends CredentialsProviderFilterDescriptor {
/**
* {@inheritDoc}
*/
@NonNull
@Override
public String getDisplayName() {
return Messages.CredentialsProviderFilter_Excludes_DisplayName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public static CredentialsProviderManager getInstanceOrDie() {
* {@inheritDoc}
*/
@Override
public boolean filter(Object context, Descriptor descriptor) {
public boolean filter(Object context, @NonNull Descriptor descriptor) {
Map<CredentialsProviderTypeRestrictionDescriptor, List<CredentialsProviderTypeRestriction>> restrictions =
restrictions();
if (restrictions != null && descriptor instanceof CredentialsDescriptor) {
Expand Down Expand Up @@ -446,6 +446,7 @@ public boolean configure(StaplerRequest req, JSONObject json) throws FormExcepti
/**
* {@inheritDoc}
*/
@NonNull
@Override
public GlobalConfigurationCategory getCategory() {
return GlobalConfigurationCategory.get(GlobalCredentialsConfiguration.Category.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
*/
package com.cloudbees.plugins.credentials;

import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.Extension;
import hudson.ExtensionList;
import hudson.ExtensionPoint;
Expand Down Expand Up @@ -228,6 +229,7 @@ public boolean filter(List<CredentialsProviderTypeRestriction> restrictions, Cre
/**
* {@inheritDoc}
*/
@NonNull
@Override
public String getDisplayName() {
return Messages.CredentialsProviderTypeRestriction_Includes_DisplayName();
Expand Down Expand Up @@ -383,6 +385,7 @@ public boolean filter(List<CredentialsProviderTypeRestriction> restrictions, Cre
/**
* {@inheritDoc}
*/
@NonNull
@Override
public String getDisplayName() {
return Messages.CredentialsProviderTypeRestriction_Excludes_DisplayName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ public CredentialsSelectHelper getDescriptor() {
/**
* {@inheritDoc}
*/
@NonNull
@Override
public String getDisplayName() {
return Messages.CredentialsSelectHelper_DisplayName();
Expand Down Expand Up @@ -567,6 +568,7 @@ public static final class WrappedCredentialsStore implements IconSpec, ModelObje
/**
* Our {@link CredentialsStore}.
*/
@NonNull
private final CredentialsStore store;

/**
Expand All @@ -577,8 +579,8 @@ public static final class WrappedCredentialsStore implements IconSpec, ModelObje
* @param token the context's {@link ContextResolver#getToken(ModelObject)}.
* @param store the {@link CredentialsStore}
*/
public WrappedCredentialsStore(ContextResolver resolver, CredentialsProvider provider,
String token, CredentialsStore store) {
public WrappedCredentialsStore(@NonNull ContextResolver resolver, @NonNull CredentialsProvider provider,
@NonNull String token, @NonNull CredentialsStore store) {
this.store = store;
this.resolver = resolver;
this.provider = provider;
Expand Down Expand Up @@ -735,6 +737,7 @@ public ModelObject getContext(String token) {
/**
* {@inheritDoc}
*/
@NonNull
@Override
public String getDisplayName() {
return "Nothing";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,13 @@ public final Set<CredentialsScope> getScopes() {
/**
* {@inheritDoc}
*/
@NonNull
public ACL getACL() {
// we really want people to implement this one, but in case of legacy implementations we need to provide
// an effective ACL implementation.
return new ACL() {
@Override
public boolean hasPermission(Authentication a, Permission permission) {
public boolean hasPermission(@NonNull Authentication a, @NonNull Permission permission) {
return CredentialsStore.this.hasPermission(a, permission);
}
};
Expand Down
Loading