Change from single user (config) to database based multi user system. #1165
Conversation
pom.xml
Outdated
| @@ -772,5 +772,17 @@ | |||
| <artifactId>encoder</artifactId> | |||
| <version>1.2.3</version> | |||
| </dependency> | |||
| <dependency> | |||
| <groupId>org.springframework.boot</groupId> | |||
There was a problem hiding this comment.
It looks the 2 new dependencies are not used. They should be removed IMO.
There was a problem hiding this comment.
Both dependencies used in SecurityConfiguration.java
by "import org.springframework.boot.jdbc.DataSourceBuilder;" (line 55)
There was a problem hiding this comment.
steve is not using spring boot though. it is a spring app, not a spring boot app.
There was a problem hiding this comment.
@fnkbsi steve uses jooq for the data access. You should be able to replace spring-jdbc by that. Then no need to datasourcebuilder.
There was a problem hiding this comment.
@goekay any feedback about the feature itself?
There was a problem hiding this comment.
i find the feature very valuable, but need time to to review the code. at a glance, i find that the implementation needs some tidying up which is already happening. thanks @fnkbsi
| import org.springframework.security.core.userdetails.UserDetailsService; | ||
| import org.springframework.security.crypto.factory.PasswordEncoderFactories; | ||
| import org.springframework.security.crypto.password.DelegatingPasswordEncoder; | ||
| //import org.springframework.security.core.userdetails.User; |
There was a problem hiding this comment.
Commented code doesn't the review. Could you remove the code instead?
…initDataSource() (changed to public) to use the dataSource in SecurityConfiguration; deleted initDataSource in dslContext()
@Autowired, also changed position; removed getDataSource Methode with the spring-boot import
…endency org.springframework artifactId spring-jdbc to the springframework section
…ectly; At dslContext(), if dataSource isn't initialized then run initDataSource()
| @@ -82,13 +84,14 @@ | |||
| @ComponentScan("de.rwth.idsg.steve") | |||
| public class BeanConfiguration implements WebMvcConfigurer { | |||
|
|
|||
| private HikariDataSource dataSource; | |||
| @Autowired private HikariDataSource dataSource; | |||
There was a problem hiding this comment.
You should remove the attribut and replace it by the bean factory method everywhere it is used (spring proxies are not supposed to instanciate more than once)
| @@ -127,7 +130,9 @@ private void initDataSource() { | |||
| */ | |||
| @Bean | |||
| public DSLContext dslContext() { | |||
| initDataSource(); | |||
There was a problem hiding this comment.
HikariDataSource dataSource = initDataSource(); // better if named dataSource()| /** | ||
| * Password encoding changed with spring-security 5.0.0. We either have to use a prefix before the password to | ||
| * indicate which actual encoder {@link DelegatingPasswordEncoder} should use [1, 2] or specify the encoder as we do. | ||
| * | ||
| * [1] https://spring.io/blog/2017/11/01/spring-security-5-0-0-rc1-released#password-storage-format | ||
| * [2] {@link PasswordEncoderFactories#createDelegatingPasswordEncoder()} | ||
| */ | ||
|
|
||
| /** |
| .authorizeHttpRequests( | ||
| req -> req.antMatchers(prefix + "/**").hasRole("ADMIN") | ||
| ) | ||
| .authorizeRequests( |
There was a problem hiding this comment.
What are the expected rights?
On my side, I'd love that users can read but not create/update/delete.
Admin can do everything.
| .antMatchers(prefix + "/ocppTags/" + "**").hasAnyRole("USER", "ADMIN") | ||
| .antMatchers(prefix + "/signout/" + "**").hasAnyRole("USER", "ADMIN") | ||
| .antMatchers(prefix + "/noAccess/" + "**").hasAnyRole("USER", "ADMIN") | ||
| .antMatchers(prefix + "/**").hasRole("ADMIN") |
There was a problem hiding this comment.
Don't forget to manage the api route
There was a problem hiding this comment.
The api route is managed by the api key security filter chain.
A similar approach for the api key or a combination of api key and user/password is possible, but needs to be discussed.
…Autowired attribute from HikariDataSource
but no "write" access
| */ | ||
| @Bean | ||
| public UserDetailsManager authenticateUsers() { | ||
| JdbcUserDetailsManager users = new JdbcUserDetailsManager(dataSource); |
There was a problem hiding this comment.
To be discussed but I think a home-made JooqUserDetailsManager is not a difficult task and will avoid an extra dependency.
… not exist clauses
…compatibiliy to MySql
|
@goekay Please, review. The change will improve the product a lot. |
Implements multi user access. Uses the steve database to store the "webusers" and their roles. Privileges on websites are set in the source code in SecurityConfiguration.java. Sites for administration of the "webuser" included.
May fixes #991