Implementa Rate Limit na API: /api/v1/sessions

[filipedeschamps]

Este PR faz basicamente duas coisas:

4ab4b6a

• Faz o database encerrar o pool de conexões de forma mais agressiva, reservando 30% de respiro (antes estava 10%).
• Faz o database aguardar 1 segundo para iniciar uma nova tentativa.
• Em condições normais, isto terá zero impacto de performance e somente será útil em situações de alto stress contra o banco de dados.

6f82ceb

• Implementa de fato o rate-limit em todos os endpoints a partir de /api utilizando o Upstash que é um Redis na nuvem e que pode ser distribuído globalmente.
• Toda implementação inicia na utilização do Edge Middleware que é controlado pelo arquivo middleware.public.js na raiz do projeto.
• Este arquivo de middleware é como se fosse o controller e a implementação do "rate limiter" está em infra/rate-limit.js.
• Então o middleware/controller faz o seguinte:
  1. Se por algum motivo acontecer algum erro, como por exemplo o Upstash estar indisponível, a request é liberada para que o serviço do TabNews não seja interrompido. Se isso acontecer, eu vou receber uma mensagem no monitoramento aqui sobre ServiceError. Mas novamente, nada disso será exposto ao usuário.
  2. Se você estiver em algum ambiente de desenvolvimento (ou teste automatizado) e não configurou as variáveis de ambiente que se conectam com o Upstash, o rate limiter não será utilizado e a request irá passar normalmente.
  3. Se está tudo configurado como esperado, o rate limiter será utilizado.
• Por enquanto, há dois tipos de estratégias para a quantidade de requests que podem ser feitas:
  1. Um limite geral por ip que é aplicado para todas as rotas da API.
  2. Um limite específico por ip, method e path que pode pode ser utilizado em rotas específicas.
  3. Uma dessas rotas específicas é um POST /api/v1/sessions que possui limites muito mais baixos e janelas muito maiores de cooldown.
• Caso um atacante esbarre no limite de requisições de rotas convencionais, ele será avisado com um 429 TooManyRequests.
• Caso um atacante esbarre no limite de requisições do POST /api/v1/sessions, ele não será avisado e de forma "invisível" será retornado um erro comum e falso de 401 Unauthorized e que nunca estará de fato batendo contra a real implementação de criar sessões. Inclusive, esse erro falso simula uma latência falsa para o atacante não perceber que foi pego caso ele analise e note uma mudança no padrão de velocidade das respostas. Eu fiquei estressando a implementação em produção e depois a implementação falsa até que as variações de latência entre as duas ficasse igual e não desse para perceber quando você parou de bater contra o endpoint de verdade e começou a bater no endpoint de mentira.
• A quantidade máxima de requisições e a janela de análise possui valores padrões no código, mas que podem ser sobrescritos utilizando variáveis de ambiente. Assim o código fonte irá mostrar valores X, porém no ambiente de homologação e produção iremos utilizar valores Y.
• Um tradeoff de fazer a request passar pelo middleware, é que não é mais possível receber um body contendo 20.000 caracteres como é possível hoje em produção. O middleware possui limites muito mais restritos que as lambdas. Esse erro foi pego pelos testes de integração e eu reduzi esse limite máximo de caracteres para 16.000. Antes de fazer alguma alteração por migration no check do banco de dados para a coluna body, sugiro maturar a implementação para ver se esse valor se sustenta.
• Em algum momento, a Equipe do Next.js inseriu uma regressão no Framework que impossibilita o Middleware fazer um fetch utilizando POST e um body. Isso foi reportado em várias issues, como essa aqui essa ação é utilizada pelo módulo do Upstash. Felizmente essa regressão foi consertada na versão canary 12.2.4-canary.11, que é o que estou utilizando nesse PR, e imagino que não irá demorar muito para entrar num release oficial.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
tabnews	✅ Ready (Inspect)	Visit Preview	Aug 16, 2022 at 1:18AM (UTC)

[coffeeispower]

Será que dá para fazer a api como naquele video que você fez que o site tava mandando coisas aleatorias inves de não retornar mais dados?

[filipedeschamps]

@aprendendofelipe o limite de 16.000 é esbarrado na publicação clone com os espaços 😂 mas a sem os espaços vai sem problemas 👍

