From 85424cfab5df740a227435edc4f65dd930a38249 Mon Sep 17 00:00:00 2001
From: Daniel Gerhardt <code@dgerhardt.net>
Date: Fri, 9 Aug 2019 16:32:35 +0200
Subject: [PATCH] Move activation and auth failure handling to service layer

Account activation is now using the ban logic as used for login failure
logic.
---
 .../controller/AuthenticationController.java  | 10 ++++----
 .../arsnova/controller/UserController.java    |  9 ++++----
 .../v2/AuthenticationController.java          | 13 ++++-------
 .../arsnova/controller/v2/UserController.java | 12 ++++------
 .../de/thm/arsnova/service/UserService.java   |  5 +++-
 .../thm/arsnova/service/UserServiceImpl.java  | 23 ++++++++++++++++++-
 6 files changed, 45 insertions(+), 27 deletions(-)

diff --git a/src/main/java/de/thm/arsnova/controller/AuthenticationController.java b/src/main/java/de/thm/arsnova/controller/AuthenticationController.java
index 7c7df3663..7dead0ff2 100644
--- a/src/main/java/de/thm/arsnova/controller/AuthenticationController.java
+++ b/src/main/java/de/thm/arsnova/controller/AuthenticationController.java
@@ -18,6 +18,7 @@
 
 package de.thm.arsnova.controller;
 
+import javax.servlet.http.HttpServletRequest;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.web.bind.annotation.PostMapping;
 import org.springframework.web.bind.annotation.RequestBody;
@@ -44,22 +45,23 @@ public class AuthenticationController {
 	}
 
 	@PostMapping("/login/registered")
-	public ClientAuthentication loginRegistered(@RequestBody final LoginCredentials loginCredentials) {
+	public ClientAuthentication loginRegistered(@RequestBody final LoginCredentials loginCredentials,
+			final HttpServletRequest request) {
 		final String loginId = loginCredentials.getLoginId().toLowerCase();
 		userService.authenticate(new UsernamePasswordAuthenticationToken(loginId, loginCredentials.getPassword()),
-				UserProfile.AuthProvider.ARSNOVA);
+				UserProfile.AuthProvider.ARSNOVA, request.getRemoteAddr());
 		return userService.getCurrentClientAuthentication();
 	}
 
 	@PostMapping("/login/guest")
-	public ClientAuthentication loginGuest() {
+	public ClientAuthentication loginGuest(final HttpServletRequest request) {
 		final ClientAuthentication currentAuthentication = userService.getCurrentClientAuthentication();
 		if (currentAuthentication != null
 				&& currentAuthentication.getAuthProvider() == UserProfile.AuthProvider.ARSNOVA_GUEST) {
 			return currentAuthentication;
 		}
 		userService.authenticate(new UsernamePasswordAuthenticationToken(null, null),
-				UserProfile.AuthProvider.ARSNOVA_GUEST);
+				UserProfile.AuthProvider.ARSNOVA_GUEST, request.getRemoteAddr());
 
 		return userService.getCurrentClientAuthentication();
 	}
diff --git a/src/main/java/de/thm/arsnova/controller/UserController.java b/src/main/java/de/thm/arsnova/controller/UserController.java
index abc74d0ce..f2e19777b 100644
--- a/src/main/java/de/thm/arsnova/controller/UserController.java
+++ b/src/main/java/de/thm/arsnova/controller/UserController.java
@@ -19,6 +19,7 @@
 package de.thm.arsnova.controller;
 
 import com.fasterxml.jackson.annotation.JsonView;
+import javax.servlet.http.HttpServletRequest;
 import org.springframework.web.bind.annotation.PathVariable;
 import org.springframework.web.bind.annotation.PostMapping;
 import org.springframework.web.bind.annotation.RequestBody;
