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

Enhancement: calculate and expose actual file permission set #1368

Merged
Merged
Show file tree
Hide file tree
Changes from 40 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
8e485c2
prevent sharing with increased permissions
butonic Dec 10, 2020
a0cd57f
refactor permission mapping to a role struct
butonic Dec 10, 2020
30af05b
stop faking dav permissions
butonic Dec 10, 2020
2974e0c
make ocis driver calculate permissions
butonic Dec 11, 2020
d0c6fdc
return all permissions when user is owner wherever possible
butonic Dec 11, 2020
c8283b0
refactor reading and assembling permissions
butonic Dec 14, 2020
ef0c8f4
fake permissions for share recipients at the driver level
butonic Dec 14, 2020
8de73c2
fix permission conversion
butonic Dec 16, 2020
bedbc4e
fix lint, less expected failed tests
butonic Dec 16, 2020
7add009
less expected failures
butonic Dec 16, 2020
2a5d5b0
add changelog
butonic Dec 16, 2020
15c657e
less expected failures
butonic Dec 16, 2020
cd48234
cleanup after rebase
butonic Dec 16, 2020
11e06d0
enforce public share permissions
butonic Dec 17, 2020
ef6daa5
implement files drop
butonic Dec 18, 2020
8683de2
file shares never have delete or create permission
butonic Dec 18, 2020
5e7afad
fix typo
butonic Dec 18, 2020
390604c
files never have a create permission
butonic Dec 18, 2020
cf51af0
differentiate not found vs unauthenticated for link tokens
butonic Dec 18, 2020
68875a7
not found token should return 403
butonic Dec 18, 2020
997ca40
guard against missing permissionset
butonic Dec 18, 2020
59c9f95
differentiate between oc:permissions and ocs:share-permissions
butonic Dec 21, 2020
98e5117
ace read flag grants GetPath
butonic Dec 21, 2020
c3a2fa5
forward more error types
butonic Dec 21, 2020
498fa3f
update expected failures
butonic Dec 21, 2020
9500fbc
return missing link target as 404
butonic Dec 22, 2020
3fda868
fix permission check
butonic Jan 4, 2021
f3f558a
treat permission denied as 401
butonic Jan 4, 2021
392a83f
update local test configs
butonic Jan 4, 2021
d85b5b6
different default permissions
butonic Jan 4, 2021
d6321bc
handle file individual share permissions
butonic Jan 4, 2021
5e8fa19
update unexpected passes
butonic Jan 4, 2021
7f2ca3a
Apply suggestions from code review
butonic Jan 5, 2021
605fb1f
clarify ocPublicPermToRole
butonic Jan 5, 2021
92db0fd
update expected test failures and changelog
butonic Jan 6, 2021
e36dd46
parent owner becomes owner of uploaded files
butonic Jan 7, 2021
0f14362
parent owner becomes owner of new folders
butonic Jan 7, 2021
074d616
fix home creation
butonic Jan 7, 2021
79f82c6
update expected failures and changelog
butonic Jan 7, 2021
c69f470
fix lint
butonic Jan 7, 2021
c3cbdd1
cleanup permissions rendering for propfind
butonic Jan 8, 2021
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
Enhancement: calculate and expose actual file permission set

Instead of hardcoding the permissions set for every file and folder to ListContainer:true, CreateContainer:true and always reporting the hardcoded string WCKDNVR for the WebDAV permissions we now aggregate the actual cs3 resource permission set in the storage drivers and correctly map them to ocs permissions and webdav permissions using a common role struct that encapsulates the mapping logic.

