Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use claims map instead of struct #2315

Merged
merged 2 commits into from
Jul 22, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
48 changes: 27 additions & 21 deletions proxy/pkg/middleware/account_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,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 @@ -44,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 @@ -56,30 +61,31 @@ 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)
var value string
var ok bool
if value, ok = claims[m.userOIDCClaim].(string); !ok || value == "" {
m.logger.Error().Str("claim", m.userOIDCClaim).Interface("claims", claims).Msg("claim not set or empty")
w.WriteHeader(http.StatusInternalServerError) // admin needs to make the idp send the right claim
return
}

u, err = m.userProvider.GetUserByClaims(req.Context(), m.userCS3Claim, value, true)

if m.autoProvisionAccounts && err == backend.ErrAccountNotFound {
m.logger.Debug().Interface("claims", claims).Interface("user", u).Msgf("User by claim not found... autoprovisioning.")
if err == backend.ErrAccountNotFound {
butonic marked this conversation as resolved.
Show resolved Hide resolved
m.logger.Debug().Str("claim", m.userOIDCClaim).Str("value", value).Msg("User by claim not found")
if !m.autoProvisionAccounts {
m.logger.Debug().Interface("claims", claims).Msg("Autoprovisioning disabled")
w.WriteHeader(http.StatusUnauthorized)
return
}
m.logger.Debug().Interface("claims", claims).Msg("Autoprovisioning user")
u, err = m.userProvider.CreateUserFromClaims(req.Context(), claims)
}

if err == backend.ErrAccountNotFound || err == backend.ErrAccountDisabled {
m.logger.Debug().Interface("claims", claims).Interface("user", u).Msgf("Unautorized")
if err == backend.ErrAccountDisabled {
butonic marked this conversation as resolved.
Show resolved Hide resolved
m.logger.Debug().Interface("claims", claims).Msg("Disabled")
w.WriteHeader(http.StatusUnauthorized)
return
}
Expand All @@ -90,17 +96,17 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) {
return
}

m.logger.Debug().Interface("claims", claims).Interface("user", u).Msgf("associated claims with uuid")
m.logger.Debug().Interface("claims", claims).Interface("user", u).Msg("associated claims with user")
}

s, err := scope.AddOwnerScope(nil)
if err != nil {
m.logger.Error().Err(err).Msgf("could not get owner scope")
m.logger.Error().Err(err).Msg("could not get owner scope")
return
}
token, err := m.tokenManager.MintToken(req.Context(), u, s)
if err != nil {
m.logger.Error().Err(err).Msgf("could not mint token")
m.logger.Error().Err(err).Msg("could not mint token")
w.WriteHeader(http.StatusInternalServerError)
return
}
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
11 changes: 6 additions & 5 deletions proxy/pkg/middleware/basic_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,12 @@ 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,
// fake oidc claims
claims := map[string]interface{}{
oidc.OwncloudUUID: user.Id.OpaqueId,
oidc.Iss: user.Id.Idp,
oidc.PreferredUsername: user.Username,
oidc.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
butonic marked this conversation as resolved.
Show resolved Hide resolved
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