Skip to content

Limiter le nombre d’actions par minute pour les utilisateurs authentifiés - #6327

Merged
francoisfreitag merged 1 commit into
masterfrom
ff/rate-limit
Jun 26, 2025
Merged

Limiter le nombre d’actions par minute pour les utilisateurs authentifiés#6327
francoisfreitag merged 1 commit into
masterfrom
ff/rate-limit

Conversation

@francoisfreitag

@francoisfreitag francoisfreitag commented Jun 12, 2025

Copy link
Copy Markdown
Member

🤔 Pourquoi ?

Sécurité : mieux identifier et bloquer les énumérations.

🍰 Comment ?

Afin de pouvoir effectuer un rendu complet de la page de 429, il faut que le middleware soit placé après ItouCurrentOrganizationMiddleware.

🏝️ Comment tester ?

  • Se connecter
  • Beaucoup appuyer sur la touche

Le testeur malin changera la limite à une requête par minute.

💻 Captures d'écran

image

Comment thread itou/utils/throttling.py
from rest_framework import throttling


class FailSafeUserRateThrottle(throttling.UserRateThrottle):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Je suis un peu chagrin par l'implémentation de DRF qui porte bien son nom SimpleRateThrottle 😬
Mais bon elle a le mérite d'être simple à lire et sûrement suffisant pour notre besoin.
Est-ce qu'on a confiance en notre redis ? Et si ce n'est pas le cas, est-ce qu'il ne faudrait pas l'activer progressivement en surchargeant get_cache_key et en renvoyant None pour 90% des utilisateurs puis 80%, etc ?

@francoisfreitag francoisfreitag Jun 23, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

On utilise le failsafe. S’il tombe, on laissera passer toutes les requêtes. Bon, les tâches async en revanche... 🙈

Je viens de tester, le script suivant utilise 185 MB de RAM (RSS) (d’après l’output de INFO MEMORY).

import random
from datetime import datetime

import django


def main():
    from django.core.cache import caches

    cache = caches["failsafe"]

    now_ts = datetime.now().timestamp()
    for i in range(1_000_000):
        cache.set(i, [now_ts] * random.randint(1, 10), 3600)


if __name__ == "__main__":
    django.setup()
    main()

Notre redis (taille S) a 100 MB d’espace disque, on pourrait le passer en L pour avoir 500 MB. On passerait à 20 € par mois.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Après une petite analyse datadog, on a max 500 utilisateurs concurrents sur une minute. Donc stocker leur historique (même s’il fait 60 requêtes) ne posera pas de soucis.

Un test plus réaliste montre 15MB used_memory_rss_human (2 MB en used_memory_human).

@celine-m-s

Copy link
Copy Markdown
Contributor

C'est super !
Je me demande si on ne devrait pas ajouter un cas pour les requêtes HTMX car une requête par seconde, ce n'est pas si rapide que ça avec de l'AJAX.

@francoisfreitag

Copy link
Copy Markdown
Member Author

Je me demande si on ne devrait pas ajouter un cas pour les requêtes HTMX car une requête par seconde, ce n'est pas si rapide que ça avec de l'AJAX.

La limite de 60/minute est exprimée en minutes pour laisser passer un chargement de resources qui enverrait plusieurs requêtes en moins d’une seconde. Par exemple, la vue GEIQ du bilan d’exécution fait une cascade de 4 ou 5 appels HTMX au premier chargement. J’imagine qu’en filtrant les candidatures, on peut facilement déclencher une dizaine de requêtes en peu de temps.

On peut suivre le nombre de 429 suite au déploiement et vérifier qu’on n’impacte pas trop les utilisateurs, et décider de l’augmenter si besoin.

@tonial tonial left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ça me parait très bien comme solution.

On pourra toujours améliorer dans le futur si c'est nécessaire 🤷

Helps identifying and preventing enumeration.

In order to render a complete menu, the RateLimitMiddleware must be
placed after the ItouCurrentOrganizationMiddleware.
@francoisfreitag
francoisfreitag added this pull request to the merge queue Jun 26, 2025
Merged via the queue into master with commit 7288e8b Jun 26, 2025
@francoisfreitag
francoisfreitag deleted the ff/rate-limit branch June 26, 2025 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants