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.

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

5 feat 8 autententicação do usuario #12

Open
wants to merge 31 commits into
base: dev
Choose a base branch
from

Conversation

ItzTas
Copy link

@ItzTas ItzTas commented Aug 23, 2024

#5 middleware de autenticação de usuario

@heyitsmepablo heyitsmepablo requested review from ed-henrique and removed request for heyitsmepablo August 23, 2024 02:41
@heyitsmepablo heyitsmepablo added the enhancement New feature or request label Aug 23, 2024
@heyitsmepablo heyitsmepablo linked an issue Aug 23, 2024 that may be closed by this pull request
5 tasks
Copy link

@ed-henrique ed-henrique left a comment

Choose a reason for hiding this comment

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

Acredito que aplicando as modificações propostas e melhorando as mensagens de erro retornadas ao usuário, esse middleware vai ficar pronto.

*/
import jwt from 'jsonwebtoken';
import moment from 'moment';
const JWT_SECRET = `${process.env['JWT_SECRET']}`;

Choose a reason for hiding this comment

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

Deveria haver uma condição checando se o JWT_SECRET que foi obtido não é vazio, porque isto poderia causar inconsistências com os tokens já existentes para os usuários. Exemplo:

if (!JWT_SECRET) {
  // Abortar execução do servidor ou obter o valor de JWT_SECRET de outra fonte
}

* @throws {JsonWebTokenError}
* @throws {TokenExpiredError}
*/
function decodeJWT(token, options = undefined) {

Choose a reason for hiding this comment

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

O parâmetro options para decodificar o JWT não é interessante nesse caso, pois permite que haja mudanças por desenvolvedores futuros em padrões essenciais que invalidariam os tokens de usuário já existentes e funcionais, como:

  • algorithms
  • ignoreExpiration
  • maxAge
  • allowInvalidAsymmetricKeyTypes

No geral, se tratando de um middleware de autenticação, o recomendado é que essas opções sejam definidas como uma constante que não deve ser alterada, ou que a função decodeJWT seja encapsulada antes de ser exportada, para que não seja possível alterar as opções por outro módulo que não seja o de middleware. Exemplo:

const defaultOptions = {
  // Defina as opções padrão aqui
}

function decodeJWT(token) {
  try {
    return jwt.verify(token, JWT_SECRET, defaultOptions);
  } catch (err) {
    // Tratamento de erros
  }
}

ou

function decodeJWTPadrao(token) {
  return decodeJWT(token, {
    // Insira as opções desejadas aqui
  });
}

// ...

export { decodeJWTPadrao, ... }

throw new Error('sem token de autorização ou nome errado');
}
const headerDiv = bearerToken.split(' ');
if (headerDiv[0] != 'Bearer' || headerDiv.length === 0) {

Choose a reason for hiding this comment

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

Essa condição poderia verificar se o tamanho de headerDiv é maior que 2, o que indicaria um token JWT corrupto, provavelmente. Isso poderia ser feito dessa forma:

if (headerDiv[0] != 'Bearer' || headerDiv.length !== 2) {
  // Lógica
}

* @param {jwt.SignOptions} [options={ expiresIn: defaultTokenExpiration }]
* @returns {String}
*/
function signJWT(payload, options = {}) {

Choose a reason for hiding this comment

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

Mesma questão do decodeJWT. O parâmetro options deve ser uma constante ou a função deve ser encapsulada.

Copy link
Author

Choose a reason for hiding this comment

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

fiz assim o sign pra eu poder colocar o tempo de expiração como 1ms no teste

Choose a reason for hiding this comment

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

Talvez fazendo de uma forma parecida à descrita aqui. Basicamente, deixa o que é pra ser exportado para testes com um nome bem explícito que é para testes, tipo testSignJWT, não deixando brecha para que os desenvolvedores se confundam no futuro.

Sinceramente, eu não gosto muito dessa abordagem, até porque em Go eu só colocaria os testes dentro do mesmo pacote do código sem exportar o que é pra ser interno. Mas anyways, a gente lida com o que a gente tem.

*
*/
import jwt from 'jsonwebtoken';
import moment from 'moment';

Choose a reason for hiding this comment

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

Moment.js é uma biblioteca legada, que apenas recebe atualizações corretivas. Não há necessidade para sua utilização, dado que é possível utilizar os métodos de Date para alcançar o mesmo resultado.

Consultar estado da biblioteca

* @returns {String}
*/
function formatObject(obj) {
return JSON.stringify(obj, null, 2);

Choose a reason for hiding this comment

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

Essa linha fica muito obscura pra quem não mexe com JavaScript direto. Talvez fosse mais interessante fazer algo assim:

return JSON.stringify(obj, NULL_KEYS_VALUE, SPACE_IDENTATION);

Dessa forma, mesmo alguém que não tenha familiaridade com os parâmetros da função JSON.stringify entenderia rapidamente.

try {
// @ts-ignore
let id = decoded.sub;
let reuslt = await pool.query('SELECT *FROM users WHERE id = $1 ', [id]);

Choose a reason for hiding this comment

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

Alterar para:

let result = await pool.query('SELECT * FROM users WHERE id = $1', [id]);

Cuidado com os nomes das variáveis para não confundir futuros desenvolvedores.

// @ts-ignore
let id = decoded.sub;
let reuslt = await pool.query('SELECT *FROM users WHERE id = $1 ', [id]);
if (reuslt.rows.length == 0) {

Choose a reason for hiding this comment

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

É interessante que haja uma condição checando se mais de um usuário foi encontrado com aquele ID, e em caso positivo, realizar um log e devolver um erro 5xx. Exemplo:

if (result.rows.lenght > 1) {
  console.log(`ERROR: Mais de um usuário foi encontrado com o id ${id}`);
  return res.status(500).json({ error: 'Houve um erro no banco de dados ao autenticar o usuário' });
}

É raro de acontecer esse tipo de situação, e ela geralmente é associada a modificação diretamente via banco de dados ao invés do uso da aplicação, mas é importante manter em mente que isso pode acontecer.


describe('AUTH TEST', () => {
afterAll(async () => {
await pool.query('TRUNCATE TABLE users CASCADE');

Choose a reason for hiding this comment

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

Muito cuidado com esse tipo de limpeza após os testes. Apenas realize esse tipo de operação se houver certeza que o banco utilizado é o de testes. Ainda mais porque esse middleware não lida com a criação de novos usuários, apenas leitura dos já existentes.

import { signJWT } from '../src/helpers/auth.js';
import { formatObject } from '../src/helpers/general_helpers.js';
import pool from '../src/database/database.js';
import moment from 'moment';

Choose a reason for hiding this comment

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

Remoção da biblioteca legado Moment.js.

Copy link

@ed-henrique ed-henrique left a comment

Choose a reason for hiding this comment

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

Ok.

Só manter em mente que essa API vai servir algum usuário ou desenvolvedor frontend, geralmente. Isso significa que erros mais descritivos são preferíveis, como O token JWT passado foi inválido ao invés de invalido.

Além disso, o servidor não faz nada se o banco de dados não estiver conectado a ele. Por isso, ferramentas como as descritas aqui na documentação do pacote pg são úteis para saber se o servidor deve ser parado.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Historia 8: Autententicação do Usuario
4 participants