https://github.com/cs3org/reva/pull/1368
https://github.com/owncloud/ocis/issues/552
https://github.com/owncloud/ocis/issues/762
https://github.com/owncloud/ocis/issues/763
https://github.com/owncloud/ocis/issues/893
https://github.com/owncloud/ocis/issues/1126
https://github.com/owncloud/ocis-reva/issues/47
https://github.com/owncloud/ocis-reva/issues/315
https://github.com/owncloud/ocis-reva/issues/316
https://github.com/owncloud/product/issues/270
29 changes: 19 additions & 10 deletions internal/grpc/services/authprovider/authprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/cs3org/reva/pkg/appctx"
"github.com/cs3org/reva/pkg/auth"
"github.com/cs3org/reva/pkg/auth/manager/registry"
"github.com/cs3org/reva/pkg/errtypes"
"github.com/cs3org/reva/pkg/rgrpc"
"github.com/cs3org/reva/pkg/rgrpc/status"
"github.com/mitchellh/mapstructure"
Expand Down Expand Up @@ -108,18 +109,26 @@ func (s *service) Authenticate(ctx context.Context, req *provider.AuthenticateRe
password := req.ClientSecret

u, err := s.authmgr.Authenticate(ctx, username, password)
if err != nil {
switch v := err.(type) {
case nil:
log.Info().Msgf("user %s authenticated", u.String())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need log level info here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was info before. see https://github.com/cs3org/reva/pull/1368/files#diff-187ee2d4618b4a46049ec9c40c111e1420baecb2622be545ec0c9e8cab196a07L119

could you raise an issue or try linkin an issue that deals with log levels?

return &provider.AuthenticateResponse{
Status: status.NewOK(ctx),
User: u,
}, nil
case errtypes.InvalidCredentials:
return &provider.AuthenticateResponse{
Status: status.NewPermissionDenied(ctx, v, "wrong password"),
}, nil
case errtypes.NotFound:
return &provider.AuthenticateResponse{
Status: status.NewNotFound(ctx, "unknown client id"),
}, nil
default:
err = errors.Wrap(err, "authsvc: error in Authenticate")
res := &provider.AuthenticateResponse{
return &provider.AuthenticateResponse{
Status: status.NewUnauthenticated(ctx, err, "error authenticating user"),
}
return res, nil
}, nil
}

log.Info().Msgf("user %s authenticated", u.String())
res := &provider.AuthenticateResponse{
Status: status.NewOK(ctx),
User: u,
}
return res, nil
}
23 changes: 15 additions & 8 deletions internal/grpc/services/gateway/authprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package gateway

import (
"context"
"fmt"

provider "github.com/cs3org/go-cs3apis/cs3/auth/provider/v1beta1"
registry "github.com/cs3org/go-cs3apis/cs3/auth/registry/v1beta1"
Expand Down Expand Up @@ -53,18 +54,24 @@ func (s *svc) Authenticate(ctx context.Context, req *gateway.AuthenticateRequest
ClientSecret: req.ClientSecret,
}
res, err := c.Authenticate(ctx, authProviderReq)
if err != nil {
log.Err(err).Msgf("gateway: error calling Authenticate for type: %s", req.Type)
switch {
case err != nil:
return &gateway.AuthenticateResponse{
Status: status.NewUnauthenticated(ctx, err, "error authenticating request"),
Status: status.NewInternal(ctx, err, fmt.Sprintf("gateway: error calling Authenticate for type: %s", req.Type)),
}, nil
}

if res.Status.Code != rpc.Code_CODE_OK {
case res.Status.Code == rpc.Code_CODE_PERMISSION_DENIED:
fallthrough
case res.Status.Code == rpc.Code_CODE_UNAUTHENTICATED:
fallthrough
case res.Status.Code == rpc.Code_CODE_NOT_FOUND:
// normal failures, no need to log
return &gateway.AuthenticateResponse{
Status: res.Status,
}, nil
case res.Status.Code != rpc.Code_CODE_OK:
err := status.NewErrorFromCode(res.Status.Code, "gateway")
log.Err(err).Msgf("error authenticating credentials to auth provider for type: %s", req.Type)
return &gateway.AuthenticateResponse{
Status: status.NewUnauthenticated(ctx, err, ""),
Status: status.NewInternal(ctx, err, fmt.Sprintf("error authenticating credentials to auth provider for type: %s", req.Type)),
}, nil
}

Expand Down
4 changes: 2 additions & 2 deletions internal/grpc/services/gateway/storageprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -936,7 +936,7 @@ func (s *svc) UnsetArbitraryMetadata(ctx context.Context, req *provider.UnsetArb
c, err := s.find(ctx, req.Ref)
if err != nil {
return &provider.UnsetArbitraryMetadataResponse{
Status: status.NewStatusFromErrType(ctx, "SetArbitraryMetadata ref="+req.Ref.String(), err),
Status: status.NewStatusFromErrType(ctx, "UnsetArbitraryMetadata ref="+req.Ref.String(), err),
}, nil
}

Expand Down Expand Up @@ -1044,7 +1044,7 @@ func (s *svc) stat(ctx context.Context, req *provider.StatRequest) (*provider.St
c, err := s.find(ctx, req.Ref)
if err != nil {
return &provider.StatResponse{
Status: status.NewStatusFromErrType(ctx, "SetArbitraryMetadata ref="+req.Ref.String(), err),
Status: status.NewStatusFromErrType(ctx, "stat ref="+req.Ref.String(), err),
}, nil
}

Expand Down
2 changes: 2 additions & 0 deletions internal/grpc/services/gateway/usershareprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ func (s *svc) CreateShare(ctx context.Context, req *collaboration.CreateShareReq
Status: status.NewInternal(ctx, err, "error getting user share provider client"),
}, nil
}
// TODO the user share manager needs to be able to decide if the current user is allowed to create that share (and not eg. incerase permissions)
// jfd: AFAICT this can only be determined by a storage driver - either the storage provider is queried first or the share manager needs to access the storage using a storage driver
res, err := c.CreateShare(ctx, req)
if err != nil {
return nil, errors.Wrap(err, "gateway: error calling CreateShare")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1"
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
"github.com/cs3org/reva/pkg/appctx"
"github.com/cs3org/reva/pkg/errtypes"
"github.com/cs3org/reva/pkg/publicshare"
"github.com/cs3org/reva/pkg/publicshare/manager/registry"
"github.com/cs3org/reva/pkg/rgrpc"
Expand Down Expand Up @@ -145,18 +146,29 @@ func (s *service) RemovePublicShare(ctx context.Context, req *link.RemovePublicS

func (s *service) GetPublicShareByToken(ctx context.Context, req *link.GetPublicShareByTokenRequest) (*link.GetPublicShareByTokenResponse, error) {
log := appctx.GetLogger(ctx)
log.Info().Msg("getting public share by token")
log.Debug().Msg("getting public share by token")

// there are 2 passes here, and the second request has no password
found, err := s.sm.GetPublicShareByToken(ctx, req.GetToken(), req.GetPassword())
if err != nil {
return nil, err
switch v := err.(type) {
case nil:
return &link.GetPublicShareByTokenResponse{
Status: status.NewOK(ctx),
Share: found,
}, nil
case errtypes.InvalidCredentials:
return &link.GetPublicShareByTokenResponse{
Status: status.NewPermissionDenied(ctx, v, "wrong password"),
}, nil
case errtypes.NotFound:
return &link.GetPublicShareByTokenResponse{
Status: status.NewNotFound(ctx, "unknown token"),
}, nil
default:
return &link.GetPublicShareByTokenResponse{
Status: status.NewInternal(ctx, v, "unexpected error"),
}, nil
}

return &link.GetPublicShareByTokenResponse{
Status: status.NewOK(ctx),
Share: found,
}, nil
}

func (s *service) GetPublicShare(ctx context.Context, req *link.GetPublicShareRequest) (*link.GetPublicShareResponse, error) {
Expand Down
Loading