Fix password reset

SpiNNaker-allocserv/pom.xml

@@ -335,6 +335,11 @@ limitations under the License.
 			<artifactId>spring-boot-test</artifactId>
 			<scope>test</scope>
 		</dependency>
+		<dependency>
+			<groupId>org.springframework.boot</groupId>
+			<artifactId>spring-boot-test-autoconfigure</artifactId>
+			<scope>test</scope>
+		</dependency>
 		<dependency>
 			<groupId>uk.ac.manchester.spinnaker</groupId>
 			<artifactId>SpiNNaker-comms</artifactId>
@@ -365,6 +370,11 @@ limitations under the License.
 			<groupId>org.springframework.security</groupId>
 			<artifactId>spring-security-core</artifactId>
 		</dependency>
+		<dependency>
+			<groupId>org.springframework.security</groupId>
+			<artifactId>spring-security-test</artifactId>
+			<scope>test</scope>
+		</dependency>
 		<dependency>
 			<groupId>org.springframework.boot</groupId>
 			<artifactId>spring-boot-starter-security</artifactId>

SpiNNaker-allocserv/src/main/java/uk/ac/manchester/spinnaker/alloc/admin/AdminControllerImpl.java

@@ -86,7 +86,9 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.stream.Collectors;
 
+import javax.servlet.http.HttpServletRequest;
 import javax.ws.rs.WebApplicationException;
 import javax.ws.rs.core.Response.Status;
 
@@ -292,13 +294,22 @@ private ModelAndView errors(DataAccessException exception) {
 	}
 
 	@ExceptionHandler(BindException.class)
