diff --git a/docs/en/user/admin/settings.md b/docs/en/user/admin/settings.md index bf23e88cb3..527716f113 100644 --- a/docs/en/user/admin/settings.md +++ b/docs/en/user/admin/settings.md @@ -36,7 +36,7 @@ Example: If anonymous access is enabled and the "_anonymous" user has full acces The url of the RSS Release Feed for SCM-Manager. This provides up-to-date version information. To disable this feature just leave the url blank. #### User converter -Internal users will automatically be converted to external on their first login using an external system. After convertion the users may only log in using the external system. +Internal users will automatically be converted to external on their first login using an external system. After conversion the users may only log in using the external system. #### Fallback Mail Domain Name This domain name will be used to create email addresses for users without one when needed. It will not be used to send mails nor will be accessed otherwise. diff --git a/docs/en/user/user/index.md b/docs/en/user/user/index.md index 4d7edfc18a..927a656c5a 100644 --- a/docs/en/user/user/index.md +++ b/docs/en/user/user/index.md @@ -19,6 +19,6 @@ The "Create User" form can be used to create new users in SCM-Manager. New users ### User Details Page The user details page shows the information about the user. -The active box shows whether the user is able to use SCM-Manager. The external box shows if it is an internal user or is managed by an external system. +The active box shows whether the user is able to use SCM-Manager. The external box shows if it is an internal user or whether it is managed by an external system. ![User-Information](assets/user-information.png) diff --git a/docs/en/user/user/settings.md b/docs/en/user/user/settings.md index 8bacefd8e6..9b97bad2c8 100644 --- a/docs/en/user/user/settings.md +++ b/docs/en/user/user/settings.md @@ -5,7 +5,7 @@ subtitle: Settings ### General In the general settings the display name, e-mail address, external flag and active status of an account can be edited. -If an user is converted from internal to external the passwords will be removed. Switching an external user to internal user a password must be set using the password modal dialogue. +If a user is converted from internal to external the password is going to be removed. When switching an external user to an internal one, a password must be set using the password modal dialogue. ![User Password Modal](assets/user-password-modal.png) diff --git a/scm-core/src/main/java/sonia/scm/security/SyncingRealmHelper.java b/scm-core/src/main/java/sonia/scm/security/SyncingRealmHelper.java index 7b8d82f557..9a4ba8294e 100644 --- a/scm-core/src/main/java/sonia/scm/security/SyncingRealmHelper.java +++ b/scm-core/src/main/java/sonia/scm/security/SyncingRealmHelper.java @@ -80,6 +80,7 @@ public final class SyncingRealmHelper { * @param ctx administration context * @param userManager user manager * @param groupManager group manager + * @deprecated */ @Deprecated public SyncingRealmHelper(AdministrationContext ctx, UserManager userManager, GroupManager groupManager) { @@ -110,17 +111,9 @@ public final class SyncingRealmHelper { public void store(final Group group) { ctx.runAsAdmin(() -> { if (groupManager.get(group.getId()) != null) { - try { - groupManager.modify(group); - } catch (NotFoundException e) { - throw new IllegalStateException("got NotFoundException though group " + group.getName() + " could be loaded", e); - } + modifyGroup(group); } else { - try { - groupManager.create(group); - } catch (AlreadyExistsException e) { - throw new IllegalStateException("got AlreadyExistsException though group " + group.getName() + " could not be loaded", e); - } + createNewGroup(group); } }); } @@ -133,30 +126,54 @@ public final class SyncingRealmHelper { public void store(final User user) { ctx.runAsAdmin(() -> { if (userManager.contains(user.getName())) { - User clone = user.clone(); - if (!externalUserConverters.isEmpty()) { - log.debug("execute available user converters"); - for (ExternalUserConverter converter : externalUserConverters) { - clone = converter.convert(clone); - } - } - - try { - userManager.modify(clone); - } catch (NotFoundException e) { - throw new IllegalStateException("got NotFoundException though user " + clone.getName() + " could be loaded", e); - } + modifyUser(user); } else { - try { - User clone = user.clone(); - // New user created by syncing realm helper is always external - clone.setExternal(true); - userManager.create(clone); - } catch (AlreadyExistsException e) { - throw new IllegalStateException("got AlreadyExistsException though user " + user.getName() + " could not be loaded", e); - - } + createNewUser(user); } }); } + + private void createNewUser(User user) { + try { + User clone = user.clone(); + // New user created by syncing realm helper is always external + clone.setExternal(true); + userManager.create(clone); + } catch (AlreadyExistsException e) { + throw new IllegalStateException("got AlreadyExistsException though user " + user.getName() + " could not be loaded", e); + + } + } + + private void modifyUser(User user) { + User clone = user.clone(); + if (!externalUserConverters.isEmpty()) { + log.debug("execute available user converters"); + for (ExternalUserConverter converter : externalUserConverters) { + clone = converter.convert(clone); + } + } + + try { + userManager.modify(clone); + } catch (NotFoundException e) { + throw new IllegalStateException("got NotFoundException though user " + clone.getName() + " could be loaded", e); + } + } + + private void createNewGroup(Group group) { + try { + groupManager.create(group); + } catch (AlreadyExistsException e) { + throw new IllegalStateException("got AlreadyExistsException though group " + group.getName() + " could not be loaded", e); + } + } + + private void modifyGroup(Group group) { + try { + groupManager.modify(group); + } catch (NotFoundException e) { + throw new IllegalStateException("got NotFoundException though group " + group.getName() + " could be loaded", e); + } + } } diff --git a/scm-core/src/main/java/sonia/scm/user/User.java b/scm-core/src/main/java/sonia/scm/user/User.java index 29ab1ba9c7..9eb7474a67 100644 --- a/scm-core/src/main/java/sonia/scm/user/User.java +++ b/scm-core/src/main/java/sonia/scm/user/User.java @@ -69,9 +69,9 @@ public class User extends BasicPropertiesAware implements Principal, ModelObject private String password; @Deprecated /** - * @deprecated Use external instead. * The user type is replaced by {@link external} flag - * Since 2.8.0 + * @since 2.8.0 + * @deprecated Use {@link external} instead. */ private String type; diff --git a/scm-ui/ui-webapp/src/users/components/UserForm.tsx b/scm-ui/ui-webapp/src/users/components/UserForm.tsx index 366a05633e..c93e074dd1 100644 --- a/scm-ui/ui-webapp/src/users/components/UserForm.tsx +++ b/scm-ui/ui-webapp/src/users/components/UserForm.tsx @@ -128,10 +128,10 @@ class UserForm extends React.Component { const { user, passwordValid } = this.state; event.preventDefault(); if (!this.isInvalid()) { + this.props.submitForm(this.state.user); if (user.password && passwordValid && user._links?.password) { setPassword((user._links.password as Link).href, user.password).catch(error => this.setState({ error })); } - this.props.submitForm(this.state.user); } }; @@ -208,7 +208,7 @@ class UserForm extends React.Component { diff --git a/scm-ui/ui-webapp/src/users/components/navLinks/SetPasswordNavLink.tsx b/scm-ui/ui-webapp/src/users/components/navLinks/SetPasswordNavLink.tsx index 978c78b833..ec6a598a6b 100644 --- a/scm-ui/ui-webapp/src/users/components/navLinks/SetPasswordNavLink.tsx +++ b/scm-ui/ui-webapp/src/users/components/navLinks/SetPasswordNavLink.tsx @@ -42,7 +42,8 @@ class ChangePasswordNavLink extends React.Component { } hasPermissionToSetPassword = () => { - return this.props.user._links.password; + const { user } = this.props; + return !user.external && user._links.password; }; } diff --git a/scm-ui/ui-webapp/src/users/components/table/Details.tsx b/scm-ui/ui-webapp/src/users/components/table/Details.tsx index 6466b0d3d5..8b94e36ff2 100644 --- a/scm-ui/ui-webapp/src/users/components/table/Details.tsx +++ b/scm-ui/ui-webapp/src/users/components/table/Details.tsx @@ -64,7 +64,7 @@ class Details extends React.Component { {t("user.externalFlag")} - + diff --git a/scm-webapp/src/main/java/sonia/scm/user/DefaultUserManager.java b/scm-webapp/src/main/java/sonia/scm/user/DefaultUserManager.java index 5bd15ee4be..00b0216818 100644 --- a/scm-webapp/src/main/java/sonia/scm/user/DefaultUserManager.java +++ b/scm-webapp/src/main/java/sonia/scm/user/DefaultUserManager.java @@ -389,8 +389,8 @@ public class DefaultUserManager extends AbstractUserManager if (user == null) { throw new NotFoundException(User.class, userId); } - if (isAnonymousUser(user)) { - throw new ChangePasswordNotAllowedException(ContextEntry.ContextBuilder.entity("PasswordChange", "-").in(User.class, user.getName()), user.getType()); + if (isAnonymousUser(user) || user.isExternal()) { + throw new ChangePasswordNotAllowedException(ContextEntry.ContextBuilder.entity("PasswordChange", "-").in(User.class, user.getName()), "external"); } user.setPassword(newPassword); this.modify(user); diff --git a/scm-webapp/src/test/java/sonia/scm/user/DefaultUserManagerTest.java b/scm-webapp/src/test/java/sonia/scm/user/DefaultUserManagerTest.java index 015be52eb3..8d26c94237 100644 --- a/scm-webapp/src/test/java/sonia/scm/user/DefaultUserManagerTest.java +++ b/scm-webapp/src/test/java/sonia/scm/user/DefaultUserManagerTest.java @@ -21,66 +21,52 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - -package sonia.scm.user; -//~--- non-JDK imports -------------------------------------------------------- +package sonia.scm.user; import com.github.sdorra.shiro.ShiroRule; import com.github.sdorra.shiro.SubjectAware; -import com.google.common.collect.Lists; - import org.assertj.core.api.Assertions; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; - import org.mockito.ArgumentCaptor; import sonia.scm.NotFoundException; import sonia.scm.store.JAXBConfigurationStoreFactory; import sonia.scm.user.xml.XmlUserDAO; -import static org.mockito.Mockito.*; - -//~--- JDK imports ------------------------------------------------------------ - -import java.util.Collections; -import java.util.List; -import org.junit.Rule; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; /** - * * @author Sebastian Sdorra */ @SubjectAware( - username = "trillian", - password = "secret", - configuration = "classpath:sonia/scm/repository/shiro.ini" + username = "trillian", + password = "secret", + configuration = "classpath:sonia/scm/repository/shiro.ini" ) -public class DefaultUserManagerTest extends UserManagerTestBase -{ +public class DefaultUserManagerTest extends UserManagerTestBase { @Rule public ShiroRule shiro = new ShiroRule(); - - private UserDAO userDAO ; - private User trillian; + private UserDAO userDAO; /** * Method description * - * * @return */ @Override - public UserManager createManager() - { + public UserManager createManager() { return new DefaultUserManager(createXmlUserDAO()); } @Before public void initDao() { - trillian = UserTestData.createTrillian(); + User trillian = UserTestData.createTrillian(); trillian.setPassword("oldEncrypted"); userDAO = mock(UserDAO.class); @@ -115,6 +101,16 @@ public class DefaultUserManagerTest extends UserManagerTestBase userManager.overwritePassword("notExisting", "---"); } + @Test(expected = ChangePasswordNotAllowedException.class) + public void shouldFailOverwritePasswordForExternalUser() { + User trillian = new User("trillian"); + trillian.setExternal(true); + when(userDAO.get("trillian")).thenReturn(trillian); + UserManager userManager = new DefaultUserManager(userDAO); + + userManager.overwritePassword("trillian", "---"); + } + @Test public void shouldSucceedOverwritePassword() { ArgumentCaptor userCaptor = ArgumentCaptor.forClass(User.class);