From 03417898ca26ce1f047c3c890ffa66092d9fd9c6 Mon Sep 17 00:00:00 2001 From: Florian Scholdei Date: Wed, 28 Oct 2020 15:29:54 +0100 Subject: [PATCH 1/6] Create _authenticated group at startup --- .../scm/lifecycle/SetupContextListener.java | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/scm-webapp/src/main/java/sonia/scm/lifecycle/SetupContextListener.java b/scm-webapp/src/main/java/sonia/scm/lifecycle/SetupContextListener.java index af95c09aa8..eade2c2783 100644 --- a/scm-webapp/src/main/java/sonia/scm/lifecycle/SetupContextListener.java +++ b/scm-webapp/src/main/java/sonia/scm/lifecycle/SetupContextListener.java @@ -30,6 +30,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.SCMContext; import sonia.scm.config.ScmConfiguration; +import sonia.scm.group.Group; +import sonia.scm.group.GroupManager; import sonia.scm.plugin.Extension; import sonia.scm.security.AnonymousMode; import sonia.scm.security.PermissionAssigner; @@ -44,6 +46,8 @@ import javax.servlet.ServletContextEvent; import javax.servlet.ServletContextListener; import java.util.Collections; +import static sonia.scm.group.GroupCollector.AUTHENTICATED; + @Extension public class SetupContextListener implements ServletContextListener { @@ -75,13 +79,17 @@ public class SetupContextListener implements ServletContextListener { private final PasswordService passwordService; private final PermissionAssigner permissionAssigner; private final ScmConfiguration scmConfiguration; + private final GroupManager groupManager; + + private static final String AUTHENTICATED_GROUP_DESCRIPTION = "Includes all authenticated users"; @Inject - public SetupAction(UserManager userManager, PasswordService passwordService, PermissionAssigner permissionAssigner, ScmConfiguration scmConfiguration) { + public SetupAction(UserManager userManager, PasswordService passwordService, PermissionAssigner permissionAssigner, ScmConfiguration scmConfiguration, GroupManager groupManager) { this.userManager = userManager; this.passwordService = passwordService; this.permissionAssigner = permissionAssigner; this.scmConfiguration = scmConfiguration; + this.groupManager = groupManager; } @Override @@ -92,6 +100,10 @@ public class SetupContextListener implements ServletContextListener { if (anonymousUserRequiredButNotExists()) { userManager.create(SCMContext.ANONYMOUS); } + + if (authenticatedGroupDoesNotExists()) { + createAuthenticatedGroup(); + } } private boolean anonymousUserRequiredButNotExists() { @@ -115,5 +127,15 @@ public class SetupContextListener implements ServletContextListener { PermissionDescriptor descriptor = new PermissionDescriptor("*"); permissionAssigner.setPermissionsForUser("scmadmin", Collections.singleton(descriptor)); } + + private boolean authenticatedGroupDoesNotExists() { + return (groupManager.get(AUTHENTICATED) == null); + } + + private void createAuthenticatedGroup() { + Group authenticated = new Group("xml", AUTHENTICATED); + authenticated.setDescription(AUTHENTICATED_GROUP_DESCRIPTION); + groupManager.create(authenticated); + } } } From 526405f1648a516b80b1fd10ec4a24c06633ced7 Mon Sep 17 00:00:00 2001 From: Florian Scholdei Date: Wed, 28 Oct 2020 17:55:31 +0100 Subject: [PATCH 2/6] Update CHANGELOG.md --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cbd392b0f..ad2cd04b14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## Unreleased +### Added +- Create _authenticated group on setup ([#1396](https://github.com/scm-manager/scm-manager/pull/1396)) + ### Fixed - Do not expose subversion commit with id 0 ([#1395](https://github.com/scm-manager/scm-manager/pull/1395)) From 81dc317f3cc950ab1c1d324a655a570711b93510 Mon Sep 17 00:00:00 2001 From: Florian Scholdei Date: Wed, 28 Oct 2020 17:58:58 +0100 Subject: [PATCH 3/6] Exclude null case if groupManager is not set --- .../src/main/java/sonia/scm/lifecycle/SetupContextListener.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scm-webapp/src/main/java/sonia/scm/lifecycle/SetupContextListener.java b/scm-webapp/src/main/java/sonia/scm/lifecycle/SetupContextListener.java index eade2c2783..00cf5ffc5a 100644 --- a/scm-webapp/src/main/java/sonia/scm/lifecycle/SetupContextListener.java +++ b/scm-webapp/src/main/java/sonia/scm/lifecycle/SetupContextListener.java @@ -129,7 +129,7 @@ public class SetupContextListener implements ServletContextListener { } private boolean authenticatedGroupDoesNotExists() { - return (groupManager.get(AUTHENTICATED) == null); + return groupManager == null || groupManager.get(AUTHENTICATED) == null; } private void createAuthenticatedGroup() { From c87fa5b3e530981e6768be5133aa0298595ed13f Mon Sep 17 00:00:00 2001 From: Florian Scholdei Date: Tue, 3 Nov 2020 08:58:06 +0100 Subject: [PATCH 4/6] Revert "Exclude null case if groupManager is not set" This reverts commit 81dc317f --- .../src/main/java/sonia/scm/lifecycle/SetupContextListener.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scm-webapp/src/main/java/sonia/scm/lifecycle/SetupContextListener.java b/scm-webapp/src/main/java/sonia/scm/lifecycle/SetupContextListener.java index 00cf5ffc5a..6bfda56c95 100644 --- a/scm-webapp/src/main/java/sonia/scm/lifecycle/SetupContextListener.java +++ b/scm-webapp/src/main/java/sonia/scm/lifecycle/SetupContextListener.java @@ -129,7 +129,7 @@ public class SetupContextListener implements ServletContextListener { } private boolean authenticatedGroupDoesNotExists() { - return groupManager == null || groupManager.get(AUTHENTICATED) == null; + return groupManager.get(AUTHENTICATED) == null; } private void createAuthenticatedGroup() { From f1115099c954e729a50f5f6cd0c0ff317ecec61c Mon Sep 17 00:00:00 2001 From: Florian Scholdei Date: Tue, 3 Nov 2020 10:45:47 +0100 Subject: [PATCH 5/6] Create unit tests, supplement missing groupManager precondition, add createAuthenticatedGroup factory --- .../scm/lifecycle/SetupContextListener.java | 3 +- .../lifecycle/SetupContextListenerTest.java | 38 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/scm-webapp/src/main/java/sonia/scm/lifecycle/SetupContextListener.java b/scm-webapp/src/main/java/sonia/scm/lifecycle/SetupContextListener.java index 6bfda56c95..30d2c7a7f2 100644 --- a/scm-webapp/src/main/java/sonia/scm/lifecycle/SetupContextListener.java +++ b/scm-webapp/src/main/java/sonia/scm/lifecycle/SetupContextListener.java @@ -81,7 +81,8 @@ public class SetupContextListener implements ServletContextListener { private final ScmConfiguration scmConfiguration; private final GroupManager groupManager; - private static final String AUTHENTICATED_GROUP_DESCRIPTION = "Includes all authenticated users"; + @VisibleForTesting + static final String AUTHENTICATED_GROUP_DESCRIPTION = "Includes all authenticated users"; @Inject public SetupAction(UserManager userManager, PasswordService passwordService, PermissionAssigner permissionAssigner, ScmConfiguration scmConfiguration, GroupManager groupManager) { diff --git a/scm-webapp/src/test/java/sonia/scm/lifecycle/SetupContextListenerTest.java b/scm-webapp/src/test/java/sonia/scm/lifecycle/SetupContextListenerTest.java index fab7b789ad..d0376f53a3 100644 --- a/scm-webapp/src/test/java/sonia/scm/lifecycle/SetupContextListenerTest.java +++ b/scm-webapp/src/test/java/sonia/scm/lifecycle/SetupContextListenerTest.java @@ -37,6 +37,8 @@ import org.mockito.junit.jupiter.MockitoSettings; import org.mockito.quality.Strictness; import sonia.scm.SCMContext; import sonia.scm.config.ScmConfiguration; +import sonia.scm.group.Group; +import sonia.scm.group.GroupManager; import sonia.scm.security.AnonymousMode; import sonia.scm.security.PermissionAssigner; import sonia.scm.security.PermissionDescriptor; @@ -56,6 +58,8 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static sonia.scm.group.GroupCollector.AUTHENTICATED; +import static sonia.scm.lifecycle.SetupContextListener.SetupAction.AUTHENTICATED_GROUP_DESCRIPTION; @ExtendWith(MockitoExtension.class) class SetupContextListenerTest { @@ -75,6 +79,9 @@ class SetupContextListenerTest { @Mock ScmConfiguration scmConfiguration; + @Mock + private GroupManager groupManager; + @Mock private PermissionAssigner permissionAssigner; @@ -96,6 +103,7 @@ class SetupContextListenerTest { @Test void shouldCreateAdminAccountIfNoUserExistsAndAssignPermissions() { + when(groupManager.get(AUTHENTICATED)).thenReturn(createAuthenticatedGroup()); when(passwordService.encryptPassword("scmadmin")).thenReturn("secret"); setupContextListener.contextInitialized(null); @@ -108,6 +116,7 @@ class SetupContextListenerTest { void shouldCreateAdminAccountIfOnlyAnonymousUserExistsAndAssignPermissions() { when(userManager.getAll()).thenReturn(Lists.newArrayList(SCMContext.ANONYMOUS)); when(userManager.contains(SCMContext.USER_ANONYMOUS)).thenReturn(true); + when(groupManager.get(AUTHENTICATED)).thenReturn(createAuthenticatedGroup()); when(passwordService.encryptPassword("scmadmin")).thenReturn("secret"); setupContextListener.contextInitialized(null); @@ -135,6 +144,7 @@ class SetupContextListenerTest { void shouldDoNothingOnSecondStart() { List users = Lists.newArrayList(UserTestData.createTrillian()); when(userManager.getAll()).thenReturn(users); + when(groupManager.get(AUTHENTICATED)).thenReturn(createAuthenticatedGroup()); setupContextListener.contextInitialized(null); @@ -146,6 +156,7 @@ class SetupContextListenerTest { void shouldCreateAnonymousUserIfRequired() { List users = Lists.newArrayList(UserTestData.createTrillian()); when(userManager.getAll()).thenReturn(users); + when(groupManager.get(AUTHENTICATED)).thenReturn(createAuthenticatedGroup()); when(scmConfiguration.getAnonymousMode()).thenReturn(AnonymousMode.FULL); setupContextListener.contextInitialized(null); @@ -157,6 +168,7 @@ class SetupContextListenerTest { void shouldNotCreateAnonymousUserIfNotRequired() { List users = Lists.newArrayList(UserTestData.createTrillian()); when(userManager.getAll()).thenReturn(users); + when(groupManager.get(AUTHENTICATED)).thenReturn(createAuthenticatedGroup()); setupContextListener.contextInitialized(null); @@ -167,6 +179,7 @@ class SetupContextListenerTest { void shouldNotCreateAnonymousUserIfAlreadyExists() { List users = Lists.newArrayList(SCMContext.ANONYMOUS); when(userManager.getAll()).thenReturn(users); + when(groupManager.get(AUTHENTICATED)).thenReturn(createAuthenticatedGroup()); when(scmConfiguration.getAnonymousMode()).thenReturn(AnonymousMode.FULL); setupContextListener.contextInitialized(null); @@ -174,6 +187,28 @@ class SetupContextListenerTest { verify(userManager, times(1)).create(SCMContext.ANONYMOUS); } + @Test + void shouldCreateAuthenticatedGroupIfMissing() { + when(groupManager.get(AUTHENTICATED)).thenReturn(null); + + setupContextListener.contextInitialized(null); + + Group authenticated = createAuthenticatedGroup(); + authenticated.setDescription(AUTHENTICATED_GROUP_DESCRIPTION); + authenticated.setExternal(true); + + verify(groupManager, times(1)).create(createAuthenticatedGroup()); + } + + @Test + void shouldNotCreateAuthenticatedGroupIfAlreadyExists() { + when(groupManager.get(AUTHENTICATED)).thenReturn(createAuthenticatedGroup()); + + setupContextListener.contextInitialized(null); + + verify(groupManager, never()).create(any()); + } + private void verifyAdminPermissionsAssigned() { ArgumentCaptor usernameCaptor = ArgumentCaptor.forClass(String.class); ArgumentCaptor> permissionCaptor = ArgumentCaptor.forClass(Collection.class); @@ -192,4 +227,7 @@ class SetupContextListenerTest { assertThat(user.getPassword()).isEqualTo("secret"); } + private Group createAuthenticatedGroup() { + return new Group("xml", AUTHENTICATED); + } } From bfa917859a3d497c5e91e8dd48a7c72c78f81e8f Mon Sep 17 00:00:00 2001 From: Florian Scholdei Date: Tue, 3 Nov 2020 10:57:46 +0100 Subject: [PATCH 6/6] Make authenticatedGroup external, fix small flaw --- .../src/main/java/sonia/scm/lifecycle/SetupContextListener.java | 1 + .../test/java/sonia/scm/lifecycle/SetupContextListenerTest.java | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/scm-webapp/src/main/java/sonia/scm/lifecycle/SetupContextListener.java b/scm-webapp/src/main/java/sonia/scm/lifecycle/SetupContextListener.java index 30d2c7a7f2..1f47f03686 100644 --- a/scm-webapp/src/main/java/sonia/scm/lifecycle/SetupContextListener.java +++ b/scm-webapp/src/main/java/sonia/scm/lifecycle/SetupContextListener.java @@ -136,6 +136,7 @@ public class SetupContextListener implements ServletContextListener { private void createAuthenticatedGroup() { Group authenticated = new Group("xml", AUTHENTICATED); authenticated.setDescription(AUTHENTICATED_GROUP_DESCRIPTION); + authenticated.setExternal(true); groupManager.create(authenticated); } } diff --git a/scm-webapp/src/test/java/sonia/scm/lifecycle/SetupContextListenerTest.java b/scm-webapp/src/test/java/sonia/scm/lifecycle/SetupContextListenerTest.java index d0376f53a3..e6984ff537 100644 --- a/scm-webapp/src/test/java/sonia/scm/lifecycle/SetupContextListenerTest.java +++ b/scm-webapp/src/test/java/sonia/scm/lifecycle/SetupContextListenerTest.java @@ -197,7 +197,7 @@ class SetupContextListenerTest { authenticated.setDescription(AUTHENTICATED_GROUP_DESCRIPTION); authenticated.setExternal(true); - verify(groupManager, times(1)).create(createAuthenticatedGroup()); + verify(groupManager, times(1)).create(authenticated); } @Test