https://tabnews-git-rate-limit-tabnews.vercel.app/fe1ipe/publicacao-clone-da-do-filipe-para-testar-a-performance-da-nova-funcao-removemarkdown-com-relacao-aos-espacos-em-branco-tabnews-atingiu-11-000-visitantes-unicos-nos-ultimos-30-dias

---

O nextjs-progressbar está reclamando do next@12.2.4-canary.11

---

Será que dá para fazer a api como naquele video que você fez que o site tava mandando coisas aleatorias inves de não retornar mais dados?

@coffee-is-power sobre a rota de sessão (que é a mais perigosa), ele meio que faz isso retornando dados falsos mas simulando de forma idêntica uma falha de login, inclusive trocando o uuid do request_id e error_id a cada tentativa como acontece numa situação normal.

Do restante das rotas poderíamos implementar, mas não vejo necessidade porque nós não vendemos os dados como aquela plataforma lá que me trolou vendia 😂 então talvez seja melhor avisar que foi esbarrado num limite. Mas isso não impede de implementar a mesma coisa para outras rotas que possam estar sendo abusadas por pessoas do mal 🤝

[aprendendofelipe]

@aprendendofelipe o limite de 16.000 é esbarrado na publicação clone com os espaços 😂 mas a sem os espaços vai sem problemas 👍

Já apaguei para facilitar 😅

Vai ter que arrumar a publicação original em produção e testar todas as outras se não tem mais nenhuma que passa desse limite

[filipedeschamps]

Vai ter que arrumar a publicação original em produção e testar todas as outras se não tem mais nenhuma que passa desse limite

select title, length(body) from contents where length(body) > 16000;

• https://www.tabnews.com.br/filipedeschamps/tabnews-atingiu-11-000-visitantes-unicos-nos-ultimos-30-dias
• https://www.tabnews.com.br/CleiberReis/lista-de-possiveis-emojis-para-publicacoes-no-tabnews-heart-eyes
• https://www.tabnews.com.br/allangrds/diario-fullstack-2-comecando-um-projeto-para-auxiliar-no-estudo
• https://www.tabnews.com.br/jackson541/models-e-consultas-em-django

[filipedeschamps]

Mesmo sabendo do problema da dependência citado ali em cima, como que a Vercel consegue instalar elas, fazer o build e deploy, e o CI não consegue, ainda mais usando o npm ci que usa o package-lock.json com esse problema de dependência resolvido?

[aprendendofelipe]

Mesmo sabendo do problema da dependência citado ali em cima, como que a Vercel consegue instalar elas, fazer o build e deploy, e o CI não consegue, ainda mais usando o npm ci que usa o package-lock.json com esse problema de dependência resolvido?

Boa pergunta. Algum bug do npm que só aparece em situações bem específicas.

Mas como contorno dessa situação, que tal retirar essa nextjs-progressbar? Ela é só um wrapper da nprogress. Podemos fazer o nosso wrapper.

Parece que a nextjs-progressbar também tem problemas com o React 18

[filipedeschamps]

Mas como contorno dessa situação, que tal retirar essa nextjs-progressbar?

Topo total! Vi o source aqui e parece ser bem simples, e por ser MIT podemos fazer a mesma coisa que foi feita no caso de limpar o markdown, que nesse caso seria ainda mais fácil porque não tem alteração, só apontar em um comentário o código oringial. Se você estiver disponível para mandar um commit nessa branch seria show 😍

[aprendendofelipe]

Já envio o commit... Só não é tão rápido pq o original está em TypeScript 😅

[filipedeschamps]

Já envio o commit... Só não é tão rápido pq o original está em TypeScript 😅

Eita é verdade 😂

[filipedeschamps]

@aprendendofelipe passou tudo e a maior delícia do nprogress é isso: "dependencies": {} 😍

[aprendendofelipe]

@filipedeschamps, por que esse limite de 16.000 caracteres pro body do conteúdo, se ele está no body da request e não no header?

Name	Limit
Maximum request body length	4 MiB
Maximum request headers length	16KiB

[filipedeschamps]

