-
-
Notifications
You must be signed in to change notification settings - Fork 367
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
Change from single user (config) to database based multi user system. #1165
base: master
Are you sure you want to change the base?
Changes from 5 commits
ae0d9d2
61e3f00
7a15857
9d84a5a
1359cc8
cc3db61
4f51fc3
45d9ab3
7aa3bd6
6564cdc
edd2365
778318f
fa7a2fc
54e7198
98b9ce8
040878c
647ca1c
42980df
0236b55
0276925
bd62f19
2f67258
6fde7dc
d2273f7
4a7a616
ab84315
ed4e602
6a6c96a
a3d6d16
a3f2fd2
291f683
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,8 +18,9 @@ | |
*/ | ||
package de.rwth.idsg.steve.config; | ||
|
||
import de.rwth.idsg.steve.SteveConfiguration; | ||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import com.fasterxml.jackson.datatype.joda.JodaModule; | ||
Check warning on line 23 in src/main/java/de/rwth/idsg/steve/config/SecurityConfiguration.java
|
||
import com.google.common.base.Strings; | ||
import de.rwth.idsg.steve.SteveProdCondition; | ||
import de.rwth.idsg.steve.web.api.ApiControllerAdvice; | ||
|
@@ -38,13 +39,13 @@ | |
import org.springframework.security.config.http.SessionCreationPolicy; | ||
import org.springframework.security.core.Authentication; | ||
import org.springframework.security.core.AuthenticationException; | ||
import org.springframework.security.core.userdetails.User; | ||
import org.springframework.security.core.userdetails.UserDetails; | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Commented code doesn't the review. Could you remove the code instead? |
||
//import org.springframework.security.core.userdetails.UserDetails; | ||
//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.crypto.password.PasswordEncoder; | ||
import org.springframework.security.provisioning.InMemoryUserDetailsManager; | ||
//import org.springframework.security.provisioning.InMemoryUserDetailsManager; | ||
import org.springframework.security.web.AuthenticationEntryPoint; | ||
import org.springframework.security.web.SecurityFilterChain; | ||
import org.springframework.security.web.authentication.preauth.AbstractPreAuthenticatedProcessingFilter; | ||
|
@@ -56,8 +57,16 @@ | |
|
||
import static de.rwth.idsg.steve.SteveConfiguration.CONFIG; | ||
|
||
import javax.sql.DataSource; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.boot.jdbc.DataSourceBuilder; | ||
import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; | ||
import org.springframework.security.provisioning.JdbcUserDetailsManager; | ||
import org.springframework.security.provisioning.UserDetailsManager; | ||
|
||
/** | ||
* @author Sevket Goekay <[email protected]> | ||
* @author Frank Brosi | ||
* @since 07.01.2015 | ||
*/ | ||
@Slf4j | ||
|
@@ -72,12 +81,18 @@ | |
* | ||
* [1] https://spring.io/blog/2017/11/01/spring-security-5-0-0-rc1-released#password-storage-format | ||
* [2] {@link PasswordEncoderFactories#createDelegatingPasswordEncoder()} | ||
*/ | ||
|
||
/** | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Useless change |
||
* | ||
* @return | ||
*/ | ||
@Bean | ||
public PasswordEncoder passwordEncoder() { | ||
return CONFIG.getAuth().getPasswordEncoder(); | ||
} | ||
|
||
/* | ||
@Bean | ||
public UserDetailsService userDetailsService() { | ||
UserDetails webPageUser = User.builder() | ||
|
@@ -88,6 +103,8 @@ | |
|
||
return new InMemoryUserDetailsManager(webPageUser); | ||
} | ||
*/ | ||
|
||
|
||
@Bean | ||
public WebSecurityCustomizer webSecurityCustomizer() { | ||
|
@@ -101,6 +118,7 @@ | |
public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception { | ||
final String prefix = CONFIG.getSpringManagerMapping(); | ||
|
||
/* | ||
return http | ||
.authorizeHttpRequests( | ||
req -> req.antMatchers(prefix + "/**").hasRole("ADMIN") | ||
|
@@ -115,6 +133,105 @@ | |
req -> req.logoutUrl(prefix + "/signout") | ||
) | ||
.build(); | ||
*/ | ||
|
||
return http | ||
.authorizeRequests( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What are the expected rights? On my side, I'd love that users can read but not create/update/delete. |
||
req -> req | ||
.antMatchers(prefix + "/home").hasAnyRole("USER", "ADMIN") | ||
.antMatchers(prefix + "/users/" + "**").hasAnyRole("USER", "ADMIN") | ||
//.antMatchers(prefix + "/usergroups/" + "**").hasAnyRole("USER", "ADMIN") | ||
.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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't forget to manage the api route There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The api route is managed by the api key security filter chain. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right. I forgot it 😅 |
||
) | ||
.sessionManagement( | ||
req -> req | ||
.invalidSessionUrl(prefix + "/signin") | ||
) | ||
.formLogin( | ||
req -> req | ||
.loginPage(prefix + "/signin") | ||
.permitAll() | ||
) | ||
.logout( | ||
req -> req | ||
.logoutUrl(prefix + "/signout") | ||
) | ||
|
||
.exceptionHandling( | ||
req -> req | ||
.accessDeniedPage(prefix + "/noAccess") | ||
) | ||
.build(); | ||
} | ||
|
||
@Autowired | ||
private DataSource dataSource; | ||
|
||
@Bean | ||
public DataSource getDataSource() { | ||
SteveConfiguration.DB dbConfig = CONFIG.getDb(); | ||
DataSourceBuilder dataSourceBuilder = DataSourceBuilder.create(); | ||
dataSourceBuilder.url("jdbc:mysql:https://" + dbConfig.getIp() | ||
+ ":" + dbConfig.getPort() + "/" + dbConfig.getSchema()); | ||
dataSourceBuilder.username(dbConfig.getUserName()); | ||
dataSourceBuilder.password(dbConfig.getPassword()); | ||
return dataSourceBuilder.build(); | ||
} | ||
|
||
/** | ||
* | ||
* @param auth | ||
* @throws Exception | ||
*/ | ||
@Autowired | ||
public void configureGlobal(AuthenticationManagerBuilder auth) | ||
throws Exception { | ||
auth.jdbcAuthentication() | ||
.passwordEncoder(CONFIG.getAuth().getPasswordEncoder()) | ||
.dataSource(dataSource) | ||
.usersByUsernameQuery("select username,password,enabled from webusers where username = ?") | ||
.authoritiesByUsernameQuery("select username,authority from webauthorities where username = ?"); | ||
} | ||
|
||
|
||
/** | ||
* | ||
* @return | ||
*/ | ||
@Bean | ||
public UserDetailsManager authenticateUsers() { | ||
JdbcUserDetailsManager users = new JdbcUserDetailsManager(dataSource); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be discussed but I think a home-made |
||
// Adapt used SQL-Commands to the right table names (user -> webuser; authorities -> webauthorities) | ||
users.setAuthoritiesByUsernameQuery("select username,authority from webauthorities where username=?"); | ||
users.setUsersByUsernameQuery("select username,password,enabled from webusers where username=?"); | ||
|
||
/* Adding the admin from config-file to the database/webusers | ||
--> changing the name in the file will not delete the corresponding webuser in the database! | ||
users.setCreateUserSql("insert into webusers (username, password, enabled) values (?,?,?)"); | ||
users.setCreateAuthoritySql("insert into webauthorities (username, authority) values (?,?)"); | ||
users.setUserExistsSql("select username from webusers where username = ?"); | ||
users.setUpdateUserSql("update webusers set password = ?, enabled = ? where username = ?"); | ||
users.setDeleteUserAuthoritiesSql("delete from webauthorities where username = ?"); | ||
|
||
UserDetails localUser = User.builder() | ||
.username(CONFIG.getAuth().getUserName()) | ||
.password(CONFIG.getAuth().getEncodedPassword()) | ||
.roles("USER") | ||
.build(); | ||
|
||
if (users.userExists(CONFIG.getAuth().getUserName())) | ||
{ | ||
users.updateUser(localUser); | ||
} | ||
else | ||
{ | ||
users.createUser(localUser); | ||
} | ||
*/ | ||
return users; | ||
} | ||
|
||
@Bean | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
/* | ||
* SteVe - SteckdosenVerwaltung - https://github.com/steve-community/steve | ||
* Copyright (C) 2013-2023 SteVe Community Team | ||
* All Rights Reserved. | ||
* | ||
* This program is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU General Public License as published by | ||
* the Free Software Foundation, either version 3 of the License, or | ||
* (at your option) any later version. | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU General Public License | ||
* along with this program. If not, see <https://www.gnu.org/licenses/>. | ||
*/ | ||
package de.rwth.idsg.steve.repository; | ||
|
||
import de.rwth.idsg.steve.repository.dto.WebUser; | ||
import de.rwth.idsg.steve.web.dto.WebUserForm; | ||
import de.rwth.idsg.steve.web.dto.WebUserQueryForm; | ||
|
||
import java.util.List; | ||
|
||
/** | ||
* @author Frank Brosi | ||
* @since 21.03.2022 | ||
*/ | ||
public interface WebUserRepository { | ||
List<WebUser.Overview> getOverview(WebUserQueryForm form); | ||
WebUser.Details getDetails(String webusername); | ||
|
||
void add(WebUserForm form); | ||
void update(WebUserForm form); | ||
void delete(String webusername, String role); | ||
void delete(String webusername); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
/* | ||
* SteVe - SteckdosenVerwaltung - https://github.com/steve-community/steve | ||
* Copyright (C) 2013-2023 SteVe Community Team | ||
* All Rights Reserved. | ||
* | ||
* This program is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU General Public License as published by | ||
* the Free Software Foundation, either version 3 of the License, or | ||
* (at your option) any later version. | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU General Public License | ||
* along with this program. If not, see <https://www.gnu.org/licenses/>. | ||
*/ | ||
package de.rwth.idsg.steve.repository.dto; | ||
|
||
import java.util.List; | ||
import jooq.steve.db.tables.records.WebauthoritiesRecord; | ||
import jooq.steve.db.tables.records.WebusersRecord; | ||
import lombok.Builder; | ||
import lombok.Getter; | ||
|
||
//import java.util.Optional; | ||
|
||
/** | ||
* @author Frank Brosi | ||
* @since 01.04.2022 | ||
*/ | ||
public class WebUser { | ||
|
||
@Getter | ||
@Builder | ||
public static final class Overview { | ||
private final Boolean enabled; | ||
private final String webusername, roles; | ||
} | ||
|
||
@Getter | ||
@Builder | ||
public static final class Details { | ||
private final WebusersRecord webusersRecord; | ||
private final List<WebauthoritiesRecord> webauthoritiesRecordList; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,129 +16,137 @@ | |
* You should have received a copy of the GNU General Public License | ||
* along with this program. If not, see <https://www.gnu.org/licenses/>. | ||
*/ | ||
package de.rwth.idsg.steve.repository.impl; | ||
|
||
import de.rwth.idsg.steve.repository.GenericRepository; | ||
import de.rwth.idsg.steve.repository.ReservationStatus; | ||
import de.rwth.idsg.steve.repository.dto.DbVersion; | ||
import de.rwth.idsg.steve.utils.DateTimeUtils; | ||
import de.rwth.idsg.steve.web.dto.Statistics; | ||
import lombok.extern.slf4j.Slf4j; | ||
import org.joda.time.DateTime; | ||
import org.jooq.DSLContext; | ||
import org.jooq.Field; | ||
import org.jooq.Record2; | ||
import org.jooq.Record8; | ||
import org.jooq.Record9; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.stereotype.Repository; | ||
|
||
import static de.rwth.idsg.steve.utils.CustomDSL.date; | ||
import static jooq.steve.db.Tables.RESERVATION; | ||
import static jooq.steve.db.Tables.TRANSACTION; | ||
import static jooq.steve.db.tables.ChargeBox.CHARGE_BOX; | ||
import static jooq.steve.db.tables.OcppTag.OCPP_TAG; | ||
import static jooq.steve.db.tables.SchemaVersion.SCHEMA_VERSION; | ||
import static jooq.steve.db.tables.User.USER; | ||
import static jooq.steve.db.tables.Webusers.WEBUSERS; | ||
import static org.jooq.impl.DSL.max; | ||
import static org.jooq.impl.DSL.select; | ||
|
||
/** | ||
* @author Sevket Goekay <[email protected]> | ||
* @since 14.08.2014 | ||
*/ | ||
@Slf4j | ||
@Repository | ||
public class GenericRepositoryImpl implements GenericRepository { | ||
|
||
@Autowired private DSLContext ctx; | ||
|
||
@Override | ||
public Statistics getStats() { | ||
DateTime now = DateTime.now(); | ||
DateTime yesterdaysNow = now.minusDays(1); | ||
|
||
Field<Integer> numChargeBoxes = | ||
ctx.selectCount() | ||
.from(CHARGE_BOX) | ||
.asField("num_charge_boxes"); | ||
|
||
Field<Integer> numOcppTags = | ||
ctx.selectCount() | ||
.from(OCPP_TAG) | ||
.asField("num_ocpp_tags"); | ||
|
||
Field<Integer> numUsers = | ||
ctx.selectCount() | ||
.from(USER) | ||
.asField("num_users"); | ||
|
||
Field<Integer> numReservations = | ||
ctx.selectCount() | ||
.from(RESERVATION) | ||
.where(RESERVATION.EXPIRY_DATETIME.greaterThan(now)) | ||
.and(RESERVATION.STATUS.eq(ReservationStatus.ACCEPTED.name())) | ||
.asField("num_reservations"); | ||
|
||
Field<Integer> numTransactions = | ||
ctx.selectCount() | ||
.from(TRANSACTION) | ||
.where(TRANSACTION.STOP_TIMESTAMP.isNull()) | ||
.asField("num_transactions"); | ||
|
||
Field<Integer> heartbeatsToday = | ||
ctx.selectCount() | ||
.from(CHARGE_BOX) | ||
.where(date(CHARGE_BOX.LAST_HEARTBEAT_TIMESTAMP).eq(date(now))) | ||
.asField("heartbeats_today"); | ||
|
||
Field<Integer> heartbeatsYesterday = | ||
ctx.selectCount() | ||
.from(CHARGE_BOX) | ||
.where(date(CHARGE_BOX.LAST_HEARTBEAT_TIMESTAMP).eq(date(yesterdaysNow))) | ||
.asField("heartbeats_yesterday"); | ||
|
||
Field<Integer> heartbeatsEarlier = | ||
ctx.selectCount() | ||
.from(CHARGE_BOX) | ||
.where(date(CHARGE_BOX.LAST_HEARTBEAT_TIMESTAMP).lessThan(date(yesterdaysNow))) | ||
.asField("heartbeats_earlier"); | ||
|
||
Record8<Integer, Integer, Integer, Integer, Integer, Integer, Integer, Integer> gs = | ||
Field<Integer> numWebUsers = | ||
ctx.selectCount() | ||
.from(WEBUSERS) | ||
.asField("num_webusers"); | ||
|
||
Record9<Integer, Integer, Integer, Integer, Integer, Integer, Integer, Integer, Integer> gs = | ||
ctx.select( | ||
numChargeBoxes, | ||
numOcppTags, | ||
numUsers, | ||
numReservations, | ||
numTransactions, | ||
heartbeatsToday, | ||
heartbeatsYesterday, | ||
heartbeatsEarlier | ||
heartbeatsEarlier, | ||
numWebUsers | ||
).fetchOne(); | ||
|
||
return Statistics.builder() | ||
.numChargeBoxes(gs.value1()) | ||
.numOcppTags(gs.value2()) | ||
.numUsers(gs.value3()) | ||
.numReservations(gs.value4()) | ||
.numTransactions(gs.value5()) | ||
.heartbeatToday(gs.value6()) | ||
.heartbeatYesterday(gs.value7()) | ||
.heartbeatEarlier(gs.value8()) | ||
.numWebUsers(gs.value9()) | ||
.build(); | ||
} | ||
|
||
@Override | ||
public DbVersion getDBVersion() { | ||
Record2<String, DateTime> record = ctx.select(SCHEMA_VERSION.VERSION, SCHEMA_VERSION.INSTALLED_ON) | ||
.from(SCHEMA_VERSION) | ||
.where(SCHEMA_VERSION.INSTALLED_RANK.eq( | ||
select(max(SCHEMA_VERSION.INSTALLED_RANK)).from(SCHEMA_VERSION))) | ||
.fetchOne(); | ||
|
||
String ts = DateTimeUtils.humanize(record.value2()); | ||
return DbVersion.builder() | ||
.version(record.value1()) | ||
.updateTimestamp(ts) | ||
.build(); | ||
} | ||
} | ||
Check warning on line 152 in src/main/java/de/rwth/idsg/steve/repository/impl/GenericRepositoryImpl.java
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks the 2 new dependencies are not used. They should be removed IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both dependencies used in SecurityConfiguration.java
by "import org.springframework.boot.jdbc.DataSourceBuilder;" (line 55)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
steve is not using spring boot though. it is a spring app, not a spring boot app.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@goekay any feedback about the feature itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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