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

Introduce permission checks for WRITE access via http #1092

Merged
merged 3 commits into from
Dec 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions changelog/unreleased/settings-permissions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Bugfix: Permission checks for settings write access

Tags: settings

There were several endpoints with write access to the settings service that were not protected by permission checks. We introduced a generic settings management permission to fix this for now. Will be more fine grained later on.

https://github.com/owncloud/ocis/pull/1092
5 changes: 4 additions & 1 deletion ocis-pkg/middleware/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package middleware

import (
"context"
"encoding/json"
"net/http"

"github.com/cs3org/reva/pkg/token/manager/jwt"
Expand Down Expand Up @@ -46,7 +47,9 @@ func ExtractAccountUUID(opts ...account.Option) func(http.Handler) http.Handler
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
token := r.Header.Get("x-access-token")
if len(token) == 0 {
next.ServeHTTP(w, r)
roleIDsJSON, _ := json.Marshal([]string{})
ctx := metadata.Set(r.Context(), RoleIDs, string(roleIDsJSON))
next.ServeHTTP(w, r.WithContext(ctx))
return
}

Expand Down
2 changes: 2 additions & 0 deletions settings/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ github.com/cs3org/go-cs3apis v0.0.0-20200730121022-c4f3d4f7ddfd/go.mod h1:UXha4T
github.com/cs3org/go-cs3apis v0.0.0-20200810113633-b00aca449666/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
github.com/cs3org/go-cs3apis v0.0.0-20201007120910-416ed6cf8b00 h1:LVl25JaflluOchVvaHWtoCynm5OaM+VNai0IYkcCSe0=
github.com/cs3org/go-cs3apis v0.0.0-20201007120910-416ed6cf8b00/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
github.com/cs3org/go-cs3apis v0.0.0-20201118090759-87929f5bae21 h1:mZpylrgnCgSeaZ5EznvHIPIKuaQHMHZDi2wkJtk4M8Y=
github.com/cs3org/go-cs3apis v0.0.0-20201118090759-87929f5bae21/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
github.com/cs3org/reva v1.1.0 h1:Gih6ECHvMMGSx523SFluFlDmNMuhYelXYShdWvjvW38=
github.com/cs3org/reva v1.1.0/go.mod h1:fBzTrNuAKdQ62ybjpdu8nyhBin90/3/3s6DGQDCdBp4=
Expand Down Expand Up @@ -1332,6 +1333,7 @@ golang.org/x/sys v0.0.0-20200720211630-cb9d2d5c5666/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20200909081042-eff7692f9009/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200918174421-af09f7315aff h1:1CPUrky56AcgSpxz/KfgzQWzfG09u5YOL8MvPYBlrL8=
golang.org/x/sys v0.0.0-20200918174421-af09f7315aff/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20201101102859-da207088b7d1 h1:a/mKvvZr9Jcc8oKfcmgzyp7OwF73JPWsQLvH1z2Kxck=
golang.org/x/sys v0.0.0-20201101102859-da207088b7d1/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
Expand Down
4 changes: 3 additions & 1 deletion settings/pkg/proto/v0/settings.pb.micro_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ func TestGetBundleHavingFullPermissionsOnAnotherRole(t *testing.T) {
defer teardown()

ctx := metadata.Set(context.Background(), middleware.AccountID, testAccountID)
ctx = metadata.Set(ctx, middleware.RoleIDs, getRoleIDAsJSON(svc.BundleUUIDRoleUser))
ctx = metadata.Set(ctx, middleware.RoleIDs, getRoleIDAsJSON(svc.BundleUUIDRoleAdmin))

saveRequest := proto.SaveBundleRequest{
Bundle: &bundleStub,
Expand All @@ -392,6 +392,8 @@ func TestGetBundleHavingFullPermissionsOnAnotherRole(t *testing.T) {
assert.Equal(t, bundleStub.Id, saveResponse.Bundle.Id)

setFullReadWriteOnBundleForAdmin(ctx, t, bundleStub.Id)

ctx = metadata.Set(ctx, middleware.RoleIDs, getRoleIDAsJSON(svc.BundleUUIDRoleUser))
getRequest := proto.GetBundleRequest{BundleId: bundleStub.Id}
getResponse, err := bundleService.GetBundle(ctx, &getRequest)
assert.Empty(t, getResponse)
Expand Down
45 changes: 39 additions & 6 deletions settings/pkg/service/v0/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,13 @@ func (g Service) RegisterDefaultRoles() {
// SaveBundle implements the BundleServiceHandler interface
func (g Service) SaveBundle(ctx context.Context, req *proto.SaveBundleRequest, res *proto.SaveBundleResponse) error {
cleanUpResource(ctx, req.Bundle.Resource)
if err := g.checkStaticPermissionsByBundleType(ctx, req.Bundle.Type); err != nil {
return err
}
if validationError := validateSaveBundle(req); validationError != nil {
return merrors.BadRequest(g.id, "%s", validationError)
}

r, err := g.manager.WriteBundle(req.Bundle)
if err != nil {
return merrors.BadRequest(g.id, "%s", err)
Expand Down Expand Up @@ -164,9 +168,13 @@ func (g Service) getFilteredBundle(roleIDs []string, bundle *proto.Bundle) *prot
// AddSettingToBundle implements the BundleServiceHandler interface
func (g Service) AddSettingToBundle(ctx context.Context, req *proto.AddSettingToBundleRequest, res *proto.AddSettingToBundleResponse) error {
cleanUpResource(ctx, req.Setting.Resource)
if err := g.checkStaticPermissionsByBundleID(ctx, req.BundleId); err != nil {
return err
}
if validationError := validateAddSettingToBundle(req); validationError != nil {
return merrors.BadRequest(g.id, "%s", validationError)
}

r, err := g.manager.AddSettingToBundle(req.BundleId, req.Setting)
if err != nil {
return merrors.BadRequest(g.id, "%s", err)
Expand All @@ -177,9 +185,13 @@ func (g Service) AddSettingToBundle(ctx context.Context, req *proto.AddSettingTo

// RemoveSettingFromBundle implements the BundleServiceHandler interface
func (g Service) RemoveSettingFromBundle(ctx context.Context, req *proto.RemoveSettingFromBundleRequest, _ *empty.Empty) error {
if err := g.checkStaticPermissionsByBundleID(ctx, req.BundleId); err != nil {
return err
}
if validationError := validateRemoveSettingFromBundle(req); validationError != nil {
return merrors.BadRequest(g.id, "%s", validationError)
}

if err := g.manager.RemoveSettingFromBundle(req.BundleId, req.SettingId); err != nil {
return merrors.BadRequest(g.id, "%s", err)
}
Expand Down Expand Up @@ -297,8 +309,8 @@ func (g Service) ListRoleAssignments(ctx context.Context, req *proto.ListRoleAss

// AssignRoleToUser implements the RoleServiceHandler interface
func (g Service) AssignRoleToUser(ctx context.Context, req *proto.AssignRoleToUserRequest, res *proto.AssignRoleToUserResponse) error {
if !g.hasRoleManagementPermission(ctx) {
return merrors.Forbidden(g.id, "the user is not allowed to assign roles")
if err := g.checkStaticPermissionsByBundleType(ctx, proto.Bundle_TYPE_ROLE); err != nil {
return err
}

req.AccountUuid = getValidatedAccountUUID(ctx, req.AccountUuid)
Expand All @@ -315,8 +327,8 @@ func (g Service) AssignRoleToUser(ctx context.Context, req *proto.AssignRoleToUs

// RemoveRoleFromUser implements the RoleServiceHandler interface
func (g Service) RemoveRoleFromUser(ctx context.Context, req *proto.RemoveRoleFromUserRequest, _ *empty.Empty) error {
if !g.hasRoleManagementPermission(ctx) {
return merrors.Forbidden(g.id, "the user is not allowed to assign roles")
if err := g.checkStaticPermissionsByBundleType(ctx, proto.Bundle_TYPE_ROLE); err != nil {
return err
}

if validationError := validateRemoveRoleFromUser(req); validationError != nil {
Expand Down Expand Up @@ -406,7 +418,7 @@ func (g Service) getValueWithIdentifier(value *proto.Value) (*proto.ValueWithIde
}, nil
}

func (g Service) hasRoleManagementPermission(ctx context.Context) bool {
func (g Service) hasStaticPermission(ctx context.Context, permissionID string) bool {
roleIDs, ok := roles.ReadRoleIDsFromContext(ctx)
if !ok {
/**
Expand All @@ -420,6 +432,27 @@ func (g Service) hasRoleManagementPermission(ctx context.Context) bool {
// tracked as OCIS-454
return true
}
p, err := g.manager.ReadPermissionByID(RoleManagementPermissionID, roleIDs)
p, err := g.manager.ReadPermissionByID(permissionID, roleIDs)
return err == nil && p != nil
}

func (g Service) checkStaticPermissionsByBundleID(ctx context.Context, bundleID string) error {
bundle, err := g.manager.ReadBundle(bundleID)
if err != nil {
return merrors.NotFound(g.id, "bundle not found: %s", err)
}
return g.checkStaticPermissionsByBundleType(ctx, bundle.Type)
}

func (g Service) checkStaticPermissionsByBundleType(ctx context.Context, bundleType proto.Bundle_Type) error {
if bundleType == proto.Bundle_TYPE_ROLE {
if !g.hasStaticPermission(ctx, RoleManagementPermissionID) {
return merrors.Forbidden(g.id, "user has no role management permission")
}
return nil
}
if !g.hasStaticPermission(ctx, SettingsManagementPermissionID) {
return merrors.Forbidden(g.id, "user has no settings management permission")
}
return nil
}
24 changes: 24 additions & 0 deletions settings/pkg/service/v0/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ const (
RoleManagementPermissionID string = "a53e601e-571f-4f86-8fec-d4576ef49c62"
// RoleManagementPermissionName is the hardcoded setting name for the role management permission
RoleManagementPermissionName string = "role-management"

// SettingsManagementPermissionID is the hardcoded setting UUID for the settings management permission
SettingsManagementPermissionID string = "79e13b30-3e22-11eb-bc51-0b9f0bad9a58"
// SettingsManagementPermissionName is the hardcoded setting name for the settings management permission
SettingsManagementPermissionName string = "settings-management"
)

// generateBundlesDefaultRoles bootstraps the default roles.
Expand Down Expand Up @@ -90,5 +95,24 @@ func generatePermissionRequests() []*settings.AddSettingToBundleRequest {
},
},
},
{
BundleId: BundleUUIDRoleAdmin,
Setting: &settings.Setting{
Id: SettingsManagementPermissionID,
Name: SettingsManagementPermissionName,
DisplayName: "Settings Management",
Description: "This permission gives full access to everything that is related to settings management.",
Resource: &settings.Resource{
Type: settings.Resource_TYPE_USER,
Id: "all",
},
Value: &settings.Setting_PermissionValue{
PermissionValue: &settings.Permission{
Operation: settings.Permission_OPERATION_READWRITE,
Constraint: settings.Permission_CONSTRAINT_ALL,
},
},
},
},
}
}