@aprendendofelipe então, eu não sei porque não vai com um body maior. Peço que me ajude a investigar isolando o teste do body que faz um POST com essa quantidade de caracteres. Tem tanto um teste fazendo POST quanto um PATCH e ambos quebravam com 20k caracteres. Eram os únicos testes que quebravam com o middleware.

[aprendendofelipe]

Só consegui confirmar, pelos testes, que o limite está em 16KiB mesmo. Se passar disso estoura o timeout de 60s. Amanhã investigo melhor

[filipedeschamps]

Depois do fetch com POST com body quebrar numa versão stable do middleware, eu não duvido ser mais uma regressão do framework.

[vercel]

@coffee-is-power is attempting to deploy a commit to the TabNews Team on Vercel.

To accomplish this, @coffee-is-power needs to request access to the Team.

Afterwards, an owner of the Team is required to accept their membership request.

If you're already a member of the respective Vercel Team, make sure that your Personal Vercel Account is connected to your GitHub account.

[coffeeispower]

Acho importante fazer o rate limit na rota de criar conteudos, porque alguem mal intencionado pode começar a spamar o tabnews com um monte de posts

[coffeeispower]

tem que ter um limite que só pode criar 1 post a cada 5 minutos ou algo assim, porque ninguem vai conseguir postar um monte de posts seguidos.

[coffeeispower]

E se for mesmo necessário, pode ter tambem uma permissão para burlar esse limite tambem.

[filipedeschamps]

Lançaram nova versão do Next e no release não encontrei o commit que faz o fix do problema anterior, mas tudo está funcionando como deveria: https://github.com/vercel/next.js/releases/tag/v12.2.5

[aprendendofelipe]

Depois do fetch com POST com body quebrar numa versão stable do middleware, eu não duvido ser mais uma regressão do framework.

Deve ser isso mesmo

Lançaram nova versão do Next e no release não encontrei o commit que faz o fix do problema anterior, mas tudo está funcionando como deveria: https://github.com/vercel/next.js/releases/tag/v12.2.5

Nos meus testes continua o limite de 16KiB na v12.2.5. A versão mais nova que consegui fazer funcionar sem esse limite foi a v12.2.2.

[filipedeschamps]

Nos meus testes continua o limite de 16KiB na v12.2.5. A versão mais nova que consegui fazer funcionar sem esse limite foi a v12.2.2.

Interessante. Você acha que vale a pena criar uma issue lá? Não vejo problema segurar esse PR até talvez eles lançarem um fix numa canary.

[aprendendofelipe]

Engraçado que no ambiente de homologação parece que está funcionando sim... Pelo menos está dando o aviso que passou dos 16000 caracteres

[filipedeschamps]

Engraçado que no ambiente de homologação parece que está funcionando sim... Pelo menos está dando o aviso que passou dos 16000 caracteres

Mas ele vai dar esse aviso, pois eu mudei a validação para 16000. Deixa eu fazer um commit de testes para voltar para 20k

[aprendendofelipe]

Mas ele vai dar esse aviso, pois eu mudei a validação para 16000. Deixa eu fazer um commit de testes para voltar para 20k

Isso, ia sugerir esse teste. No ambiente de testes não chega a dar esse aviso, pois a requisição fica sem nenhuma resposta

[filipedeschamps]

Turma, fiz um commit importante: d86deb9

1. Nos testes com as variáveis de ambiente, percebi que tinha um jeito de saber quando a request contra o /api/v1/sessions tinha caído no rate limit, que era bater no GET dessa mesma rota.
2. Como nós reimplementamos um controller fake, é necessário também reproduzir o mesmo comportamento do endpoint tanto para POST quanto para GET, mas que em nenhum momento isso gera novas requisições contra o banco de dados.
3. Então agora o GET verifica o formato do session_id (porque também daria para descobrir mandando um session_id no formato errado), e se essa validação estiver certa, daí o controller fake responde com a exata mesma mensagem que o controller real responderia.
4. Fora isso, é feito um log de erro quando a request cai nos controllers fake, pois sem isso não havia logs informando que alguém atingiu o rate limit.
5. Apesar de só estarmos com o rate limit para /api/v1/sessions, já aproveitei para implementar os logs nos endpoints que avisam que a pessoa esbarrou no rate limit.

Vejam se faz sentido 👍

