Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closes #698 - Added logic to map LDAP-Groups to internal roles. #699

Merged
merged 24 commits into from
May 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
a03e120
Implemented Basic Role resolving for LDAP users.
Apr 19, 2020
6e33a83
Implemented Configuration-Based Access Role Resolving.
Apr 8, 2020
1d807a7
Added Security Annotations for API-Endpoints
Apr 19, 2020
923afc2
Added Tests.
Apr 19, 2020
6ef6d3d
Defined write-role als basic role for all authenticated users.
Apr 21, 2020
ea2784b
Reworked Code to work with Lists of Authorities.
Apr 23, 2020
d0f5cc3
Removed unnecessary @Secured definitions.
Apr 23, 2020
213da48
Added access-role List to LocalUserDetailsService, centralized access…
Apr 28, 2020
c349711
Fixed test & committed UserRoleConfiguration
Apr 28, 2020
75262e0
Renamed RoleSettings, removed unnecessary variable, removed unnecessa…
Apr 29, 2020
420b24f
implemented suggestions from code review.
May 4, 2020
76c197b
Bugfix & Removed uneeded read-role
May 7, 2020
0e8fbb3
Fixed tests
May 7, 2020
952006e
Fix SecurityConfigurationTest
May 7, 2020
dc8f1a2
Removed LdapUserAuthorityPopulator and instead use CustomUserAuthorit…
May 11, 2020
1889370
Fixed tests.
May 12, 2020
135c38a
reworked authentication to always use the CustomLdapUserService
May 18, 2020
8a009d0
Refactoring
May 21, 2020
9fbb5ca
Fixed Tests
May 21, 2020
c643f33
Changed CustomLdapUserService to not rely on Autowired objects anymore.
May 26, 2020
64d407d
Added documentation
May 26, 2020
edce311
Minor fixes
JonasKunz May 27, 2020
3b0bf02
Merge remote-tracking branch 'inspectit/master' into Role_Based_Auth
JonasKunz May 27, 2020
936d8c2
Fixed docu
JonasKunz May 27, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package rocks.inspectit.ocelot.config.model;

import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Data;
import lombok.NoArgsConstructor;

import java.util.ArrayList;
import java.util.List;

