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

[Hotfix] - Melhorias no Websocket #1337

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

fabioselau077
Copy link
Contributor

@fabioselau077 fabioselau077 commented Jun 29, 2023

TL;DR

Feita algumas melhorias no websocket e adequando todos os eventos para a mesma tipagem (dash-case).

Websocket atual

No socket atual ele manda eventos para todos os ouvintes, sendo necessário ao receber o evento, provavelmente no front-end, validar se é do usuário logado ou não.
Todavia, o navegador recebe todos os eventos emitidos pelo socket, então muitas vezes tá gastando memória e CPU do client para receber eventos que podem nem ser dele.
Tirando também que o websocket não passa por nenhuma validação (próxima PR)

Como vai funcionar?

Em resumo, socket vai enviar os eventos para um "channel", em vez de enviar para todos. O que muda, é que você precisa entrar no channel do nome da sua sessão para receber.
Imagina que você entra em um chat em um site aleatório, ao entrar em uma sala, você será cadastrado como ouvinte daquela sala, então todos os membros vão receber os eventos do socket somente daquela sala.
Essa mesma lógica se aplica ao websocket do Server, trocando a sala pela "session" do usuário (aquela que você utiliza pra gerar o token).

O que impacta?

Vai impactar apenas os clientes que utilizam o websocket para receber os eventos, pois vão ter que adequar alguns eventos pra inglês e dash-case e também ter que entrar no channel da sessão do cliente antes de querer ouvir os eventos.
É recomendado utilizar uuid ou algum hash ao criar a "session", para evitar acesso não autorizado de terceiros na escuta de eventos.

Eventos

Evento Atual Evento Novo Padrão Inglês Padrão dash-case Status
mensagem-enviada sent-message ✔️ ✔️ Refactor
whatsapp-status whatsapp-status ✔️ ✔️ Not Changes
n/d status-find ✔️ ✔️ New Event
qrCode qr-code ✔️ ✔️ Refactor
session-logged session-logged ✔️ ✔️ Not Changes
received-message received-message ✔️ ✔️ Not Changes
incomingcall incoming-call ✔️ ✔️ Refactor
onack on-ack ✔️ ✔️ Refactor
onpresencechanged on-presence-changed ✔️ ✔️ Refactor
onreactionmessage on-reaction-message ✔️ ✔️ Refactor
onrevokedmessage on-revoked-message ✔️ ✔️ Refactor
onpollresponse on-poll-response ✔️ ✔️ Refactor
onupdatelabel on-update-label ✔️ ✔️ Refactor

Exemplos

Front-end em Vuejs

image
image

@icleitoncosta
Copy link
Collaborator

Opa @fabioselau077 perfeito seu PR, preciso dar uma avaliada melhor nos códigos, mas já sabia que estava assim, e iria fazer uma correção futura. Só preciso dar uma olhada melhor, se não irá quebrar nenhum código já em uso por ninguém, irei fazer isso, e se não houver alterações, aceito o PR aqui.
Até

@fabioselau077
Copy link
Contributor Author

Opa @fabioselau077 perfeito seu PR, preciso dar uma avaliada melhor nos códigos, mas já sabia que estava assim, e iria fazer uma correção futura. Só preciso dar uma olhada melhor, se não irá quebrar nenhum código já em uso por ninguém, irei fazer isso, e se não houver alterações, aceito o PR aqui. Até

ok, Cleiton, é que olhando a comunidade percebi que a grande maioria utiliza os webhooks em vez do websocket, por isso que fiz a PR, acredito q dá pra lançar como uma versão beta ou lançar uma nova versão e nela colocar os avisos de deprecated, por exemplo, que o nome "mensagem-enviada" será depreciada em breve, será alterada para "sent-mensage"... e daqui um tempo subir

@fabioselau077
Copy link
Contributor Author

@icleitoncosta to verificando aqui que tem eventos que não vai ter a session, fiz uma validação básica
const session = req?.session || data?.session;
Mas mesmo assim pode ficar undefined, nesse caso, vai enviar para todos os sockets ou não vai enviar nada?

