Improve SystemPasscode

* remove unused method isConfigured()

* testing passcode before user admin permission is faster when using
the recommended passcode

diff --git a/server/sonar-server/src/main/java/org/sonar/server/ce/ws/InfoAction.java b/server/sonar-server/src/main/java/org/sonar/server/ce/ws/InfoAction.java
index cc2921b22efd9707468e1388df098ae11298dbb6..32f2df3d6a71fcb4bd2a69fcc533428e4db9e900 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/ce/ws/InfoAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/ce/ws/InfoAction.java
@@ -55,7 +55,7 @@ public class InfoAction implements CeWsAction {
 
   @Override
   public void handle(Request request, Response response) throws Exception {
-    if (!userSession.isSystemAdministrator() && !systemPasscode.isValid(request)) {
+    if (!systemPasscode.isValid(request) && !userSession.isSystemAdministrator()) {
       throw AbstractUserSession.insufficientPrivilegesException();
     }
 

diff --git a/server/sonar-server/src/main/java/org/sonar/server/ce/ws/PauseAction.java b/server/sonar-server/src/main/java/org/sonar/server/ce/ws/PauseAction.java
index 769fcbafc0ac387dadbecde873b78ddc8f3aa435..51137750b999411728586250e3e0c484434905db 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/ce/ws/PauseAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/ce/ws/PauseAction.java
@@ -53,7 +53,7 @@ public class PauseAction implements CeWsAction {
 
   @Override
   public void handle(Request request, Response response) throws Exception {
-    if (!userSession.isSystemAdministrator() && !systemPasscode.isValid(request)) {
+    if (!systemPasscode.isValid(request) && !userSession.isSystemAdministrator()) {
       throw AbstractUserSession.insufficientPrivilegesException();
     }
 

diff --git a/server/sonar-server/src/main/java/org/sonar/server/ce/ws/ResumeAction.java b/server/sonar-server/src/main/java/org/sonar/server/ce/ws/ResumeAction.java
index 53527f8f129f662bdf194c057922ea75731fcfba..a4a0203dfa15f5ceef275a9522c4e31e5449b7fb 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/ce/ws/ResumeAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/ce/ws/ResumeAction.java
@@ -53,7 +53,7 @@ public class ResumeAction implements CeWsAction {
 
   @Override
   public void handle(Request request, Response response) throws Exception {
-    if (!userSession.isSystemAdministrator() && !systemPasscode.isValid(request)) {
+    if (!systemPasscode.isValid(request) && !userSession.isSystemAdministrator()) {
       throw AbstractUserSession.insufficientPrivilegesException();
     }
 

diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/ws/HealthAction.java b/server/sonar-server/src/main/java/org/sonar/server/platform/ws/HealthAction.java
index 23b9eefb9d8cf48120480e018be051bd1dc8d89e..faf47b5cac810e75708d465a2f734222ace11243 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/platform/ws/HealthAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/platform/ws/HealthAction.java
@@ -48,7 +48,7 @@ public class HealthAction implements SystemWsAction {
 
   @Override
   public void handle(Request request, Response response) throws Exception {
-    if (!isPassCodeAuthenticated(request) && !isSystemAdmin()) {
+    if (!systemPasscode.isValid(request) && !isSystemAdmin()) {
       throw new ForbiddenException("Insufficient privileges");
     }
 
@@ -63,8 +63,4 @@ public class HealthAction implements SystemWsAction {
     return userSession.isSystemAdministrator();
   }
 
-  private boolean isPassCodeAuthenticated(Request request) {
-    return systemPasscode.isConfigured() && systemPasscode.isValid(request);
-  }
-
 }

diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/ws/SafeModeHealthAction.java b/server/sonar-server/src/main/java/org/sonar/server/platform/ws/SafeModeHealthAction.java
index 70c32111503d503338e4e0bcff07756ecc93f9b5..64b746a054eaeac6a381a7c2e529df2d7ed2d0ca 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/platform/ws/SafeModeHealthAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/platform/ws/SafeModeHealthAction.java
@@ -42,7 +42,7 @@ public class SafeModeHealthAction implements SystemWsAction {
 
   @Override
   public void handle(Request request, Response response) throws Exception {
-    if (!systemPasscode.isConfigured() || !systemPasscode.isValid(request)) {
+    if (!systemPasscode.isValid(request)) {
       throw new ForbiddenException("Insufficient privileges");
     }
 

diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/SystemPasscode.java b/server/sonar-server/src/main/java/org/sonar/server/user/SystemPasscode.java
index e178c792fb6c350adc28358ae5fd4fe4bdcab064..1cf628d61205c621db0d0a2804594dcedcfdaa2a 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/user/SystemPasscode.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/user/SystemPasscode.java
@@ -25,18 +25,15 @@ import org.sonar.api.server.ws.Request;
  * Passcode for accessing some web services, usually for connecting
  * monitoring tools without using the credentials
  * of a system administrator.
+ *
+ * Important - the web services accepting passcode must be listed in
+ * {@link org.sonar.server.authentication.UserSessionInitializer#URL_USING_PASSCODE}.
  */
 public interface SystemPasscode {
 
   /**
-   * Whether the system passcode is configured in sonar.properties or not.
-   * By default passcode is not defined and {@code false} is returned.
-   */
-  boolean isConfigured();
-
-  /**
-   * Whether the configured system passcode is provided by the HTTP request or not.
-   * Returns {@code false} if {@link #isConfigured()} is {@code false}.
+   * Whether the system passcode is provided by the HTTP request or not.
+   * Returns {@code false} if passcode is not configured.
    */
   boolean isValid(Request request);
 

diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/SystemPasscodeImpl.java b/server/sonar-server/src/main/java/org/sonar/server/user/SystemPasscodeImpl.java
index 806f4bf0119d9ad3e6a8e247390ccea31b952781..d29d898317c51cf9b260d80c0709a1f3caf0e365 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/user/SystemPasscodeImpl.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/user/SystemPasscodeImpl.java
@@ -40,11 +40,6 @@ public class SystemPasscodeImpl implements SystemPasscode, Startable {
     this.configuration = configuration;
   }
 
-  @Override
-  public boolean isConfigured() {
-    return configuredPasscode != null;
-  }
-
   @Override
   public boolean isValid(Request request) {
     if (configuredPasscode == null) {

diff --git a/server/sonar-server/src/test/java/org/sonar/server/platform/ws/HealthActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/platform/ws/HealthActionTest.java
index 0915ae91d3e9ceae6aecf35ebc17277978eca40b..8c9033dce82a199e03918706267063b18d643068 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/platform/ws/HealthActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/platform/ws/HealthActionTest.java
@@ -32,7 +32,6 @@ import org.apache.commons.lang.RandomStringUtils;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
-import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.process.cluster.health.NodeDetails;
 import org.sonar.process.cluster.health.NodeHealth;
@@ -100,7 +99,7 @@ public class HealthActionTest {
 
   @Test
   public void request_fails_with_SystemPasscode_enabled_and_anonymous() {
-    when(systemPasscode.isConfigured()).thenReturn(true);
+    when(systemPasscode.isValid(any())).thenReturn(false);
     TestRequest request = underTest.newRequest();
 
     expectForbiddenException();
@@ -110,8 +109,7 @@ public class HealthActionTest {
 
   @Test
   public void request_fails_with_SystemPasscode_enabled_but_no_passcode_and_user_is_not_system_administrator() {
-    when(systemPasscode.isConfigured()).thenReturn(true);
-    when(systemPasscode.isValid(any(Request.class))).thenReturn(false);
+    when(systemPasscode.isValid(any())).thenReturn(false);
     userSessionRule.logIn();
     when(healthChecker.checkCluster()).thenReturn(randomStatusMinimalClusterHealth());
     TestRequest request = underTest.newRequest();
@@ -123,8 +121,7 @@ public class HealthActionTest {
 
   @Test
   public void request_succeeds_with_SystemPasscode_enabled_and_passcode() {
-    when(systemPasscode.isConfigured()).thenReturn(true);
-    when(systemPasscode.isValid(any(Request.class))).thenReturn(true);
+    when(systemPasscode.isValid(any())).thenReturn(true);
     when(healthChecker.checkCluster()).thenReturn(randomStatusMinimalClusterHealth());
     TestRequest request = underTest.newRequest();
 
@@ -132,30 +129,8 @@ public class HealthActionTest {
   }
 
   @Test
-  public void request_succeeds_with_SystemPasscode_disabled_and_user_is_system_administrator() {
-    when(systemPasscode.isConfigured()).thenReturn(false);
-    userSessionRule.logIn().setSystemAdministrator();
-    when(healthChecker.checkCluster()).thenReturn(randomStatusMinimalClusterHealth());
-    TestRequest request = underTest.newRequest();
-
-    request.execute();
-  }
-
-  @Test
-  public void request_succeeds_with_SystemPasscode_enabled_but_no_passcode_and_user_is_system_administrator() {
-    when(systemPasscode.isConfigured()).thenReturn(true);
-    when(systemPasscode.isValid(any(Request.class))).thenReturn(false);
-    userSessionRule.logIn().setSystemAdministrator();
-    when(healthChecker.checkCluster()).thenReturn(randomStatusMinimalClusterHealth());
-    TestRequest request = underTest.newRequest();
-
-    request.execute();
-  }
-
-  @Test
-  public void request_succeeds_with_SystemPasscode_enabled_and_passcode_and_user_is_system_administrator() {
-    when(systemPasscode.isConfigured()).thenReturn(true);
-    when(systemPasscode.isValid(any(Request.class))).thenReturn(true);
+  public void request_succeeds_with_SystemPasscode_incorrect_and_user_is_system_administrator() {
+    when(systemPasscode.isValid(any())).thenReturn(false);
     userSessionRule.logIn().setSystemAdministrator();
     when(healthChecker.checkCluster()).thenReturn(randomStatusMinimalClusterHealth());
     TestRequest request = underTest.newRequest();
@@ -381,11 +356,10 @@ public class HealthActionTest {
    */
   private void authenticateWithRandomMethod() {
     if (random.nextBoolean()) {
-      when(systemPasscode.isConfigured()).thenReturn(true);
       if (random.nextBoolean()) {
-        when(systemPasscode.isValid(any(Request.class))).thenReturn(true);
+        when(systemPasscode.isValid(any())).thenReturn(true);
       } else {
-        when(systemPasscode.isValid(any(Request.class))).thenReturn(false);
+        when(systemPasscode.isValid(any())).thenReturn(false);
         userSessionRule.logIn().setSystemAdministrator();
       }
     } else {

diff --git a/server/sonar-server/src/test/java/org/sonar/server/platform/ws/SafeModeHealthActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/platform/ws/SafeModeHealthActionTest.java
index 5f24446eddad145a2cde0a5cc55cdfd141043572..d94ddf489b2ba05c81b79b7ef346f3f961428af0 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/platform/ws/SafeModeHealthActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/platform/ws/SafeModeHealthActionTest.java
@@ -26,7 +26,6 @@ import org.apache.commons.lang.RandomStringUtils;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
-import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.health.Health;
@@ -68,9 +67,8 @@ public class SafeModeHealthActionTest {
   }
 
   @Test
-  public void request_fails_with_ForbiddenException_when_PassCode_disabled() {
-    when(systemPasscode.isConfigured()).thenReturn(false);
-    when(systemPasscode.isValid(any(Request.class))).thenReturn(random.nextBoolean());
+  public void request_fails_with_ForbiddenException_when_PassCode_disabled_or_incorrect() {
+    when(systemPasscode.isValid(any())).thenReturn(false);
     TestRequest request = underTest.newRequest();
 
     expectForbiddenException();
@@ -79,18 +77,7 @@ public class SafeModeHealthActionTest {
   }
 
   @Test
-  public void request_fails_with_ForbiddenException_when_PassCode_enabled_but_no_passcode() {
-    when(systemPasscode.isConfigured()).thenReturn(true);
-    when(systemPasscode.isValid(any(Request.class))).thenReturn(false);
-    TestRequest request = underTest.newRequest();
-
-    expectForbiddenException();
-
-    request.execute();
-  }
-
-  @Test
-  public void request_succeeds_when_PassCode_enabled_and_valid_passcode() {
+  public void request_succeeds_when_valid_passcode() {
     authenticateWithPasscode();
     when(healthChecker.checkNode())
       .thenReturn(newHealthCheckBuilder()
@@ -156,8 +143,7 @@ public class SafeModeHealthActionTest {
   }
 
   private void authenticateWithPasscode() {
-    when(systemPasscode.isConfigured()).thenReturn(true);
-    when(systemPasscode.isValid(any(Request.class))).thenReturn(true);
+    when(systemPasscode.isValid(any())).thenReturn(true);
   }
 
 }

diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/SystemPasscodeImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/SystemPasscodeImplTest.java
index 528596392e458f1a84adfede88282145a5cfc29e..954949ab78604ad7ed8d392f3a8e97ee2586c182 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/user/SystemPasscodeImplTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/user/SystemPasscodeImplTest.java
@@ -42,26 +42,6 @@ public class SystemPasscodeImplTest {
     underTest.stop();
   }
 
-  @Test
-  public void isConfigured_is_true_if_property_is_not_blank() {
-    verifyIsConfigured("foo", true);
-  }
-
-  @Test
-  public void isConfigured_is_false_if_property_value_is_blank() {
-    verifyIsConfigured("  ", false);
-  }
-
-  @Test
-  public void isConfigured_is_false_if_property_value_is_empty() {
-    verifyIsConfigured("", false);
-  }
-
-  @Test
-  public void isConfigured_is_false_if_property_is_not_defined() {
-    assertThat(underTest.isConfigured()).isFalse();
-  }
-
   @Test
   public void startup_logs_show_that_feature_is_enabled() {
     configurePasscode("foo");
@@ -77,6 +57,14 @@ public class SystemPasscodeImplTest {
     assertThat(logTester.logs(LoggerLevel.INFO)).contains("System authentication by passcode is disabled");
   }
 
+  @Test
+  public void passcode_is_disabled_if_blank_configuration() {
+    configurePasscode("");
+    underTest.start();
+
+    assertThat(logTester.logs(LoggerLevel.INFO)).contains("System authentication by passcode is disabled");
+  }
+
   @Test
   public void isValid_is_true_if_request_header_matches_configured_passcode() {
     verifyIsValid(true, "foo", "foo");
@@ -111,11 +99,6 @@ public class SystemPasscodeImplTest {
     assertThat(underTest.isValid(request)).isEqualTo(expectedResult);
   }
 
-  private void verifyIsConfigured(String propertyValue, boolean expectedResult) {
-    configurePasscode(propertyValue);
-    assertThat(underTest.isConfigured()).isEqualTo(expectedResult);
-  }
-
   private void configurePasscode(String propertyValue) {
     settings.setProperty("sonar.web.systemPasscode", propertyValue);
     underTest.start();