Implemented user authentication #32

Alekseeybg · 2023-09-27T12:42:49Z

No description provided.

…enerate jwt tokens. Authenticate users. Added request/response DTOs. Refactored existing code

…01 Unauthorized

…dden to 401 Unauthorized handling logic

…TestBase and added Authorization headers with valid token to every request.

…ar. Fixed security issue with secretKey

…Changed how we secure the secretKey. Configured tests so they work again.

… application.properties file for simplicity

[feature-22] Resource availability by resource and total resource quantity. test -> main

…validation/exception handling. Custom exceptions thrown with custom error messages showing correctly.

…on and unit tests.

… created/thrown/handled. Validation to UserRequestDto.

… refactor. Added more exceptions

…r refactor

…problem

VladoKat

The logic for validating the jwt token seems more complex than I thought it would be. We have a lot of exception handling that I am not sure when we are supposed to hit. If we have cases in which these scenarios could happen, then I suggest creating tests for them. If not, we should consider simplifying the code. (I am not saying I know how but would love to dive deeper and try to understand it)

pom.xml

src/main/java/jewellery/inventory/dto/request/UserRequestDto.java

src/main/java/jewellery/inventory/exception/GlobalExceptionHandler.java

src/main/java/jewellery/inventory/exception/not_found/UserNotFoundException.java

src/main/java/jewellery/inventory/service/AuthService.java

src/test/java/jewellery/inventory/integration/AuthIntegrationTest.java

VladoKat · 2023-09-27T20:44:38Z

src/test/java/jewellery/inventory/integration/AuthIntegrationTest.java