@icleitoncosta
Copy link
Collaborator

Opa @fabioselau077. Que PR bem documentado em? Parabéns!

Vou te passar uma ideia, do que acho como poderia ser feito.

Toda vez que for feita uma requisição do front para o server, dentro do server ter um middleware verificando o token da requisição, o mesmo token que utiliza para o webhook, inclusive a validação pode ser até a mesma.
Segue um exemplo (Ps: esse exemplo é antigo e já fiz a alguns anos para um outro projeto, mas a lógica mais ou menos é essa), mais tarde posso te mandar o código completo ajustado caso prefira.

import jwt from 'jsonwebtoken';
import { Socket } from 'socket.io';

import { config } from '../config';
import { getById } from '../services/UserService';
import { logger } from '../utils';

const verifyUserRequestSocket = async (event: Socket, next: any) => {
  try {
    const decoded = await verifyToken(event.handshake.auth.token);
    const user = await getById(decoded.id);
    event.join(user.empresaId.toString());
    next();
  } catch (error: any) {
    logger.error('Erro ao validar token de usuário.');
  }
};

export default verifyUserRequestSocket;

interface IDecoded {
  id: number;
}

function verifyToken(token: string): Promise<IDecoded> {
  return new Promise((resolve, reject) => {
    try {
      jwt.verify(
        token,
        config.secret_jwt,
        async function (err: any, decoded: any) {
          if (err) reject(err);
          resolve(decoded);
        }
      );
    } catch (error) {
      reject(error);
    }
  });
}

Já no join, usariamos algo dessa forma:

io.use(verifyUserRequestSocket);

Na conecção automaticamente joga esse client conectado dentro da sala que tem o nome da sessão. Isso evitaria criarmos um novo channelId, e colocamos uma validação de segurança no código, já os eventos, nesse inicio pode fazer a alteração deles, vamos fazer a mudança de eventos para o socket, e teremos que de qualquer forma quebrar a aplicação de alguns, pois isso está com uma falha de segurança grotesca, entrando no socket, o invasor tem acesso a todas as conversas.

PS: não validei ainda todo o código, só to adiantando aqui algumas coisas que julgo necessária para conversarmos e darmos continuidade.

@fabioselau077
Copy link
Contributor Author

Opa @fabioselau077. Que PR bem documentado em? Parabéns!

Vou te passar uma ideia, do que acho como poderia ser feito.

Toda vez que for feita uma requisição do front para o server, dentro do server ter um middleware verificando o token da requisição, o mesmo token que utiliza para o webhook, inclusive a validação pode ser até a mesma. Segue um exemplo (Ps: esse exemplo é antigo e já fiz a alguns anos para um outro projeto, mas a lógica mais ou menos é essa), mais tarde posso te mandar o código completo ajustado caso prefira.

import jwt from 'jsonwebtoken';
import { Socket } from 'socket.io';

import { config } from '../config';
import { getById } from '../services/UserService';
import { logger } from '../utils';

const verifyUserRequestSocket = async (event: Socket, next: any) => {
  try {
    const decoded = await verifyToken(event.handshake.auth.token);
    const user = await getById(decoded.id);
    event.join(user.empresaId.toString());
    next();
  } catch (error: any) {
    logger.error('Erro ao validar token de usuário.');
  }
};

export default verifyUserRequestSocket;

interface IDecoded {
  id: number;
}

function verifyToken(token: string): Promise<IDecoded> {
  return new Promise((resolve, reject) => {
    try {
      jwt.verify(
        token,
        config.secret_jwt,
        async function (err: any, decoded: any) {
          if (err) reject(err);
          resolve(decoded);
        }
      );
    } catch (error) {
      reject(error);
    }
  });
}

Já no join, usariamos algo dessa forma:

io.use(verifyUserRequestSocket);

