changed the sequence of the authenticators. added tests

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/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) && isPublicWithShareToken(r)) || isPublicShareAppOpen(r) || isPublicShareArchive(r) {
+	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

@@ -79,17 +79,23 @@ var _ = Describe("Authenticating requests", Label("OIDCAuthenticator"), func() {
 
 			req2, valid := authenticator.Authenticate(req)
 
-			Expect(valid).To(Equal(false))
-			Expect(req2).To(BeNil())
+			// 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)
 
-			Expect(valid).To(Equal(false))
-			Expect(req2).To(BeNil())
+			// 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())
 		})
 	})
 })