From 1590fd791c7fc4d6849479cad88956f37360bbbf Mon Sep 17 00:00:00 2001 From: Joel Johnson Date: Mon, 29 Jun 2015 17:13:19 -0600 Subject: [PATCH] allow advertising gitblit privileges for external URLs commit c20191fc0931a19bec0df1ab2b56f287e5d8b7c7 enabled support for hiding internal URLs, but didn't consider that it broke the evaluation of permissions (used for tickets, etc.), and caused a NPE on repoUrl.permission when trying to view the TicketPage. With all internal mechanisms disabled, it would result in the first URL being external with unknown permissions. This adds an option to use internal permissions even for external URLs. Note that this does not grant any additional permissions, but does offer the option to have gitblit advertise the full set of what is allowed, even if the external URL imposes additional restrictions. --- src/main/distrib/data/defaults.properties | 14 +++++++ .../com/gitblit/manager/ServicesManager.java | 37 ++++++++++++++----- .../com/gitblit/models/RepositoryUrl.java | 4 +- .../com/gitblit/wicket/pages/TicketPage.java | 2 +- .../wicket/panels/RepositoryUrlPanel.java | 36 +++++++++--------- 5 files changed, 63 insertions(+), 30 deletions(-) diff --git a/src/main/distrib/data/defaults.properties b/src/main/distrib/data/defaults.properties index 60c914ba..d4ebcc39 100644 --- a/src/main/distrib/data/defaults.properties +++ b/src/main/distrib/data/defaults.properties @@ -1203,6 +1203,20 @@ web.showGitDaemonUrls = true # SINCE 1.7.0 web.showSshDaemonUrls = true +# Should effective permissions be advertised for access paths defined in web.otherUrls? +# If false, gitblit will indicate unknown permissions for the external link. If true, +# gitblit will indicate permissions as defined within gitblit (including limiting to clone +# permission is the transport type is not a valid push mechaism in git.acceptedPushTransports). +# +# Configure with caution: Note that gitblit has no way of knowing if further restrictions +# are imposed by an external forwarding agent, so this may cause user confusion due to +# more rights being advertised than are available through the URL. It will NOT grant +# additional rights, but may incorrectly offer actions that are unavailable externally. +# default: false +# +# SINCE 1.7.0 +web.advertiseAccessPermissionForOtherUrls = false + # Should app-specific clone links be displayed for SourceTree, SparkleShare, etc? # # SINCE 1.3.0 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 urls = new ArrayList(); 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); } -- 2.39.5