c24elipe commented

Collaborator

Implementation of password-hashing

On what device?

This has been developed on an Arch-based Linux distro, and tested using dotnet tests and bruno

What has been done?

Created all of "service/loginService/loginService/Util/PasswordHasher.cs" as the main method for password hashing
Created unit tests in "service/loginService/loginService.Tests/Util/PasswordHasherTests.cs."
Added login service to "entryPoint/nginx.conf":
- This was done so I could get the correct URLS to work (note 127.0.0.1 is the same as localhost):
  - 127.0.0.1/api/login/
  - 127.0.0.1/api/login/auth/SignUp
  - 127.0.0.1/api/login/auth/login
Updated the table in "DockerCompose/init_sql/02-users.sql" so the new password selection could fit.
In "DockerCompose/compose.yaml", I added so the entryPoint-container is dependent on loginService and frontend. Maybe unnecessary on other devices; it just happened to be the only way for me to wrangle Docker to work.
Rewrote some basic logic in "frontend/nginx.conf"; it basically works the same as before, but is a bit more explicit to force the frontend to work with us.
"service/loginService/loginService/DatabaseLogic/DatabaseQueries.cs"; I updated "ValidateLogin" to instead be "GetUser" and made some basic logic changes + code clean up.
"service/loginService/loginService/Endpoints/AuthEndpoints.cs"; I added it so that the correct codes are given with the results.
Lastly, "service/loginService/loginService/Models/Db rows/UserRow.cs" was created to be an object of users from the database.

Good to know:

127.0.0.1/api/login/, 127.0.0.1/api/login/auth/SignUp, 127.0.0.1/api/login/auth/login are all POST and not GET endpoints, meaning you can't open them using a standard browser.

Issue:

#690

# Implementation of password-hashing ## On what device? This has been developed on an Arch-based Linux distro, and tested using dotnet tests and [bruno](http://usebruno.com/) ## What has been done? * Created all of "**service/loginService/loginService/Util/PasswordHasher.cs**" as the main method for password hashing * Created unit tests in "**service/loginService/loginService.Tests/Util/PasswordHasherTests.cs**." * Added login service to "**entryPoint/nginx.conf**": * This was done so I could get the correct URLS to work (**note** 127.0.0.1 is the same as localhost): * _127.0.0.1/api/login/_ * _127.0.0.1/api/login/auth/SignUp_ * _127.0.0.1/api/login/auth/login_ * Updated the table in "**DockerCompose/init_sql/02-users.sql**" so the new password selection could fit. * In "**DockerCompose/compose.yaml**", I added so the entryPoint-container is dependent on loginService and frontend. Maybe unnecessary on other devices; it just happened to be the only way for me to wrangle Docker to work. * Rewrote some basic logic in "**frontend/nginx.conf**"; it basically works the same as before, but is a bit more explicit to force the frontend to work with us. * "**service/loginService/loginService/DatabaseLogic/DatabaseQueries.cs**"; I updated "ValidateLogin" to instead be "GetUser" and made some basic logic changes + code clean up. * "**service/loginService/loginService/Endpoints/AuthEndpoints.cs**"; I added it so that the correct codes are given with the results. * Lastly, "**service/loginService/loginService/Models/Db rows/UserRow.cs**" was created to be an object of users from the database. ## Good to know: _127.0.0.1/api/login/_, _127.0.0.1/api/login/auth/SignUp_, _127.0.0.1/api/login/auth/login_ are all **POST** and not **GET** endpoints, meaning you can't open them using a standard browser. ## Issue: #690

c24elipe added the

needs review

label

2026-05-05 00:06:30 +00:00

c24elipe added 8 commits

2026-05-05 00:06:31 +00:00

Added basic password hashing 3475c4fc7f

Added some tests and fixes ba371a2be2

Fixed code standard violations 19cb990956

Updated variable name in authservice 9cabb7317a

Another name fix in interface 43c13f4969

Added basic login logic 609ace9c6b

Fixed error with loginservice not beign reacable 6880e9964f

Did... a lot... 26070fc0da

1. Updated ComparePassword to instead be VerifyPassword

2. added logging

3. fixed some mutations that shouldn't have been added

4. Added error codes

5. Created UserRow  (the row from the database as a object)

6. Removed app.UseHttpsRedirection

7. Fixed some mapping issues with the service

c24elipe added 1 commit

2026-05-05 00:09:06 +00:00

Updated arguments in UnitTest1 cde68eb265

c24elipe added 1 commit

2026-05-05 00:37:02 +00:00

Added comments and stuff to follow the Conventions d3de3200b5

c24elipe added 1 commit

2026-05-05 00:40:45 +00:00

Added some missed things b3bd59e1bc

c24elipe referenced this pull request

2026-05-05 00:44:12 +00:00

Implement password hashing #690

a24vinla requested review from a24vinla

2026-05-05 06:46:42 +00:00

a24vinla commented

