From 943caeef6d552a7653e0510ea6df875139ee8f03 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Tue, 30 Mar 2021 15:19:24 -0700 Subject: [PATCH 1/2] Issue 42435: When multiple primary LDAP authentication configurations are defined, only one is recognized when creating new users --- .../security/AuthenticationConfiguration.java | 4 + .../AuthenticationConfigurationCache.java | 25 ++- .../api/security/AuthenticationManager.java | 42 +++-- .../labkey/api/security/SecurityManager.java | 170 +++++------------- .../labkey/core/login/LoginController.java | 24 +-- .../org/labkey/core/security/GroupView.java | 3 - .../core/security/SecurityController.java | 26 +-- .../src/org/labkey/core/security/addUsers.jsp | 8 +- core/src/org/labkey/core/security/group.jsp | 8 +- .../specimen/ShowGroupMembersAction.java | 8 - .../study/view/specimen/groupMembers.jsp | 10 +- 11 files changed, 126 insertions(+), 202 deletions(-) diff --git a/api/src/org/labkey/api/security/AuthenticationConfiguration.java b/api/src/org/labkey/api/security/AuthenticationConfiguration.java index 55782abff8b..2efca4ba4b3 100644 --- a/api/src/org/labkey/api/security/AuthenticationConfiguration.java +++ b/api/src/org/labkey/api/security/AuthenticationConfiguration.java @@ -40,6 +40,10 @@ public interface AuthenticationConfiguration @NotNull AP getAuthenticationProvider(); boolean isEnabled(); @NotNull Map getCustomProperties(); + default @Nullable String getDomain() + { + return null; + } /** * @return Map of all property names and values that are updateable and appropriate for audit logging diff --git a/api/src/org/labkey/api/security/AuthenticationConfigurationCache.java b/api/src/org/labkey/api/security/AuthenticationConfigurationCache.java index b9c80138f11..4f8477bbcfe 100644 --- a/api/src/org/labkey/api/security/AuthenticationConfigurationCache.java +++ b/api/src/org/labkey/api/security/AuthenticationConfigurationCache.java @@ -8,6 +8,7 @@ import org.labkey.api.cache.CacheManager; import org.labkey.api.data.CoreSchema; import org.labkey.api.data.TableSelector; +import org.labkey.api.security.AuthenticationConfiguration.PrimaryAuthenticationConfiguration; import org.labkey.api.security.AuthenticationProvider.PrimaryAuthenticationProvider; import java.util.Collection; @@ -17,6 +18,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; @@ -53,6 +55,8 @@ protected Set> createCollection() } }; + private final Collection _activeDomains; + private AuthenticationConfigurationCollections() { boolean acceptOnlyFicamProviders = AuthenticationManager.isAcceptOnlyFicamProviders(); @@ -71,11 +75,6 @@ private AuthenticationConfigurationCollections() }) .collect(Collectors.groupingBy(this::getAuthenticationConfigurationFactory)); - // Bit of a hack: LdapProvider sets "ldapDomain" to the first configuration's domain. We should add getDomain() - // to AuthenticationConfiguration and collect all email domains, caching them with the collections. This is only - // used for administrative messages, so we'll continue to tolerate this approach for a little while longer. - AuthenticationManager.setLdapDomain(null); - // Add each group of configurations addConfigurations(configurationMap); @@ -85,6 +84,12 @@ private AuthenticationConfigurationCollections() .collect(Collectors.toMap(p->p, p->Collections.emptyList())); addConfigurations(permanentMap); + + _activeDomains = getActive(PrimaryAuthenticationConfiguration.class).stream() + .map(AuthenticationConfiguration::getDomain) + .filter(Objects::nonNull) + .filter(domain->!AuthenticationManager.ALL_DOMAINS.equals(domain)) + .collect(Collectors.toCollection(LinkedHashSet::new)); } // Little helper method simplifies the stream handling above @@ -151,6 +156,11 @@ private void addToMap(SetValuedMap, return null != configurations ? configurations : Collections.emptyList(); } + + private @NotNull Collection getActiveDomains() + { + return _activeDomains; + } } /** @@ -209,4 +219,9 @@ public static void clear() { CACHE.remove(CACHE_KEY); } + + public static @NotNull Collection getActiveDomains() + { + return CACHE.get(CACHE_KEY).getActiveDomains(); + } } diff --git a/api/src/org/labkey/api/security/AuthenticationManager.java b/api/src/org/labkey/api/security/AuthenticationManager.java index 0ba87d5174e..8da5c0393fb 100644 --- a/api/src/org/labkey/api/security/AuthenticationManager.java +++ b/api/src/org/labkey/api/security/AuthenticationManager.java @@ -195,22 +195,44 @@ public static void populateSettingsWithStartupProps() // Populate the general authentication properties (e.g., auto-create accounts, self registration, self-service email changes). ModuleLoader.getInstance().getConfigProperties(AUTHENTICATION_CATEGORY).stream() .filter(cp->!cp.getName().equals(PROVIDERS_KEY)) // Ignore "Authentication" -- we don't use this property anymore - .forEach(cp-> saveAuthSetting(null, cp.getName(), Boolean.parseBoolean(cp.getValue()))); + .forEach(cp->saveAuthSetting(null, cp.getName(), Boolean.parseBoolean(cp.getValue()))); } public enum Priority { High, Low } - // TODO: Replace this with a generic domain-claiming mechanism - private static String _ldapDomain = null; + // Return a collection of all email domains associated with authentication configurations, not including "*" or null + public static @NotNull Collection getActiveDomains() + { + return AuthenticationConfigurationCache.getActiveDomains(); + } - public static @Nullable String getLdapDomain() + public static HtmlString getStandardSendVerificationEmailsMessage() { - return _ldapDomain; + HtmlStringBuilder builder = HtmlStringBuilder.of("Send password verification emails to all new users"); + Collection activeDomains = getActiveDomains(); + + if (!activeDomains.isEmpty()) + { + // At the moment, only LDAP configurations can be associated with a domain, so we call out LDAP below + builder.append(" except those with email addresses that are configured for LDAP authentication (those ending in "); + builder.append( + activeDomains.stream() + .map(d->"@" + d) + .collect(Collectors.joining(", ")) + ); + + builder.append(")"); + } + + return builder.getHtmlString(); } - public static void setLdapDomain(String ldapDomain) + // Ignores domain = "*" + public static boolean isLdapEmail(ValidEmail email) { - _ldapDomain = StringUtils.trimToNull(ldapDomain); + String emailAddress = email.getEmailAddress(); + return getActiveDomains().stream() + .anyMatch(domain->StringUtils.endsWithIgnoreCase(emailAddress, "@" + domain)); } public static boolean isRegistrationEnabled() @@ -703,15 +725,15 @@ public String getMessage() /** avoid spamming the audit log **/ - private static Cache authMessages = CacheManager.getCache(100, TimeUnit.MINUTES.toMillis(10), "Authentication Messages"); + private static final Cache AUTH_MESSAGES = CacheManager.getCache(100, TimeUnit.MINUTES.toMillis(10), "Authentication Messages"); public static void addAuditEvent(@NotNull User user, HttpServletRequest request, String msg) { String key = user.getUserId() + "/" + ((null==request||null==request.getLocalAddr())?"":request.getLocalAddr()); - String prevMessage = authMessages.get(key); + String prevMessage = AUTH_MESSAGES.get(key); if (StringUtils.equals(prevMessage, msg)) return; - authMessages.put(key, msg); + AUTH_MESSAGES.put(key, msg); if (user.isGuest()) { UserManager.UserAuditEvent event = new UserManager.UserAuditEvent(ContainerManager.getRoot().getId(), msg, user); diff --git a/api/src/org/labkey/api/security/SecurityManager.java b/api/src/org/labkey/api/security/SecurityManager.java index e5a3579497c..f1f0b539072 100644 --- a/api/src/org/labkey/api/security/SecurityManager.java +++ b/api/src/org/labkey/api/security/SecurityManager.java @@ -62,13 +62,11 @@ import org.labkey.api.security.impersonation.UserImpersonationContextFactory; import org.labkey.api.security.permissions.AddUserPermission; import org.labkey.api.security.permissions.AdminPermission; -import org.labkey.api.security.permissions.ApplicationAdminPermission; import org.labkey.api.security.permissions.DeletePermission; import org.labkey.api.security.permissions.InsertPermission; import org.labkey.api.security.permissions.Permission; import org.labkey.api.security.permissions.ReadPermission; import org.labkey.api.security.permissions.SeeUserDetailsPermission; -import org.labkey.api.security.permissions.SiteAdminPermission; import org.labkey.api.security.permissions.UpdatePermission; import org.labkey.api.security.roles.EditorRole; import org.labkey.api.security.roles.FolderAdminRole; @@ -395,14 +393,14 @@ public void userAddedToSite(User user) @Override public void userDeletedFromSite(User user) { - // This clears the cache of security policies. It does not remove the policies themselves. + // This clears the cache of security policies. It does not remove the policies themselves. SecurityPolicyManager.removeAll(); } @Override public void userAccountDisabled(User user) { - // This clears the cache of security policies. It does not remove the policies themselves. + // This clears the cache of security policies. It does not remove the policies themselves. SecurityPolicyManager.removeAll(); } @@ -419,7 +417,6 @@ public void propertyChange(PropertyChangeEvent evt) } } - private static @Nullable Pair getBasicCredentials(HttpServletRequest request) { // Authorization: Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ== @@ -443,7 +440,6 @@ public void propertyChange(PropertyChangeEvent evt) return ret; } - // Authorization: Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ== private static @Nullable User authenticateBasic(HttpServletRequest request, @NotNull Pair basicCredentials) { @@ -451,7 +447,7 @@ public void propertyChange(PropertyChangeEvent evt) { String rawEmail = basicCredentials.getKey(); String password = basicCredentials.getValue(); - if (rawEmail.toLowerCase().equals("guest")) + if (rawEmail.equalsIgnoreCase("guest")) return User.guest; new ValidEmail(rawEmail); // validate email address @@ -463,13 +459,11 @@ public void propertyChange(PropertyChangeEvent evt) } } - public static boolean isBasicAuthentication(HttpServletRequest request) { return "Basic".equals(request.getAttribute(AUTHENTICATION_METHOD)); } - public static User getSessionUser(HttpSession session) { User sessionUser = null; @@ -482,7 +476,6 @@ public static User getSessionUser(HttpSession session) return sessionUser; } - public static Pair attemptAuthentication(HttpServletRequest request) throws UnsupportedEncodingException { @Nullable Pair basicCredentials = getBasicCredentials(request); @@ -581,7 +574,6 @@ else if ("true".equalsIgnoreCase(request.getHeader("LabKey-Disallow-Global-Roles return null == u || u.isGuest() ? null : new Pair<>(u, request); } - /** * Determine if an API key is present, checking basic auth first, then "apikey" header, and then the special "transform" * cookie and parameters. Return the API key if it's present; otherwise return null. @@ -632,7 +624,6 @@ else if ("true".equalsIgnoreCase(request.getHeader("LabKey-Disallow-Global-Roles return apiKey; } - public static final int SECONDS_PER_DAY = 60*60*24; public static abstract class TransformSession implements Closeable @@ -743,7 +734,6 @@ public static HttpSession setAuthenticatedUser(HttpServletRequest request, @Null return newSession; } - public static URLHelper logoutUser(HttpServletRequest request, User user, @Nullable URLHelper returnURL) { URLHelper ret = AuthenticationManager.logout(user, request, returnURL); // Let AuthenticationProvider clean up auth-specific cookies, etc. @@ -751,7 +741,6 @@ public static URLHelper logoutUser(HttpServletRequest request, User user, @Nulla return ret; } - public static void impersonateUser(ViewContext viewContext, User impersonatedUser, ActionURL returnURL) { @Nullable Container project = viewContext.getContainer().getProject(); @@ -763,14 +752,12 @@ public static void impersonateUser(ViewContext viewContext, User impersonatedUse impersonate(viewContext, new UserImpersonationContextFactory(project, user, impersonatedUser, returnURL)); } - public static void impersonateGroup(ViewContext viewContext, Group group, ActionURL returnURL) { @Nullable Container project = viewContext.getContainer().getProject(); impersonate(viewContext, new GroupImpersonationContextFactory(project, viewContext.getUser(), group, returnURL)); } - public static void impersonateRoles(ViewContext viewContext, Collection newImpersonationRoles, Set currentImpersonationRoles, ActionURL returnURL) { @Nullable Container project = viewContext.getContainer().getProject(); @@ -782,7 +769,6 @@ public static void impersonateRoles(ViewContext viewContext, Collection ne impersonate(viewContext, new RoleImpersonationContextFactory(project, user, newImpersonationRoles, currentImpersonationRoles, returnURL)); } - private static void impersonate(ViewContext viewContext, ImpersonationContextFactory factory) { // Tell the factory to start impersonating @@ -794,7 +780,6 @@ private static void impersonate(ViewContext viewContext, ImpersonationContextFac session.setAttribute(IMPERSONATION_CONTEXT_FACTORY_KEY, factory); } - public static void stopImpersonating(HttpServletRequest request, ImpersonationContextFactory factory) { factory.stopImpersonating(request); @@ -804,7 +789,6 @@ public static void stopImpersonating(HttpServletRequest request, ImpersonationCo session.removeAttribute(IMPERSONATION_CONTEXT_FACTORY_KEY); } - public static void setValidators(HttpSession session, List validators) { if (validators.isEmpty()) @@ -813,13 +797,11 @@ public static void setValidators(HttpSession session, List getValidators(HttpSession session) { return (List)session.getAttribute(AUTHENTICATION_VALIDATORS_KEY); } - private static final String passwordChars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"; public static final int tempPasswordLength = 32; @@ -833,7 +815,6 @@ public static String createTempPassword() return tempPassword.toString(); } - public static ActionURL createVerificationURL(Container c, ValidEmail email, String verification, @Nullable List> extraParameters) { return PageFlowUtil.urlProvider(LoginUrls.class).getVerificationURL(c, email, verification, extraParameters); @@ -865,14 +846,12 @@ public static boolean isVerified(ValidEmail email) return (null == getVerification(email)); } - public static boolean verify(ValidEmail email, String verification) { String dbVerification = getVerification(email); return (dbVerification != null && dbVerification.equals(verification)); } - public static void setVerification(ValidEmail email, @Nullable String verification) throws UserManagementException { int rows = new SqlExecutor(core.getSchema()).execute("UPDATE " + core.getTableInfoLogins() + " SET Verification=? WHERE LOWER(email)=LOWER(?)", verification, email.getEmailAddress()); @@ -880,13 +859,11 @@ public static void setVerification(ValidEmail email, @Nullable String verificati throw new UserManagementException(email, "Unexpected number of rows returned when setting verification: " + rows); } - public static String getVerification(ValidEmail email) { return new SqlSelector(core.getSchema(), "SELECT Verification FROM " + core.getTableInfoLogins() + " WHERE Email = ?", email.getEmailAddress()).getObject(String.class); } - public static class NewUserStatus { private final ValidEmail _email; @@ -906,7 +883,7 @@ public ValidEmail getEmail() public boolean isLdapEmail() { - return SecurityManager.isLdapEmail(_email); + return AuthenticationManager.isLdapEmail(_email); } public String getVerification() @@ -1029,7 +1006,7 @@ public static NewUserStatus addUser(ValidEmail email, @Nullable User currentUser try { - Map returnMap = Table.insert(currentUser, core.getTableInfoPrincipals(), fieldsIn); + Map returnMap = Table.insert(currentUser, core.getTableInfoPrincipals(), fieldsIn); userId = (Integer) returnMap.get("UserId"); } catch (RuntimeSQLException e) @@ -1095,7 +1072,6 @@ public static NewUserStatus addUser(ValidEmail email, @Nullable User currentUser } } - private static String displayNameFromEmail(ValidEmail email, Integer userId) { String displayName; @@ -1128,7 +1104,6 @@ else if (email.getEmailAddress().indexOf("@") > 0) return displayName; } - public static void sendEmail(Container c, User user, SecurityMessage message, String to, ActionURL verificationURL) throws ConfigurationException, MessagingException { MimeMessage m = createMessage(c, user, message, to, verificationURL); @@ -1169,7 +1144,7 @@ private static MimeMessage createMessage(Container c, User user, SecurityMessage } } - // Create record for non-LDAP login, saving email address and hashed password. Return verification token. + // Create record for non-LDAP login, saving email address and hashed password. Return verification token. public static String createLogin(ValidEmail email) throws UserManagementException { // Create a placeholder password hash and a separate email verification key that will get emailed to the new user @@ -1195,7 +1170,6 @@ public static String createLogin(ValidEmail email) throws UserManagementExceptio return verification; } - public static void setPassword(ValidEmail email, String password) throws UserManagementException { String crypt = Crypt.BCrypt.digestWithPrefix(password); @@ -1213,7 +1187,6 @@ public static void setPassword(ValidEmail email, String password) throws UserMan throw new UserManagementException(email, "Password update statement affected " + rows + " rows."); } - private static final int MAX_HISTORY = 10; private static List getCryptHistory(String email) @@ -1234,7 +1207,6 @@ private static List getCryptHistory(String email) } } - public static boolean matchesPreviousPassword(String password, User user) { List history = getCryptHistory(user.getEmail()); @@ -1248,21 +1220,18 @@ public static boolean matchesPreviousPassword(String password, User user) return false; } - public static Date getLastChanged(User user) { SqlSelector selector = new SqlSelector(core.getSchema(), new SQLFragment("SELECT LastChanged FROM " + core.getTableInfoLogins() + " WHERE Email=?", user.getEmail())); return selector.getObject(Date.class); } - // Look up email in Logins table and return the corresponding password hash public static String getPasswordHash(ValidEmail email) { return getPasswordHash(email.getEmailAddress()); } - // Look up email in Logins table and return the corresponding password hash private static String getPasswordHash(String email) { @@ -1270,7 +1239,6 @@ private static String getPasswordHash(String email) return selector.getObject(String.class); } - public static boolean matchPassword(String password, String hash) { if (StringUtils.isEmpty(hash) || hash.startsWith("disabled:")) @@ -1285,26 +1253,22 @@ else if (Crypt.MD5.acceptPrefix(hash)) return Crypt.MD5.matches(password, hash); } - // Used only in the case of email change... current email address might be invalid static boolean loginExists(String email) { return (null != getPasswordHash(email)); } - public static boolean loginExists(ValidEmail email) { return (null != getPasswordHash(email)); } - public static Group createGroup(Container c, String name) { return createGroup(c, name, PrincipalType.GROUP); } - public static Group createGroup(Container c, String name, PrincipalType type) { // Consider: add validation rules to enum @@ -1329,7 +1293,6 @@ public static Group createGroup(Container c, String name, PrincipalType type) return createGroup(c, name, type, ownerId); } - public static Group createGroup(Container c, String name, PrincipalType type, String ownerId) { String containerId = (null == c || c.isRoot()) ? null : c.getId(); @@ -1354,15 +1317,12 @@ public static Group createGroup(Container c, String name, PrincipalType type, St return group; } - // Case-insensitive existence check -- disallows groups that differ only by case private static boolean groupExists(Container c, String groupName, String ownerId) { return null != getGroupId(c, groupName, ownerId, false, true); - } - public static Group renameGroup(Group group, String newName, User currentUser) { if (group.isSystemGroup()) @@ -1380,7 +1340,6 @@ public static Group renameGroup(Group group, String newName, User currentUser) GroupCache.uncache(group.getUserId()); return getGroup(getGroupId(c, newName)); - } public static void deleteGroup(Group group) @@ -1388,7 +1347,6 @@ public static void deleteGroup(Group group) deleteGroup(group.getUserId()); } - static void deleteGroup(int groupId) { if (groupId == Group.groupAdministrators || @@ -1439,7 +1397,6 @@ static void deleteGroup(int groupId) SecurityPolicyManager.notifyPolicyChanges(resources); } - public static void deleteGroups(Container c, @Nullable PrincipalType type) { if (!(null == type || type == PrincipalType.GROUP || type == PrincipalType.MODULE)) @@ -1487,7 +1444,6 @@ public static void deleteMembers(Group group, List membersToDelet } } - public static void deleteMember(Group group, UserPrincipal principal) { int groupId = group.getUserId(); @@ -1496,7 +1452,6 @@ public static void deleteMember(Group group, UserPrincipal principal) fireDeletePrincipalFromGroup(groupId, principal); } - // Returns a list of errors public static List addMembers(Group group, Collection principals) { @@ -1517,14 +1472,12 @@ public static List addMembers(Group group, Collection Collection getValidPrincipals(Group group, Collection candidates) @@ -1612,7 +1562,6 @@ public static Collection getValidPrincipals(Group g return valid; } - // Return an error message if principal can't be added to the group, otherwise return null public static String getAddMemberError(Group group, UserPrincipal principal) { @@ -1665,7 +1614,6 @@ public static String getAddMemberError(Group group, UserPrincipal principal) return null; } - // Site groups are first (if included) followed by project groups. Each list is sorted by name (case-insensitive). public static @NotNull List getGroups(@Nullable Container project, boolean includeGlobalGroups) { @@ -1688,7 +1636,7 @@ public static UserPrincipal getPrincipal(int id) return null != principal ? principal : getGroup(id); } - /** This will preferentially return project users/groups. If no principal is found at the project level and includeSiteGroups=true, it will check site groups */ + /** This will preferentially return project users/groups. If no principal is found at the project level and includeSiteGroups=true, it will check site groups */ @Nullable public static UserPrincipal getPrincipal(String name, Container container, boolean includeSiteGroups) { @@ -1793,7 +1741,6 @@ public static HtmlString getGroupList(Container c, User u) return HtmlString.of(groupList.toString()); } - /** Returns the requested direct members of this group (non-recursive) */ public static @NotNull

Set

getGroupMembers(Group group, MemberType

memberType) { @@ -1804,7 +1751,6 @@ public static HtmlString getGroupList(Container c, User u) return principals; } - /** Returns the members of this group dictated by memberType, including those in subgroups (recursive) */ public static @NotNull

Set

getAllGroupMembers(Group group, MemberType

memberType) { @@ -1853,7 +1799,6 @@ public static HtmlString getGroupList(Container c, User u) return members; } - private static

void addMembers(Collection

principals, int[] ids, MemberType

memberType) { for (int id : ids) @@ -1864,7 +1809,6 @@ private static

void addMembers(Collection

principal } } - // get the list of group members that do not need to be direct members because they are a member of a member group (i.e. groups-in-groups) public static Map> getRedundantGroupMembers(Group group) { @@ -1990,7 +1934,6 @@ public static String getMembershipPathwayHTMLDisplay(Set> pa return sb.toString(); } - // TODO: Redundant with getProjectUsers() -- this approach should be more efficient for simple cases // TODO: Also redundant with getFolderUserids() // TODO: Cache this set @@ -2003,7 +1946,6 @@ public static Set getProjectUsersIds(Container c) return new HashSet<>(selector.getCollection(Integer.class)); } - // True fragment -- need to prepend SELECT DISTINCT() or IN () for this to be valid SQL public static SQLFragment getProjectUsersSQL(Container c) { @@ -2055,7 +1997,6 @@ public static SQLFragment getProjectUsersSQL(Container c) return projectUsers; } - public static Collection getFolderUserids(Container c) { Container project = (c.isProject() || c.isRoot()) ? c : c.getProject(); @@ -2115,10 +2056,9 @@ public static Collection getFolderUserids(Container c) return userIds; } - public static List getUsersWithPermissions(Container c, Set> perms) { - // No cache right now, but performance seems fine. After the user list and policy are cached, no other queries occur. + // No cache right now, but performance seems fine. After the user list and policy are cached, no other queries occur. Collection allUsers = UserManager.getActiveUsers(); List users = new ArrayList<>(allUsers.size()); SecurityPolicy policy = c.getPolicy(); @@ -2143,7 +2083,6 @@ public static List getUsersWithOneOf(Container c, Set> getGroupMemberNamesAndIds(String path) { @@ -2195,7 +2134,6 @@ public static List> getGroupMemberNamesAndIds(Integer grou return members; } - /** Returns both users and groups, but direct members only (not recursive) */ public static String[] getGroupMemberNames(Integer groupId) { @@ -2207,7 +2145,6 @@ public static String[] getGroupMemberNames(Integer groupId) return names; } - /** Takes string such as "/test/subfolder/Users" and returns groupId */ public static Integer getGroupId(String extraPath) { @@ -2233,29 +2170,25 @@ public static Integer getGroupId(String extraPath) return getGroupId(c, group); } - /** Takes Container (or null for root) and group name; returns groupId */ public static Integer getGroupId(@Nullable Container c, String group) { return getGroupId(c, group, null, true); } - /** Takes Container (or null for root) and group name; returns groupId */ public static Integer getGroupId(@Nullable Container c, String group, boolean throwOnFailure) { return getGroupId(c, group, null, throwOnFailure); } - public static Integer getGroupId(@Nullable Container c, String groupName, @Nullable String ownerId, boolean throwOnFailure) { return getGroupId(c, groupName, ownerId, throwOnFailure, false); } - // This is temporary... in CPAS 1.5 on PostgreSQL it was possible to create two groups in the same container that differed only - // by case (this was not possible on SQL Server). In CPAS 1.6 we disallow this on PostgreSQL... but we still need to be able to + // by case (this was not possible on SQL Server). In CPAS 1.6 we disallow this on PostgreSQL... but we still need to be able to // retrieve group IDs in a case-sensitive manner. // TODO: For CPAS 1.7: this should always be case-insensitive (we will clean up the database by renaming duplicate groups) private static Integer getGroupId(@Nullable Container c, String groupName, @Nullable String ownerId, boolean throwOnFailure, boolean caseInsensitive) @@ -2295,14 +2228,6 @@ private static Integer getGroupId(@Nullable Container c, String groupName, @Null return groupId; } - // TODO: Update to iterate through all configurations - public static boolean isLdapEmail(ValidEmail email) - { - String ldapDomain = AuthenticationManager.getLdapDomain(); - return AuthenticationManager.ALL_DOMAINS.equals(ldapDomain) || ldapDomain != null && email.getEmailAddress().endsWith("@" + ldapDomain.toLowerCase()); - } - - public interface ViewFactory { HttpView createView(ViewContext context); @@ -2319,7 +2244,6 @@ public static List getViewFactories() return VIEW_FACTORIES; } - public interface TermsOfUseProvider { /** @@ -2343,7 +2267,6 @@ public static List getTermsOfUseProviders() return TERMS_OF_USE_PROVIDERS; } - public static class TestCase extends Assert { Group groupA = null; @@ -2867,13 +2790,13 @@ public static HtmlString addUser(ViewContext context, ValidEmail email, boolean if (newUserStatus.isLdapEmail()) { - message.append(newUser.getEmail()).append(" added as a new user to the system. This user will be authenticated via LDAP."); - UserManager.addToUserHistory(newUser, newUser.getEmail() + " was added to the system. This user will be authenticated via LDAP."); + message.append(newUser.getEmail()).append(" added as a new user to the system. This user will be authenticated via LDAP."); + UserManager.addToUserHistory(newUser, newUser.getEmail() + " was added to the system. This user will be authenticated via LDAP."); } else if (sendMail) { message.append(email.getEmailAddress()).append(" added as a new user to the system and emailed successfully."); - UserManager.addToUserHistory(newUser, newUser.getEmail() + " was added to the system. Verification email was sent successfully."); + UserManager.addToUserHistory(newUser, newUser.getEmail() + " was added to the system. Verification email was sent successfully."); } else { @@ -2904,7 +2827,7 @@ else if (sendMail) User newUser = UserManager.getUser(email); if (null != newUser) - UserManager.addToUserHistory(newUser, newUser.getEmail() + " was added to the system. Sending the verification email failed."); + UserManager.addToUserHistory(newUser, newUser.getEmail() + " was added to the system. Sending the verification email failed."); } catch (SecurityManager.UserManagementException e) { @@ -2959,14 +2882,14 @@ public static void addSelfRegisteredUser(ViewContext context, ValidEmail email, try { SecurityManager.sendRegistrationEmail(context, email, null, newUserStatus, extraParameters, registrationProviderName, true); - UserManager.addToUserHistory(newUser, newUser.getEmail() + " was added to the system via self-registration. Verification email was sent successfully."); + UserManager.addToUserHistory(newUser, newUser.getEmail() + " was added to the system via self-registration. Verification email was sent successfully."); } catch (ConfigurationException e) { User createdUser = UserManager.getUser(email); if (null != createdUser) - UserManager.addToUserHistory(createdUser, createdUser.getEmail() + " was added to the system via self-registration. Sending the verification email failed."); + UserManager.addToUserHistory(createdUser, createdUser.getEmail() + " was added to the system via self-registration. Sending the verification email failed."); throw e; } } @@ -3046,22 +2969,6 @@ public static void setAdminOnlyPermissions(Container c) SecurityPolicyManager.savePolicy(policy); } - public static boolean containsOnlyAdminPermissions(Container c) - { - SecurityPolicy policy = c.getPolicy(); - - for (RoleAssignment ra : policy.getAssignments()) - { - Role role = ra.getRole(); - Set> permissions = role.getPermissions(); - if (!(role instanceof ProjectAdminRole) && !(role instanceof FolderAdminRole) - && !(permissions.contains(SiteAdminPermission.class) && !permissions.contains(ApplicationAdminPermission.class))) - return false; - } - - return true; - } - public static void setInheritPermissions(Container c) { SecurityPolicyManager.deletePolicy(c); @@ -3134,15 +3041,23 @@ protected SecurityEmailTemplate(String name) { super(name); - _replacements.add(new ReplacementParam("verificationURL", String.class, "Link for a user to set a password"){ + _replacements.add(new ReplacementParam<>("verificationURL", String.class, "Link for a user to set a password") + { @Override - public String getValue(Container c) {return _verificationUrl;} + public String getValue(Container c) + { + return _verificationUrl; + } }); - _replacements.add(new ReplacementParam("emailAddress", String.class, "The email address of the user performing the operation"){ + _replacements.add(new ReplacementParam<>("emailAddress", String.class, "The email address of the user performing the operation") + { @Override - public String getValue(Container c) {return _originatingUser == null ? null : _originatingUser.getEmail();} + public String getValue(Container c) + { + return _originatingUser == null ? null : _originatingUser.getEmail(); + } }); - _replacements.add(new ReplacementParam("recipient", String.class, "The email address on the 'to:' line") + _replacements.add(new ReplacementParam<>("recipient", String.class, "The email address on the 'to:' line") { @Override public String getValue(Container c) @@ -3182,12 +3097,12 @@ public static class RegistrationEmailTemplate extends SecurityEmailTemplate "Welcome to the ^organizationName^ ^siteShortName^ Web Site new user registration"; protected static final String DEFAULT_BODY = "^optionalMessage^\n\n" + - "You now have an account on the ^organizationName^ ^siteShortName^ web site. We are sending " + + "You now have an account on the ^organizationName^ ^siteShortName^ web site. We are sending " + "you this message to verify your email address and to allow you to create a password that will provide secure " + - "access to your data on the web site. To complete the registration process, simply click the link below or " + - "copy it to your browser's address bar. You will then be asked to choose a password.\n\n" + + "access to your data on the web site. To complete the registration process, simply click the link below or " + + "copy it to your browser's address bar. You will then be asked to choose a password.\n\n" + "^verificationURL^\n\n" + - "The ^siteShortName^ home page is ^homePageURL^. If you have any questions don't hesitate to " + + "The ^siteShortName^ home page is ^homePageURL^. If you have any questions don't hesitate to " + "contact the ^siteShortName^ team at ^systemEmail^."; @SuppressWarnings("UnusedDeclaration") // Constructor called via reflection @@ -3230,7 +3145,7 @@ public static class PasswordResetEmailTemplate extends SecurityEmailTemplate protected static final String DEFAULT_BODY = "We have reset your password on the ^organizationName^ ^siteShortName^ web site. " + "To sign in to the system you will need " + - "to specify a new password. Click the link below or copy it to your browser's address bar. You will then be " + + "to specify a new password. Click the link below or copy it to your browser's address bar. You will then be " + "asked to enter a new password.\n\n" + "^verificationURL^\n\n" + "The ^siteShortName^ home page is ^homePageURL^."; @@ -3272,15 +3187,12 @@ public static String getDisambiguatedGroupName(Group group) { int id = group.getUserId(); - switch(id) + return switch (id) { - case Group.groupAdministrators: - return "Site Administrators"; - case Group.groupUsers: - return "All Site Users"; - default: - return group.getName(); - } + case Group.groupAdministrators -> "Site Administrators"; + case Group.groupUsers -> "All Site Users"; + default -> group.getName(); + }; } public static boolean canSeeUserDetails(Container c, User user) @@ -3291,7 +3203,7 @@ public static boolean canSeeUserDetails(Container c, User user) public static boolean canSeeAuditLog(User user) { // - // Only returns true if the user has the site permission. If the user is an admin, then the permission + // Only returns true if the user has the site permission. If the user is an admin, then the permission // check on the current container filter will return true // return user.hasRootPermission(CanSeeAuditLogPermission.class); diff --git a/core/src/org/labkey/core/login/LoginController.java b/core/src/org/labkey/core/login/LoginController.java index 04393335c2a..13dc2222a20 100644 --- a/core/src/org/labkey/core/login/LoginController.java +++ b/core/src/org/labkey/core/login/LoginController.java @@ -827,12 +827,6 @@ private Pair attemptReset(String rawEmail, String providerName) return Pair.of(false, "Reset Password failed: " + rawEmail + " is not a valid email address."); } - if (SecurityManager.isLdapEmail(email)) - { - // ldap authentication users must reset through their ldap administrator - return Pair.of(false, "Reset Password failed: " + email + " is an LDAP email address. Please contact your LDAP administrator to reset the password for this account."); - } - // Every case below this point should result in the same, generic message being displayed to the user to avoid revealing any details about accounts, #33907 final User user = UserManager.getUser(email); @@ -846,7 +840,7 @@ private Pair attemptReset(String rawEmail, String providerName) if (!SecurityManager.loginExists(email)) { _log.error("Password reset attempted for an account that doesn't have a password: " + email); - return resetPasswordResponse(user, "You cannot reset the password for your account because it doesn't have a password. This usually means you log in via a single sign-on provider. Contact a server administrator if you have questions.", "Reset Password failed: " + email + " does not have a password"); + return resetPasswordResponse(user, "You cannot reset the password for your account because it doesn't have a password. This usually means you log in via LDAP or single sign-on. Contact a server administrator if you have questions.", "Reset Password failed: " + email + " does not have a password"); } if (!user.isActive()) @@ -1151,7 +1145,7 @@ private String getEmailFromCookie(HttpServletRequest request) if (null != cookies) { - // Starting in LabKey 9.1, the cookie value is URL encoded to allow for special characters like @. See #6736. + // Starting in LabKey 9.1, the cookie value is URL encoded to allow for special characters like @. See #6736. String encodedEmail = PageFlowUtil.getCookieValue(cookies, "email", null); if (null != encodedEmail) @@ -1687,7 +1681,7 @@ private AuthenticationResult attemptSetPassword(ValidEmail email, URLHelper retu } catch (UserManagementException e) { - errors.reject("setPassword", "Setting password failed: " + e.getMessage() + ". Contact the " + LookAndFeelProperties.getInstance(ContainerManager.getRoot()).getShortName() + " team."); + errors.reject("setPassword", "Setting password failed: " + e.getMessage() + ". Contact the " + LookAndFeelProperties.getInstance(ContainerManager.getRoot()).getShortName() + " team."); return null; } @@ -1699,7 +1693,7 @@ private AuthenticationResult attemptSetPassword(ValidEmail email, URLHelper retu } catch (UserManagementException e) { - errors.reject("setPassword", "Resetting verification failed. Contact the " + LookAndFeelProperties.getInstance(ContainerManager.getRoot()).getShortName() + " team."); + errors.reject("setPassword", "Resetting verification failed. Contact the " + LookAndFeelProperties.getInstance(ContainerManager.getRoot()).getShortName() + " team."); return null; } @@ -1803,7 +1797,7 @@ public static void checkVerificationErrors(boolean isVerified, User user, ValidE { if (user == null) { - errors.reject("setPassword", "This user doesn't exist. Make sure you've copied the entire link into your browser's address bar."); + errors.reject("setPassword", "This user doesn't exist. Make sure you've copied the entire link into your browser's address bar."); } else if (!user.isActive()) { @@ -1815,10 +1809,10 @@ else if (!user.isActive()) { if (!SecurityManager.loginExists(email)) { - if (SecurityManager.isLdapEmail(email)) + if (AuthenticationManager.isLdapEmail(email)) errors.reject("setPassword", "Your account will use your institution's LDAP authentication server and you do not need to set a separate password."); else - errors.reject("setPassword", "This email address is not associated with an account. Make sure you've copied the entire link into your browser's address bar."); + errors.reject("setPassword", "This email address is not associated with an account. Make sure you've copied the entire link into your browser's address bar."); } else if (SecurityManager.isVerified(email)) errors.reject("setPassword", "This email address has already been verified."); @@ -1826,7 +1820,7 @@ else if (null == verification || verification.length() < SecurityManager.tempPas errors.reject("setPassword", "Make sure you've copied the entire link into your browser's address bar."); else // Incorrect verification string - errors.reject("setPassword", "Verification failed. Make sure you've copied the entire link into your browser's address bar."); + errors.reject("setPassword", "Verification failed. Make sure you've copied the entire link into your browser's address bar."); } } @@ -1947,7 +1941,7 @@ public boolean handlePost(SetPasswordForm form, BindException errors) throws Exc } catch (InvalidEmailException e) { - errors.rejectValue("email", ERROR_MSG, "The string '" + PageFlowUtil.filter(form.getEmail()) + "' is not a valid email address. Please enter an email address in this form: user@domain.tld"); + errors.rejectValue("email", ERROR_MSG, "The string '" + PageFlowUtil.filter(form.getEmail()) + "' is not a valid email address. Please enter an email address in this form: user@domain.tld"); } return success; diff --git a/core/src/org/labkey/core/security/GroupView.java b/core/src/org/labkey/core/security/GroupView.java index d3e1e851be6..30abeb95caa 100644 --- a/core/src/org/labkey/core/security/GroupView.java +++ b/core/src/org/labkey/core/security/GroupView.java @@ -17,7 +17,6 @@ package org.labkey.core.security; import org.apache.commons.lang3.StringUtils; -import org.labkey.api.security.AuthenticationManager; import org.labkey.api.security.Group; import org.labkey.api.security.UserPrincipal; import org.labkey.api.util.HtmlString; @@ -46,7 +45,6 @@ public GroupView(Group group, Collection members, Map members; public List messages; public boolean isSystemGroup; - public String ldapDomain; public Map> redundantMembers; public String displayRedundancyReasonHTML(UserPrincipal principal) diff --git a/core/src/org/labkey/core/security/SecurityController.java b/core/src/org/labkey/core/security/SecurityController.java index c5345e155de..d8a7813ad42 100644 --- a/core/src/org/labkey/core/security/SecurityController.java +++ b/core/src/org/labkey/core/security/SecurityController.java @@ -58,6 +58,7 @@ import org.labkey.api.query.QuerySettings; import org.labkey.api.query.QueryView; import org.labkey.api.query.UserSchema; +import org.labkey.api.security.AuthenticationConfiguration.LoginFormAuthenticationConfiguration; import org.labkey.api.security.AuthenticationConfiguration.SSOAuthenticationConfiguration; import org.labkey.api.security.SecurityManager; import org.labkey.api.security.*; @@ -1789,24 +1790,27 @@ HtmlString getConfirmationMessage(boolean loginExists, String emailAddress) if (!loginExists) throw new NotFoundException(emailAddress + " does not seem to have a password"); - // TODO: Use SecurityManager.isLdapEmail() once that method is fixed. See #42435. - boolean ldapConfigured = null != AuthenticationManager.getLdapDomain(); - Collection ssoConfigs = AuthenticationManager.getActiveConfigurations(SSOAuthenticationConfiguration.class); - List authMethods = new LinkedList<>(); - String ssoDetails = ""; - if (ldapConfigured) - authMethods.add("LDAP"); + Collection formConfigs = AuthenticationManager.getActiveConfigurations(LoginFormAuthenticationConfiguration.class); + String ldapDetails = formConfigs.stream() + .filter(ac->null != ac.getDomain()) + .filter(ac->!AuthenticationManager.ALL_DOMAINS.equals(ac.getDomain())) + .filter(ac->StringUtils.endsWithIgnoreCase(emailAddress, "@" + ac.getDomain())) + .map(AuthenticationConfiguration::getDescription) + .collect(Collectors.joining(", ")); + if (!ldapDetails.isBlank()) + authMethods.add("LDAP (" + ldapDetails + ")"); + Collection ssoConfigs = AuthenticationManager.getActiveConfigurations(SSOAuthenticationConfiguration.class); if (!ssoConfigs.isEmpty()) { - authMethods.add("SSO"); - ssoDetails = " (" + + authMethods.add("SSO (" + ssoConfigs.stream() .map(AuthenticationConfiguration::getDescription) .collect(Collectors.joining(", ")) + - ")"; + ")" + ); } String guidance; @@ -1814,7 +1818,7 @@ HtmlString getConfirmationMessage(boolean loginExists, String emailAddress) if (authMethods.isEmpty()) guidance = "have no way to login!"; else - guidance = "be able to login via " + String.join(" or ", authMethods) + ssoDetails + " only."; + guidance = "be able to login via " + String.join(" or ", authMethods) + " only."; return HtmlString.of("Are you sure you want to delete the current password for " + emailAddress + "? Once deleted, this user will " + guidance); } diff --git a/core/src/org/labkey/core/security/addUsers.jsp b/core/src/org/labkey/core/security/addUsers.jsp index 427bf89da1d..052ec90e6aa 100644 --- a/core/src/org/labkey/core/security/addUsers.jsp +++ b/core/src/org/labkey/core/security/addUsers.jsp @@ -119,13 +119,7 @@

-

+

diff --git a/core/src/org/labkey/core/security/group.jsp b/core/src/org/labkey/core/security/group.jsp index dea0d780bab..6f13b899300 100644 --- a/core/src/org/labkey/core/security/group.jsp +++ b/core/src/org/labkey/core/security/group.jsp @@ -18,6 +18,7 @@ <%@ page import="org.apache.commons.lang3.StringUtils" %> <%@ page import="org.labkey.api.data.Container" %> <%@ page import="org.labkey.api.data.ContainerManager" %> +<%@ page import="org.labkey.api.security.AuthenticationManager" %> <%@ page import="org.labkey.api.security.Group" %> <%@ page import="org.labkey.api.security.PrincipalType" %> <%@ page import="org.labkey.api.security.SecurityUrls" %> @@ -298,12 +299,7 @@ else
Add New Members (enter one email address or group per line):
- Send notification emails to all new<% -if (null != bean.ldapDomain && bean.ldapDomain.length() != 0 && !org.labkey.api.security.AuthenticationManager.ALL_DOMAINS.equals(bean.ldapDomain)) -{ - %>, non-<%= h(bean.ldapDomain) %><% -} -%> users.

+ <%=AuthenticationManager.getStandardSendVerificationEmailsMessage()%>

Include a message with the new user mail (optional):

diff --git a/study/src/org/labkey/study/controllers/specimen/ShowGroupMembersAction.java b/study/src/org/labkey/study/controllers/specimen/ShowGroupMembersAction.java index 3ad5f598238..d477464aa70 100644 --- a/study/src/org/labkey/study/controllers/specimen/ShowGroupMembersAction.java +++ b/study/src/org/labkey/study/controllers/specimen/ShowGroupMembersAction.java @@ -17,7 +17,6 @@ import org.apache.commons.lang3.StringUtils; import org.labkey.api.action.FormViewAction; -import org.labkey.api.security.AuthenticationManager; import org.labkey.api.security.RequiresPermission; import org.labkey.api.security.SecurityManager; import org.labkey.api.security.SecurityUrls; @@ -235,7 +234,6 @@ public static class GroupMembersBean private final SpecimenRequestActor _actor; private final LocationImpl _location; private final User[] _members; - private final String _ldapDomain; private final ActionURL _returnUrl; public GroupMembersBean(SpecimenRequestActor actor, LocationImpl location, User[] members, ActionURL returnUrl) @@ -243,7 +241,6 @@ public GroupMembersBean(SpecimenRequestActor actor, LocationImpl location, User[ _actor = actor; _location = location; _members = members; - _ldapDomain = AuthenticationManager.getLdapDomain(); _returnUrl = returnUrl; } @@ -262,11 +259,6 @@ public LocationImpl getLocation() return _location; } - public String getLdapDomain() - { - return _ldapDomain; - } - public ActionURL getReturnUrl() { return _returnUrl; diff --git a/study/src/org/labkey/study/view/specimen/groupMembers.jsp b/study/src/org/labkey/study/view/specimen/groupMembers.jsp index 508ceac24dd..041e7576505 100644 --- a/study/src/org/labkey/study/view/specimen/groupMembers.jsp +++ b/study/src/org/labkey/study/view/specimen/groupMembers.jsp @@ -15,6 +15,7 @@ * limitations under the License. */ %> +<%@ page import="org.labkey.api.security.AuthenticationManager" %> <%@ page import="org.labkey.api.security.User" %> <%@ page import="org.labkey.api.view.HttpView" %> <%@ page import="org.labkey.api.view.JspView" %> @@ -68,14 +69,7 @@
- Send notification emails to all - new<% - if (bean.getLdapDomain() != null && bean.getLdapDomain().length() > 0 && !org.labkey.api.security.AuthenticationManager.ALL_DOMAINS.equals(bean.getLdapDomain())) - { - %>, non-<%= h(bean.getLdapDomain()) %> - <% - } - %> users

+ <%=AuthenticationManager.getStandardSendVerificationEmailsMessage()%>

<% From 05c8027285fc48585ed30f9e6425c607c24af2f5 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Wed, 31 Mar 2021 13:54:34 -0700 Subject: [PATCH 2/2] Fix spacing and broken permissions link on add users page Clarify some admin messages --- .../AuthenticationConfigurationCache.java | 3 +++ .../api/security/AuthenticationManager.java | 10 ++-------- .../labkey/api/security/SecurityManager.java | 4 ++-- .../labkey/core/login/LoginController.java | 2 +- .../src/org/labkey/core/security/addUsers.jsp | 19 +++++++++---------- 5 files changed, 17 insertions(+), 21 deletions(-) diff --git a/api/src/org/labkey/api/security/AuthenticationConfigurationCache.java b/api/src/org/labkey/api/security/AuthenticationConfigurationCache.java index 4f8477bbcfe..cbb3699591b 100644 --- a/api/src/org/labkey/api/security/AuthenticationConfigurationCache.java +++ b/api/src/org/labkey/api/security/AuthenticationConfigurationCache.java @@ -220,6 +220,9 @@ public static void clear() CACHE.remove(CACHE_KEY); } + /** + * Return a collection of all email domains associated with authentication configurations, not including "*" or null + */ public static @NotNull Collection getActiveDomains() { return CACHE.get(CACHE_KEY).getActiveDomains(); diff --git a/api/src/org/labkey/api/security/AuthenticationManager.java b/api/src/org/labkey/api/security/AuthenticationManager.java index 8da5c0393fb..609d192c201 100644 --- a/api/src/org/labkey/api/security/AuthenticationManager.java +++ b/api/src/org/labkey/api/security/AuthenticationManager.java @@ -200,16 +200,10 @@ public static void populateSettingsWithStartupProps() public enum Priority { High, Low } - // Return a collection of all email domains associated with authentication configurations, not including "*" or null - public static @NotNull Collection getActiveDomains() - { - return AuthenticationConfigurationCache.getActiveDomains(); - } - public static HtmlString getStandardSendVerificationEmailsMessage() { HtmlStringBuilder builder = HtmlStringBuilder.of("Send password verification emails to all new users"); - Collection activeDomains = getActiveDomains(); + Collection activeDomains = AuthenticationConfigurationCache.getActiveDomains(); if (!activeDomains.isEmpty()) { @@ -231,7 +225,7 @@ public static HtmlString getStandardSendVerificationEmailsMessage() public static boolean isLdapEmail(ValidEmail email) { String emailAddress = email.getEmailAddress(); - return getActiveDomains().stream() + return AuthenticationConfigurationCache.getActiveDomains().stream() .anyMatch(domain->StringUtils.endsWithIgnoreCase(emailAddress, "@" + domain)); } diff --git a/api/src/org/labkey/api/security/SecurityManager.java b/api/src/org/labkey/api/security/SecurityManager.java index f1f0b539072..3b2c8df51a3 100644 --- a/api/src/org/labkey/api/security/SecurityManager.java +++ b/api/src/org/labkey/api/security/SecurityManager.java @@ -2790,8 +2790,8 @@ public static HtmlString addUser(ViewContext context, ValidEmail email, boolean if (newUserStatus.isLdapEmail()) { - message.append(newUser.getEmail()).append(" added as a new user to the system. This user will be authenticated via LDAP."); - UserManager.addToUserHistory(newUser, newUser.getEmail() + " was added to the system. This user will be authenticated via LDAP."); + message.append(newUser.getEmail()).append(" added as a new user to the system and NOT emailed since this user will be authenticated via LDAP."); + UserManager.addToUserHistory(newUser, newUser.getEmail() + " was added to the system NOT emailed since this user will be authenticated via LDAP."); } else if (sendMail) { diff --git a/core/src/org/labkey/core/login/LoginController.java b/core/src/org/labkey/core/login/LoginController.java index 13dc2222a20..72d0684e668 100644 --- a/core/src/org/labkey/core/login/LoginController.java +++ b/core/src/org/labkey/core/login/LoginController.java @@ -1810,7 +1810,7 @@ else if (!user.isActive()) if (!SecurityManager.loginExists(email)) { if (AuthenticationManager.isLdapEmail(email)) - errors.reject("setPassword", "Your account will use your institution's LDAP authentication server and you do not need to set a separate password."); + errors.reject("setPassword", "Your account will authenticate using LDAP and you do not need to set a separate password."); else errors.reject("setPassword", "This email address is not associated with an account. Make sure you've copied the entire link into your browser's address bar."); } diff --git a/core/src/org/labkey/core/security/addUsers.jsp b/core/src/org/labkey/core/security/addUsers.jsp index 052ec90e6aa..5ac1c4d6a0d 100644 --- a/core/src/org/labkey/core/security/addUsers.jsp +++ b/core/src/org/labkey/core/security/addUsers.jsp @@ -70,7 +70,7 @@ { if (textElem.value != null && textElem.value.length > 0) { - var target = "<%=h(new UserUrlsImpl().getUserAccessURL(ContainerManager.getRoot()))%>newEmail=" + textElem.value; + var target = "<%=h(new UserUrlsImpl().getUserAccessURL(ContainerManager.getRoot()).addParameter("newEmail", null))%>" + textElem.value; window.open(target, "permissions", "height=450,width=500,scrollbars=yes,status=yes,toolbar=no,menubar=no,location=no,resizable=yes"); } } @@ -98,28 +98,27 @@ <% if (getErrors("form").hasErrors()); { %> - <% + <% } HtmlString msg = form.getMessage(); if (!HtmlString.isBlank(msg)) { - %><% + %><% } %> - + - - - - + -
<%=msg%>
<%=msg%>
Add new users. Enter one or more email addresses, each on its own line.Add new users. Enter one or more email addresses, each on its own line.
+

Clone permissions from user:Clone permissions from user: + permissions
-

+
+