Implementing Security Configuration and Role Management #10
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Owner
nklimovych
commented
May 10, 2024
nklimovych
commented
- Added SecurityConfig class with PasswordEncoder and SecurityFilterChain beans
- Implemented UserDetails interface in the User class.
- Added the implementation for the UserDetailsService interface.
- Added Role entity.
- Added RoleRepository.
- Added roles field to the User entity.
- Added Liquibase changesets to insert roles into DB and assign roles for users.
…in beans. Implemented UserDetails interface in the User class. Added the implementation for UserDetailsService interface. Added Role entity. Added RoleRepository. Added roles field to the User entity. Added Liquibase changesets to insert roles into DB and assign roles for users.
karma-o
approved these changes
May 14, 2024
|
|
||
| @Column(nullable = false, unique = true) | ||
| @Enumerated(EnumType.STRING) | ||
| private RoleName role; |
There was a problem hiding this comment.
Suggested change
| private RoleName role; | |
| private RoleName name; |
| @@ -26,6 +32,8 @@ public UserResponseDto save(UserRegistrationRequestDto requestDto) | |||
|
|
|||
| User user = userMapper.toModel(requestDto); | |||
| user.setEmail(email); | |||
| user.setPassword(passwordEncoder.encode(user.getPassword())); | |||
| user.setRoles(roleRepository.findAllByRoles(Set.of(RoleName.USER))); | |||
There was a problem hiding this comment.
If we have hardcoded only USER role here why wrap it into a set and do a findAll...()? Lets simplify it
Suggested change
| user.setRoles(roleRepository.findAllByRoles(Set.of(RoleName.USER))); | |
| user.setRoles(Collections.singleton(roleRepository.findByName(RoleName.USER))); |
|
|
||
| public interface RoleRepository extends JpaRepository<Role, Long> { | ||
|
|
||
| @Query("SELECT r FROM Role r WHERE r.role IN :roles") |
There was a problem hiding this comment.
I do not think @Querry is necessary here
Suggested change
| @Query("SELECT r FROM Role r WHERE r.role IN :roles") |
…or the new user in the 'save' method in UserService, and replace the 'findAllByRoles' method with the 'findByName' method.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.