Skip to content

Commit

Permalink
use claims map instead of struct
Browse files Browse the repository at this point in the history
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
  • Loading branch information
butonic committed Jul 21, 2021
1 parent 90ca98d commit b438a0f
Show file tree
Hide file tree
Showing 18 changed files with 163 additions and 99 deletions.
4 changes: 2 additions & 2 deletions ocis-pkg/middleware/openidconnect.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func OpenIDConnect(opts ...ocisoidc.Option) func(http.Handler) http.Handler {
}

// The claims we want to have
var claims ocisoidc.StandardClaims
var claims map[string]interface{}

// TODO cache userinfo for access token if we can determine the expiry (which works in case it is a jwt based access token)
oauth2Token := &oauth2.Token{
Expand All @@ -99,7 +99,7 @@ func OpenIDConnect(opts ...ocisoidc.Option) func(http.Handler) http.Handler {
opt.Logger.Debug().Interface("claims", claims).Interface("userInfo", userInfo).Msg("unmarshalled userinfo")
// store claims in context
// uses the original context, not the one with probably reduced security
nr := r.WithContext(ocisoidc.NewContext(r.Context(), &claims))
nr := r.WithContext(ocisoidc.NewContext(r.Context(), claims))

next.ServeHTTP(w, nr)
})
Expand Down
14 changes: 13 additions & 1 deletion ocis-pkg/oidc/claims.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
package oidc

const (
Iss = "iss"
Sub = "sub"
Email = "email"
Name = "name"
PreferredUsername = "preferred_username"
UIDNumber = "uidnumber"
GIDNumber = "gidnumber"
Groups = "groups"
OwncloudUUID = "ownclouduuid"
)

