Also run OIDC validations in supervisor authorize endpoint

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
This commit is contained in:
Ryan Richard 2020-11-06 14:44:58 -08:00 committed by Andrew Keesler
parent 4032ed32ae
commit 246471bc91
4 changed files with 45 additions and 8 deletions

View File

@ -7,11 +7,11 @@ package auth
import ( import (
"fmt" "fmt"
"net/http" "net/http"
"time"
"github.com/ory/fosite/handler/openid"
"github.com/ory/fosite" "github.com/ory/fosite"
"github.com/ory/fosite/handler/openid"
"github.com/ory/fosite/token/jwt"
"golang.org/x/oauth2" "golang.org/x/oauth2"
"k8s.io/klog/v2" "k8s.io/klog/v2"
@ -55,7 +55,22 @@ func NewHandler(
return err return err
} }
_, err = oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, &openid.DefaultSession{}) // Grant the openid scope (for now) if they asked for it so that `NewAuthorizeResponse` will perform its OIDC validations.
for _, scope := range authorizeRequester.GetRequestedScopes() {
if scope == "openid" {
authorizeRequester.GrantScope(scope)
}
}
now := time.Now()
_, err = oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, &openid.DefaultSession{
Claims: &jwt.IDTokenClaims{
// Temporary claim values to allow `NewAuthorizeResponse` to perform other OIDC validations.
Subject: "none",
AuthTime: now,
RequestedAt: now,
},
})
if err != nil { if err != nil {
klog.InfoS("authorize response error", "err", err, "details", fosite.ErrorToRFC6749Error(err).ToValues()) klog.InfoS("authorize response error", "err", err, "details", fosite.ErrorToRFC6749Error(err).ToValues())
oauthHelper.WriteAuthorizeError(w, authorizeRequester, err) oauthHelper.WriteAuthorizeError(w, authorizeRequester, err)

View File

@ -51,6 +51,13 @@ func TestAuthorizationEndpoint(t *testing.T) {
} }
`) `)
fositePromptHasNoneAndOtherValueErrorQuery = map[string]string{
"error": "invalid_request",
"error_description": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed\n\nParameter \"prompt\" was set to \"none\", but contains other values as well which is not allowed.",
"error_hint": "Parameter \"prompt\" was set to \"none\", but contains other values as well which is not allowed.",
"state": "some-state-value",
}
fositeMissingCodeChallengeErrorQuery = map[string]string{ fositeMissingCodeChallengeErrorQuery = map[string]string{
"error": "invalid_request", "error": "invalid_request",
"error_description": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed\n\nThis client must include a code_challenge when performing the authorize code flow, but it is missing.", "error_description": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed\n\nThis client must include a code_challenge when performing the authorize code flow, but it is missing.",
@ -118,6 +125,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
Clients: map[string]fosite.Client{oidc.PinnipedCLIOIDCClient().ID: oidc.PinnipedCLIOIDCClient()}, Clients: map[string]fosite.Client{oidc.PinnipedCLIOIDCClient().ID: oidc.PinnipedCLIOIDCClient()},
AuthorizeCodes: map[string]storage.StoreAuthorizeCode{}, AuthorizeCodes: map[string]storage.StoreAuthorizeCode{},
PKCES: map[string]fosite.Requester{}, PKCES: map[string]fosite.Requester{},
IDSessions: map[string]fosite.Requester{},
} }
hmacSecret := []byte("some secret - must have at least 32 bytes") hmacSecret := []byte("some secret - must have at least 32 bytes")
require.GreaterOrEqual(t, len(hmacSecret), 32, "fosite requires that hmac secrets have at least 32 bytes") require.GreaterOrEqual(t, len(hmacSecret), 32, "fosite requires that hmac secrets have at least 32 bytes")
@ -415,6 +423,22 @@ func TestAuthorizationEndpoint(t *testing.T) {
wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeMethodErrorQuery), wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeMethodErrorQuery),
wantBodyString: "", wantBodyString: "",
}, },
{
// This is just one of the many OIDC validations run by fosite. This test is to ensure that we are running
// through that part of the fosite library.
name: "prompt param is not allowed to have none and another legal value at the same time",
issuer: issuer,
idpListGetter: newIDPListGetter(upstreamOIDCIdentityProvider),
generateState: happyStateGenerator,
generatePKCE: happyPKCEGenerator,
generateNonce: happyNonceGenerator,
method: http.MethodGet,
path: modifiedHappyGetRequestPath(map[string]string{"prompt": "none login"}),
wantStatus: http.StatusFound,
wantContentType: "application/json; charset=utf-8",
wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositePromptHasNoneAndOtherValueErrorQuery),
wantBodyString: "",
},
{ {
name: "state does not have enough entropy", name: "state does not have enough entropy",
issuer: issuer, issuer: issuer,
@ -526,9 +550,6 @@ func TestAuthorizationEndpoint(t *testing.T) {
rsp := httptest.NewRecorder() rsp := httptest.NewRecorder()
subject.ServeHTTP(rsp, req) subject.ServeHTTP(rsp, req)
t.Logf("response: %#v", rsp)
t.Logf("body: %q", rsp.Body.String())
require.Equal(t, test.wantStatus, rsp.Code) require.Equal(t, test.wantStatus, rsp.Code)
requireEqualContentType(t, rsp.Header().Get("Content-Type"), test.wantContentType) requireEqualContentType(t, rsp.Header().Get("Content-Type"), test.wantContentType)

View File

@ -44,7 +44,7 @@ func FositeOauth2Helper(oauthStore interface{}, hmacSecretOfLengthAtLeast32 []by
nil, // hasher, defaults to using BCrypt when nil. Used for hashing client secrets. nil, // hasher, defaults to using BCrypt when nil. Used for hashing client secrets.
compose.OAuth2AuthorizeExplicitFactory, compose.OAuth2AuthorizeExplicitFactory,
// compose.OAuth2RefreshTokenGrantFactory, // compose.OAuth2RefreshTokenGrantFactory,
// compose.OpenIDConnectExplicitFactory, compose.OpenIDConnectExplicitFactory,
// compose.OpenIDConnectRefreshFactory, // compose.OpenIDConnectRefreshFactory,
compose.OAuth2PKCEFactory, compose.OAuth2PKCEFactory,
) )

View File

@ -74,6 +74,7 @@ func (m *Manager) SetProviders(oidcProviders ...*provider.OIDCProvider) {
Clients: map[string]fosite.Client{oidc.PinnipedCLIOIDCClient().ID: oidc.PinnipedCLIOIDCClient()}, Clients: map[string]fosite.Client{oidc.PinnipedCLIOIDCClient().ID: oidc.PinnipedCLIOIDCClient()},
AuthorizeCodes: map[string]storage.StoreAuthorizeCode{}, AuthorizeCodes: map[string]storage.StoreAuthorizeCode{},
PKCES: map[string]fosite.Requester{}, PKCES: map[string]fosite.Requester{},
IDSessions: map[string]fosite.Requester{},
} }
oauthHelper := oidc.FositeOauth2Helper(oauthStore, []byte("some secret - must have at least 32 bytes")) // TODO replace this secret oauthHelper := oidc.FositeOauth2Helper(oauthStore, []byte("some secret - must have at least 32 bytes")) // TODO replace this secret