/**
* Configuration Container for the authentication role resolving of the LDAP-Settings.
*/
@Data
@Builder
@AllArgsConstructor
@NoArgsConstructor
public class LdapRoleResolveSettings {

/**
* Roles defined in this list are granted read access.
*/
@Builder.Default
private List<String> read = new ArrayList<>();

/**
* Roles defined in this list are granted read and write access.
*/
@Builder.Default
private List<String> write = new ArrayList<>();

/**
* Roles defined in this list are granted read, write and commit access.
*/
@Builder.Default
private List<String> commit = new ArrayList<>();

/**
* Roles defined in this list are granted read, write, commit and admin access.
*/
@Builder.Default
private List<String> admin = new ArrayList<>();
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,11 @@ public class LdapSettings {

/**
* The group name which is required by a user to get admin access rights.
*
* @deprecated This property is deprecated in favor of role based access. A List of LDAP Groups that
* should be granted admin access can be defined in inspectit-config-server.security.ldap.roles.admin.
*/
@NotEmpty
@Deprecated
private String adminGroup;

/**
Expand Down Expand Up @@ -68,4 +71,10 @@ public class LdapSettings {
@NotNull
private String groupSearchFilter;

/**
* Contains the LDAP Group mapped to access roles.
*/
@Builder.Default
private LdapRoleResolveSettings roles = new LdapRoleResolveSettings();

}
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@

import io.swagger.annotations.ApiOperation;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.security.access.annotation.Secured;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
import rocks.inspectit.ocelot.agentstatus.AgentStatus;
import rocks.inspectit.ocelot.agentstatus.AgentStatusManager;
import rocks.inspectit.ocelot.rest.AbstractBaseController;
import rocks.inspectit.ocelot.security.config.UserRoleConfiguration;

import java.util.Collection;
import java.util.Map;
Expand All @@ -29,6 +31,7 @@ public Collection<AgentStatus> getAgentStatuses(@RequestParam Map<String, String
return statusManager.getAgentStatuses();
}

@Secured(UserRoleConfiguration.WRITE_ACCESS_ROLE)
@ApiOperation(value = "Clear the List of Agent Statuses", notes = "Clears the list of connected agents")
@DeleteMapping(value = "agentstatus")
public void clearAgentStatuses(@RequestParam Map<String, String> attributes) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@

import io.swagger.annotations.ApiImplicitParam;
import io.swagger.annotations.ApiOperation;
import io.swagger.annotations.ApiParam;
import org.springframework.security.access.annotation.Secured;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PutMapping;
import org.springframework.web.bind.annotation.RestController;
import rocks.inspectit.ocelot.file.FileInfo;
import rocks.inspectit.ocelot.rest.util.RequestUtil;
import rocks.inspectit.ocelot.security.config.UserRoleConfiguration;

import javax.servlet.http.HttpServletRequest;
import java.io.IOException;
Expand All @@ -30,6 +33,8 @@ public Collection<FileInfo> listContents(HttpServletRequest request) {
return fileManager.getWorkingDirectory().listConfigurationFiles(path);
}


@Secured(UserRoleConfiguration.WRITE_ACCESS_ROLE)
@ApiOperation(value = "Create a directory", notes = "Creates a new, empty directory including its parent folders. Does nothing if the directory already exists.")
@ApiImplicitParam(name = "Path", value = "The part of the url after /directories/ define the path of the directory to create.")
@PutMapping(value = "directories/**")
Expand All @@ -38,6 +43,7 @@ public void createNewDirectory(HttpServletRequest request) throws IOException {
fileManager.getWorkingDirectory().createConfigurationDirectory(path);
}

@Secured(UserRoleConfiguration.WRITE_ACCESS_ROLE)
@ApiOperation(value = "Delete a directory", notes = "Deletes a directory including its contents.")
@ApiImplicitParam(name = "Path", value = "The part of the url after /directories/ define the path of the directory to delete.")
@DeleteMapping(value = "directories/**")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import io.swagger.annotations.*;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.security.access.annotation.Secured;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.*;
import rocks.inspectit.ocelot.file.FileData;
import rocks.inspectit.ocelot.rest.util.RequestUtil;
import rocks.inspectit.ocelot.security.config.UserRoleConfiguration;

import javax.servlet.http.HttpServletRequest;
import java.io.IOException;
Expand All @@ -21,6 +23,7 @@ public class FileController extends FileBaseController {
@Autowired
private ObjectMapper objectMapper;

@Secured(UserRoleConfiguration.WRITE_ACCESS_ROLE)
@ApiOperation(value = "Write a file", notes = "Creates or overwrites a file with the provided text content")
@ApiImplicitParam(name = "Path", type = "string", value = "The part of the url after /files/ defines the path to the file to write.")
@PutMapping(value = "files/**")
Expand Down Expand Up @@ -75,6 +78,7 @@ public Object readFile(HttpServletRequest request,
});
}

@Secured(UserRoleConfiguration.WRITE_ACCESS_ROLE)
@ApiOperation(value = "Delete a file", notes = "Deletes the given file")
@ApiImplicitParam(name = "Path", type = "string", value = "The part of the url after /files/ defines the path to the file to delete.")
@DeleteMapping(value = "files/**")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@

import io.swagger.annotations.ApiOperation;
import org.apache.commons.lang3.StringUtils;
import org.springframework.security.access.annotation.Secured;
import org.springframework.web.bind.annotation.PutMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RestController;
import rocks.inspectit.ocelot.file.FileMoveDescription;
import rocks.inspectit.ocelot.file.accessor.workingdirectory.AbstractWorkingDirectoryAccessor;
import rocks.inspectit.ocelot.security.config.UserRoleConfiguration;

import java.io.IOException;