Na conecção automaticamente joga esse client conectado dentro da sala que tem o nome da sessão. Isso evitaria criarmos um novo channelId, e colocamos uma validação de segurança no código, já os eventos, nesse inicio pode fazer a alteração deles, vamos fazer a mudança de eventos para o socket, e teremos que de qualquer forma quebrar a aplicação de alguns, pois isso está com uma falha de segurança grotesca, entrando no socket, o invasor tem acesso a todas as conversas.

PS: não validei ainda todo o código, só to adiantando aqui algumas coisas que julgo necessária para conversarmos e darmos continuidade.

Mas cada request não faria uma nova connection na mesma sala ou o socket.io detecta e mantém a mesma?

@icleitoncosta
Copy link
Collaborator

Opa @fabioselau077. Que PR bem documentado em? Parabéns!
Vou te passar uma ideia, do que acho como poderia ser feito.
Toda vez que for feita uma requisição do front para o server, dentro do server ter um middleware verificando o token da requisição, o mesmo token que utiliza para o webhook, inclusive a validação pode ser até a mesma. Segue um exemplo (Ps: esse exemplo é antigo e já fiz a alguns anos para um outro projeto, mas a lógica mais ou menos é essa), mais tarde posso te mandar o código completo ajustado caso prefira.

import jwt from 'jsonwebtoken';
import { Socket } from 'socket.io';

import { config } from '../config';
import { getById } from '../services/UserService';
import { logger } from '../utils';

const verifyUserRequestSocket = async (event: Socket, next: any) => {
  try {
    const decoded = await verifyToken(event.handshake.auth.token);
    const user = await getById(decoded.id);
    event.join(user.empresaId.toString());
    next();
  } catch (error: any) {
    logger.error('Erro ao validar token de usuário.');
  }
};

export default verifyUserRequestSocket;

interface IDecoded {
  id: number;
}

function verifyToken(token: string): Promise<IDecoded> {
  return new Promise((resolve, reject) => {
    try {
      jwt.verify(
        token,
        config.secret_jwt,
        async function (err: any, decoded: any) {
          if (err) reject(err);
          resolve(decoded);
        }
      );
    } catch (error) {
      reject(error);
    }
  });
}

Já no join, usariamos algo dessa forma:

io.use(verifyUserRequestSocket);

Na conecção automaticamente joga esse client conectado dentro da sala que tem o nome da sessão. Isso evitaria criarmos um novo channelId, e colocamos uma validação de segurança no código, já os eventos, nesse inicio pode fazer a alteração deles, vamos fazer a mudança de eventos para o socket, e teremos que de qualquer forma quebrar a aplicação de alguns, pois isso está com uma falha de segurança grotesca, entrando no socket, o invasor tem acesso a todas as conversas.
PS: não validei ainda todo o código, só to adiantando aqui algumas coisas que julgo necessária para conversarmos e darmos continuidade.

Mas cada request não faria uma nova connection na mesma sala ou o socket.io detecta e mantém a mesma?

