Not exposing username, password, in REST

tomasbjerre · Jul 20, 2017 · ff78ac1 · ff78ac1
1 parent 51599e7
commit ff78ac1
Show file tree

Hide file tree

Showing 13 changed files with 171 additions and 39 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,16 @@
 
 Changelog of Pull Request Notifier for Bitbucket.
 
+## Unreleased
+### No issue
+  Not exposing username, password, in REST
+
+  [a565291be41b9b3](https://github.com/tomasbjerre/pull-request-notifier-for-bitbucket/commit/a565291be41b9b3) Tomas Bjerre *2017-07-20 18:05:37*
+
+  doc
+
+  [51599e783601d51](https://github.com/tomasbjerre/pull-request-notifier-for-bitbucket/commit/51599e783601d51) Tomas Bjerre *2017-07-17 17:21:19*
+
 ## 3.3
 ### GitHub [#233](https://github.com/tomasbjerre/pull-request-notifier-for-bitbucket/issues/233) Variable renderer tries to render extra variables
   Avoid crash when variable resolved to string with dollar sign

diff --git a/src/main/java/se/bjurr/prnfb/presentation/dto/NotificationDTO.java b/src/main/java/se/bjurr/prnfb/presentation/dto/NotificationDTO.java
@@ -8,13 +8,13 @@
 import javax.xml.bind.annotation.XmlAccessorType;
 import javax.xml.bind.annotation.XmlRootElement;
 
-import com.google.common.base.Optional;
-
 import se.bjurr.prnfb.http.UrlInvoker.HTTP_METHOD;
 import se.bjurr.prnfb.service.PrnfbRenderer.ENCODE_FOR;
 import se.bjurr.prnfb.settings.Restricted;
 import se.bjurr.prnfb.settings.TRIGGER_IF_MERGE;
 
+import com.google.common.base.Optional;
+
 @XmlRootElement
 @XmlAccessorType(FIELD)
 public class NotificationDTO implements Comparable<NotificationDTO>, Restricted {
@@ -427,4 +427,55 @@ public void setProxySchema(String proxySchema) {
   public String getProxySchema() {
     return proxySchema;
   }
+
+  @Override
+  public String toString() {
+    return "NotificationDTO [filterRegexp="
+        + filterRegexp
+        + ", filterString="
+        + filterString
+        + ", headers="
+        + headers
+        + ", injectionUrl="
+        + injectionUrl
+        + ", injectionUrlRegexp="
+        + injectionUrlRegexp
+        + ", method="
+        + method
+        + ", name="
+        + name
+        + ", password="
+        + password
+        + ", postContent="
+        + postContent
+        + ", projectKey="
+        + projectKey
+        + ", proxyPassword="
+        + proxyPassword
+        + ", proxyPort="
+        + proxyPort
+        + ", proxyServer="
+        + proxyServer
+        + ", proxySchema="
+        + proxySchema
+        + ", proxyUser="
+        + proxyUser
+        + ", repositorySlug="
+        + repositorySlug
+        + ", triggerIfCanMerge="
+        + triggerIfCanMerge
+        + ", triggerIgnoreStateList="
+        + triggerIgnoreStateList
+        + ", triggers="
+        + triggers
+        + ", url="
+        + url
+        + ", user="
+        + user
+        + ", uuid="
+        + uuid
+        + ", postContentEncoding="
+        + postContentEncoding
+        + "]";
+  }
 }
diff --git a/src/main/java/se/bjurr/prnfb/service/SettingsService.java b/src/main/java/se/bjurr/prnfb/service/SettingsService.java
@@ -9,6 +9,7 @@
 import static com.google.common.collect.Iterables.tryFind;
 import static com.google.common.collect.Lists.newArrayList;
 import static se.bjurr.prnfb.settings.PrnfbNotificationBuilder.prnfbNotificationBuilder;
+import static se.bjurr.prnfb.settings.PrnfbSettings.UNCHANGED;
 import static se.bjurr.prnfb.settings.PrnfbSettingsBuilder.prnfbSettingsBuilder;
 import static se.bjurr.prnfb.settings.PrnfbSettingsDataBuilder.prnfbSettingsDataBuilder;
 
@@ -18,18 +19,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.atlassian.bitbucket.pull.PullRequestState;
-import com.atlassian.bitbucket.user.SecurityService;
-import com.atlassian.bitbucket.util.Operation;
-import com.atlassian.sal.api.pluginsettings.PluginSettings;
-import com.atlassian.sal.api.pluginsettings.PluginSettingsFactory;
-import com.atlassian.sal.api.transaction.TransactionCallback;
-import com.atlassian.sal.api.transaction.TransactionTemplate;
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Optional;
-import com.google.common.base.Predicate;
-import com.google.gson.Gson;
-
 import se.bjurr.prnfb.listener.PrnfbPullRequestAction;
 import se.bjurr.prnfb.presentation.dto.ON_OR_OFF;
 import se.bjurr.prnfb.settings.HasUuid;
@@ -45,6 +34,18 @@
 import se.bjurr.prnfb.settings.legacy.Header;
 import se.bjurr.prnfb.settings.legacy.SettingsStorage;
 
+import com.atlassian.bitbucket.pull.PullRequestState;
+import com.atlassian.bitbucket.user.SecurityService;
+import com.atlassian.bitbucket.util.Operation;
+import com.atlassian.sal.api.pluginsettings.PluginSettings;
+import com.atlassian.sal.api.pluginsettings.PluginSettingsFactory;
+import com.atlassian.sal.api.transaction.TransactionCallback;
+import com.atlassian.sal.api.transaction.TransactionTemplate;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Optional;
+import com.google.common.base.Predicate;
+import com.google.gson.Gson;
+
 public class SettingsService {
 
   public static final String STORAGE_KEY = "se.bjurr.prnfb.pull-request-notifier-for-bitbucket-3";
@@ -233,20 +234,44 @@ private PrnfbButton doAddOrUpdateButton(PrnfbButton prnfbButton) {
     return prnfbButton;
   }
 
-  private PrnfbNotification doAddOrUpdateNotification(PrnfbNotification prnfbNotification)
+  private PrnfbNotification doAddOrUpdateNotification(PrnfbNotification newNotification)
       throws ValidationException {
-    if (findNotification(prnfbNotification.getUuid()).isPresent()) {
-      doDeleteNotification(prnfbNotification.getUuid());
+    Optional<PrnfbNotification> oldNotification = findNotification(newNotification.getUuid());
+    if (oldNotification.isPresent()) {
+      String user = keepIfUnchanged(newNotification.getUser(), oldNotification.get().getUser());
+      String password =
+          keepIfUnchanged(newNotification.getPassword(), oldNotification.get().getPassword());
+      String proxyUser =
+          keepIfUnchanged(newNotification.getProxyUser(), oldNotification.get().getProxyUser());
+      String proxyPassword =
+          keepIfUnchanged(
+              newNotification.getProxyPassword(), oldNotification.get().getProxyPassword());
+      newNotification =
+          prnfbNotificationBuilder(newNotification) //
+              .withUser(user) //
+              .withPassword(password) //
+              .withProxyUser(proxyUser) //
+              .withPassword(proxyPassword) //
+              .build();
+      doDeleteNotification(newNotification.getUuid());
     }
 
     PrnfbSettings originalSettings = doGetPrnfbSettings();
     PrnfbSettings updated =
         prnfbSettingsBuilder(originalSettings) //
-            .withNotification(prnfbNotification) //
+            .withNotification(newNotification) //
             .build();
 
     doSetPrnfbSettings(updated);
-    return prnfbNotification;
+    return newNotification;
+  }
+
+  private String keepIfUnchanged(Optional<String> newValue, Optional<String> oldValue) {
+    boolean isUnchanged = newValue.isPresent() && newValue.get().equals(UNCHANGED);
+    if (isUnchanged) {
+      return oldValue.orNull();
+    }
+    return newValue.orNull();
   }
 
   private void doDeleteButton(UUID uuid) {
@@ -303,8 +328,24 @@ private PrnfbSettings doGetPrnfbSettings() {
     return gson.fromJson(storedSettings.toString(), PrnfbSettings.class);
   }
 
-  private void doSetPrnfbSettings(PrnfbSettings PrnfbSettings) {
-    String data = gson.toJson(PrnfbSettings);
+  private void doSetPrnfbSettings(PrnfbSettings newSettings) {
+    PrnfbSettingsData oldSettingsData = doGetPrnfbSettings().getPrnfbSettingsData();
+    PrnfbSettingsData newSettingsData = newSettings.getPrnfbSettingsData();
+    String keyStorePassword =
+        keepIfUnchanged(
+            newSettingsData.getKeyStorePassword(), oldSettingsData.getKeyStorePassword());
+
+    PrnfbSettingsData adjustedSettingsData =
+        prnfbSettingsDataBuilder(newSettingsData) //
+            .setKeyStorePassword(keyStorePassword) //
+            .build();
+
+    PrnfbSettings adjustedSettings =
+        prnfbSettingsBuilder(newSettings) //
+            .setPrnfbSettingsData(adjustedSettingsData) //
+            .build();
+
+    String data = gson.toJson(adjustedSettings);
     this.pluginSettings.put(STORAGE_KEY, data);
   }
 

diff --git a/src/main/java/se/bjurr/prnfb/settings/PrnfbNotification.java b/src/main/java/se/bjurr/prnfb/settings/PrnfbNotification.java
@@ -14,14 +14,14 @@
 import java.util.List;
 import java.util.UUID;
 
-import com.atlassian.bitbucket.pull.PullRequestState;
-import com.google.common.base.MoreObjects;
-import com.google.common.base.Optional;
-
 import se.bjurr.prnfb.http.UrlInvoker.HTTP_METHOD;
 import se.bjurr.prnfb.listener.PrnfbPullRequestAction;
 import se.bjurr.prnfb.service.PrnfbRenderer.ENCODE_FOR;
 
+import com.atlassian.bitbucket.pull.PullRequestState;
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Optional;
+
 public class PrnfbNotification implements HasUuid, Restricted {
 
   private static final String DEFAULT_NAME = "Notification";

diff --git a/src/main/java/se/bjurr/prnfb/settings/PrnfbSettings.java b/src/main/java/se/bjurr/prnfb/settings/PrnfbSettings.java
@@ -6,6 +6,7 @@
 import java.util.List;
 
 public class PrnfbSettings {
+  public static final String UNCHANGED = "KEEP_THIS_TO_LEAVE_UNCHANGED";
   private final List<PrnfbButton> buttons;
   private List<PrnfbNotification> notifications = newArrayList();
   private final PrnfbSettingsData prnfbSettingsData;
@@ -68,10 +69,10 @@ public PrnfbSettingsData getPrnfbSettingsData() {
   public int hashCode() {
     final int prime = 31;
     int result = 1;
-    result = prime * result + ((this.buttons == null) ? 0 : this.buttons.hashCode());
-    result = prime * result + ((this.notifications == null) ? 0 : this.notifications.hashCode());
+    result = prime * result + (this.buttons == null ? 0 : this.buttons.hashCode());
+    result = prime * result + (this.notifications == null ? 0 : this.notifications.hashCode());
     result =
-        prime * result + ((this.prnfbSettingsData == null) ? 0 : this.prnfbSettingsData.hashCode());
+        prime * result + (this.prnfbSettingsData == null ? 0 : this.prnfbSettingsData.hashCode());
     return result;
   }
 }
diff --git a/src/main/java/se/bjurr/prnfb/settings/PrnfbSettingsDataBuilder.java b/src/main/java/se/bjurr/prnfb/settings/PrnfbSettingsDataBuilder.java
@@ -22,6 +22,7 @@ private PrnfbSettingsDataBuilder(PrnfbSettingsData settings) {
     this.keyStore = settings.getKeyStore().orNull();
     this.keyStoreType = settings.getKeyStoreType();
     this.keyStorePassword = settings.getKeyStorePassword().orNull();
+    this.adminRestriction = settings.getAdminRestriction();
   }
 
   public PrnfbSettingsData build() {

diff --git a/src/main/java/se/bjurr/prnfb/transformer/NotificationTransformer.java b/src/main/java/se/bjurr/prnfb/transformer/NotificationTransformer.java
@@ -3,18 +3,19 @@
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static com.google.common.collect.Lists.newArrayList;
 import static se.bjurr.prnfb.settings.PrnfbNotificationBuilder.prnfbNotificationBuilder;
+import static se.bjurr.prnfb.settings.PrnfbSettings.UNCHANGED;
 
 import java.util.List;
 
-import com.atlassian.bitbucket.pull.PullRequestState;
-
 import se.bjurr.prnfb.listener.PrnfbPullRequestAction;
 import se.bjurr.prnfb.presentation.dto.HeaderDTO;
 import se.bjurr.prnfb.presentation.dto.NotificationDTO;
 import se.bjurr.prnfb.settings.PrnfbHeader;
 import se.bjurr.prnfb.settings.PrnfbNotification;
 import se.bjurr.prnfb.settings.ValidationException;
 
+import com.atlassian.bitbucket.pull.PullRequestState;
+
 public class NotificationTransformer {
 
   public static NotificationDTO toNotificationDto(PrnfbNotification from) {
@@ -28,19 +29,19 @@ public static NotificationDTO toNotificationDto(PrnfbNotification from) {
     to.setMethod(from.getMethod());
     to.setName(from.getName());
     to.setHeaders(toHeaders(from.getHeaders()));
-    to.setPassword(from.getPassword().orNull());
     to.setPostContent(from.getPostContent().orNull());
     to.setPostContentEncoding(from.getPostContentEncoding());
-    to.setProxyPassword(from.getProxyPassword().orNull());
     to.setProxyPort(from.getProxyPort());
     to.setProxyServer(from.getProxyServer().orNull());
     to.setProxySchema(from.getProxySchema().orNull());
-    to.setProxyUser(from.getProxyUser().orNull());
+    to.setProxyUser(UNCHANGED);
+    to.setProxyPassword(UNCHANGED);
     to.setTriggerIfCanMerge(from.getTriggerIfCanMerge());
     to.setTriggerIgnoreStateList(toPullRequestStateStrings(from.getTriggerIgnoreStateList()));
     to.setTriggers(toStrings(from.getTriggers()));
     to.setUrl(from.getUrl());
-    to.setUser(from.getUser().orNull());
+    to.setUser(UNCHANGED);
+    to.setPassword(UNCHANGED);
     to.setUuid(from.getUuid());
     return to;
   }

diff --git a/src/main/java/se/bjurr/prnfb/transformer/SettingsTransformer.java b/src/main/java/se/bjurr/prnfb/transformer/SettingsTransformer.java
@@ -1,5 +1,6 @@
 package se.bjurr.prnfb.transformer;
 
+import static se.bjurr.prnfb.settings.PrnfbSettings.UNCHANGED;
 import static se.bjurr.prnfb.settings.PrnfbSettingsDataBuilder.prnfbSettingsDataBuilder;
 import se.bjurr.prnfb.presentation.dto.SettingsDataDTO;
 import se.bjurr.prnfb.settings.PrnfbSettingsData;
@@ -10,7 +11,7 @@ public static SettingsDataDTO toDto(PrnfbSettingsData settingsData) {
     SettingsDataDTO dto = new SettingsDataDTO();
     dto.setAdminRestriction(settingsData.getAdminRestriction());
     dto.setKeyStore(settingsData.getKeyStore().orNull());
-    dto.setKeyStorePassword(settingsData.getKeyStorePassword().orNull());
+    dto.setKeyStorePassword(UNCHANGED);
     dto.setKeyStoreType(settingsData.getKeyStoreType());
     dto.setShouldAcceptAnyCertificate(settingsData.isShouldAcceptAnyCertificate());
     return dto;

diff --git a/src/test/java/se/bjurr/prnfb/presentation/NotificationServletTest.java b/src/test/java/se/bjurr/prnfb/presentation/NotificationServletTest.java
@@ -8,6 +8,7 @@
 import static org.mockito.Mockito.when;
 import static org.mockito.MockitoAnnotations.initMocks;
 import static se.bjurr.prnfb.listener.PrnfbPullRequestAction.MERGED;
+import static se.bjurr.prnfb.settings.PrnfbSettings.UNCHANGED;
 import static se.bjurr.prnfb.test.Podam.populatedInstanceOf;
 import static se.bjurr.prnfb.transformer.NotificationTransformer.toPrnfbNotification;
 
@@ -20,13 +21,13 @@
 import org.mockito.Mock;
 import org.mockito.Mockito;
 
-import com.google.common.collect.Lists;
-
 import se.bjurr.prnfb.presentation.dto.NotificationDTO;
 import se.bjurr.prnfb.service.SettingsService;
 import se.bjurr.prnfb.service.UserCheckService;
 import se.bjurr.prnfb.settings.PrnfbNotification;
 
+import com.google.common.collect.Lists;
+
 public class NotificationServletTest {
   private PrnfbNotification notification1;
   private PrnfbNotification notification2;
@@ -93,11 +94,19 @@ public void testNotificationCanBeRead() throws Exception {
         .thenReturn(storedSettings);
 
     List<NotificationDTO> actual = (List<NotificationDTO>) this.sut.get().getEntity();
-
+    setUnchanged(notificationDto1);
+    setUnchanged(notificationDto2);
     assertThat(actual) //
         .containsOnly(this.notificationDto1, this.notificationDto2);
   }
 
+  private void setUnchanged(NotificationDTO dto) {
+    dto.setUser(UNCHANGED);
+    dto.setPassword(UNCHANGED);
+    dto.setProxyUser(UNCHANGED);
+    dto.setProxyPassword(UNCHANGED);
+  }
+
   @Test
   public void testThatNotificationCanBeListedPerProject() throws Exception {
     List<PrnfbNotification> notifications = newArrayList(this.notification1);
@@ -110,6 +119,7 @@ public void testThatNotificationCanBeListedPerProject() throws Exception {
     @SuppressWarnings("unchecked")
     Iterable<NotificationDTO> actualList = (Iterable<NotificationDTO>) actual.getEntity();
 
+    setUnchanged(notificationDto1);
     assertThat(actualList) //
         .containsOnly(this.notificationDto1);
   }
@@ -131,6 +141,8 @@ public void testThatNotificationCanBeListedPerProjectAndRepo() throws Exception
     @SuppressWarnings("unchecked")
     Iterable<NotificationDTO> actualList = (Iterable<NotificationDTO>) actual.getEntity();
 
+    setUnchanged(notificationDto1);
+
     assertThat(actualList) //
         .containsOnly(this.notificationDto1);
   }