diff --git a/auth/auth_server/controllers/base.py b/auth/auth_server/controllers/base.py index ee58adf2f..7068b18d1 100644 --- a/auth/auth_server/controllers/base.py +++ b/auth/auth_server/controllers/base.py @@ -8,7 +8,8 @@ from auth.auth_server.exceptions import Err from auth.auth_server.models.models import (Token, Type, User, Role, - PermissionKeys, VerificationCode) + PermissionKeys, VerificationCode, + VerificationCodePurpose) from auth.auth_server.auth_token.token_store import TokenStore from auth.auth_server.utils import Config, popkey, get_digest from tools.optscale_exceptions.common_exc import (WrongArgumentsException, @@ -307,13 +308,15 @@ def nested_dict_iter(nested): nested_dict_iter(downward_hierarchy) return res - def use_verification_code(self, email, code): + def use_verification_code(self, email, code, + purpose=VerificationCodePurpose.password): if not email or not code: return now = opttime.utcnow() return self.session.query(VerificationCode).filter( and_( VerificationCode.email == email, + VerificationCode.purpose == VerificationCodePurpose(purpose), VerificationCode.deleted.is_(False), VerificationCode.code == get_digest(str(code)), VerificationCode.valid_until > now diff --git a/auth/auth_server/controllers/verification_code.py b/auth/auth_server/controllers/verification_code.py index b26f0722b..5b46e5a30 100644 --- a/auth/auth_server/controllers/verification_code.py +++ b/auth/auth_server/controllers/verification_code.py @@ -1,20 +1,40 @@ import logging +import secrets from datetime import timedelta +from functools import cached_property + +import requests +import tornado.web from sqlalchemy import and_, exists from sqlalchemy.exc import IntegrityError from auth.auth_server.controllers.base import BaseController -from auth.auth_server.models.models import VerificationCode +from auth.auth_server.models.models import (VerificationCode, + VerificationCodePurpose) from auth.auth_server.controllers.base_async import BaseAsyncControllerWrapper from auth.auth_server.exceptions import Err from auth.auth_server.models.models import User from auth.auth_server.utils import get_digest +from optscale_client.herald_client.client_v2 import Client as HeraldClient from tools.optscale_exceptions.common_exc import (WrongArgumentsException, ForbiddenException) from tools.optscale_time import utcnow LOG = logging.getLogger(__name__) VERIFICATION_CODE_LIFETIME_HRS = 1 +MFA_CODE_LIFETIME_MIN = 10 GENERATION_THRESHOLD_MIN = 1 +MFA_CODE_LENGTH = 6 +MFA_CODE_TEMPLATE = 'mfa_code' + + +def get_code_lifetime(purpose): + if VerificationCodePurpose(purpose) == VerificationCodePurpose.mfa: + return timedelta(minutes=MFA_CODE_LIFETIME_MIN) + return timedelta(hours=VERIFICATION_CODE_LIFETIME_HRS) + + +def generate_code(): + return f'{secrets.randbelow(10 ** MFA_CODE_LENGTH):0{MFA_CODE_LENGTH}d}' class VerificationCodeController(BaseController): @@ -42,45 +62,57 @@ def _check_input(self, **input_): self._check_generation_timeout(email) return self._check_user(email) - def _check_generation_timeout(self, email): + def _check_generation_timeout( + self, email, purpose=VerificationCodePurpose.password): model = self._get_model_type() timeout = utcnow() - timedelta( minutes=GENERATION_THRESHOLD_MIN) code_exists = self.session.query(exists().where(and_( model.email == email, + model.purpose == VerificationCodePurpose(purpose), model.created_at >= int(timeout.timestamp()) ))).scalar() if code_exists: raise ForbiddenException(Err.OA0072, []) def create(self, **kwargs): + # the public (cluster secret) endpoint only issues recovery codes, + # MFA codes are generated and sent by auth itself (send_mfa_code) + if 'purpose' in kwargs: + raise WrongArgumentsException(Err.OA0022, ['purpose']) self.check_create_restrictions(**kwargs) self._check_input(**kwargs) return self.create_verification_code(**kwargs) - def _invalidate_verification_codes(self, email, deleted_at): + def _invalidate_verification_codes( + self, email, deleted_at, + purpose=VerificationCodePurpose.password): model = self._get_model_type() self.session.query(model).filter( model.email == email, + model.purpose == VerificationCodePurpose(purpose), model.deleted.is_(False) ).update({ model.deleted_at: deleted_at }) - def create_verification_code(self, email, code): + def create_verification_code(self, email, code, + purpose=VerificationCodePurpose.password): model_type = self._get_model_type() - LOG.info("Creating %s for %s", model_type.__name__, email) + purpose = VerificationCodePurpose(purpose) + LOG.info("Creating %s (%s) for %s", model_type.__name__, + purpose.value, email) now = utcnow() now_ts = int(now.timestamp()) params = { 'email': email, 'created_at': now_ts, - 'valid_until': now + timedelta( - hours=VERIFICATION_CODE_LIFETIME_HRS), + 'valid_until': now + get_code_lifetime(purpose), 'code': get_digest(str(code)), + 'purpose': purpose, } verification_code = model_type(**params) - self._invalidate_verification_codes(email, now_ts) + self._invalidate_verification_codes(email, now_ts, purpose) self.session.add(verification_code) try: self.session.commit() @@ -88,6 +120,41 @@ def create_verification_code(self, email, code): raise WrongArgumentsException(Err.OA0061, [str(ex)]) return verification_code + @cached_property + def herald_client(self): + return HeraldClient(url=self._config.herald_url(), + secret=self._config.cluster_secret()) + + def send_mfa_code(self, user): + """ + Generates an MFA code for the user, stores its digest (purpose mfa, + previous MFA codes invalidated) and sends it by e-mail via herald. + Raises ForbiddenException (OA0072) if a code was generated less than + a minute ago and HTTPError 503 if herald is unavailable. + The code itself is never logged. + """ + purpose = VerificationCodePurpose.mfa + self._check_generation_timeout(user.email, purpose) + code = generate_code() + verification_code = self.create_verification_code( + user.email, code, purpose) + try: + self.herald_client.email_send( + [user.email], + f'{self._config.product_name()} verification code', + template_type=MFA_CODE_TEMPLATE, + template_params={'texts': {'code': code}}) + except requests.exceptions.RequestException as exc: + LOG.error('Failed to send MFA code to user %s: %s', + user.id, type(exc).__name__) + # drop the undelivered code so the user can request a new one + # without waiting for the generation threshold + self.session.delete(verification_code) + self.session.commit() + raise tornado.web.HTTPError(503, reason='Service Unavailable') + LOG.info('MFA code sent to user %s', user.id) + return verification_code + class VerificationCodeAsyncController(BaseAsyncControllerWrapper): def _get_controller_class(self): diff --git a/auth/auth_server/tests/unittests/test_mfa_code.py b/auth/auth_server/tests/unittests/test_mfa_code.py new file mode 100644 index 000000000..01c8ea619 --- /dev/null +++ b/auth/auth_server/tests/unittests/test_mfa_code.py @@ -0,0 +1,215 @@ +import logging +from datetime import timedelta +from unittest.mock import MagicMock, patch + +import requests +import tornado.web +from freezegun import freeze_time + +from tools.optscale_exceptions.common_exc import ForbiddenException +from tools.optscale_time import utcnow +from auth.auth_server.controllers.verification_code import ( + VerificationCodeController) +from auth.auth_server.models.models import (VerificationCode, + VerificationCodePurpose) +from auth.auth_server.tests.unittests.test_api_base import TestAuthBase +from auth.auth_server.utils import get_digest + +HERALD_CLIENT = ('auth.auth_server.controllers.verification_code.' + 'HeraldClient') +LOGGER = 'auth.auth_server.controllers.verification_code' + + +class TestMfaCode(TestAuthBase): + def setUp(self, version='v2'): + super().setUp(version) + self.password = 'toor' + self.user = self.create_root_user(password=self.password) + self.config = MagicMock() + self.config.herald_url.return_value = 'http://herald' + self.config.cluster_secret.return_value = 'secret' + self.config.product_name.return_value = 'FinOps for Cloud' + self.herald_cls = patch(HERALD_CLIENT).start() + self.herald = self.herald_cls.return_value + + @property + def controller(self): + return VerificationCodeController(self.db_session, self.config) + + def _sent_code(self, call_idx=-1): + kwargs = self.herald.email_send.call_args_list[call_idx].kwargs + return kwargs['template_params']['texts']['code'] + + def _codes(self, purpose=None): + query = self.db_session.query(VerificationCode).filter( + VerificationCode.email == self.user.email) + if purpose: + query = query.filter(VerificationCode.purpose == purpose) + return query.all() + + def _create_password_code(self, code): + return self.controller.create_verification_code( + self.user.email, code) + + def test_send_mfa_code(self): + now = utcnow().replace(microsecond=0) + with freeze_time(now): + result = self.controller.send_mfa_code(self.user) + self.herald_cls.assert_called_once_with(url='http://herald', + secret='secret') + self.herald.email_send.assert_called_once() + args, kwargs = self.herald.email_send.call_args + self.assertEqual(args, ([self.user.email], + 'FinOps for Cloud verification code')) + self.assertEqual(kwargs['template_type'], 'mfa_code') + code = self._sent_code() + self.assertRegex(code, r'^\d{6}$') + self.assertEqual(result.code, get_digest(code)) + self.assertEqual(result.purpose, VerificationCodePurpose.mfa) + self.assertEqual(result.valid_until, now + timedelta(minutes=10)) + + def test_password_code_lifetime(self): + now = utcnow().replace(microsecond=0) + with freeze_time(now): + result = self._create_password_code('123456') + self.assertEqual(result.purpose, VerificationCodePurpose.password) + self.assertEqual(result.valid_until, now + timedelta(hours=1)) + + def test_mfa_code_lifetime(self): + now = utcnow() + with freeze_time(now): + self.controller.send_mfa_code(self.user) + code = self._sent_code() + with freeze_time(now + timedelta(minutes=10, seconds=1)): + self.assertFalse(self.controller.use_verification_code( + self.user.email, code, VerificationCodePurpose.mfa)) + with freeze_time(now + timedelta(minutes=9, seconds=59)): + self.assertTrue(self.controller.use_verification_code( + self.user.email, code, VerificationCodePurpose.mfa)) + + def test_password_code_lifetime_usage(self): + now = utcnow() + with freeze_time(now): + self._create_password_code('123456') + with freeze_time(now + timedelta(minutes=59)): + self.assertTrue(self.controller.use_verification_code( + self.user.email, '123456')) + + def test_mfa_code_rejected_as_password_code(self): + self.controller.send_mfa_code(self.user) + code = self._sent_code() + self.assertFalse(self.controller.use_verification_code( + self.user.email, code)) + self.assertFalse(self.controller.use_verification_code( + self.user.email, code, VerificationCodePurpose.password)) + self.assertTrue(self.controller.use_verification_code( + self.user.email, code, VerificationCodePurpose.mfa)) + + def test_password_code_rejected_for_mfa(self): + self._create_password_code('123456') + self.assertFalse(self.controller.use_verification_code( + self.user.email, '123456', VerificationCodePurpose.mfa)) + self.assertTrue(self.controller.use_verification_code( + self.user.email, '123456')) + + def test_mfa_code_cannot_obtain_token(self): + self.controller.send_mfa_code(self.user) + code = self._sent_code() + self.client.secret = None + code_, resp = self.client.post('tokens', { + 'email': self.user.email, 'verification_code': code}) + self.assertEqual(code_, 403) + self.assertEqual(resp['error']['error_code'], 'OA0071') + + def test_generation_threshold(self): + now = utcnow() + with freeze_time(now): + self.controller.send_mfa_code(self.user) + first = self._codes(VerificationCodePurpose.mfa)[0] + with freeze_time(now + timedelta(seconds=59)): + with self.assertRaises(ForbiddenException) as ctx: + self.controller.send_mfa_code(self.user) + self.assertEqual(ctx.exception.error_code, 'OA0072') + self.assertEqual(self.herald.email_send.call_count, 1) + with freeze_time(now + timedelta(minutes=1, seconds=1)): + second = self.controller.send_mfa_code(self.user) + self.assertEqual(self.herald.email_send.call_count, 2) + self.db_session.refresh(first) + self.assertNotEqual(first.deleted_at, 0) + self.assertEqual(second.deleted_at, 0) + # the first code no longer works, the second one does + with freeze_time(now + timedelta(minutes=1, seconds=2)): + self.assertFalse(self.controller.use_verification_code( + self.user.email, self._sent_code(0), + VerificationCodePurpose.mfa)) + self.assertTrue(self.controller.use_verification_code( + self.user.email, self._sent_code(1), + VerificationCodePurpose.mfa)) + + def test_purposes_are_independent(self): + now = utcnow() + with freeze_time(now): + password_code = self._create_password_code('123456') + # a fresh password code neither throttles nor invalidates MFA + mfa_code = self.controller.send_mfa_code(self.user) + self.db_session.refresh(password_code) + self.assertEqual(password_code.deleted_at, 0) + with freeze_time(now + timedelta(minutes=1, seconds=1)): + # a new password code invalidates only the previous password code + code, _ = self.client.verification_code_create( + self.user.email, '654321') + self.assertEqual(code, 201) + self.db_session.refresh(mfa_code) + self.db_session.refresh(password_code) + self.assertEqual(mfa_code.deleted_at, 0) + self.assertNotEqual(password_code.deleted_at, 0) + + def test_mfa_code_does_not_throttle_password_code(self): + now = utcnow() + with freeze_time(now): + self.controller.send_mfa_code(self.user) + with freeze_time(now + timedelta(seconds=10)): + code, _ = self.client.verification_code_create( + self.user.email, '123456') + self.assertEqual(code, 201) + + def test_herald_failure(self): + self.herald.email_send.side_effect = requests.exceptions.HTTPError( + response=MagicMock(status_code=500)) + with self.assertLogs(LOGGER, level=logging.INFO) as logs: + with self.assertRaises(tornado.web.HTTPError) as ctx: + self.controller.send_mfa_code(self.user) + self.assertEqual(ctx.exception.status_code, 503) + code = self._sent_code() + self.assertFalse(any(code in line for line in logs.output)) + # undelivered code is dropped, a new one can be requested at once + self.assertEqual(self._codes(VerificationCodePurpose.mfa), []) + self.herald.email_send.side_effect = None + self.controller.send_mfa_code(self.user) + self.assertEqual(len(self._codes(VerificationCodePurpose.mfa)), 1) + + def test_herald_connection_error(self): + self.herald.email_send.side_effect = ( + requests.exceptions.ConnectionError()) + with self.assertRaises(tornado.web.HTTPError) as ctx: + self.controller.send_mfa_code(self.user) + self.assertEqual(ctx.exception.status_code, 503) + + def test_code_never_logged(self): + with self.assertLogs(level=logging.DEBUG) as logs: + self.controller.send_mfa_code(self.user) + code = self._sent_code() + self.assertFalse(any(code in line for line in logs.output)) + + def test_api_create_rejects_purpose(self): + code, resp = self.client.post('verification_codes', { + 'email': self.user.email, 'code': '123456', 'purpose': 'mfa'}) + self.assertEqual(code, 400) + self.assertEqual(resp['error']['error_code'], 'OA0022') + self.assertEqual(self._codes(), []) + + def test_api_create_password_purpose(self): + code, resp = self.client.verification_code_create( + self.user.email, '123456') + self.assertEqual(code, 201) + self.assertEqual(resp['purpose'], 'password') diff --git a/auth/pyproject.toml b/auth/pyproject.toml index 91850a2e6..2890eb462 100644 --- a/auth/pyproject.toml +++ b/auth/pyproject.toml @@ -8,6 +8,7 @@ dependencies = [ "config-client", "cryptography==44.0.1", "google-auth==1.23.0", + "herald-client", "mysql-connector-python==9.1.0", "optscale-exceptions", "optscale-password", @@ -37,6 +38,7 @@ optscale-exceptions = { path = "../tools/optscale_exceptions" } optscale-time = { path = "../tools/optscale_time" } optscale-password = { path = "../tools/optscale_password" } config-client = { path = "../optscale_client/config_client" } +herald-client = { path = "../optscale_client/herald_client" } restapi-client = { path = "../optscale_client/rest_api_client" } auth-client = { path = "../optscale_client/auth_client" } check-alembic-down-revisions = { path = "../tools/check_alembic_down_revisions" } diff --git a/auth/uv.lock b/auth/uv.lock index 21fc866af..9a6814b2c 100644 --- a/auth/uv.lock +++ b/auth/uv.lock @@ -47,6 +47,7 @@ dependencies = [ { name = "config-client" }, { name = "cryptography" }, { name = "google-auth" }, + { name = "herald-client" }, { name = "mysql-connector-python" }, { name = "optscale-exceptions" }, { name = "optscale-password" }, @@ -82,6 +83,7 @@ requires-dist = [ { name = "config-client", directory = "../optscale_client/config_client" }, { name = "cryptography", specifier = "==44.0.1" }, { name = "google-auth", specifier = "==1.23.0" }, + { name = "herald-client", directory = "../optscale_client/herald_client" }, { name = "mysql-connector-python", specifier = "==9.1.0" }, { name = "optscale-exceptions", directory = "../tools/optscale_exceptions" }, { name = "optscale-password", directory = "../tools/optscale_password" }, @@ -403,6 +405,21 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/ad/e7/d6914822c88aa2974dbbd10903d801a28a19ce9cd8bad7e694cbbcf61528/grpcio-1.78.0-cp312-cp312-win_amd64.whl", hash = "sha256:9566fe4ababbb2610c39190791e5b829869351d14369603702e890ef3ad2d06e", size = 4797657, upload-time = "2026-02-06T09:55:49.86Z" }, ] +[[package]] +name = "herald-client" +version = "0.0.0" +source = { directory = "../optscale_client/herald_client" } +dependencies = [ + { name = "requests" }, + { name = "retrying" }, +] + +[package.metadata] +requires-dist = [ + { name = "requests", specifier = "==2.32.4" }, + { name = "retrying", specifier = "==1.4.1" }, +] + [[package]] name = "idna" version = "3.11" diff --git a/rest_api/rest_api_server/controllers/restore_password.py b/rest_api/rest_api_server/controllers/restore_password.py index 5237158b9..708f5bc72 100644 --- a/rest_api/rest_api_server/controllers/restore_password.py +++ b/rest_api/rest_api_server/controllers/restore_password.py @@ -1,6 +1,6 @@ import logging import requests -from random import randrange +import secrets from rest_api.rest_api_server.controllers.base import BaseController from rest_api.rest_api_server.controllers.base_async import BaseAsyncControllerWrapper from rest_api.rest_api_server.utils import query_url @@ -20,10 +20,7 @@ def restore_password(self, email, **kwargs): @staticmethod def _generate_code(): - ints = [] - for _ in range(6): - ints.append(str(randrange(10))) - return ''.join(ints) + return f'{secrets.randbelow(1_000_000):06d}' def create_verification_code(self, email, code): auth_client = AuthClient(url=self._config.auth_url(),