search

filipedeschamps / tabnews.com.br

Conteúdos para quem trabalha com Programação e Tecnologia.
https://tabnews.com.br
GNU General Public License v3.0
5.32k stars 388 forks source link

[Bug] Estou logado com outro usuário #544

Closed ErickCReis closed 2 years ago

ErickCReis commented 2 years ago

Acabei de entrar no TabNews e me deparei com isso:

Design sem nome

Por algum motivo estou logado com outro usuário, estou no trabalho agora então não vou conseguir investigar isso agora, mas me parece algo urgente.

Não tentei fazer publicação ou usar os tabcoins para ver se a api apresenta algum erro, mas já atualizei a página e continuei assim.

@filipedeschamps @aprendendofelipe

filipedeschamps commented 2 years ago

@ErickCReis muito obrigado por reportar! Estou investigando! Você saberia dizer se isso aconteceu do nada?

ErickCReis commented 2 years ago

Sim, ontem a noite tenho quase certeza que estava logado com meu usuário, e hoje de manhã só abri o site e estava assim.

filipedeschamps commented 2 years ago

@ErickCReis qual seu usuário dentro do TabNews?

ErickCReis commented 2 years ago

erickreis

filipedeschamps commented 2 years ago

@ErickCReis estou verificando se alguma sessão conflitou 👍

aprendendofelipe commented 2 years ago

Ué, roubou minha sessão? 😱

filipedeschamps commented 2 years ago

@ErickCReis você poderia enviar um email para contato@tabnews.com.br com o valor que está em session_id nos Cookies?

image
ErickCReis commented 2 years ago

Enviado

filipedeschamps commented 2 years ago

Investigando!

filipedeschamps commented 2 years ago

Vou reportando aqui qualquer informação que eu encontrar, mas por enquanto de fato no banco de dados a sessão que o @ErickCReis enviou por email aponta para o user_id do @aprendendofelipe e essa sessão foi criada 2022-05-26 00:25:19.423693+00 UTC e continua sendo revalidada.

Investigando 🤝

filipedeschamps commented 2 years ago

@ErickCReis invalidei a sessão, se possível peço que ao tentar fazer um refresh da página você está deslogado.

GuiLra commented 2 years ago

Aconteceu o mesmo comigo Design sem nome

filipedeschamps commented 2 years ago

@GuiLra você poderia me enviar o seu session_id para o email contato@tabnews.com.br?

filipedeschamps commented 2 years ago

Estou desconfiado que a nova rota das thumbnails esteja cacheando os cookies, ou alguma outra rota.

aprendendofelipe commented 2 years ago

@ErickCReis invalidei a sessão, se possível peço que ao tentar fazer um refresh da página você está deslogado.

Eita! Acabou de revogar o acesso no meu celular. Devia ser essa a sessão original com esse id

filipedeschamps commented 2 years ago

@aprendendofelipe faz sentido 🤝 você lembra de ter compartilhado alguma publicação com o novo sistema de thumbnail dinâmica?

aprendendofelipe commented 2 years ago

@aprendendofelipe faz sentido 🤝 você lembra de ter compartilhado alguma publicação com o novo sistema de thumbnail dinâmica?

Não compartilhei, mas cliquei no link dos exemplos que você enviou ontem

aprendendofelipe commented 2 years ago

E fiz isso pelo celular

GuiLra commented 2 years ago

@filipedeschamps Eu acho que tem a ver com esses links. Eu tmb consegui acessar a conta do @ErickCReis Imgur

filipedeschamps commented 2 years ago

@aprendendofelipe obrigado pelas informações! Estou tentando vazar minha session_id para confirmar o bug 🤝

filipedeschamps commented 2 years ago

Consegui confirmar o bug 🤝 através de uma aba logada, eu acessei uma thumbnail e isso fez o endpoint da API que gera ela ser cacheando, inclusive o cookie. Em seguida, eu peguei essa URL da thumbnail e acessei numa aba anônima e a CDN da Vercel retornou o cache, incluindo o cookie.

Está sendo feito um deploy nesse exato momento que exclui esse cache e em seguida vou invalidar todas as sessões.

filipedeschamps commented 2 years ago

Em paralelo, com este novo deploy, todos os caches já foram invalidados.

