Skip to content

Commit

Permalink
Make oidc authenticator audience agnostic
Browse files Browse the repository at this point in the history 
This change removes the audience logic from the oidc authenticator
and collapses it onto the same logic used by other audience unaware
authenticators.

oidc is audience unaware in the sense that it does not know or
understand the API server's audience.  As before, the authenticator
will continue to check that the token audience matches the
configured client ID.

The reasoning for this simplification is:

1. The previous code tries to make the client ID on the oidc token
a valid audience.  But by not returning any audience, the token is
not valid when used via token review on a server that is configured
to honor audiences (the token works against the Kube API because the
audience check is skipped).

2. It is unclear what functionality would be gained by allowing
token review to check the client ID as a valid audience.  It could
serve as a proxy to know that the token was honored by the oidc
authenticator, but that does not seem like a valid use case.

3. It has never been possible to use the client ID as an audience
with token review as it would have always failed the audience
intersection check.  Thus this change is backwards compatible.

It is strange that the oidc authenticator would be considered
audience unaware when oidc tokens have an audience claim, but from
the perspective of the Kube API (and for backwards compatibility),
these tokens are only valid for the API server's audience.

This change seems to be the least magical and most consistent way to
honor backwards compatibility and to allow oidc tokens to be used
via token review when audience support in enabled.

Signed-off-by: Monis Khan <mok@vmware.com>
  • Loading branch information
