diff --git a/changelog/unreleased/tokenInfo-endpoint-replacement.md b/changelog/unreleased/tokenInfo-endpoint-replacement.md new file mode 100644 index 00000000000..8a5d1880f3a --- /dev/null +++ b/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 diff --git a/services/proxy/pkg/command/server.go b/services/proxy/pkg/command/server.go index cfdb828337a..46987b6ed64 100644 --- a/services/proxy/pkg/command/server.go +++ b/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, diff --git a/services/proxy/pkg/middleware/authentication_test.go b/services/proxy/pkg/middleware/authentication_test.go index 17c151aebac..afc9782919d 100644 --- a/services/proxy/pkg/middleware/authentication_test.go +++ b/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: "testuser@example.com", + }, + "", + 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": "testuser@example.com", + "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)) + }) + }) +}) diff --git a/services/proxy/pkg/middleware/basic_auth.go b/services/proxy/pkg/middleware/basic_auth.go index 913e7404865..81ebadf0e84 100644 --- a/services/proxy/pkg/middleware/basic_auth.go +++ b/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. diff --git a/services/proxy/pkg/middleware/oidc_auth.go b/services/proxy/pkg/middleware/oidc_auth.go index 66101921eca..fd3f466a318 100644 --- a/services/proxy/pkg/middleware/oidc_auth.go +++ b/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 { diff --git a/services/proxy/pkg/middleware/oidc_auth_test.go b/services/proxy/pkg/middleware/oidc_auth_test.go index 589cbbc5c6e..eddb5eb72c5 100644 --- a/services/proxy/pkg/middleware/oidc_auth_test.go +++ b/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()) + }) }) }) diff --git a/services/proxy/pkg/middleware/public_share_auth.go b/services/proxy/pkg/middleware/public_share_auth.go index 403d5d598bb..c7468d6bbcc 100644 --- a/services/proxy/pkg/middleware/public_share_auth.go +++ b/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) {