[filipedeschamps]

Outra coisa que esqueci de avisar no commit d86deb9 é que era possível descobrir que a request caiu no rate limit mesmo com um POST ao enviar para ele parâmetros com formato errado, por exemplo o email num formato inválido.

Na implementação anterior do controller fake, ao enviar um email formatado errado e a request tivesse caído no rate limit, o controller respondia o padrão Dados não conferem. Verifique se os dados enviados estão corretos. Mas agora responde corretamente com a mensagem de erro de validação de email, assim como uma request verdadeira 👍

Alertas e Variáveis de Ambiente

O Axiom (sistema de logs que utilizamos) anunciou hoje que começou a receber os streams de logs vindos do Middleware da Vercel, porém nos meus testes aqui não está funcionando pela integração padrão fornecida no market place da Vercel e já notifiquei eles. Pelo menos, lá na Dashboard da Vercel os logs estão aparecendo como esperado. Assim que eles consertarem isso eu volto a testar os alertas.

Independente disso, testei tanto o caso das variáveis de ambiente do Upstash não existirem, quanto existirem mas as credenciais estarem erradas e em ambos os casos os logs foram registrados com sucesso (por enquanto só lá na Vercel). Um detalhe importante é que caso a conexão falhe com a Upstash, o fluxo cai no modo de exceção e a request é processada e devolvida normalmente.

Enquanto isso, configurei o cartão de crédito no Upstash e vou começar a subir os bancos e variáveis de ambiente finais 👍

Rebase

Vou fazer rebase dos commits desse PR e o histórico vai ser reescrito 🤝

[filipedeschamps]

Alerta disparando em Staging e usando limites customizados (que não ficam no source code) 😍

[filipedeschamps]

Merged!! Let's goooooo!!

Em paralelo, quando habilitarmos para todos os endpoints da API, sugiro lermos isso antes: https://docs.upstash.com/redis/features/globaldatabase

Hoje configurei um database global, mas talvez seja mais interessante configurar um em São Paulo também.

[aprendendofelipe]

Em paralelo, quando habilitarmos para todos os endpoints da API, sugiro lermos isso antes: https://docs.upstash.com/redis/features/globaldatabase

Hoje configurei um database global, mas talvez seja mais interessante configurar um em São Paulo também.

Mais um detalhe para analisar é que aqueles valores que eu citei anteriormente são para leituras. Gravações são bem mais caras

[aprendendofelipe]

Complementando... Aparentemente as gravações são mais caras que a leitura somente utilizando o database global.

Valores por 100 mil comandos

	Zona única	Zona múltipla	Global
Leitura	$0.2	$0.4	$0.4
Gravação	$0.2	$0.4	$1.0
Teto mensal	$120	$240	$360

Fonte: https://docs.upstash.com/redis/overall/pricing

[filipedeschamps]

@aprendendofelipe to pensando aqui se talvez faça mais sentido criar dois bancos: um em AWS SA-East-1 São Paulo e outro AWS US-East-1 North Virginia.

Em paralelo, não mudou nada na latência do endpoint de sessions, mas ele já é hiper devagar por natureza, então difícil ver o ponteiro se mexer nesse caso:

[aprendendofelipe]

@aprendendofelipe to pensando aqui se talvez faça mais sentido criar dois bancos: um em AWS SA-East-1 São Paulo e outro AWS US-East-1 North Virginia.

Para o caso específico de rate limite eu concordo! Se eu entendi direito como funciona a cobrança do uptash, e considerando que para o rate-limit não existe necessidade em manter replicação dos dados, me parece que vale sim mais a pena criar diferentes bancos Single Zone, cada um em diferentes regiões.

Mas agora que já está criado o banco Global, já vão ser cobrados os primeiros 100k, então eu deixaria um tempo rodando assim para levantar dados e comparar após mudar para Single Zone.

[aprendendofelipe]

não mudou nada na latência do endpoint de sessions, mas ele já é hiper devagar por natureza, então difícil ver o ponteiro se mexer nesse caso:

Pois é, com essa latência alta, se houvesse mudança visível no gráfico, algo de muito errado estaria ocorrendo... haha

Se der para filtrar por intervalos de tempo, talvez dê para pegar alguma mudança no tempo médio