diff --git a/plugin/pkg/authenticator/token/oidc/oidc.go b/plugin/pkg/authenticator/token/oidc/oidc.go index 1b6e8f31c..a8c95ecb8 100644 --- a/plugin/pkg/authenticator/token/oidc/oidc.go +++ b/plugin/pkg/authenticator/token/oidc/oidc.go @@ -71,10 +71,18 @@ type OIDCOptions struct { // UsernameClaim is the JWT field to use as the user's username. UsernameClaim string + // UsernamePrefix, if specified, causes claims mapping to username to be prefix with + // the provided value. A value "oidc:" would result in usernames like "oidc:john". + UsernamePrefix string + // GroupsClaim, if specified, causes the OIDCAuthenticator to try to populate the user's // groups with an ID Token field. If the GrouppClaim field is present in an ID Token the value // must be a string or list of strings. GroupsClaim string + + // GroupsPrefix, if specified, causes claims mapping to group names to be prefixed with the + // value. A value "oidc:" would result in groups like "oidc:engineering" and "oidc:marketing". + GroupsPrefix string } type OIDCAuthenticator struct { @@ -82,8 +90,10 @@ type OIDCAuthenticator struct { trustedClientID string - usernameClaim string - groupsClaim string + usernameClaim string + usernamePrefix string + groupsClaim string + groupsPrefix string httpClient *http.Client @@ -131,7 +141,9 @@ func New(opts OIDCOptions) (*OIDCAuthenticator, error) { issuerURL: opts.IssuerURL, trustedClientID: opts.ClientID, usernameClaim: opts.UsernameClaim, + usernamePrefix: opts.UsernamePrefix, groupsClaim: opts.GroupsClaim, + groupsPrefix: opts.GroupsPrefix, httpClient: &http.Client{Transport: tr}, } @@ -221,13 +233,17 @@ func (a *OIDCAuthenticator) AuthenticateToken(value string) (user.Info, bool, er if err := client.VerifyJWT(jwt); err != nil { return nil, false, err } - claims, err := jwt.Claims() if err != nil { return nil, false, err } + return a.parseTokenClaims(claims) +} - claim, ok, err := claims.StringClaim(a.usernameClaim) +// parseTokenClaims maps a set of claims to a user. It performs basic validation such as +// ensuring the email is verified. +func (a *OIDCAuthenticator) parseTokenClaims(claims jose.Claims) (user.Info, bool, error) { + username, ok, err := claims.StringClaim(a.usernameClaim) if err != nil { return nil, false, err } @@ -235,9 +251,7 @@ func (a *OIDCAuthenticator) AuthenticateToken(value string) (user.Info, bool, er return nil, false, fmt.Errorf("cannot find %q in JWT claims", a.usernameClaim) } - var username string - switch a.usernameClaim { - case "email": + if a.usernameClaim == "email" { verified, ok := claims["email_verified"] if !ok { return nil, false, errors.New("'email_verified' claim not present") @@ -255,10 +269,10 @@ func (a *OIDCAuthenticator) AuthenticateToken(value string) (user.Info, bool, er if !emailVerified { return nil, false, errors.New("email not verified") } - username = claim - default: - // For all other cases, use issuerURL + claim as the user name. - username = fmt.Sprintf("%s#%s", a.issuerURL, claim) + } + + if a.usernamePrefix != "" { + username = a.usernamePrefix + username } // TODO(yifan): Add UID, also populate the issuer to upper layer. @@ -278,5 +292,12 @@ func (a *OIDCAuthenticator) AuthenticateToken(value string) (user.Info, bool, er info.Groups = groups } } + + if a.groupsPrefix != "" { + for i, group := range info.Groups { + info.Groups[i] = a.groupsPrefix + group + } + } + return info, true, nil } diff --git a/plugin/pkg/authenticator/token/oidc/oidc_test.go b/plugin/pkg/authenticator/token/oidc/oidc_test.go index 1ae3c6d57..f94d91b1a 100644 --- a/plugin/pkg/authenticator/token/oidc/oidc_test.go +++ b/plugin/pkg/authenticator/token/oidc/oidc_test.go @@ -17,7 +17,6 @@ limitations under the License. package oidc import ( - "fmt" "os" "path" "reflect" @@ -213,7 +212,7 @@ func TestOIDCAuthentication(t *testing.T) { "sub", "", generateGoodToken(t, op, srv.URL, "client-foo", "client-foo", "sub", "user-foo", "", nil), - &user.DefaultInfo{Name: fmt.Sprintf("%s#%s", srv.URL, "user-foo")}, + &user.DefaultInfo{Name: "user-foo"}, true, "", }, @@ -308,7 +307,7 @@ func TestOIDCAuthentication(t *testing.T) { } for i, tt := range tests { - client, err := New(OIDCOptions{srv.URL, "client-foo", cert, tt.userClaim, tt.groupsClaim}) + client, err := New(OIDCOptions{srv.URL, "client-foo", cert, tt.userClaim, "", tt.groupsClaim, ""}) if err != nil { t.Errorf("Unexpected error: %v", err) continue @@ -334,3 +333,203 @@ func TestOIDCAuthentication(t *testing.T) { } } } + +func TestParseTokenClaims(t *testing.T) { + tests := []struct { + name string + + // Note this is missing a lot of configuration options because + // parseTokenClaim doesn't handle: + // + // - 'iss' claim matching issuer URL + // - 'exp' claim having not expired + // - 'sub' claim matching a trusted client id + // + // That logic has coverage in other tests. + + issuerURL string + usernameClaim string + usernamePrefix string + groupsClaim string + groupsPrefix string + + claims jose.Claims + + wantUser *user.DefaultInfo + wantErr bool + }{ + { + name: "email username", + issuerURL: "https://foo.com/", + usernameClaim: "email", + claims: jose.Claims{ + "email": "jane.doe@example.com", + "email_verified": true, + }, + wantUser: &user.DefaultInfo{ + Name: "jane.doe@example.com", + }, + }, + { + name: "no email_verified claim", + issuerURL: "https://foo.com/", + usernameClaim: "email", + claims: jose.Claims{ + "email": "jane.doe@example.com", + }, + wantErr: true, + }, + { + name: "email unverified", + issuerURL: "https://foo.com/", + usernameClaim: "email", + claims: jose.Claims{ + "email": "jane.doe@example.com", + "email_verified": false, + }, + wantErr: true, + }, + { + name: "non-email user claim", + issuerURL: "https://foo.com/", + usernameClaim: "name", + claims: jose.Claims{ + "name": "janedoe", + }, + wantUser: &user.DefaultInfo{ + Name: "janedoe", + }, + }, + { + name: "groups claim", + issuerURL: "https://foo.com/", + usernameClaim: "name", + groupsClaim: "groups", + claims: jose.Claims{ + "name": "janedoe", + "groups": []string{"foo", "bar"}, + }, + wantUser: &user.DefaultInfo{ + Name: "janedoe", + Groups: []string{"foo", "bar"}, + }, + }, + { + name: "groups claim string", + issuerURL: "https://foo.com/", + usernameClaim: "name", + groupsClaim: "groups", + claims: jose.Claims{ + "name": "janedoe", + "groups": "foo", + }, + wantUser: &user.DefaultInfo{ + Name: "janedoe", + Groups: []string{"foo"}, + }, + }, + { + name: "username prefix", + issuerURL: "https://foo.com/", + usernameClaim: "name", + groupsClaim: "groups", + usernamePrefix: "oidc:", + claims: jose.Claims{ + "name": "janedoe", + "groups": []string{"foo", "bar"}, + }, + wantUser: &user.DefaultInfo{ + Name: "oidc:janedoe", + Groups: []string{"foo", "bar"}, + }, + }, + { + name: "username prefix with email", + issuerURL: "https://foo.com/", + usernameClaim: "email", + groupsClaim: "groups", + usernamePrefix: "oidc:", + claims: jose.Claims{ + "email": "jane.doe@example.com", + "email_verified": true, + "groups": []string{"foo", "bar"}, + }, + wantUser: &user.DefaultInfo{ + Name: "oidc:jane.doe@example.com", + Groups: []string{"foo", "bar"}, + }, + }, + { + name: "groups prefix", + issuerURL: "https://foo.com/", + usernameClaim: "name", + groupsClaim: "groups", + groupsPrefix: "oidc:", + claims: jose.Claims{ + "name": "janedoe", + "groups": []string{"foo", "bar"}, + }, + wantUser: &user.DefaultInfo{ + Name: "janedoe", + Groups: []string{"oidc:foo", "oidc:bar"}, + }, + }, + { + name: "username and groups prefix", + issuerURL: "https://foo.com/", + usernameClaim: "name", + groupsClaim: "groups", + usernamePrefix: "oidc-user:", + groupsPrefix: "oidc:", + claims: jose.Claims{ + "name": "janedoe", + "groups": []string{"foo", "bar"}, + }, + wantUser: &user.DefaultInfo{ + Name: "oidc-user:janedoe", + Groups: []string{"oidc:foo", "oidc:bar"}, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + o := OIDCAuthenticator{ + issuerURL: test.issuerURL, + usernameClaim: test.usernameClaim, + usernamePrefix: test.usernamePrefix, + groupsClaim: test.groupsClaim, + groupsPrefix: test.groupsPrefix, + } + + u, ok, err := o.parseTokenClaims(test.claims) + if err != nil { + if !test.wantErr { + t.Errorf("failed to authenticate user: %v", err) + } + return + } + if test.wantErr { + t.Fatalf("expected authentication to fail") + } + + if !ok { + // We don't have any cases today when the claims can return + // no error with an unauthenticated signal. + // + // In the future we might. + t.Fatalf("user wasn't authenticated") + } + + got := &user.DefaultInfo{ + Name: u.GetName(), + UID: u.GetUID(), + Groups: u.GetGroups(), + Extra: u.GetExtra(), + } + if !reflect.DeepEqual(got, test.wantUser) { + t.Errorf("wanted user=%#v, got=%#v", test.wantUser, got) + } + }) + } +}