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

Merge pull request #318 from enj/enj/f/user_info_endpoint

Browse Source 
Fetch claims from the user info endpoint if provided
This commit is contained in:
Mo Khan 2021-01-11 14:14:20 -05:00 committed by GitHub
commit 3f08f2e11e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 183 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
internal/upstreamoidc/upstreamoidc.go
View File

@ -9,18 +9,19 @@ import (
"net/http"
"net/url"
"github.com/coreos/go-oidc"
coreosoidc "github.com/coreos/go-oidc"
"golang.org/x/oauth2"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"go.pinniped.dev/internal/httputil/httperr"
"go.pinniped.dev/internal/oidc"
"go.pinniped.dev/internal/oidc/provider"
"go.pinniped.dev/pkg/oidcclient/nonce"
"go.pinniped.dev/pkg/oidcclient/oidctypes"
"go.pinniped.dev/pkg/oidcclient/pkce"
)
func New(config *oauth2.Config, provider *oidc.Provider, client *http.Client) provider.UpstreamOIDCIdentityProviderI {
func New(config *oauth2.Config, provider *coreosoidc.Provider, client *http.Client) provider.UpstreamOIDCIdentityProviderI {
return &ProviderConfig{Config: config, Provider: provider, Client: client}
}
@ -31,7 +32,8 @@ type ProviderConfig struct {
GroupsClaim string
Config *oauth2.Config
Provider interface {
Verifier(*oidc.Config) *oidc.IDTokenVerifier
Verifier(*coreosoidc.Config) *coreosoidc.IDTokenVerifier
UserInfo(ctx context.Context, tokenSource oauth2.TokenSource) (*coreosoidc.UserInfo, error)
}
Client *http.Client
}
@ -63,7 +65,7 @@ func (p *ProviderConfig) GetGroupsClaim() string {
func (p *ProviderConfig) ExchangeAuthcodeAndValidateTokens(ctx context.Context, authcode string, pkceCodeVerifier pkce.Code, expectedIDTokenNonce nonce.Nonce, redirectURI string) (*oidctypes.Token, error) {
tok, err := p.Config.Exchange(
oidc.ClientContext(ctx, p.Client),
coreosoidc.ClientContext(ctx, p.Client),
authcode,
pkceCodeVerifier.Verifier(),
oauth2.SetAuthURLParam("redirect_uri", redirectURI),
@ -80,7 +82,7 @@ func (p *ProviderConfig) ValidateToken(ctx context.Context, tok *oauth2.Token, e
if !hasIDTok {
return nil, httperr.New(http.StatusBadRequest, "received response missing ID token")
}
validated, err := p.Provider.Verifier(&oidc.Config{ClientID: p.GetClientID()}).Verify(oidc.ClientContext(ctx, p.Client), idTok)
validated, err := p.Provider.Verifier(&coreosoidc.Config{ClientID: p.GetClientID()}).Verify(coreosoidc.ClientContext(ctx, p.Client), idTok)
if err != nil {
return nil, httperr.Wrap(http.StatusBadRequest, "received invalid ID token", err)
}
@ -97,7 +99,11 @@ func (p *ProviderConfig) ValidateToken(ctx context.Context, tok *oauth2.Token, e
var validatedClaims map[string]interface{}
if err := validated.Claims(&validatedClaims); err != nil {
return nil, httperr.Wrap(http.StatusInternalServerError, "could not unmarshal claims", err)
return nil, httperr.Wrap(http.StatusInternalServerError, "could not unmarshal id token claims", err)
}
if err := p.fetchUserInfo(ctx, tok, validatedClaims); err != nil {
return nil, httperr.Wrap(http.StatusInternalServerError, "could not fetch user info claims", err)
}
return &oidctypes.Token{
@ -116,3 +122,40 @@ func (p *ProviderConfig) ValidateToken(ctx context.Context, tok *oauth2.Token, e
},
}, nil
}
func (p *ProviderConfig) fetchUserInfo(ctx context.Context, tok *oauth2.Token, claims map[string]interface{}) error {
idTokenSubject, _ := claims[oidc.IDTokenSubjectClaim].(string)
if len(idTokenSubject) == 0 {
return nil // defer to existing ID token validation
}
userInfo, err := p.Provider.UserInfo(coreosoidc.ClientContext(ctx, p.Client), oauth2.StaticTokenSource(tok))
if err != nil {
// the user info endpoint is not required but we do not have a good way to probe if it was provided
const userInfoUnsupported = "oidc: user info endpoint is not supported by this provider"
if err.Error() == userInfoUnsupported {
return nil
}
return httperr.Wrap(http.StatusInternalServerError, "could not get user info", err)
}
// 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
// guaranteed to be about the End-User identified by the sub (subject) element of the ID Token. The sub Claim in
// the UserInfo Response MUST be verified to exactly match the sub Claim in the ID Token; if they do not match,
// 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 {
return httperr.Newf(http.StatusUnprocessableEntity, "userinfo 'sub' claim (%s) did not match id_token 'sub' claim (%s)", userInfo.Subject, idTokenSubject)
}
// merge existing claims with user info claims
if err := userInfo.Claims(&claims); err != nil {
return httperr.Wrap(http.StatusInternalServerError, "could not unmarshal user info claims", err)
}
return nil
}

136
internal/upstreamoidc/upstreamoidc_test.go
View File

@ -6,10 +6,14 @@ package upstreamoidc
import (
"context"
"encoding/json"
"errors"
"fmt"
"net/http"
"net/http/httptest"
"reflect"
"testing"
"time"
"unsafe"
"github.com/coreos/go-oidc"
"github.com/golang/mock/gomock"
@ -52,10 +56,16 @@ func TestProviderConfig(t *testing.T) {
// step crypto keypair key.pub key.priv --kty RSA --no-password --insecure --force && echo '{"nonce": "invalid-nonce"}' | step crypto jwt sign --key key.priv --aud test-client-id --sub test-user --subtle --kid="test-kid" --jti="test-jti"
invalidNonceIDToken = "eyJhbGciOiJSUzI1NiIsImtpZCI6InRlc3Qta2lkIiwidHlwIjoiSldUIn0.eyJhdWQiOiJ0ZXN0LWNsaWVudC1pZCIsImlhdCI6MTYwMjI4Mzc0MSwianRpIjoidGVzdC1qdGkiLCJuYmYiOjE2MDIyODM3NDEsIm5vbmNlIjoiaW52YWxpZC1ub25jZSIsInN1YiI6InRlc3QtdXNlciJ9.PRpq-7j5djaIAkraL-8t8ad9Xm4hM8RW67gyD1VIe0BecWeBFxsTuh3SZVKM9zmcwTgjudsyn8kQOwipDa49IN4PV8FcJA_uUJZi2wiqGJUSTG2K5I89doV_7e0RM1ZYIDDW1G2heKJNW7MbKkX7iEPr7u4MyEzswcPcupbyDA-CQFeL95vgwawoqa6yO94ympTbozqiNfj6Xyw_nHtThQnstjWsJZ9s2mUgppZezZv4HZYTQ7c3e_bzwhWgCzh2CSDJn9_Ra_n_4GcVkpHbsHTP35dFsnf0vactPx6CAu6A1-Apk-BruCktpZ3B4Ercf1UnUOHdGqzQKJtqvB03xQ" //nolint: gosec
// step crypto keypair key.pub key.priv --kty RSA --no-password --insecure --force && echo '{"foo": "bar", "bat": "baz"}' | step crypto jwt sign --key key.priv --aud test-client-id --sub '' --subtle --kid="test-kid" --jti="test-jti"
invalidSubClaim = "eyJhbGciOiJSUzI1NiIsImtpZCI6InRlc3Qta2lkIiwidHlwIjoiSldUIn0.eyJhdWQiOiJ0ZXN0LWNsaWVudC1pZCIsImJhdCI6ImJheiIsImZvbyI6ImJhciIsImlhdCI6MTYxMDIxOTY5MCwianRpIjoidGVzdC1qdGkiLCJuYmYiOjE2MTAyMTk2OTB9.CXgUarh9A8QByF_ddw0W1Cldl_n1qmry2cZh9U0Avi5sl7hb1y22MadDLQslvnx0NKx6EdbwI-El7QxDy0SzwomJomFL7WNd5gGk-Ilq9O_emaHekbpphZ5kxyudsAGUYGxrg1zysv1k5JPhnLnOUMcE7wa0uPLDWnrlAMzqHvnbjI3lakZ8v4-dfAKUIUGi3ycwuAh9BdpydwAsSNOpGBM55-O8911dqVfZKiFNNUeHYE1qlnbhCz7_ykLrljao0nRBbEf9FXGolCdhIaglt0LtaZvll9T9StIbSpcRaBGuRm8toTezmhmHjU-iCc0jGeVKsp8eTyOuJllqDSS-uw"
// step crypto keypair key.pub key.priv --kty RSA --no-password --insecure --force && echo '{"foo": "bar", "bat": "baz"}' | step crypto jwt sign --key key.priv --aud test-client-id --sub test-user --subtle --kid="test-kid" --jti="test-jti"
validIDToken = "eyJhbGciOiJSUzI1NiIsImtpZCI6InRlc3Qta2lkIiwidHlwIjoiSldUIn0.eyJhdWQiOiJ0ZXN0LWNsaWVudC1pZCIsImJhdCI6ImJheiIsImZvbyI6ImJhciIsImlhdCI6MTYwNjc2ODU5MywianRpIjoidGVzdC1qdGkiLCJuYmYiOjE2MDY3Njg1OTMsInN1YiI6InRlc3QtdXNlciJ9.DuqVZ7pGhHqKz7gNr4j2W1s1N8YrSltktH4wW19L4oD1OE2-O72jAnNj5xdjilsa8l7h9ox-5sMF0Tkh3BdRlHQK9dEtNm9tW-JreUnWJ3LCqUs-LZp4NG7edvq2sH_1Bn7O2_NQV51s8Pl04F60CndjQ4NM-6WkqDQTKyY6vJXU7idvM-6TM2HJZK-Na88cOJ9KIK37tL5DhcbsHVF47Dq8uPZ0KbjNQjJLAIi_1GeQBgc6yJhDUwRY4Xu6S0dtTHA6xTI8oSXoamt4bkViEHfJBp97LZQiNz8mku5pVc0aNwP1p4hMHxRHhLXrJjbh-Hx4YFjxtOnIq9t1mHlD4A" //nolint: gosec
)
// if the error string for unsupported user info changes, this will hopefully catch it
_, userInfoNotSupported := (&oidc.Provider{}).UserInfo(context.Background(), nil)
tests := []struct {
name string
authCode string
@ -63,6 +73,10 @@ func TestProviderConfig(t *testing.T) {
returnIDTok string
wantErr string
wantToken oidctypes.Token
userInfo *oidc.UserInfo
userInfoErr error
wantUserInfoCalled bool
}{
{
name: "exchange fails with network error",
@ -119,6 +133,8 @@ func TestProviderConfig(t *testing.T) {
},
},
},
userInfoErr: userInfoNotSupported,
wantUserInfoCalled: true,
},
{
name: "valid",
@ -146,6 +162,89 @@ func TestProviderConfig(t *testing.T) {
},
},
},
userInfoErr: userInfoNotSupported,
wantUserInfoCalled: true,
},
{
name: "user info fetch error",
authCode: "valid",
returnIDTok: validIDToken,
wantErr: "could not fetch user info claims: could not get user info: some network error",
userInfoErr: errors.New("some network error"),
},
{
name: "user info sub error",
authCode: "valid",
returnIDTok: validIDToken,
wantErr: "could not fetch user info claims: userinfo 'sub' claim (test-user-2) did not match id_token 'sub' claim (test-user)",
userInfo: &oidc.UserInfo{Subject: "test-user-2"},
},
{
name: "user info is not json",
authCode: "valid",
returnIDTok: validIDToken,
wantErr: "could not fetch user info claims: could not unmarshal user info claims: invalid character 'i' looking for beginning of value",
// claims is private field so we have to use hacks to set it
userInfo: forceUserInfoWithClaims("test-user", `invalid-json-data`),
},
{
name: "valid with user info",
authCode: "valid",
returnIDTok: validIDToken,
wantToken: oidctypes.Token{
AccessToken: &oidctypes.AccessToken{
Token: "test-access-token",
Expiry: metav1.Time{},
},
RefreshToken: &oidctypes.RefreshToken{
Token: "test-refresh-token",
},
IDToken: &oidctypes.IDToken{
Token: validIDToken,
Expiry: metav1.Time{},
Claims: map[string]interface{}{
"foo": "awesomeness", // overwrite existing claim
"bat": "baz",
"aud": "test-client-id",
"iat": 1.606768593e+09,
"jti": "test-jti",
"nbf": 1.606768593e+09,
"sub": "test-user",
"groups": "fancy-group", // add a new claim
},
},
},
// claims is private field so we have to use hacks to set it
userInfo: forceUserInfoWithClaims("test-user", `{"foo":"awesomeness","groups":"fancy-group"}`),
wantUserInfoCalled: true,
},
{
name: "invalid sub claim",
authCode: "valid",
returnIDTok: invalidSubClaim,
wantToken: oidctypes.Token{
AccessToken: &oidctypes.AccessToken{
Token: "test-access-token",
Expiry: metav1.Time{},
},
RefreshToken: &oidctypes.RefreshToken{
Token: "test-refresh-token",
},
IDToken: &oidctypes.IDToken{
Token: invalidSubClaim,
Expiry: metav1.Time{},
Claims: map[string]interface{}{
"foo": "bar",
"bat": "baz",
"aud": "test-client-id",
"iat": 1.61021969e+09,
"jti": "test-jti",
"nbf": 1.61021969e+09,
// no sub claim
},
},
},
wantUserInfoCalled: false,
},
}
for _, tt := range tests {
@ -188,7 +287,10 @@ func TestProviderConfig(t *testing.T) {
},
Scopes: []string{"scope1", "scope2"},
},
Provider: &mockProvider{},
Provider: &mockProvider{
userInfo: tt.userInfo,
userInfoErr: tt.userInfoErr,
},
}
ctx := context.Background()
@ -201,6 +303,7 @@ func TestProviderConfig(t *testing.T) {
}
require.NoError(t, err)
require.Equal(t, &tt.wantToken, tok)
require.Equal(t, tt.wantUserInfoCalled, p.Provider.(*mockProvider).called)
})
}
}
@ -225,6 +328,35 @@ func mockVerifier() *oidc.IDTokenVerifier {
})
}
type mockProvider struct{}
type mockProvider struct {
called bool
userInfo *oidc.UserInfo
userInfoErr error
}
func (m *mockProvider) Verifier(_ *oidc.Config) *oidc.IDTokenVerifier { return mockVerifier() }
func (m *mockProvider) UserInfo(_ context.Context, tokenSource oauth2.TokenSource) (*oidc.UserInfo, error) {
m.called = true
token, err := tokenSource.Token()
if err != nil {
return nil, err
}
if wantToken := "test-access-token"; wantToken != token.AccessToken {
return nil, fmt.Errorf("want token = %#v, got token = %#v", wantToken, token)
}
return m.userInfo, m.userInfoErr
}
func forceUserInfoWithClaims(subject string, claims string) *oidc.UserInfo {
userInfo := &oidc.UserInfo{Subject: subject}
// this is some dark magic to set a private field
claimsField := reflect.ValueOf(userInfo).Elem().FieldByName("claims")
claimsPointer := (*[]byte)(unsafe.Pointer(claimsField.UnsafeAddr()))
*claimsPointer = []byte(claims)
return userInfo
}