]> source.dussan.org Git - sonarqube.git/commitdiff
SONARCLOUD-413 Synchronize github user organization membership during authentication
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Wed, 13 Feb 2019 17:36:41 +0000 (18:36 +0100)
committersonartech <sonartech@sonarsource.com>
Wed, 6 Mar 2019 10:30:42 +0000 (11:30 +0100)
20 files changed:
server/sonar-db-dao/src/main/java/org/sonar/db/alm/OrganizationAlmBindingDao.java
server/sonar-db-dao/src/main/java/org/sonar/db/alm/OrganizationAlmBindingMapper.java
server/sonar-db-dao/src/main/resources/org/sonar/db/alm/OrganizationAlmBindingMapper.xml
server/sonar-db-dao/src/test/java/org/sonar/db/alm/OrganizationAlmBindingDaoTest.java
server/sonar-db-dao/src/test/java/org/sonar/db/organization/OrganizationDbTester.java
server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2CallbackContext.java [new file with mode: 0644]
server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2CallbackFilter.java
server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2ContextFactory.java
server/sonar-server/src/main/java/org/sonar/server/authentication/UserRegistrarImpl.java
server/sonar-server/src/main/java/org/sonar/server/authentication/UserRegistration.java
server/sonar-server/src/main/java/org/sonar/server/authentication/event/AuthenticationEvent.java
server/sonar-server/src/main/java/org/sonar/server/organization/MemberUpdater.java
server/sonar-server/src/main/java/org/sonar/server/organization/ws/OrganizationsWsModule.java
server/sonar-server/src/test/java/org/sonar/server/authentication/HttpHeadersAuthenticationTest.java
server/sonar-server/src/test/java/org/sonar/server/authentication/OAuth2CallbackFilterTest.java
server/sonar-server/src/test/java/org/sonar/server/authentication/OAuth2ContextFactoryTest.java
server/sonar-server/src/test/java/org/sonar/server/authentication/UserRegistrarImplOrgMembershipSyncTest.java [new file with mode: 0644]
server/sonar-server/src/test/java/org/sonar/server/authentication/UserRegistrarImplTest.java
server/sonar-server/src/test/java/org/sonar/server/organization/MemberUpdaterTest.java
server/sonar-server/src/test/java/org/sonar/server/organization/ws/OrganizationsWsModuleTest.java

index 10c91dd509946fd68cb49ce5aacaf5c9af0cb60f..d29763b0e92288cd1806491c0c8e87e502758827 100644 (file)
@@ -24,12 +24,12 @@ import java.util.List;
 import java.util.Optional;
 import org.sonar.api.utils.System2;
 import org.sonar.core.util.UuidFactory;
-import org.sonar.core.util.stream.MoreCollectors;
 import org.sonar.db.Dao;
 import org.sonar.db.DbSession;
 import org.sonar.db.organization.OrganizationDto;
 
 import static java.util.Optional.ofNullable;
