supervisor-oidc: int test passes, but impl needs refactor

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
This commit is contained in:
Andrew Keesler 2020-10-07 11:33:50 -04:00
parent 019f44982c
commit c49ebf4b57
No known key found for this signature in database
GPG Key ID: 27CE0444346F9413
4 changed files with 68 additions and 37 deletions

View File

@ -23,7 +23,6 @@ import (
"go.pinniped.dev/internal/controller/supervisorconfig" "go.pinniped.dev/internal/controller/supervisorconfig"
"go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/controllerlib"
"go.pinniped.dev/internal/downward" "go.pinniped.dev/internal/downward"
"go.pinniped.dev/internal/oidc"
"go.pinniped.dev/internal/oidc/discovery" "go.pinniped.dev/internal/oidc/discovery"
"go.pinniped.dev/internal/oidc/issuerprovider" "go.pinniped.dev/internal/oidc/issuerprovider"
) )
@ -34,10 +33,8 @@ const (
) )
func start(ctx context.Context, l net.Listener, discoveryHandler http.Handler) { func start(ctx context.Context, l net.Listener, discoveryHandler http.Handler) {
mux := http.NewServeMux()
mux.Handle(oidc.WellKnownURLPath, discoveryHandler)
server := http.Server{ server := http.Server{
Handler: mux, Handler: discoveryHandler,
} }
errCh := make(chan error) errCh := make(chan error)

View File

@ -9,21 +9,36 @@ import (
"fmt" "fmt"
"net/http" "net/http"
"net/url" "net/url"
"go.pinniped.dev/internal/oidc"
) )
// Metadata holds all fields (that we care about) from the OpenID Provider Metadata section in the // Metadata holds all fields (that we care about) from the OpenID Provider Metadata section in the
// OpenID Connect Discovery specification: // OpenID Connect Discovery specification:
// https://openid.net/specs/openid-connect-discovery-1_0.html#rfc.section.3. // https://openid.net/specs/openid-connect-discovery-1_0.html#rfc.section.3.
type Metadata struct { type Metadata struct {
// vvvRequiredvvv
Issuer string `json:"issuer"` Issuer string `json:"issuer"`
AuthorizationEndpoint string `json:"authorization_endpoint"` AuthorizationEndpoint string `json:"authorization_endpoint"`
TokenEndpoint string `json:"token_endpoint"` TokenEndpoint string `json:"token_endpoint"`
JWKSURL string `json:"jwks_url"` JWKSURI string `json:"jwks_uri"`
ResponseTypesSupported []string `json:"response_types_supported"` ResponseTypesSupported []string `json:"response_types_supported"`
SubjectTypesSupported []string `json:"subject_types_supported"` SubjectTypesSupported []string `json:"subject_types_supported"`
IDTokenSigningAlgValuesSupported []string `json:"id_token_signing_alg_values_supported"` IDTokenSigningAlgValuesSupported []string `json:"id_token_signing_alg_values_supported"`
// ^^^Required^^^
// vvvOptionalvvv
TokenEndpointAuthMethodsSupported []string `json:"token_endpoint_auth_methods_supported"`
TokenEndpointAuthSigningAlgoValuesSupported []string `json:"token_endpoint_auth_signing_alg_values_supported"`
ScopesSupported []string `json:"scopes_supported"`
ClaimsSupported []string `json:"claims_supported"`
// ^^^Optional^^^
} }
// IssuerGetter holds onto an issuer which can be retrieved via its GetIssuer function. If there is // IssuerGetter holds onto an issuer which can be retrieved via its GetIssuer function. If there is
@ -41,7 +56,7 @@ func New(ig IssuerGetter) http.Handler {
w.Header().Set("Content-Type", "application/json") w.Header().Set("Content-Type", "application/json")
issuer := ig.GetIssuer() issuer := ig.GetIssuer()
if issuer == nil { if issuer == nil || r.URL.Path != issuer.Path+oidc.WellKnownURLPath {
http.Error(w, `{"error": "OIDC discovery not available (unknown issuer)"}`, http.StatusNotFound) http.Error(w, `{"error": "OIDC discovery not available (unknown issuer)"}`, http.StatusNotFound)
return return
} }
@ -53,13 +68,17 @@ func New(ig IssuerGetter) http.Handler {
issuerURL := issuer.String() issuerURL := issuer.String()
oidcConfig := Metadata{ oidcConfig := Metadata{
Issuer: issuerURL, Issuer: issuerURL,
AuthorizationEndpoint: fmt.Sprintf("%s/oauth2/v0/auth", issuerURL), AuthorizationEndpoint: fmt.Sprintf("%s/oauth2/v0/auth", issuerURL),
TokenEndpoint: fmt.Sprintf("%s/oauth2/v0/token", issuerURL), TokenEndpoint: fmt.Sprintf("%s/oauth2/v0/token", issuerURL),
JWKSURL: fmt.Sprintf("%s/oauth2/v0/keys", issuerURL), JWKSURI: fmt.Sprintf("%s/jwks.json", issuerURL),
ResponseTypesSupported: []string{}, ResponseTypesSupported: []string{"code"},
SubjectTypesSupported: []string{}, SubjectTypesSupported: []string{"public"},
IDTokenSigningAlgValuesSupported: []string{}, IDTokenSigningAlgValuesSupported: []string{"RS256"},
TokenEndpointAuthMethodsSupported: []string{"client_secret_basic"},
TokenEndpointAuthSigningAlgoValuesSupported: []string{"RS256"},
ScopesSupported: []string{"openid", "offline"},
ClaimsSupported: []string{"groups"},
} }
if err := json.NewEncoder(w).Encode(&oidcConfig); err != nil { if err := json.NewEncoder(w).Encode(&oidcConfig); err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError) http.Error(w, err.Error(), http.StatusInternalServerError)

View File

@ -12,6 +12,7 @@ import (
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"go.pinniped.dev/internal/oidc"
"go.pinniped.dev/internal/oidc/issuerprovider" "go.pinniped.dev/internal/oidc/issuerprovider"
) )
@ -21,6 +22,7 @@ func TestDiscovery(t *testing.T) {
issuer *url.URL issuer *url.URL
method string method string
path string
wantStatus int wantStatus int
wantContentType string wantContentType string
@ -29,47 +31,58 @@ func TestDiscovery(t *testing.T) {
{ {
name: "nil issuer", name: "nil issuer",
method: http.MethodGet, method: http.MethodGet,
path: oidc.WellKnownURLPath,
wantStatus: http.StatusNotFound, wantStatus: http.StatusNotFound,
wantBody: map[string]string{ wantBody: map[string]string{
"error": "OIDC discovery not available (unknown issuer)", "error": "OIDC discovery not available (unknown issuer)",
}, },
}, },
{ {
name: "issuer without path", name: "root path mismatch",
issuer: must(url.Parse("https://some-issuer.com")), issuer: must(url.Parse("https://some-issuer.com/some/path")),
method: http.MethodGet, method: http.MethodGet,
wantStatus: http.StatusOK, path: "/some/other/path" + oidc.WellKnownURLPath,
wantContentType: "application/json", wantStatus: http.StatusNotFound,
wantBody: &Metadata{ wantBody: map[string]string{
Issuer: "https://some-issuer.com", "error": "OIDC discovery not available (unknown issuer)",
AuthorizationEndpoint: "https://some-issuer.com/oauth2/v0/auth",
TokenEndpoint: "https://some-issuer.com/oauth2/v0/token",
JWKSURL: "https://some-issuer.com/oauth2/v0/keys",
ResponseTypesSupported: []string{},
SubjectTypesSupported: []string{},
IDTokenSigningAlgValuesSupported: []string{},
}, },
}, },
{ {
name: "issuer with path", name: "well-known path mismatch",
issuer: must(url.Parse("https://some-issuer.com/some/path")),
method: http.MethodGet,
path: "/some/path/that/is/not/the/well-known/path",
wantStatus: http.StatusNotFound,
wantBody: map[string]string{
"error": "OIDC discovery not available (unknown issuer)",
},
},
{
name: "issuer path matches",
issuer: must(url.Parse("https://some-issuer.com/some/path")), issuer: must(url.Parse("https://some-issuer.com/some/path")),
method: http.MethodGet, method: http.MethodGet,
path: "/some/path" + oidc.WellKnownURLPath,
wantStatus: http.StatusOK, wantStatus: http.StatusOK,
wantContentType: "application/json", wantContentType: "application/json",
wantBody: &Metadata{ wantBody: &Metadata{
Issuer: "https://some-issuer.com/some/path", Issuer: "https://some-issuer.com/some/path",
AuthorizationEndpoint: "https://some-issuer.com/some/path/oauth2/v0/auth", AuthorizationEndpoint: "https://some-issuer.com/some/path/oauth2/v0/auth",
TokenEndpoint: "https://some-issuer.com/some/path/oauth2/v0/token", TokenEndpoint: "https://some-issuer.com/some/path/oauth2/v0/token",
JWKSURL: "https://some-issuer.com/some/path/oauth2/v0/keys", JWKSURI: "https://some-issuer.com/some/path/jwks.json",
ResponseTypesSupported: []string{}, ResponseTypesSupported: []string{"code"},
SubjectTypesSupported: []string{}, SubjectTypesSupported: []string{"public"},
IDTokenSigningAlgValuesSupported: []string{}, IDTokenSigningAlgValuesSupported: []string{"RS256"},
TokenEndpointAuthMethodsSupported: []string{"client_secret_basic"},
TokenEndpointAuthSigningAlgoValuesSupported: []string{"RS256"},
ScopesSupported: []string{"openid", "offline"},
ClaimsSupported: []string{"groups"},
}, },
}, },
{ {
name: "bad method", name: "bad method",
issuer: must(url.Parse("https://some-issuer.com")), issuer: must(url.Parse("https://some-issuer.com")),
method: http.MethodPost, method: http.MethodPost,
path: oidc.WellKnownURLPath,
wantStatus: http.StatusMethodNotAllowed, wantStatus: http.StatusMethodNotAllowed,
wantBody: map[string]string{ wantBody: map[string]string{
"error": "Method not allowed (try GET)", "error": "Method not allowed (try GET)",
@ -83,7 +96,7 @@ func TestDiscovery(t *testing.T) {
p.SetIssuer(test.issuer) p.SetIssuer(test.issuer)
handler := New(p) handler := New(p)
req := httptest.NewRequest(test.method, "/this/path/shouldnt/matter", nil) req := httptest.NewRequest(test.method, test.path, nil)
rsp := httptest.NewRecorder() rsp := httptest.NewRecorder()
handler.ServeHTTP(rsp, req) handler.ServeHTTP(rsp, req)

View File

@ -118,14 +118,16 @@ func TestSupervisorOIDCDiscovery(t *testing.T) {
// Check that the response matches our expectations. // Check that the response matches our expectations.
expectedResultTemplate := here.Doc(`{ expectedResultTemplate := here.Doc(`{
"issuer": "%s", "issuer": "%s",
"authorization_endpoint": "%s/connect/authorize", "authorization_endpoint": "%s/oauth2/v0/auth",
"token_endpoint": "%s/connect/token", "token_endpoint": "%s/oauth2/v0/token",
"token_endpoint_auth_methods_supported": ["client_secret_basic"], "token_endpoint_auth_methods_supported": ["client_secret_basic"],
"token_endpoint_auth_signing_alg_values_supported": ["RS256"], "token_endpoint_auth_signing_alg_values_supported": ["RS256"],
"jwks_uri": "%s/jwks.json", "jwks_uri": "%s/jwks.json",
"scopes_supported": ["openid", "offline"], "scopes_supported": ["openid", "offline"],
"response_types_supported": ["code"], "response_types_supported": ["code"],
"claims_supported": ["groups"], "claims_supported": ["groups"],
"subject_types_supported": ["public"],
"id_token_signing_alg_values_supported": ["RS256"]
}`) }`)
expectedJSON := fmt.Sprintf(expectedResultTemplate, issuer, issuer, issuer, issuer) expectedJSON := fmt.Sprintf(expectedResultTemplate, issuer, issuer, issuer, issuer)