From b438a0f5c51b28d718d0c3cb6a0227a799604fe3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 21 Jul 2021 15:37:12 +0000 Subject: [PATCH] use claims map instead of struct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- ocis-pkg/middleware/openidconnect.go | 4 +- ocis-pkg/oidc/claims.go | 14 ++++- ocis-pkg/oidc/context.go | 10 ++-- proxy/pkg/command/server.go | 2 + proxy/pkg/config/config.go | 2 + proxy/pkg/flagset/flagset.go | 16 ++++++ proxy/pkg/middleware/account_resolver.go | 29 +++++----- proxy/pkg/middleware/account_resolver_test.go | 55 ++++++++++--------- proxy/pkg/middleware/basic_auth.go | 15 ++--- proxy/pkg/middleware/oidc_auth.go | 12 ++-- proxy/pkg/middleware/options.go | 18 ++++++ proxy/pkg/proxy/policy/selector.go | 20 ++++--- proxy/pkg/proxy/policy/selector_test.go | 6 +- proxy/pkg/user/backend/accounts.go | 25 ++++++--- proxy/pkg/user/backend/accounts_test.go | 14 ++--- proxy/pkg/user/backend/backend.go | 4 +- proxy/pkg/user/backend/cs3.go | 3 +- proxy/pkg/user/backend/test/backend_mock.go | 13 ++--- 18 files changed, 163 insertions(+), 99 deletions(-) diff --git a/ocis-pkg/middleware/openidconnect.go b/ocis-pkg/middleware/openidconnect.go index d33fbd9d680..868eeb621d7 100644 --- a/ocis-pkg/middleware/openidconnect.go +++ b/ocis-pkg/middleware/openidconnect.go @@ -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{ @@ -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) }) diff --git a/ocis-pkg/oidc/claims.go b/ocis-pkg/oidc/claims.go index 666e6146175..a35d4322e32 100644 --- a/ocis-pkg/oidc/claims.go +++ b/ocis-pkg/oidc/claims.go @@ -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 { @@ -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"` } diff --git a/ocis-pkg/oidc/context.go b/ocis-pkg/oidc/context.go index 9cfce359dfb..1f4bf2d6502 100644 --- a/ocis-pkg/oidc/context.go +++ b/ocis-pkg/oidc/context.go @@ -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 } diff --git a/proxy/pkg/command/server.go b/proxy/pkg/command/server.go index 797c36d74b7..0fbc30abe1a 100644 --- a/proxy/pkg/command/server.go +++ b/proxy/pkg/command/server.go @@ -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), ), diff --git a/proxy/pkg/config/config.go b/proxy/pkg/config/config.go index 0a7909e0403..a0b20b2d0f6 100644 --- a/proxy/pkg/config/config.go +++ b/proxy/pkg/config/config.go @@ -119,6 +119,8 @@ type Config struct { Reva Reva PreSignedURL PreSignedURL AccountBackend string + UserOIDCClaim string + UserCS3Claim string AutoprovisionAccounts bool EnableBasicAuth bool InsecureBackends bool diff --git a/proxy/pkg/flagset/flagset.go b/proxy/pkg/flagset/flagset.go index b2cbd818e31..cf6efe76142 100644 --- a/proxy/pkg/flagset/flagset.go +++ b/proxy/pkg/flagset/flagset.go @@ -235,6 +235,8 @@ 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), @@ -242,6 +244,20 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag { 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{ diff --git a/proxy/pkg/middleware/account_resolver.go b/proxy/pkg/middleware/account_resolver.go index 3bfa4e7f3f0..77c8d91c065 100644 --- a/proxy/pkg/middleware/account_resolver.go +++ b/proxy/pkg/middleware/account_resolver.go @@ -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" @@ -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, } } @@ -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()) @@ -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.") diff --git a/proxy/pkg/middleware/account_resolver_test.go b/proxy/pkg/middleware/account_resolver_test.go index f0b456a6edc..8247cb04922 100644 --- a/proxy/pkg/middleware/account_resolver_test.go +++ b/proxy/pkg/middleware/account_resolver_test.go @@ -2,7 +2,11 @@ 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" @@ -10,20 +14,17 @@ import ( "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: "foo@example.com", - }, nil) + }, nil, oidc.Email, "mail") - req, rw := mockRequest(&oidc.StandardClaims{ - Iss: "https://idx.example.com", - Email: "foo@example.com", + req, rw := mockRequest(map[string]interface{}{ + oidc.Iss: "https://idx.example.com", + oidc.Email: "foo@example.com", }) sut.ServeHTTP(rw, req) @@ -37,11 +38,11 @@ func TestTokenIsAddedWithUsernameClaim(t *testing.T) { sut := newMockAccountResolver(&userv1beta1.User{ Id: &userv1beta1.UserId{Idp: "https://idx.example.com", OpaqueId: "123"}, Mail: "foo@example.com", - }, 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) @@ -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) @@ -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) @@ -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) @@ -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) @@ -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 @@ -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() } diff --git a/proxy/pkg/middleware/basic_auth.go b/proxy/pkg/middleware/basic_auth.go index df710c6e4b1..62c5d612255 100644 --- a/proxy/pkg/middleware/basic_auth.go +++ b/proxy/pkg/middleware/basic_auth.go @@ -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/" @@ -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))) diff --git a/proxy/pkg/middleware/oidc_auth.go b/proxy/pkg/middleware/oidc_auth.go index c21bd6a4384..f225dd96b59 100644 --- a/proxy/pkg/middleware/oidc_auth.go +++ b/proxy/pkg/middleware/oidc_auth.go @@ -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))) }) } } @@ -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) @@ -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) @@ -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 } diff --git a/proxy/pkg/middleware/options.go b/proxy/pkg/middleware/options.go index f9a8ce86ebb..e0144ca8e7c 100644 --- a/proxy/pkg/middleware/options.go +++ b/proxy/pkg/middleware/options.go @@ -41,6 +41,10 @@ type Options struct { Store storepb.StoreService // PreSignedURLConfig to configure the middleware PreSignedURLConfig config.PreSignedURL + // UserOIDCClaim to read from the oidc claims + UserOIDCClaim string + // UserCS3Claim to use when looking up a user in the CS3 API + UserCS3Claim string // AutoprovisionAccounts when an accountResolver does not exist. AutoprovisionAccounts bool // EnableBasicAuth to allow basic auth @@ -141,6 +145,20 @@ func PreSignedURLConfig(cfg config.PreSignedURL) Option { } } +// UserOIDCClaim provides a function to set the UserClaim config +func UserOIDCClaim(val string) Option { + return func(o *Options) { + o.UserOIDCClaim = val + } +} + +// UserCS3Claim provides a function to set the UserClaimType config +func UserCS3Claim(val string) Option { + return func(o *Options) { + o.UserCS3Claim = val + } +} + // AutoprovisionAccounts provides a function to set the AutoprovisionAccounts config func AutoprovisionAccounts(val bool) Option { return func(o *Options) { diff --git a/proxy/pkg/proxy/policy/selector.go b/proxy/pkg/proxy/policy/selector.go index aa33b6410b8..10e259d1945 100644 --- a/proxy/pkg/proxy/policy/selector.go +++ b/proxy/pkg/proxy/policy/selector.go @@ -98,16 +98,22 @@ func NewStaticSelector(cfg *config.StaticSelectorConf) Selector { func NewMigrationSelector(cfg *config.MigrationSelectorConf, ss accounts.AccountsService) Selector { var acc = ss return func(ctx context.Context, r *http.Request) (s string, err error) { + var claims map[string]interface{} + if claims = oidc.FromContext(r.Context()); claims == nil { + return cfg.UnauthenticatedPolicy, nil + } + var userID string - if claims := oidc.FromContext(r.Context()); claims != nil { - userID = claims.PreferredUsername - if _, err := acc.GetAccount(ctx, &accounts.GetAccountRequest{Id: userID}); err != nil { - return cfg.AccNotFoundPolicy, nil - } + var ok bool + if userID, ok = claims[oidc.PreferredUsername].(string); !ok { + // TODO clarify: what if the user just has no username ... + return cfg.AccNotFoundPolicy, nil + } - return cfg.AccFoundPolicy, nil + if _, err := acc.GetAccount(ctx, &accounts.GetAccountRequest{Id: userID}); err != nil { + return cfg.AccNotFoundPolicy, nil } + return cfg.AccFoundPolicy, nil - return cfg.UnauthenticatedPolicy, nil } } diff --git a/proxy/pkg/proxy/policy/selector_test.go b/proxy/pkg/proxy/policy/selector_test.go index 0964fc0198d..78bb2d3b383 100644 --- a/proxy/pkg/proxy/policy/selector_test.go +++ b/proxy/pkg/proxy/policy/selector_test.go @@ -69,7 +69,7 @@ func TestStaticSelector(t *testing.T) { type testCase struct { AccSvcShouldReturnError bool - Claims *oidc.StandardClaims + Claims map[string]interface{} Expected string } @@ -80,8 +80,8 @@ func TestMigrationSelector(t *testing.T) { UnauthenticatedPolicy: "unauth", } var tests = []testCase{ - {true, &oidc.StandardClaims{PreferredUsername: "Hans"}, "not_found"}, - {false, &oidc.StandardClaims{PreferredUsername: "Hans"}, "found"}, + {true, map[string]interface{}{oidc.PreferredUsername: "Hans"}, "not_found"}, + {false, map[string]interface{}{oidc.PreferredUsername: "Hans"}, "found"}, {false, nil, "unauth"}, } diff --git a/proxy/pkg/user/backend/accounts.go b/proxy/pkg/user/backend/accounts.go index 98164688868..36241c4cbea 100644 --- a/proxy/pkg/user/backend/accounts.go +++ b/proxy/pkg/user/backend/accounts.go @@ -96,18 +96,29 @@ func (a *accountsServiceBackend) Authenticate(ctx context.Context, username stri return user, nil } -func (a accountsServiceBackend) CreateUserFromClaims(ctx context.Context, claims *oidc.StandardClaims) (*cs3.User, error) { - // TODO check if fields are missing. +func (a accountsServiceBackend) CreateUserFromClaims(ctx context.Context, claims map[string]interface{}) (*cs3.User, error) { req := &accounts.CreateAccountRequest{ Account: &accounts.Account{ - DisplayName: claims.DisplayName, - PreferredName: claims.PreferredUsername, - OnPremisesSamAccountName: claims.PreferredUsername, - Mail: claims.Email, CreationType: "LocalAccount", AccountEnabled: true, }, } + var ok bool + if req.Account.DisplayName, ok = claims[oidc.Name].(string); !ok { + a.logger.Debug().Msg("Missing name claim, trying displayname") + if req.Account.DisplayName, ok = claims["displayname"].(string); !ok { + a.logger.Debug().Msg("Missing displayname claim") + } + } + if req.Account.PreferredName, ok = claims[oidc.PreferredUsername].(string); !ok { + a.logger.Warn().Msg("Missing preferred_username claim") + } else { + // also use as on premises samaccount name + req.Account.OnPremisesSamAccountName = req.Account.PreferredName + } + if req.Account.Mail, ok = claims[oidc.Email].(string); !ok { + a.logger.Warn().Msg("Missing email claim") + } created, err := a.accountsClient.CreateAccount(context.Background(), req) if err != nil { return nil, err @@ -116,7 +127,7 @@ func (a accountsServiceBackend) CreateUserFromClaims(ctx context.Context, claims user := a.accountToUser(created) if err := injectRoles(ctx, user, a.settingsRoleService); err != nil { - a.logger.Warn().Err(err).Msgf("Could not load roles... continuing without") + a.logger.Warn().Err(err).Msg("Could not load roles... continuing without") } return user, nil diff --git a/proxy/pkg/user/backend/accounts_test.go b/proxy/pkg/user/backend/accounts_test.go index 11093d94a82..507aa909aca 100644 --- a/proxy/pkg/user/backend/accounts_test.go +++ b/proxy/pkg/user/backend/accounts_test.go @@ -108,13 +108,13 @@ func TestAuthenticateFailed(t *testing.T) { func TestCreateUserFromClaims(t *testing.T) { exp := mockAccResp[0] accBackend := newAccountsBackend([]*accounts.Account{}, expectedRoles) - act, _ := accBackend.CreateUserFromClaims(context.Background(), &oidc.StandardClaims{ - DisplayName: mockAccResp[0].DisplayName, - PreferredUsername: mockAccResp[0].OnPremisesSamAccountName, - Email: mockAccResp[0].Mail, - UIDNumber: "1", - GIDNumber: "2", - Groups: []string{"g1", "g2"}, + act, _ := accBackend.CreateUserFromClaims(context.Background(), map[string]interface{}{ + oidc.Name: mockAccResp[0].DisplayName, + oidc.PreferredUsername: mockAccResp[0].OnPremisesSamAccountName, + oidc.Email: mockAccResp[0].Mail, + oidc.UIDNumber: "1", + oidc.GIDNumber: "2", + oidc.Groups: []string{"g1", "g2"}, }) assert.NotNil(t, act.Id) diff --git a/proxy/pkg/user/backend/backend.go b/proxy/pkg/user/backend/backend.go index 12539279773..97314fb5668 100644 --- a/proxy/pkg/user/backend/backend.go +++ b/proxy/pkg/user/backend/backend.go @@ -4,10 +4,10 @@ import ( "context" "encoding/json" "errors" + gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" cs3 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" - "github.com/owncloud/ocis/ocis-pkg/oidc" settings "github.com/owncloud/ocis/settings/pkg/proto/v0" "google.golang.org/grpc" ) @@ -25,7 +25,7 @@ var ( type UserBackend interface { GetUserByClaims(ctx context.Context, claim, value string, withRoles bool) (*cs3.User, error) Authenticate(ctx context.Context, username string, password string) (*cs3.User, error) - CreateUserFromClaims(ctx context.Context, claims *oidc.StandardClaims) (*cs3.User, error) + CreateUserFromClaims(ctx context.Context, claims map[string]interface{}) (*cs3.User, error) GetUserGroups(ctx context.Context, userID string) } diff --git a/proxy/pkg/user/backend/cs3.go b/proxy/pkg/user/backend/cs3.go index 8a49616e317..fda3f138c2e 100644 --- a/proxy/pkg/user/backend/cs3.go +++ b/proxy/pkg/user/backend/cs3.go @@ -9,7 +9,6 @@ import ( rpcv1beta1 "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/owncloud/ocis/ocis-pkg/log" - "github.com/owncloud/ocis/ocis-pkg/oidc" settings "github.com/owncloud/ocis/settings/pkg/proto/v0" settingsSvc "github.com/owncloud/ocis/settings/pkg/service/v0" ) @@ -103,7 +102,7 @@ func (c *cs3backend) Authenticate(ctx context.Context, username string, password return res.User, nil } -func (c *cs3backend) CreateUserFromClaims(ctx context.Context, claims *oidc.StandardClaims) (*cs3.User, error) { +func (c *cs3backend) CreateUserFromClaims(ctx context.Context, claims map[string]interface{}) (*cs3.User, error) { return nil, fmt.Errorf("CS3 Backend does not support creating users from claims") } diff --git a/proxy/pkg/user/backend/test/backend_mock.go b/proxy/pkg/user/backend/test/backend_mock.go index 29287eefa49..1ebba2b92da 100644 --- a/proxy/pkg/user/backend/test/backend_mock.go +++ b/proxy/pkg/user/backend/test/backend_mock.go @@ -6,7 +6,6 @@ package test import ( "context" "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" - "github.com/owncloud/ocis/ocis-pkg/oidc" "github.com/owncloud/ocis/proxy/pkg/user/backend" "sync" ) @@ -44,7 +43,7 @@ type UserBackendMock struct { AuthenticateFunc func(ctx context.Context, username string, password string) (*userv1beta1.User, error) // CreateUserFromClaimsFunc mocks the CreateUserFromClaims method. - CreateUserFromClaimsFunc func(ctx context.Context, claims *oidc.StandardClaims) (*userv1beta1.User, error) + CreateUserFromClaimsFunc func(ctx context.Context, claims map[string]interface{}) (*userv1beta1.User, error) // GetUserByClaimsFunc mocks the GetUserByClaims method. GetUserByClaimsFunc func(ctx context.Context, claim string, value string, withRoles bool) (*userv1beta1.User, error) @@ -68,7 +67,7 @@ type UserBackendMock struct { // Ctx is the ctx argument value. Ctx context.Context // Claims is the claims argument value. - Claims *oidc.StandardClaims + Claims map[string]interface{} } // GetUserByClaims holds details about calls to the GetUserByClaims method. GetUserByClaims []struct { @@ -135,13 +134,13 @@ func (mock *UserBackendMock) AuthenticateCalls() []struct { } // CreateUserFromClaims calls CreateUserFromClaimsFunc. -func (mock *UserBackendMock) CreateUserFromClaims(ctx context.Context, claims *oidc.StandardClaims) (*userv1beta1.User, error) { +func (mock *UserBackendMock) CreateUserFromClaims(ctx context.Context, claims map[string]interface{}) (*userv1beta1.User, error) { if mock.CreateUserFromClaimsFunc == nil { panic("UserBackendMock.CreateUserFromClaimsFunc: method is nil but UserBackend.CreateUserFromClaims was just called") } callInfo := struct { Ctx context.Context - Claims *oidc.StandardClaims + Claims map[string]interface{} }{ Ctx: ctx, Claims: claims, @@ -157,11 +156,11 @@ func (mock *UserBackendMock) CreateUserFromClaims(ctx context.Context, claims *o // len(mockedUserBackend.CreateUserFromClaimsCalls()) func (mock *UserBackendMock) CreateUserFromClaimsCalls() []struct { Ctx context.Context - Claims *oidc.StandardClaims + Claims map[string]interface{} } { var calls []struct { Ctx context.Context - Claims *oidc.StandardClaims + Claims map[string]interface{} } mock.lockCreateUserFromClaims.RLock() calls = mock.calls.CreateUserFromClaims