@@ -104,13 +105,11 @@ public class UserController extends AbstractEntityController<UserProfile> {
 	@PostMapping(ACTIVATE_MAPPING)
 	public void activate(
 			@PathVariable final String id,
-			@RequestParam final String key) {
-		final UserProfile userProfile = userService.get(id, true);
-		if (userProfile == null || !key.equals(userProfile.getAccount().getActivationKey())) {
+			@RequestParam final String key,
+			final HttpServletRequest request) {
+		if (!userService.activateAccount(id, key, request.getRemoteAddr())) {
 			throw new BadRequestException();
 		}
-		userProfile.getAccount().setActivationKey(null);
-		userService.update(userProfile);
 	}
 
 	@PostMapping(RESET_PASSWORD_MAPPING)
diff --git a/src/main/java/de/thm/arsnova/controller/v2/AuthenticationController.java b/src/main/java/de/thm/arsnova/controller/v2/AuthenticationController.java
index fd52193a8..75e64453c 100644
--- a/src/main/java/de/thm/arsnova/controller/v2/AuthenticationController.java
+++ b/src/main/java/de/thm/arsnova/controller/v2/AuthenticationController.java
@@ -133,8 +133,8 @@ public class AuthenticationController extends AbstractController {
 			final HttpServletRequest request,
 			final HttpServletResponse response
 	) throws IOException {
-		final String addr = request.getRemoteAddr();
-		if (userService.isBannedFromLogin(addr)) {
+		final String address = request.getRemoteAddr();
+		if (userService.isBannedFromLogin(address)) {
 			response.sendError(429, "Too Many Requests");
 
 			return;
@@ -144,26 +144,23 @@ public class AuthenticationController extends AbstractController {
 
 		if (registeredProperties.isEnabled() && "arsnova".equals(type)) {
 			try {
-				userService.authenticate(authRequest, UserProfile.AuthProvider.ARSNOVA);
+				userService.authenticate(authRequest, UserProfile.AuthProvider.ARSNOVA, address);
 			} catch (final AuthenticationException e) {
 				logger.info("Database authentication failed.", e);
-				userService.increaseFailedLoginCount(addr);
 				response.setStatus(HttpStatus.UNAUTHORIZED.value());
 			}
 		} else if (ldapProperties.stream().anyMatch(p -> p.isEnabled()) && "ldap".equals(type)) {
 			try {
-				userService.authenticate(authRequest, UserProfile.AuthProvider.LDAP);
+				userService.authenticate(authRequest, UserProfile.AuthProvider.LDAP, address);
 			} catch (final AuthenticationException e) {
 				logger.info("LDAP authentication failed.", e);
-				userService.increaseFailedLoginCount(addr);
 				response.setStatus(HttpStatus.UNAUTHORIZED.value());
 			}
 		} else if (guestProperties.isEnabled() && "guest".equals(type)) {
 			try {
-				userService.authenticate(authRequest, UserProfile.AuthProvider.ARSNOVA_GUEST);
+				userService.authenticate(authRequest, UserProfile.AuthProvider.ARSNOVA_GUEST, address);
 			} catch (final AuthenticationException e) {
 				logger.debug("Guest authentication failed.", e);
-				userService.increaseFailedLoginCount(addr);
 				response.setStatus(HttpStatus.UNAUTHORIZED.value());
 			}
 		} else {
diff --git a/src/main/java/de/thm/arsnova/controller/v2/UserController.java b/src/main/java/de/thm/arsnova/controller/v2/UserController.java
index aec8a783b..b1836860d 100644
--- a/src/main/java/de/thm/arsnova/controller/v2/UserController.java
+++ b/src/main/java/de/thm/arsnova/controller/v2/UserController.java
@@ -61,17 +61,13 @@ public class UserController extends AbstractController {
 	@PostMapping(value = "/{username}/activate")
 	public void activate(
 			@PathVariable final String username,
-			@RequestParam final String key, final HttpServletRequest request,
+			@RequestParam final String key,
+			final HttpServletRequest request,
 			final HttpServletResponse response) {
 		final UserProfile userProfile = userService.getByUsername(username);
-		if (null != userProfile && key.equals(userProfile.getAccount().getActivationKey())) {
-			userProfile.getAccount().setActivationKey(null);
-			userService.update(userProfile);
-
-			return;
+		if (userProfile == null || !userService.activateAccount(userProfile.getId(), key, request.getRemoteAddr())) {
+			response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
 		}
-
-		response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
 	}
 
 	@DeleteMapping(value = "/{username}/")
diff --git a/src/main/java/de/thm/arsnova/service/UserService.java b/src/main/java/de/thm/arsnova/service/UserService.java
index fd09780c5..bbfd16381 100644
--- a/src/main/java/de/thm/arsnova/service/UserService.java
+++ b/src/main/java/de/thm/arsnova/service/UserService.java
@@ -69,7 +69,8 @@ public interface UserService extends EntityService<UserProfile> {
 
 	int loggedInUsers();
 
-	void authenticate(UsernamePasswordAuthenticationToken token, UserProfile.AuthProvider authProvider);
+	void authenticate(UsernamePasswordAuthenticationToken token, UserProfile.AuthProvider authProvider,
+			String clientAddress);
 
 	User loadUser(UserProfile.AuthProvider authProvider, String loginId,
 			Collection<GrantedAuthority> grantedAuthorities, boolean autoCreate) throws UsernameNotFoundException;
@@ -90,6 +91,8 @@ public interface UserService extends EntityService<UserProfile> {
 
 	void addRoomToHistory(UserProfile userProfile, Room room);
 
+	boolean activateAccount(String id, String key, String clientAddress);
+
 	void initiatePasswordReset(String username);
 
 	boolean resetPassword(UserProfile userProfile, String key, String password);
diff --git a/src/main/java/de/thm/arsnova/service/UserServiceImpl.java b/src/main/java/de/thm/arsnova/service/UserServiceImpl.java
index 29f729b41..2a93a39bf 100644
--- a/src/main/java/de/thm/arsnova/service/UserServiceImpl.java
+++ b/src/main/java/de/thm/arsnova/service/UserServiceImpl.java
@@ -328,7 +328,11 @@ public class UserServiceImpl extends DefaultEntityServiceImpl<UserProfile> imple
 
 	@Override
 	public void authenticate(final UsernamePasswordAuthenticationToken token,
-			final UserProfile.AuthProvider authProvider) {
+			final UserProfile.AuthProvider authProvider, final String clientAddress) {
+		if (isBannedFromLogin(clientAddress)) {
+			throw new BadRequestException();
+		}
+
 		final Authentication auth;
 		switch (authProvider) {
 			case LDAP:
@@ -357,6 +361,7 @@ public class UserServiceImpl extends DefaultEntityServiceImpl<UserProfile> imple
 		}
 
 		if (!auth.isAuthenticated()) {
+			increaseFailedLoginCount(clientAddress);
 			throw new BadRequestException();
 		}
 		SecurityContextHolder.getContext().setAuthentication(auth);
@@ -538,6 +543,22 @@ public class UserServiceImpl extends DefaultEntityServiceImpl<UserProfile> imple
 		}
 	}
 
+	public boolean activateAccount(final String id, final String key, final String clientAddress) {
+		if (isBannedFromLogin(clientAddress)) {
+			return false;
+		}
+		final UserProfile userProfile = get(id, true);
+		if (userProfile == null || !key.equals(userProfile.getAccount().getActivationKey())) {
+			increaseFailedLoginCount(clientAddress);
+			return false;
+		}
+
+		userProfile.getAccount().setActivationKey(null);
+		update(userProfile);
+
+		return true;
+	}
+
 	@Override
 	public void initiatePasswordReset(final String username) {
 		final UserProfile userProfile = getByUsername(username);
-- 
GitLab