Skip to content

Commit e69804d

Browse files
gitblitgitblit
committed
Merge pull request #947 from paulsputer/LogUpdateForAllAuthentication
Log update for Fail2Ban usage
2 parents a3a18a0 + 0d7c650 commit e69804d

11 files changed

+72
-60
lines changed

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

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -310,15 +310,12 @@ public UserModel authenticate(HttpServletRequest httpRequest, boolean requiresCe
310310
if (values.length == 2) {
311311
String username = values[0];
312312
char[] password = values[1].toCharArray();
313-
user = authenticate(username, password);
313+
user = authenticate(username, password, httpRequest.getRemoteAddr());
314314
if (user != null) {
315315
flagRequest(httpRequest, AuthenticationType.CREDENTIALS, user.username);
316316
logger.debug(MessageFormat.format("{0} authenticated by BASIC request header from {1}",
317317
user.username, httpRequest.getRemoteAddr()));
318318
return validateAuthentication(user, AuthenticationType.CREDENTIALS);
319-
} else {
320-
logger.warn(MessageFormat.format("Failed login attempt for {0}, invalid credentials from {1}",
321-
username, httpRequest.getRemoteAddr()));
322319
}
323320
}
324321
}
@@ -445,7 +442,7 @@ protected void flagRequest(HttpServletRequest httpRequest, AuthenticationType au
445442
* @return a user object or null
446443
*/
447444
@Override
448-
public UserModel authenticate(String username, char[] password) {
445+
public UserModel authenticate(String username, char[] password, String remoteIP) {
449446
if (StringUtils.isEmpty(username)) {
450447
// can not authenticate empty username
451448
return null;
@@ -462,22 +459,29 @@ public UserModel authenticate(String username, char[] password) {
462459

463460
// try local authentication
464461
if (user != null && user.isLocalAccount()) {
465-
return authenticateLocal(user, password);
466-
}
467-
468-
// try registered external authentication providers
469-
for (AuthenticationProvider provider : authenticationProviders) {
470-
if (provider instanceof UsernamePasswordAuthenticationProvider) {
471-
UserModel returnedUser = provider.authenticate(usernameDecoded, password);
472-
if (returnedUser != null) {
473-
// user authenticated
474-
returnedUser.accountType = provider.getAccountType();
475-
return validateAuthentication(returnedUser, AuthenticationType.CREDENTIALS);
462+
UserModel returnedUser = authenticateLocal(user, password);
463+
if (returnedUser != null) {
464+
// user authenticated
465+
return returnedUser;
466+
}
467+
} else {
468+
// try registered external authentication providers
469+
for (AuthenticationProvider provider : authenticationProviders) {
470+
if (provider instanceof UsernamePasswordAuthenticationProvider) {
471+
UserModel returnedUser = provider.authenticate(usernameDecoded, password);
472+
if (returnedUser != null) {
473+
// user authenticated
474+
returnedUser.accountType = provider.getAccountType();
475+
return validateAuthentication(returnedUser, AuthenticationType.CREDENTIALS);
476+
}
476477
}
477478
}
478479
}
479480

480481
// could not authenticate locally or with a provider
482+
logger.warn(MessageFormat.format("Failed login attempt for {0}, invalid credentials from {1}", username,
483+
remoteIP != null ? remoteIP : "unknown"));
484+
481485
return null;
482486
}
483487

‎src/main/java/com/gitblit/manager/GitblitManager.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -649,8 +649,8 @@ public void send(Mailing mail) {
649649
*/
650650

651651
@Override
652-
public UserModel authenticate(String username, char[] password) {
653-
return authenticationManager.authenticate(username, password);
652+
public UserModel authenticate(String username, char[] password, String remoteIP) {
653+
return authenticationManager.authenticate(username, password, remoteIP);
654654
}
655655

656656
@Override

‎src/main/java/com/gitblit/manager/IAuthenticationManager.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,11 @@ public interface IAuthenticationManager extends IManager {
6565
* @see IUserService.authenticate(String, char[])
6666
* @param username
6767
* @param password
68+
* @param remoteIP
6869
* @return a user object or null
6970
* @since 1.4.0
7071
*/
71-
UserModel authenticate(String username, char[] password);
72+
UserModel authenticate(String username, char[] password, String remoteIP);
7273

7374
/**
7475
* Return the UserModel for already authenticated user.

‎src/main/java/com/gitblit/transport/ssh/UsernamePasswordAuthenticator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public boolean authenticate(String username, String password, ServerSession sess
5151
}
5252

5353
username = username.toLowerCase(Locale.US);
54-
UserModel user = authManager.authenticate(username, password.toCharArray());
54+
UserModel user = authManager.authenticate(username, password.toCharArray(), null);
5555
if (user != null) {
5656
client.setUser(user);
5757
return true;

‎src/main/java/com/gitblit/wicket/pages/RootPage.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636

3737
import org.apache.wicket.MarkupContainer;
3838
import org.apache.wicket.PageParameters;
39+
import org.apache.wicket.RequestCycle;
3940
import org.apache.wicket.behavior.HeaderContributor;
4041
import org.apache.wicket.markup.html.IHeaderContributor;
4142
import org.apache.wicket.markup.html.IHeaderResponse;
@@ -566,7 +567,9 @@ public void onSubmit() {
566567
String username = RootPage.this.username.getObject();
567568
char[] password = RootPage.this.password.getObject().toCharArray();
568569

569-
UserModel user = app().authentication().authenticate(username, password);
570+
HttpServletRequest request = ((WebRequest)RequestCycle.get().getRequest()).getHttpServletRequest();
571+
572+
UserModel user = app().authentication().authenticate(username, password, request.getRemoteAddr());
570573
if (user == null) {
571574
error(getString("gb.invalidUsernameOrPassword"));
572575
} else if (user.username.equals(Constants.FEDERATION_USER)) {

‎src/site/setup_fail2ban.mkd

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,24 @@
11
## Configure fail2ban for Gitblit-SSH
22

3-
This procedure is based on a Debian installation of [fail2ban](http://www.fail2ban.org/), but it should works in any installation.
3+
This procedure uses [fail2ban](http://www.fail2ban.org/).
44

5-
First, create a new filter file `gitblit.conf` in filter directory (Debian: `/etc/fail2ban/filter.d/`) or into `filter.conf` file. Here an example:
5+
First, create a new filter file `gitblit.conf` in filter directory (Debian/CentOS: `/etc/fail2ban/filter.d/`) or into `filter.conf` file. Here is an example:
66

77
[Definition]
8-
failregex = could not authenticate .*? \(/<HOST>:[0-9]*\) for SSH using the supplied password$
8+
failregex = Failed login attempt for .+, invalid credentials from <HOST>\s*$
9+
could not authenticate .*? \(/<HOST>:[0-9]*\) for SSH using the supplied password$
910
ignoreregex =
1011

1112
Then edit `jail.conf` to add "gitblit" service (Debian: `/etc/fail2ban/jail.conf`). For example:
1213

1314
[gitblit]
1415
enabled = true
15-
port = 22
16+
port = 443,29418
1617
protocol = tcp
1718
filter = gitblit
1819
logpath = /var/log/gitblit.log
1920

20-
Restart fail2ban to apply (Debian: `/etc/init.d/fail2ban restart`).
21+
22+
Reload fail2ban config to apply (`fail2ban-client reload`).
23+
24+
Check the status of the gitblit fail2ban jail with `fail2ban-client status gitblit`

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -657,11 +657,11 @@ public void testAuthenticate() throws Exception {
657657
user.password = "password";
658658
users.updateUserModel(user);
659659

660-
assertNotNull(auth.authenticate(user.username, user.password.toCharArray()));
660+
assertNotNull(auth.authenticate(user.username, user.password.toCharArray(), null));
661661
user.disabled = true;
662662

663663
users.updateUserModel(user);
664-
assertNull(auth.authenticate(user.username, user.password.toCharArray()));
664+
assertNull(auth.authenticate(user.username, user.password.toCharArray(), null));
665665
users.deleteUserModel(user);
666666
}
667667

‎src/test/java/com/gitblit/tests/GitBlitTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ public void testGitblitSettings() throws Exception {
176176

177177
@Test
178178
public void testAuthentication() throws Exception {
179-
assertTrue(authentication().authenticate("admin", "admin".toCharArray()) != null);
179+
assertTrue(authentication().authenticate("admin", "admin".toCharArray(), null) != null);
180180
}
181181

182182
@Test

‎src/test/java/com/gitblit/tests/HtpasswdAuthenticationTest.java

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -200,43 +200,43 @@ public void testAuthenticate()
200200
public void testAuthenticationManager()
201201
{
202202
MS.put(KEY_SUPPORT_PLAINTEXT_PWD, "true");
203-
UserModel user = auth.authenticate("user1", "pass1".toCharArray());
203+
UserModel user = auth.authenticate("user1", "pass1".toCharArray(), null);
204204
assertNotNull(user);
205205
assertEquals("user1", user.username);
206206

207-
user = auth.authenticate("user2", "pass2".toCharArray());
207+
user = auth.authenticate("user2", "pass2".toCharArray(), null);
208208
assertNotNull(user);
209209
assertEquals("user2", user.username);
210210

211211
// Test different encryptions
212-
user = auth.authenticate("plain", "passWord".toCharArray());
212+
user = auth.authenticate("plain", "passWord".toCharArray(), null);
213213
assertNotNull(user);
214214
assertEquals("plain", user.username);
215215

216216
MS.put(KEY_SUPPORT_PLAINTEXT_PWD, "false");
217-
user = auth.authenticate("crypt", "password".toCharArray());
217+
user = auth.authenticate("crypt", "password".toCharArray(), null);
218218
assertNotNull(user);
219219
assertEquals("crypt", user.username);
220220

221-
user = auth.authenticate("md5", "password".toCharArray());
221+
user = auth.authenticate("md5", "password".toCharArray(), null);
222222
assertNotNull(user);
223223
assertEquals("md5", user.username);
224224

225-
user = auth.authenticate("sha", "password".toCharArray());
225+
user = auth.authenticate("sha", "password".toCharArray(), null);
226226
assertNotNull(user);
227227
assertEquals("sha", user.username);
228228

229229

230230
// Test leading and trailing whitespace
231-
user = auth.authenticate("trailing", "whitespace".toCharArray());
231+
user = auth.authenticate("trailing", "whitespace".toCharArray(), null);
232232
assertNotNull(user);
233233
assertEquals("trailing", user.username);
234234

235-
user = auth.authenticate("tabbed", "frontAndBack".toCharArray());
235+
user = auth.authenticate("tabbed", "frontAndBack".toCharArray(), null);
236236
assertNotNull(user);
237237
assertEquals("tabbed", user.username);
238238

239-
user = auth.authenticate("leading", "whitespace".toCharArray());
239+
user = auth.authenticate("leading", "whitespace".toCharArray(), null);
240240
assertNotNull(user);
241241
assertEquals("leading", user.username);
242242
}
@@ -323,55 +323,55 @@ public void testAuthenticationMangerDenied()
323323
{
324324
UserModel user = null;
325325
MS.put(KEY_SUPPORT_PLAINTEXT_PWD, "true");
326-
user = auth.authenticate("user1", "".toCharArray());
326+
user = auth.authenticate("user1", "".toCharArray(), null);
327327
assertNull("User 'user1' falsely authenticated.", user);
328328

329-
user = auth.authenticate("user1", "pass2".toCharArray());
329+
user = auth.authenticate("user1", "pass2".toCharArray(), null);
330330
assertNull("User 'user1' falsely authenticated.", user);
331331

332-
user = auth.authenticate("user2", "lalala".toCharArray());
332+
user = auth.authenticate("user2", "lalala".toCharArray(), null);
333333
assertNull("User 'user2' falsely authenticated.", user);
334334

335335

336-
user = auth.authenticate("user3", "disabled".toCharArray());
336+
user = auth.authenticate("user3", "disabled".toCharArray(), null);
337337
assertNull("User 'user3' falsely authenticated.", user);
338338

339-
user = auth.authenticate("user4", "disabled".toCharArray());
339+
user = auth.authenticate("user4", "disabled".toCharArray(), null);
340340
assertNull("User 'user4' falsely authenticated.", user);
341341

342342

343-
user = auth.authenticate("plain", "text".toCharArray());
343+
user = auth.authenticate("plain", "text".toCharArray(), null);
344344
assertNull("User 'plain' falsely authenticated.", user);
345345

346-
user = auth.authenticate("plain", "password".toCharArray());
346+
user = auth.authenticate("plain", "password".toCharArray(), null);
347347
assertNull("User 'plain' falsely authenticated.", user);
348348

349349

350350
MS.put(KEY_SUPPORT_PLAINTEXT_PWD, "false");
351351

352-
user = auth.authenticate("crypt", "".toCharArray());
352+
user = auth.authenticate("crypt", "".toCharArray(), null);
353353
assertNull("User 'cyrpt' falsely authenticated.", user);
354354

355-
user = auth.authenticate("crypt", "passwd".toCharArray());
355+
user = auth.authenticate("crypt", "passwd".toCharArray(), null);
356356
assertNull("User 'crypt' falsely authenticated.", user);
357357

358-
user = auth.authenticate("md5", "".toCharArray());
358+
user = auth.authenticate("md5", "".toCharArray(), null);
359359
assertNull("User 'md5' falsely authenticated.", user);
360360

361-
user = auth.authenticate("md5", "pwd".toCharArray());
361+
user = auth.authenticate("md5", "pwd".toCharArray(), null);
362362
assertNull("User 'md5' falsely authenticated.", user);
363363

364-
user = auth.authenticate("sha", "".toCharArray());
364+
user = auth.authenticate("sha", "".toCharArray(), null);
365365
assertNull("User 'sha' falsely authenticated.", user);
366366

367-
user = auth.authenticate("sha", "letmein".toCharArray());
367+
user = auth.authenticate("sha", "letmein".toCharArray(), null);
368368
assertNull("User 'sha' falsely authenticated.", user);
369369

370370

371-
user = auth.authenticate(" tabbed", "frontAndBack".toCharArray());
371+
user = auth.authenticate(" tabbed", "frontAndBack".toCharArray(), null);
372372
assertNull("User 'tabbed' falsely authenticated.", user);
373373

374-
user = auth.authenticate(" leading", "whitespace".toCharArray());
374+
user = auth.authenticate(" leading", "whitespace".toCharArray(), null);
375375
assertNull("User 'leading' falsely authenticated.", user);
376376
}
377377

‎src/test/java/com/gitblit/tests/LdapAuthenticationTest.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -240,23 +240,23 @@ public void addingGroupsInLdapShouldUpdateGitBlitUsersAndGroups() throws Excepti
240240

241241
@Test
242242
public void testAuthenticationManager() {
243-
UserModel userOneModel = auth.authenticate("UserOne", "userOnePassword".toCharArray());
243+
UserModel userOneModel = auth.authenticate("UserOne", "userOnePassword".toCharArray(), null);
244244
assertNotNull(userOneModel);
245245
assertNotNull(userOneModel.getTeam("git_admins"));
246246
assertNotNull(userOneModel.getTeam("git_users"));
247247
assertTrue(userOneModel.canAdmin);
248248

249-
UserModel userOneModelFailedAuth = auth.authenticate("UserOne", "userTwoPassword".toCharArray());
249+
UserModel userOneModelFailedAuth = auth.authenticate("UserOne", "userTwoPassword".toCharArray(), null);
250250
assertNull(userOneModelFailedAuth);
251251

252-
UserModel userTwoModel = auth.authenticate("UserTwo", "userTwoPassword".toCharArray());
252+
UserModel userTwoModel = auth.authenticate("UserTwo", "userTwoPassword".toCharArray(), null);
253253
assertNotNull(userTwoModel);
254254
assertNotNull(userTwoModel.getTeam("git_users"));
255255
assertNull(userTwoModel.getTeam("git_admins"));
256256
assertNotNull(userTwoModel.getTeam("git admins"));
257257
assertTrue(userTwoModel.canAdmin);
258258

259-
UserModel userThreeModel = auth.authenticate("UserThree", "userThreePassword".toCharArray());
259+
UserModel userThreeModel = auth.authenticate("UserThree", "userThreePassword".toCharArray(), null);
260260
assertNotNull(userThreeModel);
261261
assertNotNull(userThreeModel.getTeam("git_users"));
262262
assertNull(userThreeModel.getTeam("git_admins"));
@@ -269,10 +269,10 @@ public void testBindWithUser() {
269269
settings.put(Keys.realm.ldap.username, "");
270270
settings.put(Keys.realm.ldap.password, "");
271271

272-
UserModel userOneModel = auth.authenticate("UserOne", "userOnePassword".toCharArray());
272+
UserModel userOneModel = auth.authenticate("UserOne", "userOnePassword".toCharArray(), null);
273273
assertNotNull(userOneModel);
274274

275-
UserModel userOneModelFailedAuth = auth.authenticate("UserOne", "userTwoPassword".toCharArray());
275+
UserModel userOneModelFailedAuth = auth.authenticate("UserOne", "userTwoPassword".toCharArray(), null);
276276
assertNull(userOneModelFailedAuth);
277277
}
278278

0 commit comments

Comments
 (0)