Add node level permissions ECFLOW-1960 #189

marcosbento · 2025-06-09T13:14:01Z

🌦️ >> Documentation << 🌦️
https://sites.ecmwf.int/docs/dev-section/ecflow/pull-requests/PR-189

codecov-commenter · 2025-06-09T13:35:00Z

Codecov Report

❌ Patch coverage is 90.06772% with 44 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.01%. Comparing base (519a841) to head (f375e1e).

Files with missing lines	Patch %	Lines
libs/server/src/ecflow/server/HttpServer.cpp	23.80%	16 Missing ⚠️
libs/node/src/ecflow/node/permissions/Allowed.cpp	28.57%	10 Missing ⚠️
libs/core/src/ecflow/core/Identity.hpp	68.42%	6 Missing ⚠️
.../server/src/ecflow/server/AuthorisationService.cpp	57.14%	3 Missing ⚠️
libs/base/src/ecflow/base/stc/SSyncCmd.cpp	83.33%	2 Missing ⚠️
.../src/ecflow/node/permissions/ActivePermissions.cpp	93.10%	2 Missing ⚠️
libs/base/src/ecflow/base/AuthorisationDetails.hpp	85.71%	1 Missing ⚠️
libs/base/src/ecflow/base/cts/user/CSyncCmd.cpp	75.00%	1 Missing ⚠️
.../src/ecflow/node/permissions/ActivePermissions.hpp	75.00%	1 Missing ⚠️
libs/node/src/ecflow/node/permissions/Allowed.hpp	75.00%	1 Missing ⚠️
... and 1 more

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #189      +/-   ##
===========================================
+ Coverage    49.87%   50.01%   +0.13%     
===========================================
  Files         1218     1223       +5     
  Lines       100278   100595     +317     
  Branches     14827    14860      +33     
===========================================
+ Hits         50012    50308     +296     
- Misses       50266    50287      +21

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

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.}

Copilot · 2025-09-02T07:26:34Z

libs/base/test/TestForceCmd.cpp


 #include <stdexcept>

+#include <boost/property_tree/json_parser/detail/parser.hpp>


Including detail headers from boost property_tree is generally discouraged as detail headers are implementation-specific and may change between versions. Consider using the public API instead.

Suggested change

#include <boost/property_tree/json_parser/detail/parser.hpp>

Copilot · 2025-09-02T07:26:35Z

libs/base/test/TestRequest.cpp

