Skip to content

Commit

Permalink
actually check permissions to fix tests
Browse files Browse the repository at this point in the history
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
  • Loading branch information
butonic committed Nov 5, 2020
1 parent 0574fbd commit 8e39d8b
Show file tree
Hide file tree
Showing 11 changed files with 313 additions and 98 deletions.
48 changes: 46 additions & 2 deletions accounts/pkg/service/v0/accounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@ import (
"github.com/golang/protobuf/ptypes/empty"
fieldmask_utils "github.com/mennanov/fieldmask-utils"
merrors "github.com/micro/go-micro/v2/errors"
"github.com/micro/go-micro/v2/metadata"
"github.com/owncloud/ocis/accounts/pkg/proto/v0"
"github.com/owncloud/ocis/accounts/pkg/storage"
"github.com/owncloud/ocis/ocis-pkg/middleware"
"github.com/owncloud/ocis/ocis-pkg/roles"
settings "github.com/owncloud/ocis/settings/pkg/proto/v0"
settings_svc "github.com/owncloud/ocis/settings/pkg/service/v0"
Expand Down Expand Up @@ -75,6 +77,22 @@ func (s Service) hasAccountManagementPermissions(ctx context.Context) bool {
return s.RoleManager.FindPermissionByID(ctx, roleIDs, AccountManagementPermissionID) != nil
}

func (s Service) hasSelfManagementPermissions(ctx context.Context) bool {
// get roles from context
roleIDs, ok := roles.ReadRoleIDsFromContext(ctx)
if !ok {
/**
* FIXME: with this we are skipping permission checks on all requests that are coming in without roleIDs in the
* metadata context. This is a huge security impairment, as that's the case not only for grpc requests but also
* for unauthenticated http requests and http requests coming in without hitting the ocis-proxy first.
*/
return true
}

// check if permission is present in roles of the authenticated account
return s.RoleManager.FindPermissionByID(ctx, roleIDs, SelfManagementPermissionID) != nil
}

// serviceUserToIndex temporarily adds a service user to the index, which is supposed to be removed before the lock on the handler function is released
func (s Service) serviceUserToIndex() (teardownServiceUser func()) {
if s.Config.ServiceUser.Username != "" && s.Config.ServiceUser.UUID != "" {
Expand Down Expand Up @@ -105,9 +123,12 @@ func (s Service) getInMemoryServiceUser() proto.Account {
// ListAccounts implements the AccountsServiceHandler interface
// the query contains account properties
func (s Service) ListAccounts(ctx context.Context, in *proto.ListAccountsRequest, out *proto.ListAccountsResponse) (err error) {
if !s.hasAccountManagementPermissions(ctx) {
hasSelf := s.hasSelfManagementPermissions(ctx)
hasManagement := s.hasAccountManagementPermissions(ctx)
if !hasSelf && !hasManagement {
return merrors.Forbidden(s.id, "no permission for ListAccounts")
}
onlySelf := hasSelf && !hasManagement

accLock.Lock()
defer accLock.Unlock()
Expand Down Expand Up @@ -146,6 +167,15 @@ func (s Service) ListAccounts(ctx context.Context, in *proto.ListAccountsRequest
return nil
}

if onlySelf {
// limit list to own account id
if aid, ok := metadata.Get(ctx, middleware.AccountID); ok {
in.Query = "id eq '" + aid + "'"
} else {
return merrors.InternalServerError(s.id, "account id not in context")
}
}

if in.Query == "" {
err = s.repo.LoadAccounts(ctx, &out.Accounts)
if err != nil {
Expand Down Expand Up @@ -202,9 +232,12 @@ func (s Service) findAccountsByQuery(ctx context.Context, query string) ([]strin

// GetAccount implements the AccountsServiceHandler interface
func (s Service) GetAccount(ctx context.Context, in *proto.GetAccountRequest, out *proto.Account) (err error) {
if !s.hasAccountManagementPermissions(ctx) {
hasSelf := s.hasSelfManagementPermissions(ctx)
hasManagement := s.hasAccountManagementPermissions(ctx)
if !hasSelf && !hasManagement {
return merrors.Forbidden(s.id, "no permission for GetAccount")
}
onlySelf := hasSelf && !hasManagement

accLock.Lock()
defer accLock.Unlock()
Expand All @@ -213,6 +246,17 @@ func (s Service) GetAccount(ctx context.Context, in *proto.GetAccountRequest, ou
return merrors.InternalServerError(s.id, "could not clean up account id: %v", err.Error())
}

if onlySelf {
// limit get to own account id
if aid, ok := metadata.Get(ctx, middleware.AccountID); ok {
if id != aid {
return merrors.Forbidden(s.id, "no permission for GetAccount of another user")
}
} else {
return merrors.InternalServerError(s.id, "account id not in context")
}
}

if err = s.repo.LoadAccount(ctx, id, out); err != nil {
if storage.IsNotFoundErr(err) {
return merrors.NotFound(s.id, "account not found: %v", err.Error())
Expand Down
29 changes: 26 additions & 3 deletions accounts/pkg/service/v0/permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,17 @@ import (

const (
// AccountManagementPermissionID is the hardcoded setting UUID for the account management permission
AccountManagementPermissionID string = "8e587774-d929-4215-910b-a317b1e80f73"
AccountManagementPermissionID string = "8e587774-d929-4215-910b-a317b1e80f73"
// AccountManagementPermissionName is the hardcoded setting name for the account management permission
AccountManagementPermissionName string = "account-management"
// GroupManagementPermissionID is the hardcoded setting UUID for the group management permission
GroupManagementPermissionID string = "522adfbe-5908-45b4-b135-41979de73245"
GroupManagementPermissionID string = "522adfbe-5908-45b4-b135-41979de73245"
// GroupManagementPermissionName is the hardcoded setting name for the group management permission
GroupManagementPermissionName string = "group-management"
GroupManagementPermissionName string = "group-management"
// SelfManagementPermissionID is the hardcoded setting UUID for the self management permission
SelfManagementPermissionID string = "e03070e9-4362-4cc6-a872-1c7cb2eb2b8e"
// SelfManagementPermissionName is the hardcoded setting name for the self management permission
SelfManagementPermissionName string = "self-management"
)

// RegisterPermissions registers permissions for account management and group management with the settings service.
Expand Down Expand Up @@ -78,5 +82,24 @@ func generateAccountManagementPermissionsRequests() []settings.AddSettingToBundl
},
},
},
{
BundleId: ssvc.BundleUUIDRoleUser,
Setting: &settings.Setting{
Id: SelfManagementPermissionID,
Name: SelfManagementPermissionName,
DisplayName: "Self Management",
Description: "This permission gives access to self management.",
Resource: &settings.Resource{
Type: settings.Resource_TYPE_USER,
Id: "me",
},
Value: &settings.Setting_PermissionValue{
PermissionValue: &settings.Permission{
Operation: settings.Permission_OPERATION_READWRITE,
Constraint: settings.Permission_CONSTRAINT_OWN,
},
},
},
},
}
}
2 changes: 2 additions & 0 deletions glauth/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ github.com/Azure/go-ntlmssp v0.0.0-20200615164410-66371956d46c/go.mod h1:chxPXzS
github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo=
github.com/CiscoM31/godata v0.0.0-20201003040028-eadcd34e7f06 h1:FKxVU/j9Dd8Je0YkVkm8Fxpz9zIeN21SEkcbzA6NWgY=
github.com/CiscoM31/godata v0.0.0-20201003040028-eadcd34e7f06/go.mod h1:tjaihnMBH6p5DVnGBksDQQHpErbrLvb9ek6cEWuyc7E=
github.com/DATA-DOG/go-sqlmock v1.3.3/go.mod h1:f/Ixk793poVmq4qj/V1dPUg2JEAKC73Q5eFN3EC/SaM=
github.com/GeertJohan/yubigo v0.0.0-20190917122436-175bc097e60e h1:Bqtt5C+uVk+vH/t5dmB47uDCTwxw16EYHqvJnmY2aQc=
github.com/GeertJohan/yubigo v0.0.0-20190917122436-175bc097e60e/go.mod h1:njRCDrl+1RQ/A/+KVU8Ho2EWAxUSkohOWczdW3dzDG0=
Expand Down
12 changes: 8 additions & 4 deletions ocis-pkg/middleware/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"net/http"

"github.com/cs3org/reva/pkg/token/manager/jwt"
"github.com/cs3org/reva/pkg/user"
"github.com/micro/go-micro/v2/metadata"
"github.com/owncloud/ocis/ocis-pkg/account"
)
Expand Down Expand Up @@ -49,18 +50,21 @@ func ExtractAccountUUID(opts ...account.Option) func(http.Handler) http.Handler
return
}

user, err := tokenManager.DismantleToken(r.Context(), token)
u, err := tokenManager.DismantleToken(r.Context(), token)
if err != nil {
opt.Logger.Error().Err(err)
return
}

// store user in context for request
ctx := user.ContextSetUser(r.Context(), u)

// Important: user.Id.OpaqueId is the AccountUUID. Set this way in the account uuid middleware in ocis-proxy.
// https://github.com/owncloud/ocis-proxy/blob/ea254d6036592cf9469d757d1295e0c4309d1e63/pkg/middleware/account_uuid.go#L109
ctx := context.WithValue(r.Context(), UUIDKey, user.Id.OpaqueId)
ctx = context.WithValue(ctx, UUIDKey, u.Id.OpaqueId)
// TODO: implement token manager in cs3org/reva that uses generic metadata instead of access token from header.
ctx = metadata.Set(ctx, AccountID, user.Id.OpaqueId)
ctx = metadata.Set(ctx, RoleIDs, string(user.Opaque.Map["roles"].Value))
ctx = metadata.Set(ctx, AccountID, u.Id.OpaqueId)
ctx = metadata.Set(ctx, RoleIDs, string(u.Opaque.Map["roles"].Value))
next.ServeHTTP(w, r.WithContext(ctx))
})
}
Expand Down
40 changes: 0 additions & 40 deletions ocs/pkg/middleware/accesstoken.go

This file was deleted.

12 changes: 6 additions & 6 deletions ocs/pkg/middleware/options.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package middleware

import (
"github.com/owncloud/ocis/ocs/pkg/config"
"github.com/owncloud/ocis/ocis-pkg/log"
"github.com/owncloud/ocis/ocis-pkg/roles"
)

// Option defines a single option function.
Expand All @@ -12,8 +12,8 @@ type Option func(o *Options)
type Options struct {
// Logger to use for logging, must be set
Logger log.Logger
// TokenManagerConfig for communicating with the reva token manager
TokenManagerConfig config.TokenManager
// RoleManager for looking up permissions
RoleManager *roles.Manager
}

// newOptions initializes the available default options.
Expand All @@ -34,9 +34,9 @@ func Logger(l log.Logger) Option {
}
}

// TokenManagerConfig provides a function to set the token manger config option.
func TokenManagerConfig(cfg config.TokenManager) Option {
// RoleManager provides a function to set the RoleManager option.
func RoleManager(val *roles.Manager) Option {
return func(o *Options) {
o.TokenManagerConfig = cfg
o.RoleManager = val
}
}
37 changes: 37 additions & 0 deletions ocs/pkg/middleware/requireadmin.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package middleware

import (
"net/http"

"github.com/go-chi/render"
accounts "github.com/owncloud/ocis/accounts/pkg/service/v0"
"github.com/owncloud/ocis/ocis-pkg/roles"
"github.com/owncloud/ocis/ocs/pkg/service/v0/data"
"github.com/owncloud/ocis/ocs/pkg/service/v0/response"
)

// RequireAdmin middleware is used to require the user in context to be an admin / have account management permissions
func RequireAdmin(opts ...Option) func(next http.Handler) http.Handler {
opt := newOptions(opts...)

return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

// get roles from context
roleIDs, ok := roles.ReadRoleIDsFromContext(r.Context())
if !ok {
render.Render(w, r, response.ErrRender(data.MetaUnauthorized.StatusCode, "Unauthorized"))
return
}

// check if permission is present in roles of the authenticated account
if opt.RoleManager.FindPermissionByID(r.Context(), roleIDs, accounts.AccountManagementPermissionID) != nil {
next.ServeHTTP(w, r)
return
}

render.Render(w, r, response.ErrRender(data.MetaUnauthorized.StatusCode, "Unauthorized"))

})
}
}
57 changes: 57 additions & 0 deletions ocs/pkg/middleware/requireselforadmin.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package middleware

import (
"net/http"

"github.com/cs3org/reva/pkg/user"
"github.com/go-chi/chi"
"github.com/go-chi/render"
accounts "github.com/owncloud/ocis/accounts/pkg/service/v0"
"github.com/owncloud/ocis/ocis-pkg/roles"
"github.com/owncloud/ocis/ocs/pkg/service/v0/data"
"github.com/owncloud/ocis/ocs/pkg/service/v0/response"
)

// RequireSelfOrAdmin middleware is used to require the requesting user to be an admin or the requested user himself
func RequireSelfOrAdmin(opts ...Option) func(next http.Handler) http.Handler {
opt := newOptions(opts...)

return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

u, ok := user.ContextGetUser(r.Context())
if !ok {
render.Render(w, r, response.ErrRender(data.MetaUnauthorized.StatusCode, "Unauthorized"))
return
}
if u.Id == nil || u.Id.OpaqueId == "" {
render.Render(w, r, response.ErrRender(data.MetaBadRequest.StatusCode, "user is missing an id"))
return
}
// get roles from context
roleIDs, ok := roles.ReadRoleIDsFromContext(r.Context())
if !ok {
render.Render(w, r, response.ErrRender(data.MetaUnauthorized.StatusCode, "Unauthorized"))
return
}

// check if account management permission is present in roles of the authenticated account
if opt.RoleManager.FindPermissionByID(r.Context(), roleIDs, accounts.AccountManagementPermissionID) != nil {
next.ServeHTTP(w, r)
return
}

// check if self management permission is present in roles of the authenticated account
if opt.RoleManager.FindPermissionByID(r.Context(), roleIDs, accounts.SelfManagementPermissionID) != nil {
userid := chi.URLParam(r, "userid")
if userid == "" || userid == u.Id.OpaqueId || userid == u.Username {
next.ServeHTTP(w, r)
return
}
}

render.Render(w, r, response.ErrRender(data.MetaUnauthorized.StatusCode, "Unauthorized"))

})
}
}
Loading

0 comments on commit 8e39d8b

Please sign in to comment.