Skip to content

Commit

Permalink
[JENKINS-13742] Field validation does not pass required query parameters
Browse files Browse the repository at this point in the history 
when some fields are specified in a nested Describable

# On branch master
# Changes not staged for commit:
#   (use "git add <file>..." to update what will be committed)
#   (use "git checkout -- <file>..." to discard changes in working directory)

* src/main/java/com/collabnet/ce/webservices/CTFProject.java
  (getOrCreateDocumentFolder, verifyPath): strip file separators from beginning and end of path

* src/main/java/com/collabnet/ce/webservices/CollabNetApp.java
  (fromStapler): renamed overrideAuth to connectionFactory, the former is more descriptive,
  but the checkbox takes the name of the composite field.  Added RelativeParameter
  annotations to the other arguments. Also the password field may contain the encrypted
  form, so using Secret to transform into plaintext.

* src/main/java/hudson/plugins/collabnet/ConnectionFactory.java
  Add a fromStapler method so this can be used in validations that are internal to the
  composite field, such as the modified doCheckPassword method.

* src/main/java/hudson/plugins/collabnet/auth/CNAuthProjectProperty.java
  (doFillProjectItems): simplified by using getProjectList method.

* src/main/java/hudson/plugins/collabnet/browser/TeamForge.java
  (doFillProjectItems, doFillRepoItems): Added code to logoff the CTF session

* src/main/java/hudson/plugins/collabnet/tracker/CNTracker.java
  (doFillTrackerItems): Add a null check to avoid NPE, if auth data is not found

* src/main/java/hudson/plugins/collabnet/util/CNFormFieldValidator.java
  Several methods are modified to add null checks for the CollabNetAuth arg and to
  ensure the session is logged off.  Prior to this change some methods did one
  and others did the other and some did neither.

* src/main/java/hudson/plugins/collabnet/util/ComboBoxUpdater.java
  (getUsers): Add null check on CollabNetApp arg
  • Loading branch information
