From ce7a3b7f9f3866da05f6e6903a4445d63b30c8a5 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Fri, 28 Apr 2023 19:06:10 +0200 Subject: [PATCH 1/5] support multiple issuers in the oidc provider --- go.mod | 5 +- go.sum | 13 ++ pkg/auth/manager/oidc/oidc.go | 298 +++++++++++++--------------------- 3 files changed, 129 insertions(+), 187 deletions(-) diff --git a/go.mod b/go.mod index cdc0d08bfb..3649a5d430 100644 --- a/go.mod +++ b/go.mod @@ -66,7 +66,7 @@ require ( go.opentelemetry.io/otel/trace v1.11.2 go.step.sm/crypto v0.23.2 golang.org/x/crypto v0.5.0 - golang.org/x/oauth2 v0.0.0-20221014153046-6fdb5e3db783 + golang.org/x/oauth2 v0.3.0 golang.org/x/sync v0.1.0 golang.org/x/sys v0.5.0 golang.org/x/term v0.5.0 @@ -77,6 +77,8 @@ require ( gotest.tools v2.2.0+incompatible ) +require github.com/go-jose/go-jose/v3 v3.0.0 // indirect + require ( github.com/Azure/go-ntlmssp v0.0.0-20220621081337-cb9428e4ac1e // indirect github.com/Masterminds/goutils v1.1.1 // indirect @@ -87,6 +89,7 @@ require ( github.com/bmizerany/pat v0.0.0-20170815010413-6226ea591a40 // indirect github.com/cespare/xxhash v1.1.0 // indirect github.com/cespare/xxhash/v2 v2.1.2 // indirect + github.com/coreos/go-oidc/v3 v3.5.0 github.com/davecgh/go-spew v1.1.1 // indirect github.com/dolthub/vitess v0.0.0-20221031111135-9aad77e7b39f // indirect github.com/dustin/go-humanize v1.0.0 // indirect diff --git a/go.sum b/go.sum index 0e1f467108..3959aff6e3 100644 --- a/go.sum +++ b/go.sum @@ -57,6 +57,7 @@ cloud.google.com/go/compute v1.6.0/go.mod h1:T29tfhtVbq1wvAPo0E3+7vhgmkOYeXjhFvz cloud.google.com/go/compute v1.6.1/go.mod h1:g85FgpzFvNULZ+S8AYq87axRKuf2Kh7deLqV/jJ3thU= cloud.google.com/go/compute v1.7.0/go.mod h1:435lt8av5oL9P3fv1OEzSbSUe+ybHXGMPQHHZWZxy9U= cloud.google.com/go/compute v1.14.0 h1:hfm2+FfxVmnRlh6LpB7cg1ZNU+5edAHmW679JePztk0= +cloud.google.com/go/compute/metadata v0.2.0/go.mod h1:zFmK7XCadkQkj6TtorcaGlCW1hT1fIilQDwofLpJ20k= cloud.google.com/go/compute/metadata v0.2.3 h1:mg4jlk7mCAj6xXp9UJ4fjI9VUI5rubuGBW5aJ7UnBMY= cloud.google.com/go/datacatalog v1.5.0/go.mod h1:M7GPLNQeLfWqeIm3iuiruhPzkt65+Bx8dAKvScX8jvs= cloud.google.com/go/dataflow v0.6.0/go.mod h1:9QwV89cGoxjjSR9/r7eFDqqjtvbKxAK2BaYU6PVk9UM= @@ -289,6 +290,8 @@ github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkE github.com/coreos/etcd v3.3.13+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE= github.com/coreos/go-oidc v2.2.1+incompatible h1:mh48q/BqXqgjVHpy2ZY7WnWAbenxRjsz9N1i1YxjHAk= github.com/coreos/go-oidc v2.2.1+incompatible/go.mod h1:CgnwVTmzoESiwO9qyAFEMiHoZ1nMCKZlZ9V6mm3/LKc= +github.com/coreos/go-oidc/v3 v3.5.0 h1:VxKtbccHZxs8juq7RdJntSqtXFtde9YpNpGn0yqgEHw= +github.com/coreos/go-oidc/v3 v3.5.0/go.mod h1:ecXRtV4romGPeO6ieExAsUK9cb/3fp9hXNz1tlv8PIM= github.com/coreos/go-semver v0.2.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk= github.com/coreos/go-semver v0.3.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk= github.com/coreos/go-systemd v0.0.0-20180511133405-39ca1b05acc7/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4= @@ -393,6 +396,8 @@ github.com/go-git/go-git/v5 v5.4.2/go.mod h1:gQ1kArt6d+n+BGd+/B/I74HwRTLhth2+zti github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= +github.com/go-jose/go-jose/v3 v3.0.0 h1:s6rrhirfEP/CGIoc6p+PZAeogN2SxKav6Wp7+dyMWVo= +github.com/go-jose/go-jose/v3 v3.0.0/go.mod h1:RNkWWRld676jZEYoV3+XK8L2ZnNSvIsxFMht0mSX+u8= github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= github.com/go-kit/kit v0.9.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= github.com/go-kit/kit v0.10.0 h1:dXFJfIHVvUcpSgDOV+Ne6t7jXri8Tfv2uOLHUZ2XNuo= @@ -1234,6 +1239,7 @@ golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20190701094942-4def268fd1a4/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20190820162420-60c769a6c586/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/crypto v0.0.0-20190911031432-227b76d455e7/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20190923035154-9ee001bba392/go.mod h1:/lpIB1dKB+9EgE3H3cr1v9wB50oz8l4C4h62xy7jSTY= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20191112222119-e1110fd1c708/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= @@ -1364,6 +1370,8 @@ golang.org/x/net v0.0.0-20220624214902-1bab6f366d9e/go.mod h1:XRhObCWvk6IyKnWLug golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= golang.org/x/net v0.0.0-20220909164309-bea034e7d591/go.mod h1:YDH+HFinaLZZlnHAfSS6ZXJJ9M9t4Dl22yv3iI2vPwk= golang.org/x/net v0.1.0/go.mod h1:Cx3nUiGt4eDBEyega/BKRp+/AlGL8hYe7U9odMt2Cco= +golang.org/x/net v0.3.0/go.mod h1:MBQ8lrhLObU/6UmLb4fmbmk5OcyYmqtbGd/9yIeKjEE= +golang.org/x/net v0.4.0/go.mod h1:MBQ8lrhLObU/6UmLb4fmbmk5OcyYmqtbGd/9yIeKjEE= golang.org/x/net v0.7.0 h1:rJrUqqhjsgNp7KqAIc25s9pZnjU7TUcSY7HcVZjdn1g= golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -1391,6 +1399,8 @@ golang.org/x/oauth2 v0.0.0-20220822191816-0ebed06d0094/go.mod h1:h4gKUeWbJ4rQPri golang.org/x/oauth2 v0.0.0-20220909003341-f21342109be1/go.mod h1:h4gKUeWbJ4rQPri7E0u6Gs4e9Ri2zaLxzw5DI5XGrYg= golang.org/x/oauth2 v0.0.0-20221014153046-6fdb5e3db783 h1:nt+Q6cXKz4MosCSpnbMtqiQ8Oz0pxTef2B4Vca2lvfk= golang.org/x/oauth2 v0.0.0-20221014153046-6fdb5e3db783/go.mod h1:h4gKUeWbJ4rQPri7E0u6Gs4e9Ri2zaLxzw5DI5XGrYg= +golang.org/x/oauth2 v0.3.0 h1:6l90koy8/LaBLmLu8jpHeHexzMwEita0zFfYlggy2F8= +golang.org/x/oauth2 v0.3.0/go.mod h1:rQrIauxkUhJ6CuwEXwymO2/eh4xz2ZWF1nBkcxS+tGk= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -1530,6 +1540,7 @@ golang.org/x/sys v0.0.0-20220728004956-3c1f35247d10/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220928140112-f11e5e49a4ec/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20221010170243-090e33056c14/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201113234701-d7a72108b828/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= @@ -1537,6 +1548,7 @@ golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXR golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.1.0/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= +golang.org/x/term v0.3.0/go.mod h1:q750SLmJuPmVoN1blW3UFBPREJfb1KmY3vwxfr+nFDA= golang.org/x/term v0.5.0 h1:n2a8QNdAb0sZNpU9R1ALUXBbY+w51fCQDN+7EdxNBsY= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -1549,6 +1561,7 @@ golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= +golang.org/x/text v0.5.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.7.0 h1:4BRB4x83lYWy72KwLD/qYDuTu7q9PjSagHvijDw7cLo= golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= diff --git a/pkg/auth/manager/oidc/oidc.go b/pkg/auth/manager/oidc/oidc.go index 6d8169e118..5f72782af8 100644 --- a/pkg/auth/manager/oidc/oidc.go +++ b/pkg/auth/manager/oidc/oidc.go @@ -28,7 +28,7 @@ import ( "strings" "time" - oidc "github.com/coreos/go-oidc" + "github.com/coreos/go-oidc/v3/oidc" authpb "github.com/cs3org/go-cs3apis/cs3/auth/provider/v1beta1" user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" @@ -41,6 +41,7 @@ import ( "github.com/cs3org/reva/pkg/rgrpc/todo/pool" "github.com/cs3org/reva/pkg/rhttp" "github.com/cs3org/reva/pkg/sharedconf" + "github.com/golang-jwt/jwt" "github.com/juliangruber/go-intersect" "github.com/mitchellh/mapstructure" "github.com/pkg/errors" @@ -52,7 +53,8 @@ func init() { } type mgr struct { - provider *oidc.Provider // cached on first request + providers map[string]*oidc.Provider + c *config oidcUsersMapping map[string]*oidcUserMapping } @@ -103,7 +105,9 @@ func parseConfig(m map[string]interface{}) (*config, error) { // New returns an auth manager implementation that verifies the oidc token and obtains the user claims. func New(m map[string]interface{}) (auth.Manager, error) { - manager := &mgr{} + manager := &mgr{ + providers: make(map[string]*oidc.Provider), + } err := manager.Configure(m) if err != nil { return nil, err @@ -144,103 +148,147 @@ func (am *mgr) Configure(m map[string]interface{}) error { return nil } -// The clientID would be empty as we only need to validate the clientSecret variable -// which contains the access token that we can use to contact the UserInfo endpoint -// and get the user claims. -func (am *mgr) Authenticate(ctx context.Context, _, clientSecret string) (*user.User, map[string]*authpb.Scope, error) { - ctx = am.getOAuthCtx(ctx) - log := appctx.GetLogger(ctx) - - oidcProvider, err := am.getOIDCProvider(ctx) +func extractClaims(token string) (jwt.MapClaims, error) { + var claims jwt.MapClaims + _, _, err := new(jwt.Parser).ParseUnverified(token, &claims) if err != nil { - return nil, nil, fmt.Errorf("oidc: error creating oidc provider: +%v", err) + return nil, err } + return claims, nil +} - oauth2Token := &oauth2.Token{ - AccessToken: clientSecret, +func extractIssuer(m jwt.MapClaims) (string, bool) { + issIface, ok := m["iss"] + if !ok { + return "", false } + iss, _ := issIface.(string) + return iss, iss != "" +} - // query the oidc provider for user info - userInfo, err := oidcProvider.UserInfo(ctx, oauth2.StaticTokenSource(oauth2Token)) - if err != nil { - return nil, nil, fmt.Errorf("oidc: error getting userinfo: +%v", err) +func (am *mgr) getOIDCProviderForIssuer(ctx context.Context, issuer string) (*oidc.Provider, error) { + // FIXME: op not atomic TODO: fix message and make it more clear + if am.providers[issuer] == nil { + // TODO (gdelmont): the provider should be periodically recreated + // as the public key can change over time + provider, err := oidc.NewProvider(ctx, am.c.Issuer) + if err != nil { + return nil, errors.Wrapf(err, "oidc: error creating a new oidc provider") + } + am.providers[issuer] = provider } + return am.providers[issuer], nil +} - // claims contains the standard OIDC claims like iss, iat, aud, ... and any other non-standard one. - // TODO(labkode): make claims configuration dynamic from the config file so we can add arbitrary mappings from claims to user struct. - // For now, only the group claim is dynamic. - // TODO(labkode): may do like K8s does it: https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go - var claims map[string]interface{} - if err := userInfo.Claims(&claims); err != nil { - return nil, nil, fmt.Errorf("oidc: error unmarshaling userinfo claims: %v", err) +func (am *mgr) isIssuerAllowed(issuer string) bool { + if am.c.Issuer == issuer { + return true } + for _, m := range am.oidcUsersMapping { + if m.OIDCIssuer == issuer { + return true + } + } + return false +} - log.Debug().Interface("claims", claims).Interface("userInfo", userInfo).Msg("unmarshalled userinfo") +func (am *mgr) doUserMapping(tkn *oidc.IDToken, claims jwt.MapClaims) (string, error) { + if len(am.oidcUsersMapping) == 0 { + return tkn.Subject, nil + } + // we need the custom claims for the mapping + if claims[am.c.GroupClaim] == nil { + // we are required to perform a user mapping but the group claim is not available + return tkn.Subject, nil + } - if claims["iss"] == nil { // This is not set in simplesamlphp - claims["iss"] = am.c.Issuer + mappings := make([]string, 0, len(am.oidcUsersMapping)) + for _, m := range am.oidcUsersMapping { + if m.OIDCIssuer == tkn.Issuer { + mappings = append(mappings, m.OIDCGroup) + } } - if claims["email_verified"] == nil { // This is not set in simplesamlphp - claims["email_verified"] = false + + intersection := intersect.Simple(claims[am.c.GroupClaim], mappings) + if len(intersection) > 1 { + // multiple mappings are not implemented as we cannot decide which one to choose + return "", errtypes.PermissionDenied("more than one user mapping entry exists for the given group claims") } - if claims["preferred_username"] == nil { - claims["preferred_username"] = claims[am.c.IDClaim] + if len(intersection) == 0 { + return "", errtypes.PermissionDenied("no user mapping found for the given group claim(s)") } - if claims["preferred_username"] == nil { - claims["preferred_username"] = claims["email"] + m := intersection[0].(string) + return am.oidcUsersMapping[m].Username, nil +} + +// The clientID would be empty as we only need to validate the clientSecret variable +// which contains the access token that we can use to contact the UserInfo endpoint +// and get the user claims. +func (am *mgr) Authenticate(ctx context.Context, _, clientSecret string) (*user.User, map[string]*authpb.Scope, error) { + log := appctx.GetLogger(ctx) + ctx = am.getOAuthCtx(ctx) + + claims, err := extractClaims(clientSecret) + if err != nil { + return nil, nil, errtypes.PermissionDenied("oidc token not valid") } - if claims["name"] == nil { - claims["name"] = claims[am.c.IDClaim] + + issuer, ok := extractIssuer(claims) + if !ok { + return nil, nil, errtypes.PermissionDenied("issuer not contained in the token") + } + log.Debug().Str("issuer", issuer).Msg("extracted issuer from token") + + if !am.isIssuerAllowed(issuer) { + log.Debug().Str("issuer", issuer).Msg("issuer is not in the whitelist") + return nil, nil, errtypes.PermissionDenied("issuer not recognised") } - if claims["name"] == nil { - return nil, nil, fmt.Errorf("no \"name\" attribute found in userinfo: maybe the client did not request the oidc \"profile\"-scope") + log.Debug().Str("issuer", issuer).Msg("issuer is whitelisted") + + provider, err := am.getOIDCProviderForIssuer(ctx, issuer) + if err != nil { + return nil, nil, errors.Wrap(err, "oidc: error creating oidc provider") } - if claims["email"] == nil { - return nil, nil, fmt.Errorf("no \"email\" attribute found in userinfo: maybe the client did not request the oidc \"email\"-scope") + + config := &oidc.Config{ + SkipClientIDCheck: true, } - err = am.resolveUser(ctx, claims, userInfo.Subject) + tkn, err := provider.Verifier(config).Verify(ctx, clientSecret) if err != nil { - return nil, nil, errors.Wrapf(err, "oidc: error resolving username for external user '%v'", claims["email"]) + return nil, nil, errtypes.PermissionDenied(fmt.Sprintf("oidc token not valid: %+v", err)) } - userID := &user.UserId{ - OpaqueId: claims[am.c.IDClaim].(string), // a stable non reassignable id - Idp: claims["iss"].(string), // in the scope of this issuer - Type: getUserType(claims[am.c.IDClaim].(string)), + sub, err := am.doUserMapping(tkn, claims) + if err != nil { + return nil, nil, err } + log.Debug().Str("sub", sub).Msg("mapped user from token") - gwc, err := pool.GetGatewayServiceClient(pool.Endpoint(am.c.GatewaySvc)) + client, err := pool.GetGatewayServiceClient(pool.Endpoint(am.c.GatewaySvc)) if err != nil { - return nil, nil, errors.Wrap(err, "oidc: error getting gateway grpc client") + return nil, nil, errors.Wrap(err, "error getting user provider grpc client") } - getGroupsResp, err := gwc.GetUserGroups(ctx, &user.GetUserGroupsRequest{ - UserId: userID, + userRes, err := client.GetUserByClaim(ctx, &user.GetUserByClaimRequest{ + Claim: "username", + Value: sub, }) if err != nil { - return nil, nil, errors.Wrapf(err, "oidc: error getting user groups for '%+v'", userID) + return nil, nil, errors.Wrapf(err, "error getting user by username '%v'", sub) } - if getGroupsResp.Status.Code != rpc.Code_CODE_OK { - return nil, nil, status.NewErrorFromCode(getGroupsResp.Status.Code, "oidc") + if userRes.Status.Code != rpc.Code_CODE_OK { + return nil, nil, status.NewErrorFromCode(userRes.Status.Code, "oidc") } - u := &user.User{ - Id: userID, - Username: claims["preferred_username"].(string), - Groups: getGroupsResp.Groups, - Mail: claims["email"].(string), - MailVerified: claims["email_verified"].(bool), - DisplayName: claims["name"].(string), - UidNumber: claims[am.c.UIDClaim].(int64), - GidNumber: claims[am.c.GIDClaim].(int64), - } + u := userRes.GetUser() var scopes map[string]*authpb.Scope - if userID != nil && (userID.Type == user.UserType_USER_TYPE_LIGHTWEIGHT || userID.Type == user.UserType_USER_TYPE_FEDERATED) { + if u.Id.Type == user.UserType_USER_TYPE_LIGHTWEIGHT { scopes, err = scope.AddLightweightAccountScope(authpb.Role_ROLE_OWNER, nil) if err != nil { return nil, nil, err } + // TODO (gdelmont): we may want to define a template to prettify the user info for lw account? // strip the `guest:` prefix if present in the email claim (appears to come from LDAP at CERN?) u.Mail = strings.Replace(u.Mail, "guest: ", "", 1) // and decorate the display name with the email domain to make it different from a primary account @@ -255,15 +303,6 @@ func (am *mgr) Authenticate(ctx context.Context, _, clientSecret string) (*user. return u, scopes, nil } -func (am *mgr) getUserID(claims map[string]interface{}) (int64, int64) { - uidf, _ := claims[am.c.UIDClaim].(float64) - uid := int64(uidf) - - gidf, _ := claims[am.c.GIDClaim].(float64) - gid := int64(gidf) - return uid, gid -} - func (am *mgr) getOAuthCtx(ctx context.Context) context.Context { // Sometimes for testing we need to skip the TLS check, that's why we need a // custom HTTP client. @@ -277,116 +316,3 @@ func (am *mgr) getOAuthCtx(ctx context.Context) context.Context { ctx = context.WithValue(ctx, oauth2.HTTPClient, customHTTPClient) return ctx } - -// getOIDCProvider returns a singleton OIDC provider. -func (am *mgr) getOIDCProvider(ctx context.Context) (*oidc.Provider, error) { - ctx = am.getOAuthCtx(ctx) - log := appctx.GetLogger(ctx) - - if am.provider != nil { - return am.provider, nil - } - - // Initialize a provider by specifying the issuer URL. - // Once initialized this is a singleton that is reused for further requests. - // The provider is responsible to verify the token sent by the client - // against the security keys oftentimes available in the .well-known endpoint. - provider, err := oidc.NewProvider(ctx, am.c.Issuer) - - if err != nil { - log.Error().Err(err).Msg("oidc: error creating a new oidc provider") - return nil, fmt.Errorf("oidc: error creating a new oidc provider: %+v", err) - } - - am.provider = provider - return am.provider, nil -} - -func (am *mgr) resolveUser(ctx context.Context, claims map[string]interface{}, subject string) error { - var ( - value string - resolve bool - ) - - uid, gid := am.getUserID(claims) - if uid != 0 && gid != 0 { - claims[am.c.UIDClaim] = uid - claims[am.c.GIDClaim] = gid - } - - if len(am.oidcUsersMapping) > 0 { - // map and discover the user's username when a mapping is defined - if claims[am.c.GroupClaim] == nil { - // we are required to perform a user mapping but the group claim is not available - return fmt.Errorf("no \"%s\" claim found in userinfo to map user", am.c.GroupClaim) - } - mappings := make([]string, 0, len(am.oidcUsersMapping)) - for _, m := range am.oidcUsersMapping { - if m.OIDCIssuer == claims["iss"] { - mappings = append(mappings, m.OIDCGroup) - } - } - - intersection := intersect.Simple(claims[am.c.GroupClaim], mappings) - if len(intersection) > 1 { - // multiple mappings are not implemented as we cannot decide which one to choose - return errtypes.PermissionDenied("more than one user mapping entry exists for the given group claims") - } - if len(intersection) == 0 { - return errtypes.PermissionDenied("no user mapping found for the given group claim(s)") - } - for _, m := range intersection { - value = am.oidcUsersMapping[m.(string)].Username - } - resolve = true - } else if uid == 0 || gid == 0 { - value = subject - resolve = true - } - - if !resolve { - return nil - } - - upsc, err := pool.GetGatewayServiceClient(pool.Endpoint(am.c.GatewaySvc)) - if err != nil { - return errors.Wrap(err, "error getting user provider grpc client") - } - getUserByClaimResp, err := upsc.GetUserByClaim(ctx, &user.GetUserByClaimRequest{ - Claim: "username", - Value: value, - }) - if err != nil { - return errors.Wrapf(err, "error getting user by username '%v'", value) - } - if getUserByClaimResp.Status.Code != rpc.Code_CODE_OK { - return status.NewErrorFromCode(getUserByClaimResp.Status.Code, "oidc") - } - - // take the properties of the mapped target user to override the claims - claims["preferred_username"] = getUserByClaimResp.GetUser().Username - claims[am.c.IDClaim] = getUserByClaimResp.GetUser().GetId().OpaqueId - claims["iss"] = getUserByClaimResp.GetUser().GetId().Idp - claims[am.c.UIDClaim] = getUserByClaimResp.GetUser().UidNumber - claims[am.c.GIDClaim] = getUserByClaimResp.GetUser().GidNumber - log := appctx.GetLogger(ctx).Debug().Str("username", value).Interface("claims", claims) - if uid == 0 || gid == 0 { - log.Msgf("resolveUser: claims overridden from '%s'", subject) - } else { - log.Msg("resolveUser: claims overridden from mapped user") - } - return nil -} - -func getUserType(upn string) user.UserType { - var t user.UserType - switch { - case strings.HasPrefix(upn, "guest"): - t = user.UserType_USER_TYPE_LIGHTWEIGHT - case strings.Contains(upn, "@"): - t = user.UserType_USER_TYPE_FEDERATED - default: - t = user.UserType_USER_TYPE_PRIMARY - } - return t -} From 5f545e80e43df6d5d4d276f4961cbcf7698f24cb Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Tue, 2 May 2023 11:45:18 +0200 Subject: [PATCH 2/5] fix init provider with correct issuer --- pkg/auth/manager/oidc/oidc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/auth/manager/oidc/oidc.go b/pkg/auth/manager/oidc/oidc.go index 5f72782af8..d55b6cd420 100644 --- a/pkg/auth/manager/oidc/oidc.go +++ b/pkg/auth/manager/oidc/oidc.go @@ -171,7 +171,7 @@ func (am *mgr) getOIDCProviderForIssuer(ctx context.Context, issuer string) (*oi if am.providers[issuer] == nil { // TODO (gdelmont): the provider should be periodically recreated // as the public key can change over time - provider, err := oidc.NewProvider(ctx, am.c.Issuer) + provider, err := oidc.NewProvider(ctx, issuer) if err != nil { return nil, errors.Wrapf(err, "oidc: error creating a new oidc provider") } From e6d30ff27c2af18763ccbc2b95a0248e9fe13d27 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Tue, 2 May 2023 15:22:23 +0200 Subject: [PATCH 3/5] add changelog --- changelog/unreleased/multiple-issuers-oidc.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 changelog/unreleased/multiple-issuers-oidc.md diff --git a/changelog/unreleased/multiple-issuers-oidc.md b/changelog/unreleased/multiple-issuers-oidc.md new file mode 100644 index 0000000000..0a5a4aedce --- /dev/null +++ b/changelog/unreleased/multiple-issuers-oidc.md @@ -0,0 +1,9 @@ +Enhancement: Support multiple issuer in OIDC auth driver. + +The OIDC auth driver supports now multiple issuers. Users of +external providers are then mapped to a local user by a +mapping files. Only the main issuer (defined in the config +with `issuer`) and the ones defined in the mapping are +allowed for the verification of the OIDC token. + +https://github.com/cs3org/reva/pull/3839 \ No newline at end of file From 13531a34e81eee139fe0e484249f619b7a0f8dd9 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Tue, 2 May 2023 15:56:06 +0200 Subject: [PATCH 4/5] fix lint --- changelog/unreleased/multiple-issuers-oidc.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/unreleased/multiple-issuers-oidc.md b/changelog/unreleased/multiple-issuers-oidc.md index 0a5a4aedce..5c12390bbc 100644 --- a/changelog/unreleased/multiple-issuers-oidc.md +++ b/changelog/unreleased/multiple-issuers-oidc.md @@ -1,4 +1,4 @@ -Enhancement: Support multiple issuer in OIDC auth driver. +Enhancement: Support multiple issuer in OIDC auth driver The OIDC auth driver supports now multiple issuers. Users of external providers are then mapped to a local user by a From f14bb9903be1d67278fa6deda3e3da6e5e8478cf Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Tue, 2 May 2023 16:16:08 +0200 Subject: [PATCH 5/5] trigger ci