Sim, pode acontecer mesmo, no caso usa o middleware apenas pra validar a requisição se tem o auth token e ele é valido, e pode usar o socket.on('connection' pra fazer a ligação dentro da sala correta.

Exemplo:

server.on("connection", (socket) => {
  try {
    const session = LogicaParaOSession;
    await verifyToken(socket.handshake.auth.token);
    socket.join(session);
  } catch (error: any) {
    logger.error('Erro ao validar token de usuário.');
  }
});

@fabioselau077
Copy link
Contributor Author

Opa @fabioselau077. Que PR bem documentado em? Parabéns!
Vou te passar uma ideia, do que acho como poderia ser feito.
Toda vez que for feita uma requisição do front para o server, dentro do server ter um middleware verificando o token da requisição, o mesmo token que utiliza para o webhook, inclusive a validação pode ser até a mesma. Segue um exemplo (Ps: esse exemplo é antigo e já fiz a alguns anos para um outro projeto, mas a lógica mais ou menos é essa), mais tarde posso te mandar o código completo ajustado caso prefira.

import jwt from 'jsonwebtoken';
import { Socket } from 'socket.io';

import { config } from '../config';
import { getById } from '../services/UserService';
import { logger } from '../utils';

const verifyUserRequestSocket = async (event: Socket, next: any) => {
  try {
    const decoded = await verifyToken(event.handshake.auth.token);
    const user = await getById(decoded.id);
    event.join(user.empresaId.toString());
    next();
  } catch (error: any) {
    logger.error('Erro ao validar token de usuário.');
  }
};

export default verifyUserRequestSocket;

interface IDecoded {
  id: number;
}

function verifyToken(token: string): Promise<IDecoded> {
  return new Promise((resolve, reject) => {
    try {
      jwt.verify(
        token,
        config.secret_jwt,
        async function (err: any, decoded: any) {
          if (err) reject(err);
          resolve(decoded);
        }
      );
    } catch (error) {
      reject(error);
    }
  });
}

Já no join, usariamos algo dessa forma:

io.use(verifyUserRequestSocket);

Na conecção automaticamente joga esse client conectado dentro da sala que tem o nome da sessão. Isso evitaria criarmos um novo channelId, e colocamos uma validação de segurança no código, já os eventos, nesse inicio pode fazer a alteração deles, vamos fazer a mudança de eventos para o socket, e teremos que de qualquer forma quebrar a aplicação de alguns, pois isso está com uma falha de segurança grotesca, entrando no socket, o invasor tem acesso a todas as conversas.
PS: não validei ainda todo o código, só to adiantando aqui algumas coisas que julgo necessária para conversarmos e darmos continuidade.

Mas cada request não faria uma nova connection na mesma sala ou o socket.io detecta e mantém a mesma?

Sim, pode acontecer mesmo, no caso usa o middleware apenas pra validar a requisição se tem o auth token e ele é valido, e pode usar o socket.on('connection' pra fazer a ligação dentro da sala correta.

Exemplo:

server.on("connection", (socket) => {
  try {
    const session = LogicaParaOSession;
    await verifyToken(socket.handshake.auth.token);
    socket.join(session);
  } catch (error: any) {
    logger.error('Erro ao validar token de usuário.');
  }
});

Entendi Cleiton, vi que não tem nada do JWT instalado, acha viável adicionar instalação do pacote, criação/mudança do middleware e tudo mais nessa PR ou deixamos essa PR só pra refatoração do código e envio para os channels e depois criamos outra com essa melhoria de segurança?
Acredito que se colocar tudo junto vai ficar mt coisa na mesma PR

@icleitoncosta
Copy link
Collaborator

Entendi Cleiton, vi que não tem nada do JWT instalado, acha viável adicionar instalação do pacote, criação/mudança do middleware e tudo mais nessa PR ou deixamos essa PR só pra refatoração do código e envio para os channels e depois criamos outra com essa melhoria de segurança? Acredito que se colocar tudo junto vai ficar mt coisa na mesma PR

Precisa ter o JWT não, só usei como exemplo um código que eu já tinha aqui, podemos usar o próprio bcrypt do token aqui, já que a pessoa já usa no webhook, e pra enviar mensagens, usamos o mesmo no socket.

Vou ver se eu consigo editar daqui seu PR, e já coloco o código ajustado mais tarde

@fabioselau077
Copy link
Contributor Author

fabioselau077 commented Aug 2, 2023

Entendi Cleiton, vi que não tem nada do JWT instalado, acha viável adicionar instalação do pacote, criação/mudança do middleware e tudo mais nessa PR ou deixamos essa PR só pra refatoração do código e envio para os channels e depois criamos outra com essa melhoria de segurança? Acredito que se colocar tudo junto vai ficar mt coisa na mesma PR

Precisa ter o JWT não, só usei como exemplo um código que eu já tinha aqui, podemos usar o próprio bcrypt do token aqui, já que a pessoa já usa no webhook, e pra enviar mensagens, usamos o mesmo no socket.

Vou ver se eu consigo editar daqui seu PR, e já coloco o código ajustado mais tarde

Alguma atualização desse PR, Cleiton? Se tiver alguma mudança pra fazer, pode ficar a vontade.

Acredito que o JWT dá pra fazer outro PR depois, dai essa fica somente em criar o canal e refatorar os nomes de eventos.

@nimaam
Copy link

nimaam commented Aug 23, 2023

I check the codes, you applied the channel ID to the socket.io so it is good for the performance, here we have still send the message to all client, so I think we should add the option to assign the WhatsApp number to the socket.io and filter to who the server send the socket, what I mean the client send the WP number to the server and get only the response for this client. So base on the Token we can authorize the socket.io to prevent the security issue.

@fabioselau077
Copy link
Contributor Author

I check the codes, you applied the channel ID to the socket.io so it is good for the performance, here we have still send the message to all client, so I think we should add the option to assign the WhatsApp number to the socket.io and filter to who the server send the socket, what I mean the client send the WP number to the server and get only the response for this client. So base on the Token we can authorize the socket.io to prevent the security issue.

It's a good move, feel free to contribute PR.
We are still discussing PR as it involves improvements that affect production users.
I forked it and in the meantime I've been working on it for months and no problems so far.

@icleitoncosta
Copy link
Collaborator

icleitoncosta commented Aug 27, 2023

Hi @fabioselau077 @nimaam

I've made the necessary changes in the PR. Now, you need to run the code below to connect via Socket:
(ps: use this code in your socket client)

const socket = io('http://localhost:21465', {
  auth: {
    token: '$2b$10$rQ5gQlPpv9MD2q.vPuI1F.xYmiROKVcRQRWh09DSM.bTdsE6DHNnG',
    session: 'NERDWHATS_AMERICA',
  },
  autoConnect: true,
});

This addresses the security issue. To avoid disrupting the applications of those who are already using it, I've kept the old events. However, I will add a deprecation notice starting from December 2023.

Please test the code on your machines before I release the fix in version 3.0.

Best regards,

@nimaam
Copy link

nimaam commented Aug 30, 2023

Hi @fabioselau077 @nimaam

I've made the necessary changes in the PR. Now, you need to run the code below to connect via Socket: (ps: use this code in your socket client)

const socket = io('http://localhost:21465', {
  auth: {
    token: '$2b$10$rQ5gQlPpv9MD2q.vPuI1F.xYmiROKVcRQRWh09DSM.bTdsE6DHNnG',
    session: 'NERDWHATS_AMERICA',
  },
  autoConnect: true,
});

This addresses the security issue. To avoid disrupting the applications of those who are already using it, I've kept the old events. However, I will add a deprecation notice starting from December 2023.

Please test the code on your machines before I release the fix in version 3.0.

Best regards,

I did the same but the QR code and messages anymore

@icleitoncosta
Copy link
Collaborator

Hi @fabioselau077 @nimaam

I've made the necessary changes in the PR. Now, you need to run the code below to connect via Socket: (ps: use this code in your socket client)

const socket = io('http://localhost:21465', {

auth: {

token: '$2b$10$rQ5gQlPpv9MD2q.vPuI1F.xYmiROKVcRQRWh09DSM.bTdsE6DHNnG',
session: 'NERDWHATS_AMERICA',

},

autoConnect: true,

});

This addresses the security issue. To avoid disrupting the applications of those who are already using it, I've kept the old events. However, I will add a deprecation notice starting from December 2023.

Please test the code on your machines before I release the fix in version 3.0.

Best regards,

I did the same but the QR code and messages anymore

I not understand your message

@fabioselau077
Copy link
Contributor Author

@icleitoncosta Tô vendo aqui pra testar, pq já subi essa branch na master do meu fork, n sei se consigo fazer revert no meu fork e copiar essa branch dnv, acho que vou ter qu apagar o fork e fazer as alterações na mão dos 6 arquivos, vou ver aq e te aviso

@fabioselau077
Copy link
Contributor Author

@icleitoncosta testei aqui e tá tudo ok, só fiz duas mudanças, alterei a posição depois do split pra 0 pra pegar o certo e adicionei o evento status-find (que atualmente só chama o webhook), testei e tá tudo ok

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

Successfully merging this pull request may close these issues.

None yet

3 participants