SONAR-15978 api/system/status Remove Server ID and Version for unauthenticated users

5 files changed, 48 insertions, 13 deletions

diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/UserSessionInitializer.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/UserSessionInitializer.java
index 9007d0908a0..216dc23af32 100644
--- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/UserSessionInitializer.java
+++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/UserSessionInitializer.java
@@ -53,7 +53,7 @@ public class UserSessionInitializer {
     "/batch/index", "/batch/file",
     "/maintenance/*", "/setup/*",
     "/sessions/*", "/oauth2/callback/*",
-    "/api/system/db_migration_status", "/api/system/status", "/api/system/migrate_db",
+    "/api/system/db_migration_status", "/api/system/migrate_db",
     "/api/users/identity_providers", "/api/l10n/index",
     "/api/authentication/login", "/api/authentication/logout", "/api/authentication/validate",
     "/api/project_badges/measure", "/api/project_badges/quality_gate");
@@ -65,6 +65,9 @@ public class UserSessionInitializer {
     "/api/system/liveness",
     "/api/monitoring/metrics");
 
+  private static final Set<String> URL_OPTIONAL_AUTHENTICATION = Set.of(
+    "/api/system/status");
+
   private static final UrlPattern URL_PATTERN = UrlPattern.builder()
     .includes("/*")
     .excludes(staticResourcePatterns())
