feat: Separate the storage-users and graph to handle vault storage #559
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| package auth | ||
|
|
||
| import ( | ||
|
Comment on lines
+1
to
+3
There was a problem hiding this comment. This new file is missing the standard Apache license header that other files in this package include (e.g. |
||
| "context" | ||
|
|
||
| provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" | ||
| rstatus "github.com/owncloud/reva/v2/pkg/rgrpc/status" | ||
| "github.com/rs/zerolog/log" | ||
| "google.golang.org/grpc" | ||
| "google.golang.org/grpc/codes" | ||
| grpcstatus "google.golang.org/grpc/status" | ||
| ) | ||
|
|
||
| func mfaResponse(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo) (interface{}, error) { | ||
| const msg = "MFA required to access vault storage" | ||
| switch req.(type) { | ||
| case *provider.StatRequest: | ||
| return &provider.StatResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.ListContainerRequest: | ||
| return &provider.ListContainerResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.GetPathRequest: | ||
| return &provider.GetPathResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.GetQuotaRequest: | ||
| return &provider.GetQuotaResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.InitiateFileDownloadRequest: | ||
| return &provider.InitiateFileDownloadResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.InitiateFileUploadRequest: | ||
| return &provider.InitiateFileUploadResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.CreateContainerRequest: | ||
| return &provider.CreateContainerResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.TouchFileRequest: | ||
| return &provider.TouchFileResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.DeleteRequest: | ||
| return &provider.DeleteResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.MoveRequest: | ||
| return &provider.MoveResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.CreateHomeRequest: | ||
| return &provider.CreateHomeResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.AddGrantRequest: | ||
| return &provider.AddGrantResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.RemoveGrantRequest: | ||
| return &provider.RemoveGrantResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.UpdateGrantRequest: | ||
| return &provider.UpdateGrantResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.ListGrantsRequest: | ||
| return &provider.ListGrantsResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.ListFileVersionsRequest: | ||
| return &provider.ListFileVersionsResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.RestoreFileVersionRequest: | ||
| return &provider.RestoreFileVersionResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.ListRecycleRequest: | ||
| return &provider.ListRecycleResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.RestoreRecycleItemRequest: | ||
| return &provider.RestoreRecycleItemResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.PurgeRecycleRequest: | ||
| return &provider.PurgeRecycleResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.SetArbitraryMetadataRequest: | ||
| return &provider.SetArbitraryMetadataResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.UnsetArbitraryMetadataRequest: | ||
| return &provider.UnsetArbitraryMetadataResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.ListStorageSpacesRequest: | ||
| return &provider.ListStorageSpacesResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.CreateStorageSpaceRequest: | ||
| return &provider.CreateStorageSpaceResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.UpdateStorageSpaceRequest: | ||
| return &provider.UpdateStorageSpaceResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| case *provider.DeleteStorageSpaceRequest: | ||
| return &provider.DeleteStorageSpaceResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil | ||
| default: | ||
| log.Debug().Str("method", info.FullMethod).Msg("mfa: blocking unknown request type") | ||
| return nil, grpcstatus.Errorf(codes.PermissionDenied, "mfa: %s: %T", msg, req) | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ import ( | |
| "errors" | ||
| "fmt" | ||
| "net/http" | ||
| "strings" | ||
| "time" | ||
|
|
||
| "regexp" | ||
|
|
@@ -204,8 +205,15 @@ func (s *svc) writeHTTPError(rw http.ResponseWriter, err error) { | |
| s.log.Error().Msg(err.Error()) | ||
|
|
||
| switch err.(type) { | ||
| case errtypes.NotFound: | ||
| rw.WriteHeader(http.StatusNotFound) | ||
| case errtypes.PermissionDenied: | ||
| if strings.Contains(err.Error(), "MFA required") { | ||
| rw.Header().Set("X-Ocis-Mfa-Required", "true") | ||
| rw.WriteHeader(http.StatusForbidden) | ||
| } else { | ||
| rw.WriteHeader(http.StatusNotFound) | ||
| } | ||
|
Comment on lines
+210
to
+216
There was a problem hiding this comment. Detecting the MFA condition via |
||
| case manager.ErrMaxSize, manager.ErrMaxFileCount: | ||
| rw.WriteHeader(http.StatusRequestEntityTooLarge) | ||
| case errtypes.BadRequest: | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -553,6 +553,13 @@ func (s *svc) executeSpacesCopy(ctx context.Context, w http.ResponseWriter, sele | |||||
| } | ||||||
|
|
||||||
| func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Request, srcRef, dstRef *provider.Reference, log *zerolog.Logger, destInShareJail bool) *copy { | ||||||
| // restict copy from the vault | ||||||
|
There was a problem hiding this comment. This comment has a typo: "restict" → "restrict".
Suggested change
|
||||||
| if destinationIsNotAllowed(srcRef, dstRef) { | ||||||
| w.WriteHeader(http.StatusConflict) | ||||||
| b, err := errors.Marshal(http.StatusBadRequest, "destination is not allowed", "", "") | ||||||
|
Comment on lines
+558
to
+559
Member
There was a problem hiding this comment. I think we should use the same error code, and
Author
There was a problem hiding this comment. I followed the same pattern as in the Copy and Move functions, most of the errors are handled the same way.
Member
There was a problem hiding this comment. I think it's better to at least reference the original code so we know this piece has been copied, and where it has been copied from. |
||||||
| errors.HandleWebdavError(log, w, b, err) | ||||||
| return nil | ||||||
| } | ||||||
| isChild, err := s.referenceIsChildOf(ctx, s.gatewaySelector, dstRef, srcRef) | ||||||
| if err != nil { | ||||||
| switch err.(type) { | ||||||
|
|
||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be handled by the service. Some services might allow access to some parts to anyone while accessing to other parts might require additional privileges.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, but it is out of the vault storage scope in my opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we can leave it for now... all the services should have the MFA disabled except the vault, so it should be fine in the short term. Maybe a TODO comment as a reminder to move this piece is good enough.