Add an API version to the Supervisor IDP discovery endpoint

Also rename one of the new functional opts in login.go to more
accurately reflect the intention of the opt.
This commit is contained in:
Ryan Richard 2021-05-13 10:05:56 -07:00
parent 29ca8acab4
commit 67dca688d7
10 changed files with 147 additions and 73 deletions

View File

@ -95,11 +95,15 @@ type getKubeconfigParams struct {
credentialCachePathSet bool
}
type supervisorOIDCDiscoveryResponse struct {
type supervisorOIDCDiscoveryResponseWithV1Alpha1 struct {
SupervisorDiscovery SupervisorDiscoveryResponseV1Alpha1 `json:"discovery.supervisor.pinniped.dev/v1alpha1"`
}
type SupervisorDiscoveryResponseV1Alpha1 struct {
PinnipedIDPsEndpoint string `json:"pinniped_identity_providers_endpoint"`
}
type supervisorIDPsDiscoveryResponse struct {
type supervisorIDPsDiscoveryResponseV1Alpha1 struct {
PinnipedIDPs []pinnipedIDPResponse `json:"pinniped_identity_providers"`
}
@ -800,13 +804,13 @@ func discoverIDPsDiscoveryEndpointURL(ctx context.Context, issuer string, httpCl
return "", fmt.Errorf("unable to fetch OIDC discovery data from issuer: could not read response body: %w", err)
}
var body supervisorOIDCDiscoveryResponse
var body supervisorOIDCDiscoveryResponseWithV1Alpha1
err = json.Unmarshal(rawBody, &body)
if err != nil {
return "", fmt.Errorf("unable to fetch OIDC discovery data from issuer: could not parse response JSON: %w", err)
}
return body.PinnipedIDPsEndpoint, nil
return body.SupervisorDiscovery.PinnipedIDPsEndpoint, nil
}
func discoverAllAvailableSupervisorUpstreamIDPs(ctx context.Context, pinnipedIDPsEndpoint string, httpClient *http.Client) ([]pinnipedIDPResponse, error) {
@ -831,7 +835,7 @@ func discoverAllAvailableSupervisorUpstreamIDPs(ctx context.Context, pinnipedIDP
return nil, fmt.Errorf("unable to fetch IDP discovery data from issuer: could not read response body: %w", err)
}
var body supervisorIDPsDiscoveryResponse
var body supervisorIDPsDiscoveryResponseV1Alpha1
err = json.Unmarshal(rawBody, &body)
if err != nil {
return nil, fmt.Errorf("unable to fetch IDP discovery data from issuer: could not parse response JSON: %w", err)

View File

@ -74,6 +74,16 @@ func TestGetKubeconfig(t *testing.T) {
}
}
happyOIDCDIscoveryResponse := func(issuerURL string) string {
return here.Docf(`{
"other-key": "other-value",
"discovery.supervisor.pinniped.dev/v1alpha1": {
"pinniped_identity_providers_endpoint": "%s/v1alpha1/pinniped_identity_providers"
},
"another-key": "another-value"
}`, issuerURL)
}
tests := []struct {
name string
args func(string, string) []string
@ -737,9 +747,7 @@ func TestGetKubeconfig(t *testing.T) {
jwtAuthenticator(issuerCABundle, issuerURL),
}
},
oidcDiscoveryResponse: func(issuerURL string) string {
return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL)
},
oidcDiscoveryResponse: happyOIDCDIscoveryResponse,
idpsDiscoveryStatusCode: http.StatusBadRequest,
wantLogs: func(issuerCABundle string, issuerURL string) []string {
return []string{
@ -772,9 +780,7 @@ func TestGetKubeconfig(t *testing.T) {
jwtAuthenticator(issuerCABundle, issuerURL),
}
},
oidcDiscoveryResponse: func(issuerURL string) string {
return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL)
},
oidcDiscoveryResponse: happyOIDCDIscoveryResponse,
idpsDiscoveryResponse: here.Docf(`{
"pinniped_identity_providers": [
{"name": "some-ldap-idp", "type": "ldap"},
@ -848,9 +854,7 @@ func TestGetKubeconfig(t *testing.T) {
jwtAuthenticator(issuerCABundle, issuerURL),
}
},
oidcDiscoveryResponse: func(issuerURL string) string {
return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL)
},
oidcDiscoveryResponse: happyOIDCDIscoveryResponse,
idpsDiscoveryResponse: "this is not valid JSON",
wantLogs: func(issuerCABundle string, issuerURL string) []string {
return []string{
@ -958,7 +962,11 @@ func TestGetKubeconfig(t *testing.T) {
}
},
oidcDiscoveryResponse: func(issuerURL string) string {
return `{"pinniped_identity_providers_endpoint": "https%://illegal_url"}`
return here.Doc(`{
"discovery.supervisor.pinniped.dev/v1alpha1": {
"pinniped_identity_providers_endpoint": "https%://illegal_url"
}
}`)
},
wantLogs: func(issuerCABundle string, issuerURL string) []string {
return []string{
@ -990,9 +998,7 @@ func TestGetKubeconfig(t *testing.T) {
"--upstream-identity-provider-type", "ldap",
}
},
oidcDiscoveryResponse: func(issuerURL string) string {
return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL)
},
oidcDiscoveryResponse: happyOIDCDIscoveryResponse,
idpsDiscoveryResponse: here.Docf(`{
"pinniped_identity_providers": [
{"name": "some-ldap-idp", "type": "ldap"},
@ -1021,9 +1027,7 @@ func TestGetKubeconfig(t *testing.T) {
"--upstream-identity-provider-name", "my-idp",
}
},
oidcDiscoveryResponse: func(issuerURL string) string {
return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL)
},
oidcDiscoveryResponse: happyOIDCDIscoveryResponse,
idpsDiscoveryResponse: here.Docf(`{
"pinniped_identity_providers": [
{"name": "my-idp", "type": "ldap"},
@ -1051,9 +1055,7 @@ func TestGetKubeconfig(t *testing.T) {
"--upstream-identity-provider-type", "ldap",
}
},
oidcDiscoveryResponse: func(issuerURL string) string {
return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL)
},
oidcDiscoveryResponse: happyOIDCDIscoveryResponse,
idpsDiscoveryResponse: here.Docf(`{
"pinniped_identity_providers": [
{"name": "some-oidc-idp", "type": "oidc"},
@ -1079,9 +1081,7 @@ func TestGetKubeconfig(t *testing.T) {
"--upstream-identity-provider-name", "my-nonexistent-idp",
}
},
oidcDiscoveryResponse: func(issuerURL string) string {
return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL)
},
oidcDiscoveryResponse: happyOIDCDIscoveryResponse,
idpsDiscoveryResponse: here.Docf(`{
"pinniped_identity_providers": [
{"name": "some-oidc-idp", "type": "oidc"},
@ -1598,9 +1598,7 @@ func TestGetKubeconfig(t *testing.T) {
jwtAuthenticator(issuerCABundle, issuerURL),
}
},
oidcDiscoveryResponse: func(issuerURL string) string {
return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL)
},
oidcDiscoveryResponse: happyOIDCDIscoveryResponse,
idpsDiscoveryResponse: here.Docf(`{
"pinniped_identity_providers": [
{"name": "some-ldap-idp", "type": "ldap"}
@ -1677,9 +1675,7 @@ func TestGetKubeconfig(t *testing.T) {
jwtAuthenticator(issuerCABundle, issuerURL),
}
},
oidcDiscoveryResponse: func(issuerURL string) string {
return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL)
},
oidcDiscoveryResponse: happyOIDCDIscoveryResponse,
idpsDiscoveryResponse: here.Docf(`{
"pinniped_identity_providers": [
{"name": "some-oidc-idp", "type": "oidc"}
@ -1756,9 +1752,7 @@ func TestGetKubeconfig(t *testing.T) {
jwtAuthenticator(issuerCABundle, issuerURL),
}
},
oidcDiscoveryResponse: func(issuerURL string) string {
return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL)
},
oidcDiscoveryResponse: happyOIDCDIscoveryResponse,
idpsDiscoveryResponse: here.Docf(`{
"pinniped_identity_providers": []
}`),
@ -1818,7 +1812,7 @@ func TestGetKubeconfig(t *testing.T) {
},
},
{
name: "IDP discovery endpoint is not listed in OIDC discovery document",
name: "Supervisor discovery section is not listed in OIDC discovery document",
args: func(issuerCABundle string, issuerURL string) []string {
return []string{
"--kubeconfig", "./testdata/kubeconfig.yaml",
@ -1890,6 +1884,83 @@ func TestGetKubeconfig(t *testing.T) {
base64.StdEncoding.EncodeToString([]byte(issuerCABundle)))
},
},
{
name: "IDP discovery endpoint is not listed in OIDC discovery document within the Supervisor discovery section",
args: func(issuerCABundle string, issuerURL string) []string {
return []string{
"--kubeconfig", "./testdata/kubeconfig.yaml",
"--skip-validation",
}
},
conciergeObjects: func(issuerCABundle string, issuerURL string) []runtime.Object {
return []runtime.Object{
credentialIssuer(),
jwtAuthenticator(issuerCABundle, issuerURL),
}
},
oidcDiscoveryResponse: func(issuerURL string) string {
return here.Doc(`{
"discovery.supervisor.pinniped.dev/v1alpha1": {
"wrong-key": "some-value"
}
}`)
},
idpsDiscoveryStatusCode: http.StatusBadRequest, // IDPs endpoint shouldn't be called by this test
wantLogs: func(issuerCABundle string, issuerURL string) []string {
return []string{
`"level"=0 "msg"="discovered CredentialIssuer" "name"="test-credential-issuer"`,
`"level"=0 "msg"="discovered Concierge operating in TokenCredentialRequest API mode"`,
`"level"=0 "msg"="discovered Concierge endpoint" "endpoint"="https://fake-server-url-value"`,
`"level"=0 "msg"="discovered Concierge certificate authority bundle" "roots"=0`,
`"level"=0 "msg"="discovered JWTAuthenticator" "name"="test-authenticator"`,
fmt.Sprintf(`"level"=0 "msg"="discovered OIDC issuer" "issuer"="%s"`, issuerURL),
`"level"=0 "msg"="discovered OIDC audience" "audience"="test-audience"`,
`"level"=0 "msg"="discovered OIDC CA bundle" "roots"=1`,
}
},
wantStdout: func(issuerCABundle string, issuerURL string) string {
return here.Docf(`
apiVersion: v1
clusters:
- cluster:
certificate-authority-data: ZmFrZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YS12YWx1ZQ==
server: https://fake-server-url-value
name: kind-cluster-pinniped
contexts:
- context:
cluster: kind-cluster-pinniped
user: kind-user-pinniped
name: kind-context-pinniped
current-context: kind-context-pinniped
kind: Config
preferences: {}
users:
- name: kind-user-pinniped
user:
exec:
apiVersion: client.authentication.k8s.io/v1beta1
args:
- login
- oidc
- --enable-concierge
- --concierge-api-group-suffix=pinniped.dev
- --concierge-authenticator-name=test-authenticator
- --concierge-authenticator-type=jwt
- --concierge-endpoint=https://fake-server-url-value
- --concierge-ca-bundle-data=ZmFrZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YS12YWx1ZQ==
- --issuer=%s
- --client-id=pinniped-cli
- --scopes=offline_access,openid,pinniped:request-audience
- --ca-bundle-data=%s
- --request-audience=test-audience
command: '.../path/to/pinniped'
env: []
provideClusterInfo: true
`,
issuerURL,
base64.StdEncoding.EncodeToString([]byte(issuerCABundle)))
},
},
{
name: "when OIDC discovery document 404s, dont set idp related flags",
args: func(issuerCABundle string, issuerURL string) []string {
@ -2050,9 +2121,7 @@ func TestGetKubeconfig(t *testing.T) {
jwtAuthenticator(issuerCABundle, issuerURL),
}
},
oidcDiscoveryResponse: func(issuerURL string) string {
return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL)
},
oidcDiscoveryResponse: happyOIDCDIscoveryResponse,
idpsDiscoveryResponse: here.Docf(`{
"pinniped_identity_providers": [
{"name": "some-other-ldap-idp", "type": "ldap"}
@ -2127,9 +2196,7 @@ func TestGetKubeconfig(t *testing.T) {
"--oidc-ca-bundle", f.Name(),
}
},
oidcDiscoveryResponse: func(issuerURL string) string {
return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL)
},
oidcDiscoveryResponse: happyOIDCDIscoveryResponse,
idpsDiscoveryResponse: here.Docf(`{
"pinniped_identity_providers": [
{"name": "some-ldap-idp", "type": "ldap"}
@ -2186,9 +2253,7 @@ func TestGetKubeconfig(t *testing.T) {
"--upstream-identity-provider-type", "ldap",
}
},
oidcDiscoveryResponse: func(issuerURL string) string {
return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL)
},
oidcDiscoveryResponse: happyOIDCDIscoveryResponse,
idpsDiscoveryResponse: here.Docf(`{
"pinniped_identity_providers": [
{"name": "some-ldap-idp", "type": "ldap"},
@ -2247,9 +2312,7 @@ func TestGetKubeconfig(t *testing.T) {
"--upstream-identity-provider-name", "some-ldap-idp",
}
},
oidcDiscoveryResponse: func(issuerURL string) string {
return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL)
},
oidcDiscoveryResponse: happyOIDCDIscoveryResponse,
idpsDiscoveryResponse: here.Docf(`{
"pinniped_identity_providers": [
{"name": "some-ldap-idp", "type": "ldap"},
@ -2313,7 +2376,7 @@ func TestGetKubeconfig(t *testing.T) {
w.WriteHeader(tt.oidcDiscoveryStatusCode)
_, err = w.Write([]byte(jsonResponseBody))
require.NoError(t, err)
case "/pinniped_identity_providers":
case "/v1alpha1/pinniped_identity_providers":
jsonResponseBody := tt.idpsDiscoveryResponse
if tt.idpsDiscoveryResponse == "" {
jsonResponseBody = "{}"

View File

@ -160,7 +160,7 @@ func runOIDCLogin(cmd *cobra.Command, deps oidcLoginCommandDeps, flags oidcLogin
case "oidc":
// this is the default, so don't need to do anything
case "ldap":
opts = append(opts, oidcclient.WithLDAPUpstreamIdentityProvider())
opts = append(opts, oidcclient.WithCLISendingCredentials())
default:
// Surprisingly cobra does not support this kind of flag validation. See https://github.com/spf13/pflag/issues/236
return fmt.Errorf(

View File

@ -40,11 +40,15 @@ type Metadata struct {
// vvv Custom vvv
PinnipedIDPsEndpoint string `json:"pinniped_identity_providers_endpoint"`
SupervisorDiscovery SupervisorDiscoveryMetadataV1Alpha1 `json:"discovery.supervisor.pinniped.dev/v1alpha1"`
// ^^^ Custom ^^^
}
type SupervisorDiscoveryMetadataV1Alpha1 struct {
PinnipedIDPsEndpoint string `json:"pinniped_identity_providers_endpoint"`
}
type IdentityProviderMetadata struct {
Name string `json:"name"`
Type string `json:"type"`
@ -57,7 +61,7 @@ func NewHandler(issuerURL string) http.Handler {
AuthorizationEndpoint: issuerURL + oidc.AuthorizationEndpointPath,
TokenEndpoint: issuerURL + oidc.TokenEndpointPath,
JWKSURI: issuerURL + oidc.JWKSEndpointPath,
PinnipedIDPsEndpoint: issuerURL + oidc.PinnipedIDPsPath,
SupervisorDiscovery: SupervisorDiscoveryMetadataV1Alpha1{PinnipedIDPsEndpoint: issuerURL + oidc.PinnipedIDPsPathV1Alpha1},
ResponseTypesSupported: []string{"code"},
SubjectTypesSupported: []string{"public"},
IDTokenSigningAlgValuesSupported: []string{"ES256"},

View File

@ -35,11 +35,13 @@ func TestDiscovery(t *testing.T) {
wantStatus: http.StatusOK,
wantContentType: "application/json",
wantBodyJSON: &Metadata{
Issuer: "https://some-issuer.com/some/path",
AuthorizationEndpoint: "https://some-issuer.com/some/path/oauth2/authorize",
TokenEndpoint: "https://some-issuer.com/some/path/oauth2/token",
JWKSURI: "https://some-issuer.com/some/path/jwks.json",
PinnipedIDPsEndpoint: "https://some-issuer.com/some/path/pinniped_identity_providers",
Issuer: "https://some-issuer.com/some/path",
AuthorizationEndpoint: "https://some-issuer.com/some/path/oauth2/authorize",
TokenEndpoint: "https://some-issuer.com/some/path/oauth2/token",
JWKSURI: "https://some-issuer.com/some/path/jwks.json",
SupervisorDiscovery: SupervisorDiscoveryMetadataV1Alpha1{
PinnipedIDPsEndpoint: "https://some-issuer.com/some/path/v1alpha1/pinniped_identity_providers",
},
ResponseTypesSupported: []string{"code"},
SubjectTypesSupported: []string{"public"},
IDTokenSigningAlgValuesSupported: []string{"ES256"},

View File

@ -24,7 +24,7 @@ const (
TokenEndpointPath = "/oauth2/token" //nolint:gosec // ignore lint warning that this is a credential
CallbackEndpointPath = "/callback"
JWKSEndpointPath = "/jwks.json"
PinnipedIDPsPath = "/pinniped_identity_providers"
PinnipedIDPsPathV1Alpha1 = "/v1alpha1/pinniped_identity_providers"
)
const (

View File

@ -107,7 +107,7 @@ func (m *Manager) SetProviders(federationDomains ...*provider.FederationDomainIs
m.providerHandlers[(issuerHostWithPath + oidc.JWKSEndpointPath)] = jwks.NewHandler(issuer, m.dynamicJWKSProvider)
m.providerHandlers[(issuerHostWithPath + oidc.PinnipedIDPsPath)] = idpdiscovery.NewHandler(m.upstreamIDPs)
m.providerHandlers[(issuerHostWithPath + oidc.PinnipedIDPsPathV1Alpha1)] = idpdiscovery.NewHandler(m.upstreamIDPs)
m.providerHandlers[(issuerHostWithPath + oidc.AuthorizationEndpointPath)] = auth.NewHandler(
issuer,

View File

@ -86,13 +86,13 @@ func TestManager(t *testing.T) {
err = json.Unmarshal(responseBody, &parsedDiscoveryResult)
r.NoError(err)
r.Equal(expectedIssuer, parsedDiscoveryResult.Issuer)
r.Equal(parsedDiscoveryResult.PinnipedIDPsEndpoint, expectedIssuer+oidc.PinnipedIDPsPath)
r.Equal(parsedDiscoveryResult.SupervisorDiscovery.PinnipedIDPsEndpoint, expectedIssuer+oidc.PinnipedIDPsPathV1Alpha1)
}
requirePinnipedIDPsDiscoveryRequestToBeHandled := func(requestIssuer, requestURLSuffix, expectedIDPName, expectedIDPType string) {
recorder := httptest.NewRecorder()
subject.ServeHTTP(recorder, newGetRequest(requestIssuer+oidc.PinnipedIDPsPath+requestURLSuffix))
subject.ServeHTTP(recorder, newGetRequest(requestIssuer+oidc.PinnipedIDPsPathV1Alpha1+requestURLSuffix))
r.False(fallbackHandlerWasCalled)

View File

@ -74,7 +74,7 @@ type handlerState struct {
upstreamIdentityProviderName string
upstreamIdentityProviderType string
ldapUpstreamIdentityProvider bool
cliToSendCredentials bool
requestedAudience string
@ -200,14 +200,15 @@ func WithRequestAudience(audience string) Option {
}
}
// WithLDAPUpstreamIdentityProvider causes the login flow to use CLI prompts for username and password and causes the
// WithCLISendingCredentials causes the login flow to use CLI-based prompts for username and password and causes the
// call to the Issuer's authorize endpoint to be made directly (no web browser) with the username and password on custom
// HTTP headers. This is only intended to be used when the issuer is a Pinniped Supervisor and the upstream identity
// provider is an LDAP provider. It should never be used with non-Supervisor issuers because it will send the user's
// password as a custom header, which would be ignored but could potentially get logged somewhere by the issuer.
func WithLDAPUpstreamIdentityProvider() Option {
// provider type supports this style of authentication. Currently this is supported by LDAPIdentityProviders.
// This should never be used with non-Supervisor issuers because it will send the user's password to the authorization
// endpoint as a custom header, which would be ignored but could potentially get logged somewhere by the issuer.
func WithCLISendingCredentials() Option {
return func(h *handlerState) error {
h.ldapUpstreamIdentityProvider = true
h.cliToSendCredentials = true
return nil
}
}
@ -356,7 +357,7 @@ func (h *handlerState) baseLogin() (*oidctypes.Token, error) {
// Choose the appropriate authorization and authcode exchange strategy.
var authFunc = h.webBrowserBasedAuth
if h.ldapUpstreamIdentityProvider {
if h.cliToSendCredentials {
authFunc = h.cliBasedAuth
}
@ -371,8 +372,8 @@ func (h *handlerState) baseLogin() (*oidctypes.Token, error) {
return token, err
}
// Make a direct call to the authorize endpoint and parse the authcode from the response.
// Exchange the authcode for tokens. Return the tokens or an error.
// Make a direct call to the authorize endpoint, including the user's username and password on custom http headers,
// and parse the authcode from the response. Exchange the authcode for tokens. Return the tokens or an error.
func (h *handlerState) cliBasedAuth(authorizeOptions *[]oauth2.AuthCodeOption) (*oidctypes.Token, error) {
// Ask the user for their username and password.
username, err := h.promptForValue(defaultLDAPUsernamePrompt)

View File

@ -232,7 +232,7 @@ func TestLogin(t *testing.T) { // nolint:gocyclo
require.Equal(t, []SessionCacheKey{cacheKey}, cache.sawGetKeys)
})
require.NoError(t, WithSessionCache(cache)(h))
require.NoError(t, WithLDAPUpstreamIdentityProvider()(h))
require.NoError(t, WithCLISendingCredentials()(h))
require.NoError(t, WithUpstreamIdentityProvider("some-upstream-name", "ldap")(h))
require.NoError(t, WithClient(&http.Client{
@ -875,7 +875,7 @@ func TestLogin(t *testing.T) { // nolint:gocyclo
require.Equal(t, []*oidctypes.Token{&testToken}, cache.sawPutTokens)
})
require.NoError(t, WithSessionCache(cache)(h))
require.NoError(t, WithLDAPUpstreamIdentityProvider()(h))
require.NoError(t, WithCLISendingCredentials()(h))
require.NoError(t, WithUpstreamIdentityProvider("some-upstream-name", "ldap")(h))
discoveryRequestWasMade := false