@@ -75,6 +78,10 @@ public class UserSessionInitializer {
     .includes(URL_USING_PASSCODE)
     .build();
 
+  private static final UrlPattern OPTIONAL_AUTH_URLS = UrlPattern.builder()
+    .includes(URL_OPTIONAL_AUTHENTICATION)
+    .build();
+
   private final Configuration config;
   private final ThreadLocalUserSession threadLocalSession;
   private final AuthenticationEvent authenticationEvent;
@@ -93,7 +100,7 @@ public class UserSessionInitializer {
     try {
       // Do not set user session when url is excluded
       if (URL_PATTERN.matches(path)) {
-        loadUserSession(request, response, PASSCODE_URLS.matches(path));
+        loadUserSession(request, response, PASSCODE_URLS.matches(path) || OPTIONAL_AUTH_URLS.matches(path));
       }
       return true;
     } catch (AuthenticationException e) {
@@ -117,9 +124,9 @@ public class UserSessionInitializer {
     return provider != AuthenticationEvent.Provider.LOCAL && provider != AuthenticationEvent.Provider.JWT;
   }
 
-  private void loadUserSession(HttpServletRequest request, HttpServletResponse response, boolean urlSupportsSystemPasscode) {
+  private void loadUserSession(HttpServletRequest request, HttpServletResponse response, boolean urlSupportsOptionalAuthentication) {
     UserSession session = requestAuthenticator.authenticate(request, response);
-    if (!session.isLoggedIn() && !urlSupportsSystemPasscode && config.getBoolean(CORE_FORCE_AUTHENTICATION_PROPERTY).orElse(CORE_FORCE_AUTHENTICATION_DEFAULT_VALUE)) {
+    if (!session.isLoggedIn() && !urlSupportsOptionalAuthentication && config.getBoolean(CORE_FORCE_AUTHENTICATION_PROPERTY).orElse(CORE_FORCE_AUTHENTICATION_DEFAULT_VALUE)) {
       // authentication is required
       throw AuthenticationException.newBuilder()
         .setSource(Source.local(AuthenticationEvent.Method.BASIC))
diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/UserSessionInitializerTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/UserSessionInitializerTest.java
index dcaac1ea2ef..08c4dbbdbff 100644
--- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/UserSessionInitializerTest.java
+++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/UserSessionInitializerTest.java
@@ -88,9 +88,7 @@ public class UserSessionInitializerTest {
     assertPathIsIgnored("/oauth2/callback/github");
     assertPathIsIgnored("/oauth2/callback/foo");
     assertPathIsIgnored("/api/system/db_migration_status");
-    assertPathIsIgnored("/api/system/status");
     assertPathIsIgnored("/api/system/migrate_db");
-    assertPathIsIgnored("/api/server/version");
     assertPathIsIgnored("/api/users/identity_providers");
     assertPathIsIgnored("/api/l10n/index");
 
@@ -98,7 +96,7 @@ public class UserSessionInitializerTest {
     assertPathIsIgnored("/api/project_badges/measure");
     assertPathIsIgnored("/api/project_badges/quality_gate");
 
-    // exlude passcode urls
+    // exlude urls that support passcode
     assertPathIsIgnoredWithAnonymousAccess("/api/ce/info");
     assertPathIsIgnoredWithAnonymousAccess("/api/ce/pause");
     assertPathIsIgnoredWithAnonymousAccess("/api/ce/resume");
@@ -106,6 +104,10 @@ public class UserSessionInitializerTest {
     assertPathIsIgnoredWithAnonymousAccess("/api/system/liveness");
     assertPathIsIgnoredWithAnonymousAccess("/api/monitoring/metrics");
 
+    //check that /api/system/status authentication is optional
+    assertPathIsIgnoredWithAnonymousAccess("/api/system/status");
+    assertPathIsNotIgnored("/api/system/status");
+
     // exclude static resources
     assertPathIsIgnored("/css/style.css");
     assertPathIsIgnored("/images/logo.png");
diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/platform/ws/StatusAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/platform/ws/StatusAction.java
index ad3adb5f3fe..cc5dc31a0d9 100644
--- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/platform/ws/StatusAction.java
+++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/platform/ws/StatusAction.java
@@ -21,12 +21,14 @@ package org.sonar.server.platform.ws;
 
 import com.google.common.io.Resources;
 import org.sonar.api.platform.Server;
+import org.sonar.api.server.ws.Change;
 import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.Response;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.server.app.RestartFlagHolder;
 import org.sonar.server.platform.Platform;
 import org.sonar.server.platform.db.migration.DatabaseMigrationState;
+import org.sonar.server.user.UserSession;
 import org.sonar.server.ws.WsUtils;
 import org.sonarqube.ws.System;
 
@@ -41,13 +43,15 @@ public class StatusAction implements SystemWsAction {
   private final DatabaseMigrationState migrationState;
   private final Platform platform;
   private final RestartFlagHolder restartFlagHolder;
+  private final UserSession userSession;
 
   public StatusAction(Server server, DatabaseMigrationState migrationState,
-                      Platform platform, RestartFlagHolder restartFlagHolder) {
+    Platform platform, RestartFlagHolder restartFlagHolder, UserSession userSession) {
     this.server = server;
     this.migrationState = migrationState;
     this.platform = platform;
     this.restartFlagHolder = restartFlagHolder;
+    this.userSession = userSession;
   }
 
   @Override
@@ -69,14 +73,19 @@ public class StatusAction implements SystemWsAction {
         "</p>")
       .setSince("5.2")
       .setResponseExample(Resources.getResource(this.getClass(), "example-status.json"))
+      .setChangelog(new Change("9.4", "returns server id and server version only when authenticated"))
       .setHandler(this);
   }
 
   @Override
   public void handle(Request request, Response response) throws Exception {
     System.StatusResponse.Builder protobuf = System.StatusResponse.newBuilder();
-    ofNullable(server.getId()).ifPresent(protobuf::setId);
-    ofNullable(server.getVersion()).ifPresent(protobuf::setVersion);
+
+    if(userSession.isLoggedIn()) {
+      ofNullable(server.getId()).ifPresent(protobuf::setId);
+      ofNullable(server.getVersion()).ifPresent(protobuf::setVersion);
+    }
+
     protobuf.setStatus(computeStatus());
     WsUtils.writeProtobuf(protobuf.build(), request, response);
   }
diff --git a/server/sonar-webserver-webapi/src/main/resources/org/sonar/server/platform/ws/example-status-unauthenticated.json b/server/sonar-webserver-webapi/src/main/resources/org/sonar/server/platform/ws/example-status-unauthenticated.json
new file mode 100644
index 00000000000..0c5646a1072
--- /dev/null
+++ b/server/sonar-webserver-webapi/src/main/resources/org/sonar/server/platform/ws/example-status-unauthenticated.json
@@ -0,0 +1,3 @@
+{
+  "status": "UP"
+}
diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/platform/ws/StatusActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/platform/ws/StatusActionTest.java
index 5006c5e4bba..d6dedf4cc15 100644
--- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/platform/ws/StatusActionTest.java
+++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/platform/ws/StatusActionTest.java
@@ -21,6 +21,7 @@ package org.sonar.server.platform.ws;
 
 import java.util.Date;
 import java.util.Set;
+import org.junit.Rule;
 import org.junit.Test;
 import org.sonar.api.platform.Server;
 import org.sonar.api.server.ws.WebService;
@@ -28,6 +29,7 @@ import org.sonar.server.app.RestartFlagHolder;
 import org.sonar.server.app.RestartFlagHolderImpl;
 import org.sonar.server.platform.Platform;
 import org.sonar.server.platform.db.migration.DatabaseMigrationState;
+import org.sonar.server.tester.UserSessionRule;
 import org.sonar.server.ws.WsActionTester;
 
 import static com.google.common.base.Predicates.in;
@@ -53,12 +55,15 @@ public class StatusActionTest {
     DatabaseMigrationState.Status.SUCCEEDED, DatabaseMigrationState.Status.RUNNING);
   private static final Set<Platform.Status> SUPPORTED_PLATFORM_STATUSES = of(Platform.Status.BOOTING, Platform.Status.SAFEMODE, Platform.Status.STARTING, Platform.Status.UP);
 
+  @Rule
+  public UserSessionRule userSessionRule = UserSessionRule.standalone();
+
   private static Server server = new Dummy51Server();
   private DatabaseMigrationState migrationState = mock(DatabaseMigrationState.class);
   private Platform platform = mock(Platform.class);
   private RestartFlagHolder restartFlagHolder = new RestartFlagHolderImpl();
 
-  private WsActionTester underTest = new WsActionTester(new StatusAction(server, migrationState, platform, restartFlagHolder));
+  private WsActionTester underTest = new WsActionTester(new StatusAction(server, migrationState, platform, restartFlagHolder, userSessionRule));
 
   @Test
   public void action_status_is_defined() {
@@ -66,12 +71,21 @@ public class StatusActionTest {
     assertThat(action.isPost()).isFalse();
     assertThat(action.description()).isNotEmpty();
     assertThat(action.responseExample()).isNotNull();
-
+    assertThat(action.changelog()).isNotEmpty();
     assertThat(action.params()).isEmpty();
   }
 
   @Test
-  public void verify_example() {
+  public void verify_example_unauthenticated() {
+    when(platform.status()).thenReturn(Platform.Status.UP);
+    restartFlagHolder.unset();
+
+    assertJson(underTest.newRequest().execute().getInput()).isSimilarTo(getClass().getResource("example-status-unauthenticated.json"));
+  }
+
+  @Test
+  public void verify_example_logged_in() {
+    userSessionRule.logIn();
     when(platform.status()).thenReturn(Platform.Status.UP);
     restartFlagHolder.unset();