From eafcabdafebaf935f65eb78c572dde07b5de50d9 Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Fri, 26 Aug 2022 13:31:45 +0200 Subject: [PATCH] Remove TODOs that were extracted to PR #617 --- irrd/auth/__init__.py | 3 --- irrd/auth/auth.py | 4 ++-- irrd/auth/endpoints_mntners.py | 3 --- irrd/auth/http_endpoints.py | 4 ---- irrd/auth/utils.py | 22 +++++++++++----------- irrd/server/http/app.py | 2 +- requirements.txt | 2 +- 7 files changed, 15 insertions(+), 25 deletions(-) diff --git a/irrd/auth/__init__.py b/irrd/auth/__init__.py index 818402a7f..803b22c4f 100644 --- a/irrd/auth/__init__.py +++ b/irrd/auth/__init__.py @@ -67,11 +67,8 @@ async def endpoint_wrapper(*args, **kwargs): request = next((arg for arg in list(args) + list(kwargs.values()) if isinstance(arg, Request)), None) if not request.auth.is_authenticated: - # TODO: Implement proper redirect logic - message(request, 'You must be authed', 'info') return RedirectResponse(request.url_for('ui:login'), status_code=302) - # pass on request return await func(*args, **kwargs) return endpoint_wrapper diff --git a/irrd/auth/auth.py b/irrd/auth/auth.py index b1a5b0e59..08cd70b94 100644 --- a/irrd/auth/auth.py +++ b/irrd/auth/auth.py @@ -31,9 +31,9 @@ def verify(self, plain: str, hashed: str) -> bool: return hashed == plain -secret_key = 'key!' # TODO +secret_key = 'key!' user_provider = AuthProvider() -password_verifier = MyPasswordVerifier() # TODO: pick +password_verifier = MyPasswordVerifier() login_manager = LoginManager(user_provider, password_verifier, secret_key) diff --git a/irrd/auth/endpoints_mntners.py b/irrd/auth/endpoints_mntners.py index 64283102d..028e6a06c 100644 --- a/irrd/auth/endpoints_mntners.py +++ b/irrd/auth/endpoints_mntners.py @@ -78,7 +78,6 @@ async def permission_add(request: Request, session_provider: ORMSessionProvider) 'permission_form.html', request, {'form_html': form_html, 'mntner': mntner} ) - # TODO: require password new_permission = AuthPermission( user_id=str(form.new_user.pk), mntner_id=str(mntner.pk), @@ -208,7 +207,6 @@ async def mntner_migrate_initiate(request: Request, session_provider: ORMSession 'mntner_migrate_initiate.html', request, {'form_html': form_html} ) - # TODO: email confirmation new_auth_mntner = AuthMntner( rpsl_mntner_pk=form.rpsl_mntner.pk(), rpsl_mntner_obj_id=str(form.rpsl_mntner_db_pk), @@ -284,7 +282,6 @@ async def mntner_migrate_complete(request: Request, session_provider: ORMSession form.auth_mntner.migration_token = None session_provider.session.add(form.auth_mntner) - # TODO: probably move this to RPSLMntner? form.rpsl_mntner_obj.parsed_data['auth'].append(RPSL_MNTNER_AUTH_INTERNAL) session_provider.database_handler.upsert_rpsl_object(form.rpsl_mntner_obj, origin=JournalEntryOrigin.unknown) diff --git a/irrd/auth/http_endpoints.py b/irrd/auth/http_endpoints.py index a687fc4af..27fda7f19 100644 --- a/irrd/auth/http_endpoints.py +++ b/irrd/auth/http_endpoints.py @@ -14,7 +14,6 @@ @session_provider_manager @authentication_required async def index(request: Request, session_provider: ORMSessionProvider) -> Response: - # TODO: RPKI state?? user_mntners = [ (mntner.rpsl_mntner_pk, mntner.rpsl_mntner_source) for mntner in request.auth.user.mntners @@ -58,7 +57,6 @@ async def rpsl_detail(request: Request, session_provider: ORMSessionProvider): }) -# TODO: CSRF? @session_provider_manager async def rpsl_update(request: Request, session_provider: ORMSessionProvider) -> Response: mntner_perms = defaultdict(list) @@ -86,7 +84,6 @@ async def rpsl_update(request: Request, session_provider: ORMSessionProvider) -> }) elif request.method == 'POST': - # TODO: offload db part to thread form_data = await request.form() request_meta = { 'HTTP-client-IP': request.client.host, @@ -116,7 +113,6 @@ async def user_detail(request: Request, session_provider: ORMSessionProvider) -> return template_context_render('user_detail.html', request, {'user': bound_user}) -# TODO: may need better place def filter_auth_hash_non_mntner(user: AuthUser, rpsl_object: RPSLDatabaseObject) -> str: user_mntners = [ (mntner.rpsl_mntner_pk, mntner.rpsl_mntner_source) diff --git a/irrd/auth/utils.py b/irrd/auth/utils.py index d6909b911..3666003cb 100644 --- a/irrd/auth/utils.py +++ b/irrd/auth/utils.py @@ -1,4 +1,3 @@ -# TODO: find appropriate places for all that ends up here import hashlib import secrets import typing @@ -9,6 +8,10 @@ from irrd.storage.models import AuthUser +PASSWORD_RESET_TOKEN_ROOT = date(2022, 1, 1) +PASSWORD_RESET_SECRET = 'aaaaa' +PASSWORD_RESET_VALIDITY_DAYS = 7 + # From https://github.com/accent-starlette/starlette-core/ def message(request: Request, message: typing.Any, category: str = "success") -> None: @@ -25,27 +28,24 @@ def get_messages(request: Request): def reset_token(user: AuthUser) -> str: - secret = 'aaaaaa' # TODO: actual secret key + secret = 'aaaaaa' user_key = str(user.pk) + str(user.updated) + user.password - expiry_day = date.today() + timedelta(days=7) # TODO: configurable days - expiry_days = expiry_day - date(2022, 1, 1) # TODO: constant - hash_token = secret + user_key + str(expiry_days.days) + expiry_day = date.today() + timedelta(days=PASSWORD_RESET_VALIDITY_DAYS) + expiry_days = expiry_day - PASSWORD_RESET_TOKEN_ROOT + hash_token = PASSWORD_RESET_SECRET + user_key + str(expiry_days.days) digest = hashlib.sha224(hash_token.encode('utf-8')).digest() hash = urlsafe_b64encode(digest).decode('ascii') return str(expiry_days.days) + '-' + str(hash) def validate_token(user: AuthUser, token: str) -> bool: - # TODO: more resiliency for parsing errors try: - secret = 'aaaaaa' # TODO: actual secret key expiry_days, encoded_hash = token.split('-', 1) user_key = str(user.pk) + str(user.updated) + user.password - expiry_date = date(2022, 1, 1) + timedelta(days=int(expiry_days)) # TODO: constant - hash_token = secret + user_key + expiry_days + expiry_date = PASSWORD_RESET_TOKEN_ROOT + timedelta(days=int(expiry_days)) + hash_token = PASSWORD_RESET_SECRET + user_key + expiry_days token_hash = urlsafe_b64decode(encoded_hash) expected_hash = hashlib.sha224(hash_token.encode('utf-8')).digest() - return expiry_date >= date.today() and secrets.compare_digest(token_hash, - expected_hash) # TODO: not constant time + return expiry_date >= date.today() and secrets.compare_digest(token_hash, expected_hash) except ValueError: return False diff --git a/irrd/server/http/app.py b/irrd/server/http/app.py index 99b442d18..a1ded9b8b 100644 --- a/irrd/server/http/app.py +++ b/irrd/server/http/app.py @@ -99,7 +99,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: on_shutdown=[shutdown], middleware=[ Middleware(MemoryTrimMiddleware), - Middleware(SessionMiddleware, secret_key='foo'), # TODO: restrict security? secret key + Middleware(SessionMiddleware, secret_key='foo'), Middleware(CSRFProtectMiddleware, csrf_secret='foo2'), auth_middleware, ], diff --git a/requirements.txt b/requirements.txt index 5323bb855..bfae6aadc 100644 --- a/requirements.txt +++ b/requirements.txt @@ -30,7 +30,7 @@ starlette==0.17.1 # pyup: <0.18 # ariadne conflict python-multipart==0.0.5 # required by starlette for forms imia==0.5.3 # User and login management Starlette-WTF==0.4.3 # Form handling -wtforms-bootstrap5==0.1.3 # bootstrap templating for forms TODO: requires manual patch +wtforms-bootstrap5==0.1.3 # bootstrap templating for forms # Database connections and management psycopg2-binary==2.9.3; platform_python_implementation == "CPython"