2026-05-05 08:39:52 +00:00

Collaborator

Review on #690

Whats done

Tested on Windows 11 using Chrome

App starts!
+ Ran new PasswordHasherTests, all passed

Inspected following files

loginService/Util/PasswordHasher.cs
- Indentation/code styling
  - Follows all standards set for C#
- Naming convention
  - Follows naming conventions and names are clear
- Comments
  - Very well commented, but comments on line 50 -> 51 I argue should be single line (//) comments
- General code quality
  - Good Quality!
loginService.Tests/Util/PasswordHasherTests.cs
- Indentation/code styling
  - Follows all standards set for C#/Testing
- Naming convention
  - Follows naming conventions and names are clear
- Comments
  - Very well commented
- General code quality
  - Good Quality!
entryPoint/nginx.conf
- Indentation/code styling
  - Looks the same as the rest, which is good!
DockerCompose/init_sql/02-users.sql
- Indentation/code styling
  - Not much to review here, correct change
DockerCompose/compose.yaml
- Built on my computer, should not be a problem
frontend/nginx.conf
- Looks good
loginService/DatabaseLogic/DatabaseQueries.cs
- Naming convention
  - Follows naming conventions
loginService/Endpoints/AuthEndpoints.cs
- Valid changes
loginService/Models/Db rows/UserRow.cs
- Naming convention
  - Db rows should be changed to DbRows/DBRows
- Looks good otherwise

What needs to be done

loginService/Models/Db rows
- Folder name should be changed
loginService/Util/PasswordHasher.cs line 50 -> 51
- I think it could be argued it should not be changed, I leave it to developer to decide

Conclusion

Well made PR, some parts might have to be revisited once login and user is actually implemented to the system since that will better illuminate possible faults. I can't find any such errors however.
It follows the decisions and research made regarding password hashing

# Review on #690 ## Whats done ### Tested on Windows 11 using Chrome + App starts! + Ran new PasswordHasherTests, all passed ### Inspected following files + loginService/Util/PasswordHasher.cs + Indentation/code styling + Follows all standards set for C# + Naming convention + Follows naming conventions and names are clear + Comments + Very well commented, but comments on line 50 -> 51 I argue should be single line (//) comments + General code quality + Good Quality! + loginService.Tests/Util/PasswordHasherTests.cs + Indentation/code styling + Follows all standards set for C#/Testing + Naming convention + Follows naming conventions and names are clear + Comments + Very well commented + General code quality + Good Quality! + entryPoint/nginx.conf + Indentation/code styling + Looks the same as the rest, which is good! + DockerCompose/init_sql/02-users.sql + Indentation/code styling + Not much to review here, correct change + DockerCompose/compose.yaml + Built on my computer, should not be a problem + frontend/nginx.conf + Looks good + loginService/DatabaseLogic/DatabaseQueries.cs + Naming convention + Follows naming conventions + loginService/Endpoints/AuthEndpoints.cs + Valid changes + loginService/Models/Db rows/UserRow.cs + Naming convention + Db rows should be changed to DbRows/DBRows + Looks good otherwise ## What needs to be done + loginService/Models/Db rows + Folder name should be changed + loginService/Util/PasswordHasher.cs line 50 -> 51 + I think it could be argued it should not be changed, I leave it to developer to decide ## Conclusion + Well made PR, some parts might have to be revisited once login and user is actually implemented to the system since that will better illuminate possible faults. I can't find any such errors however. + It follows the decisions and research made regarding password hashing

a24vinla added

reviewed

and removed

needs review

labels

2026-05-05 08:40:17 +00:00

a22erigr added 1 commit

2026-05-05 08:54:53 +00:00

Fixed issues noted in review and indentation 350f1ef733

- change multiline comment to single line
- renamed Db rows to DBRows

a22erigr commented

2026-05-05 09:00:25 +00:00

Collaborator

@c24elipe Can you verify that i did not forget to import the new name of model somewhere.

c24elipe added 1 commit

2026-05-05 09:04:10 +00:00

Fixed DbRows namespace 30d6fd20b8

c24elipe added 1 commit

2026-05-05 09:05:09 +00:00

Fixed comment in PasswordHasher 5891597008

c24elipe commented

2026-05-05 09:09:11 +00:00

Author

Collaborator

@a22erigr, I have now:

Fixed a minor spelling mistake in the PasswordHasher (as described in the review)
I have fixed some errors with some code using the old namespace of DbRows, so it should now be fine.

It seems to work for me now.

@a22erigr, I have now: 1. Fixed a minor spelling mistake in the PasswordHasher (as described in the review) 2. I have fixed some errors with some code using the old namespace of DbRows, so it should now be fine. It seems to work for me now. ![image](/attachments/eef4eea5-ab37-4194-9450-9acab45e4aa3) ![image](/attachments/42d92c17-faeb-4b1b-9df0-c899359c6df7)

image.png

64 KiB

image.png

57 KiB