SONAR-7740 /oauth/callback is generating a JWT token

diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/BaseContextFactory.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/BaseContextFactory.java
index 465f093f9032d20f205628f00c84b6c1d8303b46..9261f37edfd8561c483438b4302fed84fc24397c 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/authentication/BaseContextFactory.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/BaseContextFactory.java
@@ -67,7 +67,7 @@ public class BaseContextFactory {
 
     @Override
     public void authenticate(UserIdentity userIdentity) {
-      userIdentityAuthenticator.authenticate(userIdentity, identityProvider, request.getSession());
+      userIdentityAuthenticator.authenticate(userIdentity, identityProvider, request, response);
     }
   }
 }

diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2ContextFactory.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2ContextFactory.java
index 4c52ef80910f951c8aaf019948600bffb690e8ba..1652724910275322c0533e3c9d5ff045714d75b9 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2ContextFactory.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2ContextFactory.java
@@ -19,6 +19,10 @@
  */
 package org.sonar.server.authentication;
 
+import static java.lang.String.format;
+import static org.sonar.api.CoreProperties.SERVER_BASE_URL;
+import static org.sonar.server.authentication.OAuth2CallbackFilter.CALLBACK_PATH;
+
 import java.io.IOException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -27,10 +31,6 @@ import org.sonar.api.server.authentication.OAuth2IdentityProvider;
 import org.sonar.api.server.authentication.UserIdentity;
 import org.sonar.api.utils.MessageException;
 
-import static java.lang.String.format;
-import static org.sonar.api.CoreProperties.SERVER_BASE_URL;
-import static org.sonar.server.authentication.OAuth2CallbackFilter.CALLBACK_PATH;
-
 public class OAuth2ContextFactory {
 
   private final UserIdentityAuthenticator userIdentityAuthenticator;
@@ -112,7 +112,7 @@ public class OAuth2ContextFactory {
 
     @Override
     public void authenticate(UserIdentity userIdentity) {
-      userIdentityAuthenticator.authenticate(userIdentity, identityProvider, request.getSession());
+      userIdentityAuthenticator.authenticate(userIdentity, identityProvider, request, response);
     }
   }
 }

diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/UserIdentityAuthenticator.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/UserIdentityAuthenticator.java
index b0efc4e7d24562771453bb9dc316487432045c89..0e8069926d7e7bd8362b94484a6ccc759c5edfd7 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/authentication/UserIdentityAuthenticator.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/UserIdentityAuthenticator.java
@@ -19,6 +19,10 @@
  */
 package org.sonar.server.authentication;
 
+import static com.google.common.collect.FluentIterable.from;
+import static java.lang.String.format;
+import static java.util.Collections.singletonList;
+
 import com.google.common.base.Function;
 import com.google.common.collect.Sets;
 import java.util.ArrayList;
@@ -27,7 +31,8 @@ import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 import javax.annotation.Nonnull;
-import javax.servlet.http.HttpSession;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 import org.sonar.api.server.authentication.IdentityProvider;
 import org.sonar.api.server.authentication.UnauthorizedException;
 import org.sonar.api.server.authentication.UserIdentity;
@@ -43,37 +48,36 @@ import org.sonar.server.user.NewUser;
 import org.sonar.server.user.UpdateUser;
 import org.sonar.server.user.UserUpdater;
 
