From 06fa257080ab8de7986ad5debbf317eddf5608e7 Mon Sep 17 00:00:00 2001 From: Alex Lewis <alex.lewis001@gmail.com> Date: Tue, 10 Dec 2013 11:41:36 -0500 Subject: [PATCH] issue-350: Fixes issue when an apostrophe is present in a User's name --- src/main/java/com/gitblit/ConfigUserService.java | 333 +++++++++++++++---------------------------------------- 1 files changed, 92 insertions(+), 241 deletions(-) diff --git a/src/main/java/com/gitblit/ConfigUserService.java b/src/main/java/com/gitblit/ConfigUserService.java index 8a6c92f..aae7c14 100644 --- a/src/main/java/com/gitblit/ConfigUserService.java +++ b/src/main/java/com/gitblit/ConfigUserService.java @@ -35,6 +35,8 @@ import org.slf4j.LoggerFactory; import com.gitblit.Constants.AccessPermission; +import com.gitblit.Constants.AccountType; +import com.gitblit.manager.IRuntimeManager; import com.gitblit.models.TeamModel; import com.gitblit.models.UserModel; import com.gitblit.models.UserRepositoryPreferences; @@ -45,16 +47,16 @@ /** * ConfigUserService is Gitblit's default user service implementation since * version 0.8.0. - * + * * Users and their repository memberships are stored in a git-style config file * which is cached and dynamically reloaded when modified. This file is * plain-text, human-readable, and may be edited with a text editor. - * + * * Additionally, this format allows for expansion of the user model without * bringing in the complexity of a database. - * + * * @author James Moger - * + * */ public class ConfigUserService implements IUserService { @@ -63,21 +65,21 @@ private static final String USER = "user"; private static final String PASSWORD = "password"; - + private static final String DISPLAYNAME = "displayName"; - + private static final String EMAILADDRESS = "emailAddress"; - + private static final String ORGANIZATIONALUNIT = "organizationalUnit"; - + private static final String ORGANIZATION = "organization"; - + private static final String LOCALITY = "locality"; - + private static final String STATEPROVINCE = "stateProvince"; - + private static final String COUNTRYCODE = "countryCode"; - + private static final String COOKIE = "cookie"; private static final String REPOSITORY = "repository"; @@ -89,10 +91,12 @@ private static final String PRERECEIVE = "preReceiveScript"; private static final String POSTRECEIVE = "postReceiveScript"; - + private static final String STARRED = "starred"; - + private static final String LOCALE = "locale"; + + private static final String ACCOUNTTYPE = "accountType"; private final File realmFile; @@ -105,7 +109,7 @@ private final Map<String, TeamModel> teams = new ConcurrentHashMap<String, TeamModel>(); private volatile long lastModified; - + private volatile boolean forceReload; public ConfigUserService(File realmFile) { @@ -114,70 +118,17 @@ /** * Setup the user service. - * - * @param settings - * @since 0.7.0 + * + * @param runtimeManager + * @since 1.4.0 */ @Override - public void setup(IStoredSettings settings) { - } - - /** - * Does the user service support changes to credentials? - * - * @return true or false - * @since 1.0.0 - */ - @Override - public boolean supportsCredentialChanges() { - return true; - } - - /** - * Does the user service support changes to user display name? - * - * @return true or false - * @since 1.0.0 - */ - @Override - public boolean supportsDisplayNameChanges() { - return true; - } - - /** - * Does the user service support changes to user email address? - * - * @return true or false - * @since 1.0.0 - */ - @Override - public boolean supportsEmailAddressChanges() { - return true; - } - - /** - * Does the user service support changes to team memberships? - * - * @return true or false - * @since 1.0.0 - */ - public boolean supportsTeamMembershipChanges() { - return true; - } - - /** - * Does the user service support cookie authentication? - * - * @return true or false - */ - @Override - public boolean supportsCookies() { - return true; + public void setup(IRuntimeManager runtimeManager) { } /** * Returns the cookie value for the specified user. - * + * * @param model * @return cookie value */ @@ -186,19 +137,21 @@ if (!StringUtils.isEmpty(model.cookie)) { return model.cookie; } - read(); - UserModel storedModel = users.get(model.username.toLowerCase()); + UserModel storedModel = getUserModel(model.username); + if (storedModel == null) { + return null; + } return storedModel.cookie; } /** - * Authenticate a user based on their cookie. - * + * Gets the user object for the specified cookie. + * * @param cookie * @return a user object or null */ @Override - public UserModel authenticate(char[] cookie) { + public synchronized UserModel getUserModel(char[] cookie) { String hash = new String(cookie); if (StringUtils.isEmpty(hash)) { return null; @@ -208,61 +161,23 @@ if (cookies.containsKey(hash)) { model = cookies.get(hash); } + + if (model != null) { + // clone the model, otherwise all changes to this object are + // live and unpersisted + model = DeepCopier.copy(model); + } return model; } /** - * Authenticate a user based on a username and password. - * - * @param username - * @param password - * @return a user object or null - */ - @Override - public UserModel authenticate(String username, char[] password) { - read(); - UserModel returnedUser = null; - UserModel user = getUserModel(username); - if (user == null) { - return null; - } - if (user.password.startsWith(StringUtils.MD5_TYPE)) { - // password digest - String md5 = StringUtils.MD5_TYPE + StringUtils.getMD5(new String(password)); - if (user.password.equalsIgnoreCase(md5)) { - returnedUser = user; - } - } else if (user.password.startsWith(StringUtils.COMBINED_MD5_TYPE)) { - // username+password digest - String md5 = StringUtils.COMBINED_MD5_TYPE - + StringUtils.getMD5(username.toLowerCase() + new String(password)); - if (user.password.equalsIgnoreCase(md5)) { - returnedUser = user; - } - } else if (user.password.equals(new String(password))) { - // plain-text password - returnedUser = user; - } - return returnedUser; - } - - /** - * Logout a user. - * - * @param user - */ - @Override - public void logout(UserModel user) { - } - - /** * Retrieve the user object for the specified username. - * + * * @param username * @return a user object or null */ @Override - public UserModel getUserModel(String username) { + public synchronized UserModel getUserModel(String username) { read(); UserModel model = users.get(username.toLowerCase()); if (model != null) { @@ -275,7 +190,7 @@ /** * Updates/writes a complete user object. - * + * * @param model * @return true if update is successful */ @@ -286,13 +201,13 @@ /** * Updates/writes all specified user objects. - * + * * @param models a list of user models * @return true if update is successful * @since 1.2.0 */ @Override - public boolean updateUserModels(Collection<UserModel> models) { + public synchronized boolean updateUserModels(Collection<UserModel> models) { try { read(); for (UserModel model : models) { @@ -310,7 +225,7 @@ } else { // do not clobber existing team definition // maybe because this is a federated user - t.addUser(model.username); + t.addUser(model.username); } } @@ -336,7 +251,7 @@ /** * Updates/writes and replaces a complete user object keyed by username. * This method allows for renaming a user. - * + * * @param username * the old username * @param model @@ -344,9 +259,13 @@ * @return true if update is successful */ @Override - public boolean updateUserModel(String username, UserModel model) { + public synchronized boolean updateUserModel(String username, UserModel model) { UserModel originalUser = null; try { + if (!model.isLocalAccount()) { + // do not persist password + model.password = Constants.EXTERNAL_ACCOUNT; + } read(); originalUser = users.remove(username.toLowerCase()); users.put(model.username.toLowerCase(), model); @@ -394,7 +313,7 @@ /** * Deletes the user object from the user service. - * + * * @param model * @return true if successful */ @@ -405,12 +324,12 @@ /** * Delete the user object with the specified username - * + * * @param username * @return true if successful */ @Override - public boolean deleteUser(String username) { + public synchronized boolean deleteUser(String username) { try { // Read realm file read(); @@ -441,7 +360,7 @@ /** * Returns the list of all teams available to the login service. - * + * * @return list of all teams * @since 0.8.0 */ @@ -455,12 +374,12 @@ /** * Returns the list of all teams available to the login service. - * + * * @return list of all teams * @since 0.8.0 */ @Override - public List<TeamModel> getAllTeams() { + public synchronized List<TeamModel> getAllTeams() { read(); List<TeamModel> list = new ArrayList<TeamModel>(teams.values()); list = DeepCopier.copy(list); @@ -471,13 +390,13 @@ /** * Returns the list of all users who are allowed to bypass the access * restriction placed on the specified repository. - * + * * @param role * the repository name * @return list of all usernames that can bypass the access restriction */ @Override - public List<String> getTeamnamesForRepositoryRole(String role) { + public synchronized List<String> getTeamNamesForRepositoryRole(String role) { List<String> list = new ArrayList<String>(); try { read(); @@ -495,53 +414,14 @@ } /** - * Sets the list of all teams who are allowed to bypass the access - * restriction placed on the specified repository. - * - * @param role - * the repository name - * @param teamnames - * @return true if successful - */ - @Override - public boolean setTeamnamesForRepositoryRole(String role, List<String> teamnames) { - try { - Set<String> specifiedTeams = new HashSet<String>(); - for (String teamname : teamnames) { - specifiedTeams.add(teamname.toLowerCase()); - } - - read(); - - // identify teams which require add or remove role - for (TeamModel team : teams.values()) { - // team has role, check against revised team list - if (specifiedTeams.contains(team.name.toLowerCase())) { - team.addRepositoryPermission(role); - } else { - // remove role from team - team.removeRepositoryPermission(role); - } - } - - // persist changes - write(); - return true; - } catch (Throwable t) { - logger.error(MessageFormat.format("Failed to set teams for role {0}!", role), t); - } - return false; - } - - /** * Retrieve the team object for the specified team name. - * + * * @param teamname * @return a team object or null * @since 0.8.0 */ @Override - public TeamModel getTeamModel(String teamname) { + public synchronized TeamModel getTeamModel(String teamname) { read(); TeamModel model = teams.get(teamname.toLowerCase()); if (model != null) { @@ -554,7 +434,7 @@ /** * Updates/writes a complete team object. - * + * * @param model * @return true if update is successful * @since 0.8.0 @@ -566,7 +446,7 @@ /** * Updates/writes all specified team objects. - * + * * @param models a list of team models * @return true if update is successful * @since 1.2.0 @@ -589,7 +469,7 @@ /** * Updates/writes and replaces a complete team object keyed by teamname. * This method allows for renaming a team. - * + * * @param teamname * the old teamname * @param model @@ -621,7 +501,7 @@ /** * Deletes the team object from the user service. - * + * * @param model * @return true if successful * @since 0.8.0 @@ -633,7 +513,7 @@ /** * Delete the team object with the specified teamname - * + * * @param teamname * @return true if successful * @since 0.8.0 @@ -654,7 +534,7 @@ /** * Returns the list of all users available to the login service. - * + * * @return list of all usernames */ @Override @@ -664,31 +544,31 @@ Collections.sort(list); return list; } - + /** * Returns the list of all users available to the login service. - * + * * @return list of all usernames */ @Override - public List<UserModel> getAllUsers() { + public synchronized List<UserModel> getAllUsers() { read(); List<UserModel> list = new ArrayList<UserModel>(users.values()); list = DeepCopier.copy(list); Collections.sort(list); return list; - } + } /** * Returns the list of all users who are allowed to bypass the access * restriction placed on the specified repository. - * + * * @param role * the repository name * @return list of all usernames that can bypass the access restriction */ @Override - public List<String> getUsernamesForRepositoryRole(String role) { + public synchronized List<String> getUsernamesForRepositoryRole(String role) { List<String> list = new ArrayList<String>(); try { read(); @@ -706,54 +586,14 @@ } /** - * Sets the list of all uses who are allowed to bypass the access - * restriction placed on the specified repository. - * - * @param role - * the repository name - * @param usernames - * @return true if successful - */ - @Override - @Deprecated - public boolean setUsernamesForRepositoryRole(String role, List<String> usernames) { - try { - Set<String> specifiedUsers = new HashSet<String>(); - for (String username : usernames) { - specifiedUsers.add(username.toLowerCase()); - } - - read(); - - // identify users which require add or remove role - for (UserModel user : users.values()) { - // user has role, check against revised user list - if (specifiedUsers.contains(user.username.toLowerCase())) { - user.addRepositoryPermission(role); - } else { - // remove role from user - user.removeRepositoryPermission(role); - } - } - - // persist changes - write(); - return true; - } catch (Throwable t) { - logger.error(MessageFormat.format("Failed to set usernames for role {0}!", role), t); - } - return false; - } - - /** * Renames a repository role. - * + * * @param oldRole * @param newRole * @return true if successful */ @Override - public boolean renameRepositoryRole(String oldRole, String newRole) { + public synchronized boolean renameRepositoryRole(String oldRole, String newRole) { try { read(); // identify users which require role rename @@ -783,12 +623,12 @@ /** * Removes a repository role from all users. - * + * * @param role * @return true if successful */ @Override - public boolean deleteRepositoryRole(String role) { + public synchronized boolean deleteRepositoryRole(String role) { try { read(); @@ -813,7 +653,7 @@ /** * Writes the properties file. - * + * * @throws IOException */ private synchronized void write() throws IOException { @@ -835,6 +675,9 @@ } if (!StringUtils.isEmpty(model.emailAddress)) { config.setString(USER, model.username, EMAILADDRESS, model.emailAddress); + } + if (model.accountType != null) { + config.setString(USER, model.username, ACCOUNTTYPE, model.accountType.name()); } if (!StringUtils.isEmpty(model.organizationalUnit)) { config.setString(USER, model.username, ORGANIZATIONALUNIT, model.organizationalUnit); @@ -889,7 +732,7 @@ } config.setStringList(USER, model.username, REPOSITORY, permissions); } - + // user preferences if (model.getPreferences() != null) { List<String> starred = model.getPreferences().getStarredRepositories(); @@ -918,7 +761,10 @@ roles.add(Constants.NO_ROLE); } config.setStringList(TEAM, model.name, ROLE, roles); - + if (model.accountType != null) { + config.setString(TEAM, model.name, ACCOUNTTYPE, model.accountType.name()); + } + if (!model.canAdmin) { // write team permission for non-admin teams if (model.permissions == null) { @@ -1008,16 +854,20 @@ Set<String> usernames = config.getSubsections(USER); for (String username : usernames) { UserModel user = new UserModel(username.toLowerCase()); - user.password = config.getString(USER, username, PASSWORD); + user.password = config.getString(USER, username, PASSWORD); user.displayName = config.getString(USER, username, DISPLAYNAME); user.emailAddress = config.getString(USER, username, EMAILADDRESS); + user.accountType = AccountType.fromString(config.getString(USER, username, ACCOUNTTYPE)); + if (Constants.EXTERNAL_ACCOUNT.equals(user.password) && user.accountType.isLocal()) { + user.accountType = null; + } user.organizationalUnit = config.getString(USER, username, ORGANIZATIONALUNIT); user.organization = config.getString(USER, username, ORGANIZATION); user.locality = config.getString(USER, username, LOCALITY); user.stateProvince = config.getString(USER, username, STATEPROVINCE); user.countryCode = config.getString(USER, username, COUNTRYCODE); user.cookie = config.getString(USER, username, COOKIE); - user.getPreferences().locale = config.getString(USER, username, LOCALE); + user.getPreferences().locale = config.getString(USER, username, LOCALE); if (StringUtils.isEmpty(user.cookie) && !StringUtils.isEmpty(user.password)) { user.cookie = StringUtils.getSHA1(user.username + user.password); } @@ -1064,7 +914,8 @@ team.canAdmin = roles.contains(Constants.ADMIN_ROLE); team.canFork = roles.contains(Constants.FORK_ROLE); team.canCreate = roles.contains(Constants.CREATE_ROLE); - + team.accountType = AccountType.fromString(config.getString(TEAM, teamname, ACCOUNTTYPE)); + if (!team.canAdmin) { // non-admin team, read permissions team.addRepositoryPermissions(Arrays.asList(config.getStringList(TEAM, teamname, -- Gitblit v1.9.1