test code #6

don-bigdad · 2023-10-25T11:29:42Z

No description provided.

anigilator479 · 2023-10-26T11:29:59Z

src/main/java/com/example/springbootbookshop/entity/User.java

+    private String lastName;
+    private String shippingAddress;
+
+    @ManyToMany(fetch = FetchType.EAGER)


you can delete FetchType.Eager and use @entitygraph(attributePaths = "roles") in UsersRepository on findByEmail() method, it will initialize roles when the method is called

Elena-Bruyako

Good job! Let's improve your code)

Wrong link to HW

Elena-Bruyako · 2023-10-27T07:31:54Z

src/main/java/com/example/springbootbookshop/controller/AuthenticationController.java

+@RestController
+@Validated
+@RequestMapping(value = "/auth")
+public class AuthenticationController {


Do we need to add Tag annotation here?

Elena-Bruyako · 2023-10-27T07:33:00Z

src/main/java/com/example/springbootbookshop/controller/BookController.java

@@ -41,6 +42,7 @@ public List<BookDto> getAll(@PageableDefault(size = 5, page = 0)
    }

    @DeleteMapping(value = "/{id}")
+    @PreAuthorize("hasRole('ROLE_ADMIN')")


Do we need to add ROLE_USER for some endpoint?

Elena-Bruyako · 2023-10-27T07:33:53Z

src/main/java/com/example/springbootbookshop/dto/UserLoginRequestDto.java

+import jakarta.validation.constraints.Email;
+import jakarta.validation.constraints.NotNull;
+
+public record UserLoginRequestDto(@NotNull @Email String email,


@NotBlank will be better

Elena-Bruyako · 2023-10-27T07:34:00Z

src/main/java/com/example/springbootbookshop/dto/UserLoginRequestDto.java

+import jakarta.validation.constraints.NotNull;
+
+public record UserLoginRequestDto(@NotNull @Email String email,
+                                  @NotNull String password) {


Elena-Bruyako · 2023-10-27T07:35:31Z

src/main/java/com/example/springbootbookshop/dto/UserRegistrationRequestDto.java

+import jakarta.validation.constraints.NotBlank;
+import jakarta.validation.constraints.Size;
+
+@FieldMatch


Suggested change

@FieldMatch

@FieldMatch(fields = {"password", "repeatPassword"})

Elena-Bruyako · 2023-10-27T07:40:26Z

src/main/java/com/example/springbootbookshop/repository/RoleRepository.java

+
+@Repository
+public interface RoleRepository extends JpaRepository<Role, Long> {
+


remove empty line

Elena-Bruyako · 2023-10-27T07:40:52Z

src/main/java/com/example/springbootbookshop/repository/UserRepository.java

+
+@Repository
+public interface UserRepository extends JpaRepository<User, Long> {
+


Elena-Bruyako · 2023-10-27T07:42:14Z

src/main/java/com/example/springbootbookshop/repository/UserRepository.java

+@Repository
+public interface UserRepository extends JpaRepository<User, Long> {
+
+    Optional<User> findByEmail(String email);


You can add @EntityGraph(attributePaths = "roles") to avoid declare Eager fetch type in your entity

Elena-Bruyako · 2023-10-27T07:44:35Z

src/main/java/com/example/springbootbookshop/security/JwtAuthenticationFilter.java

+    }
+
+    private static String getToken(HttpServletRequest request) {
+        String bearerToken = request.getHeader("Authorization");


authorizationHeader will be more correct by meaning

Elena-Bruyako · 2023-10-27T07:49:13Z

src/main/java/com/example/springbootbookshop/service/impl/UserServiceImpl.java

+        }
+        User user = userMapper.toUserModel(requestDto);
+        user.setPassword(passwordEncoder.encode(requestDto.password()));
+


remove empty line

Elena-Bruyako

Good job! Let's improve your solution!

Elena-Bruyako · 2023-10-30T10:24:35Z

pom.xml

+        <dependency>
+            <groupId>org.liquibase</groupId>
+            <artifactId>liquibase-core</artifactId>
+            <version>${liquibase.version}</version>


Remove version, spring already provides it

Elena-Bruyako · 2023-10-30T10:24:46Z

pom.xml

+        </dependency>
+        <dependency>
+            <groupId>org.liquibase</groupId>
+            <artifactId>liquibase-maven-plugin</artifactId>


replace to plugin section

Elena-Bruyako · 2023-10-30T10:26:57Z

src/main/java/com/example/springbootbookshop/entity/Role.java

+@Data
+@Entity
+@Table(name = "roles")
+@RequiredArgsConstructor


Remove RequiredArgsConstructor

Elena-Bruyako · 2023-10-30T10:27:41Z

src/main/java/com/example/springbootbookshop/mapper/UserMapper.java

+public interface UserMapper {
+    UserDto toDto(User user);
+
+    User toUserModel(UserRegistrationRequestDto userDto);


Elena-Bruyako · 2023-10-30T10:27:48Z

src/main/java/com/example/springbootbookshop/mapper/UserMapper.java

+        nullValueCheckStrategy = NullValueCheckStrategy.ALWAYS,
+        implementationPackage = "<PACKAGE_NAME>.impl")
+public interface UserMapper {
+    UserDto toDto(User user);


Suggested change

UserDto toDto(User user);

UserDto toUserDto(User user);

Elena-Bruyako · 2023-10-30T10:31:28Z

src/main/java/com/example/springbootbookshop/security/JwtAuthenticationFilter.java

+    }
+
+    private static String getToken(HttpServletRequest request) {
+        String authorizationHeader = request.getHeader("Authorization");


Optional
there is an existing constant: org.springframework.http.HttpHeaders.AUTHORIZATION

Elena-Bruyako · 2023-10-30T10:32:10Z

src/main/java/com/example/springbootbookshop/entity/User.java

+            name = "users_roles",
+            joinColumns = @JoinColumn(name = "user_id"),
+            inverseJoinColumns = @JoinColumn(name = "role_id")
+


remove empty line

liliia-ponomarenko

Good job! Let’s fix a few mistakes ;)

liliia-ponomarenko · 2023-10-30T16:57:37Z

src/main/java/com/example/springbootbookshop/annotation/FieldMatch.java

+    String message() default "Passwords do not match";
+    Class<?>[] groups() default { };
+    Class<? extends Payload>[] payload() default { };


Suggested change

String message() default "Passwords do not match";

Class<?>[] groups() default { };

Class<? extends Payload>[] payload() default { };

String message() default "Passwords do not match";

Class<?>[] groups() default { };

Class<? extends Payload>[] payload() default { };

liliia-ponomarenko · 2023-10-30T16:58:03Z

src/main/java/com/example/springbootbookshop/annotation/PasswordMatchValidator.java

+        UserRegistrationRequestDto> {
+
+    @Override
+    public boolean isValid(UserRegistrationRequestDto dto,


you can add not null validation here

liliia-ponomarenko · 2023-10-30T16:58:21Z

src/main/java/com/example/springbootbookshop/config/SecurityConfig.java

+                                .anyRequest()
+                                .authenticated()
+                )
+                .httpBasic(Customizer.withDefaults())


why do you need basic auth if we're using JWT?

I need just remove it?

Correct, you don't need it here

liliia-ponomarenko · 2023-10-30T16:59:00Z

src/main/java/com/example/springbootbookshop/controller/AuthenticationController.java

+
+@RequiredArgsConstructor
+@RestController
+@Validated


do you really need this annotation here?

Yes,how it works without it?

In such case you don't need Validated annotation.

liliia-ponomarenko · 2023-10-30T16:59:57Z

src/main/java/com/example/springbootbookshop/dto/CreateBookRequestDto.java

 import jakarta.validation.constraints.Positive;
 import java.math.BigDecimal;

 public record CreateBookRequestDto(@NotBlank String title,
                                   @NotBlank String author,
                                   @NotBlank String isbn,
-                                   @NotNull @Positive BigDecimal price,
+                                   @NotBlank @Positive BigDecimal price,


NotNull correct here and better use PositiveOrZero

liliia-ponomarenko · 2023-10-30T17:13:03Z

src/main/java/com/example/springbootbookshop/service/impl/UserServiceImpl.java

+    public UserDto register(UserRegistrationRequestDto requestDto)
+            throws RegistrationException {
+        if (userRepository.findByEmail(requestDto.email()).isPresent()) {
+            throw new RegistrationException("User with email: " + requestDto.email()


Should you handle this exception? Maybe something else?

Create another exception?what`s wrong with this one?

liliia-ponomarenko · 2023-10-30T17:13:52Z

src/main/java/com/example/springbootbookshop/service/impl/UserServiceImpl.java

+    @Override
+    public UserDto register(UserRegistrationRequestDto requestDto)
+            throws RegistrationException {
+        if (userRepository.findByEmail(requestDto.email()).isPresent()) {


you can use existByEmail from JPA

liliia-ponomarenko · 2023-10-30T17:14:21Z

src/main/resources/db/changelog/changes/02-create-users-table.yaml

+                    nullable: false
+              - column:
+                  name: shipping_address
+                  type: VARCHAR(255)


you must have the same maximum size in the query, as it may cause a SQL exception, or use a different type (check it everywhere)

liliia-ponomarenko · 2023-10-30T17:16:01Z

src/main/resources/db/changelog/changes/02-create-users-table.yaml

+                  type: boolean
+                  defaultValueBoolean: false
+                  constraints:
+                    nullable: false


add one empty line (check it in other files)

liliia-ponomarenko · 2023-10-30T17:17:56Z

src/main/resources/db/changelog/db.changelog-master.yaml

+  - include:
+      file: db\changelog\changes\03-create-roles-table.yaml
+  - include:
+      file: db/changelog/changes/04-create-users-roles.yaml


Why different slashs? looks weird

Elena-Bruyako

Good job! Pay attention to comments that are not fixed

Elena-Bruyako · 2023-10-31T10:19:54Z

src/main/java/com/example/springbootbookshop/config/SecurityConfig.java

+                                .anyRequest()
+                                .authenticated()
+                )
+                .httpBasic(Customizer.withDefaults())


Correct, you don't need it here

Elena-Bruyako · 2023-10-31T10:23:56Z

src/main/java/com/example/springbootbookshop/controller/AuthenticationController.java

+
+@RequiredArgsConstructor
+@RestController
+@Validated


In such case you don't need Validated annotation.

Elena-Bruyako · 2023-10-31T10:25:09Z

src/main/java/com/example/springbootbookshop/dto/user/UserDto.java

@@ -0,0 +1,8 @@
+package com.example.springbootbookshop.dto.user;
+
+public record UserDto(Long id,


You can remove this class and use UserRegisterResponseDto instead

Elena-Bruyako · 2023-10-31T10:25:50Z

src/main/java/com/example/springbootbookshop/entity/Role.java

+    public String getAuthority() {
+        return "ROLE_" + name.name();
+    }
+


Elena-Bruyako · 2023-10-31T10:25:58Z

src/main/java/com/example/springbootbookshop/entity/RoleName.java

+
+public enum RoleName {
+    ADMIN,
+    MANAGER,


Elena-Bruyako · 2023-10-31T10:26:22Z

src/main/java/com/example/springbootbookshop/entity/User.java

+    @Column(nullable = false)
+    private String lastName;
+    private String shippingAddress;
+


Elena-Bruyako · 2023-10-31T10:26:30Z

src/main/java/com/example/springbootbookshop/entity/User.java

+
+    @Override
+    public boolean isEnabled() {
+        return true;


Elena-Bruyako · 2023-10-31T10:27:01Z

src/main/java/com/example/springbootbookshop/security/CustomUserDetailsService.java

+import org.springframework.security.core.userdetails.UsernameNotFoundException;
+import org.springframework.stereotype.Component;
+
+@Component


Elena-Bruyako · 2023-10-31T10:27:17Z

src/main/java/com/example/springbootbookshop/security/CustomUserDetailsService.java

+    @Override
+    public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException {
+        return userRepository.findByEmail(username).orElseThrow(() ->
+                new EntityNotFoundException("User doesn`t exist"));


Elena-Bruyako

Need to fix some comments

Elena-Bruyako · 2023-10-31T11:35:32Z

src/main/java/com/example/springbootbookshop/controller/AuthenticationController.java

+
+@RequiredArgsConstructor
+@RestController
+@Validated


Elena-Bruyako · 2023-10-31T11:36:31Z

src/main/java/com/example/springbootbookshop/security/CustomUserDetailsService.java

+import org.springframework.security.core.userdetails.UsernameNotFoundException;
+import org.springframework.stereotype.Component;
+
+@Component


Elena-Bruyako · 2023-10-31T11:38:11Z

src/main/java/com/example/springbootbookshop/security/CustomUserDetailsService.java

+    @Override
+    public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException {
+        return userRepository.findByEmail(username).orElseThrow(() ->
+                new EntityNotFoundException("User doesn`t exist"));


This method throw UsernameNotFoundException so you need to
userRepository.findByEmail(username).orElseThrow(() ->
new UsernameNotFoundException("User doesn`t exist"))

Elena-Bruyako · 2023-10-31T11:40:56Z

src/main/java/com/example/springbootbookshop/exception/RegistrationException.java

@@ -0,0 +1,11 @@
+package com.example.springbootbookshop.exception;
+
+public class RegistrationException extends RuntimeException {


Do you handle this exception ?

Elena-Bruyako · 2023-10-31T11:42:30Z

src/main/java/com/example/springbootbookshop/dto/user/UserDto.java

@@ -0,0 +1,8 @@
+package com.example.springbootbookshop.dto.user;
+
+public record UserDto(Long id,


Suggested change

public record UserDto(Long id,

public record UserResponseDto(Long id,

liliia-ponomarenko

Good job!

liliia-ponomarenko · 2023-10-31T15:30:22Z

src/main/java/com/example/springbootbookshop/mapper/UserMapper.java

+public interface UserMapper {
+    UserResponseDto toUserDto(User user);
+
+    User toUserModel(UserRegistrationRequestDto userDto);


Suggested change

User toUserModel(UserRegistrationRequestDto userDto);

User toUser(UserRegistrationRequestDto userDto);

don-bigdad added 6 commits October 25, 2023 14:29

test code

bdee66b

mvn fix

f4af5d0

mvn fix

a0fd9e1

login problem

fc9e697

login problem

c15ec0e

done

4abb214

anigilator479 approved these changes Oct 26, 2023

View reviewed changes

Elena-Bruyako suggested changes Oct 27, 2023

View reviewed changes

don-bigdad added 2 commits October 29, 2023 17:54

remove eager fetch type and some minor fixes

79d235b

minor add to controller

2cd5bbf

Elena-Bruyako suggested changes Oct 30, 2023

View reviewed changes

minor fixes in pom.xml and some checkstyle fix

c0a4119

liliia-ponomarenko suggested changes Oct 30, 2023

View reviewed changes

don-bigdad added 4 commits October 30, 2023 22:22

some fixes

8a9295e

add uncommitted

d94c9c0

add uncommitted

32a16db

add uncommitted

77ea926

Elena-Bruyako suggested changes Oct 31, 2023

View reviewed changes

fix some logic mistakes

4b9e577

Elena-Bruyako suggested changes Oct 31, 2023

View reviewed changes

change annotation name and make exception checked

52fc9fc

liliia-ponomarenko approved these changes Oct 31, 2023

View reviewed changes

name fix

cf4f797

don-bigdad merged commit 4fe75e9 into master Oct 31, 2023
3 checks passed

don-bigdad deleted the security-dev branch October 31, 2023 20:18

	@FieldMatch
	@FieldMatch(fields = {"password", "repeatPassword"})


		@Repository
		public interface RoleRepository extends JpaRepository<Role, Long> {


		@Repository
		public interface UserRepository extends JpaRepository<User, Long> {

		@@ -0,0 +1,8 @@
		package com.example.springbootbookshop.dto.user;

		public record UserDto(Long id,

		@@ -0,0 +1,11 @@
		package com.example.springbootbookshop.exception;

		public class RegistrationException extends RuntimeException {

	public record UserDto(Long id,
	public record UserResponseDto(Long id,

	User toUserModel(UserRegistrationRequestDto userDto);
	User toUser(UserRegistrationRequestDto userDto);

test code #6

test code #6

Conversation

don-bigdad commented Oct 25, 2023

Choose a reason for hiding this comment

Elena-Bruyako left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Elena-Bruyako left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liliia-ponomarenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Elena-Bruyako left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Elena-Bruyako left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liliia-ponomarenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment