Merge pull request #281 from mrjoel/mrjoel-ticketswithexternalurl

allow internal privilege evaluation external URLs

4 files changed, 49 insertions, 30 deletions

diff --git a/src/main/java/com/gitblit/manager/ServicesManager.java b/src/main/java/com/gitblit/manager/ServicesManager.java
index c911f31a..b993eb66 100644
--- a/src/main/java/com/gitblit/manager/ServicesManager.java
+++ b/src/main/java/com/gitblit/manager/ServicesManager.java
@@ -211,16 +211,35 @@ public class ServicesManager implements IServicesManager {
 		// add all other urls
 		// {0} = repository
 		// {1} = username
+		boolean advertisePermsForOther = settings.getBoolean(Keys.web.advertiseAccessPermissionForOtherUrls, false);
 		for (String url : settings.getStrings(Keys.web.otherUrls)) {
+			String externalUrl = null;
+
 			if (url.contains("{1}")) {
 				// external url requires username, only add url IF we have one
-				if (!StringUtils.isEmpty(username)) {
-					list.add(new RepositoryUrl(MessageFormat.format(url, repository.name, username), null));
+				if (StringUtils.isEmpty(username)) {
+					continue;
+				} else {
+					externalUrl = MessageFormat.format(url, repository.name, username);
 				}
 			} else {
-				// external url does not require username
-				list.add(new RepositoryUrl(MessageFormat.format(url, repository.name), null));
+				// external url does not require username, just do repo name formatting
+				externalUrl = MessageFormat.format(url, repository.name);
+			}
+
+			AccessPermission permission = null;
+			if (advertisePermsForOther) {
+				permission = user.getRepositoryPermission(repository).permission;
+				if (permission.exceeds(AccessPermission.NONE)) {
+					Transport transport = Transport.fromUrl(externalUrl);
+					if (permission.atLeast(AccessPermission.PUSH) && !acceptsPush(transport)) {
+						// downgrade the repo permission for this transport
+						// because it is not an acceptable PUSH transport
+						permission = AccessPermission.CLONE;
+					}
+				}
 			}
+			list.add(new RepositoryUrl(externalUrl, permission));
 		}
 
 		// sort transports by highest permission and then by transport security
@@ -228,13 +247,13 @@ public class ServicesManager implements IServicesManager {
 
 			@Override
 			public int compare(RepositoryUrl o1, RepositoryUrl o2) {
-				if (!o1.isExternal() && o2.isExternal()) {
-					// prefer Gitblit over external
+				if (o1.hasPermission() && !o2.hasPermission()) {
+					// prefer known permission items over unknown
 					return -1;
-				} else if (o1.isExternal() && !o2.isExternal()) {
-					// prefer Gitblit over external
+				} else if (!o1.hasPermission() && o2.hasPermission()) {
+					// prefer known permission items over unknown
 					return 1;
-				} else if (o1.isExternal() && o2.isExternal()) {
+				} else if (!o1.hasPermission() && !o2.hasPermission()) {
 					// sort by Transport ordinal
 					return o1.transport.compareTo(o2.transport);
 				} else if (o1.permission.exceeds(o2.permission)) {
diff --git a/src/main/java/com/gitblit/models/RepositoryUrl.java b/src/main/java/com/gitblit/models/RepositoryUrl.java
index d155dbda..13f69175 100644
--- a/src/main/java/com/gitblit/models/RepositoryUrl.java
+++ b/src/main/java/com/gitblit/models/RepositoryUrl.java
@@ -41,8 +41,8 @@ public class RepositoryUrl implements Serializable {
 		this.permission = permission;

 	}

 

-	public boolean isExternal() {

-		return permission == null;

+	public boolean hasPermission() {

+		return permission != null;

 	}

 

 	@Override

diff --git a/src/main/java/com/gitblit/wicket/pages/TicketPage.java b/src/main/java/com/gitblit/wicket/pages/TicketPage.java
index 1aa89540..2dbc8e9d 100644
--- a/src/main/java/com/gitblit/wicket/pages/TicketPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/TicketPage.java
@@ -752,7 +752,7 @@ public class TicketPage extends RepositoryPage {
 		if (currentPatchset == null) {

 			// no patchset available

 			RepositoryUrl repoUrl = getRepositoryUrl(user, repository);

-			boolean canPropose = repoUrl != null && repoUrl.permission.atLeast(AccessPermission.CLONE) && !UserModel.ANONYMOUS.equals(user);

+			boolean canPropose = repoUrl != null && repoUrl.hasPermission() && repoUrl.permission.atLeast(AccessPermission.CLONE) && !UserModel.ANONYMOUS.equals(user);

 			if (ticket.isOpen() && app().tickets().isAcceptingNewPatchsets(repository) && canPropose) {

 				// ticket & repo will accept a proposal patchset

 				// show the instructions for proposing a patchset

diff --git a/src/main/java/com/gitblit/wicket/panels/RepositoryUrlPanel.java b/src/main/java/com/gitblit/wicket/panels/RepositoryUrlPanel.java
index 0666fcd8..207f1250 100644
--- a/src/main/java/com/gitblit/wicket/panels/RepositoryUrlPanel.java
+++ b/src/main/java/com/gitblit/wicket/panels/RepositoryUrlPanel.java
@@ -84,7 +84,7 @@ public class RepositoryUrlPanel extends BasePanel {
 		// grab primary url from the top of the list

 		primaryUrl = repositoryUrls.size() == 0 ? null : repositoryUrls.get(0);

 

-		boolean canClone = primaryUrl != null && ((primaryUrl.permission == null) || primaryUrl.permission.atLeast(AccessPermission.CLONE));

+		boolean canClone = primaryUrl != null && (!primaryUrl.hasPermission() || primaryUrl.permission.atLeast(AccessPermission.CLONE));

 

 		if (repositoryUrls.size() == 0 || !canClone) {

 			// no urls, nothing to show.

@@ -145,7 +145,7 @@ public class RepositoryUrlPanel extends BasePanel {
 					fragment.add(content);

 					item.add(fragment);

 

-					Label permissionLabel = new Label("permission", repoUrl.isExternal() ? externalPermission : repoUrl.permission.toString());

+					Label permissionLabel = new Label("permission", repoUrl.hasPermission() ? repoUrl.permission.toString() : externalPermission);

 					WicketUtils.setPermissionClass(permissionLabel, repoUrl.permission);

 					String tooltip = getProtocolPermissionDescription(repository, repoUrl);

 					WicketUtils.setHtmlTooltip(permissionLabel, tooltip);

@@ -201,7 +201,7 @@ public class RepositoryUrlPanel extends BasePanel {
 

 		urlPanel.add(new Label("primaryUrl", primaryUrl.url).setRenderBodyOnly(true));

 

-		Label permissionLabel = new Label("primaryUrlPermission", primaryUrl.isExternal() ? externalPermission : primaryUrl.permission.toString());

+		Label permissionLabel = new Label("primaryUrlPermission", primaryUrl.hasPermission() ? primaryUrl.permission.toString() : externalPermission);

 		String tooltip = getProtocolPermissionDescription(repository, primaryUrl);

 		WicketUtils.setHtmlTooltip(permissionLabel, tooltip);

 		urlPanel.add(permissionLabel);

@@ -234,8 +234,8 @@ public class RepositoryUrlPanel extends BasePanel {
 				// filter the urls for the client app

 				List<RepositoryUrl> urls = new ArrayList<RepositoryUrl>();

 				for (RepositoryUrl repoUrl : repositoryUrls) {

-					if (clientApp.minimumPermission == null || repoUrl.permission == null) {

-						// no minimum permission or external permissions, assume it is satisfactory

+					if (clientApp.minimumPermission == null || !repoUrl.hasPermission()) {

+						// no minimum permission or untracked permissions, assume it is satisfactory

 						if (clientApp.supportsTransport(repoUrl.url)) {

 							urls.add(repoUrl);

 						}

@@ -339,7 +339,7 @@ public class RepositoryUrlPanel extends BasePanel {
 	}

 

 	protected Label createPermissionBadge(String wicketId, RepositoryUrl repoUrl) {

-		Label permissionLabel = new Label(wicketId, repoUrl.isExternal() ? externalPermission : repoUrl.permission.toString());

+		Label permissionLabel = new Label(wicketId, repoUrl.hasPermission() ? repoUrl.permission.toString() : externalPermission);

 		WicketUtils.setPermissionClass(permissionLabel, repoUrl.permission);

 		String tooltip = getProtocolPermissionDescription(repository, repoUrl);

 		WicketUtils.setHtmlTooltip(permissionLabel, tooltip);

@@ -369,18 +369,7 @@ public class RepositoryUrlPanel extends BasePanel {
 			RepositoryUrl repoUrl) {

 		if (!urlPermissionsMap.containsKey(repoUrl.url)) {

 			String note;

-			if (repoUrl.isExternal()) {

-				String protocol;

-				int protocolIndex = repoUrl.url.indexOf("://");

-				if (protocolIndex > -1) {

-					// explicit protocol specified

-					protocol = repoUrl.url.substring(0, protocolIndex);

-				} else {

-					// implicit SSH url

-					protocol = "ssh";

-				}

-				note = MessageFormat.format(getString("gb.externalPermissions"), protocol);

-			} else {

+			if (repoUrl.hasPermission()) {

 				note = null;

 				String key;

 				switch (repoUrl.permission) {

@@ -411,6 +400,17 @@ public class RepositoryUrlPanel extends BasePanel {
 					String description = MessageFormat.format(pattern, repoUrl.permission.toString());

 					note = description;

 				}

+			} else {

+				String protocol;

+				int protocolIndex = repoUrl.url.indexOf("://");

+				if (protocolIndex > -1) {

+					// explicit protocol specified

+					protocol = repoUrl.url.substring(0, protocolIndex);

+				} else {

+					// implicit SSH url

+					protocol = "ssh";

+				}

+				note = MessageFormat.format(getString("gb.externalPermissions"), protocol);

 			}

 			urlPermissionsMap.put(repoUrl.url, note);

 		}