SONAR-8716 fix check of permissions in api/user_tokens

diff --git a/server/sonar-server/src/main/java/org/sonar/server/usertoken/ws/GenerateAction.java b/server/sonar-server/src/main/java/org/sonar/server/usertoken/ws/GenerateAction.java
index 3e9dad172f6a248dc03c0c63b83576cd83402970..0436d822ebcdab83446f9c3ebe3841336c724c09 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/usertoken/ws/GenerateAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/usertoken/ws/GenerateAction.java
@@ -85,8 +85,7 @@ public class GenerateAction implements UserTokensWsAction {
   }
 
   private WsUserTokens.GenerateWsResponse doHandle(GenerateWsRequest request) {
-    DbSession dbSession = dbClient.openSession(false);
-    try {
+    try (DbSession dbSession = dbClient.openSession(false)) {
       checkWsRequest(dbSession, request);
       TokenPermissionsValidator.validate(userSession, request.getLogin());
 
@@ -96,8 +95,6 @@ public class GenerateAction implements UserTokensWsAction {
       UserTokenDto userTokenDto = insertTokenInDb(dbSession, request, tokenHash);
 
       return buildResponse(userTokenDto, token);
-    } finally {
-      dbClient.closeSession(dbSession);
     }
   }
 

diff --git a/server/sonar-server/src/main/java/org/sonar/server/usertoken/ws/RevokeAction.java b/server/sonar-server/src/main/java/org/sonar/server/usertoken/ws/RevokeAction.java
index 2fdfefc23edad982379ec584734906e028b9519e..81c4281bbaf2fc0006711be0e6349a079e69cb78 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/usertoken/ws/RevokeAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/usertoken/ws/RevokeAction.java
@@ -68,12 +68,9 @@ public class RevokeAction implements UserTokensWsAction {
   private void doHandle(RevokeWsRequest request) {
     TokenPermissionsValidator.validate(userSession, request.getLogin());
 
-    DbSession dbSession = dbClient.openSession(false);
-    try {
+    try (DbSession dbSession = dbClient.openSession(false)) {
       dbClient.userTokenDao().deleteByLoginAndName(dbSession, request.getLogin(), request.getName());
       dbSession.commit();
-    } finally {
-      dbClient.closeSession(dbSession);
     }
   }
 

diff --git a/server/sonar-server/src/main/java/org/sonar/server/usertoken/ws/SearchAction.java b/server/sonar-server/src/main/java/org/sonar/server/usertoken/ws/SearchAction.java
index c44f5cc9604a0d081b7866e69114259b63e7b5d7..9e4d233340971050481723aa426d3c1335ea516a 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/usertoken/ws/SearchAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/usertoken/ws/SearchAction.java
@@ -70,14 +70,11 @@ public class SearchAction implements UserTokensWsAction {
   private SearchWsResponse doHandle(SearchWsRequest request) {
     TokenPermissionsValidator.validate(userSession, request.getLogin());
 
-    DbSession dbSession = dbClient.openSession(false);
-    try {
+    try (DbSession dbSession = dbClient.openSession(false)) {
       String login = request.getLogin();
       checkLoginExists(dbSession, login);
       List<UserTokenDto> userTokens = dbClient.userTokenDao().selectByLogin(dbSession, login);
       return buildResponse(login, userTokens);
-    } finally {
-      dbClient.closeSession(dbSession);
     }
   }
 

diff --git a/server/sonar-server/src/main/java/org/sonar/server/usertoken/ws/TokenPermissionsValidator.java b/server/sonar-server/src/main/java/org/sonar/server/usertoken/ws/TokenPermissionsValidator.java
index ff28731c2c708810c79bc5a89e707bf84d7c53b4..1fe2b62bbcb0ad8d57b2e700d9ad5611529e646b 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/usertoken/ws/TokenPermissionsValidator.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/usertoken/ws/TokenPermissionsValidator.java
@@ -20,7 +20,6 @@
 package org.sonar.server.usertoken.ws;
 
 import javax.annotation.Nullable;
-import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.server.user.UserSession;
 
 import static org.sonar.server.user.AbstractUserSession.insufficientPrivilegesException;
@@ -31,11 +30,13 @@ class TokenPermissionsValidator {
   }
 
   static void validate(UserSession userSession, @Nullable String requestLogin) {
-    if (userSession.hasPermission(GlobalPermissions.SYSTEM_ADMIN)
-      || (requestLogin != null && requestLogin.equals(userSession.getLogin()))) {
-      return;
+    userSession.checkLoggedIn();
+    if (!userSession.isRoot() && !isLoggedInUser(userSession, requestLogin)) {
+      throw insufficientPrivilegesException();
     }
+  }
 
-    throw insufficientPrivilegesException();
+  private static boolean isLoggedInUser(UserSession userSession, @Nullable String requestLogin) {
+    return requestLogin != null && requestLogin.equals(userSession.getLogin());
   }
 }

diff --git a/server/sonar-server/src/test/java/org/sonar/server/usertoken/ws/GenerateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/usertoken/ws/GenerateActionTest.java
index 1949a38fc64fdf4bb321a8fef1f279eb355d6245..985d6121aeb512cc947076006155b86377640228 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/usertoken/ws/GenerateActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/usertoken/ws/GenerateActionTest.java
@@ -27,11 +27,11 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.sonar.api.utils.System2;
-import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.db.DbTester;
 import org.sonar.server.exceptions.BadRequestException;
 import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.exceptions.ServerException;
+import org.sonar.server.exceptions.UnauthorizedException;
 import org.sonar.server.tester.UserSessionRule;
 import org.sonar.server.usertoken.TokenGenerator;
 import org.sonar.server.ws.TestRequest;
@@ -70,9 +70,6 @@ public class GenerateActionTest {
   public void setUp() {
     when(tokenGenerator.generate()).thenReturn("123456789");
     when(tokenGenerator.hash(anyString())).thenReturn("987654321");
-    userSession
-      .logIn()
-      .setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN);
     db.users().insertUser(newUserDto().setLogin(GRACE_HOPPER));
     db.users().insertUser(newUserDto().setLogin(ADA_LOVELACE));
 
@@ -82,6 +79,8 @@ public class GenerateActionTest {
 
   @Test
   public void json_example() {
+    userSession.logIn().setRoot();
+
     String response = ws.newRequest()
       .setMediaType(MediaTypes.JSON)
       .setParam(PARAM_LOGIN, GRACE_HOPPER)
@@ -93,7 +92,7 @@ public class GenerateActionTest {
 
   @Test
   public void a_user_can_generate_token_for_himself() {
-    userSession.logIn(GRACE_HOPPER).setGlobalPermissions(GlobalPermissions.SCAN_EXECUTION);
+    userSession.logIn(GRACE_HOPPER);
 
     GenerateWsResponse response = newRequest(null, TOKEN_NAME);
 
@@ -102,6 +101,8 @@ public class GenerateActionTest {
 
   @Test
   public void fail_if_name_is_longer_than_100_characters() {
+    userSession.logIn().setRoot();
+
     expectedException.expect(IllegalArgumentException.class);
     expectedException.expectMessage("Token name length (101) is longer than the maximum authorized (100)");
 
@@ -110,6 +111,8 @@ public class GenerateActionTest {
 
   @Test
   public void fail_if_login_does_not_exist() {
+    userSession.logIn().setRoot();
+
     expectedException.expect(ForbiddenException.class);
 
     newRequest("unknown-login", "any-name");
@@ -117,6 +120,8 @@ public class GenerateActionTest {
 
   @Test
   public void fail_if_name_is_blank() {
+    userSession.logIn().setRoot();
+
     expectedException.expect(BadRequestException.class);
     expectedException.expectMessage("The 'name' parameter must not be blank");
 
@@ -125,6 +130,8 @@ public class GenerateActionTest {
 
   @Test
   public void fail_if_token_with_same_login_and_name_exists() {
+    userSession.logIn().setRoot();
+
     newRequest(GRACE_HOPPER, TOKEN_NAME);
     expectedException.expect(BadRequestException.class);
     expectedException.expectMessage("A user token with login 'grace.hopper' and name 'Third Party Application' already exists");
@@ -134,6 +141,8 @@ public class GenerateActionTest {
 
   @Test
   public void fail_if_token_hash_already_exists_in_db() {
+    userSession.logIn().setRoot();
+
     when(tokenGenerator.hash(anyString())).thenReturn("987654321");
     db.getDbClient().userTokenDao().insert(db.getSession(), newUserToken().setTokenHash("987654321"));
     db.commit();
@@ -144,13 +153,23 @@ public class GenerateActionTest {
   }
 
   @Test
-  public void fail_if_insufficient_privileges() {
-    userSession.logIn(ADA_LOVELACE).setGlobalPermissions(GlobalPermissions.SCAN_EXECUTION);
+  public void throw_ForbiddenException_if_non_administrator_creates_token_for_someone_else() {
+    userSession.logIn().setNonRoot();
+
     expectedException.expect(ForbiddenException.class);
 
     newRequest(GRACE_HOPPER, TOKEN_NAME);
   }
 
+  @Test
+  public void throw_UnauthorizedException_if_not_logged_in() {
+    userSession.anonymous();
+
+    expectedException.expect(UnauthorizedException.class);
+
+    newRequest(GRACE_HOPPER, TOKEN_NAME);
+  }
+
   private GenerateWsResponse newRequest(@Nullable String login, String name) {
     TestRequest testRequest = ws.newRequest()
       .setMediaType(MediaTypes.PROTOBUF)

diff --git a/server/sonar-server/src/test/java/org/sonar/server/usertoken/ws/RevokeActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/usertoken/ws/RevokeActionTest.java
index 40cf1aa3c341e585243ccf0d60e90eab798eeb6f..32b8e258259ce309f797ed59caa541fedd6f1b71 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/usertoken/ws/RevokeActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/usertoken/ws/RevokeActionTest.java
@@ -25,12 +25,12 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.sonar.api.utils.System2;
-import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
 import org.sonar.db.user.UserTokenDto;
 import org.sonar.server.exceptions.ForbiddenException;
+import org.sonar.server.exceptions.UnauthorizedException;
 import org.sonar.server.tester.UserSessionRule;
 import org.sonar.server.ws.TestRequest;
 import org.sonar.server.ws.WsActionTester;
@@ -42,33 +42,30 @@ import static org.sonarqube.ws.client.usertoken.UserTokensWsParameters.PARAM_NAM
 
 
 public class RevokeActionTest {
-  static final String GRACE_HOPPER = "grace.hopper";
-  static final String ADA_LOVELACE = "ada.lovelace";
-  static final String TOKEN_NAME = "token-name";
+  private static final String GRACE_HOPPER = "grace.hopper";
+  private static final String ADA_LOVELACE = "ada.lovelace";
+  private static final String TOKEN_NAME = "token-name";
 
   @Rule
   public DbTester db = DbTester.create(System2.INSTANCE);
-  DbClient dbClient = db.getDbClient();
-  final DbSession dbSession = db.getSession();
   @Rule
   public UserSessionRule userSession = UserSessionRule.standalone();
   @Rule
   public ExpectedException expectedException = ExpectedException.none();
 
-  WsActionTester ws;
+  private DbClient dbClient = db.getDbClient();
+  private final DbSession dbSession = db.getSession();
+  private WsActionTester ws;
 
   @Before
   public void setUp() {
-    userSession
-      .logIn()
-      .setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN);
-
     ws = new WsActionTester(
       new RevokeAction(dbClient, userSession));
   }
 
   @Test
   public void delete_token_in_db() {
+    userSession.logIn().setRoot();
     insertUserToken(newUserToken().setLogin(GRACE_HOPPER).setName("token-to-delete"));
     insertUserToken(newUserToken().setLogin(GRACE_HOPPER).setName("token-to-keep-1"));
     insertUserToken(newUserToken().setLogin(GRACE_HOPPER).setName("token-to-keep-2"));
@@ -83,7 +80,7 @@ public class RevokeActionTest {
 
   @Test
   public void user_can_delete_its_own_tokens() {
-    userSession.logIn(GRACE_HOPPER).setGlobalPermissions(GlobalPermissions.SCAN_EXECUTION);
+    userSession.logIn(GRACE_HOPPER);
     insertUserToken(newUserToken().setLogin(GRACE_HOPPER).setName("token-to-delete"));
 
     String response = newRequest(null, "token-to-delete");
@@ -94,20 +91,32 @@ public class RevokeActionTest {
 
   @Test
   public void does_not_fail_when_incorrect_login_or_name() {
+    userSession.logIn().setRoot();
     insertUserToken(newUserToken().setLogin(GRACE_HOPPER).setName(TOKEN_NAME));
 
     newRequest(ADA_LOVELACE, "another-token-name");
   }
 
   @Test
-  public void fail_if_insufficient_privileges() {
-    userSession.logIn().setGlobalPermissions(GlobalPermissions.SCAN_EXECUTION);
+  public void throw_ForbiddenException_if_non_administrator_revokes_token_of_someone_else() {
+    userSession.logIn();
     insertUserToken(newUserToken().setLogin(GRACE_HOPPER).setName(TOKEN_NAME));
+
     expectedException.expect(ForbiddenException.class);
 
     newRequest(GRACE_HOPPER, TOKEN_NAME);
   }
 
+  @Test
+  public void throw_UnauthorizedException_if_not_logged_in() {
+    userSession.anonymous();
+    insertUserToken(newUserToken().setLogin(GRACE_HOPPER).setName(TOKEN_NAME));
+
+    expectedException.expect(UnauthorizedException.class);
+
+    newRequest(GRACE_HOPPER, TOKEN_NAME);
+  }
+
   private String newRequest(@Nullable String login, String name) {
     TestRequest testRequest = ws.newRequest()
       .setParam(PARAM_NAME, name);

diff --git a/server/sonar-server/src/test/java/org/sonar/server/usertoken/ws/SearchActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/usertoken/ws/SearchActionTest.java
index 551168baa3458e6894f702d2bb2dcb4ed03c87f0..b934ce321165c27f9f97d86b1bc4e7d4c5c54e9a 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/usertoken/ws/SearchActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/usertoken/ws/SearchActionTest.java
@@ -27,12 +27,12 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.sonar.api.utils.System2;
-import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
 import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.exceptions.NotFoundException;
+import org.sonar.server.exceptions.UnauthorizedException;
 import org.sonar.server.tester.UserSessionRule;
 import org.sonar.server.ws.TestRequest;
 import org.sonar.server.ws.TestResponse;
@@ -62,13 +62,14 @@ public class SearchActionTest {
 
   @Before
   public void setUp() {
-    userSession.logIn().setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN);
     db.users().insertUser(newUserDto().setLogin(GRACE_HOPPER));
     db.users().insertUser(newUserDto().setLogin(ADA_LOVELACE));
   }
 
   @Test
   public void search_json_example() {
+    userSession.logIn().setRoot();
+
     dbClient.userTokenDao().insert(dbSession, newUserToken()
       .setCreatedAt(1448523067221L)
       .setName("Project scan on Travis")
@@ -96,7 +97,7 @@ public class SearchActionTest {
 
   @Test
   public void a_user_can_search_its_own_token() {
-    userSession.logIn(GRACE_HOPPER).setGlobalPermissions(GlobalPermissions.SCAN_EXECUTION);
+    userSession.logIn(GRACE_HOPPER);
     dbClient.userTokenDao().insert(dbSession, newUserToken()
       .setCreatedAt(1448523067221L)
       .setName("Project scan on Travis")
@@ -110,6 +111,8 @@ public class SearchActionTest {
 
   @Test
   public void fail_when_login_does_not_exist() {
+    userSession.logIn().setRoot();
+
     expectedException.expect(NotFoundException.class);
     expectedException.expectMessage("User with login 'unknown-login' not found");
 
@@ -117,13 +120,23 @@ public class SearchActionTest {
   }
 
   @Test
-  public void fail_when_insufficient_privileges() {
-    userSession.logIn().setGlobalPermissions(GlobalPermissions.SCAN_EXECUTION);
+  public void throw_ForbiddenException_if_a_non_root_administrator_searches_for_tokens_of_someone_else() {
+    userSession.logIn();
+
     expectedException.expect(ForbiddenException.class);
 
     newRequest(GRACE_HOPPER);
   }
 
+  @Test
+  public void throw_UnauthorizedException_if_not_logged_in() {
+    userSession.anonymous();
+
+    expectedException.expect(UnauthorizedException.class);
+
+    newRequest(GRACE_HOPPER);
+  }
+
   private SearchWsResponse newRequest(@Nullable String login) {
     TestRequest testRequest = ws.newRequest()
       .setMediaType(MediaTypes.PROTOBUF);