Skip to content
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

add set space quota permission #2459

Merged
merged 1 commit into from
Sep 1, 2021
Merged

add set space quota permission #2459

merged 1 commit into from
Sep 1, 2021

Conversation

C0rby
Copy link
Contributor

@C0rby C0rby commented Sep 1, 2021

In preparation for the upcoming spaces features a SetSpaceQuota permission was added.

I'm not entirely sure about the permission to role assignment. Especially about the ResourceType:

BundleId: BundleUUIDRoleAdmin,
  Setting: &settings.Setting{
	Id:          SetSpaceQuotaPermissionID,
	Name:        SetSpaceQuotaPermissionName,
	DisplayName: "Set Space Quota",
	Description: "This permission allows to manage space quotas.",
	Resource: &settings.Resource{
		Type: settings.Resource_TYPE_SYSTEM,
	},
	Value: &settings.Setting_PermissionValue{
		PermissionValue: &settings.Permission{
			Operation:  settings.Permission_OPERATION_READWRITE,
			Constraint: settings.Permission_CONSTRAINT_ALL,
		},
	},
  },

@sonarcloud
Copy link

sonarcloud bot commented Sep 1, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@refs
Copy link
Member

refs commented Sep 1, 2021

I just saw there is no documentation regarding the current roles and permissions service, and this is an error. There should always be documentation regarding core concepts such as this one. Let me try to shed some light as best as I can.

Let's have a look at a production example:

		{
			BundleId: BundleUUIDRoleAdmin,
			Setting: &settings.Setting{
				Id:          "7d81f103-0488-4853-bce5-98dcce36d649",
				Name:        "language-readwrite",
				DisplayName: "Permission to read and set the language (anyone)",
				Resource: &settings.Resource{
					Type: settings.Resource_TYPE_SETTING,
					Id:   settingUUIDProfileLanguage,
				},
				Value: &settings.Setting_PermissionValue{
					PermissionValue: &settings.Permission{
						Operation:  settings.Permission_OPERATION_READWRITE,
						Constraint: settings.Permission_CONSTRAINT_ALL,
					},
				},
			},
		},

This is a permission being created and can be read as:

[role admin] --- on resource type --> [user] --- can -- [RW] (let's omit CONSTRAINT_ALL)

In short, an admin can edit the language setting of any user. So we are granting any user with the admin role a power to edit the settings on any user, and that setting is a Resource_TYPE_SETTING that it is defined here

If I read yours doing the same analysis:

		{
			BundleId: BundleUUIDRoleAdmin,
			Setting: &settings.Setting{
				Id:          SetSpaceQuotaPermissionID,
				Name:        SetSpaceQuotaPermissionName,
				DisplayName: "Set Space Quota",
				Description: "This permission allows to manage space quotas.",
				Resource: &settings.Resource{
					Type: settings.Resource_TYPE_SYSTEM,
				},
				Value: &settings.Setting_PermissionValue{
					PermissionValue: &settings.Permission{
						Operation:  settings.Permission_OPERATION_READWRITE,
						Constraint: settings.Permission_CONSTRAINT_ALL,
					},
				},
			},
		},

The first thing that comes to mind is that the permission is being applied on no resource (because there is no Id) so we could interpret that as a "system wide permission for the Admin role" (we just have to enforce it). So it seems from a design point of view correct to me, the good part comes with enforcing it; this could be "simple™️" because as it is a system wide we just need to check that the current user role allows for it.

And I think you can drop the line Constraint: settings.Permission_CONSTRAINT_ALL, because as far as my grep prowess go it is not used anywhere.

@C0rby
Copy link
Contributor Author

C0rby commented Sep 1, 2021

And I think you can drop the line Constraint: settings.Permission_CONSTRAINT_ALL, because as far as my grep prowess go it is not used anywhere.

Ok but then I also can just leave it in. :D

@C0rby C0rby merged commit 1ab1fb9 into master Sep 1, 2021
@C0rby C0rby deleted the set-quota-permission branch September 1, 2021 15:10
ownclouders pushed a commit that referenced this pull request Sep 1, 2021
Merge: 7b1a7b2 aeafa46
Author: David Christofas <dchristofas@owncloud.com>
Date:   Wed Sep 1 11:10:52 2021 -0400

    Merge pull request #2459 from owncloud/set-quota-permission

    add set space quota permission
@micbar micbar mentioned this pull request Sep 15, 2021
20 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants