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

upstreamwatcher: do not truncate explicit oidc errors

Browse Source 
This change makes it easier to understand misconfigurations caused
by issuers with extraneous trailing slashes.

Signed-off-by: Mo Khan <mok@vmware.com>
This commit is contained in:
Mo Khan 2021-05-10 00:22:34 -04:00
parent 9fc7f43245
commit 56d316e8d3
No known key found for this signature in database
GPG Key ID: 52C90ADA01B269B8
3 changed files with 201 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher.go
View File

@ -13,6 +13,7 @@ import (
"net/http"
"net/url"
"sort"
"strings"
"time"
"github.com/coreos/go-oidc/v3/oidc"
@ -279,7 +280,7 @@ func (c *controller) validateIssuer(ctx context.Context, upstream *v1alpha1.OIDC
Type: typeOIDCDiscoverySucceeded,
Status: v1alpha1.ConditionFalse,
Reason: reasonUnreachable,
Message: fmt.Sprintf("failed to perform OIDC discovery against %q:\n%s", upstream.Spec.Issuer, truncateErr(err)),
Message: fmt.Sprintf("failed to perform OIDC discovery against %q:\n%s", upstream.Spec.Issuer, truncateNonOIDCErr(err)),
}
}
@ -426,11 +427,11 @@ func computeScopes(additionalScopes []string) []string {
return scopes
}
func truncateErr(err error) string {
func truncateNonOIDCErr(err error) string {
const max = 100
msg := err.Error()
if len(msg) <= max {
if len(msg) <= max || strings.HasPrefix(msg, "oidc:") {
return msg
}

165
internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go
View File

@ -602,6 +602,151 @@ Get "invalid-url-that-is-really-really-long/.well-known/openid-configuration": u
},
}},
},
{
name: "existing valid upstream with trailing slash",
inputUpstreams: []runtime.Object{&v1alpha1.OIDCIdentityProvider{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234},
Spec: v1alpha1.OIDCIdentityProviderSpec{
Issuer: testIssuerURL + "/ends-with-slash/",
TLS: &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64},
Client: v1alpha1.OIDCClient{SecretName: testSecretName},
AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes},
Claims: v1alpha1.OIDCClaims{Groups: testGroupsClaim, Username: testUsernameClaim},
},
Status: v1alpha1.OIDCIdentityProviderStatus{
Phase: "Ready",
Conditions: []v1alpha1.Condition{
{Type: "ClientCredentialsValid", Status: "True", LastTransitionTime: earlier, Reason: "Success", Message: "loaded client credentials"},
{Type: "OIDCDiscoverySucceeded", Status: "True", LastTransitionTime: earlier, Reason: "Success", Message: "discovered issuer configuration"},
},
},
}},
inputSecrets: []runtime.Object{&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testSecretName},
Type: "secrets.pinniped.dev/oidc-client",
Data: testValidSecretData,
}},
wantLogs: []string{
`upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`,
`upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="discovered issuer configuration" "reason"="Success" "status"="True" "type"="OIDCDiscoverySucceeded"`,
},
wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{
&oidctestutil.TestUpstreamOIDCIdentityProvider{
Name: testName,
ClientID: testClientID,
AuthorizationURL: *testIssuerAuthorizeURL,
Scopes: testExpectedScopes,
UsernameClaim: testUsernameClaim,
GroupsClaim: testGroupsClaim,
},
},
wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234},
Status: v1alpha1.OIDCIdentityProviderStatus{
Phase: "Ready",
Conditions: []v1alpha1.Condition{
{Type: "ClientCredentialsValid", Status: "True", LastTransitionTime: earlier, Reason: "Success", Message: "loaded client credentials", ObservedGeneration: 1234},
{Type: "OIDCDiscoverySucceeded", Status: "True", LastTransitionTime: earlier, Reason: "Success", Message: "discovered issuer configuration", ObservedGeneration: 1234},
},
},
}},
},
{
name: "issuer is invalid URL, missing trailing slash",
inputUpstreams: []runtime.Object{&v1alpha1.OIDCIdentityProvider{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
Spec: v1alpha1.OIDCIdentityProviderSpec{
Issuer: testIssuerURL + "/ends-with-slash",
TLS: &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64},
Client: v1alpha1.OIDCClient{SecretName: testSecretName},
AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes},
},
}},
inputSecrets: []runtime.Object{&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testSecretName},
Type: "secrets.pinniped.dev/oidc-client",
Data: testValidSecretData,
}},
wantErr: controllerlib.ErrSyntheticRequeue.Error(),
wantLogs: []string{
`upstream-observer "msg"="failed to perform OIDC discovery" "error"="oidc: issuer did not match the issuer returned by provider, expected \"` + testIssuerURL + `/ends-with-slash\" got \"` + testIssuerURL + `/ends-with-slash/\"" "issuer"="` + testIssuerURL + `/ends-with-slash" "name"="test-name" "namespace"="test-namespace"`,
`upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`,
`upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="failed to perform OIDC discovery against \"` + testIssuerURL + `/ends-with-slash\":\noidc: issuer did not match the issuer returned by provider, expected \"` + testIssuerURL + `/ends-with-slash\" got \"` + testIssuerURL + `/ends-with-slash/\"" "reason"="Unreachable" "status"="False" "type"="OIDCDiscoverySucceeded"`,
`upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="failed to perform OIDC discovery against \"` + testIssuerURL + `/ends-with-slash\":\noidc: issuer did not match the issuer returned by provider, expected \"` + testIssuerURL + `/ends-with-slash\" got \"` + testIssuerURL + `/ends-with-slash/\"" "name"="test-name" "namespace"="test-namespace" "reason"="Unreachable" "type"="OIDCDiscoverySucceeded"`,
},
wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{},
wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
Status: v1alpha1.OIDCIdentityProviderStatus{
Phase: "Error",
Conditions: []v1alpha1.Condition{
{
Type: "ClientCredentialsValid",
Status: "True",
LastTransitionTime: now,
Reason: "Success",
Message: "loaded client credentials",
},
{
Type: "OIDCDiscoverySucceeded",
Status: "False",
LastTransitionTime: now,
Reason: "Unreachable",
Message: `failed to perform OIDC discovery against "` + testIssuerURL + `/ends-with-slash":
oidc: issuer did not match the issuer returned by provider, expected "` + testIssuerURL + `/ends-with-slash" got "` + testIssuerURL + `/ends-with-slash/"`,
},
},
},
}},
},
{
name: "issuer is invalid URL, extra trailing slash",
inputUpstreams: []runtime.Object{&v1alpha1.OIDCIdentityProvider{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
Spec: v1alpha1.OIDCIdentityProviderSpec{
Issuer: testIssuerURL + "/",
TLS: &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64},
Client: v1alpha1.OIDCClient{SecretName: testSecretName},
AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes},
},
}},
inputSecrets: []runtime.Object{&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testSecretName},
Type: "secrets.pinniped.dev/oidc-client",
Data: testValidSecretData,
}},
wantErr: controllerlib.ErrSyntheticRequeue.Error(),
wantLogs: []string{
`upstream-observer "msg"="failed to perform OIDC discovery" "error"="oidc: issuer did not match the issuer returned by provider, expected \"` + testIssuerURL + `/\" got \"` + testIssuerURL + `\"" "issuer"="` + testIssuerURL + `/" "name"="test-name" "namespace"="test-namespace"`,
`upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`,
`upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="failed to perform OIDC discovery against \"` + testIssuerURL + `/\":\noidc: issuer did not match the issuer returned by provider, expected \"` + testIssuerURL + `/\" got \"` + testIssuerURL + `\"" "reason"="Unreachable" "status"="False" "type"="OIDCDiscoverySucceeded"`,
`upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="failed to perform OIDC discovery against \"` + testIssuerURL + `/\":\noidc: issuer did not match the issuer returned by provider, expected \"` + testIssuerURL + `/\" got \"` + testIssuerURL + `\"" "name"="test-name" "namespace"="test-namespace" "reason"="Unreachable" "type"="OIDCDiscoverySucceeded"`,
},
wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{},
wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
Status: v1alpha1.OIDCIdentityProviderStatus{
Phase: "Error",
Conditions: []v1alpha1.Condition{
{
Type: "ClientCredentialsValid",
Status: "True",
LastTransitionTime: now,
Reason: "Success",
Message: "loaded client credentials",
},
{
Type: "OIDCDiscoverySucceeded",
Status: "False",
LastTransitionTime: now,
Reason: "Unreachable",
Message: `failed to perform OIDC discovery against "` + testIssuerURL + `/":
oidc: issuer did not match the issuer returned by provider, expected "` + testIssuerURL + `/" got "` + testIssuerURL + `"`,
},
},
},
}},
},
}
for _, tt := range tests {
tt := tt
@ -730,5 +875,25 @@ func newTestIssuer(t *testing.T) (string, string) {
})
})
// handle the four issuer with trailing slash configs
// valid case in= out=
// handled above at the root of testURL
// valid case in=/ out=/
mux.HandleFunc("/ends-with-slash/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("content-type", "application/json")
_ = json.NewEncoder(w).Encode(&providerJSON{
Issuer: testURL + "/ends-with-slash/",
AuthURL: "https://example.com/authorize",
})
})
// invalid case in= out=/
// can be tested using /ends-with-slash/ endpoint
// invalid case in=/ out=
// can be tested using root endpoint
return caBundlePEM, testURL
}

