-
Notifications
You must be signed in to change notification settings - Fork 15
Add node level permissions ECFLOW-1960 #189
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
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #189 +/- ##
===========================================
- Coverage 48.70% 48.70% -0.01%
===========================================
Files 1221 1228 +7
Lines 96120 96492 +372
Branches 14833 14886 +53
===========================================
+ Hits 46818 46992 +174
- Misses 49302 49500 +198 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
5979bfd to
826cef9
Compare
826cef9 to
07d0121
Compare
0cac4ce to
f362f13
Compare
f362f13 to
6012771
Compare
ec103ab to
7c0468f
Compare
7c0468f to
8861c20
Compare
afc9cfd to
abce382
Compare
d02c59e to
741fd06
Compare
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.
Pull Request Overview
This PR implements node-level permissions for the ECflow server system. The changes introduce a comprehensive authentication and authorization framework that enables fine-grained access control based on user permissions defined at different levels in the node hierarchy.
- Replaces legacy authentication methods with a new Identity-based system that supports multiple authentication types (user, custom user, secure user, and task)
- Introduces a permissions system using PERMISSIONS variables that can be defined at server, suite, and node levels with inheritance and override capabilities
- Refactors server environment handling to separate authentication and authorization concerns into dedicated services
Reviewed Changes
Copilot reviewed 144 out of 145 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| libs/server/src/ecflow/server/ServerEnvironment.hpp | Replaces password file handling with AuthenticationService and AuthorisationService |
| libs/server/src/ecflow/server/TcpBaseServer.cpp | Adds identity extraction and assignment to commands |
| libs/node/src/ecflow/node/permissions/* | New permissions framework with Allowed, Permission, Permissions, and ActivePermissions classes |
| libs/core/src/ecflow/core/Identity.hpp | New identity system supporting various user types and authentication methods |
| libs/base/src/ecflow/base/cts/user/*.cpp | Updates user commands to use new authentication/authorization framework |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
34d88ea to
b8be18c
Compare
2311079 to
c130e9a
Compare
c130e9a to
a4a5821
Compare
f375e1e to
a2472a8
Compare
069fdec to
8559db5
Compare
6e0fdf0 to
f5b2ec8
Compare
Re ECFLOW-1960
Re ECFLOW-1960
Based on the latest discussion, the permissions selection algorithm should only permit restraining allowances when navigating the node tree from the server to the leaves (e.g. a user that starts by having 'rw' permissions, can in a lower node be allowed only 'r' but never will he be allowed to 'rwx'). Re ECFLOW-1960
The server itself (i.e. the Server implementation class) should not be involved in authorisation, as internally all user data is effectively stores as part of the Defs object. Re ECFLOW-1960
Extend the 'rwxo' permissions to 'rwxos' where 's' stands for sticky, and protects the rights of admin users for which the access rights must be safe-guarded. Once a permissions is classified as Sticky, the usual permissions override mechanism is disabled. Re ECFLOW-1960
Re ECFLOW-1960
Re ECFLOW-1960
f5b2ec8 to
df60070
Compare
🌦️ >> Documentation << 🌦️
https://sites.ecmwf.int/docs/dev-section/ecflow/pull-requests/PR-189