Skip to content

Commit

Permalink
Improvements on webUser password change. Only own password can be cha…
Browse files Browse the repository at this point in the history 
…nged
  • Loading branch information
@fnkbsi
fnkbsi committed Oct 9, 2024
1 parent fd0d461 commit bfbe877
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 27 deletions.
16 changes: 13 additions & 3 deletions src/main/java/de/rwth/idsg/steve/config/SecurityConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@
import org.springframework.security.web.authentication.www.BasicAuthenticationFilter;

import static de.rwth.idsg.steve.SteveConfiguration.CONFIG;
import org.springframework.http.HttpMethod;
import org.springframework.security.web.access.expression.WebExpressionAuthorizationManager;
import org.springframework.security.web.util.matcher.RequestMatcher;

/**
* @author Sevket Goekay <[email protected]>
Expand Down Expand Up @@ -60,6 +63,8 @@ public PasswordEncoder passwordEncoder() {
public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
final String prefix = CONFIG.getSpringManagerMapping();

RequestMatcher toOverview = (request) -> request.getParameter("backToOverview") != null;

return http
.authorizeHttpRequests(
req -> req
Expand All @@ -70,9 +75,14 @@ public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Excepti
"/WEB-INF/views/**" // https://github.com/spring-projects/spring-security/issues/13285#issuecomment-1579097065
).permitAll()
.requestMatchers(prefix + "/home").hasAnyAuthority("USER", "ADMIN")
// webuser
.requestMatchers(prefix + "/webusers").hasAnyAuthority("USER", "ADMIN")
.requestMatchers(prefix + "/webusers" + "/details/**").hasAnyAuthority("USER", "ADMIN")
// webuser
//only allowed to change the own password
.requestMatchers(prefix + "/webusers" + "/password/{name}")
.access(new WebExpressionAuthorizationManager("#name == authentication.name"))
// otherwise denies access on backToOverview!
.requestMatchers(toOverview).hasAnyAuthority("USER", "ADMIN")
.requestMatchers(HttpMethod.GET, prefix + "/webusers/**").hasAnyAuthority("USER", "ADMIN")
.requestMatchers(HttpMethod.POST, prefix + "/webusers/**").hasAuthority("ADMIN")
// users
.requestMatchers(prefix + "/users").hasAnyAuthority("USER", "ADMIN")
.requestMatchers(prefix + "/users" + "/details/**").hasAnyAuthority("USER", "ADMIN")
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/de/rwth/idsg/steve/repository/impl/WebUserRepositoryImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,8 @@ public void createUser(WebUserRecord user) {

@Override
public void updateUser(WebUserRecord user) {
// To change the password use one of the changePassword methods
ctx.update(WEB_USER)
.set(WEB_USER.PASSWORD, user.getPassword())
.set(WEB_USER.API_PASSWORD, user.getApiPassword())
.set(WEB_USER.ENABLED, user.getEnabled())
.set(WEB_USER.AUTHORITIES, user.getAuthorities())
.where(WEB_USER.USERNAME.eq(user.getUsername()))
Expand Down
15 changes: 15 additions & 0 deletions src/main/java/de/rwth/idsg/steve/service/WebUserService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,21 @@ public WebUserBaseForm getDetails(Integer webUserPk) {
form.setAuthorities(WebUserAuthority.fromJsonValue(ur.getAuthorities()));
return form;
}

Check failure on line 239 in src/main/java/de/rwth/idsg/steve/service/WebUserService.java

View workflow job for this annotation

GitHub Actions / checkstyle 

[checkstyle] reported by reviewdog 🐶
Line has trailing spaces.

Raw Output:
/github/workspace/./src/main/java/de/rwth/idsg/steve/service/WebUserService.java:239:0: error: Line has trailing spaces. (com.puppycrawl.tools.checkstyle.checks.regexp.RegexpSinglelineCheck)
public WebUserBaseForm getDetails(String webUserName) {
WebUserRecord ur = webUserRepository.loadUserByUsername(webUserName);

if (ur == null) {
throw new SteveException("There is no user with id '%s'", webUserName);
}

WebUserBaseForm form = new WebUserBaseForm();
form.setWebUserPk(ur.getWebUserPk());
form.setEnabled(ur.getEnabled());
form.setWebUsername(ur.getUsername());
form.setAuthorities(WebUserAuthority.fromJsonValue(ur.getAuthorities()));
return form;
}

// Helpers
private UserDetails loadUserByUsernameForApiInternal(String username) {
Expand Down
21 changes: 12 additions & 9 deletions src/main/java/de/rwth/idsg/steve/web/controller/WebUsersController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ public class WebUsersController {
private static final String QUERY_PATH = "/query";

private static final String DETAILS_PATH = "/details/{webUserPk}";
private static final String DELETE_ALL_PATH = "/delete/{webUserPk}";
private static final String DELETE_PATH = "/delete/{webUserPk}";
private static final String UPDATE_PATH = "/update";
private static final String ADD_PATH = "/add";
private static final String PASSWORD_PATH = "/password/{webUserPk}";
private static final String PASSWORD_PATH = "/password/{webUserName}";

// -------------------------------------------------------------------------
// HTTP methods
Expand Down Expand Up @@ -114,11 +114,11 @@ public String update(@Valid @ModelAttribute("webuserForm") WebUserBaseForm webus
webUserService.update(webuserBaseForm);
return toOverview();
}

@RequestMapping(value = PASSWORD_PATH, method = RequestMethod.GET)
public String passwordChangeGet(@PathVariable("webUserPk") Integer webUserPk, Model model) {
public String passwordChangeGet(@PathVariable("webUserName") String webUserName, Model model) {
WebUserForm webUserForm = new WebUserForm();
WebUserBaseForm webUserBaseForm = webUserService.getDetails(webUserPk);
WebUserBaseForm webUserBaseForm = webUserService.getDetails(webUserName);
webUserForm.setWebUserPk(webUserBaseForm.getWebUserPk());
webUserForm.setWebUsername(webUserBaseForm.getWebUsername());
webUserForm.setAuthorities(webUserBaseForm.getAuthorities());
Expand All @@ -136,10 +136,11 @@ public String passwordChange(@Valid @ModelAttribute("webuserForm") WebUserForm w
}

webUserService.updatePassword(webuserForm);
return toOverview();
String redirect_str = String.format("redirect:/manager/webusers/details/%s", webuserForm.getWebUserPk());

Check failure on line 139 in src/main/java/de/rwth/idsg/steve/web/controller/WebUsersController.java

View workflow job for this annotation

GitHub Actions / checkstyle 

[checkstyle] reported by reviewdog 🐶
Name 'redirect_str' must match pattern '^[a-z][a-zA-Z0-9]*$'.

Raw Output:
/github/workspace/./src/main/java/de/rwth/idsg/steve/web/controller/WebUsersController.java:139:16: error: Name 'redirect_str' must match pattern '^[a-z][a-zA-Z0-9]*$'. (com.puppycrawl.tools.checkstyle.checks.naming.LocalVariableNameCheck)
return redirect_str;
}

@RequestMapping(value = DELETE_ALL_PATH, method = RequestMethod.POST)
@RequestMapping(value = DELETE_PATH, method = RequestMethod.POST)
public String delete(@PathVariable("webUserPk") Integer webUserPk) {
webUserService.deleteUser(webUserPk);
return toOverview();
Expand All @@ -150,8 +151,10 @@ public String delete(@PathVariable("webUserPk") Integer webUserPk) {
// -------------------------------------------------------------------------

@RequestMapping(params = "backToOverview", value = PASSWORD_PATH, method = RequestMethod.POST)
public String passwordBackToOverview() {
return toOverview();
public String passwordBackToOverview(@Valid @ModelAttribute("webuserForm") WebUserForm webuserForm,
BindingResult result, Model model) {
String redirect_str = String.format("redirect:/manager/webusers/details/%s", webuserForm.getWebUserPk());

Check failure on line 156 in src/main/java/de/rwth/idsg/steve/web/controller/WebUsersController.java

View workflow job for this annotation

GitHub Actions / checkstyle 

[checkstyle] reported by reviewdog 🐶
Name 'redirect_str' must match pattern '^[a-z][a-zA-Z0-9]*$'.

Raw Output:
/github/workspace/./src/main/java/de/rwth/idsg/steve/web/controller/WebUsersController.java:156:16: error: Name 'redirect_str' must match pattern '^[a-z][a-zA-Z0-9]*$'. (com.puppycrawl.tools.checkstyle.checks.naming.LocalVariableNameCheck)
return redirect_str;
}

@RequestMapping(params = "backToOverview", value = ADD_PATH, method = RequestMethod.POST)
Expand Down
6 changes: 3 additions & 3 deletions src/main/resources/webapp/WEB-INF/views/data-man/webuserDetails.jsp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
<form:hidden path="webUserPk" readonly="true"/>
</td></tr>
<tr><td></td>
<td><a href="${ctxPath}/manager/webusers/password/${webuserForm.webUserPk}">
<td><a href="${ctxPath}/manager/webusers/password/${webuserForm.webUsername}">
<B>Change Password</B></a>
</td>
</tr>
Expand All @@ -65,8 +65,8 @@
</tr>
<tr><td></td>
<td id="add_space">
<input type="submit" name="update" value="Update">
<input type="submit" name="backToOverview" value="Back to Overview">
<input type="submit" name="update" value="Update">
<input type="submit" name="backToOverview" value="Back to Overview">
</td>
</tr>
</tbody>
Expand Down
16 changes: 6 additions & 10 deletions src/main/resources/webapp/WEB-INF/views/data-man/webuserPassword.jsp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,6 @@
--%>
<%@ include file="../00-header.jsp" %>
<script type="text/javascript">
$(document).ready(function() {
<%@ include file="../snippets/datePicker-past.js" %>
});
</script>
<spring:hasBindErrors name="webuserForm">
<div class="error">
Error while trying to change password of webuser:
Expand All @@ -36,19 +31,20 @@
</spring:hasBindErrors>
<div class="content"><div>
<section><span>Webuser change password</span></section>
<form:form action="${ctxPath}/manager/webusers/password/${webuserForm.webUserPk}" modelAttribute="webuserForm">
<form:form action="${ctxPath}/manager/webusers/password/${webuserForm.webUsername}" modelAttribute="webuserForm">
<table class="userInput">
<thead><tr><th>Webuser</th><th></th></thead>
<tbody>
<tr><td>Webusername:</td><td>${webuserForm.webUsername}<form:hidden path="webUsername" value="${webuserForm.webUsername}"/></td></tr>
<tr><td>Webusername:</td><td>${webuserForm.webUsername}
<form:hidden path="webUsername" value="${webuserForm.webUsername}"/>
<form:hidden path="webUserPk" value="${webuserForm.webUserPk}"/>
</td></tr>
<tr><td>Password:</td><td><form:password path="password" title="Set the password"/></td></tr>
<tr><td>Password confirmation:</td><td><form:password path="passwordComparison" title="Confirm the password"/></td></tr>
<tr><td></td>
<td id="add_space">
<c:set var="submitButtonName" value="change" />
<c:set var="submitButtonValue" value="change" />
<input type="submit" name="change" value="Change">
<input type="submit" name="backToOverview" value="Back to Overview">
<input type="submit" name="backToOverview" value="Back to Details">
</td>
</tr>
</tbody>
Expand Down

0 comments on commit bfbe877

Please sign in to comment.