Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
cc101f8
initial commit with CSRF mitigations
Mar 2, 2026
2851b30
added some tests for CSRF middleware
Mar 9, 2026
f1d4330
added additional CSRF tests
Mar 16, 2026
74b2efe
separated regular functioning security tests from CSRF tests
Mar 16, 2026
e2066f6
fix: bump cryptography to 46.0.5 and expand CI security coverage
deacon-mp Mar 15, 2026
f1dbc0a
fix: reuse existing fact source on operation close (#3261)
ChenFryd Mar 15, 2026
0fe954e
Bump minimatch from 3.1.2 to 3.1.5 (#3258)
dependabot[bot] Mar 15, 2026
19bddee
Add architecture field to agent deployment commands (#3260)
clenk Mar 16, 2026
6c10c27
hash passwords and API keys in main config (#3257)
uruwhy Mar 16, 2026
d8bf701
fix: replace create_subprocess_shell (#3265)
deacon-mp Mar 16, 2026
708208f
fix: replace deprecated asyncio.get_event_loop() with new_event_loop(…
deacon-mp Mar 16, 2026
99eeaa5
fix: reduce global client_max_size and add configurable setting (#3268)
deacon-mp Mar 16, 2026
81b6eb0
fix: degrade gracefully when magma plugin dist is absent (#3275)
deacon-mp Mar 16, 2026
b923bfb
fix: guard against None agent in operations summary endpoint (#3181) …
deacon-mp Mar 16, 2026
ad5eefa
fix: resolve trait-only relationship facts from source fact list on o…
deacon-mp Mar 16, 2026
0e211cf
fix: prevent operation report from returning null when link paw absen…
deacon-mp Mar 16, 2026
78d3c2a
fix: correct exfil operation filter and patch path traversal bypass (…
deacon-mp Mar 16, 2026
8513e11
fix: validate upload filename character set in file_svc (#3267)
deacon-mp Mar 16, 2026
049784b
fix: register SIGTERM handler in run_tasks() to save state on service…
deacon-mp Mar 16, 2026
1c5d5ba
added modification to test_authentication_required_middleware_authent…
Mar 16, 2026
83ffce0
added test to show that without cookie, authenticated endpoint sessio…
Mar 16, 2026
21d065c
CSRF operation tests are passing, including tests to verify resilienc…
Mar 16, 2026
cf6c1a2
security.py change allowing CSRF protections to be bypassed if the se…
Mar 16, 2026
5951697
CSRF operation creation test now passes - small change reflecting upd…
Mar 16, 2026
86528b5
linting reformatting
Mar 16, 2026
73e7850
Merge branch 'master' into VIRTS-1953
Mar 16, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion app/api/v2/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@

def make_app(services):
from .responses import json_request_validation_middleware
from .security import authentication_required_middleware_factory, pass_option_middleware
from .security import authentication_required_middleware_factory, pass_option_middleware, csrf_protect_middleware_factory

app = web.Application(
middlewares=[
pass_option_middleware,
authentication_required_middleware_factory(services['auth_svc']),
csrf_protect_middleware_factory(services['auth_svc']),
json_request_validation_middleware
]
)
Expand Down
41 changes: 41 additions & 0 deletions app/api/v2/security.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
import types

from aiohttp import web
from aiohttp_session import get_session
from hmac import compare_digest
from aiohttp.web_exceptions import HTTPForbidden


def is_handler_authentication_exempt(handler):
Expand Down Expand Up @@ -69,6 +72,44 @@ async def authentication_required_middleware(request, handler):
return authentication_required_middleware


def csrf_protect_middleware_factory(auth_svc):
"""Protect unsafe (state-modifying) requests against CSRF for session-authenticated users.

Behavior:
- Allow safe methods (GET, HEAD, OPTIONS) without checks.
- If request provides an API key (header KEY), skip CSRF checks.
- For session-authenticated requests using unsafe methods, compare the X-CSRF-Token
header to the token stored in the server-side session (recommended) and reject
requests with missing/invalid tokens with HTTP 403.
"""
@web.middleware
async def csrf_protect_middleware(request, handler):
# Skip safe methods
if request.method in ('GET', 'HEAD', 'OPTIONS'):
return await handler(request)

# If API key auth is present, skip CSRF checks
if request.headers.get('KEY'):
return await handler(request)
Comment on lines +91 to +93
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

CSRF checks are skipped whenever a KEY header is present, even if the API key is invalid. For a session-authenticated request, an attacker could add any KEY header value and bypass CSRF validation. Use auth_svc.request_has_valid_api_key(request) (and ideally only skip when API-key auth is the actual auth mechanism) rather than checking header presence.

Copilot uses AI. Check for mistakes.

# For session-authenticated requests, validate token
try:
session = await get_session(request)
token = session.get('csrf_token') if session is not None else None
header = request.headers.get('X-CSRF-Token') or request.headers.get('X-XSRF-TOKEN')
if not token or not header or not compare_digest(header, token):
raise HTTPForbidden(text='Missing or invalid CSRF token')
except HTTPForbidden:
raise
except Exception:
# If something goes wrong accessing the session, deny the request
raise HTTPForbidden(text='CSRF validation error')

return await handler(request)

return csrf_protect_middleware


@web.middleware
async def pass_option_middleware(request, handler):
"""Allow all 'OPTIONS' request to the server to return 200
Expand Down
24 changes: 23 additions & 1 deletion app/service/auth_svc.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@
from aiohttp_security import setup as setup_security
from aiohttp_security.abc import AbstractAuthorizationPolicy
from aiohttp_session import setup as setup_session
from aiohttp_session import get_session
from aiohttp_session.cookie_storage import EncryptedCookieStorage
from cryptography import fernet
import secrets

from app.service.interfaces.i_auth_svc import AuthServiceInterface
from app.service.interfaces.i_login_handler import LoginHandlerInterface
Expand Down Expand Up @@ -75,7 +77,14 @@ async def apply(self, app, users):
app.user_map = self.user_map
fernet_key = fernet.Fernet.generate_key()
secret_key = base64.urlsafe_b64decode(fernet_key)
storage = EncryptedCookieStorage(secret_key, cookie_name=COOKIE_SESSION)
storage = EncryptedCookieStorage(
secret_key,
cookie_name=COOKIE_SESSION,
secure=True,
httponly=True,
max_age=86400,
samesite='Strict'
)
setup_session(app, storage)
policy = SessionIdentityPolicy()
setup_security(app, policy, DictionaryAuthorizationPolicy(self.user_map))
Expand Down Expand Up @@ -155,6 +164,19 @@ async def handle_successful_login(self, request, username):
self.log.debug('%s logging in', username)
response = web.HTTPFound('/')
await remember(request, response, username)

# Initialize per-session CSRF token and expose it via a readable cookie for double-submit
try:
session = await get_session(request)
if 'csrf_token' not in session:
session['csrf_token'] = secrets.token_urlsafe(32)
# Set a non-HttpOnly cookie so client-side JS can read the token for double-submit.
secure_flag = (request.scheme == 'https') if hasattr(request, 'scheme') else False
response.set_cookie('XSRF-TOKEN', session['csrf_token'], httponly=False, secure=secure_flag, samesite='Lax')
Comment on lines +174 to +175
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

The XSRF-TOKEN cookie is set without max_age while the session cookie now has max_age=86400. This can leave users with a still-valid session but no readable CSRF cookie after a browser restart, causing all unsafe requests to fail until re-login. Also, deriving secure from request.scheme can be wrong behind TLS-terminating proxies; consider setting secure=True consistently (or using a trusted proxy/proto config) and align max_age/path with the session cookie.

Suggested change
secure_flag = (request.scheme == 'https') if hasattr(request, 'scheme') else False
response.set_cookie('XSRF-TOKEN', session['csrf_token'], httponly=False, secure=secure_flag, samesite='Lax')
# Align cookie attributes (max_age, path, secure) with the session cookie settings.
response.set_cookie(
'XSRF-TOKEN',
session['csrf_token'],
max_age=86400,
path='/',
httponly=False,
secure=True,
samesite='Lax',
)

Copilot uses AI. Check for mistakes.
except Exception:
# If session management or cookie setting fails, continue without exposing token.
self.log.exception('Failed to set CSRF token on login')

raise response

async def check_permissions(self, group, request):
Expand Down
88 changes: 88 additions & 0 deletions tests/api/v2/test_security.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,48 @@ async def login(request):
return app


@pytest.fixture
def csrf_webapp(event_loop, base_world):
"""Like simple_webapp but with CSRF protection middleware and POST route for /private."""
async def index(request):
return web.Response(status=200, text='hello!')

@security.authentication_exempt
async def public(request):
return web.Response(status=200, text='public')

async def private(request):
return web.Response(status=200, text='private')

@security.authentication_exempt
async def login(request):
await auth_svc.login_user(request) # Note: auth_svc defined in context function

app = web.Application()
app.router.add_get('/', index)
app.router.add_post('/login', login)
app.router.add_get('/public', public)
app.router.add_get('/private', private)
app.router.add_post('/private', private)

auth_svc = AuthService()

event_loop.run_until_complete(
auth_svc.apply(
app=app,
users=base_world.get_config('users')
)
)
event_loop.run_until_complete(auth_svc.set_login_handlers(auth_svc.get_services()))

# Ensure authentication middleware runs after session middleware
app.middlewares.append(security.authentication_required_middleware_factory(auth_svc))
# Add CSRF protection middleware after authentication
app.middlewares.append(security.csrf_protect_middleware_factory(auth_svc))

return app


def test_function_is_authentication_exempt():
def fake_handler(request):
return None
Expand Down Expand Up @@ -156,3 +198,49 @@ async def public(self, request):
client = await aiohttp_client(app)
resp = await client.get('/public')
assert resp.status == 200


# CSRF protection tests
async def test_csrf_protect_rejects_missing_token_for_session_auth(csrf_webapp, aiohttp_client):
"""A session-authenticated POST without a valid CSRF header should be rejected with 403."""
client = await aiohttp_client(csrf_webapp)

login_response = await client.post(
'/login',
data={'username': 'admin', 'password': 'admin'},
allow_redirects=False
)

assert login_response.status == 200
assert COOKIE_SESSION in login_response.cookies

post_resp = await client.post('/private')
assert post_resp.status == 403


async def test_csrf_protect_accepts_valid_token_for_session_auth(csrf_webapp, aiohttp_client):
"""A session-authenticated POST with a valid CSRF header should be allowed."""
client = await aiohttp_client(csrf_webapp)

login_response = await client.post(
'/login',
data={'username': 'admin', 'password': 'admin'},
allow_redirects=False
)

assert login_response.status == 200
# The login handler exposes the CSRF token as a readable cookie named 'XSRF-TOKEN'
token_cookie = login_response.cookies.get('XSRF-TOKEN')
assert token_cookie is not None
token = token_cookie.value

post_resp = await client.post('/private', headers={'X-CSRF-Token': token})
assert post_resp.status == 200

# TODO: Change to be a valid API Key Header (vs just any API key header) - covered by API login test
async def test_csrf_protect_skips_when_api_key_present(csrf_webapp, aiohttp_client):
"""If an API key header is present, CSRF checks should be skipped."""
client = await aiohttp_client(csrf_webapp)

post_resp = await client.post('/private', headers={HEADER_API_KEY: 'abc123'})
assert post_resp.status == 200
Loading