#42 authentication login by PawelSzambelan · Pull Request #211 · MateuszNaKodach/CodersCamp2020.Project.FullStack-Node-React.TableSoccerTournaments

PawelSzambelan · 2021-05-24T21:47:30Z

implementation for part in yellow square

codecov-commenter · 2021-05-28T14:41:15Z

Codecov Report

Merging #211 (b4420c3) into develop (5f987e6) will decrease coverage by 0.61%.
The diff coverage is 45.00%.

@@             Coverage Diff             @@
##           develop     #211      +/-   ##
===========================================
- Coverage    83.31%   82.69%   -0.62%     
===========================================
  Files          224      231       +7     
  Lines         2661     2728      +67     
  Branches       245      232      -13     
===========================================
+ Hits          2217     2256      +39     
- Misses         444      472      +28

Flag	Coverage Δ
backend	`82.69% <45.00%> (-0.62%)`	⬇️
integrationtests	`82.69% <45.00%> (-0.62%)`	⬇️
unittests	`82.69% <45.00%> (-0.62%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...application/command/GenerateTokenCommandHandler.ts	`0.00% <0.00%> (ø)`
...e/application/command/SetPasswordCommandHandler.ts	`0.00% <0.00%> (ø)`
...authentication/core/domain/event/PasswordWasSet.ts	`33.33% <0.00%> (+33.33%)`	⬆️
...ication/core/domain/event/TokenGenerationFailed.ts	`0.00% <0.00%> (ø)`
...est-api/request/PostAuthenticateUserRequestBody.ts	`0.00% <0.00%> (ø)`
...ion/rest-api/request/PostSetPasswordRequestBody.ts	`0.00% <ø> (ø)`
...c/modules/user-accounts/core/domain/UserAccount.ts	`0.00% <0.00%> (ø)`
.../modules/authentication/core/domain/UserAccount.ts	`40.00% <35.00%> (+40.00%)`	⬆️
...authentication/core/domain/event/TokenGenerated.ts	`40.00% <40.00%> (ø)`
...tication/core/application/command/GenerateToken.ts	`100.00% <100.00%> (ø)`
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0839d1f...b4420c3. Read the comment docs.

PawelSzambelan · 2021-05-28T17:51:19Z

@nowakprojects byłbym wdzięczny jak rzucisz okiem :-) przyjadę znad morza to dokończę :-) + jak to poprawić / zaimplementować ?

MateuszNaKodach · 2021-05-29T11:49:45Z

@nowakprojects byłbym wdzięczny jak rzucisz okiem :-) przyjadę znad morza to dokończę :-) + jak to poprawić / zaimplementować ?

Jak uwazasz. Eventy sie zwracaja, nie? No to mozesz wtedy jakos przeanalizowac zwrocone events i zwrocic Command.failed jesli taki event poszedl. Plusem takich eventów, nawet przy błedzie może byc wlasnie np. zablokowanie konta po 3 takich eventach.

MateuszNaKodach · 2021-05-29T11:50:12Z

backend/.env

 REST_API_PORT=5000
 API_DOCS_ENDPOINT_URL=/rest-api-docs
 GOOGLE_CLIENT_ID=1052788207529-fjnskiqrsm09i9kbujp3gmtasnp21su7.apps.googleusercontent.com
+JWT_SECRET_KEY=MOVE_IT_TO_GITHUB_SECRETS


Jaka to ma byc wartosc :) ? Napisz mi gdzies na Slacku to dodam.

chyba obojętne, potrzebuję tylko wiedzieć pod jaką nazwą to zapisałeś, bo rozumiem, że jak w github secrets zapiszesz to pod nazwą JWT_SECRET_KEY to wtedy w pliku backend.yml na dole pliku w env: pod HD_MONGO_CONNECTION_STRING dajesz w kolejnej lini
JWT_SECRET_KEY: ${{secrets.JWT_SECRET_KEY}}
i potem bezpośrednio w kodzie w pliku .ts możesz się do tego odwołać jako process.env.JWT_SECRET_KEY? Czy jednak coś źle rozumiem?

tak samo chyba by się przydało zrobić z GOOGLE_CLIENT_ID, hmmm?

Chyba musi byc HD_ najpierw, dla Heroku.
To napisz mi jakie klucz - wartosc maja byc :) Bo nie mamy wartosci tutaj.

MateuszNaKodach · 2021-05-29T11:52:28Z

backend/src/modules/authentication/core/application/command/SetPasswordCommandHandler.ts

  async execute(command: SetPassword): Promise<CommandResult> {
-    const userAccount = await this.repository.findByEmail(command.email);
-    const { state, events } = setPasswordForUserAccount(userAccount, command, this.currentTimeProvider());
+    const userAccount = await this.repository.findById(command.userId);


Czy kazdy user moze kazdemu ustawic haslo? Tutaj sie przyda sprawdzenie wlasnie aktualnego usera. Ale to moze byc kolejny PR :)

czyli, że w metodzie setPasswordForUserAccount by się przydało jeszcze sprawdzenie np. tokena? Jeśli tak to kumam o co biega, ale nie mam zielonego pojęcia jak to zrobić .... to by musiało iść przez ten middleware do sprawdzania poprawności tokena, a to trzeba osobno skodzić, Si?

hmmm... no to zalezy :P Najprosciej mozna to zrobic w metodzie execute, ale niezbyt to reuzywalne. Middleware wydaje sie lepszy. Chyba byly na kursach jakies takie rzeczy, poszukaj jak zrobic autoryzacje poprzez middleware. Ta autoryzacja mialaby tak dzialac, ze tylko user moze zmienic haslo dla samego siebie.

MateuszNaKodach · 2021-05-29T11:52:47Z

backend/src/modules/authentication/core/domain/UserAccount.ts

-  readonly email: string;
-  readonly password: string;
+  readonly userId: string;
+  readonly email: string | undefined;


Może być bez emaila?

hmmm na pewno po coś to zrobiłem, ale nie pamiętam po co.... tak czy inaczej chyba faktycznie nie jest to potrzebne. Rozumiem, że password pozostaje jako string | undefined, bo repo użytkowników będzie jedno, niezależnie w jaki sposób się będą logować / rejestrować, Si? ...więc w przypadku logowania za pomocą googla hasło będzie nam nieznane.

Jak dla mnie powinny byc dwa modele usera - jeden dla naszej authentication drugi dla Google.
Zeby nie bylo wlasnie czegos takiego jak password: string | undefined.
Chyba to tlumaczylem dosc dlugo pod koniec projektu :P

Teraz mamy schizofrenie - jestem przez Google czy nie? Nie wiemy tego po typach.

MateuszNaKodach · 2021-05-29T11:54:54Z

backend/src/modules/authentication/core/domain/UserAccount.ts

+    throw new Error('Wrong password.');
+  }
+
+  const token: string = jwt.sign({ email: command.email, userId: state.userId }, `${process.env.JWT_SECRET_KEY}`, { expiresIn: '1h' });


Tutaj mamy 2 rzeczy, ktore powinny byc w warstwie infrastruktury - hashowanie / porownywanie hasla i jwt.
Przydalyby sie interfejsy Np. TokenGenerator i jakis PasswordEncpryptor.

Bo teraz nawet wlasnie nie masz jak przetestowac tez. A w tescie np. zamockujesz, ze taki PasswordEncryptor zwrocil np. ze hasla sie nie zgadzaja. I jestes w stanie podstawic cos pod jwt. A tak to tez jest losowe.

Domena nie powinna zalezec od takich rzeczy, jak i odczytywac w sobie process.env. To takie pure function powinny byc:

https://enterprisecraftsmanship.com/posts/what-is-functional-programming/

https://enterprisecraftsmanship.com/posts/domain-model-purity-current-time/

https://enterprisecraftsmanship.com/posts/domain-model-purity-completeness/

Dlatwego zapewne nie ma testów na ten Command :)

@nowakprojects to miałeś na myśli?
jeśli tak, to w testach, tak jak napisałeś, zamockować wyniki tych metod z interfejsów i posprawdzać różne warunki i będzie ok, tak?
f327b9e

jakieś masz może pomysł na jakieś lepsze nazwy interfejsów :-P i gdzie dać ich implementacje?

backend/src/modules/authentication/core/domain/UserAccount.ts

MateuszNaKodach · 2021-05-29T11:58:38Z

backend/src/modules/authentication/presentation/rest-api/AuthenticationRouter.ts

+    const { email, password } = requestBody;
+    const commandResult = await commandPublisher.execute(new GenerateToken(email, password));
+    return commandResult.process(
+      (token: string) => response.status(StatusCodes.OK).json({ token: token }).send(),


Pieknie! :)

backend/src/modules/authentication/core/domain/UserAccount.ts

MateuszNaKodach

Super robota Paweł! :) Kilka rzeczy do ogarnięcia.

MateuszNaKodach · 2021-08-06T19:31:41Z

backend/src/modules/authentication/core/application/command/SetPasswordCommandHandler.ts

 import { setPasswordForUserAccount } from '../../domain/UserAccount';
+import bcrypt from 'bcrypt';

 export class SetPasswordCommandHandler implements CommandHandler<SetPassword> {


Aaa, w ogole kiedy to bedzie uzywane, mi przypomnij :) ? Jaki jest use case tego?

eee jeszcze nie, ale pewnie będzie ^_^, chyba fajnie mieć możliwość zmiany hasło :-P

było w miro i to zaczęła Ania wgl robić, a ja tam trochę grzebię

PawelSzambelan added 5 commits May 24, 2021 23:43

#42 internal authentification

f031b22

#42 vision of user account module

060bfa0

#42 add token in test case(but it still doesn't work)

17ffef4

#42 update with miro

89e9b6b

#42 fix for test with token

ddb4edd

#42 renaming duo to Miro

9f76ac2

PawelSzambelan marked this pull request as ready for review May 28, 2021 14:47

PawelSzambelan requested a review from MateuszNaKodach May 28, 2021 14:47

PawelSzambelan added 2 commits May 28, 2021 19:48

#42 add in memory implementation with tests (tests to fix)

0160876

#42 add TODOS

859915b

MateuszNaKodach reviewed May 29, 2021

View reviewed changes

backend/src/modules/authentication/core/domain/UserAccount.ts Outdated Show resolved Hide resolved

MateuszNaKodach reviewed May 29, 2021

View reviewed changes

backend/src/modules/authentication/core/domain/UserAccount.ts Show resolved Hide resolved

MateuszNaKodach reviewed May 29, 2021

View reviewed changes

backend/src/modules/authentication/core/domain/UserAccount.ts Outdated Show resolved Hide resolved

MateuszNaKodach requested changes May 29, 2021

View reviewed changes

PawelSzambelan added 3 commits August 6, 2021 12:33

old update for readme ...

e71256c

#42 pr change - remove undefined from email

b322d6c

#42 remove unused import

1540c66

MateuszNaKodach reviewed Aug 6, 2021

View reviewed changes

PawelSzambelan added 4 commits August 6, 2021 21:45

#42 fix for tests

b4420c3

#42 changes in env's variables

ce6d32c

#42 change userId, password, email to Value Objects

d257c4e

#42 fixes after previous commit - variable formats

a7540c2

PawelSzambelan added 3 commits August 11, 2021 14:08

#42 CR fix

de98243

#42 CR - create interfaces for token and password

f327b9e

#42 build fix

896f553

Conversation

PawelSzambelan commented May 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented May 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

PawelSzambelan commented May 28, 2021

Uh oh!

MateuszNaKodach commented May 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MateuszNaKodach left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

PawelSzambelan commented May 24, 2021 •

edited

Loading

codecov-commenter commented May 28, 2021 •

edited

Loading

MateuszNaKodach commented May 29, 2021 •

edited

Loading