From 43ceffafde7da86454c32cbac6bc5ed0f754f174 Mon Sep 17 00:00:00 2001 From: Adrian Gonzalez Date: Wed, 17 Jul 2013 15:57:26 -0400 Subject: [PATCH] Added support for ssh remote mirrors (fixes #10) --- .../stash/hook/MirrorRepositoryHook.java | 55 +++++++++++++++---- .../static/mirror-repository-hook.soy | 8 +-- .../DefaultSettingsReflectionHelperTest.java | 6 +- .../stash/hook/MirrorRepositoryHookTest.java | 41 ++++++++++++-- 4 files changed, 82 insertions(+), 28 deletions(-) diff --git a/src/main/java/com/englishtown/stash/hook/MirrorRepositoryHook.java b/src/main/java/com/englishtown/stash/hook/MirrorRepositoryHook.java index 19b2fa1..460f908 100644 --- a/src/main/java/com/englishtown/stash/hook/MirrorRepositoryHook.java +++ b/src/main/java/com/englishtown/stash/hook/MirrorRepositoryHook.java @@ -152,6 +152,12 @@ public class MirrorRepositoryHook implements AsyncPostReceiveRepositoryHook, Rep protected URI getAuthenticatedUrl(String mirrorRepoUrl, String username, String password) throws URISyntaxException { URI uri = URI.create(mirrorRepoUrl); + + // ssh doesn't have username/password + if (uri.getScheme().equals("ssh")) { + return uri; + } + String userInfo = username + ":" + password; return new URI(uri.getScheme(), userInfo, uri.getHost(), uri.getPort(), @@ -225,6 +231,8 @@ public class MirrorRepositoryHook implements AsyncPostReceiveRepositoryHook, Rep protected boolean validate(MirrorSettings ms, Settings settings, SettingsValidationErrors errors) { boolean result = true; + boolean isHttp = false; + boolean isSsh = false; if (ms.mirrorRepoUrl.isEmpty()) { result = false; @@ -233,27 +241,50 @@ public class MirrorRepositoryHook implements AsyncPostReceiveRepositoryHook, Rep URI uri; try { uri = URI.create(ms.mirrorRepoUrl); - if (!uri.getScheme().toLowerCase().startsWith("http") || ms.mirrorRepoUrl.contains("@")) { + String scheme = uri.getScheme().toLowerCase(); + + if (scheme.equals("ssh")) { + isSsh = true; + if (!ms.mirrorRepoUrl.startsWith("ssh://git@")) { + result = false; + errors.addFieldError(SETTING_MIRROR_REPO_URL + ms.suffix, + "An ssh URL should start with ssh://git@"); + } + } else if (scheme.startsWith("http")) { + isHttp = true; + if (ms.mirrorRepoUrl.contains("@")) { + result = false; + errors.addFieldError(SETTING_MIRROR_REPO_URL + ms.suffix, + "The username and password should not be included."); + } + } else { result = false; errors.addFieldError(SETTING_MIRROR_REPO_URL + ms.suffix, - "The mirror repo url must be a valid http(s) URI and the username " + - "should be specified separately."); + "The mirror repo url must be a ssh or http(s) URL."); } + } catch (Exception ex) { result = false; errors.addFieldError(SETTING_MIRROR_REPO_URL + ms.suffix, - "The mirror repo url must be a valid http(s) URI."); + "The mirror repo url must be a valid ssh or http(s) URL."); } } - if (ms.username.isEmpty()) { - result = false; - errors.addFieldError(SETTING_USERNAME + ms.suffix, "The username is required."); - } + // HTTP must have username and password + if (isHttp) { + if (ms.username.isEmpty()) { + result = false; + errors.addFieldError(SETTING_USERNAME + ms.suffix, "The username is required when using http(s)."); + } - if (ms.password.isEmpty()) { - result = false; - errors.addFieldError(SETTING_PASSWORD + ms.suffix, "The password is required."); + if (ms.password.isEmpty()) { + result = false; + errors.addFieldError(SETTING_PASSWORD + ms.suffix, "The password is required when using http(s)."); + } + } + // SSH should not have username or password + if (isSsh) { + ms.password = ms.username = ""; } return result; @@ -267,7 +298,7 @@ public class MirrorRepositoryHook implements AsyncPostReceiveRepositoryHook, Rep for (MirrorSettings ms : mirrorSettings) { values.put(SETTING_MIRROR_REPO_URL + ms.suffix, ms.mirrorRepoUrl); values.put(SETTING_USERNAME + ms.suffix, ms.username); - values.put(SETTING_PASSWORD + ms.suffix, passwordEncryptor.encrypt(ms.password)); + values.put(SETTING_PASSWORD + ms.suffix, (ms.password.isEmpty() ? ms.password : passwordEncryptor.encrypt(ms.password))); } // Unfortunately the settings are stored in an immutable map, so need to cheat with reflection diff --git a/src/main/resources/static/mirror-repository-hook.soy b/src/main/resources/static/mirror-repository-hook.soy index 72e830a..6312e3e 100644 --- a/src/main/resources/static/mirror-repository-hook.soy +++ b/src/main/resources/static/mirror-repository-hook.soy @@ -47,7 +47,7 @@ {/param} {param isRequired: true /} {param descriptionText: stash_i18n('com.englishtown.stash.hook.mirror.strings.mirrorRepoUrl.description', - 'URL to the remote mirrored repo') /} + 'The ssh or http(s) URL to the remote mirrored repo') /} {param extraClasses: 'long et-mirror-repo' /} {param errorTexts: $errors ? $errors['mirrorRepoUrl' + $index] : null /} {/call} @@ -57,9 +57,8 @@ {param labelContent} {stash_i18n('com.englishtown.stash.hook.mirror.strings.username.label', 'Username')} {/param} - {param isRequired: true /} {param descriptionText: stash_i18n('com.englishtown.stash.hook.mirror.strings.username.description', - 'The username to use for pushing to the mirror') /} + 'The username to use for pushing to the mirror over http(s)') /} {param extraClasses: 'long' /} {param errorTexts: $errors ? $errors['username' + $index] : null /} {/call} @@ -69,9 +68,8 @@ {param labelContent} {stash_i18n('com.englishtown.stash.hook.mirror.strings.password.label', 'Password')} {/param} - {param isRequired: true /} {param descriptionText: stash_i18n('com.englishtown.stash.hook.mirror.strings.password.description', - 'The password to use for pushing to the mirror') /} + 'The password to use for pushing to the mirror over http(s)') /} {param extraClasses: 'long' /} {param errorTexts: $errors ? $errors['password' + $index] : null /} {/call} diff --git a/src/test/java/com/englishtown/stash/hook/DefaultSettingsReflectionHelperTest.java b/src/test/java/com/englishtown/stash/hook/DefaultSettingsReflectionHelperTest.java index 94961aa..798be36 100644 --- a/src/test/java/com/englishtown/stash/hook/DefaultSettingsReflectionHelperTest.java +++ b/src/test/java/com/englishtown/stash/hook/DefaultSettingsReflectionHelperTest.java @@ -11,11 +11,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; /** - * Created with IntelliJ IDEA. - * User: adriangonzalez - * Date: 5/13/13 - * Time: 3:54 PM - * To change this template use File | Settings | File Templates. + * Unit tests for {@link DefaultSettingsReflectionHelper} */ public class DefaultSettingsReflectionHelperTest { diff --git a/src/test/java/com/englishtown/stash/hook/MirrorRepositoryHookTest.java b/src/test/java/com/englishtown/stash/hook/MirrorRepositoryHookTest.java index 7683de6..b59db33 100644 --- a/src/test/java/com/englishtown/stash/hook/MirrorRepositoryHookTest.java +++ b/src/test/java/com/englishtown/stash/hook/MirrorRepositoryHookTest.java @@ -56,7 +56,8 @@ public class MirrorRepositoryHookTest { @Mock private PluginSettings pluginSettings; - private final String mirrorRepoUrl = "https://stash-mirror.englishtown.com/scm/test/test.git"; + private final String mirrorRepoUrlHttp = "https://stash-mirror.englishtown.com/scm/test/test.git"; + private final String mirrorRepoUrlSsh = "ssh://git@stash-mirror.englishtown.com/scm/test/test.git"; private final String username = "test-user"; private final String password = "test-password"; private final String repository = "https://test-user:test-password@stash-mirror.englishtown.com/scm/test/test.git"; @@ -98,7 +99,7 @@ public class MirrorRepositoryHookTest { Settings settings = mock(Settings.class); when(settings.asMap()).thenReturn(map); - when(settings.getString(eq(MirrorRepositoryHook.SETTING_MIRROR_REPO_URL), eq(""))).thenReturn(mirrorRepoUrl); + when(settings.getString(eq(MirrorRepositoryHook.SETTING_MIRROR_REPO_URL), eq(""))).thenReturn(mirrorRepoUrlHttp); when(settings.getString(eq(MirrorRepositoryHook.SETTING_USERNAME), eq(""))).thenReturn(username); when(settings.getString(eq(MirrorRepositoryHook.SETTING_PASSWORD), eq(""))).thenReturn(password); @@ -123,7 +124,7 @@ public class MirrorRepositoryHookTest { MirrorRepositoryHook hook = new MirrorRepositoryHook(gitScm, mock(I18nService.class), executor, passwordEncryptor, settingsReflectionHelper, pluginSettingsFactory); MirrorRepositoryHook.MirrorSettings ms = new MirrorRepositoryHook.MirrorSettings(); - ms.mirrorRepoUrl = mirrorRepoUrl; + ms.mirrorRepoUrl = mirrorRepoUrlHttp; ms.username = username; ms.password = password; hook.runMirrorCommand(ms, mock(Repository.class)); @@ -177,7 +178,7 @@ public class MirrorRepositoryHookTest { URI result; - result = hook.getAuthenticatedUrl(mirrorRepoUrl, username, password); + result = hook.getAuthenticatedUrl(mirrorRepoUrlHttp, username, password); assertEquals(repository, result.toString()); } @@ -195,15 +196,22 @@ public class MirrorRepositoryHookTest { when(settings.getString(eq(MirrorRepositoryHook.SETTING_MIRROR_REPO_URL + "0"), eq(""))) .thenThrow(new RuntimeException("Intentional unit test exception")) .thenReturn("") + .thenReturn(mirrorRepoUrlHttp) .thenReturn("invalid uri") .thenReturn("http://should-not:have-user@stash-mirror.englishtown.com/scm/test/test.git") - .thenReturn(mirrorRepoUrl); + .thenReturn("ssh://user@stash-mirror.englishtown.com/scm/test/test.git") + .thenReturn(mirrorRepoUrlSsh) + .thenReturn(mirrorRepoUrlHttp); when(settings.getString(eq(MirrorRepositoryHook.SETTING_USERNAME + "0"), eq(""))) + .thenReturn("") + .thenReturn("") .thenReturn("") .thenReturn(username); when(settings.getString(eq(MirrorRepositoryHook.SETTING_PASSWORD + "0"), eq(""))) + .thenReturn("") + .thenReturn("") .thenReturn("") .thenReturn(password); @@ -213,11 +221,19 @@ public class MirrorRepositoryHookTest { errors = mock(SettingsValidationErrors.class); hook.validate(settings, errors, repo); verify(errors, times(1)).addFormError(anyString()); + verify(errors, never()).addFieldError(anyString(), anyString()); errors = mock(SettingsValidationErrors.class); hook.validate(settings, errors, repo); verify(errors, never()).addFormError(anyString()); verify(errors).addFieldError(eq(MirrorRepositoryHook.SETTING_MIRROR_REPO_URL + "0"), anyString()); + verify(errors, never()).addFieldError(eq(MirrorRepositoryHook.SETTING_USERNAME + "0"), anyString()); + verify(errors, never()).addFieldError(eq(MirrorRepositoryHook.SETTING_PASSWORD + "0"), anyString()); + + errors = mock(SettingsValidationErrors.class); + hook.validate(settings, errors, repo); + verify(errors, never()).addFormError(anyString()); + verify(errors, never()).addFieldError(eq(MirrorRepositoryHook.SETTING_MIRROR_REPO_URL + "0"), anyString()); verify(errors).addFieldError(eq(MirrorRepositoryHook.SETTING_USERNAME + "0"), anyString()); verify(errors).addFieldError(eq(MirrorRepositoryHook.SETTING_PASSWORD + "0"), anyString()); @@ -231,7 +247,20 @@ public class MirrorRepositoryHookTest { hook.validate(settings, errors, repo); verify(errors, never()).addFormError(anyString()); verify(errors).addFieldError(eq(MirrorRepositoryHook.SETTING_MIRROR_REPO_URL + "0"), anyString()); - verify(errors).addFieldError(anyString(), anyString()); + verify(errors, never()).addFieldError(eq(MirrorRepositoryHook.SETTING_USERNAME + "0"), anyString()); + verify(errors, never()).addFieldError(eq(MirrorRepositoryHook.SETTING_PASSWORD + "0"), anyString()); + + errors = mock(SettingsValidationErrors.class); + hook.validate(settings, errors, repo); + verify(errors, never()).addFormError(anyString()); + verify(errors).addFieldError(eq(MirrorRepositoryHook.SETTING_MIRROR_REPO_URL + "0"), anyString()); + verify(errors, never()).addFieldError(eq(MirrorRepositoryHook.SETTING_USERNAME + "0"), anyString()); + verify(errors, never()).addFieldError(eq(MirrorRepositoryHook.SETTING_PASSWORD + "0"), anyString()); + + errors = mock(SettingsValidationErrors.class); + hook.validate(settings, errors, repo); + verify(errors, never()).addFormError(anyString()); + verify(errors, never()).addFieldError(anyString(), anyString()); when(passwordEncryptor.isEncrypted(anyString())).thenReturn(true); errors = mock(SettingsValidationErrors.class);