-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: add TOTP two-factor authentication for dashboard login #8189
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
Open
Raven95676
wants to merge
8
commits into
AstrBotDevs:master
Choose a base branch
from
Raven95676:feat/totp
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
b7af106
feat: add TOTP two-factor authentication for dashboard login
Raven95676 d9b76b4
Merge remote-tracking branch 'upstream/master' into feat/totp
Raven95676 02e1323
fix: ensure TOTP verification uses UTC for accurate time comparison
Raven95676 e1d0501
fix: update recovery code validation logic for disabling TOTP
Raven95676 280f255
test: add unit tests for TOTP functionality and recovery code validation
Raven95676 0f6a059
chore: format
Raven95676 2d12187
feat: add trust_proxy_headers switch for auth rate-limit IP source
Raven95676 918edf3
feat: make dashboard auth rate-limit configurable via system settings
Raven95676 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,210 @@ | ||
| from __future__ import annotations | ||
|
|
||
| import asyncio | ||
| import base64 | ||
| import datetime | ||
| import hashlib | ||
| import hmac | ||
| import secrets | ||
|
|
||
| import pyotp | ||
| from sqlmodel import col, delete, select | ||
|
|
||
| from astrbot.core.db.po import DashboardTrustedDevice | ||
|
|
||
| TOTP_TRUSTED_DEVICE_COOKIE_NAME = "astrbot_totp_trusted_device" | ||
| TOTP_TRUSTED_DEVICE_MAX_AGE = 30 * 24 * 60 * 60 | ||
| RECOVERY_CODE_GROUP_COUNT = 4 | ||
| RECOVERY_CODE_GROUP_LENGTH = 8 | ||
| RECOVERY_CODE_LENGTH = RECOVERY_CODE_GROUP_COUNT * RECOVERY_CODE_GROUP_LENGTH | ||
| _RECOVERY_CODE_KDF_ITERATIONS = 600_000 | ||
| _RECOVERY_CODE_KDF_SALT_BYTES = 16 | ||
| _RECOVERY_CODE_KDF_ALGORITHM = "pbkdf2_sha256" | ||
|
|
||
| _last_totp_timecode: dict[str, int] = {} | ||
| _totp_replay_lock = asyncio.Lock() | ||
|
|
||
|
|
||
| def _get_totp_config(config) -> dict: | ||
| totp_config = config.get("dashboard", {}).get("totp", {}) | ||
| return totp_config if isinstance(totp_config, dict) else {} | ||
|
|
||
|
|
||
| def is_totp_enabled(config) -> bool: | ||
| """TOTP is fully configured and operational (enable + secret + recovery hash all present).""" | ||
| totp_config = _get_totp_config(config) | ||
| if not totp_config.get("enable", False): | ||
| return False | ||
| secret = totp_config.get("secret", "") | ||
| if not isinstance(secret, str) or not secret.strip(): | ||
| return False | ||
| recovery_code_hash = totp_config.get("recovery_code_hash", "") | ||
| if not isinstance(recovery_code_hash, str) or not recovery_code_hash.strip(): | ||
| return False | ||
| return True | ||
|
|
||
|
|
||
| def _get_verified_totp_timecode(secret: str, code: str) -> int | None: | ||
| code = code.strip() | ||
| try: | ||
| totp = pyotp.TOTP(secret.strip()) | ||
| now = datetime.datetime.now() | ||
|
Raven95676 marked this conversation as resolved.
Outdated
|
||
| for offset in (-1, 0, 1): | ||
| candidate_time = now + datetime.timedelta(seconds=offset * totp.interval) | ||
| if hmac.compare_digest(str(totp.at(candidate_time)), code): | ||
| return int(totp.timecode(candidate_time)) | ||
| except Exception: | ||
| return None | ||
| return None | ||
|
|
||
|
|
||
| async def consume_totp_code(secret: str, code: str) -> bool: | ||
| global _last_totp_timecode | ||
| timecode = _get_verified_totp_timecode(secret, code) | ||
| if timecode is None: | ||
| return False | ||
| secret = secret.strip() | ||
| async with _totp_replay_lock: | ||
| if _last_totp_timecode.get(secret, -1) >= timecode: | ||
| return False | ||
| _last_totp_timecode[secret] = timecode | ||
| return True | ||
|
|
||
|
|
||
| async def consume_configured_totp_code(config, code: str) -> bool: | ||
| if not is_totp_enabled(config): | ||
| return False | ||
| secret = _get_totp_config(config).get("secret", "") | ||
| return await consume_totp_code(secret, code) | ||
|
|
||
|
|
||
| def _hash_totp_trusted_device_token(config, token: str) -> str: | ||
| jwt_secret = config["dashboard"].get("jwt_secret", "") | ||
| if not isinstance(jwt_secret, str) or not jwt_secret: | ||
| return "" | ||
| return hmac.new( | ||
| jwt_secret.encode("utf-8"), | ||
| token.encode("utf-8"), | ||
| hashlib.sha256, | ||
| ).hexdigest() | ||
|
|
||
|
|
||
| def _hash_totp_secret(config) -> str: | ||
| secret = _get_totp_config(config).get("secret", "") | ||
| if not isinstance(secret, str) or not secret.strip(): | ||
| return "" | ||
| return hashlib.sha256(secret.strip().encode("utf-8")).hexdigest() | ||
|
|
||
|
|
||
| async def is_totp_trusted_device_valid(config, db, cookie_token: str) -> bool: | ||
| if not cookie_token: | ||
| return False | ||
| token_hash = _hash_totp_trusted_device_token(config, cookie_token) | ||
| totp_secret_hash = _hash_totp_secret(config) | ||
| if not token_hash or not totp_secret_hash: | ||
| return False | ||
|
|
||
| await _cleanup_expired_totp_trusted_devices(db) | ||
| async with db.get_db() as session: | ||
| result = await session.execute( | ||
| select(DashboardTrustedDevice).where( | ||
| col(DashboardTrustedDevice.token_hash) == token_hash, | ||
| col(DashboardTrustedDevice.totp_secret_hash) == totp_secret_hash, | ||
| col(DashboardTrustedDevice.expires_at) | ||
| > datetime.datetime.now(datetime.timezone.utc), | ||
| ) | ||
| ) | ||
| return result.scalar_one_or_none() is not None | ||
|
|
||
|
|
||
| async def issue_totp_trusted_device(config, db) -> str | None: | ||
| """Issue a trusted device token, save to DB, and return the raw token for cookie.""" | ||
| raw_token = secrets.token_urlsafe(48) | ||
| token_hash = _hash_totp_trusted_device_token(config, raw_token) | ||
| totp_secret_hash = _hash_totp_secret(config) | ||
| if not token_hash or not totp_secret_hash: | ||
| return None | ||
|
|
||
| expires_at = datetime.datetime.now(datetime.timezone.utc) + datetime.timedelta( | ||
| seconds=TOTP_TRUSTED_DEVICE_MAX_AGE | ||
| ) | ||
| async with db.get_db() as session: | ||
| async with session.begin(): | ||
| await session.execute( | ||
| delete(DashboardTrustedDevice).where( | ||
| col(DashboardTrustedDevice.token_hash) == token_hash | ||
| ) | ||
| ) | ||
| trusted_device = DashboardTrustedDevice.model_validate( | ||
| { | ||
| "token_hash": token_hash, | ||
| "totp_secret_hash": totp_secret_hash, | ||
| "expires_at": expires_at, | ||
| } | ||
| ) | ||
| session.add(trusted_device) | ||
| return raw_token | ||
|
|
||
|
|
||
| async def _cleanup_expired_totp_trusted_devices(db) -> None: | ||
| async with db.get_db() as session: | ||
| async with session.begin(): | ||
| await session.execute( | ||
| delete(DashboardTrustedDevice).where( | ||
| col(DashboardTrustedDevice.expires_at) | ||
| <= datetime.datetime.now(datetime.timezone.utc) | ||
| ) | ||
| ) | ||
|
|
||
|
|
||
| async def revoke_user_trusted_devices(db) -> None: | ||
| async with db.get_db() as session: | ||
| async with session.begin(): | ||
| await session.execute(delete(DashboardTrustedDevice)) | ||
|
|
||
|
|
||
| def generate_recovery_code() -> tuple[str, str]: | ||
| raw = secrets.token_bytes(20) | ||
| recovery_code = base64.b32encode(raw).decode("ascii").rstrip("=") | ||
| salt = secrets.token_hex(_RECOVERY_CODE_KDF_SALT_BYTES) | ||
| digest = hashlib.pbkdf2_hmac( | ||
| "sha256", | ||
| recovery_code.encode("utf-8"), | ||
| bytes.fromhex(salt), | ||
| _RECOVERY_CODE_KDF_ITERATIONS, | ||
| ).hex() | ||
| kdf_hash = f"{_RECOVERY_CODE_KDF_ALGORITHM}${_RECOVERY_CODE_KDF_ITERATIONS}${salt}${digest}" | ||
| parts = [ | ||
| recovery_code[i : i + RECOVERY_CODE_GROUP_LENGTH] | ||
| for i in range(0, len(recovery_code), RECOVERY_CODE_GROUP_LENGTH) | ||
| ] | ||
| return "-".join(parts), kdf_hash | ||
|
|
||
|
|
||
| def verify_recovery_code(config, code: str) -> bool: | ||
| """Verify a recovery code against configured recovery_code_hash (PBKDF2).""" | ||
| cleaned = "".join(char for char in code.upper() if char.isalnum()) | ||
| if len(cleaned) != RECOVERY_CODE_LENGTH: | ||
| return False | ||
| totp_config = _get_totp_config(config) | ||
| stored_hash = totp_config.get("recovery_code_hash", "") | ||
| if not isinstance(stored_hash, str) or not stored_hash: | ||
| return False | ||
|
|
||
| parts = stored_hash.split("$") | ||
| if len(parts) != 4 or parts[0] != _RECOVERY_CODE_KDF_ALGORITHM: | ||
| return False | ||
| try: | ||
| iterations = int(parts[1]) | ||
| salt = parts[2] | ||
| expected_digest = parts[3] | ||
| except (ValueError, IndexError): | ||
| return False | ||
|
|
||
| candidate = hashlib.pbkdf2_hmac( | ||
| "sha256", | ||
| cleaned.encode("utf-8"), | ||
| bytes.fromhex(salt), | ||
| iterations, | ||
| ).hex() | ||
| return hmac.compare_digest(candidate, expected_digest) | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (performance): Replay protection map
_last_totp_timecodecan grow unbounded over long uptime or many secrets.Because
_last_totp_timecodeis keyed by the raw secret and never cleaned up, it can grow indefinitely in long-lived processes or with frequent secret rotation. Consider adding a pruning strategy (e.g., when TOTP is disabled/rotated, or by keeping only the last N secrets) or keying by a stable hash and cleaning up when it changes, to prevent unbounded memory growth while preserving replay protection.Suggested implementation:
To fully implement this change and avoid unbounded growth:
Imports
astrbot/core/utils/totp.py, add:from collections import OrderedDictimport hashlibtyping.OrderedDictis used instead of the runtime class anywhere else, adjust the annotation accordingly:from collections.abc import MutableMappingorfrom typing import OrderedDictdepending on your typing style.Replace direct uses of
_last_totp_timecode_last_totp_timecodeis accessed directly with the raw secret as a key, change those usages to call the helpers:last = _last_totp_timecode.get(secret)(or similar) with:last = await _get_last_totp_timecode(secret)_last_totp_timecode[secret] = timecodeor mutations inside anasync with _totp_replay_lockblock with:await _update_last_totp_timecode(secret, timecode)_last_totp_timecodeusing_totp_replay_lock, remove the redundant lock usage around call sites of_get_last_totp_timecode/_update_last_totp_timecode, since the helpers already handle locking.Type annotations
int | None, replace it withOptional[int]and addfrom typing import Optional.These changes ensure the replay protection map is bounded in size, keyed by a stable hash instead of the raw secret, and automatically prunes old entries while preserving replay protection guarantees for recent/active secrets.