+  void testInvalidUrlReturnsUnauthorized() {
+    ResponseEntity<Void> response =
+        testRestTemplate.exchange("/invalid-url", HttpMethod.GET, null, Void.class);
+    assertEquals(HttpStatus.UNAUTHORIZED, response.getStatusCode());


I don't think we need this test case. We are supposed to return 401 for every request that is not authorized (except for the post to /auth/token). If a request is authorized but the url is not valid, then we should return 404. Is this not the case currently?

Removed it. The idea came from a colleague of mine who told me what integration tests they write for their project.. And with spring security I do not know how to make it so it returns 404 Not found instead of unauthorized for not existing endpoints... I think the default behavior is to return always unauthorized. I just tested - tried to hit not existing endpoint and provided valid token in the headers - it returned 401 unauthorized.

hm, that's interesting. I wonder why...

Probably because we override the default behavior and tell it to return 401 instead of 403 🗡️

src/test/java/jewellery/inventory/unit/service/security/JwtTokenServiceTest.java

…Needs more tests.

…permissions for USER and ADMIN

…ass. Added 1 test and small adjustments.

…rage

src/main/java/jewellery/inventory/security/SecurityConfig.java

src/test/java/jewellery/inventory/unit/service/security/AuthServiceTest.java

VladoKat · 2023-10-03T10:52:05Z

src/test/java/jewellery/inventory/unit/service/security/JwtTokenServiceTest.java

+
+  private String generateExpiredToken() {
+    Date expiredDate =
+        new Date(System.currentTimeMillis() - (1000 * 60 * 60 * 24)); // 1 day in the past


Please, remove the comment. You can, instead, extract the complex calculation (1000 * 60 * 60 * 24) in a variable named dayInMillis.

I see you ended up not extracting dayInMillis = (1000 * 60 * 60 * 24). Any reason why not?

…ion tests failing after changes. Refactored services related to security to be in their package in /service package.

VladoKat

Apart from the comments, here, please change passwords to be char arrays instead of Strings. Let me know if it turns out to be too much of a hassle.

src/test/java/jewellery/inventory/unit/service/security/AuthServiceTest.java

src/test/java/jewellery/inventory/integration/AuthIntegrationTest.java

VladoKat

Looks good!

…ons and authentication method instructions

sonarcloud · 2023-10-05T11:56:36Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

89.3% Coverage
0.0% Duplication

Alekseeybg and others added 19 commits September 13, 2023 15:56

[Feature-19-implement-security][WIP] Implemented spring security 6. G…

9c7e99e

…enerate jwt tokens. Authenticate users. Added request/response DTOs. Refactored existing code

[Feature-19-implement-security][WIP] Changed error 403 Forbidden to 4…

0ab3537

…01 Unauthorized

[Feature-19-implement-security][WIP] More appropriate error 403 Forbi…

8130a0f

…dden to 401 Unauthorized handling logic

[Feature-19-implement-security][WIP] Created AuthenticatedIntegration…

f18c9d7

…TestBase and added Authorization headers with valid token to every request.

[Feature-19-implement-security][WIP] Fixed code smells pointed by son…

4c03d7b

…ar. Fixed security issue with secretKey

[Feature-19-implement-security][WIP] Reverted fix of one code smell. …

1a4ce5d

…Changed how we secure the secretKey. Configured tests so they work again.

[Feature-19-implement-security][WIP] Hardcoded test secretKey in test…

f6f3d64

… application.properties file for simplicity

Merge pull request #31 from JewelleryManagement/test

582fdee

[feature-22] Resource availability by resource and total resource quantity. test -> main

[Feature-19-implement-security][WIP] Refactored parts of code. Added …

35fb3b0

…validation/exception handling. Custom exceptions thrown with custom error messages showing correctly.

[Feature-19-implement-security][WIP] Refactored code. Added integrati…

8403f02

…on and unit tests.

[Feature-19-implement-security][WIP] Refactored code. More exceptions…

44defb7

… created/thrown/handled. Validation to UserRequestDto.

[Feature-19-implement-security][WIP] Fixed (tried) code smells. Small…

55107b3

… refactor. Added more exceptions

Merge branch 'main' into feature-19-user-authentication

cb717f4

[Feature-19-implement-security][WIP] Fixed failing tests to work afte…

abc4152

…r refactor

[Feature-19-implement-security][WIP] Fixed failing integration test

fe4e610

[Feature-19-implement-security][WIP] Fixed custom exception messages …

a46b54f

…problem

[Feature-19-implement-security][WIP] Fixed final code smells

4863da9

[Feature-19-implement-security][WIP] Added more unit tests

d564bfc

[Feature-19-implement-security][WIP] Removed code smells

0d1a822

VladoKat requested changes Sep 27, 2023

View reviewed changes

Alekseeybg added 10 commits September 28, 2023 18:46

[Feature-19-implement-security][WIP] Fixes according to code review. …

c3e8b98

…Needs more tests.

[Feature-19-implement-security][WIP] fix .gitignore Changed logic to …

84c8b7b

…permissions for USER and ADMIN

[Feature-19-implement-security][WIP] fix .gitignore

01da5c3

[Feature-19-implement-security][WIP] fix .gitignore

9f0e5dd

[Feature-19-implement-security][WIP] small refactor to AuthService cl…

3e15dfa

…ass. Added 1 test and small adjustments.

[Feature-19-implement-security][WIP] Added more tests for higher cove…

dabb227

…rage

[Feature-19-implement-security][WIP] Test coverage

8c5d5d6

[Feature-19-implement-security][WIP] Refactored code. Added tests

b9a4f77

[Feature-19-implement-security][WIP] More tests

ba21727

[Feature-19-implement-security][WIP] More tests and refactor

829f88a

Alekseeybg added 3 commits October 2, 2023 15:46

[Feature-19-implement-security][WIP] More tests.

2358404

[Feature-19-implement-security][WIP] More tests.

2b0c390

[Feature-19-implement-security][WIP] Fix CORS problem

448bd5c

VladoKat requested changes Oct 3, 2023

View reviewed changes

[Feature-19-implement-security][WIP] Fix CORS problem. Fixed integrat…

8b4436d

…ion tests failing after changes. Refactored services related to security to be in their package in /service package.

VladoKat requested changes Oct 3, 2023

View reviewed changes

Alekseeybg added 2 commits October 4, 2023 19:07

[Feature-19-implement-security] Fixed review issues

512af28

[Feature-19-implement-security] Fixed last small issue.

aea23f4

VladoKat approved these changes Oct 4, 2023

View reviewed changes

[feature-19-user-authentication] update README with startup instructi…

6a7871d

…ons and authentication method instructions

VladoKat merged commit 687c066 into test Oct 5, 2023
4 checks passed

VladoKat deleted the feature-19-user-authentication branch October 5, 2023 14:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented user authentication #32

Implemented user authentication #32

Alekseeybg commented Sep 27, 2023

VladoKat left a comment

VladoKat Sep 27, 2023

Alekseeybg Sep 28, 2023

VladoKat Sep 28, 2023

Alekseeybg Oct 2, 2023

VladoKat Oct 3, 2023

Alekseeybg Oct 4, 2023

VladoKat Oct 4, 2023

VladoKat left a comment

VladoKat left a comment

sonarcloud bot commented Oct 5, 2023

Implemented user authentication #32

Implemented user authentication #32

Conversation

Alekseeybg commented Sep 27, 2023

VladoKat left a comment

Choose a reason for hiding this comment

VladoKat Sep 27, 2023

Choose a reason for hiding this comment

Alekseeybg Sep 28, 2023

Choose a reason for hiding this comment

VladoKat Sep 28, 2023

Choose a reason for hiding this comment

Alekseeybg Oct 2, 2023

Choose a reason for hiding this comment

VladoKat Oct 3, 2023

Choose a reason for hiding this comment

Alekseeybg Oct 4, 2023

Choose a reason for hiding this comment

VladoKat Oct 4, 2023

Choose a reason for hiding this comment

VladoKat left a comment

Choose a reason for hiding this comment

VladoKat left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Oct 5, 2023