Expand All @@ -16,6 +18,7 @@
@RestController
public class MoveController extends FileBaseController {

@Secured(UserRoleConfiguration.WRITE_ACCESS_ROLE)
@ApiOperation(value = "Move or rename a file or directory")
@PutMapping(value = "move")
public void moveFileOrDirectory(@RequestBody FileMoveDescription moveDescription) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.ResponseEntity;
import org.springframework.security.access.annotation.Secured;
import org.springframework.util.StringUtils;
import org.springframework.web.bind.annotation.*;
import rocks.inspectit.ocelot.mappings.AgentMappingManager;
import rocks.inspectit.ocelot.mappings.model.AgentMapping;
import rocks.inspectit.ocelot.rest.AbstractBaseController;
import rocks.inspectit.ocelot.security.audit.AuditDetail;
import rocks.inspectit.ocelot.security.audit.EntityAuditLogger;
import rocks.inspectit.ocelot.security.config.UserRoleConfiguration;

import javax.validation.Valid;
import java.io.IOException;
Expand Down Expand Up @@ -49,6 +51,7 @@ public List<AgentMapping> getMappings() {
* @param agentMappings The new {@link AgentMapping}s
* @throws IOException In case the new mappings cannot be written into a file.
*/
@Secured(UserRoleConfiguration.WRITE_ACCESS_ROLE)
@PutMapping(value = "mappings")
public void putMappings(@Valid @RequestBody List<AgentMapping> agentMappings) throws IOException {
mappingManager.setAgentMappings(agentMappings);
Expand Down Expand Up @@ -77,6 +80,7 @@ public ResponseEntity<AgentMapping> getMappingByName(@PathVariable("mappingName"
* @return 200 if the mapping has been deleted or 404 if it does not exist
* @throws IOException In case of an error during deletion
*/
@Secured(UserRoleConfiguration.WRITE_ACCESS_ROLE)
@DeleteMapping(value = "mappings/{mappingName}")
public ResponseEntity deleteMappingByName(@PathVariable("mappingName") String mappingName) throws IOException {
boolean isDeleted = mappingManager.deleteAgentMapping(mappingName);
Expand Down Expand Up @@ -104,6 +108,7 @@ public ResponseEntity deleteMappingByName(@PathVariable("mappingName") String ma
* @return 200 in case the operation was successful
* @throws IOException In case of an error
*/
@Secured(UserRoleConfiguration.WRITE_ACCESS_ROLE)
@PutMapping(value = "mappings/{mappingName}")
public ResponseEntity putMapping(@PathVariable("mappingName") String mappingName, @Valid @RequestBody AgentMapping agentMapping, @RequestParam(required = false) String before, @RequestParam(required = false) String after) throws IOException {
checkArgument(!StringUtils.isEmpty(mappingName), "The mapping name should not be empty or null.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@
import org.springframework.dao.DataAccessException;
import org.springframework.dao.EmptyResultDataAccessException;
import org.springframework.http.ResponseEntity;
import org.springframework.security.access.annotation.Secured;
import org.springframework.util.StringUtils;
import org.springframework.web.bind.annotation.*;
import org.springframework.web.util.UriComponentsBuilder;
import rocks.inspectit.ocelot.rest.AbstractBaseController;
import rocks.inspectit.ocelot.rest.ErrorInfo;
import rocks.inspectit.ocelot.security.config.UserRoleConfiguration;
import rocks.inspectit.ocelot.user.User;
import rocks.inspectit.ocelot.user.UserService;

Expand Down Expand Up @@ -45,6 +47,7 @@ public class UserController extends AbstractBaseController {
@Autowired
private UserService userService;

@Secured(UserRoleConfiguration.ADMIN_ACCESS_ROLE)
@ApiOperation(value = "Select users", notes = "Fetches the list of registered users." +
" If a username query parameter is given, the list is filtered to contain only the user matching the given username." +
" If none match, an empty list is returned.")
Expand All @@ -61,6 +64,7 @@ public List<User> selectUsers(@ApiParam("If specified only the user with the giv
}
}

@Secured(UserRoleConfiguration.ADMIN_ACCESS_ROLE)
@ApiOperation(value = "Fetch a single user", notes = "Fetches a single user based on his ID.")
@GetMapping("users/{id}")
public ResponseEntity<?> getUser(@ApiParam("The ID of the user")
Expand All @@ -71,6 +75,7 @@ public ResponseEntity<?> getUser(@ApiParam("The ID of the user")
}


@Secured(UserRoleConfiguration.ADMIN_ACCESS_ROLE)
@ApiOperation(value = "Add a new user", notes = "Registers a user with a given username and password.")
@PostMapping("users")
public ResponseEntity<?> addUser(
Expand Down Expand Up @@ -98,6 +103,7 @@ public ResponseEntity<?> addUser(
}


@Secured(UserRoleConfiguration.ADMIN_ACCESS_ROLE)
@ApiOperation(value = "Delete a user", notes = "Deletes a user based on his id. After he is deleted, he immediately is unauthorized.")
@DeleteMapping("users/{id}")
public ResponseEntity<?> deleteUser(@ApiParam("The is of the user to delete")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,26 @@
import com.google.common.annotations.VisibleForTesting;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.ldap.core.support.LdapContextSource;
import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder;
import org.springframework.security.config.annotation.method.configuration.EnableGlobalMethodSecurity;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.builders.WebSecurity;
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
import org.springframework.security.config.http.SessionCreationPolicy;
import org.springframework.security.crypto.password.LdapShaPasswordEncoder;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.security.web.authentication.www.BasicAuthenticationFilter;
import rocks.inspectit.ocelot.config.model.InspectitServerSettings;
import rocks.inspectit.ocelot.config.model.LdapSettings;
import rocks.inspectit.ocelot.filters.AccessLogFilter;
import rocks.inspectit.ocelot.security.jwt.JwtTokenFilter;
import rocks.inspectit.ocelot.security.jwt.JwtTokenManager;
import rocks.inspectit.ocelot.security.userdetails.CustomLdapUserDetailsService;
import rocks.inspectit.ocelot.security.userdetails.LocalUserDetailsService;

import javax.servlet.http.HttpServletResponse;
import java.util.Collections;

import static rocks.inspectit.ocelot.security.userdetails.LocalUserDetailsService.DEFAULT_ACCESS_USER_ROLE;

/**
* Spring security configuration enabling authentication on all except excluded endpoints.
*/
Expand All @@ -47,6 +45,9 @@ public class SecurityConfiguration extends WebSecurityConfigurerAdapter {
@Autowired
private LocalUserDetailsService localUserDetailsService;

@Autowired(required = false)
private CustomLdapUserDetailsService customLdapUserDetailsService;

@Autowired
@VisibleForTesting
InspectitServerSettings serverSettings;
Expand All @@ -68,42 +69,34 @@ public void configure(WebSecurity web) {
@Override
protected void configure(HttpSecurity http) throws Exception {
http
.csrf().disable()
.sessionManagement().sessionCreationPolicy(SessionCreationPolicy.STATELESS)
.csrf()
.disable()
.sessionManagement()
.sessionCreationPolicy(SessionCreationPolicy.STATELESS)

.and()
.cors()

.and()
.authorizeRequests()
.anyRequest().hasRole(getAccessRole())
.anyRequest()
.hasRole(UserRoleConfiguration.READ_ACCESS)

.and()
// Custom authentication endpoint to prevent sending the "WWW-Authenticate" which causes Browsers to open the basic authentication dialog.
// See the following post: https://stackoverflow.com/a/50023070/2478009
.httpBasic().authenticationEntryPoint((req, resp, authException) -> resp.sendError(HttpServletResponse.SC_UNAUTHORIZED, authException.getMessage()))
.httpBasic()
.authenticationEntryPoint((req, resp, authException) -> resp.sendError(HttpServletResponse.SC_UNAUTHORIZED, authException
.getMessage()))

.and()
//TODO: The "correct" way of selectively enabling token based would be to have multiple spring security configs.
//However, previous attempts of doing so were unsuccessful, therefore we simply exclude them manually in the filter
.addFilterBefore(
new JwtTokenFilter(tokenManager, eventPublisher, Collections.singletonList("/api/v1/account/password")),
BasicAuthenticationFilter.class
).addFilterBefore(accessLogFilter.getFilter(), JwtTokenFilter.class);
}

/**
* Returns the role name which is required by users to get access to the secured API endpoints.
* In case LDAP is not used, a constant role name is used, otherwise the configured role name of the LDAP settings is used.
*
* @return the role name to use
*/
private String getAccessRole() {
if (serverSettings.getSecurity().isLdapAuthentication()) {
return serverSettings.getSecurity().getLdap().getAdminGroup();
} else {
return DEFAULT_ACCESS_USER_ROLE;
}
)
.addFilterBefore(accessLogFilter.getFilter(), JwtTokenFilter.class);
}

@Override
Expand All @@ -117,17 +110,11 @@ protected void configure(AuthenticationManagerBuilder auth) throws Exception {
/**
* Configures the user authentication to use LDAP user management and authentication
*/
@SuppressWarnings("deprecation")
private void configureLdapAuthentication(AuthenticationManagerBuilder auth) throws Exception {
LdapContextSource contextSource = getApplicationContext().getBean(LdapContextSource.class);
LdapSettings ldapSettings = serverSettings.getSecurity().getLdap();

auth
.ldapAuthentication()
.userSearchFilter(ldapSettings.getUserSearchFilter())
.userSearchBase(ldapSettings.getUserSearchBase())
.groupSearchFilter(ldapSettings.getGroupSearchFilter())
.groupSearchBase(ldapSettings.getGroupSearchBase())
.contextSource(contextSource);
.userDetailsService(customLdapUserDetailsService)
.passwordEncoder(new LdapShaPasswordEncoder());
}

/**
Expand All @@ -138,4 +125,4 @@ private void configureLocalAuthentication(AuthenticationManagerBuilder auth) thr
.userDetailsService(localUserDetailsService)
.passwordEncoder(passwordEncoder);
}
}
}
Loading