filipedeschamps commented 2 years ago

Confirmei que com o novo deploy, o session_id não está mais sendo cacheado, nem vazado.

Em paralelo, notei o seguinte:

Na implementação do /thumbnail, a linha que definia o cache era a seguinte:

response.setHeader('Cache-Control', 's-maxage=60, stale-while-revalidate');

Porém, temos outras rotas que também definem o cache de stale-while-revalidate, como por exemplo /status, mas ese bug nunca aconteceu antes. E no endpoint de /status, está implementado assim:

response.setHeader('Cache-Control', 'public, s-maxage=1, stale-while-revalidate');

nota que tem a flag public.

Estou investigando aqui se isso foi o ponto central do problema 🤝

aprendendofelipe commented 2 years ago

Em paralelo, com este novo deploy, todos os caches já foram invalidados.

Precisa excluir os deploys recentes da Vercel, pois se estiverem no ar podem ser usados para roubar a sessão

aprendendofelipe commented 2 years ago

Será que dois acessos dentro do mesmo segundo na rota status não gera o mesmo problema?

aprendendofelipe commented 2 years ago

Estou longe de qualquer computador agora, então não consigo dar uma ajuda melhor.

filipedeschamps commented 2 years ago

Precisa excluir os deploys recentes da Vercel, pois se estiverem no ar podem ser usados para roubar a sessão

Perfeito! Exclui todos que foram criados a partir da primeira implementação, foram 19 (inclui tanto todos os testes em ambiente de Homologação quanto Produção).

filipedeschamps commented 2 years ago

Será que dois acessos dentro do mesmo segundo na rota status não gera o mesmo problema?

Essa é uma ótima pergunta, pois eu também pensava que numa lamba é possível entrar duas requests ao mesmo tempo, mas depois que eu fiz aquela bateria de testes para construir o pool do banco de dados, consegui confirmar que isso não acontece, é uma request por lambda, e se precisar paralelizar, a AWS invoca uma nova lambda. Isso também foi confirmado pelo pessoal que contribui para o módulo pg 🤝

Mas nesse momento não quero descartar nada, só afunilar um pouco a pesquisa, porque a todo momento estou acessando o /status e esse problema pipocou em quantidade só agora (com o deploy do /thumbnail) e com a raiz sendo o a sessão do seu celular. Mas novamente, esse não é o momento para descartar nada 🤝

filipedeschamps commented 2 years ago

Estou longe de qualquer computador agora, então não consigo dar uma ajuda melhor.

Sua ajuda já foi fundamental @aprendendofelipe 🤝 você ter reportado que clicou na URL da thumbnail pelo celular e ao invalidar a sessão foi a sua do celular aumentou muito a definição do problema 👍

filipedeschamps commented 2 years ago

@ErickCReis e @GuiLra uma pergunta para ter mais informações sobre a origem do bug: vocês chegaram a interagir com a URL da thumbnail lá daquela issue, ou qualquer URL de thumbnail?

ErickCReis commented 2 years ago

@ErickCReis e @GuiLra uma pergunta para ter mais informações sobre a origem do bug: vocês chegaram a interagir com a URL da thumbnail lá daquela issue, ou qualquer URL de thumbnail?

Sim, ontem acessei as 3 urls que vc colocou no pr.

GuiLra commented 2 years ago

@ErickCReis e @GuiLra uma pergunta para ter mais informações sobre a origem do bug: vocês chegaram a interagir com a URL da thumbnail lá daquela issue, ou qualquer URL de thumbnail?

Eu acessei as 3 urls que colocou no pr.

aprendendofelipe commented 2 years ago

Está sendo feito um deploy nesse exato momento que exclui esse cache e em seguida vou invalidar todas as sessões.

@filipedeschamps, você falou em invalidar todas as sessões, quer dizer de todos os usuários do TabNews? Mas ainda não fez isso, né? É que acabei de verificar em um computador que a minha sessão ainda está ativa.

filipedeschamps commented 2 years ago

@filipedeschamps, você falou em invalidar todas as sessões, quer dizer de todos os usuários do TabNews? Mas ainda não fez isso, né? É que acabei de verificar em um computador que a minha sessão ainda está ativa.

