From 2d69b7809ff47ec030e40e7df30cd2b588e86245 Mon Sep 17 00:00:00 2001 From: root Date: Mon, 17 Oct 2022 09:55:28 +0000 Subject: [PATCH 01/11] [draft] improve ocmd send sourcePath 404 error message --- internal/http/services/ocmd/send.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/http/services/ocmd/send.go b/internal/http/services/ocmd/send.go index d0106abed3..5914a78e04 100644 --- a/internal/http/services/ocmd/send.go +++ b/internal/http/services/ocmd/send.go @@ -110,13 +110,13 @@ func (h *sendHandler) Handler() http.Handler { req := &provider.StatRequest{Ref: ref} res2, err := gatewayClient.Stat(authCtx, req) if err != nil { - log.Error().Msg("error sending: stat file/folder to share") + log.Error().Msg("gatewayClient.Stat operation failed; is the storage backend reachable?") w.WriteHeader(http.StatusInternalServerError) return } if res2.Status.Code != rpc.Code_CODE_OK { - log.Error().Msg("error returned: stat file/folder to share") + log.Error().Msgf("sourcePath %s does not exist on the storage backend", sourcePath) w.WriteHeader(http.StatusInternalServerError) return } From 4b48eb056d5e895a3c02b6a5e942fbd8ef133a1c Mon Sep 17 00:00:00 2001 From: Michiel de Jong Date: Mon, 31 Oct 2022 12:20:30 +0100 Subject: [PATCH 02/11] print error message when forwarding invite fails --- internal/grpc/services/ocminvitemanager/ocminvitemanager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/grpc/services/ocminvitemanager/ocminvitemanager.go b/internal/grpc/services/ocminvitemanager/ocminvitemanager.go index 6821f7d29e..0c42995759 100644 --- a/internal/grpc/services/ocminvitemanager/ocminvitemanager.go +++ b/internal/grpc/services/ocminvitemanager/ocminvitemanager.go @@ -119,7 +119,7 @@ func (s *service) ForwardInvite(ctx context.Context, req *invitepb.ForwardInvite err := s.im.ForwardInvite(ctx, req.InviteToken, req.OriginSystemProvider) if err != nil { return &invitepb.ForwardInviteResponse{ - Status: status.NewInternal(ctx, err, "error forwarding invite"), + Status: status.NewInternal(ctx, err, "error forwarding invite:"+err.Error().message), }, nil } From a77275b1b187a99c0cc4ece129db1ab713492064 Mon Sep 17 00:00:00 2001 From: Michiel de Jong Date: Mon, 31 Oct 2022 12:21:11 +0100 Subject: [PATCH 03/11] Resolve https://github.com/cs3org/reva/issues/3365#issuecomment-1282013953 --- internal/grpc/services/ocminvitemanager/ocminvitemanager.go | 2 +- internal/http/services/ocmd/invites.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/grpc/services/ocminvitemanager/ocminvitemanager.go b/internal/grpc/services/ocminvitemanager/ocminvitemanager.go index 0c42995759..f445264cde 100644 --- a/internal/grpc/services/ocminvitemanager/ocminvitemanager.go +++ b/internal/grpc/services/ocminvitemanager/ocminvitemanager.go @@ -159,7 +159,7 @@ func (s *service) FindAcceptedUsers(ctx context.Context, req *invitepb.FindAccep acceptedUsers, err := s.im.FindAcceptedUsers(ctx, req.Filter) if err != nil { return &invitepb.FindAcceptedUsersResponse{ - Status: status.NewInternal(ctx, err, "error finding remote users"), + Status: status.NewInternal(ctx, err, "error finding remote users: "+err.Error()), }, nil } diff --git a/internal/http/services/ocmd/invites.go b/internal/http/services/ocmd/invites.go index eab71c7d90..955af35e23 100644 --- a/internal/http/services/ocmd/invites.go +++ b/internal/http/services/ocmd/invites.go @@ -314,6 +314,7 @@ func (h *invitesHandler) findAcceptedUsers(w http.ResponseWriter, r *http.Reques indentedResponse, _ := json.MarshalIndent(response, "", " ") w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) + log.Debug().Msg("findAcceptedUsers json response: " + string(indentedResponse)) if _, err := w.Write(indentedResponse); err != nil { log.Err(err).Msg("Error writing to ResponseWriter") } From 53e12c043afd7f4ad9b30e37d9173fcb6fb21eec Mon Sep 17 00:00:00 2001 From: root Date: Tue, 18 Oct 2022 13:21:45 +0000 Subject: [PATCH 04/11] changelog entry --- changelog/unreleased/improve-ocmd-error-logs.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelog/unreleased/improve-ocmd-error-logs.md diff --git a/changelog/unreleased/improve-ocmd-error-logs.md b/changelog/unreleased/improve-ocmd-error-logs.md new file mode 100644 index 0000000000..0352b89fec --- /dev/null +++ b/changelog/unreleased/improve-ocmd-error-logs.md @@ -0,0 +1,4 @@ +Enhancement: Improve error logging in ocmd flow + +https://github.com/cs3org/reva/issues/3365 +https://github.com/cs3org/reva/pull/3369 From ebdfe3894245ff97f90e770509eee1f4a9041387 Mon Sep 17 00:00:00 2001 From: root Date: Tue, 18 Oct 2022 13:50:53 +0000 Subject: [PATCH 05/11] report unexpected response codes from EFSS API --- pkg/storage/fs/nextcloud/nextcloud.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/storage/fs/nextcloud/nextcloud.go b/pkg/storage/fs/nextcloud/nextcloud.go index bb8f7ff555..d89fe63fc2 100644 --- a/pkg/storage/fs/nextcloud/nextcloud.go +++ b/pkg/storage/fs/nextcloud/nextcloud.go @@ -25,6 +25,7 @@ import ( "io" "net/http" "net/url" + "strconv" "strings" user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" @@ -229,7 +230,9 @@ func (nc *StorageDriver) do(ctx context.Context, a Action) (int, []byte, error) return 0, nil, err } log.Info().Msgf("nc.do res %s %s", url, string(body)) - + if resp.StatusCode != http.StatusOK { + return 0, nil, fmt.Errorf("Unexpected response code from EFSS API: " + strconv.Itoa(resp.StatusCode)) + } return resp.StatusCode, body, nil } From 59634844989fcd63682e10132d7aaa3cc1413e69 Mon Sep 17 00:00:00 2001 From: Michiel de Jong Date: Mon, 31 Oct 2022 15:14:58 +0000 Subject: [PATCH 06/11] fix --- internal/grpc/services/ocminvitemanager/ocminvitemanager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/grpc/services/ocminvitemanager/ocminvitemanager.go b/internal/grpc/services/ocminvitemanager/ocminvitemanager.go index f445264cde..d2b374346a 100644 --- a/internal/grpc/services/ocminvitemanager/ocminvitemanager.go +++ b/internal/grpc/services/ocminvitemanager/ocminvitemanager.go @@ -119,7 +119,7 @@ func (s *service) ForwardInvite(ctx context.Context, req *invitepb.ForwardInvite err := s.im.ForwardInvite(ctx, req.InviteToken, req.OriginSystemProvider) if err != nil { return &invitepb.ForwardInviteResponse{ - Status: status.NewInternal(ctx, err, "error forwarding invite:"+err.Error().message), + Status: status.NewInternal(ctx, err, "error forwarding invite:"+err.Error()), }, nil } From 71b9eeafb43968372bed4d57daffdf4074d9393d Mon Sep 17 00:00:00 2001 From: Michiel de Jong Date: Tue, 1 Nov 2022 12:24:04 +0100 Subject: [PATCH 07/11] Add appropriate PR link in changelog entry --- changelog/unreleased/improve-ocmd-error-logs.md | 1 + 1 file changed, 1 insertion(+) diff --git a/changelog/unreleased/improve-ocmd-error-logs.md b/changelog/unreleased/improve-ocmd-error-logs.md index 0352b89fec..5f6271cc15 100644 --- a/changelog/unreleased/improve-ocmd-error-logs.md +++ b/changelog/unreleased/improve-ocmd-error-logs.md @@ -1,4 +1,5 @@ Enhancement: Improve error logging in ocmd flow +https://github.com/cs3org/reva/pull/3419 https://github.com/cs3org/reva/issues/3365 https://github.com/cs3org/reva/pull/3369 From f4fb5cc2e2a6d4986c374432ad3413a53b81a545 Mon Sep 17 00:00:00 2001 From: Michiel de Jong Date: Tue, 1 Nov 2022 13:22:52 +0100 Subject: [PATCH 08/11] Also allow 201 responses --- pkg/storage/fs/nextcloud/nextcloud.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/storage/fs/nextcloud/nextcloud.go b/pkg/storage/fs/nextcloud/nextcloud.go index f594b5b794..f996a0e128 100644 --- a/pkg/storage/fs/nextcloud/nextcloud.go +++ b/pkg/storage/fs/nextcloud/nextcloud.go @@ -230,7 +230,7 @@ func (nc *StorageDriver) do(ctx context.Context, a Action) (int, []byte, error) return 0, nil, err } log.Info().Msgf("nc.do res %s %s", url, string(body)) - if resp.StatusCode != http.StatusOK { + if (resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated) { return 0, nil, fmt.Errorf("Unexpected response code from EFSS API: " + strconv.Itoa(resp.StatusCode)) } return resp.StatusCode, body, nil From 15a3a441ac178a9c1649333446d85fea7924f261 Mon Sep 17 00:00:00 2001 From: Michiel de Jong Date: Tue, 1 Nov 2022 13:27:00 +0100 Subject: [PATCH 09/11] Also throw on EFSS API response codes from other Nextcloud backends --- pkg/auth/manager/nextcloud/nextcloud.go | 3 +++ pkg/ocm/share/manager/nextcloud/nextcloud.go | 4 ++++ pkg/user/manager/nextcloud/nextcloud.go | 3 +++ 3 files changed, 10 insertions(+) diff --git a/pkg/auth/manager/nextcloud/nextcloud.go b/pkg/auth/manager/nextcloud/nextcloud.go index 5d01d264b1..24017784e0 100644 --- a/pkg/auth/manager/nextcloud/nextcloud.go +++ b/pkg/auth/manager/nextcloud/nextcloud.go @@ -141,6 +141,9 @@ func (am *Manager) do(ctx context.Context, a Action) (int, []byte, error) { } log.Info().Msgf("am.do response %d %s", resp.StatusCode, body) + if (resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated) { + return 0, nil, fmt.Errorf("Unexpected response code from EFSS API: " + strconv.Itoa(resp.StatusCode)) + } return resp.StatusCode, body, nil } diff --git a/pkg/ocm/share/manager/nextcloud/nextcloud.go b/pkg/ocm/share/manager/nextcloud/nextcloud.go index 57ce41d7be..8a32dbe4c2 100644 --- a/pkg/ocm/share/manager/nextcloud/nextcloud.go +++ b/pkg/ocm/share/manager/nextcloud/nextcloud.go @@ -210,6 +210,10 @@ func (sm *Manager) do(ctx context.Context, a Action, username string) (int, []by // curl -i -H 'application/json' -H 'X-Reva-Secret: shared-secret-1' -d '{"md":{"opaque_id":"fileid-/other/q/as"},"g":{"grantee":{"type":1,"Id":{"UserId":{"idp":"revanc2.docker","opaque_id":"marie"}}},"permissions":{"permissions":{"get_path":true,"initiate_file_download":true,"list_container":true,"list_file_versions":true,"stat":true}}},"provider_domain":"cern.ch","resource_type":"file","provider_id":2,"owner_opaque_id":"einstein","owner_display_name":"Albert Einstein","protocol":{"name":"webdav","options":{"sharedSecret":"secret","permissions":"webdav-property"}}}' https://nc1.docker/index.php/apps/sciencemesh/~/api/ocm/addSentShare log.Info().Msgf("am.do response %d %s", resp.StatusCode, body) + + if (resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated) { + return 0, nil, fmt.Errorf("Unexpected response code from EFSS API: " + strconv.Itoa(resp.StatusCode)) + } return resp.StatusCode, body, nil } diff --git a/pkg/user/manager/nextcloud/nextcloud.go b/pkg/user/manager/nextcloud/nextcloud.go index f425fc3300..87c22c4273 100644 --- a/pkg/user/manager/nextcloud/nextcloud.go +++ b/pkg/user/manager/nextcloud/nextcloud.go @@ -141,6 +141,9 @@ func (um *Manager) do(ctx context.Context, a Action, username string) (int, []by defer resp.Body.Close() body, err := io.ReadAll(resp.Body) + if (resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated) { + return 0, nil, fmt.Errorf("Unexpected response code from EFSS API: " + strconv.Itoa(resp.StatusCode)) + } return resp.StatusCode, body, err } From 0a7ebdbc1b6f86b5c3b5e3b67200306824c96086 Mon Sep 17 00:00:00 2001 From: Michiel de Jong Date: Tue, 1 Nov 2022 13:07:32 +0000 Subject: [PATCH 10/11] built --- pkg/auth/manager/nextcloud/nextcloud.go | 4 +++- pkg/ocm/share/manager/nextcloud/nextcloud.go | 4 +++- pkg/storage/fs/nextcloud/nextcloud.go | 2 +- pkg/user/manager/nextcloud/nextcloud.go | 3 ++- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/pkg/auth/manager/nextcloud/nextcloud.go b/pkg/auth/manager/nextcloud/nextcloud.go index 24017784e0..6932b5a7b7 100644 --- a/pkg/auth/manager/nextcloud/nextcloud.go +++ b/pkg/auth/manager/nextcloud/nextcloud.go @@ -22,8 +22,10 @@ package nextcloud import ( "context" "encoding/json" + "fmt" "io" "net/http" + "strconv" "strings" authpb "github.com/cs3org/go-cs3apis/cs3/auth/provider/v1beta1" @@ -141,7 +143,7 @@ func (am *Manager) do(ctx context.Context, a Action) (int, []byte, error) { } log.Info().Msgf("am.do response %d %s", resp.StatusCode, body) - if (resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated) { + if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated { return 0, nil, fmt.Errorf("Unexpected response code from EFSS API: " + strconv.Itoa(resp.StatusCode)) } return resp.StatusCode, body, nil diff --git a/pkg/ocm/share/manager/nextcloud/nextcloud.go b/pkg/ocm/share/manager/nextcloud/nextcloud.go index 8a32dbe4c2..9f4dd20af0 100644 --- a/pkg/ocm/share/manager/nextcloud/nextcloud.go +++ b/pkg/ocm/share/manager/nextcloud/nextcloud.go @@ -22,9 +22,11 @@ package nextcloud import ( "context" "encoding/json" + "fmt" "io" "math/rand" "net/http" + "strconv" "strings" "time" @@ -211,7 +213,7 @@ func (sm *Manager) do(ctx context.Context, a Action, username string) (int, []by log.Info().Msgf("am.do response %d %s", resp.StatusCode, body) - if (resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated) { + if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated { return 0, nil, fmt.Errorf("Unexpected response code from EFSS API: " + strconv.Itoa(resp.StatusCode)) } return resp.StatusCode, body, nil diff --git a/pkg/storage/fs/nextcloud/nextcloud.go b/pkg/storage/fs/nextcloud/nextcloud.go index f996a0e128..5d69e2a6f8 100644 --- a/pkg/storage/fs/nextcloud/nextcloud.go +++ b/pkg/storage/fs/nextcloud/nextcloud.go @@ -230,7 +230,7 @@ func (nc *StorageDriver) do(ctx context.Context, a Action) (int, []byte, error) return 0, nil, err } log.Info().Msgf("nc.do res %s %s", url, string(body)) - if (resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated) { + if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated { return 0, nil, fmt.Errorf("Unexpected response code from EFSS API: " + strconv.Itoa(resp.StatusCode)) } return resp.StatusCode, body, nil diff --git a/pkg/user/manager/nextcloud/nextcloud.go b/pkg/user/manager/nextcloud/nextcloud.go index 87c22c4273..782681d08b 100644 --- a/pkg/user/manager/nextcloud/nextcloud.go +++ b/pkg/user/manager/nextcloud/nextcloud.go @@ -24,6 +24,7 @@ import ( "fmt" "io" "net/http" + "strconv" "strings" ctxpkg "github.com/cs3org/reva/pkg/ctx" @@ -141,7 +142,7 @@ func (um *Manager) do(ctx context.Context, a Action, username string) (int, []by defer resp.Body.Close() body, err := io.ReadAll(resp.Body) - if (resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated) { + if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated { return 0, nil, fmt.Errorf("Unexpected response code from EFSS API: " + strconv.Itoa(resp.StatusCode)) } return resp.StatusCode, body, err From aa549e08fca78079b215adfdddf9aece36179429 Mon Sep 17 00:00:00 2001 From: Michiel de Jong Date: Thu, 10 Nov 2022 19:40:22 +0000 Subject: [PATCH 11/11] fix share object in OCM share backend --- pkg/ocm/share/manager/nextcloud/nextcloud.go | 33 ++++++++++++++------ 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/pkg/ocm/share/manager/nextcloud/nextcloud.go b/pkg/ocm/share/manager/nextcloud/nextcloud.go index 9f4dd20af0..5dda9d70d1 100644 --- a/pkg/ocm/share/manager/nextcloud/nextcloud.go +++ b/pkg/ocm/share/manager/nextcloud/nextcloud.go @@ -33,6 +33,7 @@ import ( userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" ctxpkg "github.com/cs3org/reva/pkg/ctx" "github.com/cs3org/reva/pkg/utils" + "github.com/google/uuid" ocmprovider "github.com/cs3org/go-cs3apis/cs3/ocm/provider/v1beta1" ocm "github.com/cs3org/go-cs3apis/cs3/sharing/ocm/v1beta1" @@ -64,6 +65,10 @@ func init() { registry.Register("nextcloud", New) } +func genID() string { + return uuid.New().String() +} + // Manager is the Nextcloud-based implementation of the share.Manager interface // see https://github.com/cs3org/reva/blob/v1.13.0/pkg/ocm/share/share.go#L30-L57 type Manager struct { @@ -261,13 +266,26 @@ func (sm *Manager) Share(ctx context.Context, md *provider.ResourceId, g *ocm.Sh return nil, errors.New("nextcloud: user and grantee are the same") } + // fixme: this should come from the EFSS backend + id := genID() + now := time.Now().UnixNano() + ts := &typespb.Timestamp{ + Seconds: uint64(now / 1000000000), + Nanos: uint32(now % 1000000000), + } + s := &ocm.Share{ + Id: &ocm.ShareId{ + OpaqueId: id, + }, Name: name, ResourceId: md, Permissions: g.Permissions, Grantee: g.Grantee, Owner: userID, Creator: userID, + Ctime: ts, + Mtime: ts, ShareType: st, } @@ -317,16 +335,13 @@ func (sm *Manager) Share(ctx context.Context, md *provider.ResourceId, g *ocm.Sh } } - _, body, err := sm.do(ctx, Action{apiMethod, string(encShare)}, username) + _, _, err = sm.do(ctx, Action{apiMethod, string(encShare)}, username) + // FIXME: this should set the id of the share + // _, body, err := sm.do(ctx, Action{apiMethod, string(encShare)}, username) - s.Id = &ocm.ShareId{ - OpaqueId: string(body), - } - now := time.Now().UnixNano() - s.Ctime = &typespb.Timestamp{ - Seconds: uint64(now / 1000000000), - Nanos: uint32(now % 1000000000), - } + // s.Id = &ocm.ShareId{ + // OpaqueId: string(body), + // } if err != nil { return nil, err }