Conversation
… views ## What this PR does Adds login/logout to the webserver (PR5 in the multi-user roadmap). All routes now require authentication. DB queries are scoped to the logged-in user via SET LOCAL ROLE, which PostgreSQL reverts automatically at transaction end — preventing role bleed on reused connections. ### Changes #### webserver/main.py - Flask-Login: LoginManager, User(UserMixin), load_user, /login, /logout - All existing routes gain @login_required - New user_db_scope(user_id) context manager: connects as webserver_role, issues SET LOCAL ROLE <user_id> via pgsql.Identifier (injection-safe), auto-reverts at transaction end - All DB queries switched from raw cml_data/cml_data_1h to cml_data_secure/cml_data_1h_secure; cml_metadata/cml_stats scoped through the user's role (RLS enforced automatically) - get_db_connection() kept as admin path (webserver_role, no SET ROLE) - get_db_connection() kept as admin path (webserver_role, no SET ROLE) t r/configs/users.json - New file with scrypt-hashed passwords fo- New file with scr d- New file with scrypt-hashed passwords fo- New file with scr d- ve- New file with scrypt-hashed passwords fo- New file with scr d- on- New file with scrypt-hashed passwords fo- New file with scr d- New fiap- New file with scrypt-hashed passwords fo- New file with scr d- webserver DATABASE_URL switched to webserver_role:webserverpassword - SECRET_KEY and USERS_CONFIG_PATH env vars - SECRE ./webserver/configs mounted read-on- SECRET_KEY and USERS_CONFIG_PATH env vars ill connects as myuser (superuser) and sees all tenants' data. Will be addressed in PR8 (feat/grafana-auth-proxy) using Grafana's auth proxy mode with per-user datasources connecting as the matching PG role.
- Update status table: PRs 3-5 marked merged, PR8 added - Add PR8 section: Grafana auth proxy + per-user PG datasources - X-WEBAUTH-USER header injected by Flask proxy - Per-user datasources connect as matching PG role (RLS enforced) - Known gap section documents current state until PR8 is merged
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #40 +/- ##
==========================================
+ Coverage 78.77% 81.18% +2.41%
==========================================
Files 24 24
Lines 2327 2371 +44
==========================================
+ Hits 1833 1925 +92
+ Misses 494 446 -48
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Mock user_db_scope context manager instead of get_db_connection - Patch current_user.id to avoid AnonymousUserMixin error - Set LOGIN_DISABLED to bypass @login_required decorator
- test_auth.py: login/logout flow, protected route redirects, open-redirect prevention, invalid credentials - test_user_db_scope.py: unknown user rejected before DB connect, rollback on exception, SET LOCAL ROLE uses pgsql.Identifier
… logout - test_helpers.py: safe_float (None/valid/NaN/inf/bad-string), load_user (known/unknown) - test_api_routes.py: /api/cml-metadata, /api/cml-map, /api/data-time-range (both branches) - test_auth.py: logout-when-authenticated verifies session is cleared Coverage: main.py 39% → 49%
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
feat/webserver-auth — Flask-Login, per-request SET LOCAL ROLE, secure views
What this PR does
Adds authentication to the webserver and enforces per-user data isolation at the database level for every request.
Changes
Authentication (Flask-Login)
@login_required; unauthenticated requests redirect to/login/loginand/logoutwebserver/configs/users.json(scrypt-hashed passwords via werkzeug)nextparameterPer-request DB role scoping
user_db_scope(user_id)context manager: connects aswebserver_role, issuesSET LOCAL ROLE <user_id>usingpsycopg2.sql.Identifier(not string interpolation), yields connection, commits/rolls back, closesSET LOCALis automatically reverted at transaction end — role bleed between requests is impossibleuser_db_scopeand the security-barrier views (cml_data_secure,cml_data_1h_secure) instead of raw tablesConfiguration
docker-compose.yml: webserver now connects aswebserver_role,SECRET_KEYandUSERS_CONFIG_PATHadded as env vars,configs/volume mounted read-onlywebserver/requirements.txt:Flask-Login==0.6.3addedTests (main.py coverage 39% → 49%)
test_auth.py: protected route redirects, login (valid/invalid/unknown), open-redirect blocked, logout clears sessiontest_user_db_scope.py: unknown user rejected before any DB call, rollback on exception,SET LOCAL ROLEusespgsql.Composabletest_helpers.py:safe_floatedge cases,load_userknown/unknowntest_api_routes.py:/api/cml-metadata,/api/cml-map,/api/data-time-range(both branches)test_api_cml_stats.pyfor new auth layerDocs
webserver/README.md: user management, password hash generation, adding usersdocs/multi-user-architecture.md: PR8 (feat/grafana-auth-proxy) planned and documentedKnown gap
Grafana currently connects as
myuser(superuser) and bypasses RLS — it sees all tenants' data. The/grafana/proxy is gated by@login_requiredso unauthenticated access is blocked, but data isolation inside Grafana is not yet enforced. This is addressed in the next PR (feat/grafana-auth-proxy).Security notes
SET LOCAL ROLEviapgsql.Identifier, never string-formatted — SQL injection not possibleuser_idis allowlisted againstUSERSbefore any SQL is composedSECRET_KEYdefaults toos.urandom(32)in dev; production should set it via env