[full-ci] [bump-reva] Replacement for TokenInfo endpoint

changelog/unreleased/tokenInfo-endpoint-replacement.md

@@ -0,0 +1,13 @@
+Enhancement: Allow to resolve public shares without the ocs tokeninfo endpoint
+
+
+Instead of querying the /v1.php/apps/files_sharing/api/v1/tokeninfo/ endpoint, a client can now resolve public and internal links by sending a PROPFIND request to /dav/public-files/{sharetoken}
+
+* authenticated clients accessing an internal link are redirected to the "real" resource (`/dav/spaces/{target-resource-id}
+* authenticated clients are able to resolve public links like before. For password protected links they need to supply the password even if they have access to the underlying resource by other means.
+* unauthenticated clients accessing an internal link get a 401 returned with  WWW-Authenticate set to Bearer (so that the client knows that it need to get a token via the IDP login page.
+* unauthenticated clients accessing a password protected link get a 401 returned with an error message to indicate the requirement for needing the link's password.
+
+https://github.com/owncloud/ocis/pull/8926
+https://github.com/cs3org/reva/pull/4653
+https://github.com/owncloud/ocis/issues/8858

services/proxy/pkg/command/server.go

@@ -272,10 +272,6 @@ func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config,
 		})
 	}
 
-	authenticators = append(authenticators, middleware.PublicShareAuthenticator{
-		Logger:              logger,
-		RevaGatewaySelector: gatewaySelector,
-	})
 	authenticators = append(authenticators, middleware.NewOIDCAuthenticator(
 		middleware.Logger(logger),
 		middleware.UserInfoCache(userInfoCache),
@@ -291,6 +287,10 @@ func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config,
 		)),
 		middleware.SkipUserInfo(cfg.OIDC.SkipUserInfo),
 	))
+	authenticators = append(authenticators, middleware.PublicShareAuthenticator{
+		Logger:              logger,
+		RevaGatewaySelector: gatewaySelector,
+	})
 	authenticators = append(authenticators, middleware.SignedURLAuthenticator{
 		Logger:             logger,
 		PreSignedURLConfig: cfg.PreSignedURL,

services/proxy/pkg/middleware/authentication_test.go

@@ -1,8 +1,27 @@
 package middleware
 
 import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"time"
+
+	gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
+	userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
+	rpcv1beta1 "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
+	"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
+	"github.com/golang-jwt/jwt/v4"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"github.com/owncloud/ocis/v2/ocis-pkg/log"
+	"github.com/owncloud/ocis/v2/ocis-pkg/oidc"
+	oidcmocks "github.com/owncloud/ocis/v2/ocis-pkg/oidc/mocks"
+	"github.com/owncloud/ocis/v2/services/proxy/pkg/router"
+	"github.com/owncloud/ocis/v2/services/proxy/pkg/user/backend"
+	"github.com/owncloud/ocis/v2/services/proxy/pkg/user/backend/mocks"
+	"github.com/stretchr/testify/mock"
+	"go-micro.dev/v4/store"
+	"google.golang.org/grpc"
 )
 
 var _ = Describe("authentication helpers", func() {
@@ -18,3 +37,167 @@ var _ = Describe("authentication helpers", func() {
 		Entry("capabilities", "/ocs/v1.php/cloud/capabilities", true),
 	)
 })
+
+var _ = Describe("Authenticating requests", Label("Authentication"), func() {
+	var (
+		authenticators []Authenticator
+	)
+	oc := oidcmocks.OIDCClient{}
+	oc.On("VerifyAccessToken", mock.Anything, mock.Anything).Return(
+		oidc.RegClaimsWithSID{
+			SessionID: "a-session-id",
+			RegisteredClaims: jwt.RegisteredClaims{
+				ExpiresAt: jwt.NewNumericDate(time.Unix(1147483647, 0)),
+			},
+		}, jwt.MapClaims{
+			"sid": "a-session-id",
+			"exp": 1147483647,
+		},
+		nil,
+	)
+
+	ub := mocks.UserBackend{}
+	ub.On("Authenticate", mock.Anything, "testuser", "testpassword").Return(
+		&userv1beta1.User{
+			Id: &userv1beta1.UserId{
+				Idp:      "IdpId",
+				OpaqueId: "OpaqueId",
+			},
+			Username: "testuser",
+			Mail:     "[email protected]",
+		},
+		"",
+		nil,
+	)
+	ub.On("Authenticate", mock.Anything, mock.Anything, mock.Anything).Return(nil, "", backend.ErrAccountNotFound)
+
+	BeforeEach(func() {
+		pool.RemoveSelector("GatewaySelector" + "com.owncloud.api.gateway")
+
+		logger := log.NewLogger()
+		authenticators = []Authenticator{
+			BasicAuthenticator{
+				Logger:       logger,
+				UserProvider: &ub,
+			},
+			&OIDCAuthenticator{
+				OIDCIss:       "http://idp.example.com",
+				Logger:        logger,
+				oidcClient:    &oc,
+				userInfoCache: store.NewMemoryStore(),
+				skipUserInfo:  true,
+			},
+			PublicShareAuthenticator{
+				Logger: logger,
+				RevaGatewaySelector: pool.GetSelector[gateway.GatewayAPIClient](
+					"GatewaySelector",
+					"com.owncloud.api.gateway",
+					func(cc *grpc.ClientConn) gateway.GatewayAPIClient {
+						return mockGatewayClient{
+							AuthenticateFunc: func(authType, clientID, clientSecret string) (string, rpcv1beta1.Code) {
+								if authType != "publicshares" {
+									return "", rpcv1beta1.Code_CODE_NOT_FOUND
+								}
+
+								if clientID == "sharetoken" && (clientSecret == "password|examples3cr3t" || clientSecret == "signature|examplesignature|exampleexpiration") {
+									return "exampletoken", rpcv1beta1.Code_CODE_OK
+								}
+
+								if clientID == "sharetoken" && clientSecret == "password|" {
+									return "otherexampletoken", rpcv1beta1.Code_CODE_OK
+								}
+
+								return "", rpcv1beta1.Code_CODE_NOT_FOUND
+							},
+						}
+					},
+				),
+			},
+		}
+	})
+
+	When("the public request must contains correct data", func() {
+		It("ensures the context oidc data when the Bearer authentication is successful", func() {
+			req := httptest.NewRequest("PROPFIND", "http://example.com/remote.php/dav/public-files/", http.NoBody)
+			req = req.WithContext(router.SetRoutingInfo(context.Background(), router.RoutingInfo{}))
+			req.Header.Set(_headerAuthorization, "Bearer jwt.token.sig")
+
+			handler := Authentication(authenticators,
+				EnableBasicAuth(true),
+			)
+			testHandler := handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				Expect(oidc.FromContext(r.Context())).To(Equal(map[string]interface{}{
+					"sid": "a-session-id",
+					"exp": int64(1147483647),
+				}))
+			}))
+			rr := httptest.NewRecorder()
+			testHandler.ServeHTTP(rr, req)
+			Expect(rr).To(HaveHTTPStatus(http.StatusOK))
+		})
+		It("ensures the context oidc data when user the Basic authentication is successful", func() {
+			req := httptest.NewRequest("PROPFIND", "http://example.com/remote.php/dav/public-files/", http.NoBody)
+			req = req.WithContext(router.SetRoutingInfo(context.Background(), router.RoutingInfo{}))
+			req.SetBasicAuth("testuser", "testpassword")
+
+			handler := Authentication(authenticators,
+				EnableBasicAuth(true),
+			)
+			testHandler := handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				Expect(oidc.FromContext(r.Context())).To(Equal(map[string]interface{}{
+					"email":              "[email protected]",
+					"ownclouduuid":       "OpaqueId",
+					"iss":                "IdpId",
+					"preferred_username": "testuser",
+				}))
+			}))
+			rr := httptest.NewRecorder()
+			testHandler.ServeHTTP(rr, req)
+			Expect(rr).To(HaveHTTPStatus(http.StatusOK))
+		})
+		It("ensures the x-access-token header when public-token URL parameter is set", func() {
+			req := httptest.NewRequest("PROPFIND", "http://example.com/dav/public-files/?public-token=sharetoken", http.NoBody)
+			req = req.WithContext(router.SetRoutingInfo(context.Background(), router.RoutingInfo{}))
+
+			handler := Authentication(authenticators,
+				EnableBasicAuth(true),
+			)
+			testHandler := handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				Expect(r.Header.Get(_headerRevaAccessToken)).To(Equal("otherexampletoken"))
+			}))
+			rr := httptest.NewRecorder()
+			testHandler.ServeHTTP(rr, req)
+			Expect(rr).To(HaveHTTPStatus(http.StatusOK))
+		})
+		It("ensures the x-access-token header when public-token URL parameter and BasicAuth are set", func() {
+			req := httptest.NewRequest("PROPFIND", "http://example.com/dav/public-files/?public-token=sharetoken", http.NoBody)
+			req.SetBasicAuth("public", "examples3cr3t")
+			req = req.WithContext(router.SetRoutingInfo(context.Background(), router.RoutingInfo{}))
+
+			handler := Authentication(authenticators,
+				EnableBasicAuth(true),
+			)
+			testHandler := handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				Expect(r.Header.Get(_headerRevaAccessToken)).To(Equal("exampletoken"))
+			}))
+			rr := httptest.NewRecorder()
+			testHandler.ServeHTTP(rr, req)
+			Expect(rr).To(HaveHTTPStatus(http.StatusOK))
+		})
+		It("ensures the x-access-token header when public-token BasicAuth is set", func() {
+			req := httptest.NewRequest("GET", "http://example.com/archiver", http.NoBody)
+			req.Header.Set("public-token", "sharetoken")
+			req = req.WithContext(router.SetRoutingInfo(context.Background(), router.RoutingInfo{}))
+
+			handler := Authentication(authenticators,
+				EnableBasicAuth(true),
+			)
+			testHandler := handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				Expect(r.Header.Get(_headerRevaAccessToken)).To(Equal("otherexampletoken"))
+			}))
+			rr := httptest.NewRecorder()
+			testHandler.ServeHTTP(rr, req)
+			Expect(rr).To(HaveHTTPStatus(http.StatusOK))
+		})
+	})
+})

services/proxy/pkg/middleware/basic_auth.go

@@ -18,7 +18,7 @@ type BasicAuthenticator struct {
 
 // Authenticate implements the authenticator interface to authenticate requests via basic auth.
 func (m BasicAuthenticator) Authenticate(r *http.Request) (*http.Request, bool) {
-	if isPublicPath(r.URL.Path) {
+	if isPublicPath(r.URL.Path) && isPublicWithShareToken(r) {
 		// The authentication of public path requests is handled by another authenticator.
 		// Since we can't guarantee the order of execution of the authenticators, we better
 		// implement an early return here for paths we can't authenticate in this authenticator.

services/proxy/pkg/middleware/oidc_auth.go

@@ -168,13 +168,16 @@ func (m OIDCAuthenticator) shouldServe(req *http.Request) bool {
 // Authenticate implements the authenticator interface to authenticate requests via oidc auth.
 func (m *OIDCAuthenticator) Authenticate(r *http.Request) (*http.Request, bool) {
 	// there is no bearer token on the request,
-	if !m.shouldServe(r) || isPublicPath(r.URL.Path) {
+	if !m.shouldServe(r) {
 		// The authentication of public path requests is handled by another authenticator.
 		// Since we can't guarantee the order of execution of the authenticators, we better
 		// implement an early return here for paths we can't authenticate in this authenticator.
 		return nil, false
 	}
 	token := strings.TrimPrefix(r.Header.Get(_headerAuthorization), _bearerPrefix)
+	if token == "" {
+		return nil, false
+	}
 
 	claims, err := m.getClaims(token, r)
 	if err != nil {

services/proxy/pkg/middleware/oidc_auth_test.go

@@ -63,5 +63,39 @@ var _ = Describe("Authenticating requests", Label("OIDCAuthenticator"), func() {
 			Expect(valid).To(Equal(true))
 			Expect(req2).ToNot(BeNil())
 		})
+		It("should successfully authenticate", func() {
+			req := httptest.NewRequest(http.MethodGet, "http://example.com/dav/public-files", http.NoBody)
+			req.Header.Set(_headerAuthorization, "Bearer jwt.token.sig")
+
+			req2, valid := authenticator.Authenticate(req)
+
+			Expect(valid).To(Equal(true))
+			Expect(req2).ToNot(BeNil())
+		})
+		It("should skip authenticate if the header ShareToken is set", func() {
+			req := httptest.NewRequest(http.MethodGet, "http://example.com/dav/public-files/", http.NoBody)
+			req.Header.Set(_headerAuthorization, "Bearer jwt.token.sig")
+			req.Header.Set(headerShareToken, "sharetoken")
+
+			req2, valid := authenticator.Authenticate(req)
+
+			// TODO Should the authentication of public path requests is handled by another authenticator?
+			//Expect(valid).To(Equal(false))
+			//Expect(req2).To(BeNil())
+			Expect(valid).To(Equal(true))
+			Expect(req2).ToNot(BeNil())
+		})
+		It("should skip authenticate if the 'public-token' is set", func() {
+			req := httptest.NewRequest(http.MethodGet, "http://example.com/dav/public-files/?public-token=sharetoken", http.NoBody)
+			req.Header.Set(_headerAuthorization, "Bearer jwt.token.sig")
+
+			req2, valid := authenticator.Authenticate(req)
+
+			// TODO Should the authentication of public path requests is handled by another authenticator?
+			//Expect(valid).To(Equal(false))
+			//Expect(req2).To(BeNil())
+			Expect(valid).To(Equal(true))
+			Expect(req2).ToNot(BeNil())
+		})
 	})
 })

services/proxy/pkg/middleware/public_share_auth.go

@@ -48,6 +48,13 @@ func isPublicShareAppOpen(r *http.Request) bool {
 		(r.URL.Query().Get(headerShareToken) != "" || r.Header.Get(headerShareToken) != "")
 }
 
+// The public-files requests can also be made in authenticated context. In these cases the OIDCAuthenticator and
+// the BasicAuthenticator needs to ignore the request when the headerShareToken exist.
+func isPublicWithShareToken(r *http.Request) bool {
+	return (strings.HasPrefix(r.URL.Path, "/dav/public-files") || strings.HasPrefix(r.URL.Path, "/remote.php/dav/public-files")) &&
+		(r.URL.Query().Get(headerShareToken) != "" || r.Header.Get(headerShareToken) != "")
+}
+
 // Authenticate implements the authenticator interface to authenticate requests via public share auth.
 func (a PublicShareAuthenticator) Authenticate(r *http.Request) (*http.Request, bool) {
 	if !isPublicPath(r.URL.Path) && !isPublicShareArchive(r) && !isPublicShareAppOpen(r) {