-
Notifications
You must be signed in to change notification settings - Fork 20
WIP: feat: Add authorization context helpers and server-side filtering for timeseries #1461
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
base: develop
Are you sure you want to change the base?
Conversation
…lter generation Add AuthorizationContextHelper and AuthorizationFilterHelper to support fine-grained authorization for CWMS Data API: - AuthorizationContextHelper parses x-cwms-auth-context header and extracts user context (username, offices, roles, persona) - AuthorizationFilterHelper generates JOOQ conditions for server-side filtering: - Office-based filtering (allowed_offices constraint) - Embargo rules (hide recent data by office) - Time window restrictions (limit historical data access) - Data classification filtering (classification levels) Both helpers follow CWMS codebase style with minimal comments and clear naming. All filtering logic generates type-safe JOOQ conditions for database-level enforcement. This infrastructure supports the authorization proxy pattern where OPA makes policy decisions and CWMS Data API enforces them via the authorization context header.
…imeSeriesDaoImpl Integrate authorization helpers into timeseries data retrieval: Controller changes (TimeSeriesController): - Create AuthorizationContextHelper and AuthorizationFilterHelper from request context - Validate office access before querying data (early validation) - Pass AuthorizationFilterHelper to DAO for server-side filtering - Log authorization context for debugging DAO changes (TimeSeriesDaoImpl): - Add method overload accepting AuthorizationFilterHelper parameter - Apply embargo filter to hide recent data based on office-specific rules - Apply time window filter to restrict historical data access - Combine authorization filters with existing filter conditions using JOOQ - All filtering happens at database level via SQL WHERE clauses Backward compatible: Existing methods without authorization filters continue to work unchanged. Authorization filtering only applied when x-cwms-auth-context header is present.
MikeNeilson
left a comment
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.
Overall it seems reasonable.
The initial Context helper processing should probably go in ApiServer.java, here:
| .before(authenticator) |
after the authenticator, and they like the authenticator itself, or what you see on line 360, set a context attributed with the appropriate created object.
| return hasAuthContext; | ||
| } | ||
|
|
||
| public Condition getOfficeFilter(Field<String> officeField, String requestedOffice) { |
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.
Return DSL.noCondition instead of null.
It makes the downstream logic cleaner (no null checks) and we've found the behavior is better.
| return officeField.in(allowedOffices); | ||
| } | ||
|
|
||
| public Condition getEmbargoFilter(Field<Timestamp> timestampField, Field<String> officeField) { |
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.
same as above, return noCondition instead of null
| } | ||
|
|
||
| // Add office-specific embargo rules | ||
| if (embargoRulesNode.has("SPK")) { |
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.
it would seem this and below should be one block that uses "requestedOffice" in the has and get methods.
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.
Do we have a means in the CWMS schema to determine if a given office has embargoRules?
Or is this more of a hold-over?
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.
we still need to sort out exactly how that gets determined and then converted into a policy.
| List<Condition> conditions = new ArrayList<>(); | ||
|
|
||
| Condition officeFilter = getOfficeFilter(officeField, requestedOffice); | ||
| if (officeFilter != null) { |
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.
can remove null checks if methods all return noCondition
| if (authFilter != null && authFilter.hasAuthorizationContext()) { | ||
| Field<String> officeField = valid.field("office_id", String.class); | ||
|
|
||
| Condition embargoFilter = authFilter.getEmbargoFilter(dataEntryDate, officeField); |
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.
same as another comments, favor returning noCondition over returning null.
…ilters * origin: Entity endpoint Controller and Integration test (#1497) Enhancements/blob clob query (#1483) Update treafik for latest docker. (#1493) Bugfix/cda 45 ts vertical datum (#1344) CDA-66: Updated TS identifier descriptor paging (#1481) add in missing expiration date to constant/seasonal levels (#1490) 1351 implement cda gui code formatter (#1460) add missing back tic CWMS Data API documentation /timeseries GET endpoints. (#1476) CDA-60: Accept Header Formatting Documentation (#1463) The temp users set needs to be a LinkedHashSet, otherwise the last user in the list isn't always the last user and the paging doesn't work. Add static analysis unit test for Controller classes (#1362) Bugfix/incorrect parameter warning cda 58 (#1470) Test updates for latest schema and correct release schema image. (#1474) CDA-54 - Implements Entity DTO and Dao (#1482) Update npm pacakges (#1478) Correct required java version (#1462) CDA-40: Exception Handling Implementation Updates (#1358)
- AuthorizationFilterHelper methods now return noCondition() for cleaner downstream logic without null checks - Refactored getEmbargoFilter to use requestedOffice parameter instead of hardcoded office-specific blocks - Simplified getAllFilters by removing null checks - Updated TimeSeriesDaoImpl to pass office parameter and remove null checks
Summary
Add authorization infrastructure to support fine-grained access control for timeseries data:
This implementation supports the authorization proxy pattern where policy decisions are made by OPA and enforcement happens in the CWMS Data API via the authorization context header.
Screenshots
N/A - Backend authorization infrastructure
Todos
Steps to Validate
Start CWMS Data API with authorization proxy:
Test office access validation:
Test embargo filtering (requires OPA policy with embargo rules):
Test time window filtering (requires OPA policy with time_window constraint):
Check logs for filter application:
Verify backward compatibility (requests without authorization header):
Local Environment Notes
Impacted Areas in Application
Notes