-	ModelAndView validationError(BindException result) {
+	ModelAndView validationError(BindException result, HttpServletRequest req) {
+		log.debug("Binding problem on request {}", req.getRequestURI());
+		if (req.getMethod() == "POST") {
+			try {
+				log.debug("Body: {}", req.getReader().lines().collect(
+						Collectors.joining()));
+			} catch (IOException e) {
+				log.debug("Failed to read request body", e);
+			}
+		}
 		if (result.hasGlobalErrors()) {
 			// I don't believe this is really reachable code
-			log.debug("binding problem", result);
+			log.debug("global binding problem", result);
 			return errors(errorMessage(result.getGlobalError()));
 		} else if (result.hasFieldErrors()) {
-			log.debug("binding problem", result);
+			log.debug("field binding problem", result);
 			return errors(errorMessage(result.getFieldError()));
 		} else {
 			// This should definitely be unreachable
@@ -416,7 +427,7 @@ public ModelAndView getUserCreationForm(ModelMap ignored) {
 	public ModelAndView createUser(UserRecord user, ModelMap model,
 			RedirectAttributes attrs) {
 		user.initCreationDefaults();
-		var realUser = userManager.createUser(user)
+		var realUser = userManager.createUser(user, this::showGroupInfoUrl)
 				.orElseThrow(() -> new AdminException(
 						"user creation failed (duplicate username?)"));
 		int id = realUser.getUserId();

SpiNNaker-allocserv/src/main/java/uk/ac/manchester/spinnaker/alloc/admin/AdminImpl.java

@@ -186,11 +186,12 @@ public Map<String, URI> listUsers(UriInfo ui) {
 	@Override
 	public Response createUser(UserRecord providedUser, UriInfo ui) {
 		log.warn("CALLED createUser({})", providedUser.getUserName());
+		var ub = ui.getBaseUriBuilder().path(DESCRIBE_USER);
 		providedUser.initCreationDefaults();
-		var realUser = userManager.createUser(providedUser)
+		var realUser = userManager.createUser(providedUser,
+				m -> ub.build(m.getUserId()))
 				.orElseThrow(() -> new RequestFailedException(NOT_MODIFIED,
 						"user already exists"));
-		var ub = ui.getBaseUriBuilder().path(DESCRIBE_USER);
 		int id = realUser.getUserId();
 		return created(ub.build(id)).type(APPLICATION_JSON)
 				.entity(realUser.sanitise()).build();
@@ -210,10 +211,10 @@ public UserRecord updateUser(int id, UserRecord providedUser, UriInfo ui,
 		log.warn("CALLED updateUser({})", providedUser.getUserName());
 		var adminUser = security.getUserPrincipal().getName();
 		providedUser.setUserId(null);
-		var ub = ui.getBaseUriBuilder().path(DESCRIBE_GROUP);
+		var ub = ui.getBaseUriBuilder().path(DESCRIBE_USER);
 		return userManager
 				.updateUser(id, providedUser, adminUser,
-						m -> ub.build(m.getGroupId()))
+						m -> ub.build(m.getUserId()))
 				.orElseThrow(AdminImpl::noUser).sanitise();
 	}
 

SpiNNaker-allocserv/src/main/java/uk/ac/manchester/spinnaker/alloc/admin/UserControl.java

@@ -108,18 +108,6 @@ public void close() {
 			super.close();
 		}
 
-		/**
-		 * Get a user.
-		 *
-		 * @param id
-		 *            User ID
-		 * @return Database row, if user exists.
-		 * @see SQLQueries#GET_USER_DETAILS
-		 */
-		Optional<UserRecord> getUser(int id) {
-			return getUserDetails.call1(UserControl::sketchUser, id);
-		}
-
 		Function<UserRecord, UserRecord>
 				populateMemberships(Function<MemberRecord, URI> urlGen) {
 			if (isNull(urlGen)) {
@@ -365,23 +353,31 @@ private static UserRecord sketchUser(Row row) {
 	 *
 	 * @param user
 	 *            The description of the user to create.
+	 * @param urlGen
+	 *            How to construct the URL for a group membership in the
+	 *            response. If {@code null}, the memberships will be omitted.
 	 * @return A description of the created user, or {@link Optional#empty()} if
 	 *         the user exists already.
 	 */
-	public Optional<UserRecord> createUser(UserRecord user) {
+	public Optional<UserRecord> createUser(UserRecord user,
+			Function<MemberRecord, URI> urlGen) {
 		// This is a slow operation; don't hold a database transaction
 		var encPass = passServices.encodePassword(user.getPassword());
 		try (var sql = new CreateSQL()) {
-			return sql.transaction(() -> createUser(user, encPass, sql));
+			return sql.transaction(
+					() -> createUser(user, encPass, urlGen, sql));
 		}
 	}
 
 	private Optional<UserRecord> createUser(UserRecord user, String encPass,
-			CreateSQL sql) {
-		return sql
-				.createUser(user.getUserName(), encPass, user.getTrustLevel(),
-						!user.getEnabled(), user.getOpenIdSubject())
-				.flatMap(sql::getUser);
+			Function<MemberRecord, URI> urlGen, CreateSQL sql) {
+		var result =  sql.createUser(user.getUserName(), encPass,
+				user.getTrustLevel(), !user.getEnabled(),
+				user.getOpenIdSubject());
+		if (result.isEmpty()) {
+			return Optional.empty();
+		}
+		return getUser(result.get(), urlGen);
 	}
 
 	/**
@@ -517,7 +513,7 @@ private Optional<UserRecord> updateUser(int id, UserRecord user,
 			}
 		}
 
-		return sql.getUser(id).map(sql.populateMemberships(urlGen));
+		return getUser(id, urlGen);
 	}
 
 	/**

SpiNNaker-allocserv/src/main/java/uk/ac/manchester/spinnaker/alloc/model/UserRecord.java

@@ -288,7 +288,6 @@ public void setOpenIdSubject(String subject) {
 	}
 
 	/** @return Whether this is an internal user. */
-	@JsonIgnore
 	public boolean isInternal() {
 		return isInternal;
 	}

SpiNNaker-allocserv/src/main/webapp/WEB-INF/views/admin/userdetails.jsp

@@ -10,7 +10,7 @@ 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
 
-    https://www.apache.org/licenses/LICENSE-2.0
+	https://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,
@@ -34,6 +34,7 @@ limitations under the License.
 </c:choose>
 
 <form:form method="POST" modelAttribute="user">
+	<form:hidden path="internal"/>
 	<form:label path="userName">User Name: </form:label>
 	<form:input path="userName" type="text"/>
 	<form:select path="trustLevel">

SpiNNaker-allocserv/src/test/java/uk/ac/manchester/spinnaker/alloc/TestSupport.java

@@ -178,7 +178,7 @@ private static void makeMachine(Connection c, int width, int height,
 		}
 	}
 
-	private static void makeUser(Connection c) {
+	protected static void makeUser(Connection c) {
 		try (var u = c.update(INSERT_USER)) {
 			u.call(USER, USER_NAME, BASIC, true);
 		}

SpiNNaker-allocserv/src/test/java/uk/ac/manchester/spinnaker/alloc/admin/AdminControllerTest.java

@@ -0,0 +1,148 @@
+/*
+ * Copyright (c) 2025 The University of Manchester
+ *
+ * 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
+ *
+ *     https://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 uk.ac.manchester.spinnaker.alloc.admin;
+
+import static uk.ac.manchester.spinnaker.alloc.admin.AdminController.BASE_PATH;
+import static uk.ac.manchester.spinnaker.alloc.admin.AdminController.CREATE_USER_PATH;
+import static uk.ac.manchester.spinnaker.alloc.admin.AdminController.USERS_PATH;
+import static org.junit.Assert.assertTrue;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrlPattern;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.forwardedUrl;
+import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf;
+
+import java.net.URI;
+import java.util.Base64;
+
+import javax.ws.rs.core.MediaType;
+
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.test.context.ActiveProfiles;
+import org.springframework.test.context.junit.jupiter.web.SpringJUnitWebConfig;
+import org.springframework.test.web.servlet.MockMvc;
+import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
+
+import uk.ac.manchester.spinnaker.alloc.TestSupport;
+import uk.ac.manchester.spinnaker.alloc.model.UserRecord;
+import uk.ac.manchester.spinnaker.alloc.security.TrustLevel;
+
+@AutoConfigureMockMvc
+@SpringBootTest
+@SpringJUnitWebConfig(TestSupport.Config.class)
+@ActiveProfiles("unittest")
+public class AdminControllerTest extends TestSupport {
+
+	@Autowired
+	private MockMvc mvc;
+
+	@Autowired
+	private UserControl userControl;
+
+	@Test
+	public void testCreateUser() throws Exception {
+		doTransactionalTest(() -> {
+			// Create an admin
+			UserRecord admin = new UserRecord();
+			admin.setUserName("admin");
+			admin.setEnabled(true);
+			admin.setHasPassword(true);
+			admin.setTrustLevel(TrustLevel.ADMIN);
+			admin.setPassword("admin");
+
+			var adminRecord = userControl.createUser(admin,
+					m -> URI.create(""));
+			assertTrue(adminRecord.isPresent());
+
+			String userPass = Base64.getEncoder().encodeToString(
+					"admin:admin".getBytes());
+			try {
+				var result = mvc.perform(
+					MockMvcRequestBuilders
+					.post(BASE_PATH + CREATE_USER_PATH)
+					.with(csrf())
+					.header("Authorization", "Basic " + userPass)
+					.param("internal", "true")
+					.param("userName", "test")
+					.param("trustLevel", "USER")
+					.param("password", "test")
+					.param("enabled", "true")
+					.param("hasPassword", "true")
+					.accept(MediaType.APPLICATION_JSON));
+				result.andExpect(
+						redirectedUrlPattern(BASE_PATH + USERS_PATH + "/*"));
+			} catch (Exception e) {
+				throw new RuntimeException(e);
+			}
+		});
+	}
+
+	@Test
+	public void updateUserPassword() throws Exception {
+		doTransactionalTest(() -> {
+			// Create an admin
+			UserRecord admin = new UserRecord();
+			admin.setUserName("admin");
+			admin.setEnabled(true);
+			admin.setHasPassword(true);
+			admin.setTrustLevel(TrustLevel.ADMIN);
+			admin.setPassword("admin");
+
+			var adminRecord = userControl.createUser(admin,
+					m -> URI.create(""));
+			assertTrue(adminRecord.isPresent());
+			log.info("Admin created: {}", adminRecord.get());
+
+			String userPass = Base64.getEncoder().encodeToString(
+					"admin:admin".getBytes());
+
+			// Create a user
+			UserRecord test = new UserRecord();
+			test.setUserName("test");
+			test.setEnabled(true);
+			test.setHasPassword(true);
+			test.setTrustLevel(TrustLevel.USER);
+			test.setPassword("test");
+
+			var testRecord = userControl.createUser(test,
+					m -> URI.create(""));
+			assertTrue(testRecord.isPresent());
+			UserRecord testUser = testRecord.get();
+
+			try {
+				var result = mvc.perform(
+					MockMvcRequestBuilders
+					.post(BASE_PATH + USERS_PATH + "/" + testUser.getUserId())
+					.with(csrf())
+					.header("Authorization", "Basic " + userPass)
+					.param("internal", "true")
+					.param("userName", "test")
+					.param("trustLevel", "USER")
+					.param("password", "testchange")
+					.accept(MediaType.APPLICATION_JSON));
+
+				// Note: not sure how to get this dynamically!
+				result.andExpect(
+					forwardedUrl("/WEB-INF/views/admin/userdetails.jsp"));
+			} catch (Exception e) {
+				throw new RuntimeException(e);
+			}
+		});
+	}
+
+
+}