+import static org.sonar.core.util.stream.MoreCollectors.toSet;
 import static org.sonar.db.DatabaseUtils.executeLargeInputs;
 
 public class OrganizationAlmBindingDao implements Dao {
@@ -51,14 +51,21 @@ public class OrganizationAlmBindingDao implements Dao {
   }
 
   public List<OrganizationAlmBindingDto> selectByOrganizations(DbSession dbSession, Collection<OrganizationDto> organizations) {
-    return executeLargeInputs(organizations.stream().map(OrganizationDto::getUuid).collect(MoreCollectors.toSet()),
-      organizationUuids -> getMapper(dbSession).selectByOrganizationUuids(organizationUuids));
+    return selectByOrganizationUuids(dbSession, organizations.stream().map(OrganizationDto::getUuid).collect(toSet()));
+  }
+
+  public List<OrganizationAlmBindingDto> selectByOrganizationUuids(DbSession dbSession, Collection<String> organizationUuids) {
+    return executeLargeInputs(organizationUuids, uuids -> getMapper(dbSession).selectByOrganizationUuids(uuids));
   }
 
   public Optional<OrganizationAlmBindingDto> selectByAlmAppInstall(DbSession dbSession, AlmAppInstallDto almAppInstall) {
     return ofNullable(getMapper(dbSession).selectByInstallationUuid(almAppInstall.getUuid()));
   }
 
+  public List<OrganizationAlmBindingDto> selectByOrganizationAlmIds(DbSession dbSession, ALM alm, Collection<String> organizationAlmIds) {
+    return executeLargeInputs(organizationAlmIds, o -> getMapper(dbSession).selectByOrganizationAlmIds(alm.getId(), o));
+  }
+
   public void insert(DbSession dbSession, OrganizationDto organization, AlmAppInstallDto almAppInstall, String url, String userUuid, boolean membersSync) {
     long now = system2.now();
     getMapper(dbSession).insert(new OrganizationAlmBindingDto()
index d59c5bfd0ced84aa4e0fc98fa358ca9a5e2b1009..2584a9dbf037dfa3c7b337ecd5253881bf959343 100644 (file)
@@ -34,6 +34,8 @@ public interface OrganizationAlmBindingMapper {
   @CheckForNull
   OrganizationAlmBindingDto selectByInstallationUuid(@Param("installationUuid") String installationUuid);
 
+  List<OrganizationAlmBindingDto> selectByOrganizationAlmIds(@Param("alm") String alm, @Param("organizationAlmIds") List<String> organizationAlmId);
+
   void insert(@Param("dto") OrganizationAlmBindingDto dto);
 
   void deleteByOrganizationUuid(@Param("organizationUuid") String organizationUuid);
index e55533434ea9bff4d0e6f6fbccd7c0b1fdf640dd..38af308113a1df6fa1a66f7eb3bd025135c8536a 100644 (file)
@@ -4,32 +4,32 @@
 <mapper namespace="org.sonar.db.alm.OrganizationAlmBindingMapper">
 
   <sql id="columns">
-    uuid,
-    organization_uuid as organizationUuid,
-    alm_app_install_uuid as almAppInstallUuid,
-    alm_id as rawAlmId,
-    url,
-    user_uuid as userUuid,
-    members_sync_enabled as membersSyncEnabled,
-    created_at as createdAt
+    oab.uuid,
+    oab.organization_uuid as organizationUuid,
+    oab.alm_app_install_uuid as almAppInstallUuid,
+    oab.alm_id as rawAlmId,
+    oab.url,
+    oab.user_uuid as userUuid,
+    oab.members_sync_enabled as membersSyncEnabled,
+    oab.created_at as createdAt
   </sql>
 
   <select id="selectByOrganizationUuid" parameterType="String" resultType="org.sonar.db.alm.OrganizationAlmBindingDto">
     select
       <include refid="columns"/>
     from
-      organization_alm_bindings
+      organization_alm_bindings oab
     where
-      organization_uuid = #{organizationUuid, jdbcType=VARCHAR}
+      oab.organization_uuid = #{organizationUuid, jdbcType=VARCHAR}
   </select>
 
   <select id="selectByOrganizationUuids" parameterType="String" resultType="org.sonar.db.alm.OrganizationAlmBindingDto">
     select
     <include refid="columns"/>
     from
-      organization_alm_bindings
+      organization_alm_bindings oab
     where
-      organization_uuid in
+      oab.organization_uuid in
       <foreach collection="organizationUuids" open="(" close=")" item="organizationUuid" separator=",">
         #{organizationUuid , jdbcType=VARCHAR}
       </foreach>
     select
     <include refid="columns"/>
     from
-      organization_alm_bindings
+      organization_alm_bindings oab
     where
-      alm_app_install_uuid = #{installationUuid, jdbcType=VARCHAR}
+      oab.alm_app_install_uuid = #{installationUuid, jdbcType=VARCHAR}
+  </select>
+
+  <select id="selectByOrganizationAlmIds" parameterType="Map" resultType="org.sonar.db.alm.OrganizationAlmBindingDto">
+    select
+    <include refid="columns"/>
+    from
+      organization_alm_bindings oab
+      inner join alm_app_installs aai on aai.uuid = oab.alm_app_install_uuid and aai.alm_id = oab.alm_id
+    where
+      aai.alm_id = #{alm, jdbcType=VARCHAR}
+      and aai.owner_id in
+      <foreach collection="organizationAlmIds" open="(" close=")" item="organizationAlmId" separator=",">
+        #{organizationAlmId , jdbcType=VARCHAR}
+      </foreach>
   </select>
 
   <insert id="insert" parameterType="Map" useGeneratedKeys="false">
index 4735bccb9a19d42114c04a11bae6667b712bdae5..8f165bda4d100cddce8409a024a12627ce19ec9a 100644 (file)
@@ -19,6 +19,7 @@
  */
 package org.sonar.db.alm;
 
+import java.util.List;
 import java.util.Optional;
 import org.junit.Rule;
 import org.junit.Test;
@@ -36,6 +37,8 @@ import static org.assertj.core.api.Assertions.entry;
 import static org.assertj.core.groups.Tuple.tuple;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
+import static org.sonar.db.alm.ALM.BITBUCKETCLOUD;
+import static org.sonar.db.alm.ALM.GITHUB;
 
 public class OrganizationAlmBindingDaoTest {
 
@@ -64,7 +67,7 @@ public class OrganizationAlmBindingDaoTest {
         OrganizationAlmBindingDto::getUrl, OrganizationAlmBindingDto::getAlm,
         OrganizationAlmBindingDto::getUserUuid, OrganizationAlmBindingDto::getCreatedAt)
       .containsExactlyInAnyOrder(dto.getUuid(), organization.getUuid(), dto.getAlmAppInstallUuid(),
-        dto.getUrl(), ALM.GITHUB,
+        dto.getUrl(), GITHUB,
         dto.getUserUuid(), NOW);
   }
 
@@ -92,7 +95,7 @@ public class OrganizationAlmBindingDaoTest {
         OrganizationAlmBindingDto::getUrl, OrganizationAlmBindingDto::getAlm,
         OrganizationAlmBindingDto::getUserUuid, OrganizationAlmBindingDto::getCreatedAt)
       .containsExactlyInAnyOrder(dto.getUuid(), organization.getUuid(), dto.getAlmAppInstallUuid(),
-        dto.getUrl(), ALM.GITHUB,
+        dto.getUrl(), GITHUB,
         dto.getUserUuid(), NOW);
 
     assertThat(underTest.selectByOrganizationUuid(db.getSession(), "unknown")).isNotPresent();
@@ -128,7 +131,7 @@ public class OrganizationAlmBindingDaoTest {
         OrganizationAlmBindingDto::getUrl, OrganizationAlmBindingDto::getAlm,
         OrganizationAlmBindingDto::getUserUuid, OrganizationAlmBindingDto::getCreatedAt)
       .containsExactlyInAnyOrder(dto.getUuid(), organization.getUuid(), dto.getAlmAppInstallUuid(),
-        dto.getUrl(), ALM.GITHUB,
+        dto.getUrl(), GITHUB,
         dto.getUserUuid(), NOW);
   }
 
@@ -145,6 +148,22 @@ public class OrganizationAlmBindingDaoTest {
     assertThat(result).isEmpty();
   }
 
+  @Test
+  public void selectByOrganizationAlmIds() {
+    AlmAppInstallDto gitHubInstall1 = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId()));
+    OrganizationAlmBindingDto organizationAlmBinding1 = db.alm().insertOrganizationAlmBinding(db.organizations().insert(), gitHubInstall1, true);
+    AlmAppInstallDto gitHubInstall2 = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId()));
+    OrganizationAlmBindingDto organizationAlmBinding2 = db.alm().insertOrganizationAlmBinding(db.organizations().insert(), gitHubInstall2, true);
+    AlmAppInstallDto bitBucketInstall = db.alm().insertAlmAppInstall(a -> a.setAlmId(BITBUCKETCLOUD.getId()));
+    OrganizationAlmBindingDto organizationAlmBinding3 = db.alm().insertOrganizationAlmBinding(db.organizations().insert(), bitBucketInstall, true);
+
+    List<OrganizationAlmBindingDto> result = underTest.selectByOrganizationAlmIds(db.getSession(), GITHUB,
+      asList(gitHubInstall1.getOwnerId(), gitHubInstall2.getOwnerId(), bitBucketInstall.getOwnerId(), "unknown"));
+
+    assertThat(result).extracting(OrganizationAlmBindingDto::getUuid)
+      .containsExactlyInAnyOrder(organizationAlmBinding1.getUuid(), organizationAlmBinding2.getUuid());
+  }
+
   @Test
   public void insert() {
     when(uuidFactory.create()).thenReturn("ABCD");
index fc27f44398e3a81a5a116105df61731a80f9d8b0..5daf1ff867da4eec0437f3d4b70ea5d25dfcdb5f 100644 (file)
@@ -25,15 +25,19 @@ import javax.annotation.Nullable;
 import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
 import org.sonar.db.permission.template.PermissionTemplateDto;
+import org.sonar.db.user.GroupMembershipDto;
+import org.sonar.db.user.GroupMembershipQuery;
 import org.sonar.db.user.UserDto;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.sonar.db.user.GroupMembershipQuery.IN;
 
 public class OrganizationDbTester {
-  private final DbTester dbTester;
+  private final DbTester db;
 
-  public OrganizationDbTester(DbTester dbTester) {
-    this.dbTester = dbTester;
+  public OrganizationDbTester(DbTester db) {
+    this.db = db;
   }
 
   /**
@@ -61,8 +65,8 @@ public class OrganizationDbTester {
    * Insert the provided {@link OrganizationDto} and commit the session
    */
   public OrganizationDto insert(OrganizationDto dto) {
-    DbSession dbSession = dbTester.getSession();
-    dbTester.getDbClient().organizationDao().insert(dbSession, dto, false);
+    DbSession dbSession = db.getSession();
+    db.getDbClient().organizationDao().insert(dbSession, dto, false);
     dbSession.commit();
     return dto;
   }
@@ -73,11 +77,11 @@ public class OrganizationDbTester {
       || portfolioDefaultTemplate.getOrganizationUuid().equals(projectDefaultTemplate.getOrganizationUuid()),
       "default template for project and portfolio must belong to the same organization");
     checkArgument(applicationDefaultTemplate == null
-        || applicationDefaultTemplate.getOrganizationUuid().equals(projectDefaultTemplate.getOrganizationUuid()),
+      || applicationDefaultTemplate.getOrganizationUuid().equals(projectDefaultTemplate.getOrganizationUuid()),
       "default template for project and application must belong to the same organization");
 
-    DbSession dbSession = dbTester.getSession();
-    dbTester.getDbClient().organizationDao().setDefaultTemplates(dbSession, projectDefaultTemplate.getOrganizationUuid(),
+    DbSession dbSession = db.getSession();
+    db.getDbClient().organizationDao().setDefaultTemplates(dbSession, projectDefaultTemplate.getOrganizationUuid(),
       new DefaultTemplates()
         .setProjectUuid(projectDefaultTemplate.getUuid())
         .setPortfoliosUuid(portfolioDefaultTemplate == null ? null : portfolioDefaultTemplate.getUuid())
@@ -87,8 +91,8 @@ public class OrganizationDbTester {
 
   public void setDefaultTemplates(OrganizationDto defaultOrganization, String projectDefaultTemplateUuid,
     @Nullable String applicationDefaultTemplateUuid, @Nullable String portfoliosDefaultTemplateUuid) {
-    DbSession dbSession = dbTester.getSession();
-    dbTester.getDbClient().organizationDao().setDefaultTemplates(dbSession, defaultOrganization.getUuid(),
+    DbSession dbSession = db.getSession();
+    db.getDbClient().organizationDao().setDefaultTemplates(dbSession, defaultOrganization.getUuid(),
       new DefaultTemplates()
         .setProjectUuid(projectDefaultTemplateUuid)
         .setApplicationsUuid(applicationDefaultTemplateUuid)
@@ -97,16 +101,38 @@ public class OrganizationDbTester {
   }
 
   public void addMember(OrganizationDto organization, UserDto... users) {
-    Arrays.stream(users).forEach(u -> dbTester.getDbClient().organizationMemberDao().insert(dbTester.getSession(), new OrganizationMemberDto().setOrganizationUuid(organization.getUuid()).setUserId(u.getId())));
-    dbTester.commit();
+    Arrays.stream(users)
+      .forEach(u -> db.getDbClient().organizationMemberDao().insert(db.getSession(), new OrganizationMemberDto().setOrganizationUuid(organization.getUuid()).setUserId(u.getId())));
+    db.commit();
   }
 
   public void setNewProjectPrivate(OrganizationDto organization, boolean newProjectPrivate) {
-    dbTester.getDbClient().organizationDao().setNewProjectPrivate(dbTester.getSession(), organization, newProjectPrivate);
-    dbTester.commit();
+    db.getDbClient().organizationDao().setNewProjectPrivate(db.getSession(), organization, newProjectPrivate);
+    db.commit();
   }
 
   public boolean getNewProjectPrivate(OrganizationDto organization) {
-    return dbTester.getDbClient().organizationDao().getNewProjectPrivate(dbTester.getSession(), organization);
+    return db.getDbClient().organizationDao().getNewProjectPrivate(db.getSession(), organization);
   }
+
+  public void assertUserIsMemberOfOrganization(OrganizationDto organization, UserDto user) {
+    assertThat(db.getDbClient().organizationMemberDao().select(db.getSession(), organization.getUuid(), user.getId())).as("User is not member of the organization").isPresent();
+    Integer defaultGroupId = db.getDbClient().organizationDao().getDefaultGroupId(db.getSession(), organization.getUuid()).get();
+    assertThat(db.getDbClient().groupMembershipDao().selectGroups(
+      db.getSession(),
+      GroupMembershipQuery.builder().membership(IN).organizationUuid(organization.getUuid()).build(),
+      user.getId(), 0, 10))
+        .extracting(GroupMembershipDto::getId)
+        .as("User is not member of the default group of the organization")
+        .containsOnly(defaultGroupId.longValue());
+  }
+
+  public void assertUserIsNotMemberOfOrganization(OrganizationDto organization, UserDto user) {
+    assertThat(db.getDbClient().organizationMemberDao().select(db.getSession(), organization.getUuid(), user.getId())).as("User is still member of the organization")
+      .isNotPresent();
+    assertThat(db.getDbClient().groupMembershipDao().countGroups(db.getSession(),
+      GroupMembershipQuery.builder().membership(IN).organizationUuid(organization.getUuid()).build(),
+      user.getId())).isZero();
+  }
+
 }
diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2CallbackContext.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2CallbackContext.java
new file mode 100644 (file)
index 0000000..1a0413b
--- /dev/null
@@ -0,0 +1,31 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2019 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.server.authentication;
+
+import java.util.Set;
+import javax.annotation.Nullable;
+import org.sonar.api.server.authentication.OAuth2IdentityProvider;
+import org.sonar.api.server.authentication.UserIdentity;
+
+public interface OAuth2CallbackContext extends OAuth2IdentityProvider.CallbackContext {
+
+  void authenticate(UserIdentity userIdentity, @Nullable Set<String> organizationAlmIds);
+
+}
index 810ce33ae0f3d58bdbf295fc3ff8a6fbedf98223..a2376fad354ae0385f2c2559d2be5e00b52406c6 100644 (file)
@@ -19,7 +19,6 @@
  */
 package org.sonar.server.authentication;
 
-import javax.annotation.CheckForNull;
 import javax.servlet.FilterChain;
 import javax.servlet.FilterConfig;
 import javax.servlet.ServletRequest;
@@ -30,10 +29,10 @@ import org.sonar.api.platform.Server;
 import org.sonar.api.server.authentication.IdentityProvider;
 import org.sonar.api.server.authentication.OAuth2IdentityProvider;
 import org.sonar.api.server.authentication.UnauthorizedException;
-import org.sonar.api.server.authentication.UserIdentity;
 import org.sonar.server.authentication.event.AuthenticationEvent;
 import org.sonar.server.authentication.event.AuthenticationException;
 import org.sonar.server.authentication.exception.RedirectionException;
+import org.sonar.server.user.ThreadLocalUserSession;
 
 import static java.lang.String.format;
 import static org.sonar.server.authentication.AuthenticationError.handleAuthenticationError;
@@ -46,13 +45,15 @@ public class OAuth2CallbackFilter extends AuthenticationFilter {
   private final OAuth2ContextFactory oAuth2ContextFactory;
   private final AuthenticationEvent authenticationEvent;
   private final OAuth2AuthenticationParameters oauth2Parameters;
+  private final ThreadLocalUserSession threadLocalUserSession;
 
   public OAuth2CallbackFilter(IdentityProviderRepository identityProviderRepository, OAuth2ContextFactory oAuth2ContextFactory,
-    Server server, AuthenticationEvent authenticationEvent, OAuth2AuthenticationParameters oauth2Parameters) {
+    Server server, AuthenticationEvent authenticationEvent, OAuth2AuthenticationParameters oauth2Parameters, ThreadLocalUserSession threadLocalUserSession) {
     super(server, identityProviderRepository);
     this.oAuth2ContextFactory = oAuth2ContextFactory;
     this.authenticationEvent = authenticationEvent;
     this.oauth2Parameters = oauth2Parameters;
+    this.threadLocalUserSession = threadLocalUserSession;
   }
 
   @Override
@@ -92,7 +93,7 @@ public class OAuth2CallbackFilter extends AuthenticationFilter {
   }
 
   private void handleOAuth2Provider(HttpServletResponse response, HttpServletRequest httpRequest, OAuth2IdentityProvider oAuth2Provider) {
-    OAuth2CallbackFilter.WrappedContext context = new OAuth2CallbackFilter.WrappedContext(oAuth2ContextFactory.newCallback(httpRequest, response, oAuth2Provider));
+    OAuth2IdentityProvider.CallbackContext context = oAuth2ContextFactory.newCallback(httpRequest, response, oAuth2Provider);
     try {
       oAuth2Provider.callback(context);
     } catch (UnauthorizedException e) {
@@ -102,8 +103,8 @@ public class OAuth2CallbackFilter extends AuthenticationFilter {
         .setPublicMessage(e.getMessage())
         .build();
     }
-    if (context.isAuthenticated()) {
-      authenticationEvent.loginSuccess(httpRequest, context.getLogin(), Source.oauth2(oAuth2Provider));
+    if (threadLocalUserSession.hasSession()) {
+      authenticationEvent.loginSuccess(httpRequest, threadLocalUserSession.getLogin(), Source.oauth2(oAuth2Provider));
     } else {
       throw AuthenticationException.newBuilder()
         .setSource(Source.oauth2(oAuth2Provider))
@@ -122,60 +123,4 @@ public class OAuth2CallbackFilter extends AuthenticationFilter {
     // Nothing to do
   }
 
-  private static final class WrappedContext implements OAuth2IdentityProvider.CallbackContext {
-    private final OAuth2IdentityProvider.CallbackContext delegate;
-    private boolean authenticated = false;
-    @CheckForNull
-    private String login;
-
-    private WrappedContext(OAuth2IdentityProvider.CallbackContext delegate) {
-      this.delegate = delegate;
-    }
-
-    @Override
-    public String getCallbackUrl() {
-      return delegate.getCallbackUrl();
-    }
-
-    @Override
-    public HttpServletRequest getRequest() {
-      return delegate.getRequest();
-    }
-
-    @Override
-    public HttpServletResponse getResponse() {
-      return delegate.getResponse();
-    }
-
-    @Override
-    public void verifyCsrfState() {
-      delegate.verifyCsrfState();
-    }
-
-    @Override
-    public void verifyCsrfState(String parameterName) {
-      delegate.verifyCsrfState(parameterName);
-    }
-
-    @Override
-    public void redirectToRequestedPage() {
-      delegate.redirectToRequestedPage();
-    }
-
-    @Override
-    public void authenticate(UserIdentity userIdentity) {
-      delegate.authenticate(userIdentity);
-      this.authenticated = true;
-      this.login = userIdentity.getLogin();
-    }
-
-    public boolean isAuthenticated() {
-      return authenticated;
-    }
-
-    @CheckForNull
-    public String getLogin() {
-      return login;
-    }
-  }
 }
index 2dbf14bc5ec959674546cdb8490f8f364ab5ccaa..bc1a23652c09e7f3882091f74ad8e2c08256b5ea 100644 (file)
@@ -21,6 +21,8 @@ package org.sonar.server.authentication;
 
 import java.io.IOException;
 import java.util.Optional;
+import java.util.Set;
+import javax.annotation.Nullable;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import org.sonar.api.platform.Server;
@@ -67,7 +69,7 @@ public class OAuth2ContextFactory {
     return new OAuthContextImpl(request, response, identityProvider);
   }
 
-  private class OAuthContextImpl implements OAuth2IdentityProvider.InitContext, OAuth2IdentityProvider.CallbackContext {
+  public class OAuthContextImpl implements OAuth2IdentityProvider.InitContext, OAuth2CallbackContext {
 
     private final HttpServletRequest request;
     private final HttpServletResponse response;
@@ -131,6 +133,11 @@ public class OAuth2ContextFactory {
 
     @Override
     public void authenticate(UserIdentity userIdentity) {
+      authenticate(userIdentity, null);
+    }
+
+    @Override
+    public void authenticate(UserIdentity userIdentity, @Nullable Set<String> organizationAlmIds) {
       Boolean allowEmailShift = oAuthParameters.getAllowEmailShift(request).orElse(false);
       Boolean allowUpdateLogin = oAuthParameters.getAllowUpdateLogin(request).orElse(false);
       UserDto userDto = userRegistrar.register(
@@ -140,6 +147,7 @@ public class OAuth2ContextFactory {
           .setSource(AuthenticationEvent.Source.oauth2(identityProvider))
           .setExistingEmailStrategy(allowEmailShift ? ExistingEmailStrategy.ALLOW : ExistingEmailStrategy.WARN)
           .setUpdateLoginStrategy(allowUpdateLogin ? UpdateLoginStrategy.ALLOW : UpdateLoginStrategy.WARN)
+          .setOrganizationAlmIds(organizationAlmIds)
           .build());
       jwtHttpHandler.generateToken(userDto, request, response);
       threadLocalUserSession.set(userSessionFactory.create(userDto));
index c3708e6e0d67cc63ec9cfe9f1b7ed55fd2a93551..a1ac076fbe488459869b63cbaf3d7b77ad39d396 100644 (file)
@@ -28,6 +28,7 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
+import java.util.function.Consumer;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import org.sonar.api.server.authentication.IdentityProvider;
@@ -36,6 +37,7 @@ import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
+import org.sonar.db.alm.ALM;
 import org.sonar.db.organization.OrganizationDto;
 import org.sonar.db.user.GroupDto;
 import org.sonar.db.user.UserDto;
@@ -46,11 +48,13 @@ import org.sonar.server.authentication.exception.EmailAlreadyExistsRedirectionEx
 import org.sonar.server.authentication.exception.UpdateLoginRedirectionException;
 import org.sonar.server.organization.DefaultOrganization;
 import org.sonar.server.organization.DefaultOrganizationProvider;
+import org.sonar.server.organization.MemberUpdater;
 import org.sonar.server.organization.OrganizationFlags;
 import org.sonar.server.organization.OrganizationUpdater;
 import org.sonar.server.user.ExternalIdentity;
 import org.sonar.server.user.NewUser;
 import org.sonar.server.user.UpdateUser;
+import org.sonar.server.user.UserSession;
 import org.sonar.server.user.UserUpdater;
 import org.sonar.server.usergroups.DefaultGroupFinder;
 
@@ -71,15 +75,17 @@ public class UserRegistrarImpl implements UserRegistrar {
   private final OrganizationFlags organizationFlags;
   private final OrganizationUpdater organizationUpdater;
   private final DefaultGroupFinder defaultGroupFinder;
+  private final MemberUpdater memberUpdater;
 
   public UserRegistrarImpl(DbClient dbClient, UserUpdater userUpdater, DefaultOrganizationProvider defaultOrganizationProvider, OrganizationFlags organizationFlags,
-    OrganizationUpdater organizationUpdater, DefaultGroupFinder defaultGroupFinder) {
+    OrganizationUpdater organizationUpdater, DefaultGroupFinder defaultGroupFinder, MemberUpdater memberUpdater) {
     this.dbClient = dbClient;
     this.userUpdater = userUpdater;
     this.defaultOrganizationProvider = defaultOrganizationProvider;
     this.organizationFlags = organizationFlags;
     this.organizationUpdater = organizationUpdater;
     this.defaultGroupFinder = defaultGroupFinder;
+    this.memberUpdater = memberUpdater;
   }
 
   @Override
@@ -120,9 +126,9 @@ public class UserRegistrarImpl implements UserRegistrar {
     Optional<UserDto> otherUserToIndex = detectEmailUpdate(dbSession, authenticatorParameters);
     NewUser newUser = createNewUser(authenticatorParameters);
     if (disabledUser == null) {
-      return userUpdater.createAndCommit(dbSession, newUser, u -> syncGroups(dbSession, authenticatorParameters.getUserIdentity(), u), toArray(otherUserToIndex));
+      return userUpdater.createAndCommit(dbSession, newUser, beforeCommit(dbSession, true, authenticatorParameters), toArray(otherUserToIndex));
     }
-    return userUpdater.reactivateAndCommit(dbSession, disabledUser, newUser, u -> syncGroups(dbSession, authenticatorParameters.getUserIdentity(), u), toArray(otherUserToIndex));
+    return userUpdater.reactivateAndCommit(dbSession, disabledUser, newUser, beforeCommit(dbSession, true, authenticatorParameters), toArray(otherUserToIndex));
   }
 
   private UserDto registerExistingUser(DbSession dbSession, UserDto userDto, UserRegistration authenticatorParameters) {
@@ -139,10 +145,17 @@ public class UserRegistrarImpl implements UserRegistrar {
     }
     detectLoginUpdate(dbSession, userDto, update, authenticatorParameters);
     Optional<UserDto> otherUserToIndex = detectEmailUpdate(dbSession, authenticatorParameters);
-    userUpdater.updateAndCommit(dbSession, userDto, update, u -> syncGroups(dbSession, authenticatorParameters.getUserIdentity(), u), toArray(otherUserToIndex));
+    userUpdater.updateAndCommit(dbSession, userDto, update, beforeCommit(dbSession, false, authenticatorParameters), toArray(otherUserToIndex));
     return userDto;
   }
 
+  private Consumer<UserDto> beforeCommit(DbSession dbSession, boolean isNewUser, UserRegistration authenticatorParameters) {
+    return user -> {
+      syncGroups(dbSession, authenticatorParameters.getUserIdentity(), user);
+      synchronizeOrganizationMembership(dbSession, user, authenticatorParameters, isNewUser);
+    };
+  }
+
   private Optional<UserDto> detectEmailUpdate(DbSession dbSession, UserRegistration authenticatorParameters) {
     String email = authenticatorParameters.getUserIdentity().getEmail();
     if (email == null) {
@@ -258,6 +271,18 @@ public class UserRegistrarImpl implements UserRegistrar {
     return organizationFlags.isEnabled(dbSession) ? Optional.empty() : Optional.of(defaultGroupFinder.findDefaultGroup(dbSession, defaultOrganizationProvider.get().getUuid()));
   }
 
+  private void synchronizeOrganizationMembership(DbSession dbSession, UserDto userDto, UserRegistration authenticatorParameters, boolean isNewUser) {
+    Set<String> almOrganizationIds = authenticatorParameters.getOrganizationAlmIds();
+    if (almOrganizationIds == null || !isNewUser || !organizationFlags.isEnabled(dbSession)) {
+      return;
+    }
+    UserSession.IdentityProvider identityProvider = UserSession.IdentityProvider.getFromKey(authenticatorParameters.getProvider().getKey());
+    if (identityProvider != UserSession.IdentityProvider.GITHUB) {
+      return;
+    }
+    memberUpdater.synchronizeUserOrganizationMembership(dbSession, userDto, ALM.GITHUB, almOrganizationIds);
+  }
+
   private static NewUser createNewUser(UserRegistration authenticatorParameters) {
     String identityProviderKey = authenticatorParameters.getProvider().getKey();
     if (!authenticatorParameters.getProvider().allowsUsersToSignUp()) {
index bf14b7cef917368ff7df14aaa63bd78a69494359..8b42c64e6a7adc2c959f4f53254bc4388f8d9292 100644 (file)
@@ -19,6 +19,9 @@
  */
 package org.sonar.server.authentication;
 
+import java.util.Set;
+import javax.annotation.CheckForNull;
+import javax.annotation.Nullable;
 import org.sonar.api.server.authentication.IdentityProvider;
 import org.sonar.api.server.authentication.UserIdentity;
 import org.sonar.server.authentication.event.AuthenticationEvent;
@@ -65,6 +68,7 @@ class UserRegistration {
   private final AuthenticationEvent.Source source;
   private final ExistingEmailStrategy existingEmailStrategy;
   private final UpdateLoginStrategy updateLoginStrategy;
+  private final Set<String> organizationAlmIds;
 
   UserRegistration(Builder builder) {
     this.userIdentity = builder.userIdentity;
@@ -72,6 +76,7 @@ class UserRegistration {
     this.source = builder.source;
     this.existingEmailStrategy = builder.existingEmailStrategy;
     this.updateLoginStrategy = builder.updateLoginStrategy;
+    this.organizationAlmIds = builder.organizationAlmIds;
   }
 
   public UserIdentity getUserIdentity() {
@@ -94,6 +99,11 @@ class UserRegistration {
     return updateLoginStrategy;
   }
 
+  @CheckForNull
+  public Set<String> getOrganizationAlmIds() {
+    return organizationAlmIds;
+  }
+
   static UserRegistration.Builder builder() {
     return new Builder();
   }
@@ -104,6 +114,7 @@ class UserRegistration {
     private AuthenticationEvent.Source source;
     private ExistingEmailStrategy existingEmailStrategy;
     private UpdateLoginStrategy updateLoginStrategy;
+    private Set<String> organizationAlmIds;
 
     public Builder setUserIdentity(UserIdentity userIdentity) {
       this.userIdentity = userIdentity;
@@ -136,6 +147,15 @@ class UserRegistration {
       return this;
     }
 
+    /**
+     * List of ALM organization the user is member of.
+     * When set to null, it means that no organization membership synchronization should be done.
+     */
+    public Builder setOrganizationAlmIds(@Nullable Set<String> organizationAlmIds) {
+      this.organizationAlmIds = organizationAlmIds;
+      return this;
+    }
+
     public UserRegistration build() {
       requireNonNull(userIdentity, "userIdentity must be set");
       requireNonNull(provider, "identityProvider must be set");
index 4e8e82acf08ea52b0a1284aff4001d71ec9580a2..0e61085c6ccd05b33fbee150ffb8817e7b2615d5 100644 (file)
@@ -32,7 +32,7 @@ import static java.util.Objects.requireNonNull;
 
 public interface AuthenticationEvent {
 
-  void loginSuccess(HttpServletRequest request, String login, Source source);
+  void loginSuccess(HttpServletRequest request, @Nullable String login, Source source);
 
   void loginFailure(HttpServletRequest request, AuthenticationException e);
 
index 24dd4eac4becf51d2dc1461a26b533904c18ee70..0f6c126a6a68d5598ffd94ff419d6f77a614bab3 100644 (file)
@@ -21,9 +21,12 @@ package org.sonar.server.organization;
 
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
+import org.sonar.db.alm.ALM;
+import org.sonar.db.alm.OrganizationAlmBindingDto;
 import org.sonar.db.organization.OrganizationDto;
 import org.sonar.db.organization.OrganizationMemberDto;
 import org.sonar.db.user.UserDto;
@@ -33,10 +36,12 @@ import org.sonar.server.usergroups.DefaultGroupFinder;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.collect.Sets.difference;
+import static com.google.common.collect.Sets.union;
 import static java.util.Collections.singletonList;
 import static java.util.stream.Collectors.toSet;
 import static org.sonar.api.CoreProperties.DEFAULT_ISSUE_ASSIGNEE;
 import static org.sonar.core.util.stream.MoreCollectors.toList;
+import static org.sonar.core.util.stream.MoreCollectors.uniqueIndex;
 import static org.sonar.db.permission.OrganizationPermission.ADMINISTER;
 
 public class MemberUpdater {
@@ -61,7 +66,7 @@ public class MemberUpdater {
       .filter(UserDto::isActive)
       .filter(u -> !currentMemberIds.contains(u.getId()))
       .collect(toList());
-    if (usersToAdd.isEmpty()){
+    if (usersToAdd.isEmpty()) {
       return;
     }
     usersToAdd.forEach(u -> addMemberInDb(dbSession, organization, u));
@@ -86,7 +91,7 @@ public class MemberUpdater {
       .filter(UserDto::isActive)
       .filter(u -> currentMemberIds.contains(u.getId()))
       .collect(toList());
-    if (usersToRemove.isEmpty()){
+    if (usersToRemove.isEmpty()) {
       return;
     }
 
@@ -98,6 +103,35 @@ public class MemberUpdater {
     userIndexer.commitAndIndex(dbSession, usersToRemove);
   }
 
+  /**
+   * Synchronize organization membership of a user from a list of ALM organization specific ids
+   * Please note that no commit will not be executed.
+   */
+  public void synchronizeUserOrganizationMembership(DbSession dbSession, UserDto user, ALM alm, Set<String> organizationAlmIds) {
+    Set<String> userOrganizationUuids = dbClient.organizationMemberDao().selectOrganizationUuidsByUser(dbSession, user.getId());
+    Set<String> userOrganizationUuidsWithMembersSyncEnabled = dbClient.organizationAlmBindingDao().selectByOrganizationUuids(dbSession, userOrganizationUuids).stream()
+      .filter(OrganizationAlmBindingDto::isMembersSyncEnable)
+      .map(OrganizationAlmBindingDto::getOrganizationUuid)
+      .collect(toSet());
+    Set<String> almOrganizationUuidsWithMembersSyncEnabled = dbClient.organizationAlmBindingDao().selectByOrganizationAlmIds(dbSession, alm, organizationAlmIds).stream()
+      .filter(OrganizationAlmBindingDto::isMembersSyncEnable)
+      .map(OrganizationAlmBindingDto::getOrganizationUuid)
+      .collect(toSet());
+
+    Set<String> organizationUuidsToBeAdded = difference(almOrganizationUuidsWithMembersSyncEnabled, userOrganizationUuidsWithMembersSyncEnabled);
+    Set<String> organizationUuidsToBeRemoved = difference(userOrganizationUuidsWithMembersSyncEnabled, almOrganizationUuidsWithMembersSyncEnabled);
+    Map<String, OrganizationDto> allOrganizationsByUuid = dbClient.organizationDao().selectByUuids(dbSession, union(organizationUuidsToBeAdded, organizationUuidsToBeRemoved))
+      .stream()
+      .collect(uniqueIndex(OrganizationDto::getUuid));
+
+    allOrganizationsByUuid.entrySet().stream()
+      .filter(entry -> organizationUuidsToBeAdded.contains(entry.getKey()))
+      .forEach(entry -> addMemberInDb(dbSession, entry.getValue(), user));
+    allOrganizationsByUuid.entrySet().stream()
+      .filter(entry -> organizationUuidsToBeRemoved.contains(entry.getKey()))
+      .forEach(entry -> removeMemberInDb(dbSession, entry.getValue(), user));
+  }
+
   private void removeMemberInDb(DbSession dbSession, OrganizationDto organization, UserDto user) {
     int userId = user.getId();
     String organizationUuid = organization.getUuid();
index d912f07069e2ab71b2b83bb57a567bba238069fb..a873bee75614a08a6f2dfa99a8c27c40f47db961 100644 (file)
@@ -39,6 +39,7 @@ public class OrganizationsWsModule extends Module {
     add(
       OrganizationsWs.class,
       OrganizationsWsSupport.class,
+      MemberUpdater.class,
       // actions
       SearchAction.class,
       SearchMembersAction.class);
@@ -50,7 +51,6 @@ public class OrganizationsWsModule extends Module {
         AddMemberAction.class,
         CreateAction.class,
         DeleteAction.class,
-        MemberUpdater.class,
         RemoveMemberAction.class,
         UpdateAction.class,
         SetMembersSyncAction.class);
index 630f1e7ca9cac016f20f1e3f8713897c64ed0b68..cb678d5738ccb97ccec4a61654311cd9b64a92ba 100644 (file)
@@ -107,7 +107,7 @@ public class HttpHeadersAuthenticationTest {
     db.getDbClient(),
     new UserUpdater(system2, mock(NewUserNotifier.class), db.getDbClient(), userIndexer, organizationFlags, defaultOrganizationProvider, organizationUpdater,
       new DefaultGroupFinder(db.getDbClient()), settings.asConfig(), localAuthentication),
-    defaultOrganizationProvider, organizationFlags, mock(OrganizationUpdater.class), new DefaultGroupFinder(db.getDbClient()));
+    defaultOrganizationProvider, organizationFlags, mock(OrganizationUpdater.class), new DefaultGroupFinder(db.getDbClient()), null);
 
   private HttpServletResponse response = mock(HttpServletResponse.class);
   private JwtHttpHandler jwtHttpHandler = mock(JwtHttpHandler.class);
index 9ceb7ff1846fe46a975b5c12a6c378be78fd57dc..b4a161915908834441023957f0bc98a6b4026cf4 100644 (file)
@@ -37,6 +37,7 @@ import org.sonar.db.user.UserDto;
 import org.sonar.server.authentication.event.AuthenticationEvent;
 import org.sonar.server.authentication.event.AuthenticationException;
 import org.sonar.server.authentication.exception.EmailAlreadyExistsRedirectionException;
+import org.sonar.server.user.ThreadLocalUserSession;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.eq;
@@ -69,10 +70,12 @@ public class OAuth2CallbackFilterTest {
   private FakeOAuth2IdentityProvider oAuth2IdentityProvider = new WellbehaveFakeOAuth2IdentityProvider(OAUTH2_PROVIDER_KEY, true, LOGIN);
   private AuthenticationEvent authenticationEvent = mock(AuthenticationEvent.class);
   private OAuth2AuthenticationParameters oAuthRedirection = mock(OAuth2AuthenticationParameters.class);
+  private ThreadLocalUserSession threadLocalUserSession = mock(ThreadLocalUserSession.class);
 
   private ArgumentCaptor<AuthenticationException> authenticationExceptionCaptor = ArgumentCaptor.forClass(AuthenticationException.class);
 
-  private OAuth2CallbackFilter underTest = new OAuth2CallbackFilter(identityProviderRepository, oAuth2ContextFactory, server, authenticationEvent, oAuthRedirection);
+  private OAuth2CallbackFilter underTest = new OAuth2CallbackFilter(identityProviderRepository, oAuth2ContextFactory, server, authenticationEvent, oAuthRedirection,
+    threadLocalUserSession);
 
   @Before
   public void setUp() throws Exception {
@@ -90,6 +93,8 @@ public class OAuth2CallbackFilterTest {
     when(server.getContextPath()).thenReturn("/sonarqube");
     when(request.getRequestURI()).thenReturn("/sonarqube/oauth2/callback/" + OAUTH2_PROVIDER_KEY);
     identityProviderRepository.addIdentityProvider(oAuth2IdentityProvider);
+    when(threadLocalUserSession.hasSession()).thenReturn(true);
+    when(threadLocalUserSession.getLogin()).thenReturn(LOGIN);
 
     underTest.doFilter(request, response, chain);
 
@@ -119,6 +124,8 @@ public class OAuth2CallbackFilterTest {
   public void do_filter_on_auth2_identity_provider() {
     when(request.getRequestURI()).thenReturn("/oauth2/callback/" + OAUTH2_PROVIDER_KEY);
     identityProviderRepository.addIdentityProvider(oAuth2IdentityProvider);
+    when(threadLocalUserSession.hasSession()).thenReturn(true);
+    when(threadLocalUserSession.getLogin()).thenReturn(LOGIN);
 
     underTest.doFilter(request, response, chain);
 
@@ -202,7 +209,8 @@ public class OAuth2CallbackFilterTest {
 
     underTest.doFilter(request, response, chain);
 
-    verify(response).sendRedirect("/sessions/email_already_exists?email=john%40email.com&login=john.github&provider=failing&existingLogin=john.bitbucket&existingProvider=bitbucket");
+    verify(response)
+      .sendRedirect("/sessions/email_already_exists?email=john%40email.com&login=john.github&provider=failing&existingLogin=john.bitbucket&existingProvider=bitbucket");
     verify(oAuthRedirection).delete(eq(request), eq(response));
   }
 
@@ -227,14 +235,14 @@ public class OAuth2CallbackFilterTest {
     assertThat(oAuth2IdentityProvider.isInitCalled()).isFalse();
   }
 
-  private static class FailWithUnauthorizedExceptionIdProvider extends FailingIdentityProvider  {
+  private static class FailWithUnauthorizedExceptionIdProvider extends FailingIdentityProvider {
     @Override
     public void callback(CallbackContext context) {
       throw new UnauthorizedException("Email john@email.com is already used");
     }
   }
 
-  private static class FailWithIllegalStateException extends FailingIdentityProvider  {
+  private static class FailWithIllegalStateException extends FailingIdentityProvider {
     @Override
     public void callback(CallbackContext context) {
       throw new IllegalStateException("Failure !");
index 9d637514c991b5181b80f7b9f5ba02eb54148a6f..9ab426ca58b3a2531cc46ffcb31a19b7d68c064f 100644 (file)
@@ -19,7 +19,9 @@
  */
 package org.sonar.server.authentication;
 
+import com.google.common.collect.ImmutableSet;
 import java.util.Optional;
+import java.util.Set;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
@@ -32,6 +34,7 @@ import org.sonar.api.platform.Server;
 import org.sonar.api.server.authentication.OAuth2IdentityProvider;
 import org.sonar.api.server.authentication.UserIdentity;
 import org.sonar.db.user.UserDto;
+import org.sonar.server.authentication.OAuth2ContextFactory.OAuthContextImpl;
 import org.sonar.server.authentication.UserRegistration.ExistingEmailStrategy;
 import org.sonar.server.authentication.UserRegistration.UpdateLoginStrategy;
 import org.sonar.server.user.TestUserSessionFactory;
@@ -183,6 +186,16 @@ public class OAuth2ContextFactoryTest {
     assertThat(userIdentityAuthenticator.getAuthenticatorParameters().getUpdateLoginStrategy()).isEqualTo(UpdateLoginStrategy.WARN);
   }
 
+  @Test
+  public void authenticate_with_organization_alm_ids() {
+    OAuthContextImpl callback = (OAuthContextImpl) newCallbackContext();
+    Set<String> organizationAlmIds = ImmutableSet.of("ABCD", "EFGH");
+
+    callback.authenticate(USER_IDENTITY, organizationAlmIds);
+
+    assertThat(userIdentityAuthenticator.getAuthenticatorParameters().getOrganizationAlmIds()).containsAll(organizationAlmIds);
+  }
+
   @Test
   public void redirect_to_home() throws Exception {
     when(server.getContextPath()).thenReturn("");
diff --git a/server/sonar-server/src/test/java/org/sonar/server/authentication/UserRegistrarImplOrgMembershipSyncTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/UserRegistrarImplOrgMembershipSyncTest.java
new file mode 100644 (file)
index 0000000..5a940b3
--- /dev/null
@@ -0,0 +1,254 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2019 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.server.authentication;
+
+import com.google.common.collect.ImmutableSet;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.sonar.api.config.internal.MapSettings;
+import org.sonar.api.resources.Qualifiers;
+import org.sonar.api.resources.ResourceTypes;
+import org.sonar.api.server.authentication.UserIdentity;
+import org.sonar.api.utils.System2;
+import org.sonar.api.utils.internal.AlwaysIncreasingSystem2;
+import org.sonar.core.util.UuidFactoryFast;
+import org.sonar.db.DbTester;
+import org.sonar.db.alm.AlmAppInstallDto;
+import org.sonar.db.component.ResourceTypesRule;
+import org.sonar.db.organization.OrganizationDto;
+import org.sonar.db.user.UserDto;
+import org.sonar.server.authentication.UserRegistration.ExistingEmailStrategy;
+import org.sonar.server.authentication.UserRegistration.UpdateLoginStrategy;
+import org.sonar.server.authentication.event.AuthenticationEvent.Source;
+import org.sonar.server.es.EsTester;
+import org.sonar.server.organization.DefaultOrganizationProvider;
+import org.sonar.server.organization.MemberUpdater;
+import org.sonar.server.organization.OrganizationUpdater;
+import org.sonar.server.organization.OrganizationUpdaterImpl;
+import org.sonar.server.organization.OrganizationValidationImpl;
+import org.sonar.server.organization.TestDefaultOrganizationProvider;
+import org.sonar.server.organization.TestOrganizationFlags;
+import org.sonar.server.permission.PermissionService;
+import org.sonar.server.permission.PermissionServiceImpl;
+import org.sonar.server.user.NewUserNotifier;
+import org.sonar.server.user.UserUpdater;
+import org.sonar.server.user.index.UserIndexer;
+import org.sonar.server.usergroups.DefaultGroupFinder;
+
+import static org.mockito.Mockito.mock;
+import static org.sonar.db.alm.ALM.BITBUCKETCLOUD;
+import static org.sonar.db.alm.ALM.GITHUB;
+import static org.sonar.server.authentication.event.AuthenticationEvent.Method.BASIC;
+
+public class UserRegistrarImplOrgMembershipSyncTest {
+
+  private System2 system2 = new AlwaysIncreasingSystem2();
+
+  private static String USER_LOGIN = "github-johndoo";
+
+  private static UserIdentity USER_IDENTITY = UserIdentity.builder()
+    .setProviderId("ABCD")
+    .setProviderLogin("johndoo")
+    .setLogin(USER_LOGIN)
+    .setName("John")
+    .setEmail("john@email.com")
+    .build();
+
+  private static TestIdentityProvider GITHUB_PROVIDER = new TestIdentityProvider()
+    .setKey("github")
+    .setName("Github")
+    .setEnabled(true)
+    .setAllowsUsersToSignUp(true);
+
+  private static TestIdentityProvider BITBUCKET_PROVIDER = new TestIdentityProvider()
+    .setKey("bitbucket")
+    .setName("Bitbucket")
+    .setEnabled(true)
+    .setAllowsUsersToSignUp(true);
+
+  private MapSettings settings = new MapSettings();
+
+  @Rule
+  public ExpectedException expectedException = ExpectedException.none();
+  @Rule
+  public DbTester db = DbTester.create(new AlwaysIncreasingSystem2());
+  @Rule
+  public EsTester es = EsTester.create();
+  private UserIndexer userIndexer = new UserIndexer(db.getDbClient(), es.client());
+  private DefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(db);
+  private OrganizationUpdater organizationUpdater = mock(OrganizationUpdater.class);
+  private TestOrganizationFlags organizationFlags = TestOrganizationFlags.standalone();
+  private CredentialsLocalAuthentication localAuthentication = new CredentialsLocalAuthentication(db.getDbClient());
+  private UserUpdater userUpdater = new UserUpdater(
+    system2,
+    mock(NewUserNotifier.class),
+    db.getDbClient(),
+    userIndexer,
+    organizationFlags,
+    defaultOrganizationProvider,
+    organizationUpdater,
+    new DefaultGroupFinder(db.getDbClient()),
+    settings.asConfig(),
+    localAuthentication);
+
+  private ResourceTypes resourceTypes = new ResourceTypesRule().setRootQualifiers(Qualifiers.PROJECT);
+  private PermissionService permissionService = new PermissionServiceImpl(resourceTypes);
+  private DefaultGroupFinder defaultGroupFinder = new DefaultGroupFinder(db.getDbClient());
+
+  private UserRegistrarImpl underTest = new UserRegistrarImpl(db.getDbClient(), userUpdater, defaultOrganizationProvider, organizationFlags,
+    new OrganizationUpdaterImpl(db.getDbClient(), mock(System2.class), UuidFactoryFast.getInstance(),
+      new OrganizationValidationImpl(), settings.asConfig(), null, null, null, permissionService),
+    defaultGroupFinder, new MemberUpdater(db.getDbClient(), defaultGroupFinder, userIndexer));
+
+  @Test
+  public void authenticate_new_github_user_syncs_organization() {
+    organizationFlags.setEnabled(true);
+    OrganizationDto organization = db.organizations().insert();
+    db.users().insertDefaultGroup(organization, "Members");
+    AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId()));
+    db.alm().insertOrganizationAlmBinding(organization, gitHubInstall, true);
+
+    underTest.register(UserRegistration.builder()
+      .setUserIdentity(USER_IDENTITY)
+      .setProvider(GITHUB_PROVIDER)
+      .setSource(Source.realm(BASIC, GITHUB_PROVIDER.getName()))
+      .setExistingEmailStrategy(ExistingEmailStrategy.ALLOW)
+      .setUpdateLoginStrategy(UpdateLoginStrategy.ALLOW)
+      .setOrganizationAlmIds(ImmutableSet.of(gitHubInstall.getOwnerId()))
+      .build());
+
+    UserDto user = db.users().selectUserByLogin(USER_LOGIN).get();
+    db.organizations().assertUserIsMemberOfOrganization(organization, user);
+  }
+
+  @Test
+  public void authenticate_new_github_user_does_not_sync_organization_when_no_org_alm_ids_provided() {
+    organizationFlags.setEnabled(true);
+    OrganizationDto organization = db.organizations().insert();
+    db.users().insertDefaultGroup(organization, "Members");
+    AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId()));
+    db.alm().insertOrganizationAlmBinding(organization, gitHubInstall, true);
+
+    underTest.register(UserRegistration.builder()
+      .setUserIdentity(USER_IDENTITY)
+      .setProvider(GITHUB_PROVIDER)
+      .setSource(Source.realm(BASIC, GITHUB_PROVIDER.getName()))
+      .setExistingEmailStrategy(ExistingEmailStrategy.ALLOW)
+      .setUpdateLoginStrategy(UpdateLoginStrategy.ALLOW)
+      .setOrganizationAlmIds(null)
+      .build());
+
+    UserDto user = db.users().selectUserByLogin(USER_LOGIN).get();
+    db.organizations().assertUserIsNotMemberOfOrganization(organization, user);
+  }
+
+  @Test
+  public void authenticate_new_bitbucket_user_does_not_sync_organization() {
+    organizationFlags.setEnabled(true);
+    OrganizationDto organization = db.organizations().insert();
+    db.users().insertDefaultGroup(organization, "Members");
+    AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlmId(BITBUCKETCLOUD.getId()));
+    db.alm().insertOrganizationAlmBinding(organization, gitHubInstall, true);
+
+    underTest.register(UserRegistration.builder()
+      .setUserIdentity(USER_IDENTITY)
+      .setProvider(BITBUCKET_PROVIDER)
+      .setSource(Source.realm(BASIC, BITBUCKET_PROVIDER.getName()))
+      .setExistingEmailStrategy(ExistingEmailStrategy.ALLOW)
+      .setUpdateLoginStrategy(UpdateLoginStrategy.ALLOW)
+      .setOrganizationAlmIds(ImmutableSet.of(gitHubInstall.getOwnerId()))
+      .build());
+
+    UserDto user = db.users().selectUserByLogin(USER_LOGIN).get();
+    db.organizations().assertUserIsNotMemberOfOrganization(organization, user);
+  }
+
+  @Test
+  public void authenticate_new_user_using_unknown_alm_does_not_sync_organization() {
+    organizationFlags.setEnabled(true);
+    OrganizationDto organization = db.organizations().insert();
+    db.users().insertDefaultGroup(organization, "Members");
+    AlmAppInstallDto almAppInstall = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId()));
+    db.alm().insertOrganizationAlmBinding(organization, almAppInstall, true);
+    TestIdentityProvider identityProvider = new TestIdentityProvider()
+      .setKey("unknown")
+      .setName("unknown")
+      .setEnabled(true)
+      .setAllowsUsersToSignUp(true);
+
+    underTest.register(UserRegistration.builder()
+      .setUserIdentity(USER_IDENTITY)
+      .setProvider(identityProvider)
+      .setSource(Source.realm(BASIC, identityProvider.getName()))
+      .setExistingEmailStrategy(ExistingEmailStrategy.ALLOW)
+      .setUpdateLoginStrategy(UpdateLoginStrategy.ALLOW)
+      .setOrganizationAlmIds(ImmutableSet.of(almAppInstall.getOwnerId()))
+      .build());
+
+    UserDto user = db.users().selectUserByLogin(USER_LOGIN).get();
+    db.organizations().assertUserIsNotMemberOfOrganization(organization, user);
+  }
+
+  @Test
+  public void authenticate_existing_github_user_does_not_sync_organization() {
+    organizationFlags.setEnabled(true);
+    OrganizationDto organization = db.organizations().insert();
+    db.users().insertDefaultGroup(organization, "Members");
+    AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId()));
+    db.alm().insertOrganizationAlmBinding(organization, gitHubInstall, true);
+    UserDto user = db.users().insertUser(u -> u
+      .setLogin("Old login")
+      .setExternalId(USER_IDENTITY.getProviderId())
+      .setExternalIdentityProvider(GITHUB_PROVIDER.getKey()));
+
+    underTest.register(UserRegistration.builder()
+      .setUserIdentity(USER_IDENTITY)
+      .setProvider(GITHUB_PROVIDER)
+      .setSource(Source.local(BASIC))
+      .setExistingEmailStrategy(ExistingEmailStrategy.FORBID)
+      .setUpdateLoginStrategy(UpdateLoginStrategy.ALLOW)
+      .setOrganizationAlmIds(ImmutableSet.of(gitHubInstall.getOwnerId()))
+      .build());
+
+    db.organizations().assertUserIsNotMemberOfOrganization(organization, user);
+  }
+
+  @Test
+  public void authenticate_disabled_github_user_syncs_organization() {
+    organizationFlags.setEnabled(true);
+    OrganizationDto organization = db.organizations().insert();
+    db.users().insertDefaultGroup(organization, "Members");
+    AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId()));
+    db.alm().insertOrganizationAlmBinding(organization, gitHubInstall, true);
+    UserDto user = db.users().insertDisabledUser(u -> u.setLogin(USER_LOGIN));
+
+    underTest.register(UserRegistration.builder()
+      .setUserIdentity(USER_IDENTITY)
+      .setProvider(GITHUB_PROVIDER)
+      .setSource(Source.local(BASIC))
+      .setExistingEmailStrategy(ExistingEmailStrategy.FORBID)
+      .setUpdateLoginStrategy(UpdateLoginStrategy.ALLOW)
+      .setOrganizationAlmIds(ImmutableSet.of(gitHubInstall.getOwnerId()))
+      .build());
+
+    db.organizations().assertUserIsMemberOfOrganization(organization, user);
+  }
+}
index ade40994d778ee580bfe9bacfcee171268bf37f2..32f6851b8e7fa13e9543d8061bd0c78730bdc27f 100644 (file)
@@ -45,6 +45,7 @@ import org.sonar.server.authentication.exception.EmailAlreadyExistsRedirectionEx
 import org.sonar.server.authentication.exception.UpdateLoginRedirectionException;
 import org.sonar.server.es.EsTester;
 import org.sonar.server.organization.DefaultOrganizationProvider;
+import org.sonar.server.organization.MemberUpdater;
 import org.sonar.server.organization.OrganizationUpdater;
 import org.sonar.server.organization.OrganizationUpdaterImpl;
 import org.sonar.server.organization.OrganizationValidationImpl;
@@ -114,11 +115,12 @@ public class UserRegistrarImplTest {
 
   private ResourceTypes resourceTypes = new ResourceTypesRule().setRootQualifiers(Qualifiers.PROJECT);
   private PermissionService permissionService = new PermissionServiceImpl(resourceTypes);
+  private DefaultGroupFinder defaultGroupFinder = new DefaultGroupFinder(db.getDbClient());
 
   private UserRegistrarImpl underTest = new UserRegistrarImpl(db.getDbClient(), userUpdater, defaultOrganizationProvider, organizationFlags,
     new OrganizationUpdaterImpl(db.getDbClient(), mock(System2.class), UuidFactoryFast.getInstance(),
       new OrganizationValidationImpl(), settings.asConfig(), null, null, null, permissionService),
-    new DefaultGroupFinder(db.getDbClient()));
+    defaultGroupFinder, new MemberUpdater(db.getDbClient(), defaultGroupFinder, userIndexer));
 
   @Test
   public void authenticate_new_user() {
index c89dc36b94d84f6916dfae31acb88fef398d848f..b9f11ab104786031cbc323dc547615541acd185f 100644 (file)
@@ -19,6 +19,7 @@
  */
 package org.sonar.server.organization;
 
+import com.google.common.collect.ImmutableSet;
 import java.util.HashSet;
 import javax.annotation.Nullable;
 import org.assertj.core.groups.Tuple;
@@ -29,6 +30,7 @@ import org.junit.rules.ExpectedException;
 import org.sonar.api.utils.System2;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbTester;
+import org.sonar.db.alm.AlmAppInstallDto;
 import org.sonar.db.component.ComponentDto;
 import org.sonar.db.organization.OrganizationDto;
 import org.sonar.db.permission.OrganizationPermission;
@@ -38,8 +40,6 @@ import org.sonar.db.property.PropertyDto;
 import org.sonar.db.property.PropertyQuery;
 import org.sonar.db.qualityprofile.QProfileDto;
 import org.sonar.db.user.GroupDto;
-import org.sonar.db.user.GroupMembershipDto;
-import org.sonar.db.user.GroupMembershipQuery;
 import org.sonar.db.user.UserDto;
 import org.sonar.server.es.EsTester;
 import org.sonar.server.es.SearchOptions;
@@ -60,9 +60,9 @@ import static org.elasticsearch.index.query.QueryBuilders.termQuery;
 import static org.sonar.api.CoreProperties.DEFAULT_ISSUE_ASSIGNEE;
 import static org.sonar.api.web.UserRole.CODEVIEWER;
 import static org.sonar.api.web.UserRole.USER;
+import static org.sonar.db.alm.ALM.GITHUB;
 import static org.sonar.db.permission.OrganizationPermission.ADMINISTER;
 import static org.sonar.db.permission.OrganizationPermission.SCAN;
-import static org.sonar.db.user.GroupMembershipQuery.IN;
 import static org.sonar.server.user.index.UserIndexDefinition.FIELD_ORGANIZATION_UUIDS;
 import static org.sonar.server.user.index.UserIndexDefinition.FIELD_UUID;
 
@@ -88,7 +88,7 @@ public class MemberUpdaterTest {
 
     underTest.addMember(db.getSession(), organization, user);
 
-    assertUserIsMember(organization, user);
+    db.organizations().assertUserIsMemberOfOrganization(organization, user);
     assertThat(userIndex.search(UserQuery.builder().build(), new SearchOptions()).getDocs())
       .extracting(UserDoc::login, UserDoc::organizationUuids)
       .containsExactlyInAnyOrder(tuple(user.getLogin(), singletonList(organization.getUuid())));
@@ -101,11 +101,11 @@ public class MemberUpdaterTest {
     UserDto user = db.users().insertUser();
     db.organizations().addMember(organization, user);
     db.users().insertMember(defaultGroup, user);
-    assertUserIsMember(organization, user);
+    db.organizations().assertUserIsMemberOfOrganization(organization, user);
 
     underTest.addMember(db.getSession(), organization, user);
 
-    assertUserIsMember(organization, user);
+    db.organizations().assertUserIsMemberOfOrganization(organization, user);
   }
 
   @Test
@@ -129,8 +129,8 @@ public class MemberUpdaterTest {
 
     underTest.addMembers(db.getSession(), organization, asList(user1, user2, disableUser));
 
-    assertUserIsMember(organization, user1);
-    assertUserIsMember(organization, user2);
+    db.organizations().assertUserIsMemberOfOrganization(organization, user1);
+    db.organizations().assertUserIsMemberOfOrganization(organization, user2);
     assertUserIsNotMember(organization, disableUser);
     assertThat(userIndex.search(UserQuery.builder().build(), new SearchOptions()).getDocs())
       .extracting(UserDoc::login, UserDoc::organizationUuids)
@@ -151,8 +151,8 @@ public class MemberUpdaterTest {
 
     underTest.addMembers(db.getSession(), organization, asList(userAlreadyMember, userNotMember));
 
-    assertUserIsMember(organization, userAlreadyMember);
-    assertUserIsMember(organization, userNotMember);
+    db.organizations().assertUserIsMemberOfOrganization(organization, userAlreadyMember);
+    db.organizations().assertUserIsMemberOfOrganization(organization, userNotMember);
     assertThat(userIndex.search(UserQuery.builder().build(), new SearchOptions()).getDocs())
       .extracting(UserDoc::login, UserDoc::organizationUuids)
       .containsExactlyInAnyOrder(
@@ -192,7 +192,7 @@ public class MemberUpdaterTest {
 
     assertUserIsNotMember(organization, user1);
     assertUserIsNotMember(organization, user2);
-    assertUserIsMember(organization, adminUser);
+    db.organizations().assertUserIsMemberOfOrganization(organization, adminUser);
   }
 
   @Test
@@ -400,19 +400,95 @@ public class MemberUpdaterTest {
     underTest.removeMembers(db.getSession(), organization, asList(admin1, admin2));
   }
 
-  private void assertUserIsMember(OrganizationDto organization, UserDto user) {
-    assertThat(dbClient.organizationMemberDao().select(db.getSession(), organization.getUuid(), user.getId())).isPresent();
-    Integer defaultGroupId = dbClient.organizationDao().getDefaultGroupId(db.getSession(), organization.getUuid()).get();
-    assertThat(db.getDbClient().groupMembershipDao().selectGroups(
-      db.getSession(),
-      GroupMembershipQuery.builder().membership(IN).organizationUuid(organization.getUuid()).build(),
-      user.getId(), 0, 10))
-        .extracting(GroupMembershipDto::getId)
-        .containsOnly(defaultGroupId.longValue());
+  @Test
+  public void synchronize_user_organization_membership() {
+    OrganizationDto organization1 = db.organizations().insert();
+    GroupDto org1defaultGroup = db.users().insertDefaultGroup(organization1, "Members");
+    AlmAppInstallDto gitHubInstall1 = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId()));
+    db.alm().insertOrganizationAlmBinding(organization1, gitHubInstall1, true);
+    OrganizationDto organization2 = db.organizations().insert();
+    db.users().insertDefaultGroup(organization2, "Members");
+    AlmAppInstallDto gitHubInstall2 = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId()));
+    db.alm().insertOrganizationAlmBinding(organization2, gitHubInstall2, true);
+    OrganizationDto organization3 = db.organizations().insert();
+    GroupDto org3defaultGroup = db.users().insertDefaultGroup(organization3, "Members");
+    AlmAppInstallDto gitHubInstall3 = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId()));
+    db.alm().insertOrganizationAlmBinding(organization3, gitHubInstall3, true);
+    // User is member of organization1 and organization3, but organization3 membership will be removed and organization2 membership will be
+    // added
+    UserDto user = db.users().insertUser();
+    db.organizations().addMember(organization1, user);
+    db.users().insertMember(org1defaultGroup, user);
+    db.organizations().addMember(organization3, user);
+    db.users().insertMember(org3defaultGroup, user);
+
+    underTest.synchronizeUserOrganizationMembership(db.getSession(), user, GITHUB, ImmutableSet.of(gitHubInstall1.getOwnerId(), gitHubInstall2.getOwnerId()));
+
+    db.organizations().assertUserIsMemberOfOrganization(organization1, user);
+    db.organizations().assertUserIsMemberOfOrganization(organization2, user);
+    assertUserIsNotMember(organization3, user);
+  }
+
+  @Test
+  public void synchronize_user_organization_membership_does_not_update_es_index() {
+    OrganizationDto organization = db.organizations().insert();
+    db.users().insertDefaultGroup(organization, "Members");
+    AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId()));
+    db.alm().insertOrganizationAlmBinding(organization, gitHubInstall, true);
+    UserDto user = db.users().insertUser();
+
+    underTest.synchronizeUserOrganizationMembership(db.getSession(), user, GITHUB, ImmutableSet.of(gitHubInstall.getOwnerId()));
+
+    assertThat(userIndex.search(UserQuery.builder().build(), new SearchOptions()).getDocs()).isEmpty();
+  }
+
+  @Test
+  public void synchronize_user_organization_membership_ignores_organization_alm_ids_match_no_existing_organizations() {
+    OrganizationDto organization = db.organizations().insert();
+    db.users().insertDefaultGroup(organization, "Members");
+    AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId()));
+    db.alm().insertOrganizationAlmBinding(organization, gitHubInstall, true);
+    UserDto user = db.users().insertUser();
+
+    underTest.synchronizeUserOrganizationMembership(db.getSession(), user, GITHUB, ImmutableSet.of("unknown"));
+
+    // User is member of no organization
+    assertThat(db.getDbClient().organizationMemberDao().selectOrganizationUuidsByUser(db.getSession(), user.getId())).isEmpty();
+  }
+
+  @Test
+  public void synchronize_user_organization_membership_ignores_organization_with_member_sync_disabled() {
+    OrganizationDto organization = db.organizations().insert();
+    db.users().insertDefaultGroup(organization, "Members");
+    AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId()));
+    db.alm().insertOrganizationAlmBinding(organization, gitHubInstall, false);
+    UserDto user = db.users().insertUser();
+
+    underTest.synchronizeUserOrganizationMembership(db.getSession(), user, GITHUB, ImmutableSet.of(gitHubInstall.getOwnerId()));
+
+    db.organizations().assertUserIsNotMemberOfOrganization(organization, user);
+  }
+
+  @Test
+  public void synchronize_user_organization_membership_does_not_remove_existing_membership_on_organization_with_member_sync_disabled() {
+    OrganizationDto organization = db.organizations().insert();
+    GroupDto org1defaultGroup = db.users().insertDefaultGroup(organization, "Members");
+    AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId()));
+    db.alm().insertOrganizationAlmBinding(organization, gitHubInstall, false);
+    UserDto user = db.users().insertUser();
+    db.users().insertMember(org1defaultGroup, user);
+    db.organizations().addMember(organization, user);
+    // User is member of a organization on which member sync is disabled
+    db.organizations().assertUserIsMemberOfOrganization(organization, user);
+
+    // The organization is not in the list, but membership should not be removed
+    underTest.synchronizeUserOrganizationMembership(db.getSession(), user, GITHUB, ImmutableSet.of("other"));
+
+    db.organizations().assertUserIsMemberOfOrganization(organization, user);
   }
 
   private void assertUserIsNotMember(OrganizationDto organization, UserDto user) {
-    assertThat(dbClient.organizationMemberDao().select(db.getSession(), organization.getUuid(), user.getId())).isNotPresent();
+    db.organizations().assertUserIsNotMemberOfOrganization(organization, user);
     SearchRequestBuilder request = es.client().prepareSearch(UserIndexDefinition.INDEX_TYPE_USER)
       .setQuery(boolQuery()
         .must(termQuery(FIELD_ORGANIZATION_UUIDS, organization.getUuid()))
index 8f8b36f71d4d0f127b94de5d979ef5721c7d143b..d6023ac7374e1aa4616d55b1dfbfa8071e4547fc 100644 (file)
@@ -39,7 +39,7 @@ public class OrganizationsWsModuleTest {
     underTest.configure(container);
 
     assertThat(container.getPicoContainer().getComponentAdapters())
-      .hasSize(COMPONENTS_IN_EMPTY_COMPONENT_CONTAINER + 4);
+      .hasSize(COMPONENTS_IN_EMPTY_COMPONENT_CONTAINER + 5);
   }
 
   @Test