Skip to content

Commit b8be18c

Browse files
marcosbentomarcosbento
committed
Move permission determination algorithm to Node
Re ECFLOW-1960
1 parent 16c85c9 commit b8be18c

File tree

9 files changed

+98
-105
lines changed

9 files changed

+98
-105
lines changed

‎libs/CMakeLists.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ set(srcs
5757
# Base -- Headers
5858
base/src/ecflow/base/AbstractClientEnv.hpp
5959
base/src/ecflow/base/AbstractServer.hpp
60-
base/src/ecflow/base/Algorithms.hpp
6160
base/src/ecflow/base/Authentication.hpp
6261
base/src/ecflow/base/AuthenticationDetails.hpp
6362
base/src/ecflow/base/Authorisation.hpp
@@ -360,6 +359,7 @@ set(srcs
360359
node/src/ecflow/node/Node.hpp
361360
node/src/ecflow/node/NodeContainer.hpp
362361
node/src/ecflow/node/NodeFwd.hpp
362+
node/src/ecflow/node/NodePathAlgorithms.hpp
363363
node/src/ecflow/node/NodeState.hpp
364364
node/src/ecflow/node/NodeStats.hpp
365365
node/src/ecflow/node/NodeTreeVisitor.hpp

‎libs/base/CMakeLists.txt‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ set(test_srcs
2323
test/TestInLimitAndLimit.cpp
2424
test/TestLogCmd.cpp
2525
test/TestMeterCmd.cpp
26-
test/TestPermissions.cpp
2726
test/TestProgramOptions.cpp
2827
test/TestQueryCmd.cpp
2928
test/TestQueueCmd.cpp
@@ -59,30 +58,6 @@ target_clangformat(u_base
5958
CONDITION ENABLE_TESTS
6059
)
6160

62-
set(test_srcs
63-
# Sources
64-
test/TestAlgorithms.cpp
65-
)
66-
67-
ecbuild_add_test(
68-
TARGET
69-
u_base_algorithms
70-
LABELS
71-
unit nightly
72-
SOURCES
73-
${test_srcs}
74-
LIBS
75-
ecflow_all
76-
test_scaffold
77-
test_harness.base
78-
Threads::Threads
79-
$<$<BOOL:${OPENSSL_FOUND}>:OpenSSL::SSL>
80-
)
81-
target_clangformat(u_base_algorithms
82-
CONDITION ENABLE_TESTS
83-
)
84-
85-
8661
# The following is not technically a test (as it makes no checks),
8762
# but a tool to measure the time it takes to generate a job file
8863
if (ENABLE_ALL_TESTS)

‎libs/node/CMakeLists.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,11 @@ set(test_srcs
3838
test/TestMissNextTimeSlot.cpp
3939
test/TestMovePeer.cpp
4040
test/TestNodeBeginRequeue.cpp
41+
test/TestNodePathAlgorithms.cpp
4142
test/TestNodeState.cpp
4243
test/TestNode_main.cpp # test entry point
4344
test/TestOrder.cpp
45+
test/TestPermissions.cpp
4446
test/TestPersistence.cpp
4547
test/TestPreProcessing.cpp
4648
test/TestRepeatWithTimeDependencies.cpp

‎libs/base/src/ecflow/base/Algorithms.hpp‎ renamed to ‎libs/node/src/ecflow/node/NodePathAlgorithms.hpp‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,14 @@
88
* nor does it submit to any jurisdiction.
99
*/
1010

11-
#ifndef ecflow_base_Algorithms_hpp
12-
#define ecflow_base_Algorithms_hpp
11+
#ifndef ecflow_node_NodePathAlgorithms_hpp
12+
#define ecflow_node_NodePathAlgorithms_hpp
1313

1414
#include <string>
1515

1616
#include <boost/algorithm/string.hpp>
1717
#include <boost/tokenizer.hpp>
1818

19-
#include "ecflow/base/AbstractServer.hpp"
2019
#include "ecflow/core/Result.hpp"
2120
#include "ecflow/node/Defs.hpp"
2221

@@ -25,7 +24,7 @@ namespace ecf {
2524
///
2625
/// Represents a path in the server's hierarchy
2726
///
28-
/// A path object is always represents a valid path (even if it does not exist).
27+
/// A path object always represents a valid path (even if it does not exist).
2928
///
3029
/// A path with 0 tokens represents the root of the hierarchy (represented by a slash, "/").
3130
/// A path with N tokens represents a path from the root to a node.
@@ -104,9 +103,8 @@ void visit(const Defs& defs, const Path& path, PREDICATE& predicate) {
104103

105104
predicate.handle(*current);
106105
}
107-
108106
}
109107

110108
} // namespace ecf
111109

112-
#endif // ecflow_base_Algorithms_hpp
110+
#endif // ecflow_node_NodePathAlgorithms_hpp

‎libs/node/src/ecflow/node/permissions/ActivePermissions.cpp‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@
1010

1111
#include "ecflow/node/permissions/ActivePermissions.hpp"
1212

13+
#include "ecflow/core/Overload.hpp"
14+
#include "ecflow/node/Defs.hpp"
15+
#include "ecflow/node/NodePathAlgorithms.hpp"
16+
1317
namespace ecf {
1418

1519
void ActivePermissions::bootstrap(const Permissions& p) {
@@ -28,4 +32,58 @@ void ActivePermissions::combine_override(const Permissions& p) {
2832
}
2933
}
3034

35+
ActivePermissions
36+
permissions_at(const Defs& defs, const std::string& path, const std::variant<Unrestricted, Rules>& permissions) {
37+
ActivePermissions active;
38+
39+
std::visit(overload{[&active](const Unrestricted&) {
40+
// when no rules are loaded, we allow everything...
41+
// Dangerous, but backward compatible!
42+
active = ActivePermissions::make_empty();
43+
},
44+
[&defs, &active, &path](const Rules& rules) {
45+
struct Visitor
46+
{
47+
Visitor(ActivePermissions& collected) : collected_{collected} {}
48+
49+
void handle(const Defs& defs) {
50+
auto p = defs.server_state().permissions();
51+
52+
// At server level, we only care about the server permissions
53+
collected_.bootstrap(p);
54+
}
55+
void handle(const Node& n) {
56+
auto p = n.permissions();
57+
58+
if (auto s = dynamic_cast<const Suite*>(&n); s) {
59+
// At node level, if the node is a Suite we bootstrap the node permissions
60+
collected_.combine_supersede(p);
61+
}
62+
else {
63+
// ... otherwise, we combine the node permissions
64+
// -- in practice, this combination only restricts node permissions;
65+
// for example, a user can't be allowed to read/write/execute a
66+
// specific node if he can't do it at a higher node level
67+
collected_.combine_override(p);
68+
}
69+
}
70+
71+
void not_found() { /* do nothing */ }
72+
73+
private:
74+
ActivePermissions& collected_;
75+
};
76+
77+
auto p = Path::make(path).value();
78+
auto v = Visitor{active};
79+
80+
ecf::visit(defs, p, v);
81+
}
82+
83+
},
84+
permissions);
85+
86+
return active;
87+
}
88+
3189
} // namespace ecf

‎libs/node/src/ecflow/node/permissions/ActivePermissions.hpp‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,20 @@
88
* nor does it submit to any jurisdiction.
99
*/
1010

11-
#ifndef ecflow_node_Permissions_HPP
12-
#define ecflow_node_Permissions_HPP
11+
#ifndef ecflow_node_permissions_ActivePermissions_HPP
12+
#define ecflow_node_permissions_ActivePermissions_HPP
1313

1414
#include <algorithm>
1515
#include <iostream>
1616
#include <set>
1717
#include <string>
18+
#include <variant>
1819
#include <vector>
1920

2021
#include "ecflow/node/permissions/Permissions.hpp"
2122

23+
class Defs;
24+
2225
namespace ecf {
2326

2427
/**
@@ -84,6 +87,23 @@ class ActivePermissions {
8487
Permissions permissions_ = Permissions::make_empty();
8588
};
8689

90+
/**
91+
* \brief This is a Tag type used to indicate that the permissions follow the nominal rules.
92+
*/
93+
struct Rules
94+
{
95+
};
96+
97+
/**
98+
* \brief This is a Tag type used to indicate that the permissions are unrestricted.
99+
*/
100+
struct Unrestricted
101+
{
102+
};
103+
104+
ActivePermissions
105+
permissions_at(const Defs& defs, const std::string& path, const std::variant<Unrestricted, Rules>& permissions);
106+
87107
} // namespace ecf
88108

89-
#endif /* ecflow_node_Permissions_HPP */
109+
#endif /* ecflow_node_permissions_ActivePermissions_HPP */

‎libs/base/test/TestAlgorithms.cpp‎ renamed to ‎libs/node/test/TestNodePathAlgorithms.cpp‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,13 @@
88
* nor does it submit to any jurisdiction.
99
*/
1010

11-
#define BOOST_TEST_MODULE Test_Base
12-
#include <boost/test/included/unit_test.hpp>
11+
#include <boost/test/unit_test.hpp>
1312

14-
#include "MockServer.hpp"
15-
#include "ecflow/base/Algorithms.hpp"
1613
#include "ecflow/node/Family.hpp"
17-
#include "ecflow/server/BaseServer.hpp"
14+
#include "ecflow/node/NodePathAlgorithms.hpp"
1815
#include "ecflow/test/scaffold/Naming.hpp"
1916

20-
BOOST_AUTO_TEST_SUITE(U_Base)
17+
BOOST_AUTO_TEST_SUITE(U_Node)
2118

2219
BOOST_AUTO_TEST_SUITE(T_Path)
2320

‎libs/base/test/TestPermissions.cpp‎ renamed to ‎libs/node/test/TestPermissions.cpp‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,19 @@
88
* nor does it submit to any jurisdiction.
99
*/
1010

11+
#include <string>
12+
#include <vector>
13+
1114
#include <boost/test/unit_test.hpp>
1215

16+
#include "ecflow/core/Filesystem.hpp"
1317
#include "ecflow/node/Defs.hpp"
1418
#include "ecflow/node/Family.hpp"
1519
#include "ecflow/node/Suite.hpp"
16-
#include "ecflow/server/AuthorisationService.hpp"
1720
#include "ecflow/test/scaffold/Naming.hpp"
1821
#include "ecflow/test/scaffold/Provisioning.hpp"
19-
#include "harness/MockServer.hpp"
2022

21-
BOOST_AUTO_TEST_SUITE(U_Base)
23+
BOOST_AUTO_TEST_SUITE(U_Node)
2224

2325
BOOST_AUTO_TEST_SUITE(T_Permissions)
2426

@@ -69,7 +71,6 @@ BOOST_AUTO_TEST_CASE(can_do_permissions) {
6971
f->addVariable(Variable("PERMISSIONS", "u2:rx,u3:rw"));
7072
auto t = f->add_task("t1");
7173

72-
MockServer server(&d);
7374
d.server_state().add_or_update_server_variable("PERMISSIONS", "a:rwxos");
7475

7576
AuthorisationService service = AuthorisationService::load_permissions_from_nodes().value();

‎libs/server/src/ecflow/server/AuthorisationService.cpp‎

Lines changed: 2 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,12 @@
1111
#include "ecflow/server/AuthorisationService.hpp"
1212

1313
#include "ecflow/base/AbstractServer.hpp"
14-
#include "ecflow/base/Algorithms.hpp"
1514
#include "ecflow/core/Overload.hpp"
1615
#include "ecflow/node/Defs.hpp"
16+
#include "ecflow/node/permissions/ActivePermissions.hpp"
1717

1818
namespace ecf {
1919

20-
struct Rules
21-
{
22-
};
23-
24-
struct Unrestricted
25-
{
26-
};
27-
2820
struct AuthorisationService::Impl
2921
{
3022
Impl() : permissions_(Unrestricted{}) {}
@@ -56,57 +48,7 @@ bool AuthorisationService::good() const {
5648

5749
ActivePermissions AuthorisationService::permissions_at(const Defs& defs, const path_t& path) const {
5850
assert(good()); // It is up to the caller to check has been properly configured
59-
60-
ActivePermissions active;
61-
62-
std::visit(overload{[&active](const Unrestricted&) {
63-
// when no rules are loaded, we allow everything...
64-
// Dangerous, but backward compatible!
65-
active = ActivePermissions::make_empty();
66-
},
67-
[&defs, &active, &path](const Rules& rules) {
68-
struct Visitor
69-
{
70-
Visitor(ActivePermissions& collected) : collected_{collected} {}
71-
72-
void handle(const Defs& defs) {
73-
auto p = defs.server_state().permissions();
74-
75-
// At server level, we only care about the server permissions
76-
collected_.bootstrap(p);
77-
}
78-
void handle(const Node& n) {
79-
auto p = n.permissions();
80-
81-
if (auto s = dynamic_cast<const Suite*>(&n); s) {
82-
// At node level, if the node is a Suite we bootstrap the node permissions
83-
collected_.combine_supersede(p);
84-
}
85-
else {
86-
// ... otherwise, we combine the node permissions
87-
// -- in practice, this combination only restricts node permissions;
88-
// for example, a user can't be allowed to read/write/execute a
89-
// specific node if he can't do it at a higher node level
90-
collected_.combine_override(p);
91-
}
92-
}
93-
94-
void not_found() { /* do nothing */ }
95-
96-
private:
97-
ActivePermissions& collected_;
98-
};
99-
100-
auto p = Path::make(path).value();
101-
auto v = Visitor{active};
102-
103-
ecf::visit(defs, p, v);
104-
}
105-
106-
},
107-
impl_->permissions_);
108-
109-
return active;
51+
return ecf::permissions_at(defs, path, impl_->permissions_);
11052
}
11153

11254
bool AuthorisationService::allows(const Identity& identity, const Defs& defs, Allowed required) const {

0 commit comments

Comments
 (0)