Skip to content
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions backend/.env
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
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
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


MONGO_REPOSITORIES=DISABLED
#MONGO_REPOSITORIES=ENABLED
Expand Down
12,241 changes: 107 additions & 12,134 deletions backend/package-lock.json

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
},
"dependencies": {
"axios": "^0.21.1",
"bcrypt": "^5.0.0",
"bcrypt": "^5.0.1",
"body-parser": "^1.19.0",
"class-transformer": "^0.4.0",
"class-validator": "^0.13.1",
Expand Down Expand Up @@ -63,6 +63,7 @@
"@types/cors": "^2.8.10",
"@types/express": "^4.17.11",
"@types/jest": "^26.0.20",
"@types/jsonwebtoken": "^8.5.1",
"@types/mongoose": "^5.10.3",
"@types/node": "^14.14.22",
"@types/nodemailer": "^6.4.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,7 @@ import { UserAccount } from '../domain/UserAccount';
export interface AuthenticationRepository {
save(userAccount: UserAccount): Promise<void>;

findByEmail(email: string): Promise<UserAccount>;
findById(userId: string): Promise<UserAccount>;

findByEmail(email: string): Promise<UserAccount | undefined>;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export class GenerateToken {
readonly email: string;
readonly password: string;

constructor(email: string, password: string) {
this.email = email;
this.password = password;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { CommandHandler } from '../../../../../shared/core/application/command/CommandHandler';
import { DomainEventPublisher } from '../../../../../shared/core/application/event/DomainEventBus';
import { CurrentTimeProvider } from '../../../../../shared/core/CurrentTimeProvider';
import { AuthenticationRepository } from '../AuthenticationRepository';
import { CommandResult } from '../../../../../shared/core/application/command/CommandResult';
import { GenerateToken } from './GenerateToken';
import { authenticateUser } from '../../domain/UserAccount';

export class GenerateTokenCommandHandler implements CommandHandler<GenerateToken> {
constructor(
private readonly eventPublisher: DomainEventPublisher,
private readonly currentTimeProvider: CurrentTimeProvider,
private readonly repository: AuthenticationRepository,
) {}

async execute(command: GenerateToken): Promise<CommandResult> {
const userAccount = await this.repository.findByEmail(command.email);
const { state, events } = await authenticateUser(userAccount, command, this.currentTimeProvider());
this.eventPublisher.publishAll(events);
return CommandResult.success(state);
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
export class SetPassword {
readonly email: string;
readonly userId: string;
readonly password: string;

constructor(email: string, password: string) {
this.email = email;
constructor(userId: string, password: string) {
this.userId = userId;
this.password = password;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { CommandResult } from '../../../../../shared/core/application/command/Co
import { SetPassword } from './SetPassword';
import { AuthenticationRepository } from '../AuthenticationRepository';
import { setPasswordForUserAccount } from '../../domain/UserAccount';
import bcrypt from 'bcrypt';

export class SetPasswordCommandHandler implements CommandHandler<SetPassword> {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

constructor(
Expand All @@ -14,8 +15,8 @@ export class SetPasswordCommandHandler implements CommandHandler<SetPassword> {
) {}

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);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

const { state, events } = await setPasswordForUserAccount(userAccount, command, this.currentTimeProvider());
await this.repository.save(state);
this.eventPublisher.publishAll(events);
return CommandResult.success();
Expand Down
59 changes: 47 additions & 12 deletions backend/src/modules/authentication/core/domain/UserAccount.ts
Original file line number Diff line number Diff line change
@@ -1,29 +1,36 @@
import { DomainCommandResult } from '../../../../shared/core/domain/DomainCommandResult';
import { PasswordWasSet } from './event/PasswordWasSet';
import jwt from 'jsonwebtoken';
import bcrypt from 'bcrypt';
import { TokenGenerated } from './event/TokenGenerated';

export class UserAccount {
readonly email: string;
readonly password: string;
readonly userId: string;
readonly email: string | undefined;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Może być bez emaila?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

readonly password: string | undefined;

constructor(props: { email: string; password: string }) {
constructor(props: { userId: string; email: string | undefined; password: string | undefined }) {
this.userId = props.userId;
this.email = props.email;
this.password = props.password;
}
}

export function setPasswordForUserAccount(
export async function setPasswordForUserAccount(
state: UserAccount | undefined,
command: { email: string; password: string },
command: { userId: string; password: string },
currentTime: Date,
): DomainCommandResult<UserAccount> {
if (state) {
throw new Error('Account with this email address already exists.');
): Promise<DomainCommandResult<UserAccount>> {
if (!state) {
throw new Error('Account with this id does not exists.');
}

const hashedPassword = await bcrypt.hash(command.password, 12);

const passwordWasSet = new PasswordWasSet({
occurredAt: currentTime,
email: command.email,
password: command.password,
userId: command.userId,
password: hashedPassword,
});

const accountWithPasswordSet = onPasswordWasSet(state, passwordWasSet);
Expand All @@ -34,9 +41,37 @@ export function setPasswordForUserAccount(
};
}

function onPasswordWasSet(state: UserAccount | undefined, event: PasswordWasSet): UserAccount {
function onPasswordWasSet(state: UserAccount, event: PasswordWasSet): UserAccount {
return new UserAccount({
email: event.email,
userId: event.userId,
email: state.email,
password: event.password,
});
}

export async function authenticateUser(
state: UserAccount | undefined,
command: { email: string; password: string },
currentTime: Date,
): Promise<DomainCommandResult<string | undefined>> {
if (!state) {
throw new Error('Such email address does not exists.');
}

const isPasswordCorrect = await bcrypt.compare(command.password, state.password!);
if (!isPasswordCorrect) {
throw new Error('Wrong password.');
}

const token: string = jwt.sign({ email: command.email, userId: state.userId }, `${process.env.JWT_SECRET_KEY}`, { expiresIn: '1h' });
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


const userAuthenticated: TokenGenerated = new TokenGenerated({
occurredAt: currentTime,
email: command.email,
});

return {
state: token,
events: [userAuthenticated],
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import { DomainEvent } from '../../../../../shared/domain/event/DomainEvent';

export class PasswordWasSet implements DomainEvent {
readonly occurredAt: Date;
readonly email: string;
readonly userId: string;
readonly password: string;

constructor(props: { occurredAt: Date; email: string; password: string }) {
constructor(props: { occurredAt: Date; userId: string; password: string }) {
this.occurredAt = props.occurredAt;
this.email = props.email;
this.userId = props.userId;
this.password = props.password;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { DomainEvent } from '../../../../../shared/domain/event/DomainEvent';

export class TokenGenerated implements DomainEvent {
readonly occurredAt: Date;
readonly email: string;

constructor(props: { occurredAt: Date; email: string }) {
this.occurredAt = props.occurredAt;
this.email = props.email;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { DomainEvent } from '../../../../../shared/domain/event/DomainEvent';

export class TokenGenerationFailed implements DomainEvent {
readonly occurredAt: Date;
readonly email: string;

constructor(props: { occurredAt: Date; email: string }) {
this.occurredAt = props.occurredAt;
this.email = props.email;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { AuthenticationRepository } from '../../../core/application/AuthenticationRepository';
import { UserAccount } from '../../../core/domain/UserAccount';

export class InMemoryAuthenticationRepository implements AuthenticationRepository {
private readonly entities: { [id: string]: UserAccount } = {};

findByEmail(email: string): Promise<UserAccount | undefined> {
return Promise.resolve(Object.values(this.entities).find((userAccount) => userAccount.email === email));
}

findById(userId: string): Promise<UserAccount> {
return Promise.resolve(this.entities[userId]);
}

async save(userAccount: UserAccount): Promise<void> {
this.entities[userAccount.userId] = userAccount;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,32 @@ import express, { Request, Response } from 'express';
import { StatusCodes } from 'http-status-codes';
import { PostSetPasswordRequestBody } from './request/PostSetPasswordRequestBody';
import { SetPassword } from '../../core/application/command/SetPassword';
import { PostAuthenticateUserRequestBody } from './request/PostAuthenticateUserRequestBody';
import { GenerateToken } from '../../core/application/command/GenerateToken';

export function authenticationRouter(commandPublisher: CommandPublisher): express.Router {
const postSetPassword = async (request: Request, response: Response) => {
const requestBody: PostSetPasswordRequestBody = request.body;
const { email, password } = requestBody;
const commandResult = await commandPublisher.execute(new SetPassword(email, password));
const { userId, password } = requestBody;
const commandResult = await commandPublisher.execute(new SetPassword(userId, password));
return commandResult.process(
() => response.status(StatusCodes.OK).send(),
(failureReason) => response.status(StatusCodes.BAD_REQUEST).json({ message: failureReason.message }),
);
};

const authenticateUser = async (request: Request, response: Response) => {
const requestBody: PostAuthenticateUserRequestBody = request.body;
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(),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pieknie! :)

(failureReason) => response.status(StatusCodes.BAD_REQUEST).json({ message: failureReason.message }),
);
};

const router = express.Router();
router.post('/passwords', postSetPassword);
router.post('/token', authenticateUser);
return router;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export class PostAuthenticateUserRequestBody {
email: string;
password: string;
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export class PostSetPasswordRequestBody {
email: string;
userId: string;
password: string;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { UserAccount } from '../domain/UserAccount';

export interface UserAccountsRepository {
save(userAccount: UserAccount): Promise<void>;

findByEmail(email: string): Promise<UserAccount>;
}
9 changes: 9 additions & 0 deletions backend/src/modules/user-accounts/core/domain/UserAccount.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export class UserAccount {
readonly userId: string;
readonly email: string;

constructor(props: { userId: string; email: string }) {
this.userId = props.userId;
this.email = props.email;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import { AuthenticationRepository } from '../../../../../src/modules/authentication/core/application/AuthenticationRepository';
import { DatabaseTestSupport } from '../../../../test-support/shared/infrastructure/DatabaseTestSupport';
import { EntityIdGenerator } from '../../../../../src/shared/core/application/EntityIdGenerator';
import { UuidEntityIdGenerator } from '../../../../../src/shared/infrastructure/core/application/UuidEntityIdGenerator';
import { UserAccount } from '../../../../../src/modules/authentication/core/domain/UserAccount';

export function AuthenticationRepositoryTestCases(props: {
name: string;
repositoryFactory: () => AuthenticationRepository;
databaseTestSupport: DatabaseTestSupport;
}): void {
return describe(props.name, () => {
const entityIdGenerator: EntityIdGenerator = new UuidEntityIdGenerator();
let repository: AuthenticationRepository;

beforeAll(async () => {
await props.databaseTestSupport.openConnection();
repository = props.repositoryFactory();
});
afterEach(async () => await props.databaseTestSupport.clearDatabase());
afterAll(async () => await props.databaseTestSupport.closeConnection());

test('findById returns undefined when nothing was saved', async () => {
const testId = entityIdGenerator.generate();
expect(await repository.findById(testId)).toBeUndefined();
});

test('findByEmail returns undefined when nothing was saved', async () => {
const testEmail = 'testEmail';
expect(await repository.findByEmail(testEmail)).toBeUndefined();
});

test('findById returns userAccount when such id exists in database', async () => {
const testId = entityIdGenerator.generate();
const testEmail = 'testEmail';
const userAccount1 = new UserAccount({
userId: testId,
email: testEmail,
password: 'testPassword',
});
const userAccount2 = new UserAccount({
userId: 'id1',
email: 'email1',
password: undefined,
});

await repository.save(userAccount1);
await repository.save(userAccount2);

expect(await repository.findById(testId)).toStrictEqual(userAccount1);
});

test('findByEmail returns userAccount when such email exists in database', async () => {
const testId = entityIdGenerator.generate();
const testEmail = 'testEmail';
const userAccount1 = new UserAccount({
userId: testId,
email: testEmail,
password: 'testPassword',
});
const userAccount2 = new UserAccount({
userId: 'id1',
email: 'email1',
password: undefined,
});

//TODO here we already have 2 users accounts in database, so
// afterEach(async () => await props.databaseTestSupport.clearDatabase());
// doesn't work - need t obe implemented
await repository.save(userAccount1);
await repository.save(userAccount2);

expect(await repository.findByEmail(testEmail)).toStrictEqual(userAccount1);
});

//TODO add test for sav()
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { InMemoryTestSupport } from '../../../../../test-support/shared/infrastructure/InMemoryTestSupport';
import { InMemoryAuthenticationRepository } from '../../../../../../src/modules/authentication/infrastructure/repository/inmemory/InMemoryAuthenticationRepository';
import { AuthenticationRepositoryTestCases } from '../AuthenticationRepositoryTestCases';

describe('AuthenticationRepository', () => {
AuthenticationRepositoryTestCases({
name: 'In Memory Implementation',
repositoryFactory: () => new InMemoryAuthenticationRepository(),
databaseTestSupport: InMemoryTestSupport,
});
});
Loading