Skip to content

Commit

Permalink
replacement for TokenInfo endpoint
Browse files Browse the repository at this point in the history
  • Loading branch information
2403905 committed May 21, 2024
1 parent fa70142 commit b7ab99d
Show file tree
Hide file tree
Showing 11 changed files with 312 additions and 10 deletions.
13 changes: 13 additions & 0 deletions changelog/unreleased/tokenInfo-endpoint-replacement.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
Enhancement: Replacement for TokenInfo Endpoint


The client should basically always send a PROPFIND to /dav/public-files/{sharetoken}

* authenticated clients accessing an internal link are redirected to the "real" resource (`/dav/spaces/{target-resource-id}
* authenticated clients accessing a public link (password protected or not) for a resource they already have access to are also redirected to the "real" resource. (and always need to supply the password)
* 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
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ require (
github.com/cenkalti/backoff v2.2.1+incompatible
github.com/coreos/go-oidc/v3 v3.10.0
github.com/cs3org/go-cs3apis v0.0.0-20231023073225-7748710e0781
github.com/cs3org/reva/v2 v2.19.2-0.20240510133919-1732d68a5591
github.com/cs3org/reva/v2 v2.19.2-0.20240514074937-7099ed49d7cc
github.com/dhowden/tag v0.0.0-20230630033851-978a0926ee25
github.com/dutchcoders/go-clamd v0.0.0-20170520113014-b970184f4d9e
github.com/egirna/icap-client v0.1.1
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1025,8 +1025,8 @@ github.com/crewjam/saml v0.4.14 h1:g9FBNx62osKusnFzs3QTN5L9CVA/Egfgm+stJShzw/c=
github.com/crewjam/saml v0.4.14/go.mod h1:UVSZCf18jJkk6GpWNVqcyQJMD5HsRugBPf4I1nl2mME=
github.com/cs3org/go-cs3apis v0.0.0-20231023073225-7748710e0781 h1:BUdwkIlf8IS2FasrrPg8gGPHQPOrQ18MS1Oew2tmGtY=
github.com/cs3org/go-cs3apis v0.0.0-20231023073225-7748710e0781/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
github.com/cs3org/reva/v2 v2.19.2-0.20240510133919-1732d68a5591 h1:6rfzawhTzz96p8ifzcjNiMknAfAdVpfqdyzmpp3+6w8=
github.com/cs3org/reva/v2 v2.19.2-0.20240510133919-1732d68a5591/go.mod h1:BOlJApKFrWRiaOoBCRxCTG5bghTTMlYaEZrRxOzKaS8=
github.com/cs3org/reva/v2 v2.19.2-0.20240514074937-7099ed49d7cc h1:ph3lAdYgF9hTwuBfurXBrnPTVBg/fa8gfLCwcxyudjs=
github.com/cs3org/reva/v2 v2.19.2-0.20240514074937-7099ed49d7cc/go.mod h1:BOlJApKFrWRiaOoBCRxCTG5bghTTMlYaEZrRxOzKaS8=
github.com/cyberdelia/templates v0.0.0-20141128023046-ca7fffd4298c/go.mod h1:GyV+0YP4qX0UQ7r2MoYZ+AvYDp12OF5yg4q8rGnyNh4=
github.com/cyphar/filepath-securejoin v0.2.4 h1:Ugdm7cg7i6ZK6x3xDF1oEu1nfkyfH53EtKeQYTC3kyg=
github.com/cyphar/filepath-securejoin v0.2.4/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4=
Expand Down
8 changes: 4 additions & 4 deletions services/proxy/pkg/command/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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,
Expand Down
183 changes: 183 additions & 0 deletions services/proxy/pkg/middleware/authentication_test.go
Original file line number Diff line number Diff line change
@@ -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() {
Expand All @@ -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))
})
})
})
2 changes: 1 addition & 1 deletion services/proxy/pkg/middleware/basic_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 4 additions & 1 deletion services/proxy/pkg/middleware/oidc_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
34 changes: 34 additions & 0 deletions services/proxy/pkg/middleware/oidc_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
})
})
7 changes: 7 additions & 0 deletions services/proxy/pkg/middleware/public_share_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading

0 comments on commit b7ab99d

Please sign in to comment.