From 9aa11943f821cb6c10a6d1c41c3d2381676f5047 Mon Sep 17 00:00:00 2001
From: James Moger <james.moger@gitblit.com>
Date: Tue, 04 Mar 2014 17:29:02 -0500
Subject: [PATCH] Implement user "disabled" flag as an alternative to deleting the account
---
src/main/java/com/gitblit/wicket/pages/SessionPage.java | 10 +++
src/main/java/com/gitblit/wicket/panels/UsersPanel.java | 7 +-
src/main/java/com/gitblit/client/EditUserDialog.java | 5 +
src/main/java/com/gitblit/wicket/GitBlitWebApp.properties | 4 +
src/main/java/com/gitblit/wicket/pages/EditUserPage.java | 2
src/test/java/com/gitblit/tests/GitBlitSuite.java | 2
src/main/java/com/gitblit/models/UserModel.java | 1
src/test/java/com/gitblit/tests/AuthenticationManagerTest.java | 66 ++++++++++++++++++++++
src/main/java/com/gitblit/manager/AuthenticationManager.java | 37 +++++++++--
src/main/resources/gitblit.css | 7 ++
releases.moxie | 1
src/main/java/com/gitblit/ConfigUserService.java | 6 ++
src/main/java/com/gitblit/wicket/pages/EditUserPage.html | 7 +-
13 files changed, 138 insertions(+), 17 deletions(-)
diff --git a/releases.moxie b/releases.moxie
index 0f4a75e..28a6235 100644
--- a/releases.moxie
+++ b/releases.moxie
@@ -78,6 +78,7 @@
- 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
diff --git a/src/main/java/com/gitblit/ConfigUserService.java b/src/main/java/com/gitblit/ConfigUserService.java
index a2a3277..9b4dd7f 100644
--- a/src/main/java/com/gitblit/ConfigUserService.java
+++ b/src/main/java/com/gitblit/ConfigUserService.java
@@ -98,6 +98,8 @@
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 @@
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 @@
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);
diff --git a/src/main/java/com/gitblit/client/EditUserDialog.java b/src/main/java/com/gitblit/client/EditUserDialog.java
index ab3ea67..676916b 100644
--- a/src/main/java/com/gitblit/client/EditUserDialog.java
+++ b/src/main/java/com/gitblit/client/EditUserDialog.java
@@ -92,6 +92,8 @@
private JCheckBox notFederatedCheckbox;
+ private JCheckBox disabledCheckbox;
+
private JTextField organizationalUnitField;
private JTextField organizationField;
@@ -153,6 +155,7 @@
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 @@
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 @@
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();
diff --git a/src/main/java/com/gitblit/manager/AuthenticationManager.java b/src/main/java/com/gitblit/manager/AuthenticationManager.java
index 4897514..ad4a985 100644
--- a/src/main/java/com/gitblit/manager/AuthenticationManager.java
+++ b/src/main/java/com/gitblit/manager/AuthenticationManager.java
@@ -198,7 +198,7 @@
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 @@
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 @@
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 @@
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 @@
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()));
@@ -283,6 +283,27 @@
}
}
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) {
@@ -338,7 +359,7 @@
// plain-text password
returnedUser = user;
}
- return returnedUser;
+ return validateAuthentication(returnedUser, AuthenticationType.CREDENTIALS);
}
// try registered external authentication providers
@@ -349,12 +370,12 @@
if (user != null) {
// user authenticated
user.accountType = provider.getAccountType();
- return user;
+ return validateAuthentication(user, AuthenticationType.CREDENTIALS);
}
}
}
}
- return user;
+ return validateAuthentication(user, AuthenticationType.CREDENTIALS);
}
/**
diff --git a/src/main/java/com/gitblit/models/UserModel.java b/src/main/java/com/gitblit/models/UserModel.java
index 63208f3..fbf311a 100644
--- a/src/main/java/com/gitblit/models/UserModel.java
+++ b/src/main/java/com/gitblit/models/UserModel.java
@@ -67,6 +67,7 @@
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>();
diff --git a/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties b/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties
index 86dd585..49a57e9 100644
--- a/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties
+++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties
@@ -648,4 +648,6 @@
gb.approve = approve
gb.hasNotReviewed = has not reviewed
gb.about = about
-gb.ticketN = ticket #{0}
\ No newline at end of file
+gb.ticketN = ticket #{0}
+gb.disableUser = disable user
+gb.disableUserDescription = prevent this account from authenticating
\ No newline at end of file
diff --git a/src/main/java/com/gitblit/wicket/pages/EditUserPage.html b/src/main/java/com/gitblit/wicket/pages/EditUserPage.html
index e78e44e..3bccdd5 100644
--- a/src/main/java/com/gitblit/wicket/pages/EditUserPage.html
+++ b/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" /> <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" /> <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" /> <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" /> <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" /> <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" /> <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" /> <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" /> <span class="help-inline"><wicket:message key="gb.disableUserDescription"></wicket:message></span></label></td></tr>
</tbody>
</table>
</div>
diff --git a/src/main/java/com/gitblit/wicket/pages/EditUserPage.java b/src/main/java/com/gitblit/wicket/pages/EditUserPage.java
index 15c35fa..b9a8480 100644
--- a/src/main/java/com/gitblit/wicket/pages/EditUserPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/EditUserPage.java
@@ -243,6 +243,8 @@
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));
diff --git a/src/main/java/com/gitblit/wicket/pages/SessionPage.java b/src/main/java/com/gitblit/wicket/pages/SessionPage.java
index 8065c5a..909342a 100644
--- a/src/main/java/com/gitblit/wicket/pages/SessionPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/SessionPage.java
@@ -56,6 +56,16 @@
// 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())
diff --git a/src/main/java/com/gitblit/wicket/panels/UsersPanel.java b/src/main/java/com/gitblit/wicket/panels/UsersPanel.java
index ed990c8..5d62655 100644
--- a/src/main/java/com/gitblit/wicket/panels/UsersPanel.java
+++ b/src/main/java/com/gitblit/wicket/panels/UsersPanel.java
@@ -57,7 +57,8 @@
@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 @@
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 @@
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);
diff --git a/src/main/resources/gitblit.css b/src/main/resources/gitblit.css
index 7894770..748a319 100644
--- a/src/main/resources/gitblit.css
+++ b/src/main/resources/gitblit.css
@@ -55,7 +55,7 @@
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 @@
color: inherit;
}
+a.list-strikethrough {
+ text-decoration: line-through;
+ color: inherit;
+}
+
a.list.subject {
font-weight: bold;
}
diff --git a/src/test/java/com/gitblit/tests/AuthenticationManagerTest.java b/src/test/java/com/gitblit/tests/AuthenticationManagerTest.java
new file mode 100644
index 0000000..84a2b74
--- /dev/null
+++ b/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);
+ }
+}
diff --git a/src/test/java/com/gitblit/tests/GitBlitSuite.java b/src/test/java/com/gitblit/tests/GitBlitSuite.java
index cba575d..c015c84 100644
--- a/src/test/java/com/gitblit/tests/GitBlitSuite.java
+++ b/src/test/java/com/gitblit/tests/GitBlitSuite.java
@@ -64,7 +64,7 @@
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");
--
Gitblit v1.9.1