+                    ++i;
                    STC_Cmd_ptr ok_or_error_cmd = cmd_request.handleRequest(&mockServer);
                    if (ok_or_error_cmd) {
+                        std::cout << " [" << i << "]: " << ok_or_error_cmd->ok() << std::endl;


Debug output should be removed or conditionally compiled for production builds. Consider using a proper logging mechanism or removing this debug statement.

Suggested change

std::cout << " [" << i << "]: " << ok_or_error_cmd->ok() << std::endl;

Copilot · 2025-09-02T07:26:35Z

libs/server/src/ecflow/server/AuthenticationService.cpp

+
+bool AuthenticationService::reload_custom_passwd_file(std::string& error) {
+    if (debug_) {
+        std::cout << "ServerEnvironment::reload_custom_passwd_file:(" << ecf_passwd_custom_file_ << ") CWD("


The debug message incorrectly refers to 'ServerEnvironment::reload_custom_passwd_file' but this method is now in AuthenticationService. The message should be updated to reflect the correct class name.

Suggested change

std::cout << "ServerEnvironment::reload_custom_passwd_file:(" << ecf_passwd_custom_file_ << ") CWD("

std::cout << "AuthenticationService::reload_custom_passwd_file:(" << ecf_passwd_custom_file_ << ") CWD("

Copilot · 2025-09-02T07:26:35Z

libs/node/src/ecflow/node/NodePerms.cpp

+ecf::Permission perms(const Node& node) {
+    auto current = &node;
+    auto perms = current->perms();


The function signature declares return type 'ecf::Permission' but line 19 calls 'current->perms()' which likely returns a different type. This will cause compilation errors due to type mismatch.

Suggested change

ecf::Permission perms(const Node& node) {

auto current = &node;

auto perms = current->perms();

ecf::Permission perms = current->perms(); // Ensure perms is of type ecf::Permission

Copilot · 2025-09-02T07:26:36Z

libs/core/test/scaffold/ecflow/test/scaffold/Provisioning.hpp

+#include <random>
 #include <string>

+#include <sys/param.h>


The sys/param.h header is not portable across all platforms and is not used in the visible code. Consider removing this include or replacing it with more portable alternatives if needed.

Suggested change

#include <sys/param.h>

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

marcosbento force-pushed the feature/node_level_permissions branch 2 times, most recently from 5979bfd to 826cef9 Compare June 10, 2025 09:29

marcosbento force-pushed the feature/node_level_permissions branch from 826cef9 to 07d0121 Compare June 19, 2025 09:16

marcosbento force-pushed the feature/node_level_permissions branch 3 times, most recently from 0cac4ce to f362f13 Compare June 27, 2025 13:10

marcosbento force-pushed the feature/node_level_permissions branch from f362f13 to 6012771 Compare July 4, 2025 10:50

marcosbento force-pushed the feature/node_level_permissions branch 5 times, most recently from ec103ab to 7c0468f Compare July 19, 2025 06:33

marcosbento force-pushed the feature/node_level_permissions branch from 7c0468f to 8861c20 Compare July 22, 2025 13:19

marcosbento force-pushed the feature/node_level_permissions branch 2 times, most recently from afc9cfd to abce382 Compare August 26, 2025 13:18

marcosbento force-pushed the feature/node_level_permissions branch 2 times, most recently from d02c59e to 741fd06 Compare September 1, 2025 15:23

marcosbento marked this pull request as ready for review September 2, 2025 07:24

marcosbento requested a review from Copilot September 2, 2025 07:24

marcosbento marked this pull request as draft September 2, 2025 07:25

Copilot AI reviewed Sep 2, 2025

View reviewed changes

marcosbento force-pushed the feature/node_level_permissions branch 3 times, most recently from 34d88ea to b8be18c Compare September 5, 2025 10:04

marcosbento force-pushed the feature/node_level_permissions branch 5 times, most recently from 2311079 to c130e9a Compare September 16, 2025 12:43

marcosbento added 7 commits September 16, 2025 14:41

Add basic 'rwxo' permissions

b324b02

Re ECFLOW-1960

Add PERMISSIONS to the server configuration

db27985

Re ECFLOW-1960

Concentrate authorisation logic at Defs level

6421c03

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

Split permission related classes into separate files

23e7014

Re ECFLOW-1960

Move permission determination algorithm to Node

a4a5821

Re ECFLOW-1960

marcosbento force-pushed the feature/node_level_permissions branch from c130e9a to a4a5821 Compare September 16, 2025 13:44

marcosbento added 4 commits September 16, 2025 15:47

Filter the Defs based on the user performing request

87d5b72

Correct criteria to use Basic/Bearer authentication

6345c00

Rename PERMISSIONS to ECF_PERMISSIONS

fc2ae47

Reduce debug logging messages

f375e1e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add node level permissions ECFLOW-1960 #189

Add node level permissions ECFLOW-1960 #189

Uh oh!

marcosbento commented Jun 9, 2025 •

edited by github-actions bot

Loading

Uh oh!

codecov-commenter commented Jun 9, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 2, 2025

Uh oh!

Copilot AI Sep 2, 2025

Uh oh!

Copilot AI Sep 2, 2025

Uh oh!

Copilot AI Sep 2, 2025

Uh oh!

Copilot AI Sep 2, 2025

Uh oh!

Uh oh!


		#include <stdexcept>

		#include <boost/property_tree/json_parser/detail/parser.hpp>

	std::cout << "ServerEnvironment::reload_custom_passwd_file:(" << ecf_passwd_custom_file_ << ") CWD("
	std::cout << "AuthenticationService::reload_custom_passwd_file:(" << ecf_passwd_custom_file_ << ") CWD("

Add node level permissions ECFLOW-1960 #189

Are you sure you want to change the base?

Add node level permissions ECFLOW-1960 #189

Uh oh!

Conversation

marcosbento commented Jun 9, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

marcosbento commented Jun 9, 2025 •

edited by github-actions bot

Loading

codecov-commenter commented Jun 9, 2025 •

edited

Loading