fix: cherry pick unified-role federation changes

This commit is contained in:
Florian Schade
2024-08-21 14:10:46 +02:00
parent 9b0c8eb14b
commit 4f2678351d
2 changed files with 88 additions and 12 deletions

View File

@@ -3,6 +3,7 @@ package unifiedrole
import (
"cmp"
"slices"
"strings"
libregraph "github.com/owncloud/libre-graph-api-go"
"google.golang.org/protobuf/proto"
@@ -29,6 +30,18 @@ const (
UnifiedRoleManagerID = "312c0871-5ef7-4b3a-85b6-0e4074c64049"
// UnifiedRoleSecureViewerID Unified role secure viewer id.
UnifiedRoleSecureViewerID = "aa97fe03-7980-45ac-9e50-b325749fd7e6"
// UnifiedRoleFederatedViewerID Unified role federated viewer id.
UnifiedRoleFederatedViewerID = "be531789-063c-48bf-a9fe-857e6fbee7da"
// UnifiedRoleFederatedEditorID Unified role federated editor id.
UnifiedRoleFederatedEditorID = "36279a93-e4e3-4bbb-8a23-53b05b560963"
// Wile the below conditions follow the SDDL syntax, they are not parsed anywhere. We use them as strings to
// represent the constraints that a role definition applies to. For the actual syntax, see the SDDL documentation
// at https://learn.microsoft.com/en-us/windows/win32/secauthz/security-descriptor-definition-language-for-conditional-aces-#conditional-expressions
// Some roles apply to a specific type of resource, for example, a role that applies to a file or a folder.
// @Resource is the placeholder for the resource that the role is applied to
// .Root, .Folder and .File are facets of the driveItem resource that indicate the type of the resource if they are present.
// UnifiedRoleConditionDrive defines constraint that matches a Driveroot/Spaceroot
UnifiedRoleConditionDrive = "exists @Resource.Root"
@@ -37,6 +50,19 @@ const (
// UnifiedRoleConditionFile defines a constraint that matches a DriveItem representing a File
UnifiedRoleConditionFile = "exists @Resource.File"
// Some roles apply to a specific type of user, for example, a role that applies to a federated user.
// @Subject is the placeholder for the subject that the role is applied to. For sharing roles this is the user that the resource is shared with.
// .UserType is the type of the user: 'Member' for a member of the organization, 'Guest' for a guest user, 'Federated' for a federated user.
// UnifiedRoleConditionFederatedUser defines a constraint that matches a federated user
UnifiedRoleConditionFederatedUser = "@Subject.UserType==\"Federated\""
// For federated sharing we need roles that combine the constraints for the resource and the user.
// UnifiedRoleConditionFileFederatedUser defines a constraint that matches a File and a federated user
UnifiedRoleConditionFileFederatedUser = UnifiedRoleConditionFile + " && " + UnifiedRoleConditionFederatedUser
// UnifiedRoleConditionFolderFederatedUser defines a constraint that matches a Folder and a federated user
UnifiedRoleConditionFolderFederatedUser = UnifiedRoleConditionFolder + " && " + UnifiedRoleConditionFederatedUser
DriveItemPermissionsCreate = "libre.graph/driveItem/permissions/create"
DriveItemChildrenCreate = "libre.graph/driveItem/children/create"
DriveItemStandardDelete = "libre.graph/driveItem/standard/delete"
@@ -149,6 +175,14 @@ var (
AllowedResourceActions: CS3ResourcePermissionsToLibregraphActions(r.CS3ResourcePermissions()),
Condition: proto.String(UnifiedRoleConditionFolder),
},
{
AllowedResourceActions: CS3ResourcePermissionsToLibregraphActions(r.CS3ResourcePermissions()),
Condition: proto.String(UnifiedRoleConditionFileFederatedUser),
},
{
AllowedResourceActions: CS3ResourcePermissionsToLibregraphActions(r.CS3ResourcePermissions()),
Condition: proto.String(UnifiedRoleConditionFolderFederatedUser),
},
},
LibreGraphWeight: proto.Int32(0),
}
@@ -183,6 +217,10 @@ var (
AllowedResourceActions: CS3ResourcePermissionsToLibregraphActions(r.CS3ResourcePermissions()),
Condition: proto.String(UnifiedRoleConditionFolder),
},
{
AllowedResourceActions: CS3ResourcePermissionsToLibregraphActions(r.CS3ResourcePermissions()),
Condition: proto.String(UnifiedRoleConditionFolderFederatedUser),
},
},
LibreGraphWeight: proto.Int32(0),
}
@@ -217,6 +255,10 @@ var (
AllowedResourceActions: CS3ResourcePermissionsToLibregraphActions(r.CS3ResourcePermissions()),
Condition: proto.String(UnifiedRoleConditionFile),
},
{
AllowedResourceActions: CS3ResourcePermissionsToLibregraphActions(r.CS3ResourcePermissions()),
Condition: proto.String(UnifiedRoleConditionFileFederatedUser),
},
},
LibreGraphWeight: proto.Int32(0),
}
@@ -295,14 +337,21 @@ func GetRole(f RoleFilter) (*libregraph.UnifiedRoleDefinition, error) {
// GetRolesByPermissions returns a list of role definitions
// that match the provided actions and constraints
func GetRolesByPermissions(roleSet []*libregraph.UnifiedRoleDefinition, actions []string, constraints string, descending bool) []*libregraph.UnifiedRoleDefinition {
func GetRolesByPermissions(roleSet []*libregraph.UnifiedRoleDefinition, actions []string, constraints string, listFederatedRoles, descending bool) []*libregraph.UnifiedRoleDefinition {
roles := make([]*libregraph.UnifiedRoleDefinition, 0, len(roleSet))
for _, role := range roleSet {
var match bool
for _, permission := range role.GetRolePermissions() {
if permission.GetCondition() != constraints {
// this is a dirty comparison because we are not really parsing the SDDL, but as long as we && the conditions we are good
isFederatedRole := strings.Contains(permission.GetCondition(), UnifiedRoleConditionFederatedUser)
switch {
case !strings.Contains(permission.GetCondition(), constraints):
continue
case listFederatedRoles && !isFederatedRole:
continue
case !listFederatedRoles && isFederatedRole:
continue
}

View File

@@ -93,9 +93,10 @@ func TestGetRolesByPermissions(t *testing.T) {
tests := map[string]struct {
givenActions []string
constraints string
listFederatedRoles bool
unifiedRoleDefinition []*libregraph.UnifiedRoleDefinition
}{
"ViewerUnifiedRole": {
"RoleViewer | folder": {
givenActions: getRoleActions(unifiedrole.RoleViewer),
constraints: unifiedrole.UnifiedRoleConditionFolder,
unifiedRoleDefinition: []*libregraph.UnifiedRoleDefinition{
@@ -103,7 +104,7 @@ func TestGetRolesByPermissions(t *testing.T) {
unifiedrole.RoleViewer,
},
},
"ViewerUnifiedRole | share": {
"RoleViewer | file": {
givenActions: getRoleActions(unifiedrole.RoleViewer),
constraints: unifiedrole.UnifiedRoleConditionFile,
unifiedRoleDefinition: []*libregraph.UnifiedRoleDefinition{
@@ -111,7 +112,15 @@ func TestGetRolesByPermissions(t *testing.T) {
unifiedrole.RoleViewer,
},
},
"NewFileEditorUnifiedRole": {
"RoleViewer | file | federated": {
givenActions: getRoleActions(unifiedrole.RoleViewer),
constraints: unifiedrole.UnifiedRoleConditionFile,
listFederatedRoles: true,
unifiedRoleDefinition: []*libregraph.UnifiedRoleDefinition{
unifiedrole.RoleViewer,
},
},
"RoleFileEditor | file": {
givenActions: getRoleActions(unifiedrole.RoleFileEditor),
constraints: unifiedrole.UnifiedRoleConditionFile,
unifiedRoleDefinition: []*libregraph.UnifiedRoleDefinition{
@@ -120,7 +129,7 @@ func TestGetRolesByPermissions(t *testing.T) {
unifiedrole.RoleFileEditor,
},
},
"NewEditorUnifiedRole": {
"RoleEditor | folder": {
givenActions: getRoleActions(unifiedrole.RoleEditor),
constraints: unifiedrole.UnifiedRoleConditionFolder,
unifiedRoleDefinition: []*libregraph.UnifiedRoleDefinition{
@@ -130,7 +139,25 @@ func TestGetRolesByPermissions(t *testing.T) {
unifiedrole.RoleEditor,
},
},
"GetRoles 1": {
"RoleEditor | folder | federated": {
givenActions: getRoleActions(unifiedrole.RoleEditor),
constraints: unifiedrole.UnifiedRoleConditionFolder,
listFederatedRoles: true,
unifiedRoleDefinition: []*libregraph.UnifiedRoleDefinition{
unifiedrole.RoleViewer,
unifiedrole.RoleEditor,
},
},
"RoleEditor | file | federated": {
givenActions: getRoleActions(unifiedrole.RoleEditor),
constraints: unifiedrole.UnifiedRoleConditionFile,
listFederatedRoles: true,
unifiedRoleDefinition: []*libregraph.UnifiedRoleDefinition{
unifiedrole.RoleViewer,
unifiedrole.RoleFileEditor,
},
},
"BuildInRoles | file": {
givenActions: getRoleActions(unifiedrole.BuildInRoles...),
constraints: unifiedrole.UnifiedRoleConditionFile,
unifiedRoleDefinition: []*libregraph.UnifiedRoleDefinition{
@@ -139,7 +166,7 @@ func TestGetRolesByPermissions(t *testing.T) {
unifiedrole.RoleFileEditor,
},
},
"GetRoles 2": {
"BuildInRoles | folder": {
givenActions: getRoleActions(unifiedrole.BuildInRoles...),
constraints: unifiedrole.UnifiedRoleConditionFolder,
unifiedRoleDefinition: []*libregraph.UnifiedRoleDefinition{
@@ -149,7 +176,7 @@ func TestGetRolesByPermissions(t *testing.T) {
unifiedrole.RoleEditor,
},
},
"GetRoles 3": {
"BuildInRoles | drive": {
givenActions: getRoleActions(unifiedrole.BuildInRoles...),
constraints: unifiedrole.UnifiedRoleConditionDrive,
unifiedRoleDefinition: []*libregraph.UnifiedRoleDefinition{
@@ -158,12 +185,12 @@ func TestGetRolesByPermissions(t *testing.T) {
unifiedrole.RoleManager,
},
},
"single": {
"custom | file": {
givenActions: []string{unifiedrole.DriveItemQuotaRead},
constraints: unifiedrole.UnifiedRoleConditionFile,
unifiedRoleDefinition: []*libregraph.UnifiedRoleDefinition{},
},
"mixed": {
"RoleEditorLite and custom | folder": {
givenActions: append(getRoleActions(unifiedrole.RoleEditorLite), unifiedrole.DriveItemQuotaRead),
constraints: unifiedrole.UnifiedRoleConditionFolder,
unifiedRoleDefinition: []*libregraph.UnifiedRoleDefinition{
@@ -176,7 +203,7 @@ func TestGetRolesByPermissions(t *testing.T) {
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
g := NewWithT(t)
generatedDefinitions := unifiedrole.GetRolesByPermissions(unifiedrole.BuildInRoles, tc.givenActions, tc.constraints, false)
generatedDefinitions := unifiedrole.GetRolesByPermissions(unifiedrole.BuildInRoles, tc.givenActions, tc.constraints, tc.listFederatedRoles, false)
g.Expect(len(generatedDefinitions)).To(Equal(len(tc.unifiedRoleDefinition)))