-import static com.google.common.collect.FluentIterable.from;
-import static java.lang.String.format;
-import static java.util.Collections.singletonList;
-
 public class UserIdentityAuthenticator {
 
   private static final Logger LOGGER = Loggers.get(UserIdentityAuthenticator.class);
 
   private final DbClient dbClient;
   private final UserUpdater userUpdater;
+  private final JwtHttpHandler jwtHttpHandler;
 
-  public UserIdentityAuthenticator(DbClient dbClient, UserUpdater userUpdater) {
+  public UserIdentityAuthenticator(DbClient dbClient, UserUpdater userUpdater, JwtHttpHandler jwtHttpHandler) {
     this.dbClient = dbClient;
     this.userUpdater = userUpdater;
+    this.jwtHttpHandler = jwtHttpHandler;
   }
 
-  public void authenticate(UserIdentity user, IdentityProvider provider, HttpSession session) {
-    long userDbId = register(user, provider);
+  public void authenticate(UserIdentity user, IdentityProvider provider, HttpServletRequest request, HttpServletResponse response) {
+    UserDto userDb = register(user, provider);
 
     // hack to disable Ruby on Rails authentication
-    session.setAttribute("user_id", userDbId);
+    request.getSession().setAttribute("user_id", userDb.getId());
+    jwtHttpHandler.generateToken(userDb.getLogin(), response);
   }
 
-  private long register(UserIdentity user, IdentityProvider provider) {
+  private UserDto register(UserIdentity user, IdentityProvider provider) {
     DbSession dbSession = dbClient.openSession(false);
     try {
       String userLogin = user.getLogin();
       UserDto userDto = dbClient.userDao().selectByLogin(dbSession, userLogin);
       if (userDto != null && userDto.isActive()) {
         registerExistingUser(dbSession, userDto, user, provider);
-        return userDto.getId();
+        return userDto;
       }
       return registerNewUser(dbSession, user, provider);
     } finally {
@@ -81,7 +85,7 @@ public class UserIdentityAuthenticator {
     }
   }
 
-  private long registerNewUser(DbSession dbSession, UserIdentity user, IdentityProvider provider) {
+  private UserDto registerNewUser(DbSession dbSession, UserIdentity user, IdentityProvider provider) {
     if (!provider.allowsUsersToSignUp()) {
       throw new UnauthorizedException(format("'%s' users are not allowed to sign up", provider.getKey()));
     }
@@ -101,7 +105,7 @@ public class UserIdentityAuthenticator {
       );
     UserDto newUser = dbClient.userDao().selectOrFailByLogin(dbSession, userLogin);
     syncGroups(dbSession, user, newUser);
-    return newUser.getId();
+    return newUser;
   }
 
   private void registerExistingUser(DbSession dbSession, UserDto userDto, UserIdentity user, IdentityProvider provider) {

diff --git a/server/sonar-server/src/test/java/org/sonar/server/authentication/BaseContextFactoryTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/BaseContextFactoryTest.java
index b90972d1d5830ae8a8f6ef81f21b493998f656a9..b74ccaf628cdf6f909c5a5a9d3ed96e4e96a1b10 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/authentication/BaseContextFactoryTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/BaseContextFactoryTest.java
@@ -19,6 +19,11 @@
  */
 package org.sonar.server.authentication;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
@@ -28,11 +33,6 @@ import org.sonar.api.platform.Server;
 import org.sonar.api.server.authentication.BaseIdentityProvider;
 import org.sonar.api.server.authentication.UserIdentity;
 
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
-
 public class BaseContextFactoryTest {
 
   static String PUBLIC_ROOT_URL = "https://mydomain.com";
@@ -74,6 +74,6 @@ public class BaseContextFactoryTest {
     when(request.getSession()).thenReturn(session);
 
     context.authenticate(USER_IDENTITY);
-    verify(userIdentityAuthenticator).authenticate(USER_IDENTITY, identityProvider, session);
+    verify(userIdentityAuthenticator).authenticate(USER_IDENTITY, identityProvider, request, response);
   }
 }

diff --git a/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuth2ContextFactoryTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuth2ContextFactoryTest.java
index b9344fb2615b8b0fa8e6263e1b1fd7b39df6fd44..c687b55ae823bec29bde2a3cec9039aca689419d 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuth2ContextFactoryTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuth2ContextFactoryTest.java
@@ -19,6 +19,11 @@
  */
 package org.sonar.server.authentication;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
@@ -31,11 +36,6 @@ import org.sonar.api.server.authentication.OAuth2IdentityProvider;
 import org.sonar.api.server.authentication.UserIdentity;
 import org.sonar.api.utils.MessageException;
 
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
-
 public class OAuth2ContextFactoryTest {
 
   @Rule
@@ -127,7 +127,7 @@ public class OAuth2ContextFactoryTest {
 
     callback.authenticate(USER_IDENTITY);
 
-    verify(userIdentityAuthenticator).authenticate(USER_IDENTITY, identityProvider, session);
+    verify(userIdentityAuthenticator).authenticate(USER_IDENTITY, identityProvider, request, response);
   }
 
   @Test

diff --git a/server/sonar-server/src/test/java/org/sonar/server/authentication/UserIdentityAuthenticatorTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/UserIdentityAuthenticatorTest.java
index 9248fdf04195ffe5d1e23ffc444b29b6308fa838..645c468fc4f9d01be3e9ceff2a68ba466ad44123 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/authentication/UserIdentityAuthenticatorTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/UserIdentityAuthenticatorTest.java
@@ -19,9 +19,18 @@
  */
 package org.sonar.server.authentication;
 
+import static com.google.common.collect.Sets.newHashSet;
+import static java.util.Collections.singletonList;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
 import org.junit.Before;
 import org.junit.Rule;
@@ -44,12 +53,6 @@ import org.sonar.server.user.NewUserNotifier;
 import org.sonar.server.user.UserUpdater;
 import org.sonar.server.user.index.UserIndexer;
 
-import static com.google.common.collect.Sets.newHashSet;
-import static java.util.Collections.singletonList;
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-
 public class UserIdentityAuthenticatorTest {
 
   static String USER_LOGIN = "github-johndoo";
@@ -81,8 +84,12 @@ public class UserIdentityAuthenticatorTest {
   UserDao userDao = dbClient.userDao();
   GroupDao groupDao = dbClient.groupDao();
   Settings settings = new Settings();
+  JwtHttpHandler jwtHttpHandler = mock(JwtHttpHandler.class);
 
+  HttpServletRequest request = mock(HttpServletRequest.class);
+  HttpServletResponse response = mock(HttpServletResponse.class);
   HttpSession httpSession = mock(HttpSession.class);
+
   UserUpdater userUpdater = new UserUpdater(
     mock(NewUserNotifier.class),
     settings,
@@ -91,17 +98,18 @@ public class UserIdentityAuthenticatorTest {
     system2
     );
 
-  UserIdentityAuthenticator underTest = new UserIdentityAuthenticator(dbClient, userUpdater);
+  UserIdentityAuthenticator underTest = new UserIdentityAuthenticator(dbClient, userUpdater, jwtHttpHandler);
 
   @Before
   public void setUp() throws Exception {
     settings.setProperty("sonar.defaultGroup", DEFAULT_GROUP);
     addGroup(DEFAULT_GROUP);
+    when(request.getSession()).thenReturn(httpSession);
   }
 
   @Test
   public void authenticate_new_user() throws Exception {
-    underTest.authenticate(USER_IDENTITY, IDENTITY_PROVIDER, httpSession);
+    underTest.authenticate(USER_IDENTITY, IDENTITY_PROVIDER, request, response);
     dbSession.commit();
 
     UserDto userDto = userDao.selectByLogin(dbSession, USER_LOGIN);
@@ -126,7 +134,7 @@ public class UserIdentityAuthenticatorTest {
       .setName("John")
       // group3 doesn't exist in db, it will be ignored
       .setGroups(newHashSet("group1", "group2", "group3"))
-      .build(), IDENTITY_PROVIDER, httpSession);
+      .build(), IDENTITY_PROVIDER, request, response);
     dbSession.commit();
 
     UserDto userDto = userDao.selectByLogin(dbSession, USER_LOGIN);
@@ -147,7 +155,7 @@ public class UserIdentityAuthenticatorTest {
       );
     dbSession.commit();
 
-    underTest.authenticate(USER_IDENTITY, IDENTITY_PROVIDER, httpSession);
+    underTest.authenticate(USER_IDENTITY, IDENTITY_PROVIDER, request, response);
     dbSession.commit();
 
     UserDto userDto = userDao.selectByLogin(dbSession, USER_LOGIN);
@@ -171,7 +179,7 @@ public class UserIdentityAuthenticatorTest {
       );
     dbSession.commit();
 
-    underTest.authenticate(USER_IDENTITY, IDENTITY_PROVIDER, httpSession);
+    underTest.authenticate(USER_IDENTITY, IDENTITY_PROVIDER, request, response);
     dbSession.commit();
 
     UserDto userDto = userDao.selectByLogin(dbSession, USER_LOGIN);
@@ -200,7 +208,7 @@ public class UserIdentityAuthenticatorTest {
       .setName("John")
       // group3 doesn't exist in db, it will be ignored
       .setGroups(newHashSet("group1", "group2", "group3"))
-      .build(), IDENTITY_PROVIDER, httpSession);
+      .build(), IDENTITY_PROVIDER, request, response);
     dbSession.commit();
 
     Set<String> userGroups = new HashSet<>(dbClient.groupMembershipDao().selectGroupsByLogins(dbSession, singletonList(USER_LOGIN)).get(USER_LOGIN));
@@ -230,7 +238,7 @@ public class UserIdentityAuthenticatorTest {
       .setName("John")
       // Only group1 is returned by the id provider => group2 will be removed
       .setGroups(newHashSet("group1"))
-      .build(), IDENTITY_PROVIDER, httpSession);
+      .build(), IDENTITY_PROVIDER, request, response);
     dbSession.commit();
 
     verifyUserGroups(USER_LOGIN, "group1");
@@ -259,7 +267,7 @@ public class UserIdentityAuthenticatorTest {
       .setName("John")
       // No group => group1 and group2 will be removed
       .setGroups(Collections.<String>emptySet())
-      .build(), IDENTITY_PROVIDER, httpSession);
+      .build(), IDENTITY_PROVIDER, request, response);
     dbSession.commit();
 
     verifyNoUserGroups(USER_LOGIN);
@@ -271,9 +279,21 @@ public class UserIdentityAuthenticatorTest {
     userDao.insert(dbSession, userDto);
     dbSession.commit();
 
-    underTest.authenticate(USER_IDENTITY, IDENTITY_PROVIDER, httpSession);
+    underTest.authenticate(USER_IDENTITY, IDENTITY_PROVIDER, request, response);
+
+    verify(httpSession).setAttribute("user_id", userDto.getId());
+  }
+
+  @Test
+  public void create_jwt_token() throws Exception {
+    UserDto userDto = UserTesting.newUserDto().setLogin(USER_LOGIN);
+    userDao.insert(dbSession, userDto);
+    dbSession.commit();
+
+    underTest.authenticate(USER_IDENTITY, IDENTITY_PROVIDER, request, response);
 
     verify(httpSession).setAttribute("user_id", userDto.getId());
+    verify(jwtHttpHandler).generateToken(USER_LOGIN, response);
   }
 
   @Test
@@ -286,7 +306,7 @@ public class UserIdentityAuthenticatorTest {
 
     thrown.expect(UnauthorizedException.class);
     thrown.expectMessage("'github' users are not allowed to sign up");
-    underTest.authenticate(USER_IDENTITY, identityProvider, httpSession);
+    underTest.authenticate(USER_IDENTITY, identityProvider, request, response);
   }
 
   @Test
@@ -301,7 +321,7 @@ public class UserIdentityAuthenticatorTest {
     thrown.expect(UnauthorizedException.class);
     thrown.expectMessage("You can't sign up because email 'john@email.com' is already used by an existing user. " +
       "This means that you probably already registered with another account.");
-    underTest.authenticate(USER_IDENTITY, IDENTITY_PROVIDER, httpSession);
+    underTest.authenticate(USER_IDENTITY, IDENTITY_PROVIDER, request, response);
   }
 
   private void verifyUserGroups(String userLogin, String... groups) {