DevVictor19 / unified-news-portal-api

An application designed to centralize the communication channel of a higher education institution. It utilizes Clean Architecture, testing, CI/CD, and MongoDB.
30 stars 1 forks source link

Referência de ROLE fraca + sugestão de alteração #3

Closed matheusdgdcampos closed 7 months ago

matheusdgdcampos commented 7 months ago

https://github.com/DevVictor19/portal-de-noticias-unificado/blob/4f9048afc1b1e4aec19400af13ee78d7edf8d5a7/src/common/enums/roles.enum.ts#L1-L8

Uma boa prática nesse caso é deixar somente o índice do ENUM identificar a ROLE e cuidar das tratativas assim fica com uma referência forte para casos de ataque ao sistema pois o índice da ROLE não condiz muito o que aquela ROLE é capaz de fazer sem perder a semântica de toda a lógica, entretanto, vale ressaltar que vale documentar a entidade no banco pois pode ser necessário em mudanças futuras. Dito isso eu sugeriria a seguinte mudança:

export enum ROLES { 
   ADMIN, 
   COORDINATOR, 
   LEADER, 
   VICE_LEADER, 
   TEACHER, 
   STUDENT, 
 } 

Nesse caso ADMIN=0, COODINATOR=1, LEADER=2 e assim sucessivamente. Espero poder ter ajudado positivamente de alguma forma ;-)

DevVictor19 commented 7 months ago

Boa sugestão @matheusdgdcampos, mas eu fiquei na dúvida.

No momento quando salvo um novo User salvo também a role dele, e no banco fica "role: 'admin'".

Seguindo sua sugestão vai começar a salvar "role: 0", "role: 1".

A sua sugestão é muito boa na questão da segurança, mas no banco não fica tão semântico, você acha que vale a pena ainda?

Lembrando que toda a manipulação dessas roles são feitas dentro de um token JWT, se alguém alterar o token e modificar a role, ao validar o token a assinatura não vai bater...

matheusdgdcampos commented 7 months ago

@DevVictor19 foi por isso que disse que essa sugestão se faz necessário a documentação do banco pois você abre mão da semântica no banco, porém, sua aplicação está segura pois tanto nas requisições quanto no seu jwt essa informação não faz menção as atribuições deste usuário. Hoje mesmo existe uma aplicação que fiz com o time que estava trabalhando que está em produção e adotamos esta estratégia e a única coisa que precisamos documentar de fato em um arquivo a parte foi o banco o resto a semântica continuou intacta. Infelizmente o mundo da programação é uma via de mão dupla mas prefiro abrir mão de coisas menos triviais do que da segurança

DevVictor19 commented 7 months ago

Beleza, muito obrigado pelo insight! Vou aderir.

commit da correção: https://github.com/DevVictor19/portal-de-noticias-unificado/commit/5477075f50750530f6cfc492304201ff1d5a9d9e