fix(collaboration): harden wopi token handling

services/collaboration/pkg/connector/fileconnector_test.go

@@ -8,6 +8,7 @@ import (
 	"path"
 	"regexp"
 	"strings"
+	"time"
 
 	appproviderv1beta1 "github.com/cs3org/go-cs3apis/cs3/app/provider/v1beta1"
 	auth "github.com/cs3org/go-cs3apis/cs3/auth/provider/v1beta1"
@@ -17,6 +18,7 @@ import (
 	link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1"
 	providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
+	"github.com/golang-jwt/jwt/v5"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/opencloud-eu/opencloud/pkg/shared"
@@ -67,10 +69,19 @@ var _ = Describe("FileConnector", func() {
 		gatewaySelector.On("Next").Return(gatewayClient, nil)
 		fc = connector.NewFileConnector(gatewaySelector, cfg, nil, nil)
 
+		// PutRelativeFile mints a wopi token from this one and verifies it
+		now := time.Now()
+		accessToken, err := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{
+			"aud": "reva",
+			"iss": "https://cloud.opencloud.test",
+			"iat": now.Unix(),
+			"exp": now.Add(5 * time.Hour).Unix(),
+			"sub": "alice",
+		}).SignedString([]byte(cfg.TokenManager.JWTSecret))
+		Expect(err).ToNot(HaveOccurred())
+
 		wopiCtx = middleware.WopiContext{
-			// a real token is needed for the PutRelativeFileSuggested tests
-			// although we aren't checking anything inside the token
-			AccessToken: "eyJhbGciOiJQUzI1NiIsImtpZCI6InByaXZhdGUta2V5IiwidHlwIjoiSldUIn0.eyJhdWQiOiJ3ZWIiLCJleHAiOjE3MjAwOTIyODAsImlhdCI6MTcyMDA5MTk4MCwiaXNzIjoiaHR0cHM6Ly9vY2lzLmpwLnNvbGlkZ2Vhci5wcnYiLCJqdGkiOiJmQldpN0FYaFFQdUhhaDJDV0VQVFFLcENmZ3BGbEFpTCIsImxnLmkiOnsiZG4iOiJicm8iLCJpZCI6Im93bkNsb3VkVVVJRD1mYWYxMTY0Ny03NDUxLTRiOWEtYmZmZS0zYjVkZGNjNTk3MmIiLCJ1biI6ImJyb3RhdG8ifSwibGcucCI6ImlkZW50aWZpZXItbGRhcCIsImxnLnQiOiIxIiwic2NwIjoib3BlbmlkIHByb2ZpbGUgZW1haWwiLCJzdWIiOiJjQXZ1elg4Z1hMWmRpWHgtQDFOV1RKdENQRHFVSjQ0bnQ0NkZ0RDlwNUw3dGplUEZkWk1WSjlFMzBOeDItZHVpN0hLQ0x4QWlXYUNUdGJYNTExSmNkSHcifQ.StpQpE4ipxk8Nhk6xgob1Tovbk6bcUVs5-fkej2hIoKoJKfR2OY-CiFQ3wwgEcFro8notxeVfOmxs36z_ezFeJBZRbxpSggcr77LFtQwlsWvD5AuAgLZN1otdvULehunXE_DtxRJZ1rqnsOBT03zKOZLx8Q7QTy6DeRuf1KQtCIowa9D4ymPM4TTmtQdiW2XjByO3OCLFEMVBfDFGPibR6gMnftGQ5kfiZGDTUVCauEXwE-msZVZ42QY-wFRppX_RIL1Z0p6T4dr_6_y-VM1lNYJ5-dB5c5rg_c03Xu1y_TIxs31-8--dtUyZmBVOZFk8bB9msNk-iaOEjzKeUZLymo_-2qVYvXxzNrkq1QA8luaLR6jec_CRT2P8wsB2nyebFU6_myKe34m6f8uqGhOzcOwPB4TpoxPx4ucQgo1CQJwQZHZsZ7Q6TVYZUXJdWwzzMuvJXmnn36iybw0Ub6On4sGKj3gHetjoJg8VnL-TQkBvf1iHX2ktRG3Nq2rnPrB2OTpi2rLpleWg_s8Y8FXxIgYqM0JG8kO1n5RPGMeYQG7qd6f9wdcaPIvgxCa_HsZtMr7eGcDzZtxp-NivgJOS6ode0ZAJ3wGU-AVhmyshpds3DFECcvkBcP_4dD52AXiAq9X3UVkVdNsxs_yB9P7zBcdsKsD6QDJv5gf-6DEu34",
+			AccessToken: accessToken,
 			FileReference: &providerv1beta1.Reference{
 				ResourceId: &providerv1beta1.ResourceId{
 					StorageId: "abc",

services/collaboration/pkg/middleware/wopicontext.go

@@ -91,8 +91,7 @@ func WopiContextAuthMiddleware(cfg *config.Config, st microstore.Store, next htt
 
 		claims := &Claims{}
 		_, err := jwt.ParseWithClaims(accessToken, claims, func(token *jwt.Token) (any, error) {
-
-			if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok {
+			if token.Method != jwt.SigningMethodHS256 {
 				return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"])
 			}
 
@@ -129,6 +128,17 @@ func WopiContextAuthMiddleware(cfg *config.Config, st microstore.Store, next htt
 
 		claims.WopiContext.AccessToken = wopiContextAccessToken
 
+		// encrypted in GenerateWopiToken, only set for view only shares
+		if claims.WopiContext.ViewOnlyToken != "" {
+			wopiContextViewOnlyToken, err := DecryptAES([]byte(cfg.Wopi.Secret), claims.WopiContext.ViewOnlyToken)
+			if err != nil {
+				wopiLogger.Error().Err(err).Msg("failed to decrypt view only token")
+				http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized)
+				return
+			}
+			claims.WopiContext.ViewOnlyToken = wopiContextViewOnlyToken
+		}
+
 		ctx = context.WithValue(ctx, wopiContextKey, claims.WopiContext)
 		// authentication for the CS3 api
 		ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.TokenHeader, claims.WopiContext.AccessToken)
@@ -203,9 +213,24 @@ func GenerateWopiToken(wopiContext WopiContext, cfg *config.Config, st microstor
 		return "", 0, err
 	}
 
+	// impersonates the owner, and the wopi token payload is readable
+	if wopiContext.ViewOnlyToken != "" {
+		cryptedViewOnlyToken, err := EncryptAES([]byte(cfg.Wopi.Secret), wopiContext.ViewOnlyToken)
+		if err != nil {
+			return "", 0, err
+		}
+		wopiContext.ViewOnlyToken = cryptedViewOnlyToken
+	}
+
+	// the wopi token inherits this expiry, so verify instead of just decoding
 	cs3Claims := &jwt.RegisteredClaims{}
-	cs3JWTparser := jwt.Parser{}
-	_, _, err = cs3JWTparser.ParseUnverified(wopiContext.AccessToken, cs3Claims)
+	_, err = jwt.ParseWithClaims(wopiContext.AccessToken, cs3Claims, func(token *jwt.Token) (any, error) {
+		if token.Method != jwt.SigningMethodHS256 {
+			return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"])
+		}
+
+		return []byte(cfg.TokenManager.JWTSecret), nil
+	})
 	if err != nil {
 		return "", 0, err
 	}
@@ -249,7 +274,10 @@ func parseWopiFileID(cfg *config.Config, path string) string {
 	}
 	// check if the fileid is a jwt
 	if strings.Contains(s[3], ".") {
-		token, err := jwt.Parse(s[3], func(_ *jwt.Token) (any, error) {
+		token, err := jwt.Parse(s[3], func(token *jwt.Token) (any, error) {
+			if token.Method != jwt.SigningMethodHS256 {
+				return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"])
+			}
 			return []byte(cfg.Wopi.ProxySecret), nil
 		})
 		if err != nil {

services/collaboration/pkg/middleware/wopicontext_test.go

@@ -2,15 +2,19 @@ package middleware_test
 
 import (
 	"context"
+	"encoding/base64"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
 	"path"
 	"strconv"
+	"strings"
+	"time"
 
 	appprovider "github.com/cs3org/go-cs3apis/cs3/app/provider/v1beta1"
 	userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
 	providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
+	"github.com/golang-jwt/jwt/v5"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/opencloud-eu/opencloud/services/collaboration/pkg/config"
@@ -29,6 +33,7 @@ var _ = Describe("Wopi Context Middleware", func() {
 		rid     *providerv1beta1.ResourceId
 		tknMngr token.Manager
 		user    *userv1beta1.User
+		owner   *userv1beta1.User
 		src     *url.URL
 	)
 
@@ -65,6 +70,16 @@ var _ = Describe("Wopi Context Middleware", func() {
 			Mail:     "admin@example.com",
 		}
 
+		owner = &userv1beta1.User{
+			Id: &userv1beta1.UserId{
+				Idp:      "example.com",
+				OpaqueId: "67890",
+				Type:     userv1beta1.UserType_USER_TYPE_PRIMARY,
+			},
+			Username: "alice",
+			Mail:     "alice@example.com",
+		}
+
 		rid = &providerv1beta1.ResourceId{
 			StorageId: "storageID",
 			OpaqueId:  "opaqueID",
@@ -130,9 +145,12 @@ var _ = Describe("Wopi Context Middleware", func() {
 			AccessToken: token,
 		}
 		// use wrong wopi secret when generating the wopi token
-		wopiToken, ttl, err := middleware.GenerateWopiToken(wopiContext, &config.Config{Wopi: config.Wopi{
-			Secret: "wrongSecret",
-		}}, nil)
+		wopiToken, ttl, err := middleware.GenerateWopiToken(wopiContext, &config.Config{
+			TokenManager: &config.TokenManager{JWTSecret: cfg.TokenManager.JWTSecret},
+			Wopi: config.Wopi{
+				Secret: "wrongSecret",
+			},
+		}, nil)
 		q := req.URL.Query()
 		q.Add("access_token", wopiToken)
 		q.Add("access_token_ttl", strconv.FormatInt(ttl, 10))
@@ -287,4 +305,97 @@ var _ = Describe("Wopi Context Middleware", func() {
 		mw.ServeHTTP(resp, req)
 		Expect(resp.Code).To(Equal(http.StatusOK))
 	})
+	It("Should not carry the view only token in plaintext", func() {
+		accessToken, err := tknMngr.MintToken(ctx, user, nil)
+		Expect(err).ToNot(HaveOccurred())
+		viewOnlyToken, err := tknMngr.MintToken(ctx, owner, nil)
+		Expect(err).ToNot(HaveOccurred())
+
+		wopiContext := middleware.WopiContext{
+			AccessToken:   accessToken,
+			ViewOnlyToken: viewOnlyToken,
+			ViewMode:      appprovider.ViewMode_VIEW_MODE_VIEW_ONLY,
+			FileReference: &providerv1beta1.Reference{
+				ResourceId: rid,
+				Path:       ".",
+			},
+		}
+		wopiToken, _, err := middleware.GenerateWopiToken(wopiContext, cfg, nil)
+		Expect(err).ToNot(HaveOccurred())
+
+		parts := strings.Split(wopiToken, ".")
+		Expect(parts).To(HaveLen(3))
+		payload, err := base64.RawURLEncoding.DecodeString(parts[1])
+		Expect(err).ToNot(HaveOccurred())
+		Expect(string(payload)).ToNot(ContainSubstring(viewOnlyToken))
+	})
+	It("Should hand the decrypted view only token to the next handler", func() {
+		accessToken, err := tknMngr.MintToken(ctx, user, nil)
+		Expect(err).ToNot(HaveOccurred())
+		viewOnlyToken, err := tknMngr.MintToken(ctx, owner, nil)
+		Expect(err).ToNot(HaveOccurred())
+
+		var seen middleware.WopiContext
+		var seenErr error
+		capturing := middleware.WopiContextAuthMiddleware(cfg, nil, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			seen, seenErr = middleware.WopiContextFromCtx(r.Context())
+			w.WriteHeader(http.StatusOK)
+		}))
+
+		wopiContext := middleware.WopiContext{
+			AccessToken:   accessToken,
+			ViewOnlyToken: viewOnlyToken,
+			ViewMode:      appprovider.ViewMode_VIEW_MODE_VIEW_ONLY,
+			FileReference: &providerv1beta1.Reference{
+				ResourceId: rid,
+				Path:       ".",
+			},
+		}
+		wopiToken, ttl, err := middleware.GenerateWopiToken(wopiContext, cfg, nil)
+		Expect(err).ToNot(HaveOccurred())
+
+		req := httptest.NewRequest("GET", src.String(), nil).WithContext(ctx)
+		q := req.URL.Query()
+		q.Add("access_token", wopiToken)
+		q.Add("access_token_ttl", strconv.FormatInt(ttl, 10))
+		req.URL.RawQuery = q.Encode()
+		resp := httptest.NewRecorder()
+
+		capturing.ServeHTTP(resp, req)
+		Expect(resp.Code).To(Equal(http.StatusOK))
+		Expect(seenErr).ToNot(HaveOccurred())
+		Expect(seen.ViewOnlyToken).To(Equal(viewOnlyToken))
+	})
+	It("Should not authorize a token signed with another hmac variant", func() {
+		accessToken, err := tknMngr.MintToken(ctx, user, nil)
+		Expect(err).ToNot(HaveOccurred())
+		cryptedAccessToken, err := middleware.EncryptAES([]byte(cfg.Wopi.Secret), accessToken)
+		Expect(err).ToNot(HaveOccurred())
+
+		// valid apart from the algorithm
+		claims := &middleware.Claims{
+			WopiContext: middleware.WopiContext{
+				AccessToken: cryptedAccessToken,
+				ViewMode:    appprovider.ViewMode_VIEW_MODE_READ_WRITE,
+				FileReference: &providerv1beta1.Reference{
+					ResourceId: rid,
+					Path:       ".",
+				},
+			},
+			RegisteredClaims: jwt.RegisteredClaims{
+				ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour)),
+			},
+		}
+		forged, err := jwt.NewWithClaims(jwt.SigningMethodHS512, claims).SignedString([]byte(cfg.Wopi.Secret))
+		Expect(err).ToNot(HaveOccurred())
+
+		req := httptest.NewRequest("GET", src.String(), nil).WithContext(ctx)
+		q := req.URL.Query()
+		q.Add("access_token", forged)
+		req.URL.RawQuery = q.Encode()
+		resp := httptest.NewRecorder()
+
+		mw.ServeHTTP(resp, req)
+		Expect(resp.Code).To(Equal(http.StatusUnauthorized))
+	})
 })

services/collaboration/pkg/service/grpc/v0/service_test.go

@@ -75,7 +75,9 @@ var _ = Describe("Discovery", func() {
 	)
 
 	BeforeEach(func() {
-		cfg = &config.Config{}
+		cfg = &config.Config{
+			TokenManager: &config.TokenManager{JWTSecret: "jwtSecret"},
+		}
 		gatewayClient = &cs3mocks.GatewayAPIClient{}
 
 		gatewaySelector := mocks.NewSelectable[gatewayv1beta1.GatewayAPIClient](GinkgoT())
@@ -175,7 +177,7 @@ var _ = Describe("Discovery", func() {
 						Path: "/path/to/file.docx",
 					},
 					ViewMode:    appproviderv1beta1.ViewMode_VIEW_MODE_READ_WRITE,
-					AccessToken: MintToken(myself, cfg.Wopi.Secret, nowTime),
+					AccessToken: MintToken(myself, cfg.TokenManager.JWTSecret, nowTime),
 				}
 				if lang != "" {
 					req.Opaque = utils.AppendPlainToOpaque(req.Opaque, "lang", lang)
@@ -235,7 +237,7 @@ var _ = Describe("Discovery", func() {
 					Path: "/path/to/file.docx",
 				},
 				ViewMode:    appproviderv1beta1.ViewMode_VIEW_MODE_READ_WRITE,
-				AccessToken: MintToken(myself, cfg.Wopi.Secret, nowTime),
+				AccessToken: MintToken(myself, cfg.TokenManager.JWTSecret, nowTime),
 			}
 			req.Opaque = utils.AppendPlainToOpaque(req.Opaque, "lang", "en")
 
@@ -278,7 +280,7 @@ var _ = Describe("Discovery", func() {
 					Path: "/path/to/file.invalid",
 				},
 				ViewMode:    appproviderv1beta1.ViewMode_VIEW_MODE_READ_WRITE,
-				AccessToken: MintToken(myself, cfg.Wopi.Secret, nowTime),
+				AccessToken: MintToken(myself, cfg.TokenManager.JWTSecret, nowTime),
 			}
 			req.Opaque = utils.AppendPlainToOpaque(req.Opaque, "lang", "en")
 
@@ -319,7 +321,7 @@ var _ = Describe("Discovery", func() {
 					Path: "/path/to/file.docx",
 				},
 				ViewMode:    appproviderv1beta1.ViewMode_VIEW_MODE_READ_WRITE,
-				AccessToken: MintToken(myself, cfg.Wopi.Secret, nowTime),
+				AccessToken: MintToken(myself, cfg.TokenManager.JWTSecret, nowTime),
 			}
 			req.Opaque = utils.AppendPlainToOpaque(req.Opaque, "lang", "en")
 			req.Opaque = utils.AppendPlainToOpaque(req.Opaque, "template", "&file_id")
@@ -362,7 +364,7 @@ var _ = Describe("Discovery", func() {
 					Path: "/path/to/file.docx",
 				},
 				ViewMode:    appproviderv1beta1.ViewMode_VIEW_MODE_READ_WRITE,
-				AccessToken: MintToken(myself, cfg.Wopi.Secret, nowTime),
+				AccessToken: MintToken(myself, cfg.TokenManager.JWTSecret, nowTime),
 			}
 			req.Opaque = utils.AppendPlainToOpaque(req.Opaque, "lang", "en")
 			req.Opaque = utils.AppendPlainToOpaque(req.Opaque, "template", "prodiderID$spaceID!opaqueID")