b1c78fe4c8
* fix(api): rate-limit magic-code verification and bound per-token attempts
The magic-link sign-in / sign-up endpoints accept a 6-digit numeric code
(900k-value space, 600s TTL) but never increment a failure counter on a
wrong-code verify and extend django.views.View rather than DRF APIView,
so DRF's AuthenticationThrottle never runs against them. The space-side
generate endpoint also lacked throttle_classes. Combined, this allowed
an unauthenticated attacker who knew a victim's email to brute-force
the code within the TTL window and log in as the victim.
- Add MAX_VERIFY_ATTEMPTS=5 in MagicCodeProvider.set_user_data: failed
comparisons now persist verify_attempts in Redis under the remaining
TTL and, on hitting the limit, delete the key and raise
EMAIL_CODE_ATTEMPT_EXHAUSTED. This is the load-bearing fix - it caps
total attempts per issued token regardless of request rate.
- Add authentication_throttle_allows() so plain Django Views can apply
AuthenticationThrottle without converting to APIView (would change
CSRF + request-parsing semantics for the redirect-flow endpoints).
- Apply the throttle to MagicSignIn/UpEndpoint and the space variants;
add throttle_classes to MagicGenerateSpaceEndpoint to match its app
sibling.
Refs GHSA-9pvm-fcf6-9234.
* fix(api): make verify-attempt increment atomic, expose throttle rate via env
Address PR review feedback:
- Replace the JSON read-modify-write of verify_attempts with a Lua
EVAL script that INCRs a dedicated counter key and EXPIREs it only
on the first increment. The previous round-trip was racy: parallel
wrong-code requests could read the same value and both write the
same incremented count, letting an attacker exceed MAX_VERIFY_ATTEMPTS
under concurrency. Counter is now reset on each new token issuance
and cleared on successful verify / exhaustion.
- Make AuthenticationThrottle.rate configurable via the
AUTHENTICATION_RATE_LIMIT env var (default 10/minute, down from 30
to tighten the budget on unauth auth-adjacent endpoints). Document
it in deployments/aio and deployments/cli variables.env.
* test(api): cover magic-code attempt cap, counter reset, and auth throttle
Add the contract tests called out in the PR test plan:
- TestMagicSignInVerifyAttempts:
- test_exhausted_after_max_wrong_attempts: after MAX_VERIFY_ATTEMPTS
wrong codes the next verify redirects with EMAIL_CODE_ATTEMPT_
EXHAUSTED_SIGN_IN and both Redis keys are deleted; a follow-up
verify reports EXPIRED.
- test_counter_increments_on_each_wrong_attempt: the dedicated
verify_attempts counter advances by exactly one per wrong POST,
matching the atomic Lua INCR.
- test_counter_resets_on_token_regeneration: regenerating the
magic-link clears the counter so the user isn't pre-locked-out by
a prior session's wrong attempts.
- TestMagicSignUpVerifyAttempts.test_signup_exhausted_after_max_wrong_attempts:
the sign-up endpoint returns EMAIL_CODE_ATTEMPT_EXHAUSTED_SIGN_UP on
the exhausting attempt.
- TestAuthenticationThrottle: exercises authentication_throttle_allows
on the plain-View redirect-flow endpoints by patching the rate down
and asserting RATE_LIMIT_EXCEEDED is appended to the redirect URL
once the per-IP budget is exceeded, for both magic-sign-in and
magic-sign-up.
Each new class clears Django cache (DRF throttle storage) and the
per-email Redis keys around every test so runs are independent.
* fix(api): clamp remaining_ttl to >=1 for verify-attempt counter EXPIRE
ri.ttl() returns 0 when the token has less than one second remaining
(Redis floors to whole seconds). The previous clamp only caught
None and < 0, so a sub-second TTL would pass through and the Lua
script's EXPIRE counter 0 would immediately delete the key — letting
an attacker bypass MAX_VERIFY_ATTEMPTS during the final second of the
token's life. Switch the comparison to <= 0.
Narrow real-world impact (sub-second window, throttle still bounds
the rate) but the cap should hold regardless of timing.
211 lines
8.6 KiB
Python
211 lines
8.6 KiB
Python
# Copyright (c) 2023-present Plane Software, Inc. and contributors
|
|
# SPDX-License-Identifier: AGPL-3.0-only
|
|
# See the LICENSE file for details.
|
|
|
|
# Python imports
|
|
import json
|
|
import os
|
|
import secrets
|
|
|
|
|
|
# Module imports
|
|
from plane.authentication.adapter.credential import CredentialAdapter
|
|
from plane.license.utils.instance_value import get_configuration_value
|
|
from plane.settings.redis import redis_instance
|
|
from plane.authentication.adapter.error import (
|
|
AUTHENTICATION_ERROR_CODES,
|
|
AuthenticationException,
|
|
)
|
|
from plane.db.models import User
|
|
|
|
|
|
class MagicCodeProvider(CredentialAdapter):
|
|
provider = "magic-code"
|
|
|
|
# Max wrong-code verification attempts per issued token before the token
|
|
# is invalidated. Prevents brute-forcing the 6-digit code space within
|
|
# the token TTL window.
|
|
MAX_VERIFY_ATTEMPTS = 5
|
|
|
|
# Atomic INCR + first-time EXPIRE for the verify-attempt counter.
|
|
# Using a dedicated counter key with this script makes the increment
|
|
# safe under concurrent wrong-code requests; a plain JSON read/modify/
|
|
# write would race and let parallel attackers exceed the cap.
|
|
_INCREMENT_VERIFY_ATTEMPTS_SCRIPT = (
|
|
'local count = redis.call("INCR", KEYS[1]) '
|
|
'if count == 1 then '
|
|
' redis.call("EXPIRE", KEYS[1], tonumber(ARGV[1])) '
|
|
'end '
|
|
'return count'
|
|
)
|
|
|
|
@staticmethod
|
|
def _verify_attempts_key(token_key):
|
|
return f"{token_key}:verify_attempts"
|
|
|
|
def __init__(self, request, key, code=None, callback=None):
|
|
(EMAIL_HOST, ENABLE_MAGIC_LINK_LOGIN) = get_configuration_value(
|
|
[
|
|
{"key": "EMAIL_HOST", "default": os.environ.get("EMAIL_HOST")},
|
|
{
|
|
"key": "ENABLE_MAGIC_LINK_LOGIN",
|
|
"default": os.environ.get("ENABLE_MAGIC_LINK_LOGIN", "1"),
|
|
},
|
|
]
|
|
)
|
|
|
|
if not (EMAIL_HOST):
|
|
raise AuthenticationException(
|
|
error_code=AUTHENTICATION_ERROR_CODES["SMTP_NOT_CONFIGURED"],
|
|
error_message="SMTP_NOT_CONFIGURED",
|
|
payload={"email": str(key)},
|
|
)
|
|
|
|
if ENABLE_MAGIC_LINK_LOGIN == "0":
|
|
raise AuthenticationException(
|
|
error_code=AUTHENTICATION_ERROR_CODES["MAGIC_LINK_LOGIN_DISABLED"],
|
|
error_message="MAGIC_LINK_LOGIN_DISABLED",
|
|
payload={"email": str(key)},
|
|
)
|
|
|
|
super().__init__(request=request, provider=self.provider, callback=callback)
|
|
self.key = key
|
|
self.code = code
|
|
|
|
def initiate(self):
|
|
## Generate a random token
|
|
token = str(secrets.randbelow(900000) + 100000)
|
|
|
|
ri = redis_instance()
|
|
|
|
key = "magic_" + str(self.key)
|
|
|
|
# Check if the key already exists in python
|
|
if ri.exists(key):
|
|
data = json.loads(ri.get(key))
|
|
|
|
current_attempt = data["current_attempt"] + 1
|
|
|
|
if data["current_attempt"] > 2:
|
|
email = str(self.key).replace("magic_", "", 1)
|
|
if User.objects.filter(email=email).exists():
|
|
raise AuthenticationException(
|
|
error_code=AUTHENTICATION_ERROR_CODES["EMAIL_CODE_ATTEMPT_EXHAUSTED_SIGN_IN"],
|
|
error_message="EMAIL_CODE_ATTEMPT_EXHAUSTED_SIGN_IN",
|
|
payload={"email": str(email)},
|
|
)
|
|
else:
|
|
raise AuthenticationException(
|
|
error_code=AUTHENTICATION_ERROR_CODES["EMAIL_CODE_ATTEMPT_EXHAUSTED_SIGN_UP"],
|
|
error_message="EMAIL_CODE_ATTEMPT_EXHAUSTED_SIGN_UP",
|
|
payload={"email": self.key},
|
|
)
|
|
|
|
value = {
|
|
"current_attempt": current_attempt,
|
|
"email": str(self.key),
|
|
"token": token,
|
|
}
|
|
expiry = 600
|
|
ri.set(key, json.dumps(value), ex=expiry)
|
|
else:
|
|
value = {"current_attempt": 0, "email": self.key, "token": token}
|
|
expiry = 600
|
|
|
|
ri.set(key, json.dumps(value), ex=expiry)
|
|
# Reset the verify-attempt counter so each newly issued token starts
|
|
# with a fresh budget of MAX_VERIFY_ATTEMPTS.
|
|
ri.delete(self._verify_attempts_key(key))
|
|
return key, token
|
|
|
|
def set_user_data(self):
|
|
ri = redis_instance()
|
|
if ri.exists(self.key):
|
|
data = json.loads(ri.get(self.key))
|
|
token = data["token"]
|
|
email = data["email"]
|
|
|
|
if str(token) == str(self.code):
|
|
super().set_user_data(
|
|
{
|
|
"email": email,
|
|
"user": {
|
|
"avatar": "",
|
|
"first_name": "",
|
|
"last_name": "",
|
|
"provider_id": "",
|
|
"is_password_autoset": True,
|
|
},
|
|
}
|
|
)
|
|
# Delete the token and its counter from redis on success.
|
|
ri.delete(self.key)
|
|
ri.delete(self._verify_attempts_key(self.key))
|
|
return
|
|
else:
|
|
email = str(self.key).replace("magic_", "", 1)
|
|
user_exists = User.objects.filter(email=email).exists()
|
|
|
|
# Atomically increment the verify-attempt counter in Redis.
|
|
# The Lua script sets the TTL only on the first increment so
|
|
# the lockout window matches the remaining token TTL and does
|
|
# not get extended by every wrong-code attempt.
|
|
# ri.ttl() returns -2 (missing), -1 (no expiry), 0 (sub-second
|
|
# remaining; Redis floors to whole seconds), or a positive int.
|
|
# Clamp to >=1 because EXPIRE key 0 immediately deletes the key
|
|
# and would let an attacker bypass the cap in the final second.
|
|
remaining_ttl = ri.ttl(self.key)
|
|
if remaining_ttl is None or remaining_ttl <= 0:
|
|
remaining_ttl = 1
|
|
verify_attempts = int(
|
|
ri.eval(
|
|
self._INCREMENT_VERIFY_ATTEMPTS_SCRIPT,
|
|
1,
|
|
self._verify_attempts_key(self.key),
|
|
remaining_ttl,
|
|
)
|
|
)
|
|
|
|
if verify_attempts >= self.MAX_VERIFY_ATTEMPTS:
|
|
# Invalidate the token (and counter) so further attempts
|
|
# must regenerate; regeneration is itself attempt-counted.
|
|
ri.delete(self.key)
|
|
ri.delete(self._verify_attempts_key(self.key))
|
|
if user_exists:
|
|
raise AuthenticationException(
|
|
error_code=AUTHENTICATION_ERROR_CODES["EMAIL_CODE_ATTEMPT_EXHAUSTED_SIGN_IN"],
|
|
error_message="EMAIL_CODE_ATTEMPT_EXHAUSTED_SIGN_IN",
|
|
payload={"email": str(email)},
|
|
)
|
|
raise AuthenticationException(
|
|
error_code=AUTHENTICATION_ERROR_CODES["EMAIL_CODE_ATTEMPT_EXHAUSTED_SIGN_UP"],
|
|
error_message="EMAIL_CODE_ATTEMPT_EXHAUSTED_SIGN_UP",
|
|
payload={"email": str(email)},
|
|
)
|
|
|
|
if user_exists:
|
|
raise AuthenticationException(
|
|
error_code=AUTHENTICATION_ERROR_CODES["INVALID_MAGIC_CODE_SIGN_IN"],
|
|
error_message="INVALID_MAGIC_CODE_SIGN_IN",
|
|
payload={"email": str(email)},
|
|
)
|
|
else:
|
|
raise AuthenticationException(
|
|
error_code=AUTHENTICATION_ERROR_CODES["INVALID_MAGIC_CODE_SIGN_UP"],
|
|
error_message="INVALID_MAGIC_CODE_SIGN_UP",
|
|
payload={"email": str(email)},
|
|
)
|
|
else:
|
|
email = str(self.key).replace("magic_", "", 1)
|
|
if User.objects.filter(email=email).exists():
|
|
raise AuthenticationException(
|
|
error_code=AUTHENTICATION_ERROR_CODES["EXPIRED_MAGIC_CODE_SIGN_IN"],
|
|
error_message="EXPIRED_MAGIC_CODE_SIGN_IN",
|
|
payload={"email": str(email)},
|
|
)
|
|
else:
|
|
raise AuthenticationException(
|
|
error_code=AUTHENTICATION_ERROR_CODES["EXPIRED_MAGIC_CODE_SIGN_UP"],
|
|
error_message="EXPIRED_MAGIC_CODE_SIGN_UP",
|
|
payload={"email": str(email)},
|
|
)
|