Support OCM v1.0 schema (#3757)

* Support OCM v1.0 schema but bail out if the options aren't empty

* Fixes following PR comments

Author: glpatcern

changelog/unreleased/ocm-reconcile.md

@@ -0,0 +1,14 @@
+Enhancement: Support OCM v1.0 schema
+
+Following cs3org/cs3apis#206, we add the fields to ensure
+backwards compatibility with OCM v1.0. However, if the
+`protocol.options` undocumented object is not empty, we bail
+out for now. Supporting interoperability with OCM v1.0
+implementations (notably Nextcloud 25) may come in the future
+if the undocumented options are fully reverse engineered. This
+is reflected in the unit tests as well.
+
+Also, added viewMode to webapp protocol options (cs3org/cs3apis#207)
+and adapted all SQL code and unit tests.
+
+https://github.com/cs3org/reva/pull/3757

go.mod

@@ -16,7 +16,7 @@ require (
 	github.com/cheggaaa/pb v1.0.29
 	github.com/coreos/go-oidc v2.2.1+incompatible
 	github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e
-	github.com/cs3org/go-cs3apis v0.0.0-20230228180528-ee4e51c97a49
+	github.com/cs3org/go-cs3apis v0.0.0-20230331073620-011a5b8a3115
 	github.com/dgraph-io/ristretto v0.1.1
 	github.com/dolthub/go-mysql-server v0.14.0
 	github.com/eventials/go-tus v0.0.0-20200718001131-45c7ec8f5d59

go.sum

@@ -305,8 +305,8 @@ github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ3
 github.com/creack/pty v1.1.11/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
 github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e h1:tqSPWQeueWTKnJVMJffz4pz0o1WuQxJ28+5x5JgaHD8=
 github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e/go.mod h1:XJEZ3/EQuI3BXTp/6DUzFr850vlxq11I6satRtz0YQ4=
-github.com/cs3org/go-cs3apis v0.0.0-20230228180528-ee4e51c97a49 h1:CG65qpsSttrPAqdK19kaZaAiRadZ5xNFVrKoIjICBBM=
-github.com/cs3org/go-cs3apis v0.0.0-20230228180528-ee4e51c97a49/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
+github.com/cs3org/go-cs3apis v0.0.0-20230331073620-011a5b8a3115 h1:WR5sDbfsHZZViXKB0036V2hobzZSxew1MomrSk1kWyI=
+github.com/cs3org/go-cs3apis v0.0.0-20230331073620-011a5b8a3115/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
 github.com/cyberdelia/templates v0.0.0-20141128023046-ca7fffd4298c/go.mod h1:GyV+0YP4qX0UQ7r2MoYZ+AvYDp12OF5yg4q8rGnyNh4=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=

internal/http/services/ocmd/protocols.go

@@ -20,13 +20,15 @@ package ocmd
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"reflect"
 	"strings"
 
 	ocm "github.com/cs3org/go-cs3apis/cs3/sharing/ocm/v1beta1"
 	providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	ocmshare "github.com/cs3org/reva/pkg/ocm/share"
+	utils "github.com/cs3org/reva/pkg/utils"
 )
 
 // Protocols is the list of protocols.
@@ -73,11 +75,12 @@ func (w *WebDAV) ToOCMProtocol() *ocm.Protocol {
 // Webapp contains the parameters for the Webapp protocol.
 type Webapp struct {
 	URITemplate string `json:"uriTemplate" validate:"required"`
+	ViewMode    string `json:"viewMode" validate:"required,dive,required,oneof=view read write"`
 }
 
 // ToOCMProtocol convert the protocol to a ocm Protocol struct.
 func (w *Webapp) ToOCMProtocol() *ocm.Protocol {
-	return ocmshare.NewWebappProtocol(w.URITemplate)
+	return ocmshare.NewWebappProtocol(w.URITemplate, utils.GetAppViewMode(w.ViewMode))
 }
 
 // Datatx contains the parameters for the Datatx protocol.
@@ -109,11 +112,22 @@ func (p *Protocols) UnmarshalJSON(data []byte) error {
 
 	for name, d := range prot {
 		var res Protocol
+
+		// we do not support the OCM v1.0 properties for now, therefore just skip or bail out
+		if name == "name" {
+			continue
+		}
+		if name == "options" {
+			var opt map[string]any
+			if err := json.Unmarshal(d, &opt); err != nil || len(opt) > 0 {
+				return fmt.Errorf("protocol options not supported: %s", string(d))
+			}
+			continue
+		}
 		ctype, ok := protocolImpl[name]
 		if !ok {
 			return fmt.Errorf("protocol %s not recognised", name)
 		}
-
 		res = reflect.New(ctype).Interface().(Protocol)
 		if err := json.Unmarshal(d, &res); err != nil {
 			return err
@@ -126,10 +140,16 @@ func (p *Protocols) UnmarshalJSON(data []byte) error {
 
 // MarshalJSON implements the Marshaler interface.
 func (p Protocols) MarshalJSON() ([]byte, error) {
-	d := make(map[string]Protocol)
+	if len(p) == 0 {
+		return nil, errors.New("no protocol defined")
+	}
+	d := make(map[string]any)
 	for _, prot := range p {
 		d[getProtocolName(prot)] = prot
 	}
+	// fill in the OCM v1.0 properties
+	d["name"] = "multi"
+	d["options"] = map[string]any{}
 	return json.Marshal(d)
 }
 

internal/http/services/ocmd/protocols_test.go

@@ -37,7 +37,19 @@ func TestUnmarshalProtocol(t *testing.T) {
 			expected: []Protocol{},
 		},
 		{
-			raw: `{"webdav":{"sharedSecret":"secret","permissions":["read","write"],"url":"http://example.org"}}`,
+			raw:      `{"name":"foo","options":{ }}`,
+			expected: []Protocol{},
+		},
+		{
+			raw: `{"name":"foo","options":{"unsupported":"value"}}`,
+			err: `protocol options not supported: {"unsupported":"value"}`,
+		},
+		{
+			raw: `{"unsupported":{}}`,
+			err: "protocol unsupported not recognised",
+		},
+		{
+			raw: `{"name":"multi","options":{},"webdav":{"sharedSecret":"secret","permissions":["read","write"],"url":"http://example.org"}}`,
 			expected: []Protocol{
 				&WebDAV{
 					SharedSecret: "secret",
@@ -47,15 +59,15 @@ func TestUnmarshalProtocol(t *testing.T) {
 			},
 		},
 		{
-			raw: `{"webapp":{"uriTemplate":"http://example.org/{test}"}}`,
+			raw: `{"name":"multi","options":{},"webapp":{"uriTemplate":"http://example.org/{test}"}}`,
 			expected: []Protocol{
 				&Webapp{
 					URITemplate: "http://example.org/{test}",
 				},
 			},
 		},
 		{
-			raw: `{"datatx":{"sharedSecret":"secret","srcUri":"http://example.org","size":10}}`,
+			raw: `{"name":"multi","options":{},"datatx":{"sharedSecret":"secret","srcUri":"http://example.org","size":10}}`,
 			expected: []Protocol{
 				&Datatx{
 					SharedSecret: "secret",
@@ -65,7 +77,7 @@ func TestUnmarshalProtocol(t *testing.T) {
 			},
 		},
 		{
-			raw: `{"webdav":{"sharedSecret":"secret","permissions":["read","write"],"url":"http://example.org"},"webapp":{"uriTemplate":"http://example.org/{test}"},"datatx":{"sharedSecret":"secret","srcUri":"http://example.org","size":10}}`,
+			raw: `{"name":"multi","options":{},"webdav":{"sharedSecret":"secret","permissions":["read","write"],"url":"http://example.org"},"webapp":{"uriTemplate":"http://example.org/{test}"},"datatx":{"sharedSecret":"secret","srcUri":"http://example.org","size":10}}`,
 			expected: []Protocol{
 				&WebDAV{
 					SharedSecret: "secret",
@@ -82,10 +94,6 @@ func TestUnmarshalProtocol(t *testing.T) {
 				},
 			},
 		},
-		{
-			raw: `{"not_existing":{}}`,
-			err: "protocol not_existing not recognised",
-		},
 	}
 
 	for _, tt := range tests {
@@ -125,11 +133,12 @@ func protocolsEqual(p1, p2 Protocols) bool {
 func TestMarshalProtocol(t *testing.T) {
 	tests := []struct {
 		in       Protocols
-		expected map[string]map[string]any
+		expected map[string]any
+		err      string
 	}{
 		{
-			in:       []Protocol{},
-			expected: map[string]map[string]any{},
+			in:  []Protocol{},
+			err: "json: error calling MarshalJSON for type ocmd.Protocols: no protocol defined",
 		},
 		{
 			in: []Protocol{
@@ -139,8 +148,10 @@ func TestMarshalProtocol(t *testing.T) {
 					URL:          "http://example.org",
 				},
 			},
-			expected: map[string]map[string]any{
-				"webdav": {
+			expected: map[string]any{
+				"name":    "multi",
+				"options": map[string]any{},
+				"webdav": map[string]any{
 					"sharedSecret": "secret",
 					"permissions":  []any{"read"},
 					"url":          "http://example.org",
@@ -151,11 +162,15 @@ func TestMarshalProtocol(t *testing.T) {
 			in: []Protocol{
 				&Webapp{
 					URITemplate: "http://example.org",
+					ViewMode:    "read",
 				},
 			},
-			expected: map[string]map[string]any{
-				"webapp": {
+			expected: map[string]any{
+				"name":    "multi",
+				"options": map[string]any{},
+				"webapp": map[string]any{
 					"uriTemplate": "http://example.org",
+					"viewMode":    "read",
 				},
 			},
 		},
@@ -167,8 +182,10 @@ func TestMarshalProtocol(t *testing.T) {
 					Size:         10,
 				},
 			},
-			expected: map[string]map[string]any{
-				"datatx": {
+			expected: map[string]any{
+				"name":    "multi",
+				"options": map[string]any{},
+				"datatx": map[string]any{
 					"sharedSecret": "secret",
 					"srcUri":       "http://example.org/source",
 					"size":         float64(10),
@@ -184,23 +201,27 @@ func TestMarshalProtocol(t *testing.T) {
 				},
 				&Webapp{
 					URITemplate: "http://example.org",
+					ViewMode:    "read",
 				},
 				&Datatx{
 					SharedSecret: "secret",
 					SourceURI:    "http://example.org/source",
 					Size:         10,
 				},
 			},
-			expected: map[string]map[string]any{
-				"webdav": {
+			expected: map[string]any{
+				"name":    "multi",
+				"options": map[string]any{},
+				"webdav": map[string]any{
 					"sharedSecret": "secret",
 					"permissions":  []any{"read"},
 					"url":          "http://example.org",
 				},
-				"webapp": {
+				"webapp": map[string]any{
 					"uriTemplate": "http://example.org",
+					"viewMode":    "read",
 				},
-				"datatx": {
+				"datatx": map[string]any{
 					"sharedSecret": "secret",
 					"srcUri":       "http://example.org/source",
 					"size":         float64(10),
@@ -211,17 +232,18 @@ func TestMarshalProtocol(t *testing.T) {
 
 	for _, tt := range tests {
 		d, err := json.Marshal(tt.in)
-		if err != nil {
-			t.Fatal("not expected error", err)
-		}
-
-		var got map[string]map[string]any
-		if err := json.Unmarshal(d, &got); err != nil {
-			t.Fatal("not expected error", err)
+		if err != nil && err.Error() != tt.err {
+			t.Fatalf("not expected error. got=%+v expected=%+v", err, tt.err)
 		}
+		if err == nil {
+			var got map[string]any
+			if err := json.Unmarshal(d, &got); err != nil {
+				t.Fatalf("not expected error %+v with input %+v", err, tt.in)
+			}
 
-		if !reflect.DeepEqual(tt.expected, got) {
-			t.Fatalf("result does not match with expected. got=%+v expected=%+v", render.AsCode(got), render.AsCode(tt.expected))
+			if !reflect.DeepEqual(tt.expected, got) {
+				t.Fatalf("result does not match with expected. got=%+v expected=%+v", render.AsCode(got), render.AsCode(tt.expected))
+			}
 		}
 	}
 }

internal/http/services/ocmd/shares.go

@@ -71,7 +71,7 @@ type createShareRequest struct {
 	ShareType         string    `json:"shareType" validate:"required,oneof=user group"` // recipient share type (user or group)
 	ResourceType      string    `json:"resourceType" validate:"required,oneof=file folder"`
 	Expiration        uint64    `json:"expiration"`
-	Protocols         Protocols `json:"protocols" validate:"required"`
+	Protocols         Protocols `json:"protocol" validate:"required"`
 }
 
 // CreateShare sends all the informations to the consumer needed to start

pkg/ocm/client/client.go

@@ -166,7 +166,7 @@ type NewShareRequest struct {
 	ShareType         string         `json:"shareType"`
 	Expiration        uint64         `json:"expiration"`
 	ResourceType      string         `json:"resourceType"`
-	Protocols         ocmd.Protocols `json:"protocols"`
+	Protocols         ocmd.Protocols `json:"protocol"`
 }
 
 func (r *NewShareRequest) toJSON() (io.Reader, error) {
@@ -183,7 +183,7 @@ type NewShareResponse struct {
 }
 
 // NewShare creates a new share.
-// https://github.com/cs3org/OCM-API/blob/223285aa4d828ed85c361c7382efd08c42b5e719/spec.yaml
+// https://github.com/cs3org/OCM-API/blob/develop/spec.yaml
 func (c *OCMClient) NewShare(ctx context.Context, endpoint string, r *NewShareRequest) (*NewShareResponse, error) {
 	url, err := url.JoinPath(endpoint, "shares")
 	if err != nil {

pkg/ocm/share/repository/sql/conversions.go

@@ -23,7 +23,7 @@ import (
 	"strconv"
 	"strings"
 
-	providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/app/provider/v1beta1"
+	appprovider "github.com/cs3org/go-cs3apis/cs3/app/provider/v1beta1"
 	userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
 	ocm "github.com/cs3org/go-cs3apis/cs3/sharing/ocm/v1beta1"
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
@@ -78,8 +78,8 @@ const (
 const (
 	// WebDAVProtocol is the WebDav protocol.
 	WebDAVProtocol Protocol = iota
-	// WebappProtcol is the Webapp protocol.
-	WebappProtcol
+	// WebappProtocol is the Webapp protocol.
+	WebappProtocol
 	// TransferProtocol is the Transfer protocol.
 	TransferProtocol
 )
@@ -171,6 +171,7 @@ type dbProtocol struct {
 	WebDAVSharedSecret   *string
 	WebDavPermissions    *int
 	WebappURITemplate    *string
+	WebappViewMode       *int
 	TransferSourceURI    *string
 	TransferSharedSecret *string
 	TransferSize         *int
@@ -272,7 +273,7 @@ func convertToCS3AccessMethod(m *dbAccessMethod) *ocm.AccessMethod {
 	case WebDAVAccessMethod:
 		return share.NewWebDavAccessMethod(conversions.RoleFromOCSPermissions(conversions.Permissions(*m.WebDAVPermissions)).CS3ResourcePermissions())
 	case WebappAccessMethod:
-		return share.NewWebappAccessMethod(providerv1beta1.ViewMode(*m.WebAppViewMode))
+		return share.NewWebappAccessMethod(appprovider.ViewMode(*m.WebAppViewMode))
 	case TransferAccessMethod:
 		return share.NewTransferAccessMethod()
 	}
@@ -285,8 +286,8 @@ func convertToCS3Protocol(p *dbProtocol) *ocm.Protocol {
 		return share.NewWebDAVProtocol(*p.WebDAVURI, *p.WebDAVSharedSecret, &ocm.SharePermissions{
 			Permissions: conversions.RoleFromOCSPermissions(conversions.Permissions(*p.WebDavPermissions)).CS3ResourcePermissions(),
 		})
-	case WebappProtcol:
-		return share.NewWebappProtocol(*p.WebappURITemplate)
+	case WebappProtocol:
+		return share.NewWebappProtocol(*p.WebappURITemplate, appprovider.ViewMode(*p.WebappViewMode))
 	case TransferProtocol:
 		return share.NewTransferProtocol(*p.TransferSourceURI, *p.TransferSharedSecret, uint64(*p.TransferSize))
 	}

pkg/ocm/share/repository/sql/init.sql

@@ -70,6 +70,7 @@ CREATE TABLE IF NOT EXISTS ocm_protocol_webdav (
 CREATE TABLE IF NOT EXISTS ocm_protocol_webapp (
     ocm_protocol_id INTEGER NOT NULL PRIMARY KEY,
     uri_template VARCHAR(255) NOT NULL,
+    view_mode INTEGER NOT NULL,
     FOREIGN KEY (ocm_protocol_id) REFERENCES ocm_received_share_protocols(id) ON DELETE CASCADE
 );