Merge pull request #62136 from rithujohn191/oidc-hd-claim

Automatic merge from submit-queue (batch tested with PRs 61241, 62136). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

OIDC required claims

**What this PR does / why we need it**: 
Currently there is no mechanism for a user to specify claims in the OIDC authentication process that are required to be present in the ID Token with an expected value. This PR adds the required claims support for the OIDC authentication. It allows users to pass in a `--oidc-required-claims` flag, and key=value pairs in the API config, which will ensure that the specified `required claims` are checked against the ID Token claims.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #61276

**Special notes for your reviewer**:
Ran the following commands to update godep files:

```
./hack/godep-restore.sh -v
./hack/godep-save.sh
./hack/update-staging-godeps.sh
./hack/update-bazel.sh
```
Since we don't officially support go 1.10, kept go version to 1.9

**Release note**:

```release-note
kube-apiserver: oidc authentication now supports requiring specific claims with `--oidc-required-claim=<claim>=<value>`
```
/sig auth
/kind feature
/assign @ericchiang
This commit is contained in:
Kubernetes Submit Queue 2018-04-11 03:25:11 -07:00 committed by GitHub
commit d1b38b21ef
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 175 additions and 14 deletions

View File

@ -59,6 +59,7 @@ type AuthenticatorConfig struct {
OIDCGroupsClaim string
OIDCGroupsPrefix string
OIDCSigningAlgs []string
OIDCRequiredClaims map[string]string
ServiceAccountKeyFiles []string
ServiceAccountLookup bool
ServiceAccountIssuer string
@ -159,7 +160,7 @@ func (config AuthenticatorConfig) New() (authenticator.Request, *spec.SecurityDe
// simply returns an error, the OpenID Connect plugin may query the provider to
// update the keys, causing performance hits.
if len(config.OIDCIssuerURL) > 0 && len(config.OIDCClientID) > 0 {
oidcAuth, err := newAuthenticatorFromOIDCIssuerURL(config.OIDCIssuerURL, config.OIDCClientID, config.OIDCCAFile, config.OIDCUsernameClaim, config.OIDCUsernamePrefix, config.OIDCGroupsClaim, config.OIDCGroupsPrefix, config.OIDCSigningAlgs)
oidcAuth, err := newAuthenticatorFromOIDCIssuerURL(config.OIDCIssuerURL, config.OIDCClientID, config.OIDCCAFile, config.OIDCUsernameClaim, config.OIDCUsernamePrefix, config.OIDCGroupsClaim, config.OIDCGroupsPrefix, config.OIDCSigningAlgs, config.OIDCRequiredClaims)
if err != nil {
return nil, nil, err
}
@ -241,7 +242,7 @@ func newAuthenticatorFromTokenFile(tokenAuthFile string) (authenticator.Token, e
}
// newAuthenticatorFromOIDCIssuerURL returns an authenticator.Token or an error.
func newAuthenticatorFromOIDCIssuerURL(issuerURL, clientID, caFile, usernameClaim, usernamePrefix, groupsClaim, groupsPrefix string, signingAlgs []string) (authenticator.Token, error) {
func newAuthenticatorFromOIDCIssuerURL(issuerURL, clientID, caFile, usernameClaim, usernamePrefix, groupsClaim, groupsPrefix string, signingAlgs []string, requiredClaims map[string]string) (authenticator.Token, error) {
const noUsernamePrefix = "-"
if usernamePrefix == "" && usernameClaim != "email" {
@ -266,6 +267,7 @@ func newAuthenticatorFromOIDCIssuerURL(issuerURL, clientID, caFile, usernameClai
GroupsClaim: groupsClaim,
GroupsPrefix: groupsPrefix,
SupportedSigningAlgs: signingAlgs,
RequiredClaims: requiredClaims,
})
if err != nil {
return nil, err

View File

@ -68,6 +68,7 @@ go_library(
"//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/validating:go_default_library",
"//vendor/k8s.io/apiserver/pkg/server:go_default_library",
"//vendor/k8s.io/apiserver/pkg/server/options:go_default_library",
"//vendor/k8s.io/apiserver/pkg/util/flag:go_default_library",
"//vendor/k8s.io/client-go/informers:go_default_library",
"//vendor/k8s.io/client-go/rest:go_default_library",
],

View File

@ -28,6 +28,7 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
genericapiserver "k8s.io/apiserver/pkg/server"
genericoptions "k8s.io/apiserver/pkg/server/options"
"k8s.io/apiserver/pkg/util/flag"
"k8s.io/kubernetes/pkg/kubeapiserver/authenticator"
authzmodes "k8s.io/kubernetes/pkg/kubeapiserver/authorizer/modes"
)
@ -64,6 +65,7 @@ type OIDCAuthenticationOptions struct {
GroupsClaim string
GroupsPrefix string
SigningAlgs []string
RequiredClaims map[string]string
}
type PasswordFileAuthenticationOptions struct {
@ -223,6 +225,11 @@ func (s *BuiltInAuthenticationOptions) AddFlags(fs *pflag.FlagSet) {
"Comma-separated list of allowed JOSE asymmetric signing algorithms. JWTs with a "+
"'alg' header value not in this list will be rejected. "+
"Values are defined by RFC 7518 https://tools.ietf.org/html/rfc7518#section-3.1.")
fs.Var(flag.NewMapStringStringNoSplit(&s.OIDC.RequiredClaims), "oidc-required-claim", ""+
"A key=value pair that describes a required claim in the ID Token. "+
"If set, the claim is verified to be present in the ID Token with a matching value. "+
"Repeat this flag to specify multiple claims.")
}
if s.PasswordFile != nil {
@ -298,6 +305,7 @@ func (s *BuiltInAuthenticationOptions) ToAuthenticationConfig() authenticator.Au
ret.OIDCUsernameClaim = s.OIDC.UsernameClaim
ret.OIDCUsernamePrefix = s.OIDC.UsernamePrefix
ret.OIDCSigningAlgs = s.OIDC.SigningAlgs
ret.OIDCRequiredClaims = s.OIDC.RequiredClaims
}
if s.PasswordFile != nil {

View File

@ -23,11 +23,14 @@ import (
)
// MapStringString can be set from the command line with the format `--flag "string=string"`.
// Multiple comma-separated key-value pairs in a single invocation are supported. For example: `--flag "a=foo,b=bar"`.
// Multiple flag invocations are supported. For example: `--flag "a=foo" --flag "b=bar"`.
// Multiple flag invocations are supported. For example: `--flag "a=foo" --flag "b=bar"`. If this is desired
// to be the only type invocation `NoSplit` should be set to true.
// Multiple comma-separated key-value pairs in a single invocation are supported if `NoSplit`
// is set to false. For example: `--flag "a=foo,b=bar"`.
type MapStringString struct {
Map *map[string]string
initialized bool
NoSplit bool
}
// NewMapStringString takes a pointer to a map[string]string and returns the
@ -36,6 +39,15 @@ func NewMapStringString(m *map[string]string) *MapStringString {
return &MapStringString{Map: m}
}
// NewMapStringString takes a pointer to a map[string]string and sets `NoSplit`
// value to `true` and returns the MapStringString flag parsing shim for that map
func NewMapStringStringNoSplit(m *map[string]string) *MapStringString {
return &MapStringString{
Map: m,
NoSplit: true,
}
}
// String implements github.com/spf13/pflag.Value
func (m *MapStringString) String() string {
pairs := []string{}
@ -56,19 +68,34 @@ func (m *MapStringString) Set(value string) error {
*m.Map = make(map[string]string)
m.initialized = true
}
for _, s := range strings.Split(value, ",") {
if len(s) == 0 {
continue
// account for comma-separated key-value pairs in a single invocation
if !m.NoSplit {
for _, s := range strings.Split(value, ",") {
if len(s) == 0 {
continue
}
arr := strings.SplitN(s, "=", 2)
if len(arr) != 2 {
return fmt.Errorf("malformed pair, expect string=string")
}
k := strings.TrimSpace(arr[0])
v := strings.TrimSpace(arr[1])
(*m.Map)[k] = v
}
arr := strings.SplitN(s, "=", 2)
if len(arr) != 2 {
return fmt.Errorf("malformed pair, expect string=string")
}
k := strings.TrimSpace(arr[0])
v := strings.TrimSpace(arr[1])
(*m.Map)[k] = v
return nil
}
// account for only one key-value pair in a single invocation
arr := strings.SplitN(value, "=", 2)
if len(arr) != 2 {
return fmt.Errorf("malformed pair, expect string=string")
}
k := strings.TrimSpace(arr[0])
v := strings.TrimSpace(arr[1])
(*m.Map)[k] = v
return nil
}
// Type implements github.com/spf13/pflag.Value

View File

@ -58,6 +58,7 @@ func TestSetMapStringString(t *testing.T) {
&MapStringString{
initialized: true,
Map: &map[string]string{},
NoSplit: false,
}, ""},
// make sure we still allocate for "initialized" maps where Map was initially set to a nil map
{"allocates map if currently nil", []string{""},
@ -65,6 +66,7 @@ func TestSetMapStringString(t *testing.T) {
&MapStringString{
initialized: true,
Map: &map[string]string{},
NoSplit: false,
}, ""},
// for most cases, we just reuse nilMap, which should be allocated by Set, and is reset before each test case
{"empty", []string{""},
@ -72,36 +74,56 @@ func TestSetMapStringString(t *testing.T) {
&MapStringString{
initialized: true,
Map: &map[string]string{},
NoSplit: false,
}, ""},
{"one key", []string{"one=foo"},
NewMapStringString(&nilMap),
&MapStringString{
initialized: true,
Map: &map[string]string{"one": "foo"},
NoSplit: false,
}, ""},
{"two keys", []string{"one=foo,two=bar"},
NewMapStringString(&nilMap),
&MapStringString{
initialized: true,
Map: &map[string]string{"one": "foo", "two": "bar"},
NoSplit: false,
}, ""},
{"one key, multi flag invocation only", []string{"one=foo,bar"},
NewMapStringStringNoSplit(&nilMap),
&MapStringString{
initialized: true,
Map: &map[string]string{"one": "foo,bar"},
NoSplit: true,
}, ""},
{"two keys, multi flag invocation only", []string{"one=foo,bar", "two=foo,bar"},
NewMapStringStringNoSplit(&nilMap),
&MapStringString{
initialized: true,
Map: &map[string]string{"one": "foo,bar", "two": "foo,bar"},
NoSplit: true,
}, ""},
{"two keys, multiple Set invocations", []string{"one=foo", "two=bar"},
NewMapStringString(&nilMap),
&MapStringString{
initialized: true,
Map: &map[string]string{"one": "foo", "two": "bar"},
NoSplit: false,
}, ""},
{"two keys with space", []string{"one=foo, two=bar"},
NewMapStringString(&nilMap),
&MapStringString{
initialized: true,
Map: &map[string]string{"one": "foo", "two": "bar"},
NoSplit: false,
}, ""},
{"empty key", []string{"=foo"},
NewMapStringString(&nilMap),
&MapStringString{
initialized: true,
Map: &map[string]string{"": "foo"},
NoSplit: false,
}, ""},
{"missing value", []string{"one"},
NewMapStringString(&nilMap),

View File

@ -98,6 +98,10 @@ type Options struct {
// https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation
SupportedSigningAlgs []string
// RequiredClaims, if specified, causes the OIDCAuthenticator to verify that all the
// required claims key value pairs are present in the ID Token.
RequiredClaims map[string]string
// now is used for testing. It defaults to time.Now.
now func() time.Time
}
@ -109,6 +113,7 @@ type Authenticator struct {
usernamePrefix string
groupsClaim string
groupsPrefix string
requiredClaims map[string]string
// Contains an *oidc.IDTokenVerifier. Do not access directly use the
// idTokenVerifier method.
@ -218,6 +223,7 @@ func newAuthenticator(opts Options, initVerifier func(ctx context.Context, a *Au
usernamePrefix: opts.UsernamePrefix,
groupsClaim: opts.GroupsClaim,
groupsPrefix: opts.GroupsPrefix,
requiredClaims: opts.RequiredClaims,
cancel: cancel,
}
@ -323,6 +329,23 @@ func (a *Authenticator) AuthenticateToken(token string) (user.Info, bool, error)
info.Groups[i] = a.groupsPrefix + group
}
}
// check to ensure all required claims are present in the ID token and have matching values.
for claim, value := range a.requiredClaims {
if !c.hasClaim(claim) {
return nil, false, fmt.Errorf("oidc: required claim %s not present in ID token", claim)
}
// NOTE: Only string values are supported as valid required claim values.
var claimValue string
if err := c.unmarshalClaim(claim, &claimValue); err != nil {
return nil, false, fmt.Errorf("oidc: parse claim %s: %v", claim, err)
}
if claimValue != value {
return nil, false, fmt.Errorf("oidc: required claim %s value does not match. Got = %s, want = %s", claim, claimValue, value)
}
}
return info, true, nil
}

View File

@ -428,6 +428,84 @@ func TestToken(t *testing.T) {
}`, valid.Unix()),
wantErr: true,
},
{
name: "required-claim",
options: Options{
IssuerURL: "https://auth.example.com",
ClientID: "my-client",
UsernameClaim: "username",
GroupsClaim: "groups",
RequiredClaims: map[string]string{
"hd": "example.com",
"sub": "test",
},
now: func() time.Time { return now },
},
signingKey: loadRSAPrivKey(t, "testdata/rsa_1.pem", jose.RS256),
pubKeys: []*jose.JSONWebKey{
loadRSAKey(t, "testdata/rsa_1.pem", jose.RS256),
},
claims: fmt.Sprintf(`{
"iss": "https://auth.example.com",
"aud": "my-client",
"username": "jane",
"hd": "example.com",
"sub": "test",
"exp": %d
}`, valid.Unix()),
want: &user.DefaultInfo{
Name: "jane",
},
},
{
name: "no-required-claim",
options: Options{
IssuerURL: "https://auth.example.com",
ClientID: "my-client",
UsernameClaim: "username",
GroupsClaim: "groups",
RequiredClaims: map[string]string{
"hd": "example.com",
},
now: func() time.Time { return now },
},
signingKey: loadRSAPrivKey(t, "testdata/rsa_1.pem", jose.RS256),
pubKeys: []*jose.JSONWebKey{
loadRSAKey(t, "testdata/rsa_1.pem", jose.RS256),
},
claims: fmt.Sprintf(`{
"iss": "https://auth.example.com",
"aud": "my-client",
"username": "jane",
"exp": %d
}`, valid.Unix()),
wantErr: true,
},
{
name: "invalid-required-claim",
options: Options{
IssuerURL: "https://auth.example.com",
ClientID: "my-client",
UsernameClaim: "username",
GroupsClaim: "groups",
RequiredClaims: map[string]string{
"hd": "example.com",
},
now: func() time.Time { return now },
},
signingKey: loadRSAPrivKey(t, "testdata/rsa_1.pem", jose.RS256),
pubKeys: []*jose.JSONWebKey{
loadRSAKey(t, "testdata/rsa_1.pem", jose.RS256),
},
claims: fmt.Sprintf(`{
"iss": "https://auth.example.com",
"aud": "my-client",
"username": "jane",
"hd": "example.org",
"exp": %d
}`, valid.Unix()),
wantErr: true,
},
{
name: "invalid-signature",
options: Options{