@enj
enj committed Feb 4, 2020
1 parent 4b29407 commit 9b23f22
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 152 deletions.
3 changes: 1 addition & 2 deletions pkg/kubeapiserver/authenticator/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,6 @@ func (config Config) New() (authenticator.Request, *spec.SecurityDefinitions, er
oidcAuth, err := newAuthenticatorFromOIDCIssuerURL(oidc.Options{
IssuerURL: config.OIDCIssuerURL,
ClientID: config.OIDCClientID,
APIAudiences: config.APIAudiences,
CAFile: config.OIDCCAFile,
UsernameClaim: config.OIDCUsernameClaim,
UsernamePrefix: config.OIDCUsernamePrefix,
Expand All @@ -177,7 +176,7 @@ func (config Config) New() (authenticator.Request, *spec.SecurityDefinitions, er
if err != nil {
return nil, nil, err
}
tokenAuthenticators = append(tokenAuthenticators, oidcAuth)
tokenAuthenticators = append(tokenAuthenticators, authenticator.WrapAudienceAgnosticToken(config.APIAudiences, oidcAuth))
}
if len(config.WebhookTokenAuthnConfigFile) > 0 {
webhookTokenAuth, err := newWebhookTokenAuthenticator(config.WebhookTokenAuthnConfigFile, config.WebhookTokenAuthnVersion, config.WebhookTokenAuthnCacheTTL, config.APIAudiences)
Expand Down
1 change: 0 additions & 1 deletion staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ go_test(
data = glob(["testdata/**"]),
embed = [":go_default_library"],
deps = [
"//staging/src/k8s.io/apiserver/pkg/authentication/authenticator:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/authentication/user:go_default_library",
"//vendor/github.com/coreos/go-oidc:go_default_library",
"//vendor/gopkg.in/square/go-jose.v2:go_default_library",
Expand Down
15 changes: 0 additions & 15 deletions staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,6 @@ type Options struct {
// See: https://openid.net/specs/openid-connect-core-1_0.html#IDToken
ClientID string

// APIAudiences are the audiences that the API server identitifes as. The
// (API audiences unioned with the ClientIDs) should have a non-empty
// intersection with the request's target audience. This preserves the
// behavior of the OIDC authenticator pre-introduction of API audiences.
APIAudiences authenticator.Audiences

// Path to a PEM encoded root certificate of the provider.
CAFile string

Expand Down Expand Up @@ -194,8 +188,6 @@ type Authenticator struct {
groupsClaim string
groupsPrefix string
requiredClaims map[string]string
clientIDs authenticator.Audiences
apiAudiences authenticator.Audiences

// Contains an *oidc.IDTokenVerifier. Do not access directly use the
// idTokenVerifier method.
Expand Down Expand Up @@ -325,8 +317,6 @@ func newAuthenticator(opts Options, initVerifier func(ctx context.Context, a *Au
groupsClaim: opts.GroupsClaim,
groupsPrefix: opts.GroupsPrefix,
requiredClaims: opts.RequiredClaims,
clientIDs: authenticator.Audiences{opts.ClientID},
apiAudiences: opts.APIAudiences,
cancel: cancel,
resolver: resolver,
}
Expand Down Expand Up @@ -542,11 +532,6 @@ func (r *claimResolver) resolve(endpoint endpoint, allClaims claims) error {
}

func (a *Authenticator) AuthenticateToken(ctx context.Context, token string) (*authenticator.Response, bool, error) {
if reqAuds, ok := authenticator.AudiencesFrom(ctx); ok {
if len(reqAuds.Intersect(a.clientIDs)) == 0 && len(reqAuds.Intersect(a.apiAudiences)) == 0 {
return nil, false, nil
}
}
if !hasCorrectIssuer(a.issuerURL, token) {
return nil, false, nil
}
Expand Down
138 changes: 4 additions & 134 deletions staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ import (

oidc "github.com/coreos/go-oidc"
jose "gopkg.in/square/go-jose.v2"

"k8s.io/apiserver/pkg/authentication/authenticator"
"k8s.io/apiserver/pkg/authentication/user"
"k8s.io/klog"
)
Expand Down Expand Up @@ -142,7 +140,6 @@ type claimsTest struct {
wantInitErr bool
claimToResponseMap map[string]string
openIDConfig string
reqAudiences authenticator.Audiences
}

// Replace formats the contents of v into the provided template.
Expand Down Expand Up @@ -299,12 +296,7 @@ func (c *claimsTest) run(t *testing.T) {
t.Fatalf("serialize token: %v", err)
}

ctx := context.Background()
if c.reqAudiences != nil {
ctx = authenticator.WithAudiences(ctx, c.reqAudiences)
}

got, ok, err := a.AuthenticateToken(ctx, token)
got, ok, err := a.AuthenticateToken(context.Background(), token)

if err != nil {
if !c.wantErr {
Expand Down Expand Up @@ -1397,83 +1389,10 @@ func TestToken(t *testing.T) {
},
},
{
name: "good token with api req audience",
options: Options{
IssuerURL: "https://auth.example.com",
ClientID: "my-client",
APIAudiences: authenticator.Audiences{"api"},
UsernameClaim: "username",
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()),
reqAudiences: authenticator.Audiences{"api"},
want: &user.DefaultInfo{
Name: "jane",
},
},
{
name: "good token with multiple api req audience",
options: Options{
IssuerURL: "https://auth.example.com",
ClientID: "my-client",
APIAudiences: authenticator.Audiences{"api", "other"},
UsernameClaim: "username",
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()),
reqAudiences: authenticator.Audiences{"api"},
want: &user.DefaultInfo{
Name: "jane",
},
},
{
name: "good token with client_id req audience",
options: Options{
IssuerURL: "https://auth.example.com",
ClientID: "my-client",
APIAudiences: authenticator.Audiences{"api"},
UsernameClaim: "username",
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()),
reqAudiences: authenticator.Audiences{"my-client"},
want: &user.DefaultInfo{
Name: "jane",
},
},
{
name: "good token with client_id and api req audience",
name: "good token with bad client id",
options: Options{
IssuerURL: "https://auth.example.com",
ClientID: "my-client",
APIAudiences: authenticator.Audiences{"api"},
UsernameClaim: "username",
now: func() time.Time { return now },
},
Expand All @@ -1483,60 +1402,11 @@ func TestToken(t *testing.T) {
},
claims: fmt.Sprintf(`{
"iss": "https://auth.example.com",
"aud": "my-client",
"aud": "my-wrong-client",
"username": "jane",
"exp": %d
}`, valid.Unix()),
reqAudiences: authenticator.Audiences{"my-client", "api"},
want: &user.DefaultInfo{
Name: "jane",
},
},
{
name: "good token with client_id and api req audience",
options: Options{
IssuerURL: "https://auth.example.com",
ClientID: "my-client",
APIAudiences: authenticator.Audiences{"api"},
UsernameClaim: "username",
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()),
reqAudiences: authenticator.Audiences{"my-client", "api"},
want: &user.DefaultInfo{
Name: "jane",
},
},
{
name: "good token with client_id and bad req audience",
options: Options{
IssuerURL: "https://auth.example.com",
ClientID: "my-client",
APIAudiences: authenticator.Audiences{"api"},
UsernameClaim: "username",
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()),
reqAudiences: authenticator.Audiences{"other"},
wantSkip: true,
wantErr: true,
},
}
for _, test := range tests {
Expand Down

0 comments on commit 9b23f22

Please sign in to comment.