Implement user "disabled" flag as an alternative to deleting the account

releases.moxie

@@ -78,6 +78,7 @@ r20: {
 	- Added FishEye hook script (pr-137)
 	- Added Redmine Fetch hook script (issue-359)
 	- Added Subgit hook contributed by TMate Software
+	- Added function to retain a user account but prohibit authentication. This is an alternative to deleting a user account.
     dependencyChanges:
 	- updated to Jetty 8.1.13
 	- updated to JGit 3.2.0

src/main/java/com/gitblit/ConfigUserService.java

@@ -98,6 +98,8 @@ public class ConfigUserService implements IUserService {
 
 	private static final String ACCOUNTTYPE = "accountType";
 
+	private static final String DISABLED = "disabled";
+
 	private final File realmFile;
 
 	private final Logger logger = LoggerFactory.getLogger(ConfigUserService.class);
@@ -701,6 +703,9 @@ public class ConfigUserService implements IUserService {
 			if (!StringUtils.isEmpty(model.countryCode)) {
 				config.setString(USER, model.username, COUNTRYCODE, model.countryCode);
 			}
+			if (model.disabled) {
+				config.setBoolean(USER, model.username, DISABLED, true);
+			}
 			if (model.getPreferences() != null) {
 				if (!StringUtils.isEmpty(model.getPreferences().locale)) {
 					config.setString(USER, model.username, LOCALE, model.getPreferences().locale);
@@ -868,6 +873,7 @@ public class ConfigUserService implements IUserService {
 					if (Constants.EXTERNAL_ACCOUNT.equals(user.password) && user.accountType.isLocal()) {
 						user.accountType = AccountType.EXTERNAL;
 					}
+					user.disabled = config.getBoolean(USER, username, DISABLED, false);
 					user.organizationalUnit = config.getString(USER, username, ORGANIZATIONALUNIT);
 					user.organization = config.getString(USER, username, ORGANIZATION);
 					user.locality = config.getString(USER, username, LOCALITY);

src/main/java/com/gitblit/client/EditUserDialog.java

@@ -92,6 +92,8 @@ public class EditUserDialog extends JDialog {
 
 	private JCheckBox notFederatedCheckbox;
 
+	private JCheckBox disabledCheckbox;
+
 	private JTextField organizationalUnitField;
 
 	private JTextField organizationField;
@@ -153,6 +155,7 @@ public class EditUserDialog extends JDialog {
 		notFederatedCheckbox = new JCheckBox(
 				Translation.get("gb.excludeFromFederationDescription"),
 				anUser.excludeFromFederation);
+		disabledCheckbox = new JCheckBox(Translation.get("gb.disableUserDescription"), anUser.disabled);
 
 		organizationalUnitField = new JTextField(anUser.organizationalUnit == null ? "" : anUser.organizationalUnit, 25);
 		organizationField = new JTextField(anUser.organization == null ? "" : anUser.organization, 25);
@@ -176,6 +179,7 @@ public class EditUserDialog extends JDialog {
 		fieldsPanel.add(newFieldPanel(Translation.get("gb.canCreate"), canCreateCheckbox));
 		fieldsPanel.add(newFieldPanel(Translation.get("gb.excludeFromFederation"),
 				notFederatedCheckbox));
+		fieldsPanel.add(newFieldPanel(Translation.get("gb.disableUser"), disabledCheckbox));
 
 		JPanel attributesPanel = new JPanel(new GridLayout(0, 1, 5, 2));
 		attributesPanel.add(newFieldPanel(Translation.get("gb.organizationalUnit") + " (OU)", organizationalUnitField));
@@ -355,6 +359,7 @@ public class EditUserDialog extends JDialog {
 		user.canFork = canForkCheckbox.isSelected();
 		user.canCreate = canCreateCheckbox.isSelected();
 		user.excludeFromFederation = notFederatedCheckbox.isSelected();
+		user.disabled = disabledCheckbox.isSelected();
 
 		user.organizationalUnit = organizationalUnitField.getText().trim();
 		user.organization = organizationField.getText().trim();

src/main/java/com/gitblit/manager/AuthenticationManager.java

@@ -198,7 +198,7 @@ public class AuthenticationManager implements IAuthenticationManager {
 						flagWicketSession(AuthenticationType.CONTAINER);
 						logger.debug(MessageFormat.format("{0} authenticated by servlet container principal from {1}",
 								user.username, httpRequest.getRemoteAddr()));
-						return user;
+						return validateAuthentication(user, AuthenticationType.CONTAINER);
 					} else if (settings.getBoolean(Keys.realm.container.autoCreateAccounts, false)
 							&& !internalAccount) {
 						// auto-create user from an authenticated container principal
@@ -210,7 +210,7 @@ public class AuthenticationManager implements IAuthenticationManager {
 						flagWicketSession(AuthenticationType.CONTAINER);
 						logger.debug(MessageFormat.format("{0} authenticated and created by servlet container principal from {1}",
 								user.username, httpRequest.getRemoteAddr()));
-						return user;
+						return validateAuthentication(user, AuthenticationType.CONTAINER);
 					} else if (!internalAccount) {
 						logger.warn(MessageFormat.format("Failed to find UserModel for {0}, attempted servlet container authentication from {1}",
 								principal.getName(), httpRequest.getRemoteAddr()));
@@ -231,7 +231,7 @@ public class AuthenticationManager implements IAuthenticationManager {
 				flagWicketSession(AuthenticationType.CERTIFICATE);
 				logger.debug(MessageFormat.format("{0} authenticated by client certificate {1} from {2}",
 						user.username, metadata.serialNumber, httpRequest.getRemoteAddr()));
-				return user;
+				return validateAuthentication(user, AuthenticationType.CERTIFICATE);
 			} else {
 				logger.warn(MessageFormat.format("Failed to find UserModel for {0}, attempted client certificate ({1}) authentication from {2}",
 						model.username, metadata.serialNumber, httpRequest.getRemoteAddr()));
@@ -253,7 +253,7 @@ public class AuthenticationManager implements IAuthenticationManager {
 				flagWicketSession(AuthenticationType.COOKIE);
 				logger.debug(MessageFormat.format("{0} authenticated by cookie from {1}",
 					user.username, httpRequest.getRemoteAddr()));
-				return user;
+				return validateAuthentication(user, AuthenticationType.COOKIE);
 			}
 		}
 
@@ -275,7 +275,7 @@ public class AuthenticationManager implements IAuthenticationManager {
 					flagWicketSession(AuthenticationType.CREDENTIALS);
 					logger.debug(MessageFormat.format("{0} authenticated by BASIC request header from {1}",
 							user.username, httpRequest.getRemoteAddr()));
-					return user;
+					return validateAuthentication(user, AuthenticationType.CREDENTIALS);
 				} else {
 					logger.warn(MessageFormat.format("Failed login attempt for {0}, invalid credentials from {1}",
 							username, httpRequest.getRemoteAddr()));
@@ -285,6 +285,27 @@ public class AuthenticationManager implements IAuthenticationManager {
 		return null;
 	}
 
+	/**
+	 * This method allows the authentication manager to reject authentication
+	 * attempts.  It is called after the username/secret have been verified to
+	 * ensure that the authentication technique has been logged.
+	 *
+	 * @param user
+	 * @return
+	 */
+	protected UserModel validateAuthentication(UserModel user, AuthenticationType type) {
+		if (user == null) {
+			return null;
+		}
+		if (user.disabled) {
+			// user has been disabled
+			logger.warn("Rejected {} authentication attempt by disabled account \"{}\"",
+					type, user.username);
+			return null;
+		}
+		return user;
+	}
+
 	protected void flagWicketSession(AuthenticationType authenticationType) {
 		RequestCycle requestCycle = RequestCycle.get();
 		if (requestCycle != null) {
@@ -338,7 +359,7 @@ public class AuthenticationManager implements IAuthenticationManager {
 				// plain-text password
 				returnedUser = user;
 			}
-			return returnedUser;
+			return validateAuthentication(returnedUser, AuthenticationType.CREDENTIALS);
 		}
 
 		// try registered external authentication providers
@@ -349,12 +370,12 @@ public class AuthenticationManager implements IAuthenticationManager {
 					if (user != null) {
 						// user authenticated
 						user.accountType = provider.getAccountType();
-						return user;
+						return validateAuthentication(user, AuthenticationType.CREDENTIALS);
 					}
 				}
 			}
 		}
-		return user;
+		return validateAuthentication(user, AuthenticationType.CREDENTIALS);
 	}
 
 	/**

src/main/java/com/gitblit/models/UserModel.java

@@ -67,6 +67,7 @@ public class UserModel implements Principal, Serializable, Comparable<UserModel>
 	public boolean canFork;
 	public boolean canCreate;
 	public boolean excludeFromFederation;
+	public boolean disabled;
 	// retained for backwards-compatibility with RPC clients
 	@Deprecated
 	public final Set<String> repositories = new HashSet<String>();

src/main/java/com/gitblit/wicket/GitBlitWebApp.properties

@@ -648,4 +648,6 @@ gb.looksGood = looks good
 gb.approve = approve
 gb.hasNotReviewed = has not reviewed
 gb.about = about
-gb.ticketN = ticket #{0}
+gb.ticketN = ticket #{0}
+gb.disableUser = disable user
+gb.disableUserDescription = prevent this account from authenticating

src/main/java/com/gitblit/wicket/pages/EditUserPage.html

@@ -31,10 +31,11 @@
 				<tr><th><wicket:message key="gb.confirmPassword"></wicket:message></th><td class="edit"><input type="password" wicket:id="confirmPassword" size="30" tabindex="3" /></td></tr>
 				<tr><th><wicket:message key="gb.displayName"></wicket:message></th><td class="edit"><input type="text" wicket:id="displayName" size="30" tabindex="4" /></td></tr>
 				<tr><th><wicket:message key="gb.emailAddress"></wicket:message></th><td class="edit"><input type="text" wicket:id="emailAddress" size="30" tabindex="5" /></td></tr>
-				<tr><th><wicket:message key="gb.canAdmin"></wicket:message></th><td class="edit"><label class="checkbox"><input type="checkbox" wicket:id="canAdmin" tabindex="6" /> &nbsp;<span class="help-inline"><wicket:message key="gb.canAdminDescription"></wicket:message></span></label></td></tr>				
-				<tr><th><wicket:message key="gb.canCreate"></wicket:message></th><td class="edit"><label class="checkbox"><input type="checkbox" wicket:id="canCreate" tabindex="7" /> &nbsp;<span class="help-inline"><wicket:message key="gb.canCreateDescription"></wicket:message></span></label></td></tr>				
-				<tr><th><wicket:message key="gb.canFork"></wicket:message></th><td class="edit"><label class="checkbox"><input type="checkbox" wicket:id="canFork" tabindex="8" /> &nbsp;<span class="help-inline"><wicket:message key="gb.canForkDescription"></wicket:message></span></label></td></tr>				
+				<tr><th><wicket:message key="gb.canAdmin"></wicket:message></th><td class="edit"><label class="checkbox"><input type="checkbox" wicket:id="canAdmin" tabindex="6" /> &nbsp;<span class="help-inline"><wicket:message key="gb.canAdminDescription"></wicket:message></span></label></td></tr>
+				<tr><th><wicket:message key="gb.canCreate"></wicket:message></th><td class="edit"><label class="checkbox"><input type="checkbox" wicket:id="canCreate" tabindex="7" /> &nbsp;<span class="help-inline"><wicket:message key="gb.canCreateDescription"></wicket:message></span></label></td></tr>
+				<tr><th><wicket:message key="gb.canFork"></wicket:message></th><td class="edit"><label class="checkbox"><input type="checkbox" wicket:id="canFork" tabindex="8" /> &nbsp;<span class="help-inline"><wicket:message key="gb.canForkDescription"></wicket:message></span></label></td></tr>
 				<tr><th><wicket:message key="gb.excludeFromFederation"></wicket:message></th><td class="edit"><label class="checkbox"><input type="checkbox" wicket:id="excludeFromFederation" tabindex="9" /> &nbsp;<span class="help-inline"><wicket:message key="gb.excludeFromFederationDescription"></wicket:message></span></label></td></tr>				
+				<tr><th><wicket:message key="gb.disableUser"></wicket:message></th><td class="edit"><label class="checkbox"><input type="checkbox" wicket:id="disabled" tabindex="10" /> &nbsp;<span class="help-inline"><wicket:message key="gb.disableUserDescription"></wicket:message></span></label></td></tr>
 			</tbody>
 		</table>
 		</div>

src/main/java/com/gitblit/wicket/pages/EditUserPage.java

@@ -243,6 +243,8 @@ public class EditUserPage extends RootSubPage {
 		form.add(new CheckBox("canFork").setEnabled(app().settings().getBoolean(Keys.web.allowForking, true)));
 		form.add(new CheckBox("canCreate"));
 		form.add(new CheckBox("excludeFromFederation"));
+		form.add(new CheckBox("disabled"));
+
 		form.add(new RegistrantPermissionsPanel("repositories",	RegistrantType.REPOSITORY, repos, permissions, getAccessPermissions()));
 		form.add(teams.setEnabled(editTeams));
 

src/main/java/com/gitblit/wicket/pages/SessionPage.java

@@ -56,6 +56,16 @@ public abstract class SessionPage extends WebPage {
 			// any changes to permissions or roles (issue-186)
 			UserModel user = app().users().getUserModel(session.getUser().username);
 
+			if (user.disabled) {
+				// user was disabled during session
+				HttpServletResponse response = ((WebResponse) getRequestCycle().getResponse())
+						.getHttpServletResponse();
+				app().authentication().logout(response, user);
+				session.setUser(null);
+				session.invalidateNow();
+				return;
+			}
+
 			// validate cookie during session (issue-361)
 			if (user != null && app().settings().getBoolean(Keys.web.allowCookieAuthentication, true)) {
 				HttpServletRequest request = ((WebRequest) getRequestCycle().getRequest())

src/main/java/com/gitblit/wicket/panels/UsersPanel.java

@@ -57,7 +57,8 @@ public class UsersPanel extends BasePanel {
 			@Override
 			public void populateItem(final Item<UserModel> item) {
 				final UserModel entry = item.getModelObject();
-				LinkPanel editLink = new LinkPanel("username", "list", entry.username,
+				String css = "list" + (entry.disabled ? "-strikethrough":"");
+				LinkPanel editLink = new LinkPanel("username", css, entry.username,
 						EditUserPage.class, WicketUtils.newUsernameParameter(entry.username));
 				WicketUtils.setHtmlTooltip(editLink, getString("gb.edit") + " " + entry.getDisplayName());
 				item.add(editLink);
@@ -65,7 +66,7 @@ public class UsersPanel extends BasePanel {
 				if (StringUtils.isEmpty(entry.displayName)) {
 					item.add(new Label("displayName").setVisible(false));
 				} else {
-					editLink = new LinkPanel("displayName", "list", entry.getDisplayName(),
+					editLink = new LinkPanel("displayName", css, entry.getDisplayName(),
 						EditUserPage.class, WicketUtils.newUsernameParameter(entry.username));
 					WicketUtils.setHtmlTooltip(editLink, getString("gb.edit") + " " + entry.getDisplayName());
 					item.add(editLink);
@@ -74,7 +75,7 @@ public class UsersPanel extends BasePanel {
 				if (StringUtils.isEmpty(entry.emailAddress)) {
 					item.add(new Label("emailAddress").setVisible(false));
 				} else {
-					editLink = new LinkPanel("emailAddress", "list", entry.emailAddress,
+					editLink = new LinkPanel("emailAddress", css, entry.emailAddress,
 						EditUserPage.class, WicketUtils.newUsernameParameter(entry.username));
 					WicketUtils.setHtmlTooltip(editLink, getString("gb.edit") + " " + entry.getDisplayName());
 					item.add(editLink);

src/main/resources/gitblit.css

@@ -55,7 +55,7 @@ a.bugtraq {
 	white-space: nowrap;
 	vertical-align: baseline;
 }
-
+
 [class^="icon-"], [class*=" icon-"] i {
 	/* override for a links that look like bootstrap buttons */
 	vertical-align: text-bottom;
@@ -993,6 +993,11 @@ a.list {
 	color: inherit;
 }
 
+a.list-strikethrough {
+	text-decoration: line-through;
+	color: inherit;
+}
+
 a.list.subject {
 	font-weight: bold;
 }

src/test/java/com/gitblit/tests/AuthenticationManagerTest.java

@@ -0,0 +1,66 @@
+/*
+ * Copyright 2013 gitblit.com.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.gitblit.tests;
+
+import java.util.HashMap;
+
+import org.junit.Test;
+
+import com.gitblit.manager.AuthenticationManager;
+import com.gitblit.manager.IAuthenticationManager;
+import com.gitblit.manager.IUserManager;
+import com.gitblit.manager.RuntimeManager;
+import com.gitblit.manager.UserManager;
+import com.gitblit.models.UserModel;
+import com.gitblit.tests.mock.MemorySettings;
+
+/**
+ * Class for testing local authentication.
+ *
+ * @author James Moger
+ *
+ */
+public class AuthenticationManagerTest extends GitblitUnitTest {
+
+	IUserManager users;
+
+    MemorySettings getSettings() {
+    	return new MemorySettings(new HashMap<String, Object>());
+    }
+
+    IAuthenticationManager newAuthenticationManager() {
+    	RuntimeManager runtime = new RuntimeManager(getSettings(), GitBlitSuite.BASEFOLDER).start();
+    	users = new UserManager(runtime).start();
+    	AuthenticationManager auth = new AuthenticationManager(runtime, users).start();
+    	return auth;
+    }
+
+    @Test
+    public void testAuthenticate() throws Exception {
+    	IAuthenticationManager auth = newAuthenticationManager();
+
+    	UserModel user = new UserModel("sunnyjim");
+		user.password = "password";
+		users.updateUserModel(user);
+
+		assertNotNull(auth.authenticate(user.username, user.password.toCharArray()));
+		user.disabled = true;
+
+		users.updateUserModel(user);
+		assertNull(auth.authenticate(user.username, user.password.toCharArray()));
+		users.deleteUserModel(user);
+    }
+}

src/test/java/com/gitblit/tests/GitBlitSuite.java

@@ -64,7 +64,7 @@ import com.gitblit.utils.JGitUtils;
 		GroovyScriptTest.class, LuceneExecutorTest.class, RepositoryModelTest.class,
 		FanoutServiceTest.class, Issue0259Test.class, Issue0271Test.class, HtpasswdAuthenticationTest.class,
 		ModelUtilsTest.class, JnaUtilsTest.class, LdapSyncServiceTest.class, FileTicketServiceTest.class,
-		BranchTicketServiceTest.class, RedisTicketServiceTest.class  })
+		BranchTicketServiceTest.class, RedisTicketServiceTest.class, AuthenticationManagerTest.class })
 public class GitBlitSuite {
 
 	public static final File BASEFOLDER = new File("data");