diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java index 7e6c96a065..f33c1ecea3 100644 --- a/core/src/main/java/hudson/FilePath.java +++ b/core/src/main/java/hudson/FilePath.java @@ -80,8 +80,10 @@ import java.net.URLConnection; import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.InvalidPathException; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.LinkOption; +import java.nio.file.Paths; import java.nio.file.StandardCopyOption; import java.nio.file.attribute.FileAttribute; import java.nio.file.attribute.PosixFilePermission; @@ -3134,33 +3136,61 @@ public final class FilePath implements Serializable { if (new File(potentialChildRelativePath).isAbsolute()) { throw new IllegalArgumentException("Only a relative path is supported, the given path is absolute: " + potentialChildRelativePath); } + + Path parentAbsolutePath = Util.fileToPath(parentFile.getAbsoluteFile()); + Path parentRealPath; + try { + parentRealPath = parentAbsolutePath.toRealPath(); + } + catch(NoSuchFileException e) { + throw new IllegalArgumentException("The parent does not exist"); + } - Path parent = parentFile.getAbsoluteFile().toPath().normalize(); - + // example: "a/b/c" that will become "b/c" then just "c", and finally an empty string String remainingPath = potentialChildRelativePath; - File currentFile = parentFile; + + Path currentFilePath = parentFile.toPath(); while (!remainingPath.isEmpty()) { - File directChild = this.getDirectChild(currentFile, remainingPath); - File childUsingFullPath = new File(currentFile, remainingPath); - remainingPath = childUsingFullPath.getAbsolutePath().substring(directChild.getAbsolutePath().length()); - - File childFileSymbolic = Util.resolveSymlinkToFile(directChild); + Path directChild = this.getDirectChild(currentFilePath, remainingPath); + Path childUsingFullPath = currentFilePath.resolve(remainingPath); + Path rel = directChild.toAbsolutePath().relativize(childUsingFullPath.toAbsolutePath()); + remainingPath = rel.toString(); + + File childFileSymbolic = Util.resolveSymlinkToFile(directChild.toFile()); if (childFileSymbolic == null) { - currentFile = directChild; + currentFilePath = directChild; } else { - currentFile = childFileSymbolic; + currentFilePath = childFileSymbolic.toPath(); + } + + Path currentFileAbsolutePath = currentFilePath.toAbsolutePath(); + try{ + Path child = currentFileAbsolutePath.toRealPath(); + if (!child.startsWith(parentRealPath)) { + return false; + } + } catch (NoSuchFileException e) { + // nonexistent file + // in case this folder / file will be copied somewhere else, + // it becomes the responsibility of that system to check the isDescendant with the existing links + // we are not taking the parentRealPath to avoid possible problem + try { + Path child = currentFileAbsolutePath.normalize(); + Path parent = parentAbsolutePath.normalize(); + return child.startsWith(parent); + } catch (InvalidPathException e2) { + throw new IOException(e2); + } } } - //TODO could be refactored using Util#isDescendant(File, File) from 2.80+ - Path child = currentFile.getAbsoluteFile().toPath().normalize(); - return child.startsWith(parent); + return true; } - private @CheckForNull File getDirectChild(File parentFile, String childPath){ - File current = new File(parentFile, childPath); - while (current != null && !parentFile.equals(current.getParentFile())) { - current = current.getParentFile(); + private @CheckForNull Path getDirectChild(Path parentPath, String childPath){ + Path current = parentPath.resolve(childPath); + while (current != null && !parentPath.equals(current.getParent())) { + current = current.getParent(); } return current; } diff --git a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java index 504eaba0d1..94e475e10a 100644 --- a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java +++ b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java @@ -23,6 +23,7 @@ */ package hudson.model; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.FilePath; import hudson.Util; import java.io.IOException; @@ -38,9 +39,12 @@ import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.Locale; +import java.util.Objects; import java.util.StringTokenizer; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.stream.Collectors; +import java.util.stream.Stream; import javax.servlet.ServletException; import javax.servlet.http.HttpServletResponse; import jenkins.model.Jenkins; @@ -67,6 +71,9 @@ import org.kohsuke.stapler.StaplerResponse; * @author Kohsuke Kawaguchi */ public final class DirectoryBrowserSupport implements HttpResponse { + // escape hatch for SECURITY-904 to keep legacy behavior + @SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "Accessible via System Groovy Scripts") + public static boolean ALLOW_SYMLINK_ESCAPE = Boolean.getBoolean(DirectoryBrowserSupport.class.getName() + ".allowSymlinkEscape"); public final ModelObject owner; @@ -215,13 +222,19 @@ public final class DirectoryBrowserSupport implements HttpResponse { String base = _base.toString(); String rest = _rest.toString(); + if(!ALLOW_SYMLINK_ESCAPE && (root.supportIsDescendant() && !root.isDescendant(base))){ + LOGGER.log(Level.WARNING, "Trying to access a file outside of the directory, target: "+ base); + rsp.sendError(HttpServletResponse.SC_FORBIDDEN, "Trying to access a file outside of the directory, target: " + base); + return; + } + // this is the base file/directory VirtualFile baseFile = base.isEmpty() ? root : root.child(base); if(baseFile.isDirectory()) { if(zip) { rsp.setContentType("application/zip"); - zip(rsp.getOutputStream(), baseFile, rest); + zip(rsp, root, baseFile, rest); return; } if (plain) { @@ -249,8 +262,8 @@ public final class DirectoryBrowserSupport implements HttpResponse { } List> glob = null; - - if(rest.length()>0) { + boolean patternUsed = rest.length() > 0; + if(patternUsed) { // the rest is Ant glob pattern glob = patternScan(baseFile, rest, createBackRef(restSize)); } else @@ -260,13 +273,15 @@ public final class DirectoryBrowserSupport implements HttpResponse { } if(glob!=null) { + List> filteredGlob = keepReadabilityOnlyOnDescendants(baseFile, patternUsed, glob); + // serve glob req.setAttribute("it", this); List parentPaths = buildParentPath(base,restSize); req.setAttribute("parentPath",parentPaths); req.setAttribute("backPath", createBackRef(restSize)); req.setAttribute("topPath", createBackRef(parentPaths.size()+restSize)); - req.setAttribute("files", glob); + req.setAttribute("files", filteredGlob); req.setAttribute("icon", icon); req.setAttribute("path", path); req.setAttribute("pattern",rest); @@ -330,6 +345,57 @@ public final class DirectoryBrowserSupport implements HttpResponse { rsp.serveFile(req, in, lastModified, -1, length, baseFile.getName() ); } } + + private List> keepReadabilityOnlyOnDescendants(VirtualFile root, boolean patternUsed, List> pathFragmentsList){ + Stream> pathFragmentsStream = pathFragmentsList.stream().map((List pathFragments) -> { + List mappedFragments = new ArrayList<>(pathFragments.size()); + String relativePath = ""; + for (int i = 0; i < pathFragments.size(); i++) { + Path current = pathFragments.get(i); + if (i == 0) { + relativePath = current.title; + } else { + relativePath += "/" + current.title; + } + + if (!current.isReadable) { + if (patternUsed) { + // we do not want to leak information about existence of folders / files satisfying the pattern inside that folder + return null; + } + mappedFragments.add(current); + return mappedFragments; + } else { + if (isDescendant(root, relativePath)) { + mappedFragments.add(current); + } else { + if (patternUsed) { + // we do not want to leak information about existence of folders / files satisfying the pattern inside that folder + return null; + } + mappedFragments.add(Path.createNotReadableVersionOf(current)); + return mappedFragments; + } + } + } + return mappedFragments; + }); + + if (patternUsed) { + pathFragmentsStream = pathFragmentsStream.filter(Objects::nonNull); + } + + return pathFragmentsStream.collect(Collectors.toList()); + } + + private boolean isDescendant(VirtualFile root, String relativePath){ + try { + return ALLOW_SYMLINK_ESCAPE || !root.supportIsDescendant() || root.isDescendant(relativePath); + } + catch (IOException e) { + return false; + } + } private String getPath(StaplerRequest req) { String path = req.getRestOfPath(); @@ -363,7 +429,8 @@ public final class DirectoryBrowserSupport implements HttpResponse { return buf.toString(); } - private static void zip(OutputStream outputStream, VirtualFile dir, String glob) throws IOException { + private static void zip(StaplerResponse rsp, VirtualFile root, VirtualFile dir, String glob) throws IOException, InterruptedException { + OutputStream outputStream = rsp.getOutputStream(); try (ZipOutputStream zos = new ZipOutputStream(outputStream)) { zos.setEncoding(System.getProperty("file.encoding")); // TODO JENKINS-20663 make this overridable via query parameter // TODO consider using run(Callable) here @@ -375,18 +442,24 @@ public final class DirectoryBrowserSupport implements HttpResponse { } else { relativePath = n; } - // In ZIP archives "All slashes MUST be forward slashes" (http://pkware.com/documents/casestudies/APPNOTE.TXT) - // TODO On Linux file names can contain backslashes which should not treated as file separators. - // Unfortunately, only the file separator char of the master is known (File.separatorChar) - // but not the file separator char of the (maybe remote) "dir". - ZipEntry e = new ZipEntry(relativePath.replace('\\', '/')); - VirtualFile f = dir.child(n); - e.setTime(f.lastModified()); - zos.putNextEntry(e); - try (InputStream in = f.open()) { - IOUtils.copy(in, zos); + + String targetFile = dir.toString().substring(root.toString().length()) + n; + if (!ALLOW_SYMLINK_ESCAPE && root.supportIsDescendant() && !root.isDescendant(targetFile)) { + LOGGER.log(Level.INFO, "Trying to access a file outside of the directory: " + root + ", illicit target: " + targetFile); + } else { + // In ZIP archives "All slashes MUST be forward slashes" (http://pkware.com/documents/casestudies/APPNOTE.TXT) + // TODO On Linux file names can contain backslashes which should not treated as file separators. + // Unfortunately, only the file separator char of the master is known (File.separatorChar) + // but not the file separator char of the (maybe remote) "dir". + ZipEntry e = new ZipEntry(relativePath.replace('\\', '/')); + VirtualFile f = dir.child(n); + e.setTime(f.lastModified()); + zos.putNextEntry(e); + try (InputStream in = f.open()) { + IOUtils.copy(in, zos); + } + zos.closeEntry(); } - zos.closeEntry(); } } } @@ -458,6 +531,10 @@ public final class DirectoryBrowserSupport implements HttpResponse { return size; } + public static Path createNotReadableVersionOf(Path that){ + return new Path(that.href, that.title, that.isFolder, that.size, false); + } + private static final long serialVersionUID = 1L; } diff --git a/core/src/main/java/hudson/model/Run.java b/core/src/main/java/hudson/model/Run.java index 0e09a19397..ab4c0528eb 100644 --- a/core/src/main/java/hudson/model/Run.java +++ b/core/src/main/java/hudson/model/Run.java @@ -1173,7 +1173,7 @@ public abstract class Run ,RunT extends Run, ModelObjectWithContextMenu, StaplerProxy { + public static final XStream2 XSTREAM = new XStream2(); + private static final Logger LOGGER = Logger.getLogger(User.class.getName()); + static final String CONFIG_XML = "config.xml"; + + /** + * Escape hatch for StaplerProxy-based access control + */ + @Restricted(NoExternalUse.class) + public static /* Script Console modifiable */ boolean SKIP_PERMISSION_CHECK = Boolean.getBoolean(User.class.getName() + ".skipPermissionCheck"); + + /** + * Jenkins now refuses to let the user login if he/she doesn't exist in {@link SecurityRealm}, + * which was necessary to make sure users removed from the backend will get removed from the frontend. + *

+ * Unfortunately this infringed some legitimate use cases of creating Jenkins-local users for + * automation purposes. This escape hatch switch can be enabled to resurrect that behaviour. + *

+ * @see JENKINS-22346. + */ + public static boolean ALLOW_NON_EXISTENT_USER_TO_LOGIN = SystemProperties.getBoolean(User.class.getName() + ".allowNonExistentUserToLogin"); + + /** + * Jenkins historically created a (usually) ephemeral user record when an user with Overall/Administer permission + * accesses a /user/arbitraryName URL. + *

+ * Unfortunately this constitutes a CSRF vulnerability, as malicious users can make admins create arbitrary numbers + * of ephemeral user records, so the behavior was changed in Jenkins 2.TODO / 2.32.2. + *

+ * As some users may be relying on the previous behavior, setting this to true restores the previous behavior. This + * is not recommended. + *

+ * SECURITY-406. + */ + @Restricted(NoExternalUse.class) + public static boolean ALLOW_USER_CREATION_VIA_URL = SystemProperties.getBoolean(User.class.getName() + ".allowUserCreationViaUrl"); + /** * The username of the 'unknown' user used to avoid null user references. */ @@ -147,23 +172,72 @@ public class User extends AbstractModelObject implements AccessControlled, Descr */ private static final String[] ILLEGAL_PERSISTED_USERNAMES = new String[]{ACL.ANONYMOUS_USERNAME, ACL.SYSTEM_USERNAME, UNKNOWN_USERNAME}; - private transient final String id; + private final int version = 10; // Not currently used, but it may be helpful in the future to store a version. + private String id; private volatile String fullName; - private volatile String description; - /** - * List of {@link UserProperty}s configured for this project. - */ @CopyOnWrite - private volatile List properties = new ArrayList(); + private volatile List properties = new ArrayList<>(); + static { + XSTREAM.alias("user", User.class); + } private User(String id, String fullName) { this.id = id; this.fullName = fullName; - load(); + load(id); + } + + private void load(String userId) { + clearExistingProperties(); + loadFromUserConfigFile(userId); + removeNullsThatFailedToLoad(); + allocateDefaultPropertyInstancesAsNeeded(); + setUserToProperties(); + } + + private void setUserToProperties() { + for (UserProperty p : properties) { + p.setUser(this); + } + } + + private void allocateDefaultPropertyInstancesAsNeeded() { + for (UserPropertyDescriptor d : UserProperty.all()) { + if (getProperty(d.clazz) == null) { + UserProperty up = d.newInstance(this); + if (up != null) + properties.add(up); + } + } + } + + private void removeNullsThatFailedToLoad() { + properties.removeIf(Objects::isNull); + } + + private void loadFromUserConfigFile(String userId) { + XmlFile config = getConfigFile(); + try { + if ( config != null && config.exists()) { + config.unmarshal(this); + this.id = userId; + } + } catch (IOException e) { + LOGGER.log(Level.SEVERE, "Failed to load " + config, e); + } + } + + private void clearExistingProperties() { + properties.clear(); + } + + private XmlFile getConfigFile() { + File existingUserFolder = getExistingUserFolder(); + return existingUserFolder == null ? null : new XmlFile(XSTREAM, new File(existingUserFolder, CONFIG_XML)); } /** @@ -175,7 +249,7 @@ public class User extends AbstractModelObject implements AccessControlled, Descr */ @Nonnull public static IdStrategy idStrategy() { - Jenkins j = Jenkins.getInstance(); + Jenkins j = Jenkins.get(); SecurityRealm realm = j.getSecurityRealm(); if (realm == null) { return IdStrategy.CASE_INSENSITIVE; @@ -183,71 +257,41 @@ public class User extends AbstractModelObject implements AccessControlled, Descr return realm.getUserIdStrategy(); } - public int compareTo(User that) { + public int compareTo(@Nonnull User that) { return idStrategy().compare(this.id, that.id); } - /** - * Loads the other data from disk if it's available. - */ - private synchronized void load() { - properties.clear(); - - XmlFile config = getConfigFile(); - try { - if(config.exists()) - config.unmarshal(this); - } catch (IOException e) { - LOGGER.log(Level.SEVERE, "Failed to load "+config,e); - } - - // remove nulls that have failed to load - for (Iterator itr = properties.iterator(); itr.hasNext();) { - if(itr.next()==null) - itr.remove(); - } - - // allocate default instances if needed. - // doing so after load makes sure that newly added user properties do get reflected - for (UserPropertyDescriptor d : UserProperty.all()) { - if(getProperty(d.clazz)==null) { - UserProperty up = d.newInstance(this); - if(up!=null) - properties.add(up); - } - } - - for (UserProperty p : properties) - p.setUser(this); - } - @Exported public String getId() { return id; } - public @Nonnull String getUrl() { - return "user/"+Util.rawEncode(idStrategy().keyFor(id)); + public @Nonnull + String getUrl() { + return "user/" + Util.rawEncode(idStrategy().keyFor(id)); } - public @Nonnull String getSearchUrl() { - return "/user/"+Util.rawEncode(idStrategy().keyFor(id)); + public @Nonnull + String getSearchUrl() { + return "/user/" + Util.rawEncode(idStrategy().keyFor(id)); } /** * The URL of the user page. */ - @Exported(visibility=999) - public @Nonnull String getAbsoluteUrl() { - return Jenkins.getInstance().getRootUrl()+getUrl(); + @Exported(visibility = 999) + public @Nonnull + String getAbsoluteUrl() { + return Jenkins.get().getRootUrl() + getUrl(); } /** * Gets the human readable name of this user. * This is configurable by the user. */ - @Exported(visibility=999) - public @Nonnull String getFullName() { + @Exported(visibility = 999) + public @Nonnull + String getFullName() { return fullName; } @@ -256,18 +300,19 @@ public class User extends AbstractModelObject implements AccessControlled, Descr * If the input parameter is empty, the user's ID will be set. */ public void setFullName(String name) { - if(Util.fixEmptyAndTrim(name)==null) name=id; + if (Util.fixEmptyAndTrim(name) == null) name = id; this.fullName = name; } @Exported - public @CheckForNull String getDescription() { + public @CheckForNull + String getDescription() { return description; } - /** * Sets the description of the user. + * * @since 1.609 */ public void setDescription(String description) { @@ -277,7 +322,7 @@ public class User extends AbstractModelObject implements AccessControlled, Descr /** * Gets the user properties configured for this user. */ - public Map,UserProperty> getProperties() { + public Map, UserProperty> getProperties() { return Descriptor.toMap(properties); } @@ -286,8 +331,8 @@ public class User extends AbstractModelObject implements AccessControlled, Descr */ public synchronized void addProperty(@Nonnull UserProperty p) throws IOException { UserProperty old = getProperty(p.getClass()); - List ps = new ArrayList(properties); - if(old!=null) + List ps = new ArrayList<>(properties); + if (old != null) ps.remove(old); ps.add(p); p.setUser(this); @@ -298,7 +343,7 @@ public class User extends AbstractModelObject implements AccessControlled, Descr /** * List of all {@link UserProperty}s exposed primarily for the remoting API. */ - @Exported(name="property",inline=true) + @Exported(name = "property", inline = true) public List getAllProperties() { if (hasPermission(Jenkins.ADMINISTER)) { return Collections.unmodifiableList(properties); @@ -306,13 +351,13 @@ public class User extends AbstractModelObject implements AccessControlled, Descr return Collections.emptyList(); } - + /** * Gets the specific property, or null. */ public T getProperty(Class clazz) { for (UserProperty p : properties) { - if(clazz.isInstance(p)) + if (clazz.isInstance(p)) return clazz.cast(p); } return null; @@ -320,20 +365,20 @@ public class User extends AbstractModelObject implements AccessControlled, Descr /** * Creates an {@link Authentication} object that represents this user. - * + *

* This method checks with {@link SecurityRealm} if the user is a valid user that can login to the security realm. * If {@link SecurityRealm} is a kind that does not support querying information about other users, this will * use {@link LastGrantedAuthoritiesProperty} to pick up the granted authorities as of the last time the user has * logged in. * - * @throws UsernameNotFoundException - * If this user is not a valid user in the backend {@link SecurityRealm}. + * @throws UsernameNotFoundException If this user is not a valid user in the backend {@link SecurityRealm}. * @since 1.419 */ - public @Nonnull Authentication impersonate() throws UsernameNotFoundException { + public @Nonnull + Authentication impersonate() throws UsernameNotFoundException { return this.impersonate(this.getUserDetailsForImpersonation()); } - + /** * This method checks with {@link SecurityRealm} if the user is a valid user that can login to the security realm. * If {@link SecurityRealm} is a kind that does not support querying information about other users, this will @@ -341,42 +386,39 @@ public class User extends AbstractModelObject implements AccessControlled, Descr * logged in. * * @return userDetails for the user, in case he's not found but seems legitimate, we provide a userDetails with minimum access - * - * @throws UsernameNotFoundException - * If this user is not a valid user in the backend {@link SecurityRealm}. + * @throws UsernameNotFoundException If this user is not a valid user in the backend {@link SecurityRealm}. */ - public @Nonnull UserDetails getUserDetailsForImpersonation() throws UsernameNotFoundException { + public @Nonnull + UserDetails getUserDetailsForImpersonation() throws UsernameNotFoundException { ImpersonatingUserDetailsService userDetailsService = new ImpersonatingUserDetailsService( - Jenkins.getInstance().getSecurityRealm().getSecurityComponents().userDetails + Jenkins.get().getSecurityRealm().getSecurityComponents().userDetails ); - + try { UserDetails userDetails = userDetailsService.loadUserByUsername(id); - LOGGER.log(Level.FINE, "Impersonation of the user {0} was a success", new Object[]{ id }); + LOGGER.log(Level.FINE, "Impersonation of the user {0} was a success", id); return userDetails; } catch (UserMayOrMayNotExistException e) { - LOGGER.log(Level.FINE, "The user {0} may or may not exist in the SecurityRealm, so we provide minimum access", new Object[]{ id }); - // backend can't load information about other users. so use the stored information if available + LOGGER.log(Level.FINE, "The user {0} may or may not exist in the SecurityRealm, so we provide minimum access", id); } catch (UsernameNotFoundException e) { - // if the user no longer exists in the backend, we need to refuse impersonating this user - if(ALLOW_NON_EXISTENT_USER_TO_LOGIN){ - LOGGER.log(Level.FINE, "The user {0} was not found in the SecurityRealm but we are required to let it pass, due to ALLOW_NON_EXISTENT_USER_TO_LOGIN", new Object[]{ id }); - }else{ - LOGGER.log(Level.FINE, "The user {0} was not found in the SecurityRealm", new Object[]{ id }); + if (ALLOW_NON_EXISTENT_USER_TO_LOGIN) { + LOGGER.log(Level.FINE, "The user {0} was not found in the SecurityRealm but we are required to let it pass, due to ALLOW_NON_EXISTENT_USER_TO_LOGIN", id); + } else { + LOGGER.log(Level.FINE, "The user {0} was not found in the SecurityRealm", id); throw e; } } catch (DataAccessException e) { // seems like it's in the same boat as UserMayOrMayNotExistException - LOGGER.log(Level.FINE, "The user {0} retrieval just threw a DataAccess exception with msg = {1}, so we provide minimum access", new Object[]{ id, e.getMessage() }); + LOGGER.log(Level.FINE, "The user {0} retrieval just threw a DataAccess exception with msg = {1}, so we provide minimum access", new Object[]{id, e.getMessage()}); } - + return new LegitimateButUnknownUserDetails(id); } /** * Only used for a legitimate user we have no idea about. We give it only minimum access */ - private static class LegitimateButUnknownUserDetails extends org.acegisecurity.userdetails.User{ + private static class LegitimateButUnknownUserDetails extends org.acegisecurity.userdetails.User { private LegitimateButUnknownUserDetails(String username) throws IllegalArgumentException { super( username, "", @@ -393,7 +435,8 @@ public class User extends AbstractModelObject implements AccessControlled, Descr * @see #getUserDetailsForImpersonation() */ @Restricted(NoExternalUse.class) - public @Nonnull Authentication impersonate(@Nonnull UserDetails userDetails) { + public @Nonnull + Authentication impersonate(@Nonnull UserDetails userDetails) { return new UsernamePasswordAuthenticationToken(userDetails.getUsername(), "", userDetails.getAuthorities()); } @@ -401,12 +444,12 @@ public class User extends AbstractModelObject implements AccessControlled, Descr * Accepts the new description. */ @RequirePOST - public synchronized void doSubmitDescription( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException { + public void doSubmitDescription(StaplerRequest req, StaplerResponse rsp) throws IOException { checkPermission(Jenkins.ADMINISTER); description = req.getParameter("description"); save(); - + rsp.sendRedirect("."); // go to the top page } @@ -422,177 +465,77 @@ public class User extends AbstractModelObject implements AccessControlled, Descr /** * Gets the {@link User} object by its id or full name. * - * @param create - * If true, this method will never return null for valid input - * (by creating a new {@link User} object if none exists.) - * If false, this method will return null if {@link User} object - * with the given name doesn't exist. + * @param create If true, this method will never return null for valid input + * (by creating a new {@link User} object if none exists.) + * If false, this method will return null if {@link User} object + * with the given name doesn't exist. * @return Requested user. May be {@code null} if a user does not exist and - * {@code create} is false. + * {@code create} is false. * @deprecated use {@link User#get(String, boolean, java.util.Map)} */ @Deprecated - public static @Nullable User get(String idOrFullName, boolean create) { + public static @Nullable + User get(String idOrFullName, boolean create) { return get(idOrFullName, create, Collections.emptyMap()); } /** * Gets the {@link User} object by its id or full name. - * + *

* In order to resolve the user ID, the method invokes {@link CanonicalIdResolver} extension points. * Note that it may cause significant performance degradation. * If you are sure the passed value is a User ID, it is recommended to use {@link #getById(String, boolean)}. * - * @param create - * If true, this method will never return null for valid input - * (by creating a new {@link User} object if none exists.) - * If false, this method will return null if {@link User} object - * with the given name doesn't exist. - * - * @param context - * contextual environment this user idOfFullName was retrieved from, - * that can help resolve the user ID - * - * @return - * An existing or created user. May be {@code null} if a user does not exist and - * {@code create} is false. + * @param create If true, this method will never return null for valid input + * (by creating a new {@link User} object if none exists.) + * If false, this method will return null if {@link User} object + * with the given name doesn't exist. + * @param context contextual environment this user idOfFullName was retrieved from, + * that can help resolve the user ID + * @return An existing or created user. May be {@code null} if a user does not exist and + * {@code create} is false. */ - public static @Nullable User get(String idOrFullName, boolean create, @Nonnull Map context) { - - if(idOrFullName==null) + public static @Nullable + User get(String idOrFullName, boolean create, @Nonnull Map context) { + if (idOrFullName == null) { return null; + } - // TODO: In many cases the method should receive the canonical ID. - // Maybe it makes sense to try AllUsers.byName().get(idkey) before invoking all resolvers and other stuff - // oleg-nenashev: FullNameResolver with User.getAll() loading and iteration makes me think it's a good idea. + User user = AllUsers.get(idOrFullName); + if (user != null) { + return user; + } String id = CanonicalIdResolver.resolve(idOrFullName, context); - // DefaultUserCanonicalIdResolver will always return a non-null id if all other CanonicalIdResolver failed - if (id == null) { - throw new IllegalStateException("The user id should be always non-null thanks to DefaultUserCanonicalIdResolver"); - } - return getOrCreate(id, idOrFullName, create); + return getOrCreateById(id, idOrFullName, create); } /** * Retrieve a user by its ID, and create a new one if requested. - * @return - * An existing or created user. May be {@code null} if a user does not exist and - * {@code create} is false. + * + * @return An existing or created user. May be {@code null} if a user does not exist and + * {@code create} is false. */ - private static @Nullable User getOrCreate(@Nonnull String id, @Nonnull String fullName, boolean create) { - return getOrCreate(id, fullName, create, getUnsanitizedLegacyConfigFileFor(id)); - } - - private static @Nullable User getOrCreate(@Nonnull String id, @Nonnull String fullName, boolean create, File unsanitizedLegacyConfigFile) { - String idkey = idStrategy().keyFor(id); - - byNameLock.readLock().lock(); - User u; - try { - u = AllUsers.byName().get(idkey); - } finally { - byNameLock.readLock().unlock(); - } - final File configFile = getConfigFileFor(id); - boolean mustMigrateLegacyConfig = isMigrationRequiredForLegacyConfigFile(unsanitizedLegacyConfigFile, configFile); - if (mustMigrateLegacyConfig) { - File ancestor = unsanitizedLegacyConfigFile.getParentFile(); - if (!configFile.exists()) { - try { - Files.createDirectory(configFile.getParentFile().toPath()); - Files.move(unsanitizedLegacyConfigFile.toPath(), configFile.toPath()); - LOGGER.log(Level.INFO, "Migrated user record from {0} to {1}", new Object[] {unsanitizedLegacyConfigFile, configFile}); - } catch (IOException | InvalidPathException e) { - LOGGER.log( - Level.WARNING, - String.format("Failed to migrate user record from %s to %s", unsanitizedLegacyConfigFile, configFile), - e); - } - } - - // Don't clean up ancestors with other children; the directories should be cleaned up when the last child - // is migrated - File tmp = ancestor; - try { - while (!ancestor.equals(getRootDir())) { - try (DirectoryStream stream = Files.newDirectoryStream(ancestor.toPath())) { - if (!stream.iterator().hasNext()) { - tmp = ancestor; - ancestor = tmp.getParentFile(); - Files.deleteIfExists(tmp.toPath()); - } else { - break; - } - } - } - } catch (IOException | InvalidPathException e) { - if (LOGGER.isLoggable(Level.FINE)) { - LOGGER.log(Level.FINE, "Could not delete " + tmp + " when cleaning up legacy user directories", e); - } - } - } - - if (u==null && (create || configFile.exists())) { - User tmp = new User(id, fullName); - User prev; - byNameLock.readLock().lock(); - try { - prev = AllUsers.byName().putIfAbsent(idkey, u = tmp); - } finally { - byNameLock.readLock().unlock(); - } - if (prev != null) { - u = prev; // if some has already put a value in the map, use it - if (LOGGER.isLoggable(Level.FINE) && !fullName.equals(prev.getFullName())) { - LOGGER.log(Level.FINE, "mismatch on fullName (‘" + fullName + "’ vs. ‘" + prev.getFullName() + "’) for ‘" + id + "’", new Throwable()); - } - } else if (!id.equals(fullName) && !configFile.exists()) { - // JENKINS-16332: since the fullName may not be recoverable from the id, and various code may store the id only, we must save the fullName + private static @Nullable + User getOrCreateById(@Nonnull String id, @Nonnull String fullName, boolean create) { + User u = AllUsers.get(id); + if (u == null && (create || UserIdMapper.getInstance().isMapped(id))) { + u = new User(id, fullName); + AllUsers.put(id, u); + if (!id.equals(fullName) && !UserIdMapper.getInstance().isMapped(id)) { try { u.save(); } catch (IOException x) { - LOGGER.log(Level.WARNING, null, x); + LOGGER.log(Level.WARNING, "Failed to save user configuration for " + id, x); } } } return u; } - - private static boolean isMigrationRequiredForLegacyConfigFile(@Nonnull File legacyConfigFile, @Nonnull File newConfigFile){ - boolean mustMigrateLegacyConfig = legacyConfigFile.exists() && !legacyConfigFile.equals(newConfigFile); - if(mustMigrateLegacyConfig){ - try{ - // TODO Could be replace by Util.isDescendant(getRootDir(), legacyConfigFile) in 2.80+ - String canonicalLegacy = legacyConfigFile.getCanonicalPath(); - String canonicalUserDir = getRootDir().getCanonicalPath(); - if(!canonicalLegacy.startsWith(canonicalUserDir + File.separator)){ - // without that check, the application config.xml could be moved (i.e. erased from application PoV) - mustMigrateLegacyConfig = false; - LOGGER.log(Level.WARNING, String.format( - "Attempt to escape from users directory with %s, migration aborted, see SECURITY-897 for more information", - legacyConfigFile.getAbsolutePath() - )); - } - } - catch (IOException e){ - mustMigrateLegacyConfig = false; - LOGGER.log( - Level.WARNING, - String.format( - "Failed to determine the canonical path of %s, migration aborted, see SECURITY-897 for more information", - legacyConfigFile.getAbsolutePath() - ), - e - ); - } - } - return mustMigrateLegacyConfig; - } /** * Gets the {@link User} object by its id or full name. - * + *

* Creates a user on-demand. * *

@@ -602,18 +545,19 @@ public class User extends AbstractModelObject implements AccessControlled, Descr * This is slow (see JENKINS-23281). * * @deprecated This method is deprecated, because it causes unexpected {@link User} creation - * by API usage code and causes performance degradation of used to retrieve users by ID. - * Use {@link #getById} when you know you have an ID. - * Otherwise use {@link #getOrCreateByIdOrFullName(String)} or {@link #get(String, boolean, Map)}. + * by API usage code and causes performance degradation of used to retrieve users by ID. + * Use {@link #getById} when you know you have an ID. + * Otherwise use {@link #getOrCreateByIdOrFullName(String)} or {@link #get(String, boolean, Map)}. */ @Deprecated - public static @Nonnull User get(String idOrFullName) { + public static @Nonnull + User get(String idOrFullName) { return getOrCreateByIdOrFullName(idOrFullName); } /** * Get the user by ID or Full Name. - * + *

* If the user does not exist, creates a new one on-demand. * *

@@ -627,137 +571,105 @@ public class User extends AbstractModelObject implements AccessControlled, Descr * @since 2.91 */ public static @Nonnull User getOrCreateByIdOrFullName(@Nonnull String idOrFullName) { - return get(idOrFullName,true, Collections.emptyMap()); + return get(idOrFullName, true, Collections.emptyMap()); } /** * Gets the {@link User} object representing the currently logged-in user, or null * if the current user is anonymous. + * * @since 1.172 */ - public static @CheckForNull User current() { + public static @CheckForNull + User current() { return get(Jenkins.getAuthentication()); } /** * Gets the {@link User} object representing the supplied {@link Authentication} or * {@code null} if the supplied {@link Authentication} is either anonymous or {@code null} + * * @param a the supplied {@link Authentication} . * @return a {@link User} object for the supplied {@link Authentication} or {@code null} * @since 1.609 */ - public static @CheckForNull User get(@CheckForNull Authentication a) { - if(a == null || a instanceof AnonymousAuthenticationToken) + public static @CheckForNull + User get(@CheckForNull Authentication a) { + if (a == null || a instanceof AnonymousAuthenticationToken) return null; - // Since we already know this is a name, we can just call getOrCreate with the name directly. - String id = a.getName(); - return getById(id, true); + // Since we already know this is a name, we can just call getOrCreateById with the name directly. + return getById(a.getName(), true); } /** * Gets the {@link User} object by its id * - * @param id - * the id of the user to retrieve and optionally create if it does not exist. - * @param create - * If true, this method will never return null for valid input (by creating a - * new {@link User} object if none exists.) If false, this method will return - * null if {@link User} object with the given id doesn't exist. + * @param id the id of the user to retrieve and optionally create if it does not exist. + * @param create If true, this method will never return null for valid input (by creating a + * new {@link User} object if none exists.) If false, this method will return + * null if {@link User} object with the given id doesn't exist. * @return the a User whose id is id, or null if create is false - * and the user does not exist. + * and the user does not exist. * @since 1.651.2 / 2.3 */ - public static @Nullable User getById(String id, boolean create) { - return getOrCreate(id, id, create); + public static @Nullable + User getById(String id, boolean create) { + return getOrCreateById(id, id, create); } /** * Gets all the users. */ - public static @Nonnull Collection getAll() { + public static @Nonnull + Collection getAll() { final IdStrategy strategy = idStrategy(); - byNameLock.readLock().lock(); - ArrayList r; - try { - r = new ArrayList(AllUsers.byName().values()); - } finally { - byNameLock.readLock().unlock(); - } - Collections.sort(r,new Comparator() { - - public int compare(User o1, User o2) { - return strategy.compare(o1.getId(), o2.getId()); - } - }); - return r; + ArrayList users = new ArrayList<>(AllUsers.values()); + users.sort((o1, o2) -> strategy.compare(o1.getId(), o2.getId())); + return users; } /** * To be called from {@link Jenkins#reload} only. */ @Restricted(NoExternalUse.class) - public static void reload() { - byNameLock.readLock().lock(); - try { - AllUsers.byName().clear(); - } finally { - byNameLock.readLock().unlock(); - } - UserDetailsCache.get().invalidateAll(); - AllUsers.scanAll(); - } - - /** - * @deprecated Used to be called by test harnesses; now ignored in that case. - */ - @Deprecated - public static void clear() { - if (ExtensionList.lookup(AllUsers.class).isEmpty()) { - // Historically this was called by JenkinsRule prior to startup. Ignore! - return; - } - byNameLock.writeLock().lock(); - try { - AllUsers.byName().clear(); - } finally { - byNameLock.writeLock().unlock(); - } + public static void reload() throws IOException { + UserIdMapper.getInstance().reload(); + AllUsers.reload(); } /** * Called when changing the {@link IdStrategy}. + * * @since 1.566 */ public static void rekey() { - final IdStrategy strategy = idStrategy(); - byNameLock.writeLock().lock(); + /* There are many and varied ways in which this could cause erratic or + problematic behavior. Such changes should really only occur during initial + setup and under very controlled situations. After this sort of a change + the whole webapp should restart. It's possible that this rekeying, + or greater issues in the realm change, could affect currently logged + in users and even the user making the change. */ try { - ConcurrentMap byName = AllUsers.byName(); - for (Map.Entry e : byName.entrySet()) { - String idkey = strategy.keyFor(e.getValue().id); - if (!idkey.equals(e.getKey())) { - // need to remap - byName.remove(e.getKey()); - byName.putIfAbsent(idkey, e.getValue()); - } - } - } finally { - byNameLock.writeLock().unlock(); - UserDetailsCache.get().invalidateAll(); + reload(); + } catch (IOException e) { + LOGGER.log(Level.SEVERE, "Failed to perform rekey operation.", e); } } /** * Returns the user name. */ - public @Nonnull String getDisplayName() { + public @Nonnull + String getDisplayName() { return getFullName(); } - /** true if {@link AbstractBuild#hasParticipant} or {@link hudson.model.Cause.UserIdCause} */ - private boolean relatedTo(@Nonnull AbstractBuild b) { + /** + * true if {@link AbstractBuild#hasParticipant} or {@link hudson.model.Cause.UserIdCause} + */ + private boolean relatedTo(@Nonnull AbstractBuild b) { if (b.hasParticipant(this)) { return true; } @@ -778,50 +690,64 @@ public class User extends AbstractModelObject implements AccessControlled, Descr */ @SuppressWarnings("unchecked") @WithBridgeMethods(List.class) - public @Nonnull RunList getBuilds() { - return RunList.fromJobs((Iterable)Jenkins.getInstance().allItems(Job.class)).filter(new Predicate>() { - @Override public boolean apply(Run r) { - return r instanceof AbstractBuild && relatedTo((AbstractBuild) r); - } - }); + public @Nonnull + RunList getBuilds() { + return RunList.fromJobs((Iterable) Jenkins.get(). + allItems(Job.class)).filter((Predicate>) r -> r instanceof AbstractBuild && relatedTo((AbstractBuild) r)); } /** * Gets all the {@link AbstractProject}s that this user has committed to. + * * @since 1.191 */ - public @Nonnull Set> getProjects() { - Set> r = new HashSet>(); - for (AbstractProject p : Jenkins.getInstance().allItems(AbstractProject.class)) - if(p.hasParticipant(this)) + public @Nonnull + Set> getProjects() { + Set> r = new HashSet<>(); + for (AbstractProject p : Jenkins.get().allItems(AbstractProject.class)) + if (p.hasParticipant(this)) r.add(p); return r; } - public @Override String toString() { + public @Override + String toString() { return fullName; } /** - * The file we save our configuration. + * Called by tests in the JTH. Otherwise this shouldn't be called. + * Even in the tests this usage is questionable. */ - protected final XmlFile getConfigFile() { - return new XmlFile(XSTREAM,getConfigFileFor(id)); + @Deprecated + public static void clear() { + if (ExtensionList.lookup(AllUsers.class).isEmpty()) { + return; + } + UserIdMapper.getInstance().clear(); + AllUsers.clear(); } - private static final File getConfigFileFor(String id) { - return new File(getRootDir(), idStrategy().filenameOf(id) +"/config.xml"); + /** + * Returns the folder that store all the user information. + * Useful for plugins to save a user-specific file aside the config.xml. + * Exposes implementation details that may be subject to change. + * + * @since 2.129 + */ + public File getUserFolder() { + return getExistingUserFolder(); } - private static File getUnsanitizedLegacyConfigFileFor(String id) { - return new File(getRootDir(), idStrategy().legacyFilenameOf(id) + "/config.xml"); + private File getExistingUserFolder() { + return UserIdMapper.getInstance().getDirectory(id); } /** * Gets the directory where Hudson stores user information. */ - private static File getRootDir() { - return new File(Jenkins.getInstance().getRootDir(), "users"); + static File getRootDir() { + return new File(Jenkins.get().getRootDir(), "users"); } /** @@ -834,12 +760,11 @@ public class User extends AbstractModelObject implements AccessControlled, Descr * * @param id ID to be checked * @return {@code true} if the username or fullname is valid. - * For {@code null} or blank IDs returns {@code false}. + * For {@code null} or blank IDs returns {@code false}. * @since 1.600 */ public static boolean isIdOrFullnameAllowed(@CheckForNull String id) { - //TODO: StringUtils.isBlank() checks the null value, but FindBugs is not smart enough. Remove it later - if (id == null || StringUtils.isBlank(id)) { + if (StringUtils.isBlank(id)) { return false; } final String trimmedId = id.trim(); @@ -851,49 +776,49 @@ public class User extends AbstractModelObject implements AccessControlled, Descr } /** - * Save the settings to a file. + * Save the user configuration. */ - public synchronized void save() throws IOException, FormValidation { - if (! isIdOrFullnameAllowed(id)) { + public synchronized void save() throws IOException { + if (!isIdOrFullnameAllowed(id)) { throw FormValidation.error(Messages.User_IllegalUsername(id)); } - if (! isIdOrFullnameAllowed(fullName)) { + if (!isIdOrFullnameAllowed(fullName)) { throw FormValidation.error(Messages.User_IllegalFullname(fullName)); } - if(BulkChange.contains(this)) return; - getConfigFile().write(this); - SaveableListener.fireOnChange(this, getConfigFile()); + if (BulkChange.contains(this)) { + return; + } + XmlFile xmlFile = new XmlFile(XSTREAM, constructUserConfigFile()); + xmlFile.write(this); + SaveableListener.fireOnChange(this, xmlFile); } - private Object writeReplace() { - return XmlFile.replaceIfNotAtTopLevel(this, () -> new Replacer(this)); + private File constructUserConfigFile() throws IOException { + return new File(putUserFolderIfAbsent(), CONFIG_XML); } - private static class Replacer { - private final String id; - Replacer(User u) { - id = u.getId(); - } - private Object readResolve() { - return getById(id, false); - } + + private File putUserFolderIfAbsent() throws IOException { + return UserIdMapper.getInstance().putIfAbsent(id, true); } /** * Deletes the data directory and removes this user from Hudson. * - * @throws IOException - * if we fail to delete. + * @throws IOException if we fail to delete. */ - public synchronized void delete() throws IOException { - final IdStrategy strategy = idStrategy(); - byNameLock.readLock().lock(); - try { - AllUsers.byName().remove(strategy.keyFor(id)); - } finally { - byNameLock.readLock().unlock(); + public void delete() throws IOException { + String idKey = idStrategy().keyFor(id); + File existingUserFolder = getExistingUserFolder(); + UserIdMapper.getInstance().remove(id); + AllUsers.remove(id); + deleteExistingUserFolder(existingUserFolder); + UserDetailsCache.get().invalidate(idKey); + } + + private void deleteExistingUserFolder(File existingUserFolder) throws IOException { + if (existingUserFolder != null && existingUserFolder.exists()) { + Util.deleteRecursive(existingUserFolder); } - Util.deleteRecursive(new File(getRootDir(), strategy.filenameOf(id))); - UserDetailsCache.get().invalidate(strategy.keyFor(id)); } /** @@ -907,7 +832,7 @@ public class User extends AbstractModelObject implements AccessControlled, Descr * Accepts submission from the configuration page. */ @RequirePOST - public void doConfigSubmit( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException, FormException { + public void doConfigSubmit(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException, FormException { checkPermission(Jenkins.ADMINISTER); JSONObject json = req.getSubmittedForm(); @@ -915,13 +840,13 @@ public class User extends AbstractModelObject implements AccessControlled, Descr fullName = json.getString("fullName"); description = json.getString("description"); - List props = new ArrayList(); + List props = new ArrayList<>(); int i = 0; for (UserPropertyDescriptor d : UserProperty.all()) { UserProperty p = getProperty(d.clazz); JSONObject o = json.optJSONObject("userProperty" + (i++)); - if (o!=null) { + if (o != null) { if (p != null) { p = p.reconfigure(req, o); } else { @@ -930,7 +855,7 @@ public class User extends AbstractModelObject implements AccessControlled, Descr p.setUser(this); } - if (p!=null) + if (p != null) props.add(p); } this.properties = props; @@ -941,14 +866,14 @@ public class User extends AbstractModelObject implements AccessControlled, Descr UserDetailsCache.get().invalidate(oldFullName); } - FormApply.success(".").generateResponse(req,rsp,this); + FormApply.success(".").generateResponse(req, rsp, this); } /** * Deletes this user from Hudson. */ @RequirePOST - public void doDoDelete(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException { + public void doDoDelete(StaplerRequest req, StaplerResponse rsp) throws IOException { checkPermission(Jenkins.ADMINISTER); if (idStrategy().equals(id, Jenkins.getAuthentication().getName())) { rsp.sendError(HttpServletResponse.SC_BAD_REQUEST, "Cannot delete self"); @@ -969,9 +894,9 @@ public class User extends AbstractModelObject implements AccessControlled, Descr } public void doRssLatest(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException { - final List lastBuilds = new ArrayList(); - for (AbstractProject p : Jenkins.getInstance().allItems(AbstractProject.class)) { - for (AbstractBuild b = p.getLastBuild(); b != null; b = b.getPreviousBuild()) { + final List lastBuilds = new ArrayList<>(); + for (AbstractProject p : Jenkins.get().allItems(AbstractProject.class)) { + for (AbstractBuild b = p.getLastBuild(); b != null; b = b.getPreviousBuild()) { if (relatedTo(b)) { lastBuilds.add(b); break; @@ -980,44 +905,22 @@ public class User extends AbstractModelObject implements AccessControlled, Descr } // historically these have been reported sorted by project name, we switched to the lazy iteration // so we only have to sort the sublist of runs rather than the full list of irrelevant projects - Collections.sort(lastBuilds, new Comparator() { - @Override - public int compare(Run o1, Run o2) { - return Items.BY_FULL_NAME.compare(o1.getParent(), o2.getParent()); - } - }); + lastBuilds.sort((o1, o2) -> Items.BY_FULL_NAME.compare(o1.getParent(), o2.getParent())); rss(req, rsp, " latest build", RunList.fromRuns(lastBuilds), Run.FEED_ADAPTER_LATEST); } private void rss(StaplerRequest req, StaplerResponse rsp, String suffix, RunList runs, FeedAdapter adapter) throws IOException, ServletException { - RSS.forwardToRss(getDisplayName()+ suffix, getUrl(), runs.newBuilds(), adapter, req, rsp); - } - - /** - * This lock is used to guard access to the {@link AllUsers#byName} map. Use - * {@link java.util.concurrent.locks.ReadWriteLock#readLock()} for normal access and - * {@link java.util.concurrent.locks.ReadWriteLock#writeLock()} for {@link #rekey()} or any other operation - * that requires operating on the map as a whole. - */ - private static final ReadWriteLock byNameLock = new ReentrantReadWriteLock(); - - /** - * Used to load/save user configuration. - */ - public static final XStream2 XSTREAM = new XStream2(); - - private static final Logger LOGGER = Logger.getLogger(User.class.getName()); - - static { - XSTREAM.alias("user",User.class); + RSS.forwardToRss(getDisplayName() + suffix, getUrl(), runs.newBuilds(), adapter, req, rsp); } + @Override + @Nonnull public ACL getACL() { - ACL base = Jenkins.getInstance().getAuthorizationStrategy().getACL(this); + ACL base = Jenkins.get().getAuthorizationStrategy().getACL(this); // always allow a non-anonymous user full control of himself. return ACL.lambda((a, permission) -> (idStrategy().equals(a.getName(), id) && !(a instanceof AnonymousAuthenticationToken)) - || base.hasPermission(a, permission)); + || base.hasPermission(a, permission)); } /** @@ -1026,21 +929,23 @@ public class User extends AbstractModelObject implements AccessControlled, Descr public boolean canDelete() { final IdStrategy strategy = idStrategy(); return hasPermission(Jenkins.ADMINISTER) && !strategy.equals(id, Jenkins.getAuthentication().getName()) - && new File(getRootDir(), strategy.filenameOf(id)).exists(); + && UserIdMapper.getInstance().isMapped(id); } /** * Checks for authorities (groups) associated with this user. * If the caller lacks {@link Jenkins#ADMINISTER}, or any problems arise, returns an empty list. * {@link SecurityRealm#AUTHENTICATED_AUTHORITY} and the username, if present, are omitted. - * @since 1.498 + * * @return a possibly empty list + * @since 1.498 */ - public @Nonnull List getAuthorities() { - if (!Jenkins.getInstance().hasPermission(Jenkins.ADMINISTER)) { + public @Nonnull + List getAuthorities() { + if (!Jenkins.get().hasPermission(Jenkins.ADMINISTER)) { return Collections.emptyList(); } - List r = new ArrayList(); + List r = new ArrayList<>(); Authentication authentication; try { authentication = impersonate(); @@ -1057,29 +962,29 @@ public class User extends AbstractModelObject implements AccessControlled, Descr r.add(n); } } - Collections.sort(r, String.CASE_INSENSITIVE_ORDER); + r.sort(String.CASE_INSENSITIVE_ORDER); return r; } public Object getDynamic(String token) { - for(Action action: getTransientActions()){ - if(Objects.equals(action.getUrlName(), token)) + for (Action action : getTransientActions()) { + if (Objects.equals(action.getUrlName(), token)) return action; } - for(Action action: getPropertyActions()){ - if(Objects.equals(action.getUrlName(), token)) + for (Action action : getPropertyActions()) { + if (Objects.equals(action.getUrlName(), token)) return action; } return null; } - + /** * Return all properties that are also actions. - * + * * @return the list can be empty but never null. read only. */ public List getPropertyActions() { - List actions = new ArrayList(); + List actions = new ArrayList<>(); for (UserProperty userProp : getProperties().values()) { if (userProp instanceof Action) { actions.add((Action) userProp); @@ -1087,29 +992,29 @@ public class User extends AbstractModelObject implements AccessControlled, Descr } return Collections.unmodifiableList(actions); } - + /** * Return all transient actions associated with this user. - * + * * @return the list can be empty but never null. read only. */ public List getTransientActions() { - List actions = new ArrayList(); - for (TransientUserActionFactory factory: TransientUserActionFactory.all()) { + List actions = new ArrayList<>(); + for (TransientUserActionFactory factory : TransientUserActionFactory.all()) { actions.addAll(factory.createFor(this)); } return Collections.unmodifiableList(actions); } public ContextMenu doContextMenu(StaplerRequest request, StaplerResponse response) throws Exception { - return new ContextMenu().from(this,request,response); + return new ContextMenu().from(this, request, response); } @Override @Restricted(NoExternalUse.class) public Object getTarget() { if (!SKIP_PERMISSION_CHECK) { - if (!Jenkins.getInstance().hasPermission(Jenkins.READ)) { + if (!Jenkins.get().hasPermission(Jenkins.READ)) { // work around Stapler bug when returning null from getTarget() try { Stapler.getCurrentResponse().sendError(SC_NOT_FOUND); // send same response body Stapler would send @@ -1121,74 +1026,96 @@ public class User extends AbstractModelObject implements AccessControlled, Descr return this; } - /** - * Escape hatch for StaplerProxy-based access control - */ - @Restricted(NoExternalUse.class) - public static /* Script Console modifiable */ boolean SKIP_PERMISSION_CHECK = Boolean.getBoolean(User.class.getName() + ".skipPermissionCheck"); - - /** * Gets list of Illegal usernames, for which users should not be created. * Always includes users from {@link #ILLEGAL_PERSISTED_USERNAMES} + * * @return List of usernames */ @Restricted(NoExternalUse.class) /*package*/ static Set getIllegalPersistedUsernames() { - // TODO: This method is designed for further extensibility via system properties. To be extended in a follow-up issue - final Set res = new HashSet<>(); - res.addAll(Arrays.asList(ILLEGAL_PERSISTED_USERNAMES)); - return res; + return new HashSet<>(Arrays.asList(ILLEGAL_PERSISTED_USERNAMES)); } - /** Per-{@link Jenkins} holder of all known {@link User}s. */ + private Object writeReplace() { + return XmlFile.replaceIfNotAtTopLevel(this, () -> new Replacer(this)); + } + + private static class Replacer { + private final String id; + + Replacer(User u) { + id = u.getId(); + } + + private Object readResolve() { + return getById(id, false); + } + } + + /** + * Per-{@link Jenkins} holder of all known {@link User}s. + */ @Extension @Restricted(NoExternalUse.class) public static final class AllUsers { - @Initializer(after = InitMilestone.JOB_LOADED) // so Jenkins.loadConfig has been called + private final ConcurrentMap byName = new ConcurrentHashMap<>(); + + @Initializer(after = InitMilestone.JOB_LOADED) public static void scanAll() { - IdStrategy strategy = idStrategy(); - File[] subdirs = getRootDir().listFiles((FileFilter) DirectoryFileFilter.INSTANCE); - if (subdirs != null) { - for (File subdir : subdirs) { - if (subdir.equals(getRootDir())) { - continue; // ignore the parent directory in case of stray config.xml - } - File configFile = new File(subdir, "config.xml"); - if (configFile.exists()) { - String name = strategy.idFromFilename(subdir.getName()); - getOrCreate(name, /* calls load(), probably clobbering this anyway */name, true, configFile); - } - } + for (String userId : UserIdMapper.getInstance().getConvertedUserIds()) { + User user = new User(userId, userId); + getInstance().byName.putIfAbsent(idStrategy().keyFor(userId), user); } } - @GuardedBy("User.byNameLock") - private final ConcurrentMap byName = new ConcurrentHashMap(); - /** * Keyed by {@link User#id}. This map is used to ensure * singleton-per-id semantics of {@link User} objects. - * + *

* The key needs to be generated by {@link IdStrategy#keyFor(String)}. */ - @GuardedBy("User.byNameLock") - static ConcurrentMap byName() { - return ExtensionList.lookupSingleton(AllUsers.class).byName; + private static AllUsers getInstance() { + return ExtensionList.lookupSingleton(AllUsers.class); } + private static void reload() { + getInstance().byName.clear(); + UserDetailsCache.get().invalidateAll(); + scanAll(); + } + + private static void clear() { + getInstance().byName.clear(); + } + + private static void remove(String id) { + getInstance().byName.remove(idStrategy().keyFor(id)); + } + + private static User get(String id) { + return getInstance().byName.get(idStrategy().keyFor(id)); + } + + private static void put(String id, User user) { + getInstance().byName.putIfAbsent(idStrategy().keyFor(id), user); + } + + private static Collection values() { + return getInstance().byName.values(); + } } /** * Resolves User IDs by ID, full names or other strings. - * + *

* This extension point may be useful to map SCM user names to Jenkins {@link User} IDs. * Currently the extension point is used in {@link User#get(String, boolean, Map)}. * - * @since 1.479 * @see jenkins.model.DefaultUserCanonicalIdResolver * @see FullNameIdResolver + * @since 1.479 */ public static abstract class CanonicalIdResolver extends AbstractDescribableImpl implements ExtensionPoint, Comparable { @@ -1200,23 +1127,22 @@ public class User extends AbstractModelObject implements AccessControlled, Descr public static final String REALM = "realm"; @Override - public int compareTo(CanonicalIdResolver o) { + public int compareTo(@Nonnull CanonicalIdResolver o) { // reverse priority order - int i = getPriority(); - int j = o.getPriority(); - return i>j ? -1 : (i==j ? 0:1); + return Integer.compare(o.getPriority(), getPriority()); } /** * extract user ID from idOrFullName with help from contextual infos. * can return null if no user ID matched the input */ - public abstract @CheckForNull String resolveCanonicalId(String idOrFullName, @Nonnull Map context); + public abstract @CheckForNull + String resolveCanonicalId(String idOrFullName, Map context); /** * Gets priority of the resolver. * Higher priority means that it will be checked earlier. - * + *

* Overriding methods must not use {@link Integer#MIN_VALUE}, because it will cause collisions * with {@link jenkins.model.DefaultUserCanonicalIdResolver}. * @@ -1226,10 +1152,11 @@ public class User extends AbstractModelObject implements AccessControlled, Descr return 1; } - //TODO: It is too late to use Extension Point ordinals, right? //Such sorting and collection rebuild is not good for User#get(...) method performance. + /** * Gets all extension points, sorted by priority. + * * @return Sorted list of extension point implementations. * @since 2.93 */ @@ -1241,18 +1168,18 @@ public class User extends AbstractModelObject implements AccessControlled, Descr /** * Resolves users using all available {@link CanonicalIdResolver}s. + * * @param idOrFullName ID or full name of the user - * @param context Context + * @param context Context * @return Resolved User ID or {@code null} if the user ID cannot be resolved. * @since 2.93 */ @CheckForNull public static String resolve(@Nonnull String idOrFullName, @Nonnull Map context) { for (CanonicalIdResolver resolver : CanonicalIdResolver.all()) { - //TODO: add try/catch for Runtime exceptions? It should not happen now && it may cause performance degradation String id = resolver.resolveCanonicalId(idOrFullName, context); if (id != null) { - LOGGER.log(Level.FINE, "{0} mapped {1} to {2}", new Object[] {resolver, idOrFullName, id}); + LOGGER.log(Level.FINE, "{0} mapped {1} to {2}", new Object[]{resolver, idOrFullName, id}); return id; } } @@ -1267,7 +1194,8 @@ public class User extends AbstractModelObject implements AccessControlled, Descr /** * Resolve user ID from full name */ - @Extension @Symbol("fullName") + @Extension + @Symbol("fullName") public static class FullNameIdResolver extends CanonicalIdResolver { @Override @@ -1293,15 +1221,10 @@ public class User extends AbstractModelObject implements AccessControlled, Descr @Restricted(NoExternalUse.class) public static class UserIDCanonicalIdResolver extends User.CanonicalIdResolver { - private static /* not final */ boolean SECURITY_243_FULL_DEFENSE = + private static /* not final */ boolean SECURITY_243_FULL_DEFENSE = SystemProperties.getBoolean(User.class.getName() + ".SECURITY_243_FULL_DEFENSE", true); - private static final ThreadLocal resolving = new ThreadLocal() { - @Override - protected Boolean initialValue() { - return false; - } - }; + private static final ThreadLocal resolving = ThreadLocal.withInitial(() -> false); @Override public String resolveCanonicalId(String idOrFullName, Map context) { @@ -1335,30 +1258,4 @@ public class User extends AbstractModelObject implements AccessControlled, Descr } - /** - * Jenkins now refuses to let the user login if he/she doesn't exist in {@link SecurityRealm}, - * which was necessary to make sure users removed from the backend will get removed from the frontend. - *

- * Unfortunately this infringed some legitimate use cases of creating Jenkins-local users for - * automation purposes. This escape hatch switch can be enabled to resurrect that behaviour. - * - * JENKINS-22346. - */ - public static boolean ALLOW_NON_EXISTENT_USER_TO_LOGIN = SystemProperties.getBoolean(User.class.getName()+".allowNonExistentUserToLogin"); - - /** - * Jenkins historically created a (usually) ephemeral user record when an user with Overall/Administer permission - * accesses a /user/arbitraryName URL. - *

- * Unfortunately this constitutes a CSRF vulnerability, as malicious users can make admins create arbitrary numbers - * of ephemeral user records, so the behavior was changed in Jenkins 2.TODO / 2.32.2. - *

- * As some users may be relying on the previous behavior, setting this to true restores the previous behavior. This - * is not recommended. - * - * SECURITY-406. - */ - @Restricted(NoExternalUse.class) - public static boolean ALLOW_USER_CREATION_VIA_URL = SystemProperties.getBoolean(User.class.getName() + ".allowUserCreationViaUrl"); - } diff --git a/core/src/main/java/hudson/model/UserIdMapper.java b/core/src/main/java/hudson/model/UserIdMapper.java new file mode 100644 index 0000000000..d0e394f88b --- /dev/null +++ b/core/src/main/java/hudson/model/UserIdMapper.java @@ -0,0 +1,201 @@ +/* + * The MIT License + * + * Copyright 2018 CloudBees, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +package hudson.model; + +import hudson.Extension; +import hudson.ExtensionList; +import hudson.Util; +import hudson.XmlFile; +import hudson.init.InitMilestone; +import hudson.init.Initializer; +import hudson.util.XStream2; +import jenkins.model.IdStrategy; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.NoSuchFileException; +import java.nio.file.Path; +import java.util.Collections; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.logging.Level; +import java.util.logging.Logger; +import java.util.regex.Pattern; + +@Restricted(NoExternalUse.class) +@Extension +public class UserIdMapper { + + private static final XStream2 XSTREAM = new XStream2(); + static final String MAPPING_FILE = "users.xml"; + private static final Logger LOGGER = Logger.getLogger(UserIdMapper.class.getName()); + private static final int PREFIX_MAX = 15; + private static final Pattern PREFIX_PATTERN = Pattern.compile("[^A-Za-z0-9]"); + private final int version = 1; // Not currently used, but it may be helpful in the future to store a version. + + private transient File usersDirectory; + private Map idToDirectoryNameMap = new ConcurrentHashMap<>(); + + static UserIdMapper getInstance() { + return ExtensionList.lookupSingleton(UserIdMapper.class); + } + + public UserIdMapper() { + } + + @Initializer(after = InitMilestone.PLUGINS_STARTED, before = InitMilestone.JOB_LOADED) + public File init() throws IOException { + usersDirectory = createUsersDirectoryAsNeeded(); + load(); + return usersDirectory; + } + + File getDirectory(String userId) { + String directoryName = idToDirectoryNameMap.get(getIdStrategy().keyFor(userId)); + return directoryName == null ? null : new File(usersDirectory, directoryName); + } + + File putIfAbsent(String userId, boolean saveToDisk) throws IOException { + String idKey = getIdStrategy().keyFor(userId); + String directoryName = idToDirectoryNameMap.get(idKey); + File directory = null; + if (directoryName == null) { + synchronized (this) { + directoryName = idToDirectoryNameMap.get(idKey); + if (directoryName == null) { + directory = createDirectoryForNewUser(userId); + directoryName = directory.getName(); + idToDirectoryNameMap.put(idKey, directoryName); + if (saveToDisk) { + save(); + } + } + } + } + return directory == null ? new File(usersDirectory, directoryName) : directory; + } + + boolean isMapped(String userId) { + return idToDirectoryNameMap.containsKey(getIdStrategy().keyFor(userId)); + } + + Set getConvertedUserIds() { + return Collections.unmodifiableSet(idToDirectoryNameMap.keySet()); + } + + void remove(String userId) throws IOException { + idToDirectoryNameMap.remove(getIdStrategy().keyFor(userId)); + save(); + } + + void clear() { + idToDirectoryNameMap.clear(); + } + + void reload() throws IOException { + clear(); + load(); + } + + protected IdStrategy getIdStrategy() { + return User.idStrategy(); + } + + protected File getUsersDirectory() { + return User.getRootDir(); + } + + private XmlFile getXmlConfigFile() { + File file = getConfigFile(usersDirectory); + return new XmlFile(XSTREAM, file); + } + + static File getConfigFile(File usersDirectory) { + return new File(usersDirectory, MAPPING_FILE); + } + + private File createDirectoryForNewUser(String userId) throws IOException { + try { + Path tempDirectory = Files.createTempDirectory(Util.fileToPath(usersDirectory), generatePrefix(userId)); + return tempDirectory.toFile(); + } catch (IOException e) { + LOGGER.log(Level.SEVERE, "Error creating directory for user: " + userId, e); + throw e; + } + } + + private String generatePrefix(String userId) { + String fullPrefix = PREFIX_PATTERN.matcher(userId).replaceAll(""); + return fullPrefix.length() > PREFIX_MAX - 1 ? fullPrefix.substring(0, PREFIX_MAX - 1) + '_' : fullPrefix + '_'; + } + + private File createUsersDirectoryAsNeeded() throws IOException { + File usersDirectory = getUsersDirectory(); + if (!usersDirectory.exists()) { + try { + Files.createDirectory(usersDirectory.toPath()); + } catch (IOException e) { + LOGGER.log(Level.SEVERE, "Unable to create users directory: " + usersDirectory, e); + throw e; + } + } + return usersDirectory; + } + + synchronized void save() throws IOException { + try { + getXmlConfigFile().write(this); + } catch (IOException ioe) { + LOGGER.log(Level.WARNING, "Error saving userId mapping file.", ioe); + throw ioe; + } + } + + private void load() throws IOException { + UserIdMigrator migrator = new UserIdMigrator(usersDirectory, getIdStrategy()); + if (migrator.needsMigration()) { + try { + migrator.migrateUsers(this); + } catch (IOException ioe) { + LOGGER.log(Level.SEVERE, "Error migrating users.", ioe); + throw ioe; + } + } else { + XmlFile config = getXmlConfigFile(); + try { + config.unmarshal(this); + } catch (NoSuchFileException e) { + LOGGER.log(Level.FINE, "User id mapping file does not exist. It will be created when a user is saved."); + } catch (IOException e) { + LOGGER.log(Level.WARNING, "Failed to load " + config, e); + throw e; + } + } + } + +} diff --git a/core/src/main/java/hudson/model/UserIdMigrator.java b/core/src/main/java/hudson/model/UserIdMigrator.java new file mode 100644 index 0000000000..f5b6b2f43f --- /dev/null +++ b/core/src/main/java/hudson/model/UserIdMigrator.java @@ -0,0 +1,104 @@ +/* + * The MIT License + * + * Copyright (c) 2018 CloudBees, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +package hudson.model; + +import jenkins.model.IdStrategy; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; + +import java.io.File; +import java.io.IOException; +import java.nio.file.CopyOption; +import java.nio.file.Files; +import java.nio.file.StandardCopyOption; +import java.util.HashMap; +import java.util.Map; +import java.util.logging.Level; +import java.util.logging.Logger; + +@Restricted(NoExternalUse.class) +class UserIdMigrator { + + private static final Logger LOGGER = Logger.getLogger(UserIdMigrator.class.getName()); + private static final String EMPTY_USERNAME_DIRECTORY_NAME = "emptyUsername"; + + private final File usersDirectory; + private final IdStrategy idStrategy; + + UserIdMigrator(File usersDirectory, IdStrategy idStrategy) { + this.usersDirectory = usersDirectory; + this.idStrategy = idStrategy; + } + + boolean needsMigration() { + File mappingFile = UserIdMapper.getConfigFile(usersDirectory); + if (mappingFile.exists() && mappingFile.isFile()) { + LOGGER.finest("User mapping file already exists. No migration needed."); + return false; + } + File[] userDirectories = listUserDirectories(); + return userDirectories != null && userDirectories.length > 0; + } + + private File[] listUserDirectories() { + return usersDirectory.listFiles(file -> file.isDirectory() && new File(file, User.CONFIG_XML).exists()); + } + + Map scanExistingUsers() throws IOException { + Map users = new HashMap<>(); + File[] userDirectories = listUserDirectories(); + if (userDirectories != null) { + for (File directory : userDirectories) { + String userId = idStrategy.idFromFilename(directory.getName()); + users.put(userId, directory); + } + } + addEmptyUsernameIfExists(users); + return users; + } + + private void addEmptyUsernameIfExists(Map users) throws IOException { + File emptyUsernameConfigFile = new File(usersDirectory, User.CONFIG_XML); + if (emptyUsernameConfigFile.exists()) { + File newEmptyUsernameDirectory = new File(usersDirectory, EMPTY_USERNAME_DIRECTORY_NAME); + Files.createDirectory(newEmptyUsernameDirectory.toPath()); + File newEmptyUsernameConfigFile = new File(newEmptyUsernameDirectory, User.CONFIG_XML); + Files.move(emptyUsernameConfigFile.toPath(), newEmptyUsernameConfigFile.toPath()); + users.put("", newEmptyUsernameDirectory); + } + } + + void migrateUsers(UserIdMapper mapper) throws IOException { + LOGGER.fine("Beginning migration of users to userId mapping."); + Map existingUsers = scanExistingUsers(); + for (Map.Entry existingUser : existingUsers.entrySet()) { + File newDirectory = mapper.putIfAbsent(existingUser.getKey(), false); + LOGGER.log(Level.INFO, "Migrating user '" + existingUser.getKey() + "' from 'users/" + existingUser.getValue().getName() + "/' to 'users/" + newDirectory.getName() + "/'"); + Files.move(existingUser.getValue().toPath(), newDirectory.toPath(), StandardCopyOption.REPLACE_EXISTING); + } + mapper.save(); + LOGGER.fine("Completed migration of users to userId mapping."); + } + +} diff --git a/core/src/main/java/hudson/scheduler/CronTab.java b/core/src/main/java/hudson/scheduler/CronTab.java index 120b8bc2a1..77118499ad 100644 --- a/core/src/main/java/hudson/scheduler/CronTab.java +++ b/core/src/main/java/hudson/scheduler/CronTab.java @@ -356,6 +356,14 @@ public final class CronTab { continue OUTER; } else { f.setTo(cal,next); + //check if value was actually set + if (f.valueOf(cal) != next) { + // we need to roll over to the next field. + f.rollUp(cal, 1); + f.setTo(cal,f.first(this)); + // since higher order field is affected by this, we need to restart from all over + continue OUTER; + } if (f.redoAdjustmentIfModified) continue OUTER; // when we modify DAY_OF_MONTH and DAY_OF_WEEK, do it all over from the top } diff --git a/core/src/main/java/jenkins/model/IdStrategy.java b/core/src/main/java/jenkins/model/IdStrategy.java index f90e892e0b..7b216a143f 100644 --- a/core/src/main/java/jenkins/model/IdStrategy.java +++ b/core/src/main/java/jenkins/model/IdStrategy.java @@ -37,6 +37,9 @@ import org.kohsuke.stapler.DataBoundConstructor; import javax.annotation.Nonnull; import java.util.Comparator; import java.util.Locale; +import java.util.function.Function; +import java.util.regex.Matcher; +import java.util.regex.Pattern; /** * The strategy to use for manipulating converting names (e.g. user names, group names, etc) into ids. @@ -46,54 +49,63 @@ import java.util.Locale; public abstract class IdStrategy extends AbstractDescribableImpl implements ExtensionPoint, Comparator { + private static final Pattern PSEUDO_UNICODE_PATTERN = Pattern.compile("\\$[a-f0-9]{4}"); + private static final Pattern CAPITALIZATION_PATTERN = Pattern.compile("~[a-z]"); + /** * The default case insensitive strategy. */ public static IdStrategy CASE_INSENSITIVE = new CaseInsensitive(); /** - * Converts an ID into a name for use as a filename. The return value must not contain any filesystem unsafe - * characters or names. See https://msdn.microsoft.com/en-us/library/aa365247.aspx for information on reserved - * names. + * No longer used. This method is now a no-op but the signature is retained for backward compatibility. * * @param id the id. * @return the name. Must be filesystem safe. + * @deprecated No current use. */ - @Nonnull - public abstract String filenameOf(@Nonnull String id); - - /** - * Converts an ID into a name for use as a filename. - * - * Legacy implementation used only for migrating old style config files to the new implementation. - * @param id the id - * @return the name - */ - @Nonnull - @Restricted(ProtectedExternally.class) - public String legacyFilenameOf(@Nonnull String id) { - return filenameOf(id); + @Deprecated + public String filenameOf(@Nonnull String id) { + return null; } + /** + * No longer used. This method is now a no-op but the signature is retained for backward compatibility. + * + * @param id the id + * @return the name + * @deprecated No current use. + */ + @Deprecated + @Restricted(ProtectedExternally.class) + public String legacyFilenameOf(@Nonnull String id) { + return null; + } /** * Converts a filename into the corresponding id. This may contain filesystem unsafe characters. + * * @param filename the filename. * @return the corresponding id. * @since 1.577 + * @deprecated Use only for migrating to new format. After the migration an id is no longer represented by a filename (directory). */ + @Deprecated public String idFromFilename(@Nonnull String filename) { return filename; } /** - * Converts an ID into a key for use in a Java Map. + * Converts an ID into a key for use in a Java Map or similar. This controls uniqueness of ids and how multiple different + * ids may map to the same id. For example, all different capitalizations of "Foo" may map to the same value "foo". * * @param id the id. * @return the key. */ @Nonnull - public abstract String keyFor(@Nonnull String id); + public String keyFor(@Nonnull String id) { + return id; + } /** * Compare two IDs and return {@code true} IFF the two ids are the same. Normally we expect that this should be @@ -109,7 +121,7 @@ public abstract class IdStrategy extends AbstractDescribableImpl imp } /** - * Compare tow IDs and return their sorting order. If {@link #equals(String, String)} is {@code true} then this + * Compare two IDs and return their sorting order. If {@link #equals(String, String)} is {@code true} then this * must return {@code 0} but {@link #compare(String, String)} returning {@code 0} need not imply that * {@link #equals(String, String)} is {@code true}. * @@ -160,7 +172,26 @@ public abstract class IdStrategy extends AbstractDescribableImpl imp * Returns all the registered {@link IdStrategy} descriptors. */ public static DescriptorExtensionList all() { - return Jenkins.getInstance().getDescriptorList(IdStrategy.class); + return Jenkins.get().getDescriptorList(IdStrategy.class); + } + + String applyPatternRepeatedly(@Nonnull Pattern pattern, @Nonnull String filename, + @Nonnull Function converter) { + StringBuilder id = new StringBuilder(); + int beginIndex = 0; + Matcher matcher = pattern.matcher(filename); + while (matcher.find()) { + String group = matcher.group(); + id.append(filename, beginIndex, matcher.start()); + id.append(converter.apply(group)); + beginIndex = matcher.end(); + } + id.append(filename.substring(beginIndex)); + return id.toString(); + } + + Character convertPseudoUnicode(String matchedGroup) { + return (char) Integer.parseInt(matchedGroup.substring(1), 16); } /** @@ -171,147 +202,10 @@ public abstract class IdStrategy extends AbstractDescribableImpl imp @DataBoundConstructor public CaseInsensitive() {} - @Override - @Nonnull - public String filenameOf(@Nonnull String id) { - if (id.isEmpty() || id.matches("[a-zA-Z0-9_. @-]+")) { - id = id.toLowerCase(Locale.ENGLISH); - switch (id) { - case "": - case ".": - return "$002f"; - case "..": - return "$002e$002e"; - case "con": - return "$0063on"; - case "prn": - return "$0070rn"; - case "aux": - return "$0061ux"; - case "nul": - return "$006eul"; - case "com1": - return "$0063om1"; - case "com2": - return "$0063om2"; - case "com3": - return "$0063om3"; - case "com4": - return "$0063om4"; - case "com5": - return "$0063om5"; - case "com6": - return "$0063om6"; - case "com7": - return "$0063om7"; - case "com8": - return "$0063om8"; - case "com9": - return "$0063om9"; - case "lpt1": - return "$006cpt1"; - case "lpt2": - return "$006cpt2"; - case "lpt3": - return "$006cpt3"; - case "lpt4": - return "$006cpt4"; - case "lpt5": - return "$006cpt5"; - case "lpt6": - return "$006cpt6"; - case "lpt7": - return "$006cpt7"; - case "lpt8": - return "$006cpt8"; - case "lpt9": - return "$006cpt9"; - default: - if (id.endsWith(".")) { - return id.substring(0,id.length()-1)+"$002e"; - } else if (id.startsWith("-")) { - return "$002d" + id.substring(1); - } - return id; - } - } else { - StringBuilder buf = new StringBuilder(id.length() + 16); - for (char c : id.toCharArray()) { - if ('a' <= c && c <= 'z') { - buf.append(c); - } else if ('A' <= c && c <= 'Z') { - buf.append(Character.toLowerCase(c)); - } else if ('0' <= c && c <= '9') { - buf.append(c); - } else if ('_' == c || '-' == c || ' ' == c || '@' == c || '.' == c) { - buf.append(c); - } else { - buf.append('$'); - buf.append(StringUtils.leftPad(Integer.toHexString(c & 0xffff), 4, '0')); - } - } - return buf.toString(); - } - } - - @Nonnull - @Override - public String legacyFilenameOf(@Nonnull String id) { - return id.toLowerCase(Locale.ENGLISH); - } - @Override public String idFromFilename(@Nonnull String filename) { - if (filename.matches("[A-Za-z0-9_. @-]+")) { - return filename.toLowerCase(Locale.ENGLISH); - } else { - StringBuilder buf = new StringBuilder(filename.length()); - final char[] chars = filename.toCharArray(); - for (int i = 0; i < chars.length; i++) { - char c = chars[i]; - if ('a' <= c && c <= 'z') { - buf.append(c); - } else if ('A' <= c && c <= 'a') { - buf.append(Character.toLowerCase(c)); - } else if ('0' <= c && c <= '9') { - buf.append(c); - } else if ('_' == c || '.' == c || '-' == c || ' ' == c || '@' == c) { - buf.append(c); - } else if (c == '$') { - StringBuilder hex = new StringBuilder(4); - i++; - if (i < chars.length) { - hex.append(chars[i]); - } else { - break; - } - i++; - if (i < chars.length) { - hex.append(chars[i]); - } else { - break; - } - i++; - if (i < chars.length) { - hex.append(chars[i]); - } else { - break; - } - i++; - if (i < chars.length) { - hex.append(chars[i]); - } else { - break; - } - try { - buf.append(Character.valueOf((char)Integer.parseInt(hex.toString(), 16))); - } catch (NumberFormatException x) { - buf.append('$').append(hex); - } - } - } - return buf.toString(); - } + String id = applyPatternRepeatedly(PSEUDO_UNICODE_PATTERN, filename, this::convertPseudoUnicode); + return id.toLowerCase(Locale.ENGLISH); } /** @@ -337,6 +231,7 @@ public abstract class IdStrategy extends AbstractDescribableImpl imp /** * {@inheritDoc} */ + @Nonnull @Override public String getDisplayName() { return Messages.IdStrategy_CaseInsensitive_DisplayName(); @@ -352,176 +247,14 @@ public abstract class IdStrategy extends AbstractDescribableImpl imp @DataBoundConstructor public CaseSensitive() {} - /** - * {@inheritDoc} - */ - @Override - @Nonnull - public String filenameOf(@Nonnull String id) { - if (id.isEmpty() || id.matches("[a-z0-9_. @-]+")) { - switch (id) { - case "": - case ".": - return "$002f"; - case "..": - return "$002e$002e"; - case "con": - return "$0063on"; - case "prn": - return "$0070rn"; - case "aux": - return "$0061ux"; - case "nul": - return "$006eul"; - case "com1": - return "$0063om1"; - case "com2": - return "$0063om2"; - case "com3": - return "$0063om3"; - case "com4": - return "$0063om4"; - case "com5": - return "$0063om5"; - case "com6": - return "$0063om6"; - case "com7": - return "$0063om7"; - case "com8": - return "$0063om8"; - case "com9": - return "$0063om9"; - case "lpt1": - return "$006cpt1"; - case "lpt2": - return "$006cpt2"; - case "lpt3": - return "$006cpt3"; - case "lpt4": - return "$006cpt4"; - case "lpt5": - return "$006cpt5"; - case "lpt6": - return "$006cpt6"; - case "lpt7": - return "$006cpt7"; - case "lpt8": - return "$006cpt8"; - case "lpt9": - return "$006cpt9"; - default: - if (id.endsWith(".")) { - return id.substring(0,id.length()-1)+"$002e"; - } else if (id.startsWith("-")) { - return "$002d" + id.substring(1); - } - return id; - } - } else { - StringBuilder buf = new StringBuilder(id.length() + 16); - for (char c : id.toCharArray()) { - if ('a' <= c && c <= 'z') { - buf.append(c); - } else if ('0' <= c && c <= '9') { - buf.append(c); - } else if ('_' == c || '-' == c || ' ' == c || '@' == c || '.' == c) { - buf.append(c); - } else if ('A' <= c && c <= 'Z') { - buf.append('~'); - buf.append(Character.toLowerCase(c)); - } else { - buf.append('$'); - buf.append(StringUtils.leftPad(Integer.toHexString(c & 0xffff), 4, '0')); - } - } - return buf.toString(); - } - } - - /** - * {@inheritDoc} - */ - @Nonnull - @Override - public String legacyFilenameOf(@Nonnull String id) { - if (id.matches("[a-z0-9_. @-]+")) { - return id; - } else { - StringBuilder buf = new StringBuilder(id.length() + 16); - for (char c : id.toCharArray()) { - if ('a' <= c && c <= 'z') { - buf.append(c); - } else if ('0' <= c && c <= '9') { - buf.append(c); - } else if ('_' == c || '.' == c || '-' == c || ' ' == c || '@' == c) { - buf.append(c); - } else if ('A' <= c && c <= 'Z') { - buf.append('~'); - buf.append(Character.toLowerCase(c)); - } else { - buf.append('$'); - buf.append(StringUtils.leftPad(Integer.toHexString(c & 0xffff), 4, '0')); - } - } - return buf.toString(); - } - } - @Override public String idFromFilename(@Nonnull String filename) { - if (filename.matches("[a-z0-9_. -]+")) { - return filename; - } else { - StringBuilder buf = new StringBuilder(filename.length()); - final char[] chars = filename.toCharArray(); - for (int i = 0; i < chars.length; i++) { - char c = chars[i]; - if ('a' <= c && c <= 'z') { - buf.append(c); - } else if ('0' <= c && c <= '9') { - buf.append(c); - } else if ('_' == c || '.' == c || '-' == c || ' ' == c || '@' == c) { - buf.append(c); - } else if (c == '~') { - i++; - if (i < chars.length) { - buf.append(Character.toUpperCase(chars[i])); - } - } else if (c == '$') { - StringBuilder hex = new StringBuilder(4); - i++; - if (i < chars.length) { - hex.append(chars[i]); - } else { - break; - } - i++; - if (i < chars.length) { - hex.append(chars[i]); - } else { - break; - } - i++; - if (i < chars.length) { - hex.append(chars[i]); - } else { - break; - } - i++; - if (i < chars.length) { - hex.append(chars[i]); - } else { - break; - } - try { - buf.append(Character.valueOf((char)Integer.parseInt(hex.toString(), 16))); - } catch (NumberFormatException x) { - buf.append('$').append(hex); - } - } - } - return buf.toString(); - } + String id = applyPatternRepeatedly(CAPITALIZATION_PATTERN, filename, this::convertCapitalizedAscii); + return applyPatternRepeatedly(PSEUDO_UNICODE_PATTERN, id, this::convertPseudoUnicode); + } + + private Character convertCapitalizedAscii(String encoded) { + return encoded.toUpperCase().charAt(1); } /** @@ -532,15 +265,6 @@ public abstract class IdStrategy extends AbstractDescribableImpl imp return StringUtils.equals(id1, id2); } - /** - * {@inheritDoc} - */ - @Override - @Nonnull - public String keyFor(@Nonnull String id) { - return id; - } - /** * {@inheritDoc} */ @@ -576,15 +300,6 @@ public abstract class IdStrategy extends AbstractDescribableImpl imp @DataBoundConstructor public CaseSensitiveEmailAddress() {} - /** - * {@inheritDoc} - */ - @Override - @Nonnull - public String filenameOf(@Nonnull String id) { - return super.filenameOf(keyFor(id)); - } - /** * {@inheritDoc} */ diff --git a/core/src/main/java/jenkins/util/VirtualFile.java b/core/src/main/java/jenkins/util/VirtualFile.java index c1ea07d536..eaa16e73d0 100644 --- a/core/src/main/java/jenkins/util/VirtualFile.java +++ b/core/src/main/java/jenkins/util/VirtualFile.java @@ -48,6 +48,8 @@ import java.nio.file.LinkOption; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.Deque; +import java.util.LinkedList; import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; @@ -64,6 +66,7 @@ import org.apache.tools.ant.types.selectors.TokenizedPath; import org.apache.tools.ant.types.selectors.TokenizedPattern; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.Beta; +import org.kohsuke.accmod.restrictions.NoExternalUse; /** * Abstraction over {@link File}, {@link FilePath}, or other items such as network resources or ZIP entries. @@ -353,6 +356,27 @@ public abstract class VirtualFile implements Comparable, Serializab return null; } + /** + * Determine if the implementation supports the {@link #isDescendant(String)} method + * + * TODO un-restrict it in a weekly after the patch + */ + @Restricted(NoExternalUse.class) + public boolean supportIsDescendant() { + return false; + } + + /** + * Check if the relative path is really a descendant of this folder, following the symbolic links. + * Meant to be used in coordination with {@link #child(String)}. + * + * TODO un-restrict it in a weekly after the patch + */ + @Restricted(NoExternalUse.class) + public boolean isDescendant(String childRelativePath) throws IOException { + return false; + } + /** * Creates a virtual file wrapper for a local file. * @param f a disk file (need not exist) @@ -459,6 +483,7 @@ public abstract class VirtualFile implements Comparable, Serializab throw new IOException(e); } } + private boolean isIllegalSymlink() { // TODO JENKINS-26838 try { String myPath = f.toPath().toRealPath(new LinkOption[0]).toString(); @@ -474,6 +499,54 @@ public abstract class VirtualFile implements Comparable, Serializab } return false; } + + /** + * TODO un-restrict it in a weekly after the patch + */ + @Override + @Restricted(NoExternalUse.class) + public boolean supportIsDescendant() { + return true; + } + + /** + * TODO un-restrict it in a weekly after the patch + */ + @Override + @Restricted(NoExternalUse.class) + public boolean isDescendant(String potentialChildRelativePath) throws IOException { + if (new File(potentialChildRelativePath).isAbsolute()) { + throw new IllegalArgumentException("Only a relative path is supported, the given path is absolute: " + potentialChildRelativePath); + } + + FilePath root = new FilePath(this.root); + String relativePath = computeRelativePathToRoot(); + + try { + return root.isDescendant(relativePath + potentialChildRelativePath); + } + catch (InterruptedException e) { + return false; + } + } + + /** + * To be kept in sync with {@link FilePathVF#computeRelativePathToRoot()} + */ + private String computeRelativePathToRoot(){ + if (this.root.equals(this.f)) { + return ""; + } + + Deque relativePath = new LinkedList<>(); + File current = this.f; + while (current != null && !current.equals(this.root)) { + relativePath.addFirst(current.getName()); + current = current.getParentFile(); + } + + return String.join(File.separator, relativePath) + File.separator; + } } /** @@ -482,12 +555,14 @@ public abstract class VirtualFile implements Comparable, Serializab * @return a wrapper */ public static VirtualFile forFilePath(final FilePath f) { - return new FilePathVF(f); + return new FilePathVF(f, f); } private static final class FilePathVF extends VirtualFile { private final FilePath f; - FilePathVF(FilePath f) { + private final FilePath root; + FilePathVF(FilePath f, FilePath root) { this.f = f; + this.root = root; } @Override public String getName() { return f.getName(); @@ -532,7 +607,7 @@ public abstract class VirtualFile implements Comparable, Serializab List kids = f.list(); VirtualFile[] vfs = new VirtualFile[kids.size()]; for (int i = 0; i < vfs.length; i++) { - vfs[i] = forFilePath(kids.get(i)); + vfs[i] = new FilePathVF(kids.get(i), this.root); } return vfs; } catch (InterruptedException x) { @@ -547,7 +622,7 @@ public abstract class VirtualFile implements Comparable, Serializab } } @Override public VirtualFile child(String name) { - return forFilePath(f.child(name)); + return new FilePathVF(f.child(name), this.root); } @Override public long length() throws IOException { try { @@ -591,6 +666,53 @@ public abstract class VirtualFile implements Comparable, Serializab throw new IOException(x); } } + + /** + * TODO un-restrict it in a weekly after the patch + */ + @Override + @Restricted(NoExternalUse.class) + public boolean supportIsDescendant() { + return true; + } + + /** + * TODO un-restrict it in a weekly after the patch + */ + @Override + @Restricted(NoExternalUse.class) + public boolean isDescendant(String potentialChildRelativePath) throws IOException { + if (new File(potentialChildRelativePath).isAbsolute()) { + throw new IllegalArgumentException("Only a relative path is supported, the given path is absolute: " + potentialChildRelativePath); + } + + String relativePath = computeRelativePathToRoot(); + + try { + return this.root.isDescendant(relativePath + potentialChildRelativePath); + } + catch (InterruptedException e) { + return false; + } + } + + /** + * To be kept in sync with {@link FileVF#computeRelativePathToRoot()} + */ + private String computeRelativePathToRoot(){ + if (this.root.equals(this.f)) { + return ""; + } + + LinkedList relativePath = new LinkedList<>(); + FilePath current = this.f; + while (current != null && !current.equals(this.root)) { + relativePath.addFirst(current.getName()); + current = current.getParent(); + } + + return String.join(File.separator, relativePath) + File.separator; + } } private static final class Scanner extends MasterToSlaveFileCallable> { private final String includes, excludes; diff --git a/core/src/test/java/hudson/FilePathSEC904Test.java b/core/src/test/java/hudson/FilePathSEC904Test.java new file mode 100644 index 0000000000..f72050de61 --- /dev/null +++ b/core/src/test/java/hudson/FilePathSEC904Test.java @@ -0,0 +1,312 @@ +/* + * The MIT License + * + * Copyright (c) 2004-2010, Sun Microsystems, Inc., Kohsuke Kawaguchi, Alan Harder + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +package hudson; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.jvnet.hudson.test.Issue; + +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.concurrent.TimeUnit; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeTrue; + +//TODO to be merged back in FilePathTest after security release +public class FilePathSEC904Test { + + @Rule + public TemporaryFolder temp = new TemporaryFolder(); + + @Test + @Issue("SECURITY-904") + public void isDescendant_regularFiles() throws IOException, InterruptedException { + // root + // /workspace + // /sub + // sub-regular.txt + // regular.txt + // /protected + // secret.txt + FilePath rootFolder = new FilePath(temp.newFolder("root")); + FilePath workspaceFolder = rootFolder.child("workspace"); + FilePath subFolder = workspaceFolder.child("sub"); + FilePath protectedFolder = rootFolder.child("protected"); + + FilePath regularFile = workspaceFolder.child("regular.txt"); + regularFile.write("regular-file", StandardCharsets.UTF_8.name()); + FilePath subRegularFile = subFolder.child("sub-regular.txt"); + subRegularFile.write("sub-regular-file", StandardCharsets.UTF_8.name()); + + FilePath secretFile = protectedFolder.child("secret.txt"); + secretFile.write("secrets", StandardCharsets.UTF_8.name()); + + assertTrue(workspaceFolder.isDescendant(".")); + assertTrue(workspaceFolder.isDescendant("regular.txt")); + assertTrue(workspaceFolder.isDescendant("./regular.txt")); + assertTrue(workspaceFolder.isDescendant("sub/sub-regular.txt")); + assertTrue(workspaceFolder.isDescendant("sub//sub-regular.txt")); + assertTrue(workspaceFolder.isDescendant("sub/../sub/sub-regular.txt")); + assertTrue(workspaceFolder.isDescendant("./sub/../sub/sub-regular.txt")); + + // nonexistent files + assertTrue(workspaceFolder.isDescendant("nonexistent.txt")); + assertTrue(workspaceFolder.isDescendant("sub/nonexistent.txt")); + assertTrue(workspaceFolder.isDescendant("nonexistent/nonexistent.txt")); + assertFalse(workspaceFolder.isDescendant("../protected/nonexistent.txt")); + assertFalse(workspaceFolder.isDescendant("../nonexistent/nonexistent.txt")); + + // the intermediate path "./.." goes out of workspace and so is refused + assertFalse(workspaceFolder.isDescendant("./../workspace")); + assertFalse(workspaceFolder.isDescendant("./../workspace/")); + assertFalse(workspaceFolder.isDescendant("./../workspace/regular.txt")); + assertFalse(workspaceFolder.isDescendant("../workspace/regular.txt")); + assertFalse(workspaceFolder.isDescendant("./../../root/workspace/regular.txt")); + + // attempt to reach other folder + assertFalse(workspaceFolder.isDescendant("../protected/secret.txt")); + assertFalse(workspaceFolder.isDescendant("./../protected/secret.txt")); + } + + @Test + @Issue("SECURITY-904") + public void isDescendant_regularSymlinks() throws IOException, InterruptedException { + // root + // /workspace + // /a + // a.txt + // /b + // _a => symlink to ../a + // _atxt => symlink to ../a/a.txt + // regular.txt + // _nonexistent => symlink to nonexistent (nonexistent folder) + // _nonexistentUp => symlink to ../nonexistent (nonexistent folder + illegal) + // _protected => symlink to ../protected (illegal) + // _secrettxt => symlink to ../protected/secret.txt (illegal) + // /protected + // secret.txt + FilePath rootFolder = new FilePath(temp.newFolder("root")); + FilePath workspaceFolder = rootFolder.child("workspace"); + FilePath aFolder = workspaceFolder.child("a"); + FilePath bFolder = workspaceFolder.child("b"); + FilePath protectedFolder = rootFolder.child("protected"); + + FilePath regularFile = workspaceFolder.child("regular.txt"); + regularFile.write("regular-file", StandardCharsets.UTF_8.name()); + FilePath aFile = aFolder.child("a.txt"); + aFile.write("a-file", StandardCharsets.UTF_8.name()); + FilePath bFile = bFolder.child("a.txt"); + bFile.write("b-file", StandardCharsets.UTF_8.name()); + bFolder.child("_a").symlinkTo("../a", null); + bFolder.child("_atxt").symlinkTo("../a/a.txt", null); + // illegal symlinks + workspaceFolder.child("_protected").symlinkTo("../protected", null); + workspaceFolder.child("_nonexistent").symlinkTo("nonexistent", null); + workspaceFolder.child("_nonexistentUp").symlinkTo("../nonexistent", null); + workspaceFolder.child("_secrettxt").symlinkTo("../protected/secret.txt", null); + + FilePath secretFile = protectedFolder.child("secret.txt"); + secretFile.write("secrets", StandardCharsets.UTF_8.name()); + + assertTrue(workspaceFolder.isDescendant("regular.txt")); + assertTrue(workspaceFolder.isDescendant("_nonexistent")); + assertTrue(workspaceFolder.isDescendant("a")); + assertTrue(workspaceFolder.isDescendant("a/a.txt")); + assertTrue(workspaceFolder.isDescendant("b")); + assertTrue(workspaceFolder.isDescendant("b/_a/a.txt")); + assertTrue(workspaceFolder.isDescendant("b/_atxt")); + + // nonexistent but illegal + assertFalse(workspaceFolder.isDescendant("_nonexistentUp")); + // illegal symlinks + assertFalse(workspaceFolder.isDescendant("_protected")); + assertFalse(workspaceFolder.isDescendant("_protected/")); + assertFalse(workspaceFolder.isDescendant("_protected/secret.txt")); + assertFalse(workspaceFolder.isDescendant("./_protected/secret.txt")); + assertFalse(workspaceFolder.isDescendant("_secrettxt")); + assertFalse(workspaceFolder.isDescendant("./_secrettxt")); + } + + @Test + @Issue("SECURITY-904") + public void isDescendant_windowsSpecificSymlinks() throws Exception { + assumeTrue(Functions.isWindows()); + // root + // /workspace + // /a + // a.txt + // /b + // b.txt + // _a => junction to ../a + // regular.txt + // _nonexistent => junction to nonexistent (nonexistent folder) + // _nonexistentUp => junction to ../nonexistent (nonexistent and illegal) + // _protected => junction to ../protected (illegal) + // /protected + // secret.txt + File root = temp.newFolder("root"); + FilePath rootFolder = new FilePath(root); + FilePath workspaceFolder = rootFolder.child("workspace"); + FilePath aFolder = workspaceFolder.child("a"); + FilePath bFolder = workspaceFolder.child("b"); + FilePath protectedFolder = rootFolder.child("protected"); + + FilePath regularFile = workspaceFolder.child("regular.txt"); + regularFile.write("regular-file", StandardCharsets.UTF_8.name()); + FilePath aFile = aFolder.child("a.txt"); + aFile.write("a-file", StandardCharsets.UTF_8.name()); + FilePath bFile = bFolder.child("a.txt"); + bFile.write("b-file", StandardCharsets.UTF_8.name()); + + createJunction(new File(root, "/workspace/b/_a"), new File(root, "/workspace/a")); + createJunction(new File(root, "/workspace/_nonexistent"), new File(root, "/workspace/nonexistent")); + createJunction(new File(root, "/workspace/_nonexistentUp"), new File(root, "/nonexistent")); + createJunction(new File(root, "/workspace/_protected"), new File(root, "/protected")); + + FilePath secretFile = protectedFolder.child("secret.txt"); + secretFile.write("secrets", StandardCharsets.UTF_8.name()); + + assertTrue(workspaceFolder.isDescendant("b")); + assertTrue(workspaceFolder.isDescendant("b/_a/a.txt")); + + // nonexistent and not proven illegal, the junction links are not resolved + // by Util.resolveSymlinkToFile / neither Path.toRealPath under Windows + assertTrue(workspaceFolder.isDescendant("_nonexistent")); + assertTrue(workspaceFolder.isDescendant("_nonexistent/")); + assertTrue(workspaceFolder.isDescendant("_nonexistent/..")); + assertTrue(workspaceFolder.isDescendant("_nonexistentUp")); + + // illegal symlinks + assertFalse(workspaceFolder.isDescendant("_protected")); + assertFalse(workspaceFolder.isDescendant("_protected/../a")); + } + + private void createJunction(File from, File to) throws Exception { + Process p = Runtime.getRuntime().exec(new String[]{"cmd", "/c", "mklink", "/J", from.getAbsolutePath(), to.getAbsolutePath()}); + p.waitFor(2, TimeUnit.SECONDS); + } + + @Test(expected = IllegalArgumentException.class) + @Issue("SECURITY-904") + public void isDescendant_throwIfParentDoesNotExist_symlink() throws Exception { + FilePath rootFolder = new FilePath(temp.newFolder("root")); + FilePath aFolder = rootFolder.child("a"); + aFolder.mkdirs(); + FilePath linkToNonexistent = aFolder.child("linkToNonexistent"); + linkToNonexistent.symlinkTo("__nonexistent__", null); + + linkToNonexistent.isDescendant("."); + } + + @Test(expected = IllegalArgumentException.class) + @Issue("SECURITY-904") + public void isDescendant_throwIfParentDoesNotExist_directNonexistent() throws Exception { + FilePath rootFolder = new FilePath(temp.newFolder("root")); + FilePath nonexistent = rootFolder.child("nonexistent"); + nonexistent.isDescendant("."); + } + + @Test(expected = IllegalArgumentException.class) + @Issue("SECURITY-904") + public void isDescendant_throwIfAbsolutePathGiven() throws Exception { + FilePath rootFolder = new FilePath(temp.newFolder("root")); + rootFolder.mkdirs(); + rootFolder.isDescendant(temp.newFile().getAbsolutePath()); + } + + @Test + @Issue("SECURITY-904") + public void isDescendant_worksEvenInSymbolicWorkspace() throws Exception { + // root + // /w + // /_workspace => symlink to ../workspace + // /workspace + // /a + // a.txt + // /b + // _a => symlink to ../a + // _atxt => symlink to ../a/a.txt + // regular.txt + // _nonexistent => symlink to nonexistent (nonexistent folder) + // _nonexistentUp => symlink to ../nonexistent (nonexistent folder + illegal) + // _protected => symlink to ../protected (illegal) + // _secrettxt => symlink to ../protected/secret.txt (illegal) + // /protected + // secret.txt + FilePath rootFolder = new FilePath(temp.newFolder("root")); + FilePath wFolder = rootFolder.child("w"); + FilePath workspaceFolder = rootFolder.child("workspace"); + FilePath aFolder = workspaceFolder.child("a"); + FilePath bFolder = workspaceFolder.child("b"); + FilePath protectedFolder = rootFolder.child("protected"); + + FilePath regularFile = workspaceFolder.child("regular.txt"); + regularFile.write("regular-file", StandardCharsets.UTF_8.name()); + FilePath aFile = aFolder.child("a.txt"); + aFile.write("a-file", StandardCharsets.UTF_8.name()); + FilePath bFile = bFolder.child("a.txt"); + bFile.write("b-file", StandardCharsets.UTF_8.name()); + bFolder.child("_a").symlinkTo("../a", null); + bFolder.child("_atxt").symlinkTo("../a/a.txt", null); + // illegal symlinks + workspaceFolder.child("_protected").symlinkTo("../protected", null); + workspaceFolder.child("_protected2").symlinkTo("../../protected", null); + workspaceFolder.child("_nonexistent").symlinkTo("nonexistent", null); + workspaceFolder.child("_nonexistentUp").symlinkTo("../nonexistent", null); + workspaceFolder.child("_secrettxt").symlinkTo("../protected/secret.txt", null); + workspaceFolder.child("_secrettxt2").symlinkTo("../../protected/secret.txt", null); + + wFolder.mkdirs(); + FilePath symbolicWorkspace = wFolder.child("_w"); + symbolicWorkspace.symlinkTo("../workspace", null); + + FilePath secretFile = protectedFolder.child("secret.txt"); + secretFile.write("secrets", StandardCharsets.UTF_8.name()); + + assertTrue(symbolicWorkspace.isDescendant("regular.txt")); + assertTrue(symbolicWorkspace.isDescendant("_nonexistent")); + assertTrue(symbolicWorkspace.isDescendant("a")); + assertTrue(symbolicWorkspace.isDescendant("a/a.txt")); + assertTrue(symbolicWorkspace.isDescendant("b")); + assertTrue(symbolicWorkspace.isDescendant("b/_a/a.txt")); + assertTrue(symbolicWorkspace.isDescendant("b/_atxt")); + + // nonexistent but illegal + assertFalse(symbolicWorkspace.isDescendant("_nonexistentUp")); + // illegal symlinks + assertFalse(symbolicWorkspace.isDescendant("_protected")); + assertFalse(symbolicWorkspace.isDescendant("_protected/")); + assertFalse(symbolicWorkspace.isDescendant("_protected/secret.txt")); + assertFalse(symbolicWorkspace.isDescendant("./_protected/secret.txt")); + assertFalse(symbolicWorkspace.isDescendant("_protected2")); + assertFalse(symbolicWorkspace.isDescendant("_protected2/secret.txt")); + assertFalse(symbolicWorkspace.isDescendant("_secrettxt")); + assertFalse(symbolicWorkspace.isDescendant("./_secrettxt")); + assertFalse(symbolicWorkspace.isDescendant("_secrettxt2")); + } +} diff --git a/core/src/test/java/hudson/UtilSEC904Test.java b/core/src/test/java/hudson/UtilSEC904Test.java new file mode 100644 index 0000000000..7bb7896395 --- /dev/null +++ b/core/src/test/java/hudson/UtilSEC904Test.java @@ -0,0 +1,84 @@ +/* + * The MIT License + * + * Copyright (c) 2004-2010, Sun Microsystems, Inc., Kohsuke Kawaguchi, + * Daniel Dyer, Erik Ramfelt, Richard Bair, id:cactusman + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +package hudson; + +import hudson.model.TaskListener; +import org.apache.commons.io.FileUtils; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import java.io.File; +import java.nio.file.Files; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +/** + * @author Kohsuke Kawaguchi + */ +//TODO merge into UtilTest after the security release +public class UtilSEC904Test { + + @Rule public TemporaryFolder tmp = new TemporaryFolder(); + + @Test + public void resolveSymlinkToFile() throws Exception { + // root + // /a + // /aa + // aa.txt + // /_b => symlink to /root/b + // /b + // /_a => symlink to /root/a + File root = tmp.getRoot(); + File a = new File(root, "a"); + File aa = new File(a, "aa"); + aa.mkdirs(); + File aaTxt = new File(aa, "aa.txt"); + FileUtils.write(aaTxt, "aa"); + + File b = new File(root, "b"); + b.mkdir(); + + File _a = new File(b, "_a"); + Util.createSymlink(_a.getParentFile(), a.getAbsolutePath(), _a.getName(), TaskListener.NULL); + + File _b = new File(a, "_b"); + Util.createSymlink(_b.getParentFile(), b.getAbsolutePath(), _b.getName(), TaskListener.NULL); + + assertTrue(Files.isSymbolicLink(_a.toPath())); + assertTrue(Files.isSymbolicLink(_b.toPath())); + + // direct symlinks are resolved + assertEquals(Util.resolveSymlinkToFile(_a), a); + assertEquals(Util.resolveSymlinkToFile(_b), b); + + // intermediate symlinks are NOT resolved + assertNull(Util.resolveSymlinkToFile(new File(_a, "aa"))); + assertNull(Util.resolveSymlinkToFile(new File(_a, "aa/aa.txt"))); + } +} diff --git a/core/src/test/java/hudson/model/TestUserIdMapper.java b/core/src/test/java/hudson/model/TestUserIdMapper.java new file mode 100644 index 0000000000..52a845c1cf --- /dev/null +++ b/core/src/test/java/hudson/model/TestUserIdMapper.java @@ -0,0 +1,27 @@ +package hudson.model; + +import jenkins.model.IdStrategy; + +import java.io.File; +import java.io.IOException; + +class TestUserIdMapper extends UserIdMapper { + + private File usersDirectory; + private IdStrategy idStrategy; + + TestUserIdMapper(File usersDirectory, IdStrategy idStrategy) throws IOException { + this.usersDirectory = usersDirectory; + this.idStrategy = idStrategy; + } + + @Override + protected File getUsersDirectory() { + return usersDirectory; + } + + @Override + protected IdStrategy getIdStrategy() { + return idStrategy; + } +} diff --git a/core/src/test/java/hudson/model/UserIdMapperTest.java b/core/src/test/java/hudson/model/UserIdMapperTest.java new file mode 100644 index 0000000000..4fe171ff4d --- /dev/null +++ b/core/src/test/java/hudson/model/UserIdMapperTest.java @@ -0,0 +1,309 @@ +/* + * The MIT License + * + * Copyright (c) 2018 CloudBees, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +package hudson.model; + +import jenkins.model.IdStrategy; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TestName; + +import java.io.File; +import java.io.IOException; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; + +public class UserIdMapperTest { + + @Rule + public TestName name= new TestName(); + + @Test + public void testNonexistentFileLoads() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + } + + @Test + public void testEmptyGet() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + assertThat(mapper.getDirectory("anything"), nullValue()); + } + + @Test + public void testSimplePutGet() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + String user1 = "user1"; + File directory = mapper.putIfAbsent(user1, true); + assertThat(directory, is(mapper.getDirectory(user1))); + } + + @Test + public void testMultiple() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + String user1 = "user1"; + File directory1 = mapper.putIfAbsent(user1, true); + String user2 = "user2"; + File directory2 = mapper.putIfAbsent(user2, true); + String user3 = "user3"; + File directory3 = mapper.putIfAbsent(user3, true); + assertThat(directory1, is(mapper.getDirectory(user1))); + assertThat(directory2, is(mapper.getDirectory(user2))); + assertThat(directory3, is(mapper.getDirectory(user3))); + } + + @Test + public void testMultipleSaved() throws IOException { + File usersDirectory = UserIdMigratorTest.createTestDirectory(getClass(), name); + IdStrategy idStrategy = IdStrategy.CASE_INSENSITIVE; + UserIdMapper mapper = new TestUserIdMapper(usersDirectory, idStrategy); + mapper.init(); + String user1 = "user1"; + File directory1 = mapper.putIfAbsent(user1, true); + String user2 = "user2"; + File directory2 = mapper.putIfAbsent(user2, true); + String user3 = "user3"; + File directory3 = mapper.putIfAbsent(user3, true); + mapper = new TestUserIdMapper(usersDirectory, idStrategy); + mapper.init(); + assertThat(directory1, is(mapper.getDirectory(user1))); + assertThat(directory2, is(mapper.getDirectory(user2))); + assertThat(directory3, is(mapper.getDirectory(user3))); + } + + @Test + public void testRepeatPut() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + String user1 = "user1"; + File directory1 = mapper.putIfAbsent(user1, true); + File directory2 = mapper.putIfAbsent(user1, true); + assertThat(directory1, is(directory2)); + } + + @Test + public void testIsNotMapped() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + assertThat(mapper.isMapped("anything"), is(false)); + } + + @Test + public void testIsMapped() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + String user1 = "user1"; + File directory = mapper.putIfAbsent(user1, true); + assertThat(mapper.isMapped(user1), is(true)); + } + + @Test + public void testInitialUserIds() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + assertThat(mapper.getConvertedUserIds(), empty()); + } + + @Test + public void testSingleUserIds() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + String user1 = "user1"; + File directory = mapper.putIfAbsent(user1, true); + assertThat(mapper.getConvertedUserIds(), hasSize(1)); + assertThat(mapper.getConvertedUserIds().iterator().next(), is(user1)); + } + + @Test + public void testMultipleUserIds() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + String user1 = "user1"; + File directory = mapper.putIfAbsent(user1, true); + String user2 = "user2"; + File directory2 = mapper.putIfAbsent(user2, true); + assertThat(mapper.getConvertedUserIds(), hasSize(2)); + assertThat(mapper.getConvertedUserIds(), hasItems(user1, user2)); + } + + @Test + public void testRemove() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + String user1 = "user1"; + File directory = mapper.putIfAbsent(user1, true); + mapper.remove(user1); + assertThat(mapper.isMapped(user1), is(false)); + } + + @Test + public void testRemoveAfterSave() throws IOException { + File usersDirectory = UserIdMigratorTest.createTestDirectory(getClass(), name); + IdStrategy idStrategy = IdStrategy.CASE_INSENSITIVE; + UserIdMapper mapper = new TestUserIdMapper(usersDirectory, idStrategy); + mapper.init(); + String user1 = "user1"; + File directory = mapper.putIfAbsent(user1, true); + mapper = new TestUserIdMapper(usersDirectory, idStrategy); + mapper.init(); + mapper.remove(user1); + mapper = new TestUserIdMapper(usersDirectory, idStrategy); + assertThat(mapper.isMapped(user1), is(false)); + } + + @Test + public void testPutGetCaseInsensitive() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + String user1 = "user1"; + File directory = mapper.putIfAbsent(user1, true); + assertThat(mapper.getDirectory(user1.toUpperCase()), notNullValue()); + } + + @Test + public void testPutGetCaseSensitive() throws IOException { + IdStrategy idStrategy = new IdStrategy.CaseSensitive(); + UserIdMapper mapper = createUserIdMapper(idStrategy); + String user1 = "user1"; + File directory = mapper.putIfAbsent(user1, true); + assertThat(mapper.getDirectory(user1.toUpperCase()), nullValue()); + } + + @Test + public void testIsMappedCaseInsensitive() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + String user1 = "user1"; + File directory = mapper.putIfAbsent(user1, true); + assertThat(mapper.isMapped(user1.toUpperCase()), is(true)); + } + + @Test + public void testIsMappedCaseSensitive() throws IOException { + IdStrategy idStrategy = new IdStrategy.CaseSensitive(); + UserIdMapper mapper = createUserIdMapper(idStrategy); + String user1 = "user1"; + File directory = mapper.putIfAbsent(user1, true); + assertThat(mapper.isMapped(user1.toUpperCase()), is(false)); + } + + @Test + public void testRemoveCaseInsensitive() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + String user1 = "user1"; + File directory = mapper.putIfAbsent(user1, true); + mapper.remove(user1.toUpperCase()); + assertThat(mapper.isMapped(user1), is(false)); + } + + @Test + public void testRemoveCaseSensitive() throws IOException { + IdStrategy idStrategy = new IdStrategy.CaseSensitive(); + UserIdMapper mapper = createUserIdMapper(idStrategy); + String user1 = "user1"; + File directory = mapper.putIfAbsent(user1, true); + mapper.remove(user1.toUpperCase()); + assertThat(mapper.isMapped(user1), is(true)); + } + + @Test + public void testRepeatRemove() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + String user1 = "user1"; + File directory1 = mapper.putIfAbsent(user1, true); + mapper.remove(user1); + mapper.remove(user1); + assertThat(mapper.isMapped(user1), is(false)); + } + + @Test + public void testClear() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + String user1 = "user1"; + File directory1 = mapper.putIfAbsent(user1, true); + mapper.clear(); + assertThat(mapper.isMapped(user1), is(false)); + assertThat(mapper.getConvertedUserIds(), empty()); + } + + @Test + public void testReload() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + String user1 = "user1"; + File directory1 = mapper.putIfAbsent(user1, true); + mapper.reload(); + assertThat(mapper.isMapped(user1), is(true)); + assertThat(mapper.getConvertedUserIds(), hasSize(1)); + } + + @Test + public void testDirectoryFormatBasic() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + String user1 = "1user"; + File directory1 = mapper.putIfAbsent(user1, true); + assertThat(directory1.getName(), startsWith(user1 + '_')); + } + + @Test + public void testDirectoryFormatLongerUserId() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + String user1 = "muchlongeruserid"; + File directory1 = mapper.putIfAbsent(user1, true); + assertThat(directory1.getName(), startsWith("muchlongeruser_")); + } + + @Test + public void testDirectoryFormatAllSuppressedCharacters() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + String user1 = "!@#$%^"; + File directory1 = mapper.putIfAbsent(user1, true); + assertThat(directory1.getName(), startsWith("_")); + } + + @Test + public void testDirectoryFormatSingleCharacter() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + String user1 = "."; + File directory1 = mapper.putIfAbsent(user1, true); + assertThat(directory1.getName(), startsWith("_")); + } + + @Test + public void testDirectoryFormatMixed() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + String user1 = "a$b!c^d~e@f"; + File directory1 = mapper.putIfAbsent(user1, true); + assertThat(directory1.getName(), startsWith("abcdef_")); + } + + @Test(expected = IOException.class) + public void testXmlFileCorrupted() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + } + + @Test + public void testDuplicatedUserId() throws IOException { + UserIdMapper mapper = createUserIdMapper(IdStrategy.CASE_INSENSITIVE); + assertThat(mapper.isMapped("user2"), is(true)); + assertThat(mapper.isMapped("user1"), is(true)); + } + + private UserIdMapper createUserIdMapper(IdStrategy idStrategy) throws IOException { + File usersDirectory = UserIdMigratorTest.createTestDirectory(getClass(), name); + TestUserIdMapper mapper = new TestUserIdMapper(usersDirectory, idStrategy); + mapper.init(); + return mapper; + } + +} diff --git a/core/src/test/java/hudson/model/UserIdMigratorTest.java b/core/src/test/java/hudson/model/UserIdMigratorTest.java new file mode 100644 index 0000000000..6c5745abdb --- /dev/null +++ b/core/src/test/java/hudson/model/UserIdMigratorTest.java @@ -0,0 +1,205 @@ +/* + * The MIT License + * + * Copyright (c) 2018 CloudBees, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +package hudson.model; + +import jenkins.model.IdStrategy; +import org.apache.commons.io.FileUtils; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TestName; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.Map; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; + +public class UserIdMigratorTest { + + private static final String BASE_RESOURCE_PATH = "src/test/resources/hudson/model/"; + + @Rule + public TestName name = new TestName(); + + @Test + public void needsMigrationBasic() throws IOException { + UserIdMigrator migrator = createUserIdMigrator(); + assertThat(migrator.needsMigration(), is(true)); + } + + @Test + public void needsMigrationFalse() throws IOException { + UserIdMigrator migrator = createUserIdMigrator(); + assertThat(migrator.needsMigration(), is(false)); + } + + @Test + public void needsMigrationNoneExisting() throws IOException { + UserIdMigrator migrator = createUserIdMigrator(); + assertThat(migrator.needsMigration(), is(false)); + } + + @Test + public void needsMigrationNoUserConfigFiles() throws IOException { + UserIdMigrator migrator = createUserIdMigrator(); + assertThat(migrator.needsMigration(), is(false)); + } + + @Test + public void scanExistingUsersNone() throws IOException { + File usersDirectory = createTestDirectory(getClass(), name); + UserIdMigrator migrator = new UserIdMigrator(usersDirectory, IdStrategy.CASE_INSENSITIVE); + Map userMappings = migrator.scanExistingUsers(); + assertThat(userMappings.keySet(), empty()); + } + + @Test + public void scanExistingUsersNoUsersDirectory() throws IOException { + UserIdMigrator migrator = createUserIdMigrator(); + Map userMappings = migrator.scanExistingUsers(); + assertThat(userMappings.keySet(), empty()); + } + + @Test + public void scanExistingUsersBasic() throws IOException { + UserIdMigrator migrator = createUserIdMigrator(); + Map userMappings = migrator.scanExistingUsers(); + assertThat(userMappings.keySet(), hasSize(2)); + assertThat(userMappings.keySet(), hasItems("admin", "jane")); + } + + @Test + public void scanExistingUsersLegacy() throws IOException { + UserIdMigrator migrator = createUserIdMigrator(); + Map userMappings = migrator.scanExistingUsers(); + assertThat(userMappings.keySet(), hasSize(8)); + assertThat(userMappings.keySet(), hasItems("foo/bar", "foo/bar/baz", "/", "..", "bla$phem.us", "make$1000000", "big$money", "~com1")); + } + + @Test + public void scanExistingUsersOldLegacy() throws IOException { + UserIdMigrator migrator = createUserIdMigrator(); + Map userMappings = migrator.scanExistingUsers(); + assertThat(userMappings.keySet(), hasSize(4)); + assertThat(userMappings.keySet(), hasItems("make\u1000000", "\u306f\u56fd\u5185\u3067\u6700\u5927", "\u1000yyy", "zzz\u1000")); + } + + @Test + public void emptyUsernameConfigScanned() throws IOException { + UserIdMigrator migrator = createUserIdMigrator(); + Map userMappings = migrator.scanExistingUsers(); + assertThat(userMappings.keySet(), hasSize(2)); + assertThat(userMappings.keySet(), hasItems("admin", "")); + } + + @Test + public void scanExistingUsersCaseSensitive() throws IOException { + File usersDirectory = createTestDirectory(getClass(), name); + UserIdMigrator migrator = new UserIdMigrator(usersDirectory, new IdStrategy.CaseSensitive()); + Map userMappings = migrator.scanExistingUsers(); + assertThat(userMappings.keySet(), hasSize(3)); + assertThat(userMappings.keySet(), hasItems("admin", "Fred", "Jane")); + } + + @Test + public void migrateSimpleUser() throws IOException { + File usersDirectory = createTestDirectory(getClass(), name); + IdStrategy idStrategy = IdStrategy.CASE_INSENSITIVE; + UserIdMigrator migrator = new UserIdMigrator(usersDirectory, idStrategy); + TestUserIdMapper mapper = new TestUserIdMapper(usersDirectory, idStrategy); + mapper.init(); + assertThat(migrator.needsMigration(), is(false)); + mapper = new TestUserIdMapper(usersDirectory, idStrategy); + mapper.init(); + assertThat(mapper.getConvertedUserIds().size(), is(1)); + assertThat(mapper.isMapped("fred"), is(true)); + } + + @Test + public void migrateMultipleUsers() throws IOException { + File usersDirectory = createTestDirectory(getClass(), name); + IdStrategy idStrategy = IdStrategy.CASE_INSENSITIVE; + UserIdMigrator migrator = new UserIdMigrator(usersDirectory, idStrategy); + TestUserIdMapper mapper = new TestUserIdMapper(usersDirectory, idStrategy); + mapper.init(); + assertThat(migrator.needsMigration(), is(false)); + mapper = new TestUserIdMapper(usersDirectory, idStrategy); + mapper.init(); + assertThat(mapper.getConvertedUserIds().size(), is(3)); + assertThat(mapper.isMapped("fred"), is(true)); + assertThat(mapper.isMapped("foo/bar"), is(true)); + assertThat(mapper.isMapped("zzz\u1000"), is(true)); + } + + @Test + public void migrateUsersXml() throws IOException { + File usersDirectory = createTestDirectory(getClass(), name); + IdStrategy idStrategy = IdStrategy.CASE_INSENSITIVE; + UserIdMigrator migrator = new UserIdMigrator(usersDirectory, idStrategy); + TestUserIdMapper mapper = new TestUserIdMapper(usersDirectory, idStrategy); + mapper.init(); + assertThat(migrator.needsMigration(), is(false)); + mapper = new TestUserIdMapper(usersDirectory, idStrategy); + mapper.init(); + assertThat(mapper.getConvertedUserIds().size(), is(1)); + assertThat(mapper.isMapped("users.xml"), is(true)); + } + + @Test + public void migrateEntireDirectory() throws IOException { + File usersDirectory = createTestDirectory(getClass(), name); + IdStrategy idStrategy = IdStrategy.CASE_INSENSITIVE; + UserIdMigrator migrator = new UserIdMigrator(usersDirectory, idStrategy); + TestUserIdMapper mapper = new TestUserIdMapper(usersDirectory, idStrategy); + mapper.init(); + File fredDirectory = mapper.getDirectory("fred"); + File otherFile = new File(fredDirectory, "otherfile.txt"); + assertThat(otherFile.exists(), is(true)); + File originalFredDirectory = new File(usersDirectory, "fred"); + assertThat(originalFredDirectory.exists(), is(false)); + } + + static File createTestDirectory(Class clazz, TestName testName) throws IOException { + File tempDirectory = Files.createTempDirectory(Paths.get("target"), "userIdMigratorTest").toFile(); + tempDirectory.deleteOnExit(); + copyTestDataIfExists(clazz, testName, tempDirectory); + return new File(tempDirectory, "users"); + } + + static void copyTestDataIfExists(Class clazz, TestName testName, File tempDirectory) throws IOException { + File resourcesDirectory = new File(BASE_RESOURCE_PATH + clazz.getSimpleName(), testName.getMethodName()); + if (resourcesDirectory.exists()) { + FileUtils.copyDirectory(resourcesDirectory, tempDirectory); + } + } + + private UserIdMigrator createUserIdMigrator() throws IOException { + File usersDirectory = createTestDirectory(getClass(), name); + return new UserIdMigrator(usersDirectory, IdStrategy.CASE_INSENSITIVE); + } + +} diff --git a/core/src/test/java/hudson/scheduler/CronTabTest.java b/core/src/test/java/hudson/scheduler/CronTabTest.java index cae0fdc7ed..66894c3546 100644 --- a/core/src/test/java/hudson/scheduler/CronTabTest.java +++ b/core/src/test/java/hudson/scheduler/CronTabTest.java @@ -321,4 +321,10 @@ public class CronTabTest { new CronTab("0 0 31 7 *").floor(cal); // would infinite loop } + @Issue("SECURITY-1193") + @Test(timeout = 1000L) public void testCeilLongMonths() throws Exception { + Calendar cal = Calendar.getInstance(); + cal.set(Calendar.MONTH, Calendar.NOVEMBER); + new CronTab("0 0 31 * *").ceil(cal); // would infinite loop + } } diff --git a/core/src/test/java/jenkins/model/IdStrategyTest.java b/core/src/test/java/jenkins/model/IdStrategyTest.java index 70b1599edb..ea84528106 100644 --- a/core/src/test/java/jenkins/model/IdStrategyTest.java +++ b/core/src/test/java/jenkins/model/IdStrategyTest.java @@ -2,101 +2,233 @@ package jenkins.model; import org.junit.Test; -import java.util.Locale; - -import static org.junit.Assert.assertEquals; +import static org.hamcrest.core.Is.is; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; public class IdStrategyTest { - private IdStrategy idStrategy; @Test public void caseInsensitive() { - idStrategy = new IdStrategy.CaseInsensitive(); - assertRestrictedNames(); + IdStrategy idStrategy = new IdStrategy.CaseInsensitive(); - assertRoundTrip("foo", "foo"); - assertRoundTrip("foo/bar", "foo$002fbar"); - assertRoundTrip("../test", "..$002ftest"); - assertRoundTrip("0123 _-@~a", "0123 _-@$007ea"); - assertRoundTrip("foo.", "foo$002e"); - assertRoundTrip("-foo", "$002dfoo"); + assertRestrictedNames(idStrategy); + + assertThat(idStrategy.idFromFilename("foo"), is("foo")); + assertThat(idStrategy.idFromFilename("foo$002fbar"), is("foo/bar")); + assertThat(idStrategy.idFromFilename("..$002ftest"), is("../test")); + assertThat(idStrategy.idFromFilename("0123 _-@$007ea"), is("0123 _-@~a")); + assertThat(idStrategy.idFromFilename("foo$002e"), is("foo.")); + assertThat(idStrategy.idFromFilename("$002dfoo"), is("-foo")); // Should not return the same username due to case insensitivity - assertCaseInsensitiveRoundTrip("Foo", "foo"); - assertCaseInsensitiveRoundTrip("Foo/Bar", "foo$002fbar"); - assertCaseInsensitiveRoundTrip("../Test", "..$002ftest"); - assertCaseInsensitiveRoundTrip("NUL", "$006eul"); - assertEquals("foo", idStrategy.idFromFilename("~foo")); - assertEquals("0123 _-@a", idStrategy.idFromFilename("0123 _-@~a")); - assertEquals("big$money", idStrategy.idFromFilename("big$money")); + assertThat(idStrategy.idFromFilename("Foo"), is("foo")); + assertThat(idStrategy.idFromFilename("Foo$002fBar"), is("foo/bar")); + assertThat(idStrategy.idFromFilename("..$002fTest"), is("../test")); + assertThat(idStrategy.idFromFilename("$006eul"), is("nul")); + + assertThat(idStrategy.idFromFilename("~foo"), is("~foo")); + assertThat(idStrategy.idFromFilename("0123 _-@~a"), is("0123 _-@~a")); + assertThat(idStrategy.idFromFilename("big$money"), is("big$money")); + + assertThat(idStrategy.idFromFilename("$00c1aaa"), is("\u00e1aaa")); + assertThat(idStrategy.idFromFilename("$00e1aaa"), is("\u00e1aaa")); + assertThat(idStrategy.idFromFilename("aaaa$00e1"), is("aaaa\u00e1")); + assertThat(idStrategy.idFromFilename("aaaa$00e1kkkk"), is("aaaa\u00e1kkkk")); + assertThat(idStrategy.idFromFilename("aa$00e1zz$00e9pp"), is("aa\u00e1zz\u00e9pp")); + assertThat(idStrategy.idFromFilename("$306f$56fd$5185$3067$6700$5927"), is("\u306f\u56fd\u5185\u3067\u6700\u5927")); + + assertThat(idStrategy.idFromFilename("$00E1aaa"), is("$00e1aaa")); + assertThat(idStrategy.idFromFilename("$001gggg"), is("$001gggg")); + assertThat(idStrategy.idFromFilename("rrr$t123"), is("rrr$t123")); + } + + @Test + public void caseInsensitivePassesThroughOldLegacy() { + IdStrategy idStrategy = new IdStrategy.CaseInsensitive(); + + assertThat(idStrategy.idFromFilename("make\u1000000"), is("make\u1000000")); + assertThat(idStrategy.idFromFilename("\u306f\u56fd\u5185\u3067\u6700\u5927"), is("\u306f\u56fd\u5185\u3067\u6700\u5927")); + assertThat(idStrategy.idFromFilename("~fred"), is("~fred")); + assertThat(idStrategy.idFromFilename("~1fred"), is("~1fred")); } @Test public void caseSensitive() { - idStrategy = new IdStrategy.CaseSensitive(); - assertRestrictedNames(); + IdStrategy idStrategy = new IdStrategy.CaseSensitive(); - assertRoundTrip("foo", "foo"); - assertRoundTrip("Foo", "~foo"); - assertRoundTrip("foo/bar", "foo$002fbar"); - assertRoundTrip("Foo/Bar", "~foo$002f~bar"); - assertRoundTrip("../test", "..$002ftest"); - assertRoundTrip("../Test", "..$002f~test"); - assertRoundTrip("0123 _-@~a", "0123 _-@$007ea"); - assertRoundTrip("0123 _-@A", "0123 _-@~a"); - assertRoundTrip("foo.", "foo$002e"); - assertRoundTrip("-foo", "$002dfoo"); - assertRoundTrip("Con", "~con"); - assertRoundTrip("Prn", "~prn"); - assertRoundTrip("Aux", "~aux"); - assertRoundTrip("Nul", "~nul"); - assertRoundTrip("Com1", "~com1"); - assertRoundTrip("Lpt1", "~lpt1"); + assertRestrictedNames(idStrategy); + + assertThat(idStrategy.idFromFilename("foo"), is("foo")); + assertThat(idStrategy.idFromFilename("~foo"), is("Foo")); + assertThat(idStrategy.idFromFilename("foo$002fbar"), is("foo/bar")); + assertThat(idStrategy.idFromFilename("~foo$002f~bar"), is("Foo/Bar")); + assertThat(idStrategy.idFromFilename("..$002ftest"), is("../test")); + assertThat(idStrategy.idFromFilename("..$002f~test"), is("../Test")); + assertThat(idStrategy.idFromFilename("0123 _-@$007ea"), is("0123 _-@~a")); + assertThat(idStrategy.idFromFilename("0123 _-@~a"), is("0123 _-@A")); + assertThat(idStrategy.idFromFilename("foo$002e"), is("foo.")); + assertThat(idStrategy.idFromFilename("$002dfoo"), is("-foo")); + assertThat(idStrategy.idFromFilename("~con"), is("Con")); + assertThat(idStrategy.idFromFilename("~prn"), is("Prn")); + assertThat(idStrategy.idFromFilename("~aux"), is("Aux")); + assertThat(idStrategy.idFromFilename("~nul"), is("Nul")); + assertThat(idStrategy.idFromFilename("~com1"), is("Com1")); + assertThat(idStrategy.idFromFilename("~lpt1"), is("Lpt1")); + assertThat(idStrategy.idFromFilename("big$money"), is("big$money")); + + assertThat(idStrategy.idFromFilename("$00c1aaa"), is("\u00c1aaa")); + assertThat(idStrategy.idFromFilename("$00e1aaa"), is("\u00e1aaa")); + assertThat(idStrategy.idFromFilename("aaaa$00e1"), is("aaaa\u00e1")); + assertThat(idStrategy.idFromFilename("aaaa$00e1kkkk"), is("aaaa\u00e1kkkk")); + assertThat(idStrategy.idFromFilename("aa$00e1zz$00e9pp"), is("aa\u00e1zz\u00e9pp")); + assertThat(idStrategy.idFromFilename("$306f$56fd$5185$3067$6700$5927"), is("\u306f\u56fd\u5185\u3067\u6700\u5927")); + + assertThat(idStrategy.idFromFilename("$00E1aaa"), is("$00E1aaa")); + assertThat(idStrategy.idFromFilename("$001gggg"), is("$001gggg")); + assertThat(idStrategy.idFromFilename("rRr$t123"), is("rRr$t123")); + + assertThat(idStrategy.idFromFilename("iiii _-@$007~ea"), is("iiii _-@$007Ea")); } - private void assertRestrictedNames() { - assertEquals("$002f", idStrategy.filenameOf(".")); - // "." and "/" are equivalent from an implementation standpoint, but both should return "/" - assertEquals("/", idStrategy.idFromFilename(idStrategy.filenameOf("."))); + @Test + public void caseSensitivePassesThroughOldLegacy() { + IdStrategy idStrategy = new IdStrategy.CaseSensitive(); - assertEquals("$002f", idStrategy.filenameOf("")); - // "" and "/" are equivalent from an implementation standpoint, but both should return "/" - assertEquals("/", idStrategy.idFromFilename(idStrategy.filenameOf(""))); - - assertRoundTrip("/", "$002f"); - assertRoundTrip("..", "$002e$002e"); - assertRoundTrip("con", "$0063on"); - assertRoundTrip("prn", "$0070rn"); - assertRoundTrip("aux", "$0061ux"); - assertRoundTrip("nul", "$006eul"); - assertRoundTrip("com1", "$0063om1"); - assertRoundTrip("com2", "$0063om2"); - assertRoundTrip("com3", "$0063om3"); - assertRoundTrip("com4", "$0063om4"); - assertRoundTrip("com5", "$0063om5"); - assertRoundTrip("com6", "$0063om6"); - assertRoundTrip("com7", "$0063om7"); - assertRoundTrip("com8", "$0063om8"); - assertRoundTrip("com9", "$0063om9"); - assertRoundTrip("lpt1", "$006cpt1"); - assertRoundTrip("lpt2", "$006cpt2"); - assertRoundTrip("lpt3", "$006cpt3"); - assertRoundTrip("lpt4", "$006cpt4"); - assertRoundTrip("lpt5", "$006cpt5"); - assertRoundTrip("lpt6", "$006cpt6"); - assertRoundTrip("lpt7", "$006cpt7"); - assertRoundTrip("lpt8", "$006cpt8"); - assertRoundTrip("lpt9", "$006cpt9"); + assertThat(idStrategy.idFromFilename("make\u1000000"), is("make\u1000000")); + assertThat(idStrategy.idFromFilename("\u306f\u56fd\u5185\u3067\u6700\u5927"), is("\u306f\u56fd\u5185\u3067\u6700\u5927")); + assertThat(idStrategy.idFromFilename("~1fred"), is("~1fred")); } - private void assertRoundTrip(String username, String filename) { - assertEquals(filename, idStrategy.filenameOf(username)); - assertEquals(username, idStrategy.idFromFilename(filename)); + @Test + public void testEqualsCaseInsensitive() { + IdStrategy idStrategy = IdStrategy.CASE_INSENSITIVE; + assertTrue(idStrategy.equals("user1", "User1")); + assertTrue(idStrategy.equals("User1", "user1")); + assertFalse(idStrategy.equals("User1", "user2")); + String sameUser = "sameUser"; + assertTrue(idStrategy.equals(sameUser, sameUser)); } - private void assertCaseInsensitiveRoundTrip(String username, String filename) { - assertEquals(filename, idStrategy.filenameOf(username)); - assertEquals(username.toLowerCase(Locale.ENGLISH), idStrategy.idFromFilename(filename)); + @Test + public void testEqualsCaseSensitive() { + IdStrategy idStrategy = new IdStrategy.CaseSensitive(); + assertFalse(idStrategy.equals("user1", "User1")); + assertFalse(idStrategy.equals("User1", "user1")); + assertFalse(idStrategy.equals("User1", "user2")); + String sameUser = "sameUser"; + assertTrue(idStrategy.equals(sameUser, sameUser)); } + + @Test + public void testEqualsCaseSensitiveEmailAddress() { + IdStrategy idStrategy = new IdStrategy.CaseSensitiveEmailAddress(); + assertFalse(idStrategy.equals("john.smith@acme.org", "John.Smith@acme.org")); + assertFalse(idStrategy.equals("john.smith@acme.org", "John.Smith@ACME.org")); + assertFalse(idStrategy.equals("john.smith@acme.org", "John.Smith@ACME.org")); + assertFalse(idStrategy.equals("john.smith@acme.org", "John.Smith@acme.ORG")); + assertFalse(idStrategy.equals("John@smith@acme.org", "john@Smith@acme.ORG")); + String sameUser = "john.smith@acme.org"; + assertTrue(idStrategy.equals(sameUser, sameUser)); + assertTrue(idStrategy.equals("John.Smith@ACME.org", "John.Smith@acme.org")); + assertTrue(idStrategy.equals("John.Smith@acme.ORG", "John.Smith@acme.org")); + assertTrue(idStrategy.equals("john@smith@ACME.org", "john@smith@acme.org")); + } + + @Test + public void testKeyForCaseInsensitive() { + IdStrategy idStrategy = IdStrategy.CASE_INSENSITIVE; + assertThat(idStrategy.keyFor("user1"), is("user1")); + assertThat(idStrategy.keyFor("User1"), is("user1")); + assertThat(idStrategy.keyFor("USER1"), is("user1")); + } + + @Test + public void testKeyForCaseSensitive() { + IdStrategy idStrategy = new IdStrategy.CaseSensitive(); + assertThat(idStrategy.keyFor("user1"), is("user1")); + assertThat(idStrategy.keyFor("User1"), is("User1")); + assertThat(idStrategy.keyFor("USER1"), is("USER1")); + } + + @Test + public void testKeyForCaseSensitiveEmailAddress() { + IdStrategy idStrategy = new IdStrategy.CaseSensitiveEmailAddress(); + assertThat(idStrategy.keyFor("john.smith@acme.org"), is("john.smith@acme.org")); + assertThat(idStrategy.keyFor("John.Smith@acme.org"), is("John.Smith@acme.org")); + assertThat(idStrategy.keyFor("John.Smith@ACME.org"), is("John.Smith@acme.org")); + assertThat(idStrategy.keyFor("John.Smith@acme.ORG"), is("John.Smith@acme.org")); + assertThat(idStrategy.keyFor("john.smith"), is("john.smith")); + assertThat(idStrategy.keyFor("John.Smith"), is("John.Smith")); + assertThat(idStrategy.keyFor("john@smith@acme.org"), is("john@smith@acme.org")); + assertThat(idStrategy.keyFor("John@Smith@acme.org"), is("John@Smith@acme.org")); + } + + @Test + public void testCompareCaseInsensitive() { + IdStrategy idStrategy = IdStrategy.CASE_INSENSITIVE; + assertTrue(idStrategy.compare("user1", "user2") < 0); + assertTrue(idStrategy.compare("user2", "user1") > 0); + assertTrue(idStrategy.compare("user1", "user1") == 0); + assertTrue(idStrategy.compare("USER1", "user2") < 0); + assertTrue(idStrategy.compare("USER2", "user1") > 0); + assertTrue(idStrategy.compare("User1", "user1") == 0); + } + + @Test + public void testCompareCaseSensitive() { + IdStrategy idStrategy = new IdStrategy.CaseSensitive(); + assertTrue(idStrategy.compare("user1", "user2") < 0); + assertTrue(idStrategy.compare("user2", "user1") > 0); + assertTrue(idStrategy.compare("user1", "user1") == 0); + assertTrue(idStrategy.compare("USER1", "user2") < 0); + assertTrue(idStrategy.compare("USER2", "user1") < 0); + assertTrue(idStrategy.compare("User1", "user1") < 0); + } + + @Test + public void testCompareCaseSensitiveEmail() { + IdStrategy idStrategy = new IdStrategy.CaseSensitiveEmailAddress(); + assertTrue(idStrategy.compare("john.smith@acme.org", "john.smith@acme.org") == 0); + assertTrue(idStrategy.compare("John.Smith@acme.org", "John.Smith@acme.org") == 0); + assertTrue(idStrategy.compare("John.Smith@ACME.org", "John.Smith@acme.org") == 0); + assertTrue(idStrategy.compare("John.Smith@acme.ORG", "John.Smith@acme.org") == 0); + assertTrue(idStrategy.compare("john.smith", "john.smith") == 0); + assertTrue(idStrategy.compare("John.Smith", "John.Smith") == 0); + assertTrue(idStrategy.compare("john@smith@acme.org", "john@smith@acme.org") == 0); + assertTrue(idStrategy.compare("John@Smith@acme.org", "John@Smith@acme.org") == 0); + + assertTrue(idStrategy.compare("John.Smith@acme.org", "john.smith@acme.org") < 0); + assertTrue(idStrategy.compare("john.smith@acme.org", "John.Smith@acme.org") > 0); + } + + private void assertRestrictedNames(IdStrategy idStrategy) { + assertThat(idStrategy.idFromFilename("$002f"), is("/")); + + assertThat(idStrategy.idFromFilename("$002e$002e"), is("..")); + assertThat(idStrategy.idFromFilename("$0063on"), is("con")); + assertThat(idStrategy.idFromFilename("$0070rn"), is("prn")); + assertThat(idStrategy.idFromFilename("$0061ux"), is("aux")); + assertThat(idStrategy.idFromFilename("$006eul"), is("nul")); + assertThat(idStrategy.idFromFilename("$0063om1"), is("com1")); + assertThat(idStrategy.idFromFilename("$0063om2"), is("com2")); + assertThat(idStrategy.idFromFilename("$0063om3"), is("com3")); + assertThat(idStrategy.idFromFilename("$0063om4"), is("com4")); + assertThat(idStrategy.idFromFilename("$0063om5"), is("com5")); + assertThat(idStrategy.idFromFilename("$0063om6"), is("com6")); + assertThat(idStrategy.idFromFilename("$0063om7"), is("com7")); + assertThat(idStrategy.idFromFilename("$0063om8"), is("com8")); + assertThat(idStrategy.idFromFilename("$0063om9"), is("com9")); + assertThat(idStrategy.idFromFilename("$006cpt1"), is("lpt1")); + assertThat(idStrategy.idFromFilename("$006cpt2"), is("lpt2")); + assertThat(idStrategy.idFromFilename("$006cpt3"), is("lpt3")); + assertThat(idStrategy.idFromFilename("$006cpt4"), is("lpt4")); + assertThat(idStrategy.idFromFilename("$006cpt5"), is("lpt5")); + assertThat(idStrategy.idFromFilename("$006cpt6"), is("lpt6")); + assertThat(idStrategy.idFromFilename("$006cpt7"), is("lpt7")); + assertThat(idStrategy.idFromFilename("$006cpt8"), is("lpt8")); + assertThat(idStrategy.idFromFilename("$006cpt9"), is("lpt9")); + } + } diff --git a/core/src/test/java/jenkins/util/VirtualFileSEC904Test.java b/core/src/test/java/jenkins/util/VirtualFileSEC904Test.java new file mode 100644 index 0000000000..8b25c1c040 --- /dev/null +++ b/core/src/test/java/jenkins/util/VirtualFileSEC904Test.java @@ -0,0 +1,180 @@ +/* + * The MIT License + * + * Copyright 2015 Jesse Glick. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package jenkins.util; + +import hudson.FilePath; +import hudson.model.TaskListener; +import org.apache.commons.io.FileUtils; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.junit.rules.TemporaryFolder; +import org.jvnet.hudson.test.Issue; + +import java.io.File; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +//TODO merge into VirtualFileTest after the security release +public class VirtualFileSEC904Test { + + @Rule + public TemporaryFolder tmp = new TemporaryFolder(); + + // root + // /a + // /aa + // /aaa + // /_b2 => symlink to /root/b + // aa.txt + // /ab + // ab.txt + // /_b => symlink to /root/b + // /b + // /_a => symlink to /root/a + // /ba + // ba.txt + private void prepareFileStructureForIsDescendant() throws Exception { + File root = tmp.getRoot(); + File a = new File(root, "a"); + File aa = new File(a, "aa"); + File aaa = new File(aa, "aaa"); + aaa.mkdirs(); + File aaTxt = new File(aa, "aa.txt"); + FileUtils.write(aaTxt, "aa"); + + File ab = new File(a, "ab"); + ab.mkdirs(); + File abTxt = new File(ab, "ab.txt"); + FileUtils.write(abTxt, "ab"); + + File b = new File(root, "b"); + + File ba = new File(b, "ba"); + ba.mkdirs(); + File baTxt = new File(ba, "ba.txt"); + FileUtils.write(baTxt, "ba"); + + File _a = new File(b, "_a"); + new FilePath(_a).symlinkTo(a.getAbsolutePath(), TaskListener.NULL); + + File _b = new File(a, "_b"); + new FilePath(_b).symlinkTo(b.getAbsolutePath(), TaskListener.NULL); + File _b2 = new File(aaa, "_b2"); + new FilePath(_b2).symlinkTo(b.getAbsolutePath(), TaskListener.NULL); + } + + @Issue("SECURITY-904") + @Test public void forFile_isDescendant() throws Exception { + this.prepareFileStructureForIsDescendant(); + + File root = tmp.getRoot(); + File a = new File(root, "a"); + File aa = new File(a, "aa"); + VirtualFile virtualRoot = VirtualFile.forFile(root); + // keep the root information for isDescendant + VirtualFile virtualRootChildA = virtualRoot.child("a"); + VirtualFile virtualFromA = VirtualFile.forFile(a); + + checkCommonAssertionForIsDescendant(virtualRoot, virtualRootChildA, virtualFromA, aa.getAbsolutePath()); + } + + @Test + @Issue("SECURITY-904") + public void forFilePath_isDescendant() throws Exception { + this.prepareFileStructureForIsDescendant(); + + File root = tmp.getRoot(); + File a = new File(root, "a"); + File aa = new File(a, "aa"); + VirtualFile virtualRoot = VirtualFile.forFilePath(new FilePath(root)); + // keep the root information for isDescendant + VirtualFile virtualRootChildA = virtualRoot.child("a"); + VirtualFile virtualFromA = VirtualFile.forFilePath(new FilePath(a)); + + checkCommonAssertionForIsDescendant(virtualRoot, virtualRootChildA, virtualFromA, aa.getAbsolutePath()); + } + + private void checkCommonAssertionForIsDescendant(VirtualFile virtualRoot, VirtualFile virtualRootChildA, VirtualFile virtualFromA, String absolutePath) throws Exception { + try { + virtualRootChildA.isDescendant(absolutePath); + fail("isDescendant should have refused the absolute path"); + } catch (IllegalArgumentException e) {} + + assertTrue(virtualRootChildA.isDescendant("aa")); + assertTrue(virtualRootChildA.isDescendant("aa/aa.txt")); + assertTrue(virtualRootChildA.isDescendant("aa\\aa.txt")); + assertTrue(virtualRootChildA.isDescendant("ab")); + assertTrue(virtualRootChildA.isDescendant("ab/ab.txt")); + assertTrue(virtualRootChildA.isDescendant("ab//ab.txt")); + assertTrue(virtualRootChildA.isDescendant("ab/nonExistingFile.txt")); + assertTrue(virtualRootChildA.isDescendant("nonExistingFolder")); + assertTrue(virtualRootChildA.isDescendant("nonExistingFolder/nonExistingFile.txt")); + + assertTrue(virtualRootChildA.isDescendant("_b")); + assertTrue(virtualRootChildA.isDescendant("_b/ba")); + assertTrue(virtualRootChildA.isDescendant("_b/ba/ba.txt")); + assertTrue(virtualRootChildA.isDescendant("aa/aaa/_b2")); + assertTrue(virtualRootChildA.isDescendant("aa/aaa/_b2/ba")); + assertTrue(virtualRootChildA.isDescendant("aa/aaa/_b2/ba/ba.txt")); + + // such approach could be used to check existence of file inside symlink + assertTrue(virtualRootChildA.isDescendant("_b/ba/ba-unexistingFile.txt")); + + // we go outside, then inside = forbidden, could be used to check existence of symlinks + assertTrue(virtualRootChildA.isDescendant("_b/_a")); + assertTrue(virtualRootChildA.isDescendant("_b/_a/aa")); + assertTrue(virtualRootChildA.isDescendant("_b/_a/aa/aa.txt")); + + assertTrue(virtualFromA.isDescendant("aa")); + assertFalse(virtualFromA.isDescendant("_b")); + assertFalse(virtualFromA.isDescendant("_b/ba/ba-unexistingFile.txt")); + assertFalse(virtualFromA.isDescendant("_b/_a")); + assertFalse(virtualFromA.isDescendant("_b/_a/aa")); + assertFalse(virtualFromA.isDescendant("_b/_a/aa/aa.txt")); + assertFalse(virtualFromA.isDescendant("aa/aaa/_b2")); + assertFalse(virtualFromA.isDescendant("aa/aaa/_b2/ba")); + assertFalse(virtualFromA.isDescendant("aa/aaa/_b2/ba/ba.txt")); + + assertTrue(virtualRoot.isDescendant("aa")); + assertTrue(virtualRoot.isDescendant("aa/aa.txt")); + assertTrue(virtualRoot.isDescendant("ab")); + assertTrue(virtualRoot.isDescendant("ab/ab.txt")); + assertTrue(virtualRoot.isDescendant("ab/nonExistingFile.txt")); + assertTrue(virtualRoot.isDescendant("nonExistingFolder")); + assertTrue(virtualRoot.isDescendant("nonExistingFolder/nonExistingFile.txt")); + + assertTrue(virtualRoot.isDescendant("_b")); + assertTrue(virtualRoot.isDescendant("_b/ba")); + assertTrue(virtualRoot.isDescendant("_b/ba/ba.txt")); + assertTrue(virtualRoot.isDescendant("_b/_a")); + assertTrue(virtualRoot.isDescendant("_b/_a/aa")); + assertTrue(virtualRoot.isDescendant("_b/_a/aa/")); + assertTrue(virtualRoot.isDescendant("_b/_a/aa/../ab/ab.txt")); + assertTrue(virtualRoot.isDescendant("_b/_a/aa/aa.txt")); + } +} diff --git a/core/src/test/resources/hudson/model/UserIdMapperTest/testDuplicatedUserId/users/users.xml b/core/src/test/resources/hudson/model/UserIdMapperTest/testDuplicatedUserId/users/users.xml new file mode 100644 index 0000000000..b094fd36f9 --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMapperTest/testDuplicatedUserId/users/users.xml @@ -0,0 +1,17 @@ + + + + + user1 + user2957281690483826750 + + + user1 + user6639871027538302990 + + + user2 + user22222 + + + diff --git a/core/src/test/resources/hudson/model/UserIdMapperTest/testXmlFileCorrupted/users/users.xml b/core/src/test/resources/hudson/model/UserIdMapperTest/testXmlFileCorrupted/users/users.xml new file mode 100644 index 0000000000..ad31ae2638 --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMapperTest/testXmlFileCorrupted/users/users.xml @@ -0,0 +1,5 @@ + + + fred + Fred Smith + diff --git a/test/src/test/resources/hudson/model/UserTest/emptyUsernameConfigMigrated/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/emptyUsernameConfigScanned/config.xml similarity index 100% rename from test/src/test/resources/hudson/model/UserTest/emptyUsernameConfigMigrated/config.xml rename to core/src/test/resources/hudson/model/UserIdMigratorTest/emptyUsernameConfigScanned/config.xml diff --git a/test/src/test/resources/hudson/model/UserTest/emptyUsernameConfigMigrated/users/admin/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/emptyUsernameConfigScanned/users/admin/config.xml similarity index 100% rename from test/src/test/resources/hudson/model/UserTest/emptyUsernameConfigMigrated/users/admin/config.xml rename to core/src/test/resources/hudson/model/UserIdMigratorTest/emptyUsernameConfigScanned/users/admin/config.xml diff --git a/test/src/test/resources/hudson/model/UserTest/emptyUsernameConfigMigrated/users/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/emptyUsernameConfigScanned/users/config.xml similarity index 100% rename from test/src/test/resources/hudson/model/UserTest/emptyUsernameConfigMigrated/users/config.xml rename to core/src/test/resources/hudson/model/UserIdMigratorTest/emptyUsernameConfigScanned/users/config.xml diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/migrateEntireDirectory/users/fred/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/migrateEntireDirectory/users/fred/config.xml new file mode 100644 index 0000000000..ad31ae2638 --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/migrateEntireDirectory/users/fred/config.xml @@ -0,0 +1,5 @@ + + + fred + Fred Smith + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/migrateEntireDirectory/users/fred/otherfile.txt b/core/src/test/resources/hudson/model/UserIdMigratorTest/migrateEntireDirectory/users/fred/otherfile.txt new file mode 100644 index 0000000000..1b376877f4 --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/migrateEntireDirectory/users/fred/otherfile.txt @@ -0,0 +1 @@ +Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/migrateMultipleUsers/users/foo$002fbar/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/migrateMultipleUsers/users/foo$002fbar/config.xml new file mode 100644 index 0000000000..fddfc41176 --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/migrateMultipleUsers/users/foo$002fbar/config.xml @@ -0,0 +1,4 @@ + + + Foo Bar + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/migrateMultipleUsers/users/fred/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/migrateMultipleUsers/users/fred/config.xml new file mode 100644 index 0000000000..ad31ae2638 --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/migrateMultipleUsers/users/fred/config.xml @@ -0,0 +1,5 @@ + + + fred + Fred Smith + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/migrateMultipleUsers/users/zzzက/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/migrateMultipleUsers/users/zzzက/config.xml new file mode 100644 index 0000000000..9216f71ae3 --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/migrateMultipleUsers/users/zzzက/config.xml @@ -0,0 +1,4 @@ + + + Old Legacy + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/migrateSimpleUser/users/fred/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/migrateSimpleUser/users/fred/config.xml new file mode 100644 index 0000000000..ad31ae2638 --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/migrateSimpleUser/users/fred/config.xml @@ -0,0 +1,5 @@ + + + fred + Fred Smith + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/migrateUsersXml/users/users.xml/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/migrateUsersXml/users/users.xml/config.xml new file mode 100644 index 0000000000..ad31ae2638 --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/migrateUsersXml/users/users.xml/config.xml @@ -0,0 +1,5 @@ + + + fred + Fred Smith + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/needsMigrationBasic/users/jane_4975613142751794216/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/needsMigrationBasic/users/jane_4975613142751794216/config.xml new file mode 100644 index 0000000000..6465bde25c --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/needsMigrationBasic/users/jane_4975613142751794216/config.xml @@ -0,0 +1,5 @@ + + + jane + Fred Smith + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/needsMigrationFalse/users/users.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/needsMigrationFalse/users/users.xml new file mode 100644 index 0000000000..c8017032d1 --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/needsMigrationFalse/users/users.xml @@ -0,0 +1,9 @@ + + + + + fred + user4975613142751794216 + + + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/needsMigrationNoUserConfigFiles/users/users.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/needsMigrationNoUserConfigFiles/users/users.xml new file mode 100644 index 0000000000..c8017032d1 --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/needsMigrationNoUserConfigFiles/users/users.xml @@ -0,0 +1,9 @@ + + + + + fred + user4975613142751794216 + + + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersBasic/users/admin/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersBasic/users/admin/config.xml new file mode 100644 index 0000000000..3eae7e9ffa --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersBasic/users/admin/config.xml @@ -0,0 +1,3 @@ + + + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersBasic/users/jane/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersBasic/users/jane/config.xml new file mode 100644 index 0000000000..3eae7e9ffa --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersBasic/users/jane/config.xml @@ -0,0 +1,3 @@ + + + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersCaseSensitive/users/Fred/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersCaseSensitive/users/Fred/config.xml new file mode 100644 index 0000000000..3eae7e9ffa --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersCaseSensitive/users/Fred/config.xml @@ -0,0 +1,3 @@ + + + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersCaseSensitive/users/admin/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersCaseSensitive/users/admin/config.xml new file mode 100644 index 0000000000..3eae7e9ffa --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersCaseSensitive/users/admin/config.xml @@ -0,0 +1,3 @@ + + + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersCaseSensitive/users/~jane/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersCaseSensitive/users/~jane/config.xml new file mode 100644 index 0000000000..3eae7e9ffa --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersCaseSensitive/users/~jane/config.xml @@ -0,0 +1,3 @@ + + + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/$002e$002e/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/$002e$002e/config.xml new file mode 100644 index 0000000000..3eae7e9ffa --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/$002e$002e/config.xml @@ -0,0 +1,3 @@ + + + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/$002f/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/$002f/config.xml new file mode 100644 index 0000000000..3eae7e9ffa --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/$002f/config.xml @@ -0,0 +1,3 @@ + + + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/big$money/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/big$money/config.xml new file mode 100644 index 0000000000..3eae7e9ffa --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/big$money/config.xml @@ -0,0 +1,3 @@ + + + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/bla$0024phem.us/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/bla$0024phem.us/config.xml new file mode 100644 index 0000000000..3eae7e9ffa --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/bla$0024phem.us/config.xml @@ -0,0 +1,3 @@ + + + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/foo$002fbar$002fbaz/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/foo$002fbar$002fbaz/config.xml new file mode 100644 index 0000000000..3eae7e9ffa --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/foo$002fbar$002fbaz/config.xml @@ -0,0 +1,3 @@ + + + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/foo$002fbar/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/foo$002fbar/config.xml new file mode 100644 index 0000000000..3eae7e9ffa --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/foo$002fbar/config.xml @@ -0,0 +1,3 @@ + + + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/make$00241000000/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/make$00241000000/config.xml new file mode 100644 index 0000000000..3eae7e9ffa --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/make$00241000000/config.xml @@ -0,0 +1,3 @@ + + + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/~com1/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/~com1/config.xml new file mode 100644 index 0000000000..3eae7e9ffa --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersLegacy/users/~com1/config.xml @@ -0,0 +1,3 @@ + + + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersOldLegacy/users/makeက000/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersOldLegacy/users/makeက000/config.xml new file mode 100644 index 0000000000..3eae7e9ffa --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersOldLegacy/users/makeက000/config.xml @@ -0,0 +1,3 @@ + + + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersOldLegacy/users/zzzက/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersOldLegacy/users/zzzက/config.xml new file mode 100644 index 0000000000..3eae7e9ffa --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersOldLegacy/users/zzzက/config.xml @@ -0,0 +1,3 @@ + + + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersOldLegacy/users/ကyyy/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersOldLegacy/users/ကyyy/config.xml new file mode 100644 index 0000000000..3eae7e9ffa --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersOldLegacy/users/ကyyy/config.xml @@ -0,0 +1,3 @@ + + + diff --git a/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersOldLegacy/users/は国内で最大/config.xml b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersOldLegacy/users/は国内で最大/config.xml new file mode 100644 index 0000000000..3eae7e9ffa --- /dev/null +++ b/core/src/test/resources/hudson/model/UserIdMigratorTest/scanExistingUsersOldLegacy/users/は国内で最大/config.xml @@ -0,0 +1,3 @@ + + + diff --git a/test/src/test/java/hudson/cli/ReloadConfigurationCommandTest.java b/test/src/test/java/hudson/cli/ReloadConfigurationCommandTest.java index 1f4094dfdf..3d9a9f4b3f 100644 --- a/test/src/test/java/hudson/cli/ReloadConfigurationCommandTest.java +++ b/test/src/test/java/hudson/cli/ReloadConfigurationCommandTest.java @@ -107,19 +107,21 @@ public class ReloadConfigurationCommandTest { @Test public void reloadUserConfig() throws Exception { + String originalName = "oldName"; + String temporaryName = "newName"; { User user = User.get("some_user", true, null); - user.setFullName("oldName"); + user.setFullName(originalName); user.save(); + assertThat(user.getFullName(), equalTo(originalName)); - replace("users/some_user/config.xml", "oldName", "newName"); - - assertThat(user.getFullName(), equalTo("oldName")); + user.setFullName(temporaryName); + assertThat(user.getFullName(), equalTo(temporaryName)); } reloadJenkinsConfigurationViaCliAndWait(); { User user = User.getById("some_user", false); - assertThat(user.getFullName(), equalTo("newName")); + assertThat(user.getFullName(), equalTo(originalName)); } } diff --git a/test/src/test/java/hudson/model/DirectoryBrowserSupportSEC904Test.java b/test/src/test/java/hudson/model/DirectoryBrowserSupportSEC904Test.java new file mode 100644 index 0000000000..06c9b057a4 --- /dev/null +++ b/test/src/test/java/hudson/model/DirectoryBrowserSupportSEC904Test.java @@ -0,0 +1,606 @@ +/* + * The MIT License + * + * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +package hudson.model; + +import com.gargoylesoftware.htmlunit.Page; +import com.gargoylesoftware.htmlunit.UnexpectedPage; +import hudson.FilePath; +import hudson.Functions; +import hudson.tasks.BatchFile; +import hudson.tasks.Shell; +import org.apache.commons.io.FileUtils; +import org.apache.commons.io.IOUtils; +import org.junit.Assume; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.Issue; +import org.jvnet.hudson.test.JenkinsRule; + +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.net.HttpURLConnection; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.URL; +import java.nio.file.Files; +import java.util.List; +import java.util.stream.Collectors; +import java.util.zip.ZipEntry; +import java.util.zip.ZipFile; + +import static org.hamcrest.CoreMatchers.allOf; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.hasSize; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; + +public class DirectoryBrowserSupportSEC904Test { + + @Rule + public JenkinsRule j = new JenkinsRule(); + + @Test + @Issue("SECURITY-904") + public void symlink_outsideWorkspace_areNotAllowed() throws Exception { + FreeStyleProject p = j.createFreeStyleProject(); + + File secretsFolder = new File(j.jenkins.getRootDir(), "secrets"); + File secretTarget = new File(secretsFolder, "goal.txt"); + String secretContent = "secret"; + FileUtils.write(secretTarget, secretContent); + + /* + * secrets/ + * goal.txt + * workspace/ + * intermediateFolder/ + * public2.key + * otherFolder/ + * to_secret3 -> ../../../../secrets/ + * to_secret2 -> ../../../secrets/ + * to_secret_goal2 -> ../../../secrets/goal.txt + * public1.key + * to_secret1 -> ../../secrets/ + * to_secret_goal1 -> ../../secrets/goal.txt + * + */ + if (Functions.isWindows()) { + // no need to test mklink /H since we cannot create an hard link to a non-existing file + // and so you need to have access to the master file system directly which is already a problem + + String script = loadContentFromResource("outsideWorkspaceStructure.bat"); + p.getBuildersList().add(new BatchFile(script)); + } else { + String script = loadContentFromResource("outsideWorkspaceStructure.sh"); + p.getBuildersList().add(new Shell(script)); + } + + assertEquals(Result.SUCCESS, p.scheduleBuild2(0).get().getResult()); + + JenkinsRule.WebClient wc = j.createWebClient(); + wc.getOptions().setThrowExceptionOnFailingStatusCode(false); + { // workspace root must be reachable (regular case) + Page page = wc.goTo(p.getUrl() + "ws/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, allOf( + containsString("public1.key"), + containsString("intermediateFolder"), + containsString("to_secrets1"), + containsString("to_secrets_goal1"), + not(containsString("to_secrets2")), + not(containsString("to_secrets_goal2")) + )); + } + { // to_secrets1 not reachable + Page page = wc.goTo(p.getUrl() + "ws/to_secrets1/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + { // to_secrets_goal1 not reachable + Page page = wc.goTo(p.getUrl() + "ws/to_secrets_goal1/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + { // intermediateFolder must be reachable (regular case) + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, allOf( + not(containsString("to_secrets1")), + not(containsString("to_secrets_goal1")), + containsString("to_secrets2"), + containsString("to_secrets_goal2") + )); + } + { // to_secrets2 not reachable + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + { // using symbolic in the intermediate path + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2/master.key", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + { // to_secrets_goal2 not reachable + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets_goal2/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + + // pattern search feature + { // the pattern allow us to search inside the files / folders, + // without the patch the master.key from inside the outside symlinks would have been linked + Page page = wc.goTo(p.getUrl() + "ws/**/*.key", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, allOf( + not(containsString("master.key")), + containsString("public1.key"), + containsString("public2.key") + )); + } + + // zip feature + { // all the outside folders / files are not included in the zip + Page zipPage = wc.goTo(p.getUrl() + "ws/*zip*/ws.zip", null); + assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + + List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); + assertThat(entryNames, containsInAnyOrder( + p.getName() + "/intermediateFolder/public2.key", + p.getName() + "/public1.key" + )); + } + { // all the outside folders / files are not included in the zip + Page zipPage = wc.goTo(p.getUrl() + "ws/intermediateFolder/*zip*/intermediateFolder.zip", null); + assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + + List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); + assertThat(entryNames, contains("intermediateFolder/public2.key")); + } + } + + /* + * If the glob filter is used, we do not want that it leaks some information. + * Presence of a folder means that the folder contains one or multiple results, so we need to hide it completely + */ + @Test + @Issue("SECURITY-904") + public void symlink_avoidLeakingInformation_aboutIllegalFolder() throws Exception { + FreeStyleProject p = j.createFreeStyleProject(); + + File secretsFolder = new File(j.jenkins.getRootDir(), "secrets"); + File secretTarget = new File(secretsFolder, "goal.txt"); + String secretContent = "secret"; + FileUtils.write(secretTarget, secretContent); + FileUtils.write(new File(secretsFolder, "public_fake1.key"), secretContent); + FileUtils.write(new File(secretsFolder, "public_fake2.key"), secretContent); + FileUtils.write(new File(secretsFolder, "public_fake3.key"), secretContent); + + /* + * secrets/ + * goal.txt + * public_fake1.key + * public_fake2.key + * public_fake3.key + * workspace/ + * intermediateFolder/ + * public2.key + * otherFolder/ + * to_secret3 -> ../../../../secrets/ + * to_secret2 -> ../../../secrets/ + * to_secret_goal2 -> ../../../secrets/goal.txt + * public1.key + * to_secret1 -> ../../secrets/ + * to_secret_goal1 -> ../../secrets/goal.txt + * + */ + if (Functions.isWindows()) { + // no need to test mklink /H since we cannot create an hard link to a non-existing file + // and so you need to have access to the master file system directly which is already a problem + + String script = loadContentFromResource("outsideWorkspaceStructure.bat"); + p.getBuildersList().add(new BatchFile(script)); + } else { + String script = loadContentFromResource("outsideWorkspaceStructure.sh"); + p.getBuildersList().add(new Shell(script)); + } + + assertEquals(Result.SUCCESS, p.scheduleBuild2(0).get().getResult()); + + JenkinsRule.WebClient wc = j.createWebClient(); + wc.getOptions().setThrowExceptionOnFailingStatusCode(false); + + // the pattern allow us to search inside the files / folders, + // but it should not provide / leak information about non readable folders + + { // without the patch the otherFolder and to_secrets[1,2,3] will appear in the results (once) + Page page = wc.goTo(p.getUrl() + "ws/**/goal.txt", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, allOf( + // really not satisfying the query + not(containsString("public1.key")), + not(containsString("public2.key")), + // those following presences would have leak information that there is some file satisfying that pattern inside + not(containsString("to_secrets")), + not(containsString("to_secrets2")), + not(containsString("to_secrets3")) + )); + } + { // without the patch the otherFolder and to_secrets[1,2,3] will appear in the results (3 times each) + Page page = wc.goTo(p.getUrl() + "ws/**/public*.key", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, allOf( + containsString("public1.key"), + containsString("public2.key"), + // those following presences would have leak information that there is some file satisfying that pattern inside + not(containsString("otherFolder")), + not(containsString("to_secrets")), + not(containsString("to_secrets2")), + not(containsString("to_secrets3")) + )); + } + } + + // The hard links (mklink /H) to file are impossible to be detected and will allow a user to retrieve any file in the system + // to achieve that they should already have access to the system or the Script Console. + @Test + @Issue("SECURITY-904") + public void junctionAndSymlink_outsideWorkspace_areNotAllowed_windowsJunction() throws Exception { + Assume.assumeTrue(Functions.isWindows()); + + FreeStyleProject p = j.createFreeStyleProject(); + + File secretsFolder = new File(j.jenkins.getRootDir(), "secrets"); + File secretTarget = new File(secretsFolder, "goal.txt"); + String secretContent = "secret"; + FileUtils.write(secretTarget, secretContent); + + /* + * secrets/ + * goal.txt + * workspace/ + * intermediateFolder/ + * public2.key + * otherFolder/ + * to_secret3s -> symlink ../../../../secrets/ + * to_secret3j -> junction ../../../../secrets/ + * to_secret2s -> symlink ../../../secrets/ + * to_secret2j -> junction ../../../secrets/ + * to_secret_goal2 -> symlink ../../../secrets/goal.txt + * public1.key + * to_secret1s -> symlink ../../secrets/ + * to_secret1j -> junction ../../secrets/ + * to_secret_goal1 -> symlink ../../secrets/goal.txt + * + */ + String script = loadContentFromResource("outsideWorkspaceStructureWithJunctions.bat"); + p.getBuildersList().add(new BatchFile(script)); + + assertEquals(Result.SUCCESS, p.scheduleBuild2(0).get().getResult()); + + JenkinsRule.WebClient wc = j.createWebClient(); + wc.getOptions().setThrowExceptionOnFailingStatusCode(false); + { // workspace root must be reachable (regular case) + Page page = wc.goTo(p.getUrl() + "ws/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, allOf( + containsString("public1.key"), + containsString("intermediateFolder"), + containsString("to_secrets1j"), + containsString("to_secrets1s"), + containsString("to_secrets_goal1"), + not(containsString("to_secrets2")), + not(containsString("to_secrets_goal2")) + )); + } + { // to_secrets1s not reachable + Page page = wc.goTo(p.getUrl() + "ws/to_secrets1s/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + { // to_secrets1j not reachable + Page page = wc.goTo(p.getUrl() + "ws/to_secrets1j/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + { // to_secrets_goal1 not reachable + Page page = wc.goTo(p.getUrl() + "ws/to_secrets_goal1/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + { // intermediateFolder must be reachable (regular case) + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, allOf( + not(containsString("to_secrets1")), + not(containsString("to_secrets_goal1")), + containsString("to_secrets2s"), + containsString("to_secrets2j"), + containsString("to_secrets_goal2") + )); + } + { // to_secrets2s not reachable + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2s/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + { // to_secrets2j not reachable + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2j/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + { // using symbolic in the intermediate path + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2s/master.key", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + { // using symbolic in the intermediate path + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2j/master.key", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + { // to_secrets_goal2 not reachable + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets_goal2/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + + // pattern search feature + { // the pattern allow us to search inside the files / folders, + // without the patch the master.key from inside the outside symlinks would have been linked + Page page = wc.goTo(p.getUrl() + "ws/**/*.key", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, allOf( + not(containsString("master.key")), + containsString("public1.key"), + containsString("public2.key"), + containsString("intermediateFolder"), + not(containsString("otherFolder")), + not(containsString("to_secrets3j")), + not(containsString("to_secrets3s")), + not(containsString("to_secrets2j")), + not(containsString("to_secrets2s")), + not(containsString("to_secrets1j")), + not(containsString("to_secrets1s")) + )); + } + + // zip feature + { // all the outside folders / files are not included in the zip + Page zipPage = wc.goTo(p.getUrl() + "ws/*zip*/ws.zip", null); + assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + + List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); + assertThat(entryNames, containsInAnyOrder( + p.getName() + "/intermediateFolder/public2.key", + p.getName() + "/public1.key" + )); + } + { // all the outside folders / files are not included in the zip + Page zipPage = wc.goTo(p.getUrl() + "ws/intermediateFolder/*zip*/intermediateFolder.zip", null); + assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + + List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); + assertThat(entryNames, contains("intermediateFolder/public2.key")); + } + } + + private List getListOfEntriesInDownloadedZip(UnexpectedPage zipPage) throws Exception { + List result; + + File zipfile = null; + ZipFile readzip = null; + try { + zipfile = download(zipPage); + + readzip = new ZipFile(zipfile); + result = readzip.stream().map(ZipEntry::getName).collect(Collectors.toList()); + } + finally { + if (readzip != null) { + readzip.close(); + } + if (zipfile != null) { + zipfile.delete(); + } + } + return result; + } + + //TODO taken from existing test without modification to ease merge back + private File download(UnexpectedPage page) throws IOException { + File file = File.createTempFile("DirectoryBrowserSupport", "zipDownload"); + file.delete(); + try (InputStream is = page.getInputStream(); + OutputStream os = Files.newOutputStream(file.toPath())) { + IOUtils.copy(is, os); + } + + return file; + } + + @Test + @Issue("SECURITY-904") + public void directSymlink_forTestingZip() throws Exception { + FreeStyleProject p = j.createFreeStyleProject(); + + assertEquals(Result.SUCCESS, p.scheduleBuild2(0).get().getResult()); + FilePath ws = p.getSomeWorkspace(); + + /* + * secrets/ + * goal.txt + * workspace/ + * /a1/to_secrets1 + * /b1/b2/to_secrets1 + * /c1/c2/c3/to_secrets1 + */ + File secretsFolder = new File(j.jenkins.getRootDir(), "secrets"); + FilePath a1 = ws.child("a1"); + a1.mkdirs(); + a1.child("to_secrets1").symlinkTo(secretsFolder.getAbsolutePath(), TaskListener.NULL); + FilePath b2 = ws.child("b1").child("b2"); + b2.mkdirs(); + b2.child("to_secrets2").symlinkTo(secretsFolder.getAbsolutePath(), TaskListener.NULL); + FilePath c3 = ws.child("c1").child("c2").child("c3"); + c3.mkdirs(); + c3.child("to_secrets3").symlinkTo(secretsFolder.getAbsolutePath(), TaskListener.NULL); + + JenkinsRule.WebClient wc = j.createWebClient(); + wc.getOptions().setThrowExceptionOnFailingStatusCode(false); + { + Page zipPage = wc.goTo(p.getUrl() + "ws/*zip*/ws.zip", null); + assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + + List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); + assertThat(entryNames, hasSize(0)); + } + { + Page zipPage = wc.goTo(p.getUrl() + "ws/a1/*zip*/a1.zip", null); + assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + + List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); + assertThat(entryNames, hasSize(0)); + } + { + Page zipPage = wc.goTo(p.getUrl() + "ws/b1/b2/*zip*/b2.zip", null); + assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + + List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); + assertThat(entryNames, hasSize(0)); + } + { + Page zipPage = wc.goTo(p.getUrl() + "ws/c1/c2/c3/*zip*/c3.zip", null); + assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + + List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); + assertThat(entryNames, hasSize(0)); + } + } + + @Test + @Issue("SECURITY-904") + public void symlink_insideWorkspace_areStillAllowed() throws Exception { + FreeStyleProject p = j.createFreeStyleProject(); + + // build once to have the workspace set up + assertEquals(Result.SUCCESS, p.scheduleBuild2(0).get().getResult()); + + File jobWorkspaceFolder = new File(new File(j.jenkins.getRootDir(), "workspace"), p.name); + File folderInsideWorkspace = new File(jobWorkspaceFolder, "asset"); + folderInsideWorkspace.mkdir(); + File fileTarget = new File(folderInsideWorkspace, "goal.txt"); + String publicContent = "not-secret"; + FileUtils.write(fileTarget, publicContent); + + /* + * workspace/ + * asset/ + * goal.txt + * intermediateFolder/ + * to_internal2 -> ../asset + * to_internal_goal2 -> ../asset/goal.txt + * to_internal1 -> ./asset/ + * to_internal_goal1 -> ./asset/goal.txt + */ + if (Functions.isWindows()) { + String script = loadContentFromResource("insideWorkspaceStructure.bat"); + p.getBuildersList().add(new BatchFile(script)); + } else { + String script = loadContentFromResource("insideWorkspaceStructure.sh"); + p.getBuildersList().add(new Shell(script)); + } + + assertEquals(Result.SUCCESS, p.scheduleBuild2(0).get().getResult()); + + JenkinsRule.WebClient wc = j.createWebClient(); + wc.getOptions().setThrowExceptionOnFailingStatusCode(false); + { // workspace root must be reachable (regular case) + Page page = wc.goTo(p.getUrl() + "ws/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, allOf( + containsString("asset"), + containsString("to_internal1"), + containsString("to_internal_goal1"), + containsString("intermediateFolder"), + not(containsString("to_internal2")), + not(containsString("to_internal_goal2") + ))); + } + { // to_internal1 reachable + Page page = wc.goTo(p.getUrl() + "ws/to_internal1/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, containsString("goal.txt")); + } + { // to_internal_goal1 reachable + Page page = wc.goTo(p.getUrl() + "ws/to_internal_goal1/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, containsString(publicContent)); + } + { // to_internal2 reachable + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_internal2/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, containsString("goal.txt")); + } + { // to_internal_goal2 reachable + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_internal_goal2/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, containsString(publicContent)); + } + { // direct to goal + Page page = wc.goTo(p.getUrl() + "ws/asset/goal.txt/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, containsString(publicContent)); + } + { // the zip will only contain folder from inside the workspace + Page page = wc.goTo(p.getUrl() + "ws/*zip*/ws.zip", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + } + { // the zip will only contain folder from inside the workspace + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/*zip*/intermediateFolder.zip", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + } + { // the zip will only contain folder from inside the workspace + Page page = wc.goTo(p.getUrl() + "ws/asset/*zip*/asset.zip", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + } + } + + private String loadContentFromResource(String fileNameInResources) throws IOException, URISyntaxException { + URL resourceUrl = DirectoryBrowserSupportSEC904Test.class.getResource(DirectoryBrowserSupportSEC904Test.class.getSimpleName() + "/" + fileNameInResources); + if (resourceUrl == null) { + fail("The resource with fileName " + fileNameInResources + " is not present in the resources of the test"); + } + File resourceFile = new File(resourceUrl.toURI()); + return FileUtils.readFileToString(resourceFile); + } +} diff --git a/test/src/test/java/hudson/model/UserIdMigratorTest.java b/test/src/test/java/hudson/model/UserIdMigratorTest.java new file mode 100644 index 0000000000..733a6a8b09 --- /dev/null +++ b/test/src/test/java/hudson/model/UserIdMigratorTest.java @@ -0,0 +1,61 @@ +/* + * The MIT License + * + * Copyright (c) 2018 CloudBees, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +package hudson.model; + +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.reactor.ReactorException; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.recipes.LocalData; + +import java.io.IOException; + +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; + +public class UserIdMigratorTest { + + @Rule public JenkinsRule j = new JenkinsRule(); + + @Test + @LocalData + public void migrateSimpleUser() throws InterruptedException, ReactorException, IOException { + String userId = "fred"; + User fred = User.getById(userId, false); + assertThat(fred.getFullName(), is("Fred Smith")); + } + + @Test + @LocalData + public void migrateMultipleUsers() throws InterruptedException, ReactorException, IOException { + assertThat(User.getAll().size(), is(3)); + User fred = User.getById("fred", false); + assertThat(fred.getFullName(), is("Fred Smith")); + User legacyUser = User.getById("foo/bar", false); + assertThat(legacyUser.getFullName(), is("Foo Bar")); + User oldLegacyUser = User.getById("zzz\u1000", false); + assertThat(oldLegacyUser.getFullName(), is("Old Legacy")); + } + +} diff --git a/test/src/test/java/hudson/model/UserPropertyTest.java b/test/src/test/java/hudson/model/UserPropertyTest.java index d1b398fa2a..4b29da0241 100644 --- a/test/src/test/java/hudson/model/UserPropertyTest.java +++ b/test/src/test/java/hudson/model/UserPropertyTest.java @@ -19,8 +19,6 @@ import java.io.IOException; import java.util.Collections; import java.util.List; -import static hudson.model.User.idStrategy; -import static hudson.model.UserPropertyTest.InnerUserClass.TEST_FILE; import static java.lang.System.currentTimeMillis; import static java.util.Collections.emptyMap; import static org.apache.commons.io.FileUtils.writeStringToFile; @@ -80,7 +78,8 @@ public class UserPropertyTest { User user = User.get("nestedUserReference", false, emptyMap()); assertThat("nested reference should be updated after jenkins start", user, nestedUserSet()); - File testFile = new File(j.getInstance().getRootDir() + "/users/nesteduserreference/" + TEST_FILE); + SetUserUserProperty property = user.getProperty(SetUserUserProperty.class); + File testFile = property.getInnerUserClass().userFile; List fileLines = FileUtils.readLines(testFile); assertThat(fileLines, hasSize(1)); @@ -89,7 +88,6 @@ public class UserPropertyTest { user = User.get("nestedUserReference", false, Collections.emptyMap()); assertThat("nested reference should exist after user configuration change", user, nestedUserSet()); - testFile = new File(j.getInstance().getRootDir() + "/users/nesteduserreference/" + TEST_FILE); fileLines = FileUtils.readLines(testFile); assertThat(fileLines, hasSize(1)); } @@ -167,9 +165,10 @@ public class UserPropertyTest { * Class that should get setUser(User) object reference update. */ public static class InnerUserClass extends AbstractDescribableImpl { - public static final String TEST_FILE = "test.txt"; private transient User user; + private transient File userFile; + @DataBoundConstructor public InnerUserClass() { } @@ -184,16 +183,16 @@ public class UserPropertyTest { public void setUser(User user) { this.user = user; try { - writeStringToFile(getUserFile(), String.valueOf(currentTimeMillis()), true); + File userFile = getUserFile(); + writeStringToFile(userFile, String.valueOf(currentTimeMillis()), true); } catch (IOException e) { Throwables.propagate(e); } } private File getUserFile() throws IOException { - final File usersRootDir = new File(Jenkins.getInstance().getRootDir(), "users"); - final File userDir = new File(usersRootDir, idStrategy().filenameOf(user.getId())); - final File userFile = new File(userDir, TEST_FILE); + userFile = File.createTempFile("user", ".txt"); + userFile.deleteOnExit(); if (!userFile.exists()) { userFile.createNewFile(); } diff --git a/test/src/test/java/hudson/model/UserTest.java b/test/src/test/java/hudson/model/UserTest.java index 34bb26a1c9..3e2e21b5bc 100644 --- a/test/src/test/java/hudson/model/UserTest.java +++ b/test/src/test/java/hudson/model/UserTest.java @@ -1,19 +1,19 @@ /* * The MIT License - * + * * Copyright (c) 2004-2012, Sun Microsystems, Inc., Kohsuke Kawaguchi, Erik Ramfelt, * Vincent Latombe - * + * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal * in the Software without restriction, including without limitation the rights * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell * copies of the Software, and to permit persons to whom the Software is * furnished to do so, subject to the following conditions: - * + * * The above copyright notice and this permission notice shall be included in * all copies or substantial portions of the Software. - * + * * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE @@ -44,10 +44,8 @@ import hudson.security.UserMayOrMayNotExistException; import hudson.tasks.MailAddressResolver; import java.io.File; import java.io.IOException; -import java.io.PrintStream; import java.net.URL; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Locale; @@ -67,7 +65,6 @@ import org.acegisecurity.userdetails.UsernameNotFoundException; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; -import static org.junit.Assume.*; import org.junit.Rule; import org.junit.Test; @@ -86,11 +83,11 @@ public class UserTest { private final String testString; private UserPropertyDescriptor descriptorImpl = new UserPropertyDescriptorImpl(); - + public UserPropertyImpl(String testString) { this.testString = testString; } - + public String getTestString() { return testString; } @@ -99,7 +96,7 @@ public class UserTest { public UserPropertyDescriptor getDescriptor() { return descriptorImpl; } - + public String getIconFileName() { return "/images/24x24/gear.png"; } @@ -111,7 +108,7 @@ public class UserTest { public String getUrlName() { return "userpropertyimpl"; } - + public static class UserPropertyDescriptorImpl extends UserPropertyDescriptor { @Override public UserProperty newInstance(User user) { @@ -122,20 +119,20 @@ public class UserTest { @Issue("JENKINS-2331") @Test public void userPropertySummaryAndActionAreShownInUserPage() throws Exception { - + UserProperty property = new UserPropertyImpl("NeedleInPage"); UserProperty.all().add(property.getDescriptor()); - + User user = User.get("user-test-case"); user.addProperty(property); - + HtmlPage page = j.createWebClient().goTo("user/user-test-case"); - + WebAssert.assertTextPresentInElement(page, "NeedleInPage", "main-panel"); WebAssert.assertTextPresentInElement(page, ((Action) property).getDisplayName(), "side-panel"); - + } - + /** * Asserts that the default user avatar can be fetched (ie no 404) */ @@ -171,7 +168,7 @@ public class UserTest { seccon.setAuthentication(orig); } } - + @Test public void testGetUser() throws Exception { { @@ -196,10 +193,8 @@ public class UserTest { User user = User.get("john smith"); User user2 = User.get("John Smith"); assertSame("Users should have the same id.", user.getId(), user2.getId()); - assertEquals(user.getId(), User.idStrategy().idFromFilename(User.idStrategy().filenameOf(user.getId()))); - assertEquals(user2.getId(), User.idStrategy().idFromFilename(User.idStrategy().filenameOf(user2.getId()))); } - + @Test public void caseSensitivity() { j.jenkins.setSecurityRealm(new IdStrategySpecifyingSecurityRealm(new IdStrategy.CaseSensitive())); @@ -207,11 +202,7 @@ public class UserTest { User user2 = User.get("John Smith"); assertNotSame("Users should not have the same id.", user.getId(), user2.getId()); assertEquals("john smith", User.idStrategy().keyFor(user.getId())); - assertEquals("john smith", User.idStrategy().filenameOf(user.getId())); assertEquals("John Smith", User.idStrategy().keyFor(user2.getId())); - assertEquals("~john ~smith", User.idStrategy().filenameOf(user2.getId())); - assertEquals(user.getId(), User.idStrategy().idFromFilename(User.idStrategy().filenameOf(user.getId()))); - assertEquals(user2.getId(), User.idStrategy().idFromFilename(User.idStrategy().filenameOf(user2.getId()))); } @Test @@ -221,15 +212,10 @@ public class UserTest { User user2 = User.get("John.Smith@acme.org"); assertNotSame("Users should not have the same id.", user.getId(), user2.getId()); assertEquals("john.smith@acme.org", User.idStrategy().keyFor(user.getId())); - assertEquals("john.smith@acme.org", User.idStrategy().filenameOf(user.getId())); assertEquals("John.Smith@acme.org", User.idStrategy().keyFor(user2.getId())); - assertEquals("~john.~smith@acme.org", User.idStrategy().filenameOf(user2.getId())); user2 = User.get("john.smith@ACME.ORG"); assertEquals("Users should have the same id.", user.getId(), user2.getId()); assertEquals("john.smith@acme.org", User.idStrategy().keyFor(user2.getId())); - assertEquals("john.smith@acme.org", User.idStrategy().filenameOf(user2.getId())); - assertEquals(user.getId(), User.idStrategy().idFromFilename(User.idStrategy().filenameOf(user.getId()))); - assertEquals(user2.getId(), User.idStrategy().idFromFilename(User.idStrategy().filenameOf(user2.getId()))); } private static class IdStrategySpecifyingSecurityRealm extends HudsonPrivateSecurityRealm { @@ -244,21 +230,10 @@ public class UserTest { } } - @Issue("JENKINS-24317") - @LocalData - @Test public void migration() throws Exception { - assumeFalse("was not a problem on a case-insensitive FS to begin with", new File(j.jenkins.getRootDir(), "users/bob").isDirectory()); - User bob = User.get("bob"); - assertEquals("Bob Smith", bob.getFullName()); - assertEquals("Bob Smith", User.get("Bob").getFullName()); - assertEquals("nonexistent", User.get("nonexistent").getFullName()); - assertEquals("[bob]", Arrays.toString(new File(j.jenkins.getRootDir(), "users").list())); - } - @Test public void testAddAndGetProperty() throws Exception { { - User user = User.get("John Smith"); + User user = User.get("John Smith"); UserProperty prop = new SomeUserProperty(); user.addProperty(prop); assertNotNull("User should have SomeUserProperty property.", user.getProperty(SomeUserProperty.class)); @@ -274,15 +249,15 @@ public class UserTest { @Test public void testImpersonateAndCurrent() { j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); - User user = User.get("John Smith"); + User user = User.get("John Smith"); assertNotSame("User John Smith should not be the current user.", User.current().getId(), user.getId()); - SecurityContextHolder.getContext().setAuthentication(user.impersonate()); + SecurityContextHolder.getContext().setAuthentication(user.impersonate()); assertEquals("User John Smith should be the current user.", user.getId(), User.current().getId()); } @Test public void testGetUnknown() { - User user = User.get("John Smith"); + User user = User.get("John Smith"); assertNotNull("User should not be null.", user); } @@ -298,22 +273,19 @@ public class UserTest { assertNotNull("User John Smith should be created.", user); assertTrue("Jenkins should contain user John Smith.", User.getAll().contains(user)); } - + @Test public void testReload() throws Exception { - { - User user = User.get("John Smith", true, Collections.emptyMap()); + String originalName = "John Smith"; + User user = User.get(originalName, true, Collections.emptyMap()); user.save(); - String config = user.getConfigFile().asString(); - config = config.replace("John Smith", "Alice Smith"); - PrintStream st = new PrintStream(user.getConfigFile().getFile()); - st.print(config); - } + String temporaryName = "Alice Smith"; + user.setFullName(temporaryName); + j.jenkins.reload(); - { - User user = User.get("John Smith", false, Collections.emptyMap()); - assertEquals("User should have full name Alice Smith.", "Alice Smith", user.getFullName()); - } + + user = User.get(originalName, false, Collections.emptyMap()); + assertEquals("User should have original name.", originalName, user.getFullName()); } @Test @@ -383,8 +355,9 @@ public class UserTest { { User user = User.get("John Smith", true, Collections.emptyMap()); user.save(); + File configFolder = user.getUserFolder(); user.delete(); - assertFalse("User should be deleted with his persistent data.", user.getConfigFile().exists()); + assertFalse("User should be deleted with his persistent data.", configFolder.exists()); assertFalse("User should be deleted from memory.", User.getAll().contains(user)); user = User.get("John Smith", false, Collections.emptyMap()); assertNull("User should be deleted from memory.", user); @@ -404,7 +377,7 @@ public class UserTest { @Test public void testDoConfigSubmit() throws Exception { - GlobalMatrixAuthorizationStrategy auth = new GlobalMatrixAuthorizationStrategy(); + GlobalMatrixAuthorizationStrategy auth = new GlobalMatrixAuthorizationStrategy(); j.jenkins.setAuthorizationStrategy(auth); j.jenkins.setCrumbIssuer(null); HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(false); @@ -430,7 +403,7 @@ public class UserTest { } } form = j.createWebClient().withBasicCredentials(user2.getId(), "password").goTo(user2.getUrl() + "/configure").getFormByName("config"); - + form.getInputByName("_.fullName").setValueAttribute("John"); j.submit(form); assertEquals("User should be albe to configure himself.", "John", user2.getFullName()); @@ -440,7 +413,7 @@ public class UserTest { /* TODO cannot follow what this is purporting to test @Test public void testDoDoDelete() throws Exception { - GlobalMatrixAuthorizationStrategy auth = new GlobalMatrixAuthorizationStrategy(); + GlobalMatrixAuthorizationStrategy auth = new GlobalMatrixAuthorizationStrategy(); j.jenkins.setAuthorizationStrategy(auth); j.jenkins.setCrumbIssuer(null); HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(false); @@ -486,7 +459,7 @@ public class UserTest { @Test public void testHasPermission() throws IOException { - GlobalMatrixAuthorizationStrategy auth = new GlobalMatrixAuthorizationStrategy(); + GlobalMatrixAuthorizationStrategy auth = new GlobalMatrixAuthorizationStrategy(); j.jenkins.setAuthorizationStrategy(auth); j.jenkins.setCrumbIssuer(null); HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(false); @@ -505,7 +478,7 @@ public class UserTest { @Test public void testCanDelete() throws IOException { - GlobalMatrixAuthorizationStrategy auth = new GlobalMatrixAuthorizationStrategy(); + GlobalMatrixAuthorizationStrategy auth = new GlobalMatrixAuthorizationStrategy(); j.jenkins.setAuthorizationStrategy(auth); j.jenkins.setCrumbIssuer(null); HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(false); @@ -628,6 +601,7 @@ public class UserTest { assertEquals("victim2 is a real (canonical) user ID", "victim2", User.get("Victim2").getId()); } + private static class ExternalSecurityRealm extends AbstractPasswordBasedSecurityRealm { @Override public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException { @@ -729,115 +703,38 @@ public class UserTest { } @Test - @Issue("SECURITY-499") - public void createdUsersHaveCorrectConfigLocation() { - assertCorrectConfig(User.getById("admin", true), "users/admin/config.xml"); - assertCorrectConfig(User.getById("foo", true), "users/foo/config.xml"); - assertCorrectConfig(User.getById("foo/bar", true), "users/foo$002fbar/config.xml"); - assertCorrectConfig(User.getById("foo/bar/baz", true), "users/foo$002fbar$002fbaz/config.xml"); - assertCorrectConfig(User.getById("/", true), "users/$002f/config.xml"); - assertCorrectConfig(User.getById(".", true), "users/$002f/config.xml"); - assertCorrectConfig(User.getById("..", true), "users/$002e$002e/config.xml"); - assertCorrectConfig(User.getById("../config.xml", true), "users/..$002fconfig.xml/config.xml"); + @LocalData + public void differentUserIdInConfigFileIsIgnored() { + String fredUserId = "fred"; + User fred = User.getById(fredUserId, false); + assertThat(fred, notNullValue()); + assertThat(fred.getId(), is(fredUserId)); + assertThat(fred.getFullName(), is("Fred Smith")); + User jane = User.getById("jane", false); + assertThat(jane, nullValue()); } @Test - @Issue("SECURITY-499") @LocalData - public void legacyUserConfigDirsMigrated() { - File rootDir = new File(Jenkins.getInstance().getRootDir(), "users"); - - User admin = User.getById("admin", false); - assertCorrectConfig(admin, "users/admin/config.xml"); - assertTrue(admin.getConfigFile().getFile().exists()); - assertThat(admin.getFullName(), equalTo("Admin")); - - User foo = User.getById("foo", false); - File fooDir = new File(rootDir, "foo"); - assertCorrectConfig(foo, "users/foo/config.xml"); - assertTrue(foo.getConfigFile().getFile().exists()); - assertTrue(fooDir.exists()); - assertThat(foo.getFullName(), equalTo("Foo")); - - User fooBar = User.getById("foo/bar", false); - File fooBarDir = new File(fooDir, "bar"); - assertCorrectConfig(fooBar, "users/foo$002fbar/config.xml"); - assertTrue(fooBar.getConfigFile().getFile().exists()); - assertTrue(fooDir.exists()); - assertTrue(fooBarDir.exists()); - assertThat(fooBar.getFullName(), equalTo("Foo Bar")); - - User fooBaz = User.getById("foo/baz", false); - File fooBazDir = new File(fooDir, "baz"); - assertCorrectConfig(fooBaz, "users/foo$002fbaz/config.xml"); - assertTrue(fooBaz.getConfigFile().getFile().exists()); - assertTrue(fooDir.exists()); - assertFalse(fooBazDir.exists()); - assertThat(fooBaz.getFullName(), equalTo("Foo Baz")); - - User fooBarBaz = User.getById("foo/bar/baz", false); - File fooBarBazDir = new File(fooBarDir, "baz"); - assertCorrectConfig(fooBarBaz, "users/foo$002fbar$002fbaz/config.xml"); - assertTrue(fooBarBaz.getConfigFile().getFile().exists()); - assertTrue(fooDir.exists()); - assertFalse(fooBarBazDir.exists()); - assertFalse(fooBarDir.exists()); - assertThat(fooBarBaz.getFullName(), equalTo("Foo Bar Baz")); - - User slash = User.getById("/", false); - File slashDir = new File(rootDir, "$002f"); - assertCorrectConfig(slash, "users/$002f/config.xml"); - assertTrue(slash.getConfigFile().getFile().exists()); - assertTrue(slashDir.exists()); - assertFalse(new File(rootDir, "config.xml").exists()); - assertThat(slash.getFullName(), equalTo("Slash")); + public void corruptConfigFile() { + String fredUserId = "fred"; + User fred = User.getById(fredUserId, true); + assertThat(fred, notNullValue()); + assertThat(fred.getFullName(), is("fred")); } @Test - @Issue("SECURITY-499") - @LocalData - public void emptyUsernameConfigMigrated() { - File rootDir = new File(Jenkins.getInstance().getRootDir(), "users"); - - User admin = User.getById("admin", false); - assertCorrectConfig(admin, "users/admin/config.xml"); - assertTrue(admin.getConfigFile().getFile().exists()); - assertThat(admin.getFullName(), equalTo("Admin")); - - User empty = User.getById("", false); - File emptyDir = new File(rootDir, "$002f"); - assertCorrectConfig(empty, "users/$002f/config.xml"); - assertTrue(empty.getConfigFile().getFile().exists()); - assertTrue(emptyDir.exists()); - assertFalse(new File(rootDir, "config.xml").exists()); - assertThat(empty.getFullName(), equalTo("Empty")); + public void parentDirectoryUserDoesNotExist() { + String userId = "admin"; + User admin = User.getById(userId, true); + assertNotNull(admin); + assertThat(admin.getId(), is(userId)); + User parentDirectoryUserId = User.getById("../" + admin, false); + assertThat(parentDirectoryUserId, nullValue()); } - @Issue("JENKINS-47909") - @LocalData - @Test - public void shellyUsernameMigrated() { - File rootDir = new File(Jenkins.getInstance().getRootDir(), "users"); - User user = User.getById("bla$phem.us", false); - assertCorrectConfig(user, "users/bla$0024phem.us/config.xml"); - assertFalse(new File(rootDir, "bla$phem.us").exists()); - assertTrue(user.getConfigFile().getFile().exists()); - assertThat(user.getFullName(), equalTo("Weird Username")); - user = User.getById("make\u1000000", false); - assertNotNull("we do not prevent accesses to the phony name, alas", user); - user = User.getById("make$1000000", false); - assertCorrectConfig(user, "users/make$00241000000/config.xml"); - assertFalse(new File(rootDir, "make$1000000").exists()); - assertTrue("but asking for the real name triggers migration", user.getConfigFile().getFile().exists()); - assertThat(user.getFullName(), equalTo("Greedy Fella")); - } + public static class SomeUserProperty extends UserProperty { - private static void assertCorrectConfig(User user, String unixPath) { - assertThat(user.getConfigFile().getFile().getPath(), endsWith(unixPath.replace('/', File.separatorChar))); - } - - public static class SomeUserProperty extends UserProperty { - @TestExtension public static class DescriptorImpl extends UserPropertyDescriptor { @Override diff --git a/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java b/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java index 5f018f29a3..ce027fc2b2 100644 --- a/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java +++ b/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java @@ -54,10 +54,8 @@ import static org.junit.Assert.*; import jenkins.security.SecurityListener; import org.apache.commons.lang.StringUtils; -import java.io.UnsupportedEncodingException; import java.lang.reflect.Field; -import hudson.security.pages.SignupPage; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -66,7 +64,6 @@ import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.JenkinsRule.WebClient; import org.jvnet.hudson.test.TestExtension; import org.jvnet.hudson.test.WithoutJenkins; -import org.jvnet.hudson.test.recipes.LocalData; import javax.annotation.Nonnull; @@ -89,34 +86,6 @@ public class HudsonPrivateSecurityRealmTest { field.set(null, null); } - /** - * Tests the data compatibility with Hudson before 1.283. - * Starting 1.283, passwords are now stored hashed. - */ - @Test - @Issue("JENKINS-2381") - @LocalData - public void dataCompatibilityWith1_282() throws Exception { - // make sure we can login with the same password as before - WebClient wc = j.createWebClient().login("alice", "alice"); - - try { - // verify the sanity that the password is really used - // this should fail - j.createWebClient().login("bob", "bob"); - fail(); - } catch (FailingHttpStatusCodeException e) { - assertEquals(401,e.getStatusCode()); - } - - // resubmit the config and this should force the data store to be rewritten - HtmlPage p = wc.goTo("user/alice/configure"); - j.submit(p.getFormByName("config")); - - // verify that we can still login - j.createWebClient().login("alice", "alice"); - } - @Test @WithoutJenkins public void hashCompatibility() { diff --git a/test/src/test/java/hudson/tasks/ArtifactArchiverTest.java b/test/src/test/java/hudson/tasks/ArtifactArchiverTest.java index d64e7a1570..7ceccf5a23 100644 --- a/test/src/test/java/hudson/tasks/ArtifactArchiverTest.java +++ b/test/src/test/java/hudson/tasks/ArtifactArchiverTest.java @@ -163,7 +163,7 @@ public class ArtifactArchiverTest { assertFalse(kids[0].isDirectory()); assertFalse(kids[0].isFile()); assertFalse(kids[0].exists()); - j.createWebClient().assertFails(b.getUrl() + "artifact/hack", HttpURLConnection.HTTP_NOT_FOUND); + j.createWebClient().assertFails(b.getUrl() + "artifact/hack", HttpURLConnection.HTTP_FORBIDDEN); } static class CreateArtifact extends TestBuilder { diff --git a/test/src/test/java/jenkins/model/JenkinsReloadConfigurationTest.java b/test/src/test/java/jenkins/model/JenkinsReloadConfigurationTest.java index 89016dae70..ba120e1b46 100644 --- a/test/src/test/java/jenkins/model/JenkinsReloadConfigurationTest.java +++ b/test/src/test/java/jenkins/model/JenkinsReloadConfigurationTest.java @@ -56,18 +56,20 @@ public class JenkinsReloadConfigurationTest { @Test public void reloadUserConfigUsingGlobalReload() throws Exception { + String originalName = "oldName"; + String temporaryName = "newName"; { User user = User.get("some_user", true, null); - user.setFullName("oldName"); + user.setFullName(originalName); user.save(); + assertEquals(originalName, user.getFullName()); - replace("users/some_user/config.xml", "oldName", "newName"); - - assertEquals("oldName", user.getFullName()); + user.setFullName(temporaryName); + assertEquals(temporaryName, user.getFullName()); } j.jenkins.reload(); { - assertEquals("newName", User.getById("some_user", false).getFullName()); + assertEquals(originalName, User.getById("some_user", false).getFullName()); } } diff --git a/test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/insideWorkspaceStructure.bat b/test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/insideWorkspaceStructure.bat new file mode 100644 index 0000000000..7e4b744d4a --- /dev/null +++ b/test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/insideWorkspaceStructure.bat @@ -0,0 +1,7 @@ +mklink /D to_internal1 .\asset +mklink to_internal_goal1 .\asset\goal.txt + +mkdir intermediateFolder +cd intermediateFolder +mklink /D to_internal2 ..\asset +mklink to_internal_goal2 ..\asset\goal.txt diff --git a/test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/insideWorkspaceStructure.sh b/test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/insideWorkspaceStructure.sh new file mode 100644 index 0000000000..475b24f3fe --- /dev/null +++ b/test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/insideWorkspaceStructure.sh @@ -0,0 +1,8 @@ +#!/bin/sh +ln -s ./asset to_internal1 +ln -s ./asset/goal.txt to_internal_goal1 + +mkdir intermediateFolder +cd intermediateFolder +ln -s ../asset to_internal2 +ln -s ../asset/goal.txt to_internal_goal2 diff --git a/test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/outsideWorkspaceStructure.bat b/test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/outsideWorkspaceStructure.bat new file mode 100644 index 0000000000..6159b02242 --- /dev/null +++ b/test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/outsideWorkspaceStructure.bat @@ -0,0 +1,13 @@ +echo public-1 > public1.key +mklink /D to_secrets1 ..\..\secrets +mklink to_secrets_goal1 ..\..\secrets\goal.txt + +mkdir intermediateFolder +cd intermediateFolder +echo public-2 > public2.key +mklink /D to_secrets2 ..\..\..\secrets +mklink to_secrets_goal2 ..\..\..\secrets\goal.txt + +mkdir otherFolder +cd otherFolder +mklink /D to_secrets3 ..\..\..\..\secrets diff --git a/test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/outsideWorkspaceStructure.sh b/test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/outsideWorkspaceStructure.sh new file mode 100644 index 0000000000..d154f42c67 --- /dev/null +++ b/test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/outsideWorkspaceStructure.sh @@ -0,0 +1,14 @@ +#!/bin/sh +echo 'public-1' > public1.key +ln -s ../../secrets to_secrets1 +ln -s ../../secrets/goal.txt to_secrets_goal1 + +mkdir intermediateFolder +cd intermediateFolder +echo 'public-2' > public2.key +ln -s ../../../secrets to_secrets2 +ln -s ../../../secrets/goal.txt to_secrets_goal2 + +mkdir otherFolder +cd otherFolder +ln -s ../../../../secrets to_secrets3 diff --git a/test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/outsideWorkspaceStructureWithJunctions.bat b/test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/outsideWorkspaceStructureWithJunctions.bat new file mode 100644 index 0000000000..58ed542e74 --- /dev/null +++ b/test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/outsideWorkspaceStructureWithJunctions.bat @@ -0,0 +1,16 @@ +echo public-1 > public1.key +mklink /D to_secrets1s ..\..\secrets +mklink /J to_secrets1j ..\..\secrets +mklink to_secrets_goal1 ..\..\secrets\goal.txt + +mkdir intermediateFolder +cd intermediateFolder +echo public-2 > public2.key +mklink /D to_secrets2s ..\..\..\secrets +mklink /J to_secrets2j ..\..\..\secrets +mklink to_secrets_goal2 ..\..\..\secrets\goal.txt + +mkdir otherFolder +cd otherFolder +mklink /D to_secrets3s ..\..\..\..\secrets +mklink /J to_secrets3j ..\..\..\..\secrets diff --git a/test/src/test/resources/hudson/model/JobTest/readPermission.zip b/test/src/test/resources/hudson/model/JobTest/readPermission.zip index 0332f404e4..4ffbc34eb4 100644 Binary files a/test/src/test/resources/hudson/model/JobTest/readPermission.zip and b/test/src/test/resources/hudson/model/JobTest/readPermission.zip differ diff --git a/test/src/test/resources/hudson/model/UserIdMigratorTest/migrateMultipleUsers/users/foo$002fbar/config.xml b/test/src/test/resources/hudson/model/UserIdMigratorTest/migrateMultipleUsers/users/foo$002fbar/config.xml new file mode 100644 index 0000000000..fddfc41176 --- /dev/null +++ b/test/src/test/resources/hudson/model/UserIdMigratorTest/migrateMultipleUsers/users/foo$002fbar/config.xml @@ -0,0 +1,4 @@ + + + Foo Bar + diff --git a/test/src/test/resources/hudson/model/UserIdMigratorTest/migrateMultipleUsers/users/fred/config.xml b/test/src/test/resources/hudson/model/UserIdMigratorTest/migrateMultipleUsers/users/fred/config.xml new file mode 100644 index 0000000000..ad31ae2638 --- /dev/null +++ b/test/src/test/resources/hudson/model/UserIdMigratorTest/migrateMultipleUsers/users/fred/config.xml @@ -0,0 +1,5 @@ + + + fred + Fred Smith + diff --git a/test/src/test/resources/hudson/model/UserIdMigratorTest/migrateMultipleUsers/users/zzzက/config.xml b/test/src/test/resources/hudson/model/UserIdMigratorTest/migrateMultipleUsers/users/zzzက/config.xml new file mode 100644 index 0000000000..9216f71ae3 --- /dev/null +++ b/test/src/test/resources/hudson/model/UserIdMigratorTest/migrateMultipleUsers/users/zzzက/config.xml @@ -0,0 +1,4 @@ + + + Old Legacy + diff --git a/test/src/test/resources/hudson/model/UserIdMigratorTest/migrateSimpleUser/users/fred/config.xml b/test/src/test/resources/hudson/model/UserIdMigratorTest/migrateSimpleUser/users/fred/config.xml new file mode 100644 index 0000000000..ad31ae2638 --- /dev/null +++ b/test/src/test/resources/hudson/model/UserIdMigratorTest/migrateSimpleUser/users/fred/config.xml @@ -0,0 +1,5 @@ + + + fred + Fred Smith + diff --git a/test/src/test/resources/hudson/model/UserPropertyTest/nestedUserReference/users/nestedUserReference/config.xml b/test/src/test/resources/hudson/model/UserPropertyTest/nestedUserReference/users/user4975613142751794216/config.xml similarity index 100% rename from test/src/test/resources/hudson/model/UserPropertyTest/nestedUserReference/users/nestedUserReference/config.xml rename to test/src/test/resources/hudson/model/UserPropertyTest/nestedUserReference/users/user4975613142751794216/config.xml diff --git a/test/src/test/resources/hudson/model/UserPropertyTest/nestedUserReference/users/users.xml b/test/src/test/resources/hudson/model/UserPropertyTest/nestedUserReference/users/users.xml new file mode 100644 index 0000000000..a713bd7998 --- /dev/null +++ b/test/src/test/resources/hudson/model/UserPropertyTest/nestedUserReference/users/users.xml @@ -0,0 +1,9 @@ + + + + + nesteduserreference + user4975613142751794216 + + + diff --git a/test/src/test/resources/hudson/model/UserTest/legacyUserConfigDirsMigrated/config.xml b/test/src/test/resources/hudson/model/UserTest/corruptConfigFile/users/user4975613142751794216/config.xml similarity index 92% rename from test/src/test/resources/hudson/model/UserTest/legacyUserConfigDirsMigrated/config.xml rename to test/src/test/resources/hudson/model/UserTest/corruptConfigFile/users/user4975613142751794216/config.xml index e8902ede4e..2daf2c7407 100644 --- a/test/src/test/resources/hudson/model/UserTest/legacyUserConfigDirsMigrated/config.xml +++ b/test/src/test/resources/hudson/model/UserTest/corruptConfigFile/users/user4975613142751794216/config.xml @@ -1,7 +1,8 @@ - + - 1.0 + 2.144-SNAPSHOT + RESTART 2 NORMAL true diff --git a/test/src/test/resources/hudson/model/UserTest/corruptConfigFile/users/users.xml b/test/src/test/resources/hudson/model/UserTest/corruptConfigFile/users/users.xml new file mode 100644 index 0000000000..c8017032d1 --- /dev/null +++ b/test/src/test/resources/hudson/model/UserTest/corruptConfigFile/users/users.xml @@ -0,0 +1,9 @@ + + + + + fred + user4975613142751794216 + + + diff --git a/test/src/test/resources/hudson/model/UserTest/differentUserIdInConfigFileIsIgnored/users/user4975613142751794216/config.xml b/test/src/test/resources/hudson/model/UserTest/differentUserIdInConfigFileIsIgnored/users/user4975613142751794216/config.xml new file mode 100644 index 0000000000..6465bde25c --- /dev/null +++ b/test/src/test/resources/hudson/model/UserTest/differentUserIdInConfigFileIsIgnored/users/user4975613142751794216/config.xml @@ -0,0 +1,5 @@ + + + jane + Fred Smith + diff --git a/test/src/test/resources/hudson/model/UserTest/differentUserIdInConfigFileIsIgnored/users/users.xml b/test/src/test/resources/hudson/model/UserTest/differentUserIdInConfigFileIsIgnored/users/users.xml new file mode 100644 index 0000000000..c8017032d1 --- /dev/null +++ b/test/src/test/resources/hudson/model/UserTest/differentUserIdInConfigFileIsIgnored/users/users.xml @@ -0,0 +1,9 @@ + + + + + fred + user4975613142751794216 + + + diff --git a/test/src/test/resources/hudson/model/UserTest/legacyUserConfigDirsMigrated/users/admin/config.xml b/test/src/test/resources/hudson/model/UserTest/legacyUserConfigDirsMigrated/users/admin/config.xml deleted file mode 100644 index 91fc2f3e1f..0000000000 --- a/test/src/test/resources/hudson/model/UserTest/legacyUserConfigDirsMigrated/users/admin/config.xml +++ /dev/null @@ -1,29 +0,0 @@ - - - Admin - - - {AQAAABAAAAAwu9tm9n3SIC+bkaGz5WfsKwJz+a04aUUWiq4hACNLemh6f62j2cgIT9KuyMGtXvr7ab6GWBQGzThOrJfK1etETQ==} - - - - - - all - false - false - - - - - - - - - false - - - #jbcrypt:$2a$10$eDQD3w/iXX0VWWKb3Kv5QOKjFg0Yj3Hk9Z.Y43iFHJQOUYmHdvVuS - - - diff --git a/test/src/test/resources/hudson/model/UserTest/legacyUserConfigDirsMigrated/users/config.xml b/test/src/test/resources/hudson/model/UserTest/legacyUserConfigDirsMigrated/users/config.xml deleted file mode 100644 index f6d70dcd6e..0000000000 --- a/test/src/test/resources/hudson/model/UserTest/legacyUserConfigDirsMigrated/users/config.xml +++ /dev/null @@ -1,29 +0,0 @@ - - - Slash - - - {AQAAABAAAAAwkYgvFl7YHG1866pnHDawvcy2VQ0jlbQd85vu9WWI22SjmLmJS0TMMS4w+3XqvEt8nHx9/0U06huIcMj3XuDPuQ==} - - - - - - all - false - false - - - - - - - - - false - - - #jbcrypt:$2a$10$dTn2EpIBa2E0T6R.2XK/HucVkyqdu9qGQ6FmBOYdZlpeN2mtv.Ez2 - - - diff --git a/test/src/test/resources/hudson/model/UserTest/legacyUserConfigDirsMigrated/users/foo/bar/baz/config.xml b/test/src/test/resources/hudson/model/UserTest/legacyUserConfigDirsMigrated/users/foo/bar/baz/config.xml deleted file mode 100644 index 25769bc78c..0000000000 --- a/test/src/test/resources/hudson/model/UserTest/legacyUserConfigDirsMigrated/users/foo/bar/baz/config.xml +++ /dev/null @@ -1,29 +0,0 @@ - - - Foo Bar Baz - - - {AQAAABAAAAAwykg9OxsaVN4tRz33o0I7RoexPAcg52/IhBfyoRqKMNNMjFH3SREdkGrpde49dhP1Otr6wbrbG8dE+E/bsd+fZQ==} - - - - - - all - false - false - - - - - - - - - false - - - #jbcrypt:$2a$10$9FVRXp7wUK4VFULhuZqBguHtsqGa9KRmJzfe6FPTjE8Ve0D/Bca.a - - - diff --git a/test/src/test/resources/hudson/model/UserTest/legacyUserConfigDirsMigrated/users/foo/bar/config.xml b/test/src/test/resources/hudson/model/UserTest/legacyUserConfigDirsMigrated/users/foo/bar/config.xml deleted file mode 100644 index d36c3e6ad7..0000000000 --- a/test/src/test/resources/hudson/model/UserTest/legacyUserConfigDirsMigrated/users/foo/bar/config.xml +++ /dev/null @@ -1,29 +0,0 @@ - - - Foo Bar - - - {AQAAABAAAAAwqrNFnYtDmkxAb5Qs1j0lciURihCOMJ7xMp4fI279OIBsfW3WtxiigLfOzn+3yPU5v2d0/hY1ABrz/elzhs7xFA==} - - - - - - all - false - false - - - - - - - - - false - - - #jbcrypt:$2a$10$1D8djmPHEmDX9TxJ4DG4eud936OUb8dB3rp9hTFStx5fIZ4.ZdRGO - - - diff --git a/test/src/test/resources/hudson/model/UserTest/legacyUserConfigDirsMigrated/users/foo/baz/config.xml b/test/src/test/resources/hudson/model/UserTest/legacyUserConfigDirsMigrated/users/foo/baz/config.xml deleted file mode 100644 index 9bd5850c42..0000000000 --- a/test/src/test/resources/hudson/model/UserTest/legacyUserConfigDirsMigrated/users/foo/baz/config.xml +++ /dev/null @@ -1,29 +0,0 @@ - - - Foo Baz - - - {AQAAABAAAAAwkxG8Z1KYPi1+vDNVMpTlAypMDOGQJFjr75hA+SUXdcohXsDRsi2L1OTrRvbmFlK02pYleeogvPoblSUuQsJOSA==} - - - - - - all - false - false - - - - - - - - - false - - - #jbcrypt:$2a$10$RSZgsv46gLn5LJhpagbilOC/Pkc.74scxNbw6RqONsjc9fF3X4vry - - - diff --git a/test/src/test/resources/hudson/model/UserTest/legacyUserConfigDirsMigrated/users/foo/config.xml b/test/src/test/resources/hudson/model/UserTest/legacyUserConfigDirsMigrated/users/foo/config.xml deleted file mode 100644 index 6c4219c942..0000000000 --- a/test/src/test/resources/hudson/model/UserTest/legacyUserConfigDirsMigrated/users/foo/config.xml +++ /dev/null @@ -1,29 +0,0 @@ - - - Foo - - - {AQAAABAAAAAwre0RkJCk2SvcRB7Lce+95AQodPejQb5rJMjuyd6yrSB/hQeJZzPkW4lbT9ueJO0Ui5CpYeRPEVHQMxvvvgy6dQ==} - - - - - - all - false - false - - - - - - - - - false - - - #jbcrypt:$2a$10$1sYZ.uQWbRqioql2bqLkfOjuORvXD2b42HMwLhU/9EB2TVfjvSpAG - - - diff --git a/test/src/test/resources/hudson/model/UserTest/migration.zip b/test/src/test/resources/hudson/model/UserTest/migration.zip deleted file mode 100644 index c5b73331c0..0000000000 Binary files a/test/src/test/resources/hudson/model/UserTest/migration.zip and /dev/null differ diff --git a/test/src/test/resources/hudson/model/UserTest/shellyUsernameMigrated.zip b/test/src/test/resources/hudson/model/UserTest/shellyUsernameMigrated.zip deleted file mode 100644 index 5d57a6a6a2..0000000000 Binary files a/test/src/test/resources/hudson/model/UserTest/shellyUsernameMigrated.zip and /dev/null differ diff --git a/test/src/test/resources/hudson/security/ExtendedReadPermissionTest.zip b/test/src/test/resources/hudson/security/ExtendedReadPermissionTest.zip deleted file mode 100644 index 2084f20e15..0000000000 Binary files a/test/src/test/resources/hudson/security/ExtendedReadPermissionTest.zip and /dev/null differ diff --git a/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/config.xml b/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/config.xml new file mode 100644 index 0000000000..2a9a865b87 --- /dev/null +++ b/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/config.xml @@ -0,0 +1,40 @@ + + + 1.324-SNAPSHOT (private-09/11/2009 02:51-dty) + 2 + NORMAL + true + + hudson.model.Hudson.Administer:admin + hudson.model.Item.Read:bob + hudson.model.Item.Configure:alice + hudson.model.Item.Read:alice + hudson.model.Hudson.Read:bob + hudson.model.Hudson.Read:admin + hudson.model.Hudson.Read:alice + hudson.model.Hudson.Read:charlie + hudson.model.Item.Read:charlie + hudson.model.Item.ExtendedRead:charlie + + + false + + + + + 5 + 0 + + + + All + + + All + 0 + + + + + true + diff --git a/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/jobs/a/config.xml b/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/jobs/a/config.xml new file mode 100644 index 0000000000..f7568beeaa --- /dev/null +++ b/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/jobs/a/config.xml @@ -0,0 +1,16 @@ + + + + + false + + + true + false + false + + false + + + + \ No newline at end of file diff --git a/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/secret.key b/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/secret.key new file mode 100644 index 0000000000..b29b25fcb5 --- /dev/null +++ b/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/secret.key @@ -0,0 +1 @@ +bd50c69c18335c3c4a3280f5444c4d87469b42e1b4766ac2af4dac530ab6f8cf \ No newline at end of file diff --git a/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/users/user11111111111111/config.xml b/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/users/user11111111111111/config.xml new file mode 100644 index 0000000000..206936caa6 --- /dev/null +++ b/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/users/user11111111111111/config.xml @@ -0,0 +1,21 @@ + + + bob + + + All + + + + All + + + + + zNMhSw:2d42c2730e9b405dcf091e33e4269e7fa2ae8020d167abc60a1cdc0112d72d7a + + + bob@example.com + + + \ No newline at end of file diff --git a/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/users/user2222222222222/config.xml b/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/users/user2222222222222/config.xml new file mode 100644 index 0000000000..18aa0ef58d --- /dev/null +++ b/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/users/user2222222222222/config.xml @@ -0,0 +1,21 @@ + + + charlie + + + All + + + + All + + + + + qxVPYr:8ed58e55b0b6146641e2a740363ff3678ad15cc8043c3258592314eaf79719d5 + + + charlie@example.com + + + \ No newline at end of file diff --git a/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/users/user4975613142751794216/config.xml b/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/users/user4975613142751794216/config.xml new file mode 100644 index 0000000000..aa792194e7 --- /dev/null +++ b/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/users/user4975613142751794216/config.xml @@ -0,0 +1,21 @@ + + + admin + + + All + + + + All + + + + + VxooAI:dfee980f5a932f67590bd6e99623fcfe4c14beb01dc031440925207c9233b1be + + + admin@example.com + + + \ No newline at end of file diff --git a/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/users/user8888888888888888888/config.xml b/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/users/user8888888888888888888/config.xml new file mode 100644 index 0000000000..5fe0c3a909 --- /dev/null +++ b/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/users/user8888888888888888888/config.xml @@ -0,0 +1,21 @@ + + + alice + + + All + + + + All + + + + + hWlWwy:dd85aa451f437bd739f2ee7f958cd22d28b746dd62bf9caf0a369925169145b6 + + + alice@example.com + + + \ No newline at end of file diff --git a/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/users/users.xml b/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/users/users.xml new file mode 100644 index 0000000000..db93243a46 --- /dev/null +++ b/test/src/test/resources/hudson/security/ExtendedReadPermissionTest/users/users.xml @@ -0,0 +1,21 @@ + + + + + admin + user4975613142751794216 + + + alice + user8888888888888888888 + + + bob + user11111111111111 + + + charlie + user2222222222222 + + +