Correto! Assim que eu terminar a investigação no PR #546 e ter certeza desse comportamento vou invalidar. Estou no último teste e conseguindo isolar ou não, eu vou invalidar todas sessões e publicar no TabNews um postmortem 🤝

filipedeschamps commented 2 years ago

Identifiquei onde está o problema.

Bom, continuando as pesquisas aqui, rodei a mesma bateria de testes em produção no /status que rodei no /thumbnail e não é possível replicar o mesmo bug. No /thumbnail foi zero atrito replicar o bug (no momento que a rota estava cacheada, o cookie estava lá), e no /status não consigo de forma alguma. Então minha pesquisa foi o seguinte:

  1. Comecei especulando que há algo de especial na flag public do Cache-Control, pois essa foi a única mudança que fiz para evitar que o Cookie fosse armazenado. Comecei a cavucar um monte de artigo, como esse aqui e também perguntas no Stack Overflow, e nenhuma delas respondeu com clareza o que o public faz, a não ser falando que isso faz um cache ficar público (mas não exatamente quais informações ficam cacheadas).
  2. Então para aprofundar a investigação, eu criei o PR #546 que reconstrói o bug no endpoint /status no Ambiente de Homologação.
  3. Eu saí de uma posição sem bug e com os mesmos 60 segundos de cache que tinha no /thumbnail (diferente do cache de 1 segundo que originalmente essa rota tinha).
  4. Mas o interessante é que mesmo removendo a flag public o bug não se replica. Eu não sei se os servidores de CDN para o cache em ambiente de homologação são diferentes do de produção.
  5. Encucado com a situação, li com calma o Controller que implementa o /thumbnail e identifiquei o maior ofensor, que é isso aqui.
  6. Ele está injetando a sessão do usuário na request, coisa que o endpoint /status não faz. Então o cache do stale-while-revalidate está de fato cacheando tudo, mas isso não é um problema se você não injetar nessa rota a sessão do usuário e por consequência não cachear o Set-Cookie de retorno.
  7. Então se o Set-Cookie de retorno que devolve a sessão atualizada ficar cacheado na CDN, qualquer navegador que ler esse comando vai de fato setar o cookie de sessão no lado do client.
  8. Então com isso, injetei o usuário no /status e agora consigo replicar o bug 100% das vezes.

Estou invalidando todas as sessões, criando o postmortem (vou publicar eles juntos) e criar outro PR para remover a injeção do usuário naquela rota, mesmo que ela por enquanto não esteja mais sendo cacheada.

Em paralelo, nas rotas com cache, podemos pedir para não fazer o cache do Set-Cookie a pesar de que o resultado não é garantido, mas isso é o início para ficar mais protegido nesse ponto.

Muito obrigado a todos pela ajuda nos dados!!!

Assim que resetar as sessões, publicar o postmortem e fazer o merge do PR que remove a injeção do usuário irei fechar essa issue 🤝

filipedeschamps commented 2 years ago

Sessões invalidadas e postmortem criado: https://www.tabnews.com.br/filipedeschamps/postmortem-todas-as-sessoes-foram-invalidadas-por-conta-do-endpoint-thumbnail

Vou fazer o PR removendo a injeção do usuário no endpoint /thumbnail 🤝

filipedeschamps commented 2 years ago

Feito o merge do PR #547 que faz parar a injeção da sessão do usuário no /thumbnail.

Ambiente de Produção antes deste deploy:

Note que o session_id é retornado no Set-Cookie do response como esperado, pelo fato do endpoint estar injetando e revalidando a sessão do usuário:

image

Ambiente de Produção depois deste deploy:

Set-Cookie não está sendo mais retornado no response, e agora esse retorno pode voltar a ter cache:

image

Aprendizados

  1. Tomar cuidado redobrado em qualquer endpoint que seja escolhido fazer qualquer tipo de cache público.
  2. Set-Cookie também consegue ser cacheado na CDN.
  3. Pesquisar mais sobre no-cache="Set-Cookie" e se é possível definir isso de forma global (e se tem algum efeito).

Estou fechando esta issue, mas qualquer outra informação peço que volte a abrir ela ou dependendo do quão sensível a informação é, entrar em contato através de contato@tabnews.com.br 🤝