@jmcnally
jmcnally committed Feb 1, 2013
1 parent 85cde14 commit 028c3cf
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 47 deletions.
12 changes: 12 additions & 0 deletions src/main/java/com/collabnet/ce/webservices/CTFProject.java
View file
Expand Up @@ -169,6 +169,7 @@ public CTFDocumentFolder getRootFolder() throws RemoteException {
* Gets to the folder from a path string like "foo/bar/zot", if necessary by creating intermediate directories.
*/
public CTFDocumentFolder getOrCreateDocumentFolder(String documentPath) throws RemoteException {
documentPath = normalizePath(documentPath);
String[] folderNames = documentPath.split("/");
int i = 0;
// find the root folder since the first document path may or may not
Expand All @@ -190,6 +191,16 @@ public CTFDocumentFolder getOrCreateDocumentFolder(String documentPath) throws R
return cur;
}

private String normalizePath(String documentPath) {
if (documentPath.startsWith("/")) {
documentPath = documentPath.substring(1);
}
if (documentPath.endsWith("/")) {
documentPath = documentPath.substring(0, documentPath.length() - 1);
}
return documentPath;
}

/**
* Verify a document folder path. If at any point the folder
* is missing, return the name of the first missing folder.
Expand All @@ -199,6 +210,7 @@ public CTFDocumentFolder getOrCreateDocumentFolder(String documentPath) throws R
* @throws RemoteException
*/
public String verifyPath(String documentPath) throws RemoteException {
documentPath = normalizePath(documentPath);
String[] folderNames = documentPath.split("/");
int i = 0;
CTFDocumentFolder cur = getRootFolder();
Expand Down
16 changes: 13 additions & 3 deletions src/main/java/com/collabnet/ce/webservices/CollabNetApp.java
View file
Expand Up @@ -14,9 +14,11 @@
import com.collabnet.ce.soap50.webservices.rbac.IRbacAppSoap;
import com.collabnet.ce.soap50.webservices.scm.IScmAppSoap;
import com.collabnet.ce.soap50.webservices.tracker.ITrackerAppSoap;
import hudson.RelativePath;
import hudson.plugins.collabnet.share.TeamForgeShare;
import hudson.plugins.collabnet.util.CNHudsonUtil;
import hudson.plugins.collabnet.util.CommonUtil;
import hudson.util.Secret;
import org.apache.axis.AxisFault;
import org.apache.log4j.Logger;
import org.kohsuke.stapler.QueryParameter;
Expand All @@ -38,6 +40,7 @@
import java.util.Collection;
import java.util.List;


/***
* This class represents the connection to the CollabNet webservice.
* Since it contains login/logout data, other webservices will
Expand Down Expand Up @@ -408,12 +411,19 @@ public CollabNetAppException(String msg) {

/**
* A databinding method from Stapler.
* @param connectionFactory relates to the checkbox to override the global authentication parameters
*/
public static CollabNetApp fromStapler(@QueryParameter boolean overrideAuth, @QueryParameter String url,
@QueryParameter String username, @QueryParameter String password) {
public static CollabNetApp fromStapler(@QueryParameter boolean connectionFactory,
@QueryParameter @RelativePath("connectionFactory") String url,
@QueryParameter @RelativePath("connectionFactory") String username,
@QueryParameter @RelativePath("connectionFactory") String password) {

// form may contain password either entered by user or the encrypted value
password = Secret.fromString(password).getPlainText();

TeamForgeShare.TeamForgeShareDescriptor descriptor =
TeamForgeShare.getTeamForgeShareDescriptor();
if (descriptor != null && descriptor.useGlobal() && !overrideAuth) {
if (descriptor != null && descriptor.useGlobal() && !connectionFactory) {
url = descriptor.getCollabNetUrl();
username = descriptor.getUsername();
password = descriptor.getPassword();
Expand Down
19 changes: 18 additions & 1 deletion src/main/java/hudson/plugins/collabnet/ConnectionFactory.java
View file
Expand Up @@ -6,6 +6,7 @@
import hudson.model.Descriptor;
import hudson.plugins.collabnet.util.CNFormFieldValidator;
import hudson.plugins.collabnet.util.CNHudsonUtil;
import hudson.plugins.collabnet.util.CommonUtil;
import hudson.util.FormValidation;
import hudson.util.Secret;
import org.kohsuke.stapler.DataBoundConstructor;
Expand Down Expand Up @@ -67,6 +68,20 @@ public int hashCode() {
return result;
}

/**
* A databinding method from Stapler.
*/
public static ConnectionFactory fromStapler(@QueryParameter String url,
@QueryParameter String username,
@QueryParameter String password) {

if (CommonUtil.unset(url) || CommonUtil.unset(username) || CommonUtil.unset(password)) {
return null;
}
return new ConnectionFactory(url, username, password);
}


@Extension
public static final class DescriptorImpl extends Descriptor<ConnectionFactory> {
public String getDisplayName() { return ""; }
Expand All @@ -92,7 +107,9 @@ public FormValidation doCheckUsermame(@QueryParameter String value) {
/**
* Check that a password is present and allows login.
*/
public FormValidation doCheckPassword(CollabNetApp app, @QueryParameter String value) {
public FormValidation doCheckPassword(ConnectionFactory connectionFactory, @QueryParameter String value) {
CollabNetApp app = (connectionFactory == null) ? null : CNHudsonUtil.getCollabNetApp(
connectionFactory.getUrl(), connectionFactory.getUsername(), connectionFactory.getPassword().getPlainText());
return CNFormFieldValidator.loginCheck(app,value);
}
}
Expand Down
5 changes: 1 addition & 4 deletions src/main/java/hudson/plugins/collabnet/auth/CNAuthProjectProperty.java
View file
Expand Up @@ -332,10 +332,7 @@ public FormValidation doCheckProject(@QueryParameter String value) throws Remote
*/
public ComboBoxModel doFillProjectItems() throws RemoteException {
CollabNetApp conn = CNConnection.getInstance();
if (conn == null) {
return new ComboBoxModel();
}
return ComboBoxUpdater.toModel(conn.getProjects());
return ComboBoxUpdater.getProjectList(conn);
}

/**
Expand Down
8 changes: 6 additions & 2 deletions src/main/java/hudson/plugins/collabnet/browser/TeamForge.java
View file
Expand Up @@ -260,15 +260,19 @@ public FormValidation doCheckRepo(StaplerRequest req) throws RemoteException {
* JSON string into the response data.
*/
public ComboBoxModel doFillProjectItems(CollabNetApp cna) throws IOException {
return ComboBoxUpdater.getProjectList(cna);
ComboBoxModel projects = ComboBoxUpdater.getProjectList(cna);
CNHudsonUtil.logoff(cna);
return projects;
}

/**
* Gets a list of repos to choose from and write them as a
* JSON string into the response data.
*/
public ComboBoxModel doFillRepoItems(CollabNetApp cna, @QueryParameter String project) throws RemoteException {
return ComboBoxUpdater.getRepos(cna,project);
ComboBoxModel repos = ComboBoxUpdater.getRepos(cna,project);
CNHudsonUtil.logoff(cna);
return repos;
}
}
}
2 changes: 1 addition & 1 deletion src/main/java/hudson/plugins/collabnet/tracker/CNTracker.java
View file
Expand Up @@ -637,7 +637,7 @@ public FormValidation doCheckRelease(CollabNetApp cna, @QueryParameter String pr
* JSON string into the response data.
*/
public ComboBoxModel doFillTrackerItems(CollabNetApp cna, @QueryParameter String project) throws RemoteException {
return ComboBoxUpdater.getTrackerList(cna.getProjectByTitle(project));
return ComboBoxUpdater.getTrackerList((cna == null) ? null : cna.getProjectByTitle(project));
}

/**
Expand Down
86 changes: 53 additions & 33 deletions src/main/java/hudson/plugins/collabnet/util/CNFormFieldValidator.java
View file
Expand Up @@ -221,23 +221,28 @@ public static FormValidation projectCheck(CollabNetApp app, String project) thro
* password, project, and path.
*/
public static FormValidation documentPathCheck(CollabNetApp app, String project, String path) throws IOException {
path = path.replaceAll("/+", "/");
path = CommonUtil.stripSlashes(path);
if (CommonUtil.unset(path)) {
return FormValidation.error("The path is required.");
}
checkInterpretedString(path);
try {
path = path.replaceAll("/+", "/");
path = CommonUtil.stripSlashes(path);
if (CommonUtil.unset(path)) {
return FormValidation.error("The path is required.");
}
checkInterpretedString(path);

CTFProject p = app.getProjectByTitle(project);
if (p != null) {
String missing = p.verifyPath(path);
if (missing != null) {
CNHudsonUtil.logoff(app);
return FormValidation.warning(String.format(
"Folder '%s' could not be found in path '%s'. It (and any subfolders) will be created dynamically.", missing, path));
if (app == null) {
return FormValidation.ok();
}
CTFProject p = app.getProjectByTitle(project);
if (p != null) {
String missing = p.verifyPath(path);
if (missing != null) {
return FormValidation.warning(String.format(
"Folder '%s' could not be found in path '%s'. It (and any subfolders) will be created dynamically.", missing, path));
}
}
} finally {
CNHudsonUtil.logoff(app);
}
CNHudsonUtil.logoff(app);
return FormValidation.ok();
}

Expand All @@ -246,10 +251,13 @@ public static FormValidation documentPathCheck(CollabNetApp app, String project,
* a url, username, password, project, and package.
*/
public static FormValidation packageCheck(CollabNetApp cna, String project, String rpackage) throws RemoteException {
if (CommonUtil.unset(rpackage)) {
return FormValidation.error("The package is required.");
}
try {
if (CommonUtil.unset(rpackage)) {
return FormValidation.error("The package is required.");
}
if (cna == null) {
return FormValidation.ok();
}
CTFProject p = cna.getProjectByTitle(project);
if (p != null) {
CTFPackage pkg = p.getPackages().byTitle(rpackage);
Expand All @@ -276,6 +284,10 @@ public static FormValidation releaseCheck(CollabNetApp cna, String project, Stri
return FormValidation.ok();
}
}

if (cna == null) {
return FormValidation.ok();
}
CTFProject p = cna.getProjectByTitle(project);
if (p==null) return FormValidation.ok(); // not entered yet?

Expand Down Expand Up @@ -306,9 +318,9 @@ public static FormValidation repoCheck(StaplerRequest request) throws RemoteExce
String project = request.getParameter("project");
String repoName = request.getParameter("repo");
CollabNetApp cna = CNHudsonUtil.getCollabNetApp(request);
if (cna==null) return FormValidation.ok();

try {
if (cna==null) return FormValidation.ok();

CTFProject p = cna.getProjectByTitle(project);
if (CommonUtil.unset(repoName)) {
return FormValidation.error("The repository name is required.");
Expand Down Expand Up @@ -337,16 +349,22 @@ public static FormValidation trackerCheck(StaplerRequest request) throws RemoteE
return FormValidation.error("The tracker is required.");
}
CollabNetApp cna = CNHudsonUtil.getCollabNetApp(request);
CTFProject p = cna.getProjectByTitle(project);
if (p!=null) {
CTFTracker t = p.getTrackers().byTitle(tracker);
if (t == null) {
CNHudsonUtil.logoff(cna);
return FormValidation.warning("Tracker could not be found.");
try {
if (cna == null) {
return FormValidation.ok();
}

CTFProject p = cna.getProjectByTitle(project);
if (p!=null) {
CTFTracker t = p.getTrackers().byTitle(tracker);
if (t == null) {
return FormValidation.warning("Tracker could not be found.");
}
}
return FormValidation.ok();
} finally {
CNHudsonUtil.logoff(cna);
}
CNHudsonUtil.logoff(cna);
return FormValidation.ok();
}

/**
Expand All @@ -362,10 +380,10 @@ public static FormValidation assignCheck(StaplerRequest request) throws RemoteEx
String project = request.getParameter("project");

CollabNetApp cna = CNHudsonUtil.getCollabNetApp(request);
if (cna == null) {
return FormValidation.ok();
}
try {
if (cna == null) {
return FormValidation.ok();
}
CTFProject p = cna.getProjectById(project);
if (p == null) {
return FormValidation.ok();
Expand Down Expand Up @@ -410,9 +428,11 @@ public static FormValidation userListCheck(String userStr) throws RemoteExceptio
*/
private static Collection<String> getInvalidUsers(CollabNetApp cna, String userStr) throws RemoteException {
Collection<String> invalidUsers = new ArrayList<String>();
for (String user: CommonUtil.splitCommaStr(userStr)) {
if (!cna.isUsernameValid(user)) {
invalidUsers.add(user);
if (cna != null) {
for (String user: CommonUtil.splitCommaStr(userStr)) {
if (!cna.isUsernameValid(user)) {
invalidUsers.add(user);
}
}
}
return invalidUsers;
Expand Down
9 changes: 6 additions & 3 deletions src/main/java/hudson/plugins/collabnet/util/ComboBoxUpdater.java
View file
Expand Up @@ -108,9 +108,12 @@ public static ComboBoxModel getTrackerList(CTFProject p) throws RemoteException
}

public static ComboBoxModel getUsers(CollabNetApp cna, String project) throws RemoteException {
CTFProject p = cna.getProjectByTitle(project);
if (p==null) return EMPTY_MODEL;
return toModel(p.getMembers());
if (cna!=null) {
CTFProject p = cna.getProjectByTitle(project);
if (p==null) return EMPTY_MODEL;
return toModel(p.getMembers());
}
return EMPTY_MODEL;
}

/**
Expand Down

0 comments on commit 028c3cf

Please sign in to comment.