Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Somente exibir campo de Contato quando logado(staff ou user) #77

Merged
merged 4 commits into from
May 13, 2024
Merged

Somente exibir campo de Contato quando logado(staff ou user) #77

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9f5fabc
feat: only show contact on shelter public routes on authorized roles
kelvinsb May 11, 2024
13d65ba
fix: remove unused and unauthorized contact field on list route
kelvinsb May 11, 2024
8db01be
feat: return contact when user logged, instead of some Roles
kelvinsb May 12, 2024
ecdf8f4
fix: suggestions about separation
kelvinsb May 13, 2024
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
3 changes: 3 additions & 0 deletions src/decorators/UserDecorator/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { UserDecorator } from './user.decorator';

export { UserDecorator };
kelvinsb marked this conversation as resolved.
Show resolved Hide resolved
8 changes: 8 additions & 0 deletions src/decorators/UserDecorator/user.decorator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { createParamDecorator, ExecutionContext } from '@nestjs/common';

export const UserDecorator = createParamDecorator(
(data: unknown, ctx: ExecutionContext) => {
const request = ctx.switchToHttp().getRequest();
return request?.user;
},
);
10 changes: 10 additions & 0 deletions src/guards/apply-user.guard.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { Injectable } from '@nestjs/common';
import { AuthGuard } from '@nestjs/passport';

@Injectable()
export class ApplyUser extends AuthGuard('jwt') {
handleRequest(err: any, user: any) {
if (user) return user;
return null;
}
}
40 changes: 26 additions & 14 deletions src/guards/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,34 @@ async function canActivate(context: ExecutionContext, allowed: AccessLevel[]) {
if (request.user) {
const { userId, sessionId } = request.user;

const session = await service.session.findUnique({
where: { id: sessionId, active: true, user: { id: userId } },
include: {
user: true,
},
});

if (
session &&
allowed.some((permission) => permission === session.user.accessLevel)
) {
return true;
}
return isRightSessionRole(allowed, sessionId, userId);
}

return false;
}

export { canActivate };
async function isRightSessionRole(
allowed: AccessLevel[],
sessionId?: string,
userId?: string,
) {
if (!sessionId) return false;
if (!userId) return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

esses dois ifs aqui ficaram redundantes, não?
porque se não tiver sessionId nem userId, a query abaixo já vai retornar false de qualquer jeito

Copy link
Contributor Author

Choose a reason for hiding this comment

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

É só uma guard clause pra se não for informada nem gastar uma query com dado inválido. Posso remover se for o caso...


const session = await service.session.findUnique({
where: { id: sessionId, active: true, user: { id: userId } },
include: {
user: true,
},
});

if (
session &&
allowed.some((permission) => permission === session.user.accessLevel)
) {
return true;
}
return false;
}

export { canActivate, isRightSessionRole };
kelvinsb marked this conversation as resolved.
Show resolved Hide resolved
7 changes: 5 additions & 2 deletions src/shelter/shelter.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import { ApiTags } from '@nestjs/swagger';
import { ShelterService } from './shelter.service';
import { ServerResponse } from '../utils';
import { StaffGuard } from '@/guards/staff.guard';
import { ApplyUser } from '@/guards/apply-user.guard';
import { UserDecorator } from '@/decorators/UserDecorator';

@ApiTags('Abrigos')
@Controller('shelters')
Expand All @@ -35,9 +37,10 @@ export class ShelterController {
}

@Get(':id')
async show(@Param('id') id: string) {
@UseGuards(ApplyUser)
Copy link
Contributor

Choose a reason for hiding this comment

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

aqui eu acho que não precisa do ApplyUser, pode usar direto @UseGuards(UserGuard) e aí ele vai aprovar todo mundo que tenha, no mínimo, a permissão de User, visto que é uma hierarquia de permissões

Copy link
Contributor Author

Choose a reason for hiding this comment

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

O problema que usar o @UseGuards(UserGuard) ele vai aplicar a restrição pra rota toda em vez de apenas um atributo(o de telefone). Então usuários não logados não vão conseguir ver os detalhes do abrigo quebrando essa página.
As screens com o projeto rodando essa brach:
image
image

Copy link
Contributor

Choose a reason for hiding this comment

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

aaah sim, te entendi. faz sentido.

pergunta: o nest.js não deveria ter um "ApplyUser" nativo não?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

não sei/consegui achar, se souber como eu altero já também

async show(@UserDecorator() user: any, @Param('id') id: string) {
try {
const data = await this.shelterService.show(id);
const data = await this.shelterService.show(id, user);
return new ServerResponse(200, 'Successfully get shelter', data);
} catch (err: any) {
this.logger.error(`Failed to get shelter: ${err}`);
Expand Down
8 changes: 5 additions & 3 deletions src/shelter/shelter.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,10 @@ export class ShelterService {
});
}

async show(id: string) {
async show(id: string, user: any) {
const isLogged =
Boolean(user) && Boolean(user?.sessionId) && Boolean(user?.userId);
Copy link
Contributor

Choose a reason for hiding this comment

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

essa lógica aqui está correta.
porém, não acho que a camada de service precise receber o contexto do usuário que está logado.
por isso, essa lógica deveria ficar na camada de controller e aqui o método show poderia receber um boolean por parametro chamado shouldShowContact que por padrão é false.

assim a funcionalidade segue a mesma, mas mantemos a integridade da responsabilidade da service

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sim, é uma boa, vou alterar
Já aproveitando o gancho: será que não seria interessante já seguir o padrão repository também pra puxar essas partes do banco e deixar a camada de service somente com as regras de negócio? até facilitaria o teste de services sem precisar mockar...


const data = await this.prismaService.shelter.findFirst({
where: {
id,
Expand All @@ -72,7 +75,7 @@ export class ShelterService {
pix: true,
shelteredPeople: true,
capacity: true,
contact: true,
contact: isLogged,
petFriendly: true,
prioritySum: true,
latitude: true,
Expand Down Expand Up @@ -137,7 +140,6 @@ export class ShelterService {
pix: true,
address: true,
capacity: true,
contact: true,
petFriendly: true,
shelteredPeople: true,
prioritySum: true,
Expand Down
4 changes: 3 additions & 1 deletion src/shelter/types/search.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ export interface IFilterFormProps {
tags: ShelterTagInfo | null;
}

export type SearchShelterTagResponse = Shelter & {
type AllowedShelterFields = Omit<Shelter, 'contact'>;

export type SearchShelterTagResponse = AllowedShelterFields & {
shelterSupplies: (ShelterSupply & { supply: Supply })[];
};

Expand Down