// The ProviderMetadata describes an idp.
// see https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata
type ProviderMetadata struct {
Expand Down Expand Up @@ -179,5 +191,5 @@ type StandardClaims struct {
GIDNumber string `json:"gidnumber,omitempty"`

// OcisID is a unique, persistent, non reassignable user id
OcisID string `json:"ocis.id,omitempty"`
OcisID string `json:"ownclouduuid,omitempty"`
}
10 changes: 5 additions & 5 deletions ocis-pkg/oidc/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ import "context"
// contextKey is the key for oidc claims in a context
type contextKey struct{}

// NewContext makes a new context that contains the OpenID Connect claims.
func NewContext(parent context.Context, c *StandardClaims) context.Context {
// NewContext makes a new context that contains the OpenID connect claims in a map.
func NewContext(parent context.Context, c map[string]interface{}) context.Context {
return context.WithValue(parent, contextKey{}, c)
}

// FromContext returns the StandardClaims stored in a context, or nil if there isn't one.
func FromContext(ctx context.Context) *StandardClaims {
s, _ := ctx.Value(contextKey{}).(*StandardClaims)
// FromContext returns the claims map stored in a context, or nil if there isn't one.
func FromContext(ctx context.Context) map[string]interface{} {
s, _ := ctx.Value(contextKey{}).(map[string]interface{})
return s
}
2 changes: 2 additions & 0 deletions proxy/pkg/command/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,8 @@ func loadMiddlewares(ctx context.Context, l log.Logger, cfg *config.Config) alic
middleware.Logger(l),
middleware.UserProvider(userProvider),
middleware.TokenManagerConfig(cfg.TokenManager),
middleware.UserOIDCClaim(cfg.UserOIDCClaim),
middleware.UserCS3Claim(cfg.UserCS3Claim),
middleware.AutoprovisionAccounts(cfg.AutoprovisionAccounts),
),

Expand Down
2 changes: 2 additions & 0 deletions proxy/pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ type Config struct {
Reva Reva
PreSignedURL PreSignedURL
AccountBackend string
UserOIDCClaim string
UserCS3Claim string
AutoprovisionAccounts bool
EnableBasicAuth bool
InsecureBackends bool
Expand Down
16 changes: 16 additions & 0 deletions proxy/pkg/flagset/flagset.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,13 +235,29 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag {
Destination: &cfg.OIDC.UserinfoCache.Size,
},

// account related config

&cli.BoolFlag{
Name: "autoprovision-accounts",
Value: flags.OverrideDefaultBool(cfg.AutoprovisionAccounts, false),
Usage: "create accounts from OIDC access tokens to learn new users",
EnvVars: []string{"PROXY_AUTOPROVISION_ACCOUNTS"},
Destination: &cfg.AutoprovisionAccounts,
},
&cli.StringFlag{
Name: "user-oidc-claim",
Value: flags.OverrideDefaultString(cfg.UserOIDCClaim, "email"),
Usage: "The OIDC claim that is used to identify users, eg. 'ownclouduuid', 'uid', 'cn' or 'email'",
EnvVars: []string{"PROXY_USER_OIDC_CLAIM"},
Destination: &cfg.UserOIDCClaim,
},
&cli.StringFlag{
Name: "user-cs3-claim",
Value: flags.OverrideDefaultString(cfg.UserCS3Claim, "mail"),
Usage: "The claim to use when looking up a user in the CS3 API, eg. 'userid' or 'mail'",
EnvVars: []string{"PROXY_USER_CS3_CLAIM"},
Destination: &cfg.UserCS3Claim,
},

// Pre Signed URLs
&cli.StringSliceFlag{
Expand Down
29 changes: 14 additions & 15 deletions proxy/pkg/middleware/account_resolver.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package middleware

import (
"net/http"

"github.com/cs3org/reva/pkg/auth/scope"
"github.com/owncloud/ocis/proxy/pkg/user/backend"
"net/http"

tokenPkg "github.com/cs3org/reva/pkg/token"
"github.com/cs3org/reva/pkg/token/manager/jwt"
Expand Down Expand Up @@ -32,6 +33,8 @@ func AccountResolver(optionSetters ...Option) func(next http.Handler) http.Handl
logger: logger,
tokenManager: tokenManager,
userProvider: options.UserProvider,
userOIDCClaim: options.UserOIDCClaim,
userCS3Claim: options.UserCS3Claim,
autoProvisionAccounts: options.AutoprovisionAccounts,
}
}
Expand All @@ -43,8 +46,11 @@ type accountResolver struct {
tokenManager tokenPkg.Manager
userProvider backend.UserBackend
autoProvisionAccounts bool
userOIDCClaim string
userCS3Claim string
}

// TODO do not use the context to store values: https://medium.com/@cep21/how-to-correctly-use-context-context-in-go-1-7-8f2c0fafdf39
func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) {
claims := oidc.FromContext(req.Context())
u, ok := revauser.ContextGetUser(req.Context())
Expand All @@ -55,22 +61,15 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) {
}

if u == nil && claims != nil {
var claim, value string
switch {
case claims.PreferredUsername != "":
claim, value = "username", claims.PreferredUsername
case claims.Email != "":
claim, value = "mail", claims.Email
case claims.OcisID != "":
//claim, value = "id", claims.OcisID
default:
// TODO allow lookup by custom claim, eg an id ... or sub
m.logger.Error().Msg("Could not lookup account, no mail or preferred_username claim set")
w.WriteHeader(http.StatusInternalServerError)
}

var err error
u, err = m.userProvider.GetUserByClaims(req.Context(), claim, value, true)
if value, ok := claims[m.userOIDCClaim].(string); ok && value != "" {
u, err = m.userProvider.GetUserByClaims(req.Context(), m.userCS3Claim, value, true)
} else {
m.logger.Error().Str("claim", m.userOIDCClaim).Interface("claims", claims).Msg("claim not set or empty")
w.WriteHeader(http.StatusInternalServerError)
return
}

if m.autoProvisionAccounts && err == backend.ErrAccountNotFound {
m.logger.Debug().Interface("claims", claims).Interface("user", u).Msgf("User by claim not found... autoprovisioning.")
Expand Down
55 changes: 29 additions & 26 deletions proxy/pkg/middleware/account_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,29 @@ package middleware

import (
"context"
"github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
"net/http"
"net/http/httptest"
"testing"

userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
"github.com/cs3org/reva/pkg/token"
"github.com/owncloud/ocis/ocis-pkg/log"
"github.com/owncloud/ocis/ocis-pkg/oidc"
"github.com/owncloud/ocis/proxy/pkg/config"
"github.com/owncloud/ocis/proxy/pkg/user/backend"
"github.com/owncloud/ocis/proxy/pkg/user/backend/test"
"github.com/stretchr/testify/assert"
"net/http"
"net/http/httptest"
"testing"
)

func TestTokenIsAddedWithMailClaim(t *testing.T) {
sut := newMockAccountResolver(&userv1beta1.User{
Id: &userv1beta1.UserId{Idp: "https://idx.example.com", OpaqueId: "123"},
Mail: "[email protected]",
}, nil)
}, nil, oidc.Email, "mail")

req, rw := mockRequest(&oidc.StandardClaims{
Iss: "https://idx.example.com",
Email: "[email protected]",
req, rw := mockRequest(map[string]interface{}{
oidc.Iss: "https://idx.example.com",
oidc.Email: "[email protected]",
})

sut.ServeHTTP(rw, req)
Expand All @@ -37,11 +38,11 @@ func TestTokenIsAddedWithUsernameClaim(t *testing.T) {
sut := newMockAccountResolver(&userv1beta1.User{
Id: &userv1beta1.UserId{Idp: "https://idx.example.com", OpaqueId: "123"},
Mail: "[email protected]",
}, nil)
}, nil, oidc.PreferredUsername, "username")

req, rw := mockRequest(&oidc.StandardClaims{
Iss: "https://idx.example.com",
PreferredUsername: "foo",
req, rw := mockRequest(map[string]interface{}{
oidc.Iss: "https://idx.example.com",
oidc.PreferredUsername: "foo",
})

sut.ServeHTTP(rw, req)
Expand All @@ -53,7 +54,7 @@ func TestTokenIsAddedWithUsernameClaim(t *testing.T) {
}

func TestNSkipOnNoClaims(t *testing.T) {
sut := newMockAccountResolver(nil, backend.ErrAccountDisabled)
sut := newMockAccountResolver(nil, backend.ErrAccountDisabled, oidc.Email, "mail")
req, rw := mockRequest(nil)

sut.ServeHTTP(rw, req)
Expand All @@ -64,10 +65,10 @@ func TestNSkipOnNoClaims(t *testing.T) {
}

func TestUnauthorizedOnUserNotFound(t *testing.T) {
sut := newMockAccountResolver(nil, backend.ErrAccountNotFound)
req, rw := mockRequest(&oidc.StandardClaims{
Iss: "https://idx.example.com",
PreferredUsername: "foo",
sut := newMockAccountResolver(nil, backend.ErrAccountNotFound, oidc.PreferredUsername, "username")
req, rw := mockRequest(map[string]interface{}{
oidc.Iss: "https://idx.example.com",
oidc.PreferredUsername: "foo",
})

sut.ServeHTTP(rw, req)
Expand All @@ -78,10 +79,10 @@ func TestUnauthorizedOnUserNotFound(t *testing.T) {
}

func TestUnauthorizedOnUserDisabled(t *testing.T) {
sut := newMockAccountResolver(nil, backend.ErrAccountDisabled)
req, rw := mockRequest(&oidc.StandardClaims{
Iss: "https://idx.example.com",
PreferredUsername: "foo",
sut := newMockAccountResolver(nil, backend.ErrAccountDisabled, oidc.PreferredUsername, "username")
req, rw := mockRequest(map[string]interface{}{
oidc.Iss: "https://idx.example.com",
oidc.PreferredUsername: "foo",
})

sut.ServeHTTP(rw, req)
Expand All @@ -92,9 +93,9 @@ func TestUnauthorizedOnUserDisabled(t *testing.T) {
}

func TestInternalServerErrorOnMissingMailAndUsername(t *testing.T) {
sut := newMockAccountResolver(nil, backend.ErrAccountDisabled)
req, rw := mockRequest(&oidc.StandardClaims{
Iss: "https://idx.example.com",
sut := newMockAccountResolver(nil, backend.ErrAccountNotFound, oidc.Email, "mail")
req, rw := mockRequest(map[string]interface{}{
oidc.Iss: "https://idx.example.com",
})

sut.ServeHTTP(rw, req)
Expand All @@ -104,7 +105,7 @@ func TestInternalServerErrorOnMissingMailAndUsername(t *testing.T) {
assert.Equal(t, http.StatusInternalServerError, rw.Code)
}

func newMockAccountResolver(userBackendResult *userv1beta1.User, userBackendErr error) http.Handler {
func newMockAccountResolver(userBackendResult *userv1beta1.User, userBackendErr error, oidcclaim, cs3claim string) http.Handler {
mock := &test.UserBackendMock{
GetUserByClaimsFunc: func(ctx context.Context, claim string, value string, withRoles bool) (*userv1beta1.User, error) {
return userBackendResult, userBackendErr
Expand All @@ -115,11 +116,13 @@ func newMockAccountResolver(userBackendResult *userv1beta1.User, userBackendErr
Logger(log.NewLogger()),
UserProvider(mock),
TokenManagerConfig(config.TokenManager{JWTSecret: "secret"}),
UserOIDCClaim(oidcclaim),
UserCS3Claim(cs3claim),
AutoprovisionAccounts(false),
)(mockHandler{})
}

func mockRequest(claims *oidc.StandardClaims) (*http.Request, *httptest.ResponseRecorder) {
func mockRequest(claims map[string]interface{}) (*http.Request, *httptest.ResponseRecorder) {
if claims == nil {
return httptest.NewRequest("GET", "http://example.com/foo", nil), httptest.NewRecorder()
}
Expand Down
15 changes: 8 additions & 7 deletions proxy/pkg/middleware/basic_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ package middleware

import (
"fmt"
"net/http"
"strings"

"github.com/owncloud/ocis/ocis-pkg/log"
"github.com/owncloud/ocis/ocis-pkg/oidc"
"github.com/owncloud/ocis/proxy/pkg/user/backend"
"net/http"
"strings"
)

const publicFilesEndpoint = "/remote.php/dav/public-files/"
Expand Down Expand Up @@ -65,11 +66,11 @@ func BasicAuth(optionSetters ...Option) func(next http.Handler) http.Handler {
return
}

claims := &oidc.StandardClaims{
OcisID: user.Id.OpaqueId,
Iss: user.Id.Idp,
PreferredUsername: user.Username,
Email: user.Mail,
claims := map[string]interface{}{
"ownclouduuid": user.Id.OpaqueId,
"iss": user.Id.Idp,
"preferred_username": user.Username,
"email": user.Mail,
}

next.ServeHTTP(w, req.WithContext(oidc.NewContext(req.Context(), claims)))
Expand Down
12 changes: 4 additions & 8 deletions proxy/pkg/middleware/oidc_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,11 @@ func OIDCAuth(optionSetters ...Option) func(next http.Handler) http.Handler {
}

// inject claims to the request context for the account_uuid middleware.
req = req.WithContext(oidc.NewContext(req.Context(), &claims))
req = req.WithContext(oidc.NewContext(req.Context(), claims))

// store claims in context
// uses the original context, not the one with probably reduced security
next.ServeHTTP(w, req.WithContext(oidc.NewContext(req.Context(), &claims)))
next.ServeHTTP(w, req.WithContext(oidc.NewContext(req.Context(), claims)))
})
}
}
Expand All @@ -78,7 +78,7 @@ type oidcAuth struct {
tokenCacheTTL time.Duration
}

func (m oidcAuth) getClaims(token string, req *http.Request) (claims oidc.StandardClaims, status int) {
func (m oidcAuth) getClaims(token string, req *http.Request) (claims map[string]interface{}, status int) {
hit := m.tokenCache.Load(token)
if hit == nil {
// TODO cache userinfo for access token if we can determine the expiry (which works in case it is a jwt based access token)
Expand All @@ -96,16 +96,12 @@ func (m oidcAuth) getClaims(token string, req *http.Request) (claims oidc.Standa
return
}

// TODO allow extracting arbitrary claims ... or require idp to send a specific claim
if err := userInfo.Claims(&claims); err != nil {
m.logger.Error().Err(err).Interface("userinfo", userInfo).Msg("failed to unmarshal userinfo claims")
status = http.StatusInternalServerError
return
}

//TODO: This should be read from the token instead of config
claims.Iss = m.oidcIss

expiration := m.extractExpiration(token)
m.tokenCache.Store(token, claims, expiration)

Expand All @@ -114,7 +110,7 @@ func (m oidcAuth) getClaims(token string, req *http.Request) (claims oidc.Standa
}

var ok bool
if claims, ok = hit.V.(oidc.StandardClaims); !ok {
if claims, ok = hit.V.(map[string]interface{}); !ok {
status = http.StatusInternalServerError
return
}
Expand Down
Loading

0 comments on commit b438a0f

Please sign in to comment.