32
test/integration/supervisor_upstream_test.go
View File

@ -43,6 +43,38 @@ Get "https://127.0.0.1:444444/issuer/.well-known/openid-configuration": dial tcp
})
})
t.Run("invalid issuer with trailing slash", func(t *testing.T) {
t.Parallel()
spec := v1alpha1.OIDCIdentityProviderSpec{
Issuer: env.SupervisorTestUpstream.Issuer + "/",
TLS: &v1alpha1.TLSSpec{
CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorTestUpstream.CABundle)),
},
AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{
AdditionalScopes: []string{"email", "profile"},
},
Client: v1alpha1.OIDCClient{
SecretName: library.CreateClientCredsSecret(t, "test-client-id", "test-client-secret").Name,
},
}
upstream := library.CreateTestOIDCIdentityProvider(t, spec, v1alpha1.PhaseError)
expectUpstreamConditions(t, upstream, []v1alpha1.Condition{
{
Type: "ClientCredentialsValid",
Status: v1alpha1.ConditionTrue,
Reason: "Success",
Message: "loaded client credentials",
},
{
Type: "OIDCDiscoverySucceeded",
Status: v1alpha1.ConditionFalse,
Reason: "Unreachable",
Message: `failed to perform OIDC discovery against "` + env.SupervisorTestUpstream.Issuer + `/":
oidc: issuer did not match the issuer returned by provider, expected "` + env.SupervisorTestUpstream.Issuer + `/" got "` + env.SupervisorTestUpstream.Issuer + `"`,
},
})
})
t.Run("valid", func(t *testing.T) {
t.Parallel()
spec := v1alpha1.OIDCIdentityProviderSpec{