djpbessems/ContainerImage.Pinniped
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Check for subject matching with upstream refresh

Browse Source 
Signed-off-by: Margo Crawford <margaretc@vmware.com>
This commit is contained in:
Margo Crawford 2021-11-29 16:44:58 -08:00
parent d438bfbc99
commit ed96b597c7
3 changed files with 208 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
internal/oidc/provider/dynamic_upstream_idp_provider.go
View File

@ -103,6 +103,13 @@ type StoredRefreshAttributes struct {
AdditionalAttributes map[string]string
}
type StoredRefreshAttributes struct {
Username string
Subject string
DN string
AuthTime time.Time
}
type DynamicUpstreamIDPProvider interface {
SetOIDCIdentityProviders(oidcIDPs []UpstreamOIDCIdentityProviderI)
GetOIDCIdentityProviders() []UpstreamOIDCIdentityProviderI

110
internal/upstreamoidc/upstreamoidc.go
View File

@ -7,6 +7,7 @@ package upstreamoidc
import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
@ -129,6 +130,7 @@ func (p *ProviderConfig) ExchangeAuthcodeAndValidateTokens(ctx context.Context,
return p.ValidateToken(ctx, tok, expectedIDTokenNonce)
}
// TODO this is reused between the client and the supervisor... don't change it.
func (p *ProviderConfig) PerformRefresh(ctx context.Context, refreshToken string) (*oauth2.Token, error) {
// Use the provided HTTP client to benefit from its CA, proxy, and other settings.
httpClientContext := coreosoidc.ClientContext(ctx, p.Client)
@ -236,6 +238,57 @@ func (p *ProviderConfig) tryRevokeRefreshToken(
}
}
func (p *ProviderConfig) ValidateRefresh(ctx context.Context, tok *oauth2.Token, storedAttributes provider.StoredRefreshAttributes) error {
idTok, hasIDTok := tok.Extra("id_token").(string)
var validatedClaims = make(map[string]interface{})
if hasIDTok {
coreosConfig := &coreosoidc.Config{ClientID: p.GetClientID()}
coreosClientContext := coreosoidc.ClientContext(ctx, p.Client)
verifier := p.Provider.Verifier(coreosConfig)
validated, err := verifier.Verify(coreosClientContext, idTok)
if err != nil {
return httperr.Wrap(http.StatusBadRequest, "received invalid ID token", err)
}
if err := validated.Claims(&validatedClaims); err != nil {
return httperr.Wrap(http.StatusInternalServerError, "could not unmarshal id token claims", err)
}
maybeLogClaims("claims from ID token", p.Name, validatedClaims)
}
originalUpstreamSubject, err := extractUpstreamSubjectFromDownstream(storedAttributes.Subject)
if err != nil {
return httperr.Wrap(http.StatusInternalServerError, "could not parse stored subject", err)
}
// it's okay to not have an id token. It's okay to have an id token with a subject.
// but if we have an id token without a subject that's a problem.
idTokenSubject, _ := validatedClaims[oidc.IDTokenSubjectClaim].(string)
switch {
case len(idTokenSubject) > 0:
// TODO url escape
if url.QueryEscape(idTokenSubject) != originalUpstreamSubject {
return httperr.Newf(http.StatusInternalServerError, "subject from id token did not match previous stored value. New subject: %s. Old subject: %s", idTokenSubject, originalUpstreamSubject)
}
case len(validatedClaims) == 0:
validatedClaims[oidc.IDTokenSubjectClaim] = originalUpstreamSubject
default:
return httperr.New(http.StatusInternalServerError, "id token did not have a subject")
}
if err := p.maybeFetchUserInfoAndMergeClaims(ctx, tok, validatedClaims); err != nil {
return err
}
return nil
}
func extractUpstreamSubjectFromDownstream(downstreamSubject string) (string, error) {
if !strings.Contains(downstreamSubject, "?sub=") {
return "", errors.New("downstream subject did not contain original upstream subject")
}
return strings.Split(downstreamSubject, "?sub=")[1], nil
}
// ValidateToken will validate the ID token. It will also merge the claims from the userinfo endpoint response,
// if the provider offers the userinfo endpoint.
func (p *ProviderConfig) ValidateToken(ctx context.Context, tok *oauth2.Token, expectedIDTokenNonce nonce.Nonce) (*oidctypes.Token, error) {
@ -264,8 +317,11 @@ func (p *ProviderConfig) ValidateToken(ctx context.Context, tok *oauth2.Token, e
}
maybeLogClaims("claims from ID token", p.Name, validatedClaims)
if err := p.maybeFetchUserInfoAndMergeClaims(ctx, tok, validatedClaims); err != nil {
return nil, httperr.Wrap(http.StatusInternalServerError, "could not fetch user info claims", err)
idTokenSubject, _ := validatedClaims[oidc.IDTokenSubjectClaim].(string)
if len(idTokenSubject) > 0 {
if err := p.maybeFetchUserInfoAndMergeClaims(ctx, tok, validatedClaims); err != nil {
return nil, httperr.Wrap(http.StatusInternalServerError, "could not fetch user info claims", err)
}
}
return &oidctypes.Token{
@ -286,29 +342,22 @@ func (p *ProviderConfig) ValidateToken(ctx context.Context, tok *oauth2.Token, e
}
func (p *ProviderConfig) maybeFetchUserInfoAndMergeClaims(ctx context.Context, tok *oauth2.Token, claims map[string]interface{}) error {
// TODO separate this.
// extract: fetching userinfo
// - validate some userinfo? subject stuff: for refresh subjects must match but also match stored subject
// - extract: merging claims
// - deciding when to do each of those things
idTokenSubject, _ := claims[oidc.IDTokenSubjectClaim].(string)
if len(idTokenSubject) == 0 {
return nil // defer to existing ID token validation
}
providerJSON := &struct {
UserInfoURL string `json:"userinfo_endpoint"`
}{}
if err := p.Provider.Claims(providerJSON); err != nil {
// this should never happen because we should have already parsed these claims at an earlier stage
return httperr.Wrap(http.StatusInternalServerError, "could not unmarshal discovery JSON", err)
userInfo, err := p.fetchUserInfo(ctx, tok)
if err != nil {
return err
}
// implementing the user info endpoint is not required, skip this logic when it is absent
if len(providerJSON.UserInfoURL) == 0 {
if userInfo == nil {
return nil
}
userInfo, err := p.Provider.UserInfo(coreosoidc.ClientContext(ctx, p.Client), oauth2.StaticTokenSource(tok))
if err != nil {
return httperr.Wrap(http.StatusInternalServerError, "could not get user info", err)
}
// TODO if there is no idTokenSubject, defer to checking the stored claims.
// The sub (subject) Claim MUST always be returned in the UserInfo Response.
//
// NOTE: Due to the possibility of token substitution attacks (see Section 16.11), the UserInfo Response is not
@ -317,7 +366,7 @@ func (p *ProviderConfig) maybeFetchUserInfoAndMergeClaims(ctx context.Context, t
// the UserInfo Response values MUST NOT be used.
//
// http://openid.net/specs/openid-connect-core-1_0.html#UserInfoResponse
if len(userInfo.Subject) == 0 || userInfo.Subject != idTokenSubject {
if (len(idTokenSubject) > 0) && (len(userInfo.Subject) == 0 || userInfo.Subject != idTokenSubject) {
return httperr.Newf(http.StatusUnprocessableEntity, "userinfo 'sub' claim (%s) did not match id_token 'sub' claim (%s)", userInfo.Subject, idTokenSubject)
}
@ -331,6 +380,27 @@ func (p *ProviderConfig) maybeFetchUserInfoAndMergeClaims(ctx context.Context, t
return nil
}
func (p *ProviderConfig) fetchUserInfo(ctx context.Context, tok *oauth2.Token) (*coreosoidc.UserInfo, error) {
providerJSON := &struct {
UserInfoURL string `json:"userinfo_endpoint"`
}{}
if err := p.Provider.Claims(providerJSON); err != nil {
// this should never happen because we should have already parsed these claims at an earlier stage
return nil, httperr.Wrap(http.StatusInternalServerError, "could not unmarshal discovery JSON", err)
}
// implementing the user info endpoint is not required, skip this logic when it is absent
if len(providerJSON.UserInfoURL) == 0 {
return nil, nil
}
userInfo, err := p.Provider.UserInfo(coreosoidc.ClientContext(ctx, p.Client), oauth2.StaticTokenSource(tok))
if err != nil {
return nil, httperr.Wrap(http.StatusInternalServerError, "could not get user info", err)
}
return userInfo, nil
}
func maybeLogClaims(msg, name string, claims map[string]interface{}) {
if plog.Enabled(plog.LevelAll) { // log keys and values at all level
data, _ := json.Marshal(claims) // nothing we can do if it fails, but it really never should

111
internal/upstreamoidc/upstreamoidc_test.go
View File

@ -16,6 +16,8 @@ import (
"time"
"unsafe"
"go.pinniped.dev/internal/oidc/provider"
"github.com/coreos/go-oidc/v3/oidc"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/require"
@ -584,6 +586,115 @@ func TestProviderConfig(t *testing.T) {
if tt.wantErr != "" {
require.EqualError(t, err, tt.wantErr)
}
})
}
})
t.Run("ValidateRefresh", func(t *testing.T) {
testTokenWithoutIDToken := &oauth2.Token{
AccessToken: "test-access-token",
// the library sets the original refresh token into the result, even though the server did not return that
RefreshToken: "test-initial-refresh-token",
TokenType: "test-token-type",
Expiry: time.Now().Add(42 * time.Second),
}
tests := []struct {
name string
token *oauth2.Token
storedAttributes provider.StoredRefreshAttributes
userInfo *oidc.UserInfo
rawClaims []byte
userInfoErr error
wantErr string
}{
{
name: "id, access and refresh token returned from refresh",
token: testTokenWithoutIDToken.WithExtra(map[string]interface{}{"id_token": "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJzb21lLXN1YmplY3QiLCJuYW1lIjoiUGlubnkgVGhlU2VhbCIsImlzcyI6InNvbWUtaXNzdWVyIiwiaWF0IjoxNTE2MjM5MDIyfQ.qVMIsmrMLiMW7kJUNO4RpiBnkd6MYKdXXMMb6UWVXL0zXkalb1tP2DO3Yp73ZfWqCNdNEa1qqSJIR9F46cnAp04pZmOvVqTWCmaXaxvklRb_QzdlKi-3QNg7wEJ50a11vhUTJxLYrkH9pJpPILzd8Kx0yp43iEJbBG3nFVZFOSQOh7wCGUF4QkNLRMT9buq8DiypKPmjksVmObkF3AbvQ1IvaA_UPBmFvL-pun-TjUBR37xT7JI64wbczJJclThaqTcK6bQcbiBG4jonckYIIryoL8UUBSbxQGGz7lQK5_77Q4PTXAE3c5hrg-c6RK52YNBm-B5Gx7vAeBavrQJaPQ"}),
storedAttributes: provider.StoredRefreshAttributes{Subject: "some-issuer?sub=some-subject"},
userInfo: forceUserInfoWithClaims("some-subject", `{"foo": "bar"}`),
rawClaims: []byte(`{"userinfo_endpoint": "not-empty"}`),
},
{
name: "id token exists but has no subject",
token: testTokenWithoutIDToken.WithExtra(map[string]interface{}{"id_token": "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJuYW1lIjoiUGlubnkgVGhlU2VhbCIsImlhdCI6MTUxNjIzOTAyMn0.FmkkY7MCnhD98F5Aupq_YdW54pH90O60O9SwjxWHZb6_k6LoJcjZEMyxba1Fg5tlGLOSqH7BffZOfMUZ5YqVeA7ZIZ9fhAiudGdJqKz5nI394xzR-EejGqBI0fTAuxa_0VsLSC5mV0hTctK01kvemPwMLhIUn_HHr0-khugPFZH8zlYxAFmBpe9YxWK5w1JcTWuSMjRn6_b1RDSgxpff-LcmvVz6akcnlb2j8Od7S9JinsSqKjb7zBjQmcVGT6BGdN43tsBJMEOzil6xpvsH_KIToWlyqmvCQFp_udcKWuUNNLdypGW4eMXvPF5GINMDhki0GZDoKLkPnHVk_mZFXw"}),
storedAttributes: provider.StoredRefreshAttributes{Subject: "some-issuer?sub=some-subject"},
userInfo: &oidc.UserInfo{Subject: "some-subject"},
rawClaims: []byte(`{"userinfo_endpoint": "not-empty"}`),
wantErr: "id token did not have a subject",
},
{
name: "id token doesn't exist but userinfo does",
token: testTokenWithoutIDToken,
storedAttributes: provider.StoredRefreshAttributes{Subject: "some-issuer?sub=some-subject"},
userInfo: forceUserInfoWithClaims("some-subject", `{"foo": "bar"}`),
rawClaims: []byte(`{"userinfo_endpoint": "not-empty"}`),
},
{
name: "subject in id token doesn't match stored subject",
token: testTokenWithoutIDToken.WithExtra(map[string]interface{}{"id_token": "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJzb21lLXN1YmplY3QiLCJuYW1lIjoiUGlubnkgVGhlU2VhbCIsImlzcyI6InNvbWUtaXNzdWVyIiwiaWF0IjoxNTE2MjM5MDIyfQ.qVMIsmrMLiMW7kJUNO4RpiBnkd6MYKdXXMMb6UWVXL0zXkalb1tP2DO3Yp73ZfWqCNdNEa1qqSJIR9F46cnAp04pZmOvVqTWCmaXaxvklRb_QzdlKi-3QNg7wEJ50a11vhUTJxLYrkH9pJpPILzd8Kx0yp43iEJbBG3nFVZFOSQOh7wCGUF4QkNLRMT9buq8DiypKPmjksVmObkF3AbvQ1IvaA_UPBmFvL-pun-TjUBR37xT7JI64wbczJJclThaqTcK6bQcbiBG4jonckYIIryoL8UUBSbxQGGz7lQK5_77Q4PTXAE3c5hrg-c6RK52YNBm-B5Gx7vAeBavrQJaPQ"}),
storedAttributes: provider.StoredRefreshAttributes{Subject: "some-issuer?sub=some-other-subject"},
wantErr: "subject from id token did not match previous stored value. New subject: some-subject. Old subject: some-other-subject",
},
{
name: "malformed stored subject",
token: testTokenWithoutIDToken,
storedAttributes: provider.StoredRefreshAttributes{Subject: "wrong-format"},
wantErr: "could not parse stored subject: downstream subject did not contain original upstream subject",
},
{
name: "subject in user info doesn't match stored subject",
token: testTokenWithoutIDToken,
storedAttributes: provider.StoredRefreshAttributes{Subject: "some-issuer?sub=some-other-subject"},
userInfo: forceUserInfoWithClaims("some-subject", `{"foo": "bar"}`),
rawClaims: []byte(`{"userinfo_endpoint": "not-empty"}`),
wantErr: "userinfo 'sub' claim (some-subject) did not match id_token 'sub' claim (some-other-subject)", // TODO is this a good enough error message?...
},
{
name: "subject in id token doesn't match userinfo",
token: testTokenWithoutIDToken.WithExtra(map[string]interface{}{"id_token": "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJzb21lLXN1YmplY3QiLCJuYW1lIjoiUGlubnkgVGhlU2VhbCIsImlhdCI6MTUxNjIzOTAyMn0.P_97A2ONBt-YV0JHM3DDugNhG-codkK8fLU4EOdlGHcVBmWVbbW1g7-U0jjB9gcvUt24TS5z0TxEnKNUG8LiqS-8FbHGjv8DN1gAz4huBxNqmMYwzGsFNKDBcUMpv9DNFT-oIrxSf3pPYFlg7N0YKzbMPRwVaL1iQmzeQDCQieV3mDCWXpxpdLa1Mfj_NRsEe4027xOZYJn14pn83h92rG0VcoV2e39LuuEir-tRtDniY4WFIMPudNJdC0NwoMoRIQuHStCIN5AkbHPuhGQivqoObWX6RgNU18qb6CcCi86WhG45uHO77gfz5TbGsyVhr5LRBEZP5HPqsq5D6853zQ"}),
storedAttributes: provider.StoredRefreshAttributes{Subject: "some-issuer?sub=some-subject"},
userInfo: &oidc.UserInfo{Subject: "some-other-subject"},
rawClaims: []byte(`{"userinfo_endpoint": "not-empty"}`),
wantErr: "userinfo 'sub' claim (some-other-subject) did not match id_token 'sub' claim (some-subject)",
},
{
name: "id token with format that isn't a jwt returned from refresh",
token: testTokenWithoutIDToken.WithExtra(map[string]interface{}{"id_token": "not-jwt-format"}),
userInfo: &oidc.UserInfo{Subject: "test-user-2"},
rawClaims: []byte(`{"userinfo_endpoint": "not-empty"}`),
wantErr: "received invalid ID token: oidc: malformed jwt: square/go-jose: compact JWS format must have three parts",
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
p := ProviderConfig{
Name: "test-name",
UsernameClaim: "test-username-claim",
GroupsClaim: "test-groups-claim",
Config: &oauth2.Config{
ClientID: "test-client-id",
ClientSecret: "test-client-secret",
Endpoint: oauth2.Endpoint{
AuthURL: "https://example.com",
TokenURL: "https://example.com",
AuthStyle: oauth2.AuthStyleInParams,
},
Scopes: []string{"scope1", "scope2"},
},
Provider: &mockProvider{
rawClaims: tt.rawClaims,
userInfo: tt.userInfo,
userInfoErr: tt.userInfoErr,
},
}
err := p.ValidateRefresh(context.Background(), tt.token, tt.storedAttributes)
if tt.wantErr != "" {
require.Error(t, err)
require.Equal(t, err.Error(), tt.wantErr)
} else {
require.NoError(t, err)
}