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

Merge pull request #876 from vmware-tanzu/upstream_refresh_revocation_during_gc

Browse Source 
Revoke upstream OIDC refresh tokens during downstream session garbage collection
This commit is contained in:
Mo Khan 2021-11-23 20:15:37 -05:00 committed by GitHub
commit 1611cf681a
18 changed files with 2128 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go
View File

@ -344,6 +344,44 @@ func (c *oidcWatcherController) validateIssuer(ctx context.Context, upstream *v1
c.validatorCache.putProvider(&upstream.Spec, discoveredProvider, httpClient)
}
// Get the revocation endpoint, if there is one. Many providers do not offer a revocation endpoint.
var additionalDiscoveryClaims struct {
// "revocation_endpoint" is specified by https://datatracker.ietf.org/doc/html/rfc8414#section-2
RevocationEndpoint string `json:"revocation_endpoint"`
}
if err := discoveredProvider.Claims(&additionalDiscoveryClaims); err != nil {
// This shouldn't actually happen because the above call to NewProvider() would have already returned this error.
return &v1alpha1.Condition{
Type: typeOIDCDiscoverySucceeded,
Status: v1alpha1.ConditionFalse,
Reason: reasonInvalidResponse,
Message: fmt.Sprintf("failed to unmarshal OIDC discovery response from %q:\n%s", upstream.Spec.Issuer, truncateMostLongErr(err)),
}
}
if additionalDiscoveryClaims.RevocationEndpoint != "" {
// Found a revocation URL. Try to parse it.
revocationURL, err := url.Parse(additionalDiscoveryClaims.RevocationEndpoint)
if err != nil {
return &v1alpha1.Condition{
Type: typeOIDCDiscoverySucceeded,
Status: v1alpha1.ConditionFalse,
Reason: reasonInvalidResponse,
Message: fmt.Sprintf("failed to parse revocation endpoint URL: %v", err),
}
}
// Don't want to send refresh tokens to an insecure revocation endpoint, so require that it use https.
if revocationURL.Scheme != "https" {
return &v1alpha1.Condition{
Type: typeOIDCDiscoverySucceeded,
Status: v1alpha1.ConditionFalse,
Reason: reasonInvalidResponse,
Message: fmt.Sprintf(`revocation endpoint URL scheme must be "https", not %q`, revocationURL.Scheme),
}
}
// Remember the URL for later use.
result.RevocationURL = revocationURL
}
// Parse out and validate the discovered authorize endpoint.
authURL, err := url.Parse(discoveredProvider.Endpoint().AuthURL)
if err != nil {

247
internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go
View File

@ -115,6 +115,8 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
testIssuerCABase64 := base64.StdEncoding.EncodeToString([]byte(testIssuerCA))
testIssuerAuthorizeURL, err := url.Parse("https://example.com/authorize")
require.NoError(t, err)
testIssuerRevocationURL, err := url.Parse("https://example.com/revoke")
require.NoError(t, err)
wrongCA, err := certauthority.New("foo", time.Hour)
require.NoError(t, err)
@ -152,7 +154,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
inputSecrets []runtime.Object
wantErr string
wantLogs []string
wantResultingCache []provider.UpstreamOIDCIdentityProviderI
wantResultingCache []*oidctestutil.TestUpstreamOIDCIdentityProvider
wantResultingUpstreams []v1alpha1.OIDCIdentityProvider
}{
{
@ -176,7 +178,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`,
`oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="secret \"test-client-secret\" not found" "name"="test-name" "namespace"="test-namespace" "reason"="SecretNotFound" "type"="ClientCredentialsValid"`,
},
wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{},
wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{},
wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
Status: v1alpha1.OIDCIdentityProviderStatus{
@ -223,7 +225,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`,
`oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="referenced Secret \"test-client-secret\" has wrong type \"some-other-type\" (should be \"secrets.pinniped.dev/oidc-client\")" "name"="test-name" "namespace"="test-namespace" "reason"="SecretWrongType" "type"="ClientCredentialsValid"`,
},
wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{},
wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{},
wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
Status: v1alpha1.OIDCIdentityProviderStatus{
@ -269,7 +271,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`,
`oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="referenced Secret \"test-client-secret\" is missing required keys [\"clientID\" \"clientSecret\"]" "name"="test-name" "namespace"="test-namespace" "reason"="SecretMissingKeys" "type"="ClientCredentialsValid"`,
},
wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{},
wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{},
wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
Status: v1alpha1.OIDCIdentityProviderStatus{
@ -318,7 +320,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`,
`oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="spec.certificateAuthorityData is invalid: illegal base64 data at input byte 7" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidTLSConfig" "type"="OIDCDiscoverySucceeded"`,
},
wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{},
wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{},
wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
Status: v1alpha1.OIDCIdentityProviderStatus{
@ -367,7 +369,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`,
`oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="spec.certificateAuthorityData is invalid: no certificates found" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidTLSConfig" "type"="OIDCDiscoverySucceeded"`,
},
wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{},
wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{},
wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
Status: v1alpha1.OIDCIdentityProviderStatus{
@ -414,7 +416,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`,
`oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="failed to perform OIDC discovery against \"invalid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\":\nGet \"invalid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee/.well-known/openid-configuration\": unsupported protocol [truncated 9 chars]" "name"="test-name" "namespace"="test-namespace" "reason"="Unreachable" "type"="OIDCDiscoverySucceeded"`,
},
wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{},
wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{},
wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
Status: v1alpha1.OIDCIdentityProviderStatus{
@ -463,7 +465,7 @@ Get "invalid-url-that-is-really-really-long-nanananananananannanananan-batman-na
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`,
`oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="failed to perform OIDC discovery against \"` + testIssuerURL + `/valid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\":\nGet \"` + testIssuerURL + `/valid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee/.well-known/openid-configuration\": x509: certificate signed by unknown authority" "name"="test-name" "namespace"="test-namespace" "reason"="Unreachable" "type"="OIDCDiscoverySucceeded"`,
},
wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{},
wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{},
wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
Status: v1alpha1.OIDCIdentityProviderStatus{
@ -511,7 +513,7 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`,
`oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="failed to parse authorization endpoint URL: parse \"%\": invalid URL escape \"%\"" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidResponse" "type"="OIDCDiscoverySucceeded"`,
},
wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{},
wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{},
wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
Status: v1alpha1.OIDCIdentityProviderStatus{
@ -536,6 +538,53 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana
},
}},
},
{
name: "issuer returns invalid revocation URL",
inputUpstreams: []runtime.Object{&v1alpha1.OIDCIdentityProvider{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
Spec: v1alpha1.OIDCIdentityProviderSpec{
Issuer: testIssuerURL + "/invalid-revocation-url",
TLS: &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64},
Client: v1alpha1.OIDCClient{SecretName: testSecretName},
},
}},
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{
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`,
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="failed to parse revocation endpoint URL: parse \"%\": invalid URL escape \"%\"" "reason"="InvalidResponse" "status"="False" "type"="OIDCDiscoverySucceeded"`,
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`,
`oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="failed to parse revocation endpoint URL: parse \"%\": invalid URL escape \"%\"" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidResponse" "type"="OIDCDiscoverySucceeded"`,
},
wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{},
wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
Status: v1alpha1.OIDCIdentityProviderStatus{
Phase: "Error",
Conditions: []v1alpha1.Condition{
happyAdditionalAuthorizeParametersValidCondition,
{
Type: "ClientCredentialsValid",
Status: "True",
LastTransitionTime: now,
Reason: "Success",
Message: "loaded client credentials",
},
{
Type: "OIDCDiscoverySucceeded",
Status: "False",
LastTransitionTime: now,
Reason: "InvalidResponse",
Message: `failed to parse revocation endpoint URL: parse "%": invalid URL escape "%"`,
},
},
},
}},
},
{
name: "issuer returns insecure authorize URL",
inputUpstreams: []runtime.Object{&v1alpha1.OIDCIdentityProvider{
@ -558,7 +607,7 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`,
`oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="authorization endpoint URL scheme must be \"https\", not \"http\"" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidResponse" "type"="OIDCDiscoverySucceeded"`,
},
wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{},
wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{},
wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
Status: v1alpha1.OIDCIdentityProviderStatus{
@ -583,6 +632,53 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana
},
}},
},
{
name: "issuer returns insecure revocation URL",
inputUpstreams: []runtime.Object{&v1alpha1.OIDCIdentityProvider{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
Spec: v1alpha1.OIDCIdentityProviderSpec{
Issuer: testIssuerURL + "/insecure-revocation-url",
TLS: &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64},
Client: v1alpha1.OIDCClient{SecretName: testSecretName},
},
}},
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{
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`,
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="revocation endpoint URL scheme must be \"https\", not \"http\"" "reason"="InvalidResponse" "status"="False" "type"="OIDCDiscoverySucceeded"`,
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`,
`oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="revocation endpoint URL scheme must be \"https\", not \"http\"" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidResponse" "type"="OIDCDiscoverySucceeded"`,
},
wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{},
wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
Status: v1alpha1.OIDCIdentityProviderStatus{
Phase: "Error",
Conditions: []v1alpha1.Condition{
happyAdditionalAuthorizeParametersValidCondition,
{
Type: "ClientCredentialsValid",
Status: "True",
LastTransitionTime: now,
Reason: "Success",
Message: "loaded client credentials",
},
{
Type: "OIDCDiscoverySucceeded",
Status: "False",
LastTransitionTime: now,
Reason: "InvalidResponse",
Message: `revocation endpoint URL scheme must be "https", not "http"`,
},
},
},
}},
},
{
name: "upstream with error becomes valid",
inputUpstreams: []runtime.Object{&v1alpha1.OIDCIdentityProvider{
@ -615,11 +711,12 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="discovered issuer configuration" "reason"="Success" "status"="True" "type"="OIDCDiscoverySucceeded"`,
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`,
},
wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{
&oidctestutil.TestUpstreamOIDCIdentityProvider{
wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{
{
Name: testName,
ClientID: testClientID,
AuthorizationURL: *testIssuerAuthorizeURL,
RevocationURL: testIssuerRevocationURL,
Scopes: append(testExpectedScopes, "xyz"), // includes openid only once
UsernameClaim: testUsernameClaim,
GroupsClaim: testGroupsClaim,
@ -669,11 +766,67 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="discovered issuer configuration" "reason"="Success" "status"="True" "type"="OIDCDiscoverySucceeded"`,
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`,
},
wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{
&oidctestutil.TestUpstreamOIDCIdentityProvider{
wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{
{
Name: testName,
ClientID: testClientID,
AuthorizationURL: *testIssuerAuthorizeURL,
RevocationURL: testIssuerRevocationURL,
Scopes: testDefaultExpectedScopes,
UsernameClaim: testUsernameClaim,
GroupsClaim: testGroupsClaim,
AllowPasswordGrant: false,
AdditionalAuthcodeParams: map[string]string{},
ResourceUID: testUID,
},
},
wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: v1alpha1.OIDCIdentityProviderStatus{
Phase: "Ready",
Conditions: []v1alpha1.Condition{
{Type: "AdditionalAuthorizeParametersValid", Status: "True", LastTransitionTime: earlier, Reason: "Success", Message: "additionalAuthorizeParameters parameter names are allowed", ObservedGeneration: 1234},
{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: "existing valid upstream with no revocation endpoint in the discovery document",
inputUpstreams: []runtime.Object{&v1alpha1.OIDCIdentityProvider{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Spec: v1alpha1.OIDCIdentityProviderSpec{
Issuer: testIssuerURL + "/valid-without-revocation",
TLS: &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64},
Client: v1alpha1.OIDCClient{SecretName: testSecretName},
Claims: v1alpha1.OIDCClaims{Groups: testGroupsClaim, Username: testUsernameClaim},
},
Status: v1alpha1.OIDCIdentityProviderStatus{
Phase: "Ready",
Conditions: []v1alpha1.Condition{
happyAdditionalAuthorizeParametersValidConditionEarlier,
{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{
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`,
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="discovered issuer configuration" "reason"="Success" "status"="True" "type"="OIDCDiscoverySucceeded"`,
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`,
},
wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{
{
Name: testName,
ClientID: testClientID,
AuthorizationURL: *testIssuerAuthorizeURL,
RevocationURL: nil, // no revocation URL is set in the cached provider because none was returned by discovery
Scopes: testDefaultExpectedScopes,
UsernameClaim: testUsernameClaim,
GroupsClaim: testGroupsClaim,
@ -726,11 +879,12 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="discovered issuer configuration" "reason"="Success" "status"="True" "type"="OIDCDiscoverySucceeded"`,
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`,
},
wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{
&oidctestutil.TestUpstreamOIDCIdentityProvider{
wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{
{
Name: testName,
ClientID: testClientID,
AuthorizationURL: *testIssuerAuthorizeURL,
RevocationURL: testIssuerRevocationURL,
Scopes: testExpectedScopes,
UsernameClaim: testUsernameClaim,
GroupsClaim: testGroupsClaim,
@ -785,11 +939,12 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="discovered issuer configuration" "reason"="Success" "status"="True" "type"="OIDCDiscoverySucceeded"`,
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`,
},
wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{
&oidctestutil.TestUpstreamOIDCIdentityProvider{
wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{
{
Name: testName,
ClientID: testClientID,
AuthorizationURL: *testIssuerAuthorizeURL,
RevocationURL: testIssuerRevocationURL,
Scopes: testExpectedScopes, // does not include the default scopes
UsernameClaim: testUsernameClaim,
GroupsClaim: testGroupsClaim,
@ -846,7 +1001,7 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="the following additionalAuthorizeParameters are not allowed: response_type,scope,client_id,state,nonce,code_challenge,code_challenge_method,redirect_uri,hd" "reason"="DisallowedParameterName" "status"="False" "type"="AdditionalAuthorizeParametersValid"`,
`oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="the following additionalAuthorizeParameters are not allowed: response_type,scope,client_id,state,nonce,code_challenge,code_challenge_method,redirect_uri,hd" "name"="test-name" "namespace"="test-namespace" "reason"="DisallowedParameterName" "type"="AdditionalAuthorizeParametersValid"`,
},
wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{},
wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{},
wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234, UID: testUID},
Status: v1alpha1.OIDCIdentityProviderStatus{
@ -884,7 +1039,7 @@ Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nananananananana
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`,
`oidc-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{},
wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{},
wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
Status: v1alpha1.OIDCIdentityProviderStatus{
@ -933,7 +1088,7 @@ oidc: issuer did not match the issuer returned by provider, expected "` + testIs
`oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`,
`oidc-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{},
wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{},
wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
Status: v1alpha1.OIDCIdentityProviderStatus{
@ -1011,6 +1166,7 @@ oidc: issuer did not match the issuer returned by provider, expected "` + testIs
require.Equal(t, tt.wantResultingCache[i].AllowsPasswordGrant(), actualIDP.AllowsPasswordGrant())
require.Equal(t, tt.wantResultingCache[i].GetAdditionalAuthcodeParams(), actualIDP.GetAdditionalAuthcodeParams())
require.Equal(t, tt.wantResultingCache[i].GetResourceUID(), actualIDP.GetResourceUID())
require.Equal(t, tt.wantResultingCache[i].GetRevocationURL(), actualIDP.GetRevocationURL())
require.ElementsMatch(t, tt.wantResultingCache[i].GetScopes(), actualIDP.GetScopes())
// We always want to use the proxy from env on these clients, so although the following assertions
@ -1083,18 +1239,30 @@ func newTestIssuer(t *testing.T) (string, string) {
caBundlePEM, testURL := testutil.TLSTestServer(t, mux.ServeHTTP)
type providerJSON struct {
Issuer string `json:"issuer"`
AuthURL string `json:"authorization_endpoint"`
TokenURL string `json:"token_endpoint"`
JWKSURL string `json:"jwks_uri"`
Issuer string `json:"issuer"`
AuthURL string `json:"authorization_endpoint"`
TokenURL string `json:"token_endpoint"`
RevocationURL string `json:"revocation_endpoint,omitempty"`
JWKSURL string `json:"jwks_uri"`
}
// At the root of the server, serve an issuer with a valid discovery response.
mux.HandleFunc("/.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,
AuthURL: "https://example.com/authorize",
Issuer: testURL,
AuthURL: "https://example.com/authorize",
RevocationURL: "https://example.com/revoke",
})
})
// At "/valid-without-revocation", serve an issuer with a valid discovery response which does not have a revocation endpoint.
mux.HandleFunc("/valid-without-revocation/.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 + "/valid-without-revocation",
AuthURL: "https://example.com/authorize",
RevocationURL: "", // none
})
})
@ -1107,6 +1275,16 @@ func newTestIssuer(t *testing.T) (string, string) {
})
})
// At "/invalid-revocation-url", serve an issuer that returns an invalid revocation URL (not parseable).
mux.HandleFunc("/invalid-revocation-url/.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 + "/invalid-revocation-url",
AuthURL: "https://example.com/authorize",
RevocationURL: "%",
})
})
// At "/insecure", serve an issuer that returns an insecure authorization URL (not https://).
mux.HandleFunc("/insecure/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("content-type", "application/json")
@ -1116,6 +1294,16 @@ func newTestIssuer(t *testing.T) (string, string) {
})
})
// At "/insecure", serve an issuer that returns an insecure authorization URL (not https://).
mux.HandleFunc("/insecure-revocation-url/.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 + "/insecure-revocation-url",
AuthURL: "https://example.com/authorize",
RevocationURL: "http://example.com/revoke",
})
})
// handle the four issuer with trailing slash configs
// valid case in= out=
@ -1125,8 +1313,9 @@ func newTestIssuer(t *testing.T) (string, string) {
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",
Issuer: testURL + "/ends-with-slash/",
AuthURL: "https://example.com/authorize",
RevocationURL: "https://example.com/revoke",
})
})

176
internal/controller/supervisorstorage/garbage_collector.go
View File

@ -4,31 +4,50 @@
package supervisorstorage
import (
"context"
"errors"
"fmt"
"time"
coreosoidc "github.com/coreos/go-oidc/v3/oidc"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/util/clock"
corev1informers "k8s.io/client-go/informers/core/v1"
"k8s.io/client-go/kubernetes"
"k8s.io/utils/strings/slices"
pinnipedcontroller "go.pinniped.dev/internal/controller"
"go.pinniped.dev/internal/controllerlib"
"go.pinniped.dev/internal/crud"
"go.pinniped.dev/internal/fositestorage/accesstoken"
"go.pinniped.dev/internal/fositestorage/authorizationcode"
"go.pinniped.dev/internal/fositestorage/openidconnect"
"go.pinniped.dev/internal/fositestorage/pkce"
"go.pinniped.dev/internal/fositestorage/refreshtoken"
"go.pinniped.dev/internal/oidc/provider"
"go.pinniped.dev/internal/plog"
"go.pinniped.dev/internal/psession"
)
const minimumRepeatInterval = 30 * time.Second
type garbageCollectorController struct {
idpCache UpstreamOIDCIdentityProviderICache
secretInformer corev1informers.SecretInformer
kubeClient kubernetes.Interface
clock clock.Clock
timeOfMostRecentSweep time.Time
}
// UpstreamOIDCIdentityProviderICache is a thread safe cache that holds a list of validated upstream OIDC IDP configurations.
type UpstreamOIDCIdentityProviderICache interface {
GetOIDCIdentityProviders() []provider.UpstreamOIDCIdentityProviderI
}
func GarbageCollectorController(
idpCache UpstreamOIDCIdentityProviderICache,
clock clock.Clock,
kubeClient kubernetes.Interface,
secretInformer corev1informers.SecretInformer,
@ -46,6 +65,7 @@ func GarbageCollectorController(
controllerlib.Config{
Name: "garbage-collector-controller",
Syncer: &garbageCollectorController{
idpCache: idpCache,
secretInformer: secretInformer,
kubeClient: kubeClient,
clock: clock,
@ -97,33 +117,163 @@ func (c *garbageCollectorController) Sync(ctx controllerlib.Context) error {
garbageCollectAfterTime, err := time.Parse(crud.SecretLifetimeAnnotationDateFormat, timeString)
if err != nil {
plog.WarningErr("could not parse resource timestamp for garbage collection", err, logKV(secret))
plog.WarningErr("could not parse resource timestamp for garbage collection", err, logKV(secret)...)
continue
}
if garbageCollectAfterTime.Before(frozenClock.Now()) {
err = c.kubeClient.CoreV1().Secrets(secret.Namespace).Delete(ctx.Context, secret.Name, metav1.DeleteOptions{
Preconditions: &metav1.Preconditions{
UID: &secret.UID,
ResourceVersion: &secret.ResourceVersion,
},
})
if err != nil {
plog.WarningErr("failed to garbage collect resource", err, logKV(secret))
continue
}
plog.Info("storage garbage collector deleted resource", logKV(secret))
if !garbageCollectAfterTime.Before(frozenClock.Now()) {
// not old enough yet
continue
}
storageType, isSessionStorage := secret.Labels[crud.SecretLabelKey]
if isSessionStorage {
err := c.maybeRevokeUpstreamOIDCRefreshToken(ctx.Context, storageType, secret)
if err != nil {
plog.WarningErr("garbage collector could not revoke upstream refresh token", err, logKV(secret)...)
// If the error is of a type that is worth retrying, then do not delete the Secret right away.
// A future call to Sync will try revocation again for that secret. However, if the Secret is
// getting too old, then just delete it anyway. We don't want to extend the lifetime of these
// session Secrets by too much time, since the garbage collector is the only thing that is
// cleaning them out of etcd storage.
fourHoursAgo := frozenClock.Now().Add(-4 * time.Hour)
nowIsLessThanFourHoursBeyondSecretGCTime := garbageCollectAfterTime.After(fourHoursAgo)
if errors.As(err, &retryableRevocationError{}) && nowIsLessThanFourHoursBeyondSecretGCTime {
// Hasn't been very long since secret expired, so skip deletion to try revocation again later.
continue
}
}
}
err = c.kubeClient.CoreV1().Secrets(secret.Namespace).Delete(ctx.Context, secret.Name, metav1.DeleteOptions{
Preconditions: &metav1.Preconditions{
UID: &secret.UID,
ResourceVersion: &secret.ResourceVersion,
},
})
if err != nil {
plog.WarningErr("failed to garbage collect resource", err, logKV(secret)...)
continue
}
plog.Info("storage garbage collector deleted resource", logKV(secret)...)
}
return nil
}
func (c *garbageCollectorController) maybeRevokeUpstreamOIDCRefreshToken(ctx context.Context, storageType string, secret *v1.Secret) error {
// All session storage types hold upstream refresh tokens when the upstream IDP is an OIDC provider.
// However, some of them will be outdated because they are not updated by fosite after creation.
// Our goal below is to always revoke the latest upstream refresh token that we are holding for the
// session, and only the latest.
switch storageType {
case authorizationcode.TypeLabelValue:
authorizeCodeSession, err := authorizationcode.ReadFromSecret(secret)
if err != nil {
return err
}
// Check if this downstream authcode was already used. If it was already used (i.e. not active anymore), then
// the latest upstream refresh token can be found in one of the other storage types handled below instead.
if !authorizeCodeSession.Active {
return nil
}
// When the downstream authcode was never used, then its storage must contain the latest upstream refresh token.
return c.revokeUpstreamOIDCRefreshToken(ctx, authorizeCodeSession.Request.Session.(*psession.PinnipedSession).Custom, secret)
case accesstoken.TypeLabelValue:
// For access token storage, check if the "offline_access" scope was granted on the downstream session.
// If it was granted, then the latest upstream refresh token should be found in the refresh token storage instead.
// If it was not granted, then the user could not possibly have performed a downstream refresh, so the
// access token storage has the latest version of the upstream refresh token.
accessTokenSession, err := accesstoken.ReadFromSecret(secret)
if err != nil {
return err
}
pinnipedSession := accessTokenSession.Request.Session.(*psession.PinnipedSession)
if slices.Contains(accessTokenSession.Request.GetGrantedScopes(), coreosoidc.ScopeOfflineAccess) {
return nil
}
return c.revokeUpstreamOIDCRefreshToken(ctx, pinnipedSession.Custom, secret)
case refreshtoken.TypeLabelValue:
// For refresh token storage, always revoke its upstream refresh token. This refresh token storage could
// be the result of the initial downstream authcode exchange, or it could be the result of a downstream
// refresh. Either way, it always contains the latest upstream refresh token when it exists.
refreshTokenSession, err := refreshtoken.ReadFromSecret(secret)
if err != nil {
return err
}
return c.revokeUpstreamOIDCRefreshToken(ctx, refreshTokenSession.Request.Session.(*psession.PinnipedSession).Custom, secret)
case pkce.TypeLabelValue:
// For PKCE storage, its very existence means that the authcode was never exchanged, because these
// are deleted during authcode exchange. No need to do anything, since the upstream refresh token
// revocation is handled by authcode storage case above.
return nil
case openidconnect.TypeLabelValue:
// For OIDC storage, there is no need to do anything for reasons similar to the PKCE storage.
// These are not deleted during authcode exchange, probably due to a bug in fosite, even though it
// will never be read or updated again. However, the refresh token contained inside will be revoked
// by one of the other cases above.
return nil
default:
// There are no other storage types, so this should never happen in practice.
return errors.New("garbage collector saw invalid label on Secret when trying to determine if upstream revocation was needed")
}
}
func (c *garbageCollectorController) revokeUpstreamOIDCRefreshToken(ctx context.Context, customSessionData *psession.CustomSessionData, secret *v1.Secret) error {
// When session was for another upstream IDP type, e.g. LDAP, there is no upstream OIDC refresh token involved.
if customSessionData.ProviderType != psession.ProviderTypeOIDC {
return nil
}
// Try to find the provider that was originally used to create the stored session.
var foundOIDCIdentityProviderI provider.UpstreamOIDCIdentityProviderI
for _, p := range c.idpCache.GetOIDCIdentityProviders() {
if p.GetName() == customSessionData.ProviderName && p.GetResourceUID() == customSessionData.ProviderUID {
foundOIDCIdentityProviderI = p
break
}
}
if foundOIDCIdentityProviderI == nil {
return fmt.Errorf("could not find upstream OIDC provider named %q with resource UID %q", customSessionData.ProviderName, customSessionData.ProviderUID)
}
// Revoke the upstream refresh token. This is a noop if the upstream provider does not offer a revocation endpoint.
err := foundOIDCIdentityProviderI.RevokeRefreshToken(ctx, customSessionData.OIDC.UpstreamRefreshToken)
if err != nil {
// This could be a network failure, a 503 result which we should retry
// (see https://datatracker.ietf.org/doc/html/rfc7009#section-2.2.1),
// or any other non-200 response from the revocation endpoint.
// Regardless of which, it is probably worth retrying.
return retryableRevocationError{wrapped: err}
}
plog.Trace("garbage collector successfully revoked upstream OIDC refresh token (or provider has no revocation endpoint)", logKV(secret)...)
return nil
}
type retryableRevocationError struct {
wrapped error
}
func (e retryableRevocationError) Error() string {
return fmt.Sprintf("retryable revocation error: %v", e.wrapped)
}
func (e retryableRevocationError) Unwrap() error {
return e.wrapped
}
func logKV(secret *v1.Secret) []interface{} {
return []interface{}{
"secretName", secret.Name,
"secretNamespace", secret.Namespace,
"secretType", string(secret.Type),
"garbageCollectAfter", secret.Annotations[crud.SecretLifetimeAnnotationKey],
"storageTypeLabelValue", secret.Labels[crud.SecretLabelKey],
}
}

743
internal/controller/supervisorstorage/garbage_collector_test.go
View File

@ -5,10 +5,12 @@ package supervisorstorage
import (
"context"
"encoding/json"
"errors"
"testing"
"time"
"github.com/ory/fosite"
"github.com/sclevine/spec"
"github.com/sclevine/spec/report"
"github.com/stretchr/testify/require"
@ -23,7 +25,14 @@ import (
kubetesting "k8s.io/client-go/testing"
"go.pinniped.dev/internal/controllerlib"
"go.pinniped.dev/internal/fositestorage/accesstoken"
"go.pinniped.dev/internal/fositestorage/authorizationcode"
"go.pinniped.dev/internal/fositestorage/refreshtoken"
"go.pinniped.dev/internal/oidc/clientregistry"
"go.pinniped.dev/internal/oidc/provider"
"go.pinniped.dev/internal/psession"
"go.pinniped.dev/internal/testutil"
"go.pinniped.dev/internal/testutil/oidctestutil"
)
func TestGarbageCollectorControllerInformerFilters(t *testing.T) {
@ -39,6 +48,7 @@ func TestGarbageCollectorControllerInformerFilters(t *testing.T) {
observableWithInformerOption = testutil.NewObservableWithInformerOption()
secretsInformer := kubeinformers.NewSharedInformerFactory(nil, 0).Core().V1().Secrets()
_ = GarbageCollectorController(
nil,
clock.RealClock{},
nil,
secretsInformer,
@ -129,9 +139,10 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
// Defer starting the informers until the last possible moment so that the
// nested Before's can keep adding things to the informer caches.
var startInformersAndController = func() {
var startInformersAndController = func(idpCache provider.DynamicUpstreamIDPProvider) {
// Set this at the last second to allow for injection of server override.
subject = GarbageCollectorController(
idpCache,
fakeClock,
deleteOptionsRecorder,
kubeInformers.Core().V1().Secrets(),
@ -183,7 +194,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
when("there are secrets without the garbage-collect-after annotation", func() {
it("does not delete those secrets", func() {
startInformersAndController()
startInformersAndController(nil)
r.NoError(controllerlib.TestSync(t, subject, *syncContext))
require.Empty(t, kubeClient.Actions())
@ -194,7 +205,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
})
})
when("there are secrets with the garbage-collect-after annotation", func() {
when("there are any secrets with the garbage-collect-after annotation", func() {
it.Before(func() {
firstExpiredSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
@ -236,7 +247,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
})
it("should delete any that are past their expiration", func() {
startInformersAndController()
startInformersAndController(nil)
r.NoError(controllerlib.TestSync(t, subject, *syncContext))
r.ElementsMatch(
@ -260,6 +271,724 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
})
})
when("there are valid, expired authcode secrets", func() {
it.Before(func() {
activeOIDCAuthcodeSession := &authorizationcode.Session{
Version: "2",
Active: true,
Request: &fosite.Request{
ID: "request-id-1",
Client: &clientregistry.Client{},
Session: &psession.PinnipedSession{
Custom: &psession.CustomSessionData{
ProviderUID: "upstream-oidc-provider-uid",
ProviderName: "upstream-oidc-provider-name",
ProviderType: psession.ProviderTypeOIDC,
OIDC: &psession.OIDCSessionData{
UpstreamRefreshToken: "fake-upstream-refresh-token",
},
},
},
},
}
activeOIDCAuthcodeSessionJSON, err := json.Marshal(activeOIDCAuthcodeSession)
r.NoError(err)
activeOIDCAuthcodeSessionSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "activeOIDCAuthcodeSession",
Namespace: installedInNamespace,
UID: "uid-123",
ResourceVersion: "rv-123",
Annotations: map[string]string{
"storage.pinniped.dev/garbage-collect-after": frozenNow.Add(-time.Second).Format(time.RFC3339),
},
Labels: map[string]string{
"storage.pinniped.dev/type": authorizationcode.TypeLabelValue,
},
},
Data: map[string][]byte{
"pinniped-storage-data": activeOIDCAuthcodeSessionJSON,
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/" + authorizationcode.TypeLabelValue,
}
_, err = authorizationcode.ReadFromSecret(activeOIDCAuthcodeSessionSecret)
r.NoError(err, "the test author accidentally formed an invalid authcode secret")
r.NoError(kubeInformerClient.Tracker().Add(activeOIDCAuthcodeSessionSecret))
r.NoError(kubeClient.Tracker().Add(activeOIDCAuthcodeSessionSecret))
inactiveOIDCAuthcodeSession := &authorizationcode.Session{
Version: "2",
Active: false,
Request: &fosite.Request{
ID: "request-id-2",
Client: &clientregistry.Client{},
Session: &psession.PinnipedSession{
Custom: &psession.CustomSessionData{
ProviderUID: "upstream-oidc-provider-uid",
ProviderName: "upstream-oidc-provider-name",
ProviderType: psession.ProviderTypeOIDC,
OIDC: &psession.OIDCSessionData{
UpstreamRefreshToken: "other-fake-upstream-refresh-token",
},
},
},
},
}
inactiveOIDCAuthcodeSessionJSON, err := json.Marshal(inactiveOIDCAuthcodeSession)
r.NoError(err)
inactiveOIDCAuthcodeSessionSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "inactiveOIDCAuthcodeSession",
Namespace: installedInNamespace,
UID: "uid-456",
ResourceVersion: "rv-456",
Annotations: map[string]string{
"storage.pinniped.dev/garbage-collect-after": frozenNow.Add(-time.Second).Format(time.RFC3339),
},
Labels: map[string]string{
"storage.pinniped.dev/type": authorizationcode.TypeLabelValue,
},
},
Data: map[string][]byte{
"pinniped-storage-data": inactiveOIDCAuthcodeSessionJSON,
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/" + authorizationcode.TypeLabelValue,
}
_, err = authorizationcode.ReadFromSecret(inactiveOIDCAuthcodeSessionSecret)
r.NoError(err, "the test author accidentally formed an invalid authcode secret")
r.NoError(kubeInformerClient.Tracker().Add(inactiveOIDCAuthcodeSessionSecret))
r.NoError(kubeClient.Tracker().Add(inactiveOIDCAuthcodeSessionSecret))
})
it("should revoke upstream tokens only from the active authcode secrets and delete them all", func() {
happyOIDCUpstream := oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().
WithName("upstream-oidc-provider-name").
WithResourceUID("upstream-oidc-provider-uid").
WithRevokeRefreshTokenError(nil)
idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
startInformersAndController(idpListerBuilder.Build())
r.NoError(controllerlib.TestSync(t, subject, *syncContext))
// The upstream refresh token is only revoked for the active authcode session.
idpListerBuilder.RequireExactlyOneCallToRevokeRefreshToken(t,
"upstream-oidc-provider-name",
&oidctestutil.RevokeRefreshTokenArgs{
Ctx: syncContext.Context,
RefreshToken: "fake-upstream-refresh-token",
},
)
// Both authcode session secrets are deleted.
r.ElementsMatch(
[]kubetesting.Action{
kubetesting.NewDeleteAction(secretsGVR, installedInNamespace, "activeOIDCAuthcodeSession"),
kubetesting.NewDeleteAction(secretsGVR, installedInNamespace, "inactiveOIDCAuthcodeSession"),
},
kubeClient.Actions(),
)
r.ElementsMatch(
[]metav1.DeleteOptions{
testutil.NewPreconditions("uid-123", "rv-123"),
testutil.NewPreconditions("uid-456", "rv-456"),
},
*deleteOptions,
)
})
})
when("there is an invalid, expired authcode secret", func() {
it.Before(func() {
invalidOIDCAuthcodeSession := &authorizationcode.Session{
Version: "2",
Active: true,
Request: &fosite.Request{
ID: "", // it is invalid for there to be a missing request ID
Client: &clientregistry.Client{},
Session: &psession.PinnipedSession{
Custom: &psession.CustomSessionData{
ProviderUID: "upstream-oidc-provider-uid",
ProviderName: "upstream-oidc-provider-name",
ProviderType: psession.ProviderTypeOIDC,
OIDC: &psession.OIDCSessionData{
UpstreamRefreshToken: "fake-upstream-refresh-token",
},
},
},
},
}
invalidOIDCAuthcodeSessionJSON, err := json.Marshal(invalidOIDCAuthcodeSession)
r.NoError(err)
invalidOIDCAuthcodeSessionSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "invalidOIDCAuthcodeSession",
Namespace: installedInNamespace,
UID: "uid-123",
ResourceVersion: "rv-123",
Annotations: map[string]string{
"storage.pinniped.dev/garbage-collect-after": frozenNow.Add(-time.Second).Format(time.RFC3339),
},
Labels: map[string]string{
"storage.pinniped.dev/type": authorizationcode.TypeLabelValue,
},
},
Data: map[string][]byte{
"pinniped-storage-data": invalidOIDCAuthcodeSessionJSON,
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/" + authorizationcode.TypeLabelValue,
}
r.NoError(kubeInformerClient.Tracker().Add(invalidOIDCAuthcodeSessionSecret))
r.NoError(kubeClient.Tracker().Add(invalidOIDCAuthcodeSessionSecret))
})
it("should remove the secret without revoking any upstream tokens", func() {
happyOIDCUpstream := oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().
WithName("upstream-oidc-provider-name").
WithResourceUID("upstream-oidc-provider-uid").
WithRevokeRefreshTokenError(nil)
idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
startInformersAndController(idpListerBuilder.Build())
r.NoError(controllerlib.TestSync(t, subject, *syncContext))
// Nothing to revoke since we couldn't read the invalid secret.
idpListerBuilder.RequireExactlyZeroCallsToRevokeRefreshToken(t)
// The invalid authcode session secrets is still deleted because it is expired.
r.ElementsMatch(
[]kubetesting.Action{
kubetesting.NewDeleteAction(secretsGVR, installedInNamespace, "invalidOIDCAuthcodeSession"),
},
kubeClient.Actions(),
)
r.ElementsMatch(
[]metav1.DeleteOptions{
testutil.NewPreconditions("uid-123", "rv-123"),
},
*deleteOptions,
)
})
})
when("there is a valid, expired authcode secret but its upstream name does not match any existing upstream", func() {
it.Before(func() {
wrongProviderNameOIDCAuthcodeSession := &authorizationcode.Session{
Version: "2",
Active: true,
Request: &fosite.Request{
ID: "request-id-1",
Client: &clientregistry.Client{},
Session: &psession.PinnipedSession{
Custom: &psession.CustomSessionData{
ProviderUID: "upstream-oidc-provider-uid",
ProviderName: "upstream-oidc-provider-name-will-not-match",
ProviderType: psession.ProviderTypeOIDC,
OIDC: &psession.OIDCSessionData{
UpstreamRefreshToken: "fake-upstream-refresh-token",
},
},
},
},
}
wrongProviderNameOIDCAuthcodeSessionJSON, err := json.Marshal(wrongProviderNameOIDCAuthcodeSession)
r.NoError(err)
wrongProviderNameOIDCAuthcodeSessionSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "wrongProviderNameOIDCAuthcodeSession",
Namespace: installedInNamespace,
UID: "uid-123",
ResourceVersion: "rv-123",
Annotations: map[string]string{
"storage.pinniped.dev/garbage-collect-after": frozenNow.Add(-time.Second).Format(time.RFC3339),
},
Labels: map[string]string{
"storage.pinniped.dev/type": authorizationcode.TypeLabelValue,
},
},
Data: map[string][]byte{
"pinniped-storage-data": wrongProviderNameOIDCAuthcodeSessionJSON,
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/" + authorizationcode.TypeLabelValue,
}
_, err = authorizationcode.ReadFromSecret(wrongProviderNameOIDCAuthcodeSessionSecret)
r.NoError(err, "the test author accidentally formed an invalid authcode secret")
r.NoError(kubeInformerClient.Tracker().Add(wrongProviderNameOIDCAuthcodeSessionSecret))
r.NoError(kubeClient.Tracker().Add(wrongProviderNameOIDCAuthcodeSessionSecret))
})
it("should remove the secret without revoking any upstream tokens", func() {
happyOIDCUpstream := oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().
WithName("upstream-oidc-provider-name").
WithResourceUID("upstream-oidc-provider-uid").
WithRevokeRefreshTokenError(nil)
idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
startInformersAndController(idpListerBuilder.Build())
r.NoError(controllerlib.TestSync(t, subject, *syncContext))
// Nothing to revoke since we couldn't find the upstream in the cache.
idpListerBuilder.RequireExactlyZeroCallsToRevokeRefreshToken(t)
// The authcode session secrets is still deleted because it is expired.
r.ElementsMatch(
[]kubetesting.Action{
kubetesting.NewDeleteAction(secretsGVR, installedInNamespace, "wrongProviderNameOIDCAuthcodeSession"),
},
kubeClient.Actions(),
)
r.ElementsMatch(
[]metav1.DeleteOptions{
testutil.NewPreconditions("uid-123", "rv-123"),
},
*deleteOptions,
)
})
})
when("there is a valid, expired authcode secret but its upstream UID does not match any existing upstream", func() {
it.Before(func() {
wrongProviderNameOIDCAuthcodeSession := &authorizationcode.Session{
Version: "2",
Active: true,
Request: &fosite.Request{
ID: "request-id-1",
Client: &clientregistry.Client{},
Session: &psession.PinnipedSession{
Custom: &psession.CustomSessionData{
ProviderUID: "upstream-oidc-provider-uid-will-not-match",
ProviderName: "upstream-oidc-provider-name",
ProviderType: psession.ProviderTypeOIDC,
OIDC: &psession.OIDCSessionData{
UpstreamRefreshToken: "fake-upstream-refresh-token",
},
},
},
},
}
wrongProviderNameOIDCAuthcodeSessionJSON, err := json.Marshal(wrongProviderNameOIDCAuthcodeSession)
r.NoError(err)
wrongProviderNameOIDCAuthcodeSessionSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "wrongProviderNameOIDCAuthcodeSession",
Namespace: installedInNamespace,
UID: "uid-123",
ResourceVersion: "rv-123",
Annotations: map[string]string{
"storage.pinniped.dev/garbage-collect-after": frozenNow.Add(-time.Second).Format(time.RFC3339),
},
Labels: map[string]string{
"storage.pinniped.dev/type": authorizationcode.TypeLabelValue,
},
},
Data: map[string][]byte{
"pinniped-storage-data": wrongProviderNameOIDCAuthcodeSessionJSON,
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/" + authorizationcode.TypeLabelValue,
}
_, err = authorizationcode.ReadFromSecret(wrongProviderNameOIDCAuthcodeSessionSecret)
r.NoError(err, "the test author accidentally formed an invalid authcode secret")
r.NoError(kubeInformerClient.Tracker().Add(wrongProviderNameOIDCAuthcodeSessionSecret))
r.NoError(kubeClient.Tracker().Add(wrongProviderNameOIDCAuthcodeSessionSecret))
})
it("should remove the secret without revoking any upstream tokens", func() {
happyOIDCUpstream := oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().
WithName("upstream-oidc-provider-name").
WithResourceUID("upstream-oidc-provider-uid").
WithRevokeRefreshTokenError(nil)
idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
startInformersAndController(idpListerBuilder.Build())
r.NoError(controllerlib.TestSync(t, subject, *syncContext))
// Nothing to revoke since we couldn't find the upstream in the cache.
idpListerBuilder.RequireExactlyZeroCallsToRevokeRefreshToken(t)
// The authcode session secrets is still deleted because it is expired.
r.ElementsMatch(
[]kubetesting.Action{
kubetesting.NewDeleteAction(secretsGVR, installedInNamespace, "wrongProviderNameOIDCAuthcodeSession"),
},
kubeClient.Actions(),
)
r.ElementsMatch(
[]metav1.DeleteOptions{
testutil.NewPreconditions("uid-123", "rv-123"),
},
*deleteOptions,
)
})
})
when("there is a valid, recently expired authcode secret but the upstream revocation fails", func() {
it.Before(func() {
activeOIDCAuthcodeSession := &authorizationcode.Session{
Version: "2",
Active: true,
Request: &fosite.Request{
ID: "request-id-1",
Client: &clientregistry.Client{},
Session: &psession.PinnipedSession{
Custom: &psession.CustomSessionData{
ProviderUID: "upstream-oidc-provider-uid",
ProviderName: "upstream-oidc-provider-name",
ProviderType: psession.ProviderTypeOIDC,
OIDC: &psession.OIDCSessionData{
UpstreamRefreshToken: "fake-upstream-refresh-token",
},
},
},
},
}
activeOIDCAuthcodeSessionJSON, err := json.Marshal(activeOIDCAuthcodeSession)
r.NoError(err)
activeOIDCAuthcodeSessionSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "activeOIDCAuthcodeSession",
Namespace: installedInNamespace,
UID: "uid-123",
ResourceVersion: "rv-123",
Annotations: map[string]string{
// expired almost 4 hours ago, but not quite 4 hours
"storage.pinniped.dev/garbage-collect-after": frozenNow.Add((-time.Hour * 4) + time.Second).Format(time.RFC3339),
},
Labels: map[string]string{
"storage.pinniped.dev/type": authorizationcode.TypeLabelValue,
},
},
Data: map[string][]byte{
"pinniped-storage-data": activeOIDCAuthcodeSessionJSON,
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/" + authorizationcode.TypeLabelValue,
}
_, err = authorizationcode.ReadFromSecret(activeOIDCAuthcodeSessionSecret)
r.NoError(err, "the test author accidentally formed an invalid authcode secret")
r.NoError(kubeInformerClient.Tracker().Add(activeOIDCAuthcodeSessionSecret))
r.NoError(kubeClient.Tracker().Add(activeOIDCAuthcodeSessionSecret))
})
it("keeps the secret for a while longer so the revocation can be retried on a future sync", func() {
happyOIDCUpstream := oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().
WithName("upstream-oidc-provider-name").
WithResourceUID("upstream-oidc-provider-uid").
WithRevokeRefreshTokenError(errors.New("some upstream revocation error")) // the upstream revocation will fail
idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
startInformersAndController(idpListerBuilder.Build())
r.NoError(controllerlib.TestSync(t, subject, *syncContext))
// Tried to revoke it, although this revocation will fail.
idpListerBuilder.RequireExactlyOneCallToRevokeRefreshToken(t,
"upstream-oidc-provider-name",
&oidctestutil.RevokeRefreshTokenArgs{
Ctx: syncContext.Context,
RefreshToken: "fake-upstream-refresh-token",
},
)
// The authcode session secrets is not deleted.
r.Empty(kubeClient.Actions())
})
})
when("there is a valid, long-since expired authcode secret but the upstream revocation fails", func() {
it.Before(func() {
activeOIDCAuthcodeSession := &authorizationcode.Session{
Version: "2",
Active: true,
Request: &fosite.Request{
ID: "request-id-1",
Client: &clientregistry.Client{},
Session: &psession.PinnipedSession{
Custom: &psession.CustomSessionData{
ProviderUID: "upstream-oidc-provider-uid",
ProviderName: "upstream-oidc-provider-name",
ProviderType: psession.ProviderTypeOIDC,
OIDC: &psession.OIDCSessionData{
UpstreamRefreshToken: "fake-upstream-refresh-token",
},
},
},
},
}
activeOIDCAuthcodeSessionJSON, err := json.Marshal(activeOIDCAuthcodeSession)
r.NoError(err)
activeOIDCAuthcodeSessionSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "activeOIDCAuthcodeSession",
Namespace: installedInNamespace,
UID: "uid-123",
ResourceVersion: "rv-123",
Annotations: map[string]string{
// expired just over 4 hours ago
"storage.pinniped.dev/garbage-collect-after": frozenNow.Add((-time.Hour * 4) - time.Second).Format(time.RFC3339),
},
Labels: map[string]string{
"storage.pinniped.dev/type": authorizationcode.TypeLabelValue,
},
},
Data: map[string][]byte{
"pinniped-storage-data": activeOIDCAuthcodeSessionJSON,
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/" + authorizationcode.TypeLabelValue,
}
_, err = authorizationcode.ReadFromSecret(activeOIDCAuthcodeSessionSecret)
r.NoError(err, "the test author accidentally formed an invalid authcode secret")
r.NoError(kubeInformerClient.Tracker().Add(activeOIDCAuthcodeSessionSecret))
r.NoError(kubeClient.Tracker().Add(activeOIDCAuthcodeSessionSecret))
})
it("deletes the secret because it has probably been retrying revocation for hours without success", func() {
happyOIDCUpstream := oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().
WithName("upstream-oidc-provider-name").
WithResourceUID("upstream-oidc-provider-uid").
WithRevokeRefreshTokenError(errors.New("some upstream revocation error")) // the upstream revocation will fail
idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
startInformersAndController(idpListerBuilder.Build())
r.NoError(controllerlib.TestSync(t, subject, *syncContext))
// Tried to revoke it, although this revocation will fail.
idpListerBuilder.RequireExactlyOneCallToRevokeRefreshToken(t,
"upstream-oidc-provider-name",
&oidctestutil.RevokeRefreshTokenArgs{
Ctx: syncContext.Context,
RefreshToken: "fake-upstream-refresh-token",
},
)
// The authcode session secrets is deleted.
r.ElementsMatch(
[]kubetesting.Action{
kubetesting.NewDeleteAction(secretsGVR, installedInNamespace, "activeOIDCAuthcodeSession"),
},
kubeClient.Actions(),
)
r.ElementsMatch(
[]metav1.DeleteOptions{
testutil.NewPreconditions("uid-123", "rv-123"),
},
*deleteOptions,
)
})
})
when("there are valid, expired access token secrets", func() {
it.Before(func() {
offlineAccessGrantedOIDCAccessTokenSession := &accesstoken.Session{
Version: "2",
Request: &fosite.Request{
GrantedScope: fosite.Arguments{"scope1", "scope2", "offline_access"},
ID: "request-id-1",
Client: &clientregistry.Client{},
Session: &psession.PinnipedSession{
Custom: &psession.CustomSessionData{
ProviderUID: "upstream-oidc-provider-uid",
ProviderName: "upstream-oidc-provider-name",
ProviderType: psession.ProviderTypeOIDC,
OIDC: &psession.OIDCSessionData{
UpstreamRefreshToken: "offline-access-granted-fake-upstream-refresh-token",
},
},
},
},
}
offlineAccessGrantedOIDCAccessTokenSessionJSON, err := json.Marshal(offlineAccessGrantedOIDCAccessTokenSession)
r.NoError(err)
offlineAccessGrantedOIDCAccessTokenSessionSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "offlineAccessGrantedOIDCAccessTokenSession",
Namespace: installedInNamespace,
UID: "uid-123",
ResourceVersion: "rv-123",
Annotations: map[string]string{
"storage.pinniped.dev/garbage-collect-after": frozenNow.Add(-time.Second).Format(time.RFC3339),
},
Labels: map[string]string{
"storage.pinniped.dev/type": accesstoken.TypeLabelValue,
},
},
Data: map[string][]byte{
"pinniped-storage-data": offlineAccessGrantedOIDCAccessTokenSessionJSON,
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/" + accesstoken.TypeLabelValue,
}
_, err = accesstoken.ReadFromSecret(offlineAccessGrantedOIDCAccessTokenSessionSecret)
r.NoError(err, "the test author accidentally formed an invalid accesstoken secret")
r.NoError(kubeInformerClient.Tracker().Add(offlineAccessGrantedOIDCAccessTokenSessionSecret))
r.NoError(kubeClient.Tracker().Add(offlineAccessGrantedOIDCAccessTokenSessionSecret))
offlineAccessNotGrantedOIDCAccessTokenSession := &accesstoken.Session{
Version: "2",
Request: &fosite.Request{
GrantedScope: fosite.Arguments{"scope1", "scope2"},
ID: "request-id-2",
Client: &clientregistry.Client{},
Session: &psession.PinnipedSession{
Custom: &psession.CustomSessionData{
ProviderUID: "upstream-oidc-provider-uid",
ProviderName: "upstream-oidc-provider-name",
ProviderType: psession.ProviderTypeOIDC,
OIDC: &psession.OIDCSessionData{
UpstreamRefreshToken: "fake-upstream-refresh-token",
},
},
},
},
}
offlineAccessNotGrantedOIDCAccessTokenSessionJSON, err := json.Marshal(offlineAccessNotGrantedOIDCAccessTokenSession)
r.NoError(err)
offlineAccessNotGrantedOIDCAccessTokenSessionSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "offlineAccessNotGrantedOIDCAccessTokenSession",
Namespace: installedInNamespace,
UID: "uid-456",
ResourceVersion: "rv-456",
Annotations: map[string]string{
"storage.pinniped.dev/garbage-collect-after": frozenNow.Add(-time.Second).Format(time.RFC3339),
},
Labels: map[string]string{
"storage.pinniped.dev/type": accesstoken.TypeLabelValue,
},
},
Data: map[string][]byte{
"pinniped-storage-data": offlineAccessNotGrantedOIDCAccessTokenSessionJSON,
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/" + accesstoken.TypeLabelValue,
}
_, err = accesstoken.ReadFromSecret(offlineAccessNotGrantedOIDCAccessTokenSessionSecret)
r.NoError(err, "the test author accidentally formed an invalid accesstoken secret")
r.NoError(kubeInformerClient.Tracker().Add(offlineAccessNotGrantedOIDCAccessTokenSessionSecret))
r.NoError(kubeClient.Tracker().Add(offlineAccessNotGrantedOIDCAccessTokenSessionSecret))
})
it("should revoke upstream tokens only from the active authcode secrets and delete them all", func() {
happyOIDCUpstream := oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().
WithName("upstream-oidc-provider-name").
WithResourceUID("upstream-oidc-provider-uid").
WithRevokeRefreshTokenError(nil)
idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
startInformersAndController(idpListerBuilder.Build())
r.NoError(controllerlib.TestSync(t, subject, *syncContext))
// The upstream refresh token is only revoked for the downstream session which had offline_access granted.
idpListerBuilder.RequireExactlyOneCallToRevokeRefreshToken(t,
"upstream-oidc-provider-name",
&oidctestutil.RevokeRefreshTokenArgs{
Ctx: syncContext.Context,
RefreshToken: "fake-upstream-refresh-token",
},
)
// Both session secrets are deleted.
r.ElementsMatch(
[]kubetesting.Action{
kubetesting.NewDeleteAction(secretsGVR, installedInNamespace, "offlineAccessGrantedOIDCAccessTokenSession"),
kubetesting.NewDeleteAction(secretsGVR, installedInNamespace, "offlineAccessNotGrantedOIDCAccessTokenSession"),
},
kubeClient.Actions(),
)
r.ElementsMatch(
[]metav1.DeleteOptions{
testutil.NewPreconditions("uid-123", "rv-123"),
testutil.NewPreconditions("uid-456", "rv-456"),
},
*deleteOptions,
)
})
})
when("there are valid, expired refresh secrets", func() {
it.Before(func() {
oidcRefreshSession := &refreshtoken.Session{
Version: "2",
Request: &fosite.Request{
ID: "request-id-1",
Client: &clientregistry.Client{},
Session: &psession.PinnipedSession{
Custom: &psession.CustomSessionData{
ProviderUID: "upstream-oidc-provider-uid",
ProviderName: "upstream-oidc-provider-name",
ProviderType: psession.ProviderTypeOIDC,
OIDC: &psession.OIDCSessionData{
UpstreamRefreshToken: "fake-upstream-refresh-token",
},
},
},
},
}
oidcRefreshSessionJSON, err := json.Marshal(oidcRefreshSession)
r.NoError(err)
oidcRefreshSessionSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "oidcRefreshSession",
Namespace: installedInNamespace,
UID: "uid-123",
ResourceVersion: "rv-123",
Annotations: map[string]string{
"storage.pinniped.dev/garbage-collect-after": frozenNow.Add(-time.Second).Format(time.RFC3339),
},
Labels: map[string]string{
"storage.pinniped.dev/type": refreshtoken.TypeLabelValue,
},
},
Data: map[string][]byte{
"pinniped-storage-data": oidcRefreshSessionJSON,
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/" + refreshtoken.TypeLabelValue,
}
_, err = refreshtoken.ReadFromSecret(oidcRefreshSessionSecret)
r.NoError(err, "the test author accidentally formed an invalid refresh token secret")
r.NoError(kubeInformerClient.Tracker().Add(oidcRefreshSessionSecret))
r.NoError(kubeClient.Tracker().Add(oidcRefreshSessionSecret))
})
it("should revoke upstream tokens from the secrets and delete them all", func() {
happyOIDCUpstream := oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().
WithName("upstream-oidc-provider-name").
WithResourceUID("upstream-oidc-provider-uid").
WithRevokeRefreshTokenError(nil)
idpListerBuilder := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream.Build())
startInformersAndController(idpListerBuilder.Build())
r.NoError(controllerlib.TestSync(t, subject, *syncContext))
// The upstream refresh token is revoked.
idpListerBuilder.RequireExactlyOneCallToRevokeRefreshToken(t,
"upstream-oidc-provider-name",
&oidctestutil.RevokeRefreshTokenArgs{
Ctx: syncContext.Context,
RefreshToken: "fake-upstream-refresh-token",
},
)
// The secret is deleted.
r.ElementsMatch(
[]kubetesting.Action{
kubetesting.NewDeleteAction(secretsGVR, installedInNamespace, "oidcRefreshSession"),
},
kubeClient.Actions(),
)
r.ElementsMatch(
[]metav1.DeleteOptions{
testutil.NewPreconditions("uid-123", "rv-123"),
},
*deleteOptions,
)
})
})
when("very little time has passed since the previous sync call", func() {
it.Before(func() {
// Add a secret that will expire in 20 seconds.
@ -277,7 +1006,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
})
it("should do nothing to avoid being super chatty since it is called for every change to any Secret, until more time has passed", func() {
startInformersAndController()
startInformersAndController(nil)
require.Empty(t, kubeClient.Actions())
// Run sync once with the current time set to frozenTime.
@ -342,7 +1071,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
})
it("does not delete that secret", func() {
startInformersAndController()
startInformersAndController(nil)
r.NoError(controllerlib.TestSync(t, subject, *syncContext))
r.ElementsMatch(
@ -391,7 +1120,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
})
it("ignores the error and continues on to delete the next expired Secret", func() {
startInformersAndController()
startInformersAndController(nil)
r.NoError(controllerlib.TestSync(t, subject, *syncContext))
r.ElementsMatch(

76
internal/crud/crud.go
View File

@ -51,22 +51,20 @@ type JSON interface{} // document that we need valid JSON types
func New(resource string, secrets corev1client.SecretInterface, clock func() time.Time, lifetime time.Duration) Storage {
return &secretsStorage{
resource: resource,
secretType: corev1.SecretType(fmt.Sprintf(secretTypeFormat, resource)),
secretVersion: []byte(secretVersion),
secrets: secrets,
clock: clock,
lifetime: lifetime,
resource: resource,
secretType: secretType(resource),
secrets: secrets,
clock: clock,
lifetime: lifetime,
}
}
type secretsStorage struct {
resource string
secretType corev1.SecretType
secretVersion []byte
secrets corev1client.SecretInterface
clock func() time.Time
lifetime time.Duration
resource string
secretType corev1.SecretType
secrets corev1client.SecretInterface
clock func() time.Time
lifetime time.Duration
}
func (s *secretsStorage) Create(ctx context.Context, signature string, data JSON, additionalLabels map[string]string) (string, error) {
@ -86,28 +84,14 @@ func (s *secretsStorage) Get(ctx context.Context, signature string, data JSON) (
if err != nil {
return "", fmt.Errorf("failed to get %s for signature %s: %w", s.resource, signature, err)
}
if err := s.validateSecret(secret); err != nil {
return "", err
}
if err := json.Unmarshal(secret.Data[secretDataKey], data); err != nil {
return "", fmt.Errorf("failed to decode %s for signature %s: %w", s.resource, signature, err)
err = FromSecret(s.resource, secret, data)
if err != nil {
return "", fmt.Errorf("error during get for signature %s: %w", signature, err)
}
return secret.ResourceVersion, nil
}
func (s *secretsStorage) validateSecret(secret *corev1.Secret) error {
if secret.Type != s.secretType {
return fmt.Errorf("%w: %s must equal %s", ErrSecretTypeMismatch, secret.Type, s.secretType)
}
if labelResource := secret.Labels[SecretLabelKey]; labelResource != s.resource {
return fmt.Errorf("%w: %s must equal %s", ErrSecretLabelMismatch, labelResource, s.resource)
}
if !bytes.Equal(secret.Data[secretVersionKey], s.secretVersion) {
return ErrSecretVersionMismatch // TODO should this be fatal or not?
}
return nil
}
func (s *secretsStorage) Update(ctx context.Context, signature, resourceVersion string, data JSON) (string, error) {
// Note: There may be a small bug here in that toSecret will move the SecretLifetimeAnnotationKey date forward
// instead of keeping the storage resource's original SecretLifetimeAnnotationKey value. However, we only use
@ -154,6 +138,36 @@ func (s *secretsStorage) DeleteByLabel(ctx context.Context, labelName string, la
return nil
}
// FromSecret is similar to Get, but for when you already have a Secret in hand, e.g. from an informer.
// It validates and unmarshals the Secret. The data parameter is filled in as the result.
func FromSecret(resource string, secret *corev1.Secret, data JSON) error {
if err := validateSecret(resource, secret); err != nil {
return err
}
if err := json.Unmarshal(secret.Data[secretDataKey], data); err != nil {
return fmt.Errorf("failed to decode %s: %w", resource, err)
}
return nil
}
func secretType(resource string) corev1.SecretType {
return corev1.SecretType(fmt.Sprintf(secretTypeFormat, resource))
}
func validateSecret(resource string, secret *corev1.Secret) error {
secretType := corev1.SecretType(fmt.Sprintf(secretTypeFormat, resource))
if secret.Type != secretType {
return fmt.Errorf("%w: %s must equal %s", ErrSecretTypeMismatch, secret.Type, secretType)
}
if labelResource := secret.Labels[SecretLabelKey]; labelResource != resource {
return fmt.Errorf("%w: %s must equal %s", ErrSecretLabelMismatch, labelResource, resource)
}
if !bytes.Equal(secret.Data[secretVersionKey], []byte(secretVersion)) {
return ErrSecretVersionMismatch // TODO should this be fatal or not?
}
return nil
}
//nolint: gochecknoglobals
var b32 = base32.StdEncoding.WithPadding(base32.NoPadding)
@ -190,7 +204,7 @@ func (s *secretsStorage) toSecret(signature, resourceVersion string, data JSON,
},
Data: map[string][]byte{
secretDataKey: buf,
secretVersionKey: s.secretVersion,
secretVersionKey: []byte(secretVersion),
},
Type: s.secretType,
}, nil

166
internal/crud/crud_test.go
View File

@ -1,4 +1,4 @@
// Copyright 2020 the Pinniped contributors. All Rights Reserved.
// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package crud
@ -793,7 +793,8 @@ func TestStorage(t *testing.T) {
require.Empty(t, rv1)
require.Empty(t, out.Data)
require.True(t, errors.Is(err, ErrSecretTypeMismatch))
require.EqualError(t, err, "secret storage data has incorrect type: storage.pinniped.dev/candies-not must equal storage.pinniped.dev/candies")
require.EqualError(t, err, "error during get for signature 5aUhdNmfWLW3yKX8Zfz5ztS5IiiWBgu36Gja-o2xl0I: "+
"secret storage data has incorrect type: storage.pinniped.dev/candies-not must equal storage.pinniped.dev/candies")
return nil
},
@ -856,7 +857,8 @@ func TestStorage(t *testing.T) {
require.Empty(t, rv1)
require.Empty(t, out.Data)
require.True(t, errors.Is(err, ErrSecretLabelMismatch))
require.EqualError(t, err, "secret storage data has incorrect label: candies-are-bad must equal candies")
require.EqualError(t, err, "error during get for signature 5aUhdNmfWLW3yKX8Zfz5ztS5IiiWBgu36Gja-o2xl0I: "+
"secret storage data has incorrect label: candies-are-bad must equal candies")
return nil
},
@ -919,7 +921,8 @@ func TestStorage(t *testing.T) {
require.Empty(t, rv1)
require.Empty(t, out.Data)
require.True(t, errors.Is(err, ErrSecretVersionMismatch))
require.EqualError(t, err, "secret storage data has incorrect version")
require.EqualError(t, err, "error during get for signature 5aUhdNmfWLW3yKX8Zfz5ztS5IiiWBgu36Gja-o2xl0I: "+
"secret storage data has incorrect version")
return nil
},
@ -981,7 +984,8 @@ func TestStorage(t *testing.T) {
rv1, err := storage.Get(ctx, signature, out)
require.Empty(t, rv1)
require.Empty(t, out.Data)
require.EqualError(t, err, "failed to decode candies for signature 5aUhdNmfWLW3yKX8Zfz5ztS5IiiWBgu36Gja-o2xl0I: invalid character '}' looking for beginning of value")
require.EqualError(t, err, "error during get for signature 5aUhdNmfWLW3yKX8Zfz5ztS5IiiWBgu36Gja-o2xl0I: "+
"failed to decode candies: invalid character '}' looking for beginning of value")
return nil
},
@ -1095,3 +1099,155 @@ func errString(err error) string {
return err.Error()
}
func TestFromSecret(t *testing.T) {
fakeNow := time.Date(2030, time.January, 1, 0, 0, 0, 0, time.UTC)
lifetime := time.Minute * 10
fakeNowPlusLifetimeAsString := metav1.Time{Time: fakeNow.Add(lifetime)}.Format(time.RFC3339)
type testJSON struct {
Data string
}
tests := []struct {
name string
resource string
secret *corev1.Secret
wantData *testJSON
wantErr string
}{
{
name: "happy path",
resource: "candies",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "pinniped-storage-candies-lvzgyywdc2dhjdbgf5jvzfyphosigvhnsh6qlse3blumogoqhqhq",
Namespace: "some-namespace",
ResourceVersion: "",
Labels: map[string]string{
"storage.pinniped.dev/type": "candies",
},
Annotations: map[string]string{
"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`{"Data":"snorlax"}`),
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/candies",
},
wantData: &testJSON{Data: "snorlax"},
},
{
name: "can't unmarshal",
resource: "candies",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "pinniped-storage-candies-lvzgyywdc2dhjdbgf5jvzfyphosigvhnsh6qlse3blumogoqhqhq",
Namespace: "some-namespace",
ResourceVersion: "",
Labels: map[string]string{
"storage.pinniped.dev/type": "candies",
},
Annotations: map[string]string{
"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`not-json`),
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/candies",
},
wantData: &testJSON{Data: "snorlax"},
wantErr: "failed to decode candies: invalid character 'o' in literal null (expecting 'u')",
},
{
name: "wrong storage version",
resource: "candies",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "pinniped-storage-candies-lvzgyywdc2dhjdbgf5jvzfyphosigvhnsh6qlse3blumogoqhqhq",
Namespace: "some-namespace",
ResourceVersion: "",
Labels: map[string]string{
"storage.pinniped.dev/type": "candies",
},
Annotations: map[string]string{
"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`{"Data":"snorlax"}`),
"pinniped-storage-version": []byte("wrong-version-here"),
},
Type: "storage.pinniped.dev/candies",
},
wantData: &testJSON{Data: "snorlax"},
wantErr: "secret storage data has incorrect version",
},
{
name: "wrong type label",
resource: "candies",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "pinniped-storage-candies-lvzgyywdc2dhjdbgf5jvzfyphosigvhnsh6qlse3blumogoqhqhq",
Namespace: "some-namespace",
ResourceVersion: "",
Labels: map[string]string{
"storage.pinniped.dev/type": "candies",
},
Annotations: map[string]string{
"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`{"Data":"snorlax"}`),
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/not-candies",
},
wantData: &testJSON{Data: "snorlax"},
wantErr: "secret storage data has incorrect type: storage.pinniped.dev/not-candies must equal storage.pinniped.dev/candies",
},
{
name: "wrong secret type",
resource: "candies",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "pinniped-storage-candies-lvzgyywdc2dhjdbgf5jvzfyphosigvhnsh6qlse3blumogoqhqhq",
Namespace: "some-namespace",
ResourceVersion: "",
Labels: map[string]string{
"storage.pinniped.dev/type": "candies",
},
Annotations: map[string]string{
"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`{"Data":"snorlax"}`),
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/not-candies",
},
wantData: &testJSON{Data: "snorlax"},
wantErr: "secret storage data has incorrect type: storage.pinniped.dev/not-candies must equal storage.pinniped.dev/candies",
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
data := &testJSON{}
err := FromSecret("candies", tt.secret, data)
if tt.wantErr == "" {
require.NoError(t, err)
require.Equal(t, data, tt.wantData)
} else {
require.EqualError(t, err, tt.wantErr)
}
})
}
}

28
internal/fositestorage/accesstoken/accesstoken.go
View File

@ -10,6 +10,7 @@ import (
"github.com/ory/fosite"
"github.com/ory/fosite/handler/oauth2"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
@ -42,7 +43,7 @@ type accessTokenStorage struct {
storage crud.Storage
}
type session struct {
type Session struct {
Request *fosite.Request `json:"request"`
Version string `json:"version"`
}
@ -51,6 +52,23 @@ func New(secrets corev1client.SecretInterface, clock func() time.Time, sessionSt
return &accessTokenStorage{storage: crud.New(TypeLabelValue, secrets, clock, sessionStorageLifetime)}
}
// ReadFromSecret reads the contents of a Secret as a Session.
func ReadFromSecret(secret *v1.Secret) (*Session, error) {
session := newValidEmptyAccessTokenSession()
err := crud.FromSecret(TypeLabelValue, secret, session)
if err != nil {
return nil, err
}
if session.Version != accessTokenStorageVersion {
return nil, fmt.Errorf("%w: access token session has version %s instead of %s",
ErrInvalidAccessTokenRequestVersion, session.Version, accessTokenStorageVersion)
}
if session.Request.ID == "" {
return nil, fmt.Errorf("malformed access token session: %w", ErrInvalidAccessTokenRequestData)
}
return session, nil
}
func (a *accessTokenStorage) RevokeAccessToken(ctx context.Context, requestID string) error {
return a.storage.DeleteByLabel(ctx, fositestorage.StorageRequestIDLabelName, requestID)
}
@ -64,7 +82,7 @@ func (a *accessTokenStorage) CreateAccessTokenSession(ctx context.Context, signa
_, err = a.storage.Create(
ctx,
signature,
&session{Request: request, Version: accessTokenStorageVersion},
&Session{Request: request, Version: accessTokenStorageVersion},
map[string]string{fositestorage.StorageRequestIDLabelName: requester.GetID()},
)
return err
@ -84,7 +102,7 @@ func (a *accessTokenStorage) DeleteAccessTokenSession(ctx context.Context, signa
return a.storage.Delete(ctx, signature)
}
func (a *accessTokenStorage) getSession(ctx context.Context, signature string) (*session, string, error) {
func (a *accessTokenStorage) getSession(ctx context.Context, signature string) (*Session, string, error) {
session := newValidEmptyAccessTokenSession()
rv, err := a.storage.Get(ctx, signature, session)
@ -108,8 +126,8 @@ func (a *accessTokenStorage) getSession(ctx context.Context, signature string) (
return session, rv, nil
}
func newValidEmptyAccessTokenSession() *session {
return &session{
func newValidEmptyAccessTokenSession() *Session {
return &Session{
Request: &fosite.Request{
Client: &clientregistry.Client{},
Session: &psession.PinnipedSession{},

117
internal/fositestorage/accesstoken/accesstoken_test.go
View File

@ -10,6 +10,7 @@ import (
"time"
"github.com/ory/fosite"
"github.com/ory/fosite/handler/openid"
"github.com/pkg/errors"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
@ -277,3 +278,119 @@ func makeTestSubject() (context.Context, *fake.Clientset, corev1client.SecretInt
secrets := client.CoreV1().Secrets(namespace)
return context.Background(), client, secrets, New(secrets, clock.NewFakeClock(fakeNow).Now, lifetime)
}
func TestReadFromSecret(t *testing.T) {
tests := []struct {
name string
secret *corev1.Secret
wantSession *Session
wantErr string
}{
{
name: "happy path",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "pinniped-storage-access-token-pwu5zs7lekbhnln2w4",
ResourceVersion: "",
Labels: map[string]string{
"storage.pinniped.dev/type": "access-token",
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token"}}}},"version":"2","active": true}`),
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/access-token",
},
wantSession: &Session{
Version: "2",
Request: &fosite.Request{
ID: "abcd-1",
Client: &clientregistry.Client{},
Session: &psession.PinnipedSession{
Fosite: &openid.DefaultSession{
Username: "snorlax",
Subject: "panda",
},
Custom: &psession.CustomSessionData{
ProviderUID: "fake-provider-uid",
ProviderName: "fake-provider-name",
ProviderType: "fake-provider-type",
OIDC: &psession.OIDCSessionData{
UpstreamRefreshToken: "fake-upstream-refresh-token",
},
},
},
},
},
},
{
name: "wrong secret type",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "pinniped-storage-access-token-pwu5zs7lekbhnln2w4",
ResourceVersion: "",
Labels: map[string]string{
"storage.pinniped.dev/type": "access-token",
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`{"request":{"id":"abcd-1"},"version":"2","active": true}`),
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/not-access-token",
},
wantErr: "secret storage data has incorrect type: storage.pinniped.dev/not-access-token must equal storage.pinniped.dev/access-token",
},
{
name: "wrong session version",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "pinniped-storage-access-token-pwu5zs7lekbhnln2w4",
ResourceVersion: "",
Labels: map[string]string{
"storage.pinniped.dev/type": "access-token",
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`{"request":{"id":"abcd-1"},"version":"wrong-version-here","active": true}`),
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/access-token",
},
wantErr: "access token request data has wrong version: access token session has version wrong-version-here instead of 2",
},
{
name: "missing request",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "pinniped-storage-access-token-pwu5zs7lekbhnln2w4",
ResourceVersion: "",
Labels: map[string]string{
"storage.pinniped.dev/type": "access-token",
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`{"version":"2","active": true}`),
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/access-token",
},
wantErr: "malformed access token session: access token request data must be present",
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
session, err := ReadFromSecret(tt.secret)
if tt.wantErr == "" {
require.NoError(t, err)
require.Equal(t, tt.wantSession, session)
} else {
require.EqualError(t, err, tt.wantErr)
require.Nil(t, session)
}
})
}
}

28
internal/fositestorage/authorizationcode/authorizationcode.go
View File

@ -11,6 +11,7 @@ import (
"github.com/ory/fosite"
"github.com/ory/fosite/handler/oauth2"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
@ -38,7 +39,7 @@ type authorizeCodeStorage struct {
storage crud.Storage
}
type AuthorizeCodeSession struct {
type Session struct {
Active bool `json:"active"`
Request *fosite.Request `json:"request"`
Version string `json:"version"`
@ -48,6 +49,23 @@ func New(secrets corev1client.SecretInterface, clock func() time.Time, sessionSt
return &authorizeCodeStorage{storage: crud.New(TypeLabelValue, secrets, clock, sessionStorageLifetime)}
}
// ReadFromSecret reads the contents of a Secret as a Session.
func ReadFromSecret(secret *v1.Secret) (*Session, error) {
session := NewValidEmptyAuthorizeCodeSession()
err := crud.FromSecret(TypeLabelValue, secret, session)
if err != nil {
return nil, err
}
if session.Version != authorizeCodeStorageVersion {
return nil, fmt.Errorf("%w: authorization code session has version %s instead of %s",
ErrInvalidAuthorizeRequestVersion, session.Version, authorizeCodeStorageVersion)
}
if session.Request.ID == "" {
return nil, fmt.Errorf("malformed authorization code session: %w", ErrInvalidAuthorizeRequestData)
}
return session, nil
}
func (a *authorizeCodeStorage) CreateAuthorizeCodeSession(ctx context.Context, signature string, requester fosite.Requester) error {
// This conversion assumes that we do not wrap the default type in any way
// i.e. we use the default fosite.OAuth2Provider.NewAuthorizeRequest implementation
@ -70,7 +88,7 @@ func (a *authorizeCodeStorage) CreateAuthorizeCodeSession(ctx context.Context, s
// of the consent authorization request. It is used to identify the session.
// signature for lookup in the DB
_, err = a.storage.Create(ctx, signature, &AuthorizeCodeSession{Active: true, Request: request, Version: authorizeCodeStorageVersion}, nil)
_, err = a.storage.Create(ctx, signature, &Session{Active: true, Request: request, Version: authorizeCodeStorageVersion}, nil)
return err
}
@ -108,7 +126,7 @@ func (a *authorizeCodeStorage) InvalidateAuthorizeCodeSession(ctx context.Contex
return nil
}
func (a *authorizeCodeStorage) getSession(ctx context.Context, signature string) (*AuthorizeCodeSession, string, error) {
func (a *authorizeCodeStorage) getSession(ctx context.Context, signature string) (*Session, string, error) {
session := NewValidEmptyAuthorizeCodeSession()
rv, err := a.storage.Get(ctx, signature, session)
@ -137,8 +155,8 @@ func (a *authorizeCodeStorage) getSession(ctx context.Context, signature string)
return session, rv, nil
}
func NewValidEmptyAuthorizeCodeSession() *AuthorizeCodeSession {
return &AuthorizeCodeSession{
func NewValidEmptyAuthorizeCodeSession() *Session {
return &Session{
Request: &fosite.Request{
Client: &clientregistry.Client{},
Session: &psession.PinnipedSession{},

118
internal/fositestorage/authorizationcode/authorizationcode_test.go
View File

@ -18,6 +18,7 @@ import (
fuzz "github.com/google/gofuzz"
"github.com/ory/fosite"
"github.com/ory/fosite/handler/oauth2"
"github.com/ory/fosite/handler/openid"
"github.com/pkg/errors"
"github.com/stretchr/testify/require"
"gopkg.in/square/go-jose.v2"
@ -397,3 +398,120 @@ func TestFuzzAndJSONNewValidEmptyAuthorizeCodeSession(t *testing.T) {
// thus if AuthorizeRequest changes, we will detect it here (though we could possibly miss an omitempty field)
require.JSONEq(t, ExpectedAuthorizeCodeSessionJSONFromFuzzing, authorizeCodeSessionJSONFromFuzzing)
}
func TestReadFromSecret(t *testing.T) {
tests := []struct {
name string
secret *corev1.Secret
wantSession *Session
wantErr string
}{
{
name: "happy path",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "pinniped-storage-authcode-pwu5zs7lekbhnln2w4",
ResourceVersion: "",
Labels: map[string]string{
"storage.pinniped.dev/type": "authcode",
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token"}}}},"version":"2","active": true}`),
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/authcode",
},
wantSession: &Session{
Version: "2",
Active: true,
Request: &fosite.Request{
ID: "abcd-1",
Client: &clientregistry.Client{},
Session: &psession.PinnipedSession{
Fosite: &openid.DefaultSession{
Username: "snorlax",
Subject: "panda",
},
Custom: &psession.CustomSessionData{
ProviderUID: "fake-provider-uid",
ProviderName: "fake-provider-name",
ProviderType: "fake-provider-type",
OIDC: &psession.OIDCSessionData{
UpstreamRefreshToken: "fake-upstream-refresh-token",
},
},
},
},
},
},
{
name: "wrong secret type",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "pinniped-storage-authcode-pwu5zs7lekbhnln2w4",
ResourceVersion: "",
Labels: map[string]string{
"storage.pinniped.dev/type": "authcode",
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`{"request":{"id":"abcd-1"},"version":"2","active": true}`),
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/not-authcode",
},
wantErr: "secret storage data has incorrect type: storage.pinniped.dev/not-authcode must equal storage.pinniped.dev/authcode",
},
{
name: "wrong session version",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "pinniped-storage-authcode-pwu5zs7lekbhnln2w4",
ResourceVersion: "",
Labels: map[string]string{
"storage.pinniped.dev/type": "authcode",
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`{"request":{"id":"abcd-1"},"version":"wrong-version-here","active": true}`),
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/authcode",
},
wantErr: "authorization request data has wrong version: authorization code session has version wrong-version-here instead of 2",
},
{
name: "missing request",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "pinniped-storage-authcode-pwu5zs7lekbhnln2w4",
ResourceVersion: "",
Labels: map[string]string{
"storage.pinniped.dev/type": "authcode",
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`{"version":"2","active": true}`),
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/authcode",
},
wantErr: "malformed authorization code session: authorization request data must be present",
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
session, err := ReadFromSecret(tt.secret)
if tt.wantErr == "" {
require.NoError(t, err)
require.Equal(t, tt.wantSession, session)
} else {
require.EqualError(t, err, tt.wantErr)
require.Nil(t, session)
}
})
}
}

28
internal/fositestorage/refreshtoken/refreshtoken.go
View File

@ -10,6 +10,7 @@ import (
"github.com/ory/fosite"
"github.com/ory/fosite/handler/oauth2"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
@ -42,7 +43,7 @@ type refreshTokenStorage struct {
storage crud.Storage
}
type session struct {
type Session struct {
Request *fosite.Request `json:"request"`
Version string `json:"version"`
}
@ -51,6 +52,23 @@ func New(secrets corev1client.SecretInterface, clock func() time.Time, sessionSt
return &refreshTokenStorage{storage: crud.New(TypeLabelValue, secrets, clock, sessionStorageLifetime)}
}
// ReadFromSecret reads the contents of a Secret as a Session.
func ReadFromSecret(secret *v1.Secret) (*Session, error) {
session := newValidEmptyRefreshTokenSession()
err := crud.FromSecret(TypeLabelValue, secret, session)
if err != nil {
return nil, err
}
if session.Version != refreshTokenStorageVersion {
return nil, fmt.Errorf("%w: refresh token session has version %s instead of %s",
ErrInvalidRefreshTokenRequestVersion, session.Version, refreshTokenStorageVersion)
}
if session.Request.ID == "" {
return nil, fmt.Errorf("malformed refresh token session: %w", ErrInvalidRefreshTokenRequestData)
}
return session, nil
}
func (a *refreshTokenStorage) RevokeRefreshToken(ctx context.Context, requestID string) error {
return a.storage.DeleteByLabel(ctx, fositestorage.StorageRequestIDLabelName, requestID)
}
@ -64,7 +82,7 @@ func (a *refreshTokenStorage) CreateRefreshTokenSession(ctx context.Context, sig
_, err = a.storage.Create(
ctx,
signature,
&session{Request: request, Version: refreshTokenStorageVersion},
&Session{Request: request, Version: refreshTokenStorageVersion},
map[string]string{fositestorage.StorageRequestIDLabelName: requester.GetID()},
)
return err
@ -84,7 +102,7 @@ func (a *refreshTokenStorage) DeleteRefreshTokenSession(ctx context.Context, sig
return a.storage.Delete(ctx, signature)
}
func (a *refreshTokenStorage) getSession(ctx context.Context, signature string) (*session, string, error) {
func (a *refreshTokenStorage) getSession(ctx context.Context, signature string) (*Session, string, error) {
session := newValidEmptyRefreshTokenSession()
rv, err := a.storage.Get(ctx, signature, session)
@ -108,8 +126,8 @@ func (a *refreshTokenStorage) getSession(ctx context.Context, signature string)
return session, rv, nil
}
func newValidEmptyRefreshTokenSession() *session {
return &session{
func newValidEmptyRefreshTokenSession() *Session {
return &Session{
Request: &fosite.Request{
Client: &clientregistry.Client{},
Session: &psession.PinnipedSession{},

117
internal/fositestorage/refreshtoken/refreshtoken_test.go
View File

@ -10,6 +10,7 @@ import (
"time"
"github.com/ory/fosite"
"github.com/ory/fosite/handler/openid"
"github.com/pkg/errors"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
@ -277,3 +278,119 @@ func makeTestSubject() (context.Context, *fake.Clientset, corev1client.SecretInt
secrets := client.CoreV1().Secrets(namespace)
return context.Background(), client, secrets, New(secrets, clock.NewFakeClock(fakeNow).Now, lifetime)
}
func TestReadFromSecret(t *testing.T) {
tests := []struct {
name string
secret *corev1.Secret
wantSession *Session
wantErr string
}{
{
name: "happy path",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "pinniped-storage-refresh-token-pwu5zs7lekbhnln2w4",
ResourceVersion: "",
Labels: map[string]string{
"storage.pinniped.dev/type": "refresh-token",
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","session":{"fosite":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token"}}}},"version":"2","active": true}`),
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/refresh-token",
},
wantSession: &Session{
Version: "2",
Request: &fosite.Request{
ID: "abcd-1",
Client: &clientregistry.Client{},
Session: &psession.PinnipedSession{
Fosite: &openid.DefaultSession{
Username: "snorlax",
Subject: "panda",
},
Custom: &psession.CustomSessionData{
ProviderUID: "fake-provider-uid",
ProviderName: "fake-provider-name",
ProviderType: "fake-provider-type",
OIDC: &psession.OIDCSessionData{
UpstreamRefreshToken: "fake-upstream-refresh-token",
},
},
},
},
},
},
{
name: "wrong secret type",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "pinniped-storage-refresh-token-pwu5zs7lekbhnln2w4",
ResourceVersion: "",
Labels: map[string]string{
"storage.pinniped.dev/type": "refresh-token",
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`{"request":{"id":"abcd-1"},"version":"2","active": true}`),
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/not-refresh-token",
},
wantErr: "secret storage data has incorrect type: storage.pinniped.dev/not-refresh-token must equal storage.pinniped.dev/refresh-token",
},
{
name: "wrong session version",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "pinniped-storage-refresh-token-pwu5zs7lekbhnln2w4",
ResourceVersion: "",
Labels: map[string]string{
"storage.pinniped.dev/type": "refresh-token",
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`{"request":{"id":"abcd-1"},"version":"wrong-version-here","active": true}`),
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/refresh-token",
},
wantErr: "refresh token request data has wrong version: refresh token session has version wrong-version-here instead of 2",
},
{
name: "missing request",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "pinniped-storage-refresh-token-pwu5zs7lekbhnln2w4",
ResourceVersion: "",
Labels: map[string]string{
"storage.pinniped.dev/type": "refresh-token",
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`{"version":"2","active": true}`),
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/refresh-token",
},
wantErr: "malformed refresh token session: refresh token request data must be present",
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
session, err := ReadFromSecret(tt.secret)
if tt.wantErr == "" {
require.NoError(t, err)
require.Equal(t, tt.wantSession, session)
} else {
require.EqualError(t, err, tt.wantErr)
require.Nil(t, session)
}
})
}
}

14
internal/mocks/mockupstreamoidcidentityprovider/mockupstreamoidcidentityprovider.go
View File

@ -215,6 +215,20 @@ func (mr *MockUpstreamOIDCIdentityProviderIMockRecorder) PerformRefresh(arg0, ar
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PerformRefresh", reflect.TypeOf((*MockUpstreamOIDCIdentityProviderI)(nil).PerformRefresh), arg0, arg1)
}
// RevokeRefreshToken mocks base method.
func (m *MockUpstreamOIDCIdentityProviderI) RevokeRefreshToken(arg0 context.Context, arg1 string) error {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "RevokeRefreshToken", arg0, arg1)
ret0, _ := ret[0].(error)
return ret0
}
// RevokeRefreshToken indicates an expected call of RevokeRefreshToken.
func (mr *MockUpstreamOIDCIdentityProviderIMockRecorder) RevokeRefreshToken(arg0, arg1 interface{}) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RevokeRefreshToken", reflect.TypeOf((*MockUpstreamOIDCIdentityProviderI)(nil).RevokeRefreshToken), arg0, arg1)
}
// ValidateToken mocks base method.
func (m *MockUpstreamOIDCIdentityProviderI) ValidateToken(arg0 context.Context, arg1 *oauth2.Token, arg2 nonce.Nonce) (*oidctypes.Token, error) {
m.ctrl.T.Helper()

3
internal/oidc/provider/dynamic_upstream_idp_provider.go
View File

@ -68,6 +68,9 @@ type UpstreamOIDCIdentityProviderI interface {
// validate the ID token.
PerformRefresh(ctx context.Context, refreshToken string) (*oauth2.Token, error)
// RevokeRefreshToken will attempt to revoke the given token, if the provider has a revocation endpoint.
RevokeRefreshToken(ctx context.Context, refreshToken string) error
// ValidateToken will validate the ID token. It will also merge the claims from the userinfo endpoint response
// into the ID token's claims, if the provider offers the userinfo endpoint. It returns the validated/updated
// tokens, or an error.

1
internal/supervisor/server/server.go
View File

@ -125,6 +125,7 @@ func prepareControllers(
NewManager().
WithController(
supervisorstorage.GarbageCollectorController(
dynamicUpstreamIDPProvider,
clock.RealClock{},
kubeClient,
secretInformer,

85
internal/testutil/oidctestutil/oidctestutil.go
View File

@ -68,6 +68,13 @@ type PerformRefreshArgs struct {
ExpectedSubject string
}
// RevokeRefreshTokenArgs is used to spy on calls to
// TestUpstreamOIDCIdentityProvider.RevokeRefreshTokenArgsFunc().
type RevokeRefreshTokenArgs struct {
Ctx context.Context
RefreshToken string
}
// ValidateTokenArgs is used to spy on calls to
// TestUpstreamOIDCIdentityProvider.ValidateTokenFunc().
type ValidateTokenArgs struct {
@ -137,6 +144,7 @@ type TestUpstreamOIDCIdentityProvider struct {
ClientID string
ResourceUID types.UID
AuthorizationURL url.URL
RevocationURL *url.URL
UsernameClaim string
GroupsClaim string
Scopes []string
@ -158,6 +166,8 @@ type TestUpstreamOIDCIdentityProvider struct {
PerformRefreshFunc func(ctx context.Context, refreshToken string) (*oauth2.Token, error)
RevokeRefreshTokenFunc func(ctx context.Context, refreshToken string) error
ValidateTokenFunc func(ctx context.Context, tok *oauth2.Token, expectedIDTokenNonce nonce.Nonce) (*oidctypes.Token, error)
exchangeAuthcodeAndValidateTokensCallCount int
@ -166,6 +176,8 @@ type TestUpstreamOIDCIdentityProvider struct {
passwordCredentialsGrantAndValidateTokensArgs []*PasswordCredentialsGrantAndValidateTokensArgs
performRefreshCallCount int
performRefreshArgs []*PerformRefreshArgs
revokeRefreshTokenCallCount int
revokeRefreshTokenArgs []*RevokeRefreshTokenArgs
validateTokenCallCount int
validateTokenArgs []*ValidateTokenArgs
}
@ -192,6 +204,10 @@ func (u *TestUpstreamOIDCIdentityProvider) GetAuthorizationURL() *url.URL {
return &u.AuthorizationURL
}
func (u *TestUpstreamOIDCIdentityProvider) GetRevocationURL() *url.URL {
return u.RevocationURL
}
func (u *TestUpstreamOIDCIdentityProvider) GetScopes() []string {
return u.Scopes
}
@ -262,6 +278,18 @@ func (u *TestUpstreamOIDCIdentityProvider) PerformRefresh(ctx context.Context, r
return u.PerformRefreshFunc(ctx, refreshToken)
}
func (u *TestUpstreamOIDCIdentityProvider) RevokeRefreshToken(ctx context.Context, refreshToken string) error {
if u.revokeRefreshTokenArgs == nil {
u.revokeRefreshTokenArgs = make([]*RevokeRefreshTokenArgs, 0)
}
u.revokeRefreshTokenCallCount++
u.revokeRefreshTokenArgs = append(u.revokeRefreshTokenArgs, &RevokeRefreshTokenArgs{
Ctx: ctx,
RefreshToken: refreshToken,
})
return u.RevokeRefreshTokenFunc(ctx, refreshToken)
}
func (u *TestUpstreamOIDCIdentityProvider) PerformRefreshCallCount() int {
return u.performRefreshCallCount
}
@ -273,6 +301,17 @@ func (u *TestUpstreamOIDCIdentityProvider) PerformRefreshArgs(call int) *Perform
return u.performRefreshArgs[call]
}
func (u *TestUpstreamOIDCIdentityProvider) RevokeRefreshTokenCallCount() int {
return u.performRefreshCallCount
}
func (u *TestUpstreamOIDCIdentityProvider) RevokeRefreshTokenArgs(call int) *RevokeRefreshTokenArgs {
if u.revokeRefreshTokenArgs == nil {
u.revokeRefreshTokenArgs = make([]*RevokeRefreshTokenArgs, 0)
}
return u.revokeRefreshTokenArgs[call]
}
func (u *TestUpstreamOIDCIdentityProvider) ValidateToken(ctx context.Context, tok *oauth2.Token, expectedIDTokenNonce nonce.Nonce) (*oidctypes.Token, error) {
if u.validateTokenArgs == nil {
u.validateTokenArgs = make([]*ValidateTokenArgs, 0)
@ -513,6 +552,43 @@ func (b *UpstreamIDPListerBuilder) RequireExactlyZeroCallsToValidateToken(t *tes
)
}
func (b *UpstreamIDPListerBuilder) RequireExactlyOneCallToRevokeRefreshToken(
t *testing.T,
expectedPerformedByUpstreamName string,
expectedArgs *RevokeRefreshTokenArgs,
) {
t.Helper()
var actualArgs *RevokeRefreshTokenArgs
var actualNameOfUpstreamWhichMadeCall string
actualCallCountAcrossAllOIDCUpstreams := 0
for _, upstreamOIDC := range b.upstreamOIDCIdentityProviders {
callCountOnThisUpstream := upstreamOIDC.revokeRefreshTokenCallCount
actualCallCountAcrossAllOIDCUpstreams += callCountOnThisUpstream
if callCountOnThisUpstream == 1 {
actualNameOfUpstreamWhichMadeCall = upstreamOIDC.Name
actualArgs = upstreamOIDC.revokeRefreshTokenArgs[0]
}
}
require.Equal(t, 1, actualCallCountAcrossAllOIDCUpstreams,
"should have been exactly one call to RevokeRefreshToken() by all OIDC upstreams",
)
require.Equal(t, expectedPerformedByUpstreamName, actualNameOfUpstreamWhichMadeCall,
"RevokeRefreshToken() was called on the wrong OIDC upstream",
)
require.Equal(t, expectedArgs, actualArgs)
}
func (b *UpstreamIDPListerBuilder) RequireExactlyZeroCallsToRevokeRefreshToken(t *testing.T) {
t.Helper()
actualCallCountAcrossAllOIDCUpstreams := 0
for _, upstreamOIDC := range b.upstreamOIDCIdentityProviders {
actualCallCountAcrossAllOIDCUpstreams += upstreamOIDC.revokeRefreshTokenCallCount
}
require.Equal(t, 0, actualCallCountAcrossAllOIDCUpstreams,
"expected exactly zero calls to RevokeRefreshToken()",
)
}
func NewUpstreamIDPListerBuilder() *UpstreamIDPListerBuilder {
return &UpstreamIDPListerBuilder{}
}
@ -534,6 +610,7 @@ type TestUpstreamOIDCIdentityProviderBuilder struct {
authcodeExchangeErr error
passwordGrantErr error
performRefreshErr error
revokeRefreshTokenErr error
validateTokenErr error
}
@ -650,6 +727,11 @@ func (u *TestUpstreamOIDCIdentityProviderBuilder) WithValidateTokenError(err err
return u
}
func (u *TestUpstreamOIDCIdentityProviderBuilder) WithRevokeRefreshTokenError(err error) *TestUpstreamOIDCIdentityProviderBuilder {
u.revokeRefreshTokenErr = err
return u
}
func (u *TestUpstreamOIDCIdentityProviderBuilder) Build() *TestUpstreamOIDCIdentityProvider {
return &TestUpstreamOIDCIdentityProvider{
Name: u.name,
@ -679,6 +761,9 @@ func (u *TestUpstreamOIDCIdentityProviderBuilder) Build() *TestUpstreamOIDCIdent
}
return u.refreshedTokens, nil
},
RevokeRefreshTokenFunc: func(ctx context.Context, refreshToken string) error {
return u.revokeRefreshTokenErr
},
ValidateTokenFunc: func(ctx context.Context, tok *oauth2.Token, expectedIDTokenNonce nonce.Nonce) (*oidctypes.Token, error) {
if u.validateTokenErr != nil {
return nil, u.validateTokenErr

106
internal/upstreamoidc/upstreamoidc.go
View File

@ -8,8 +8,10 @@ import (
"context"
"encoding/json"
"fmt"
"io"
"net/http"
"net/url"
"strings"
coreosoidc "github.com/coreos/go-oidc/v3/oidc"
"golang.org/x/oauth2"
@ -40,6 +42,7 @@ type ProviderConfig struct {
Client *http.Client
AllowPasswordGrant bool
AdditionalAuthcodeParams map[string]string
RevocationURL *url.URL // will commonly be nil: many providers do not offer this
Provider interface {
Verifier(*coreosoidc.Config) *coreosoidc.IDTokenVerifier
Claims(v interface{}) error
@ -53,6 +56,10 @@ func (p *ProviderConfig) GetResourceUID() types.UID {
return p.ResourceUID
}
func (p *ProviderConfig) GetRevocationURL() *url.URL {
return p.RevocationURL
}
func (p *ProviderConfig) GetAdditionalAuthcodeParams() map[string]string {
return p.AdditionalAuthcodeParams
}
@ -130,6 +137,105 @@ func (p *ProviderConfig) PerformRefresh(ctx context.Context, refreshToken string
return p.Config.TokenSource(httpClientContext, &oauth2.Token{RefreshToken: refreshToken}).Token()
}
// RevokeRefreshToken will attempt to revoke the given token, if the provider has a revocation endpoint.
func (p *ProviderConfig) RevokeRefreshToken(ctx context.Context, refreshToken string) error {
if p.RevocationURL == nil {
plog.Trace("RevokeRefreshToken() was called but upstream provider has no available revocation endpoint", "providerName", p.Name)
return nil
}
// First try using client auth in the request params.
tryAnotherClientAuthMethod, err := p.tryRevokeRefreshToken(ctx, refreshToken, false)
if tryAnotherClientAuthMethod {
// Try again using basic auth this time. Overwrite the first client auth error,
// which isn't useful anymore when retrying.
_, err = p.tryRevokeRefreshToken(ctx, refreshToken, true)
}
return err
}
// tryRevokeRefreshToken will call the revocation endpoint using either basic auth or by including
// client auth in the request params. It will return an error when the request failed. If the
// request failed for a reason that might be due to bad client auth, then it will return true
// for the tryAnotherClientAuthMethod return value, indicating that it might be worth trying
// again using the other client auth method.
// RFC 7009 defines how to make a revocation request and how to interpret the response.
// See https://datatracker.ietf.org/doc/html/rfc7009#section-2.1 for details.
func (p *ProviderConfig) tryRevokeRefreshToken(
ctx context.Context,
refreshToken string,
useBasicAuth bool,
) (tryAnotherClientAuthMethod bool, err error) {
clientID := p.Config.ClientID
clientSecret := p.Config.ClientSecret
// Use the provided HTTP client to benefit from its CA, proxy, and other settings.
httpClient := p.Client
params := url.Values{
"token": []string{refreshToken},
"token_type_hint": []string{"refresh_token"},
}
if !useBasicAuth {
params["client_id"] = []string{clientID}
params["client_secret"] = []string{clientSecret}
}
req, err := http.NewRequestWithContext(ctx, http.MethodPost, p.RevocationURL.String(), strings.NewReader(params.Encode()))
if err != nil {
// This shouldn't really happen since we already know that the method and URL are legal.
return false, err
}
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
if useBasicAuth {
req.SetBasicAuth(clientID, clientSecret)
}
resp, err := httpClient.Do(req)
if err != nil {
// Couldn't connect to the server or some similar error.
return false, err
}
defer resp.Body.Close()
switch resp.StatusCode {
case http.StatusOK:
// Success!
plog.Trace("RevokeRefreshToken() got 200 OK response from provider's revocation endpoint", "providerName", p.Name, "usedBasicAuth", useBasicAuth)
return false, nil
case http.StatusBadRequest:
// Bad request might be due to bad client auth method. Try to detect that.
plog.Trace("RevokeRefreshToken() got 400 Bad Request response from provider's revocation endpoint", "providerName", p.Name, "usedBasicAuth", useBasicAuth)
body, err := io.ReadAll(resp.Body)
if err != nil {
return false,
fmt.Errorf("error reading response body on response with status code %d: %w", resp.StatusCode, err)
}
var parsedResp struct {
ErrorType string `json:"error"`
ErrorDescription string `json:"error_description"`
}
bodyStr := strings.TrimSpace(string(body)) // trimmed for logging purposes
err = json.Unmarshal(body, &parsedResp)
if err != nil {
return false,
fmt.Errorf("error parsing response body %q on response with status code %d: %w", bodyStr, resp.StatusCode, err)
}
err = fmt.Errorf("server responded with status %d with body: %s", resp.StatusCode, bodyStr)
if parsedResp.ErrorType != "invalid_client" {
// Got an error unrelated to client auth, so not worth trying again.
return false, err
}
// Got an "invalid_client" response, which might mean client auth failed, so it may be worth trying again
// using another client auth method. See https://datatracker.ietf.org/doc/html/rfc6749#section-5.2
plog.Trace("RevokeRefreshToken()'s 400 Bad Request response from provider's revocation endpoint was type 'invalid_client'", "providerName", p.Name, "usedBasicAuth", useBasicAuth)
return true, err
default:
// Any other error is probably not due to failed client auth.
plog.Trace("RevokeRefreshToken() got unexpected error response from provider's revocation endpoint", "providerName", p.Name, "usedBasicAuth", useBasicAuth, "statusCode", resp.StatusCode)
return false, fmt.Errorf("server responded with status %d", resp.StatusCode)
}
}
// ValidateToken will validate the ID token. It will also merge the claims from the userinfo endpoint response,
// if the provider offers the userinfo endpoint.
func (p *ProviderConfig) ValidateToken(ctx context.Context, tok *oauth2.Token, expectedIDTokenNonce nonce.Nonce) (*oidctypes.Token, error) {

137
internal/upstreamoidc/upstreamoidc_test.go
View File

@ -10,6 +10,7 @@ import (
"fmt"
"net/http"
"net/http/httptest"
"net/url"
"reflect"
"testing"
"time"
@ -454,6 +455,142 @@ func TestProviderConfig(t *testing.T) {
}
})
t.Run("RevokeRefreshToken", func(t *testing.T) {
tests := []struct {
name string
nilRevocationURL bool
statusCodes []int
returnErrBodies []string
wantErr string
wantNumRequests int
}{
{
name: "success without calling the server when there is no revocation URL set",
nilRevocationURL: true,
wantNumRequests: 0,
},
{
name: "success when the server returns 200 OK on the first call",
statusCodes: []int{http.StatusOK},
wantNumRequests: 1,
},
{
name: "success when the server returns 400 Bad Request on the first call due to client auth, then 200 OK on second call",
statusCodes: []int{http.StatusBadRequest, http.StatusOK},
// https://datatracker.ietf.org/doc/html/rfc6749#section-5.2 defines this as the error for client auth failure
returnErrBodies: []string{`{ "error":"invalid_client", "error_description":"unhappy" }`},
wantNumRequests: 2,
},
{
name: "error when the server returns 400 Bad Request on the first call due to client auth, then any 400 error on second call",
statusCodes: []int{http.StatusBadRequest, http.StatusBadRequest},
returnErrBodies: []string{`{ "error":"invalid_client", "error_description":"unhappy" }`, `{ "error":"anything", "error_description":"unhappy" }`},
wantErr: `server responded with status 400 with body: { "error":"anything", "error_description":"unhappy" }`,
wantNumRequests: 2,
},
{
name: "error when the server returns 400 Bad Request with bad JSON body on the first call",
statusCodes: []int{http.StatusBadRequest},
returnErrBodies: []string{`invalid JSON body`},
wantErr: `error parsing response body "invalid JSON body" on response with status code 400: invalid character 'i' looking for beginning of value`,
wantNumRequests: 1,
},
{
name: "error when the server returns 400 Bad Request with empty body",
statusCodes: []int{http.StatusBadRequest},
returnErrBodies: []string{``},
wantErr: `error parsing response body "" on response with status code 400: unexpected end of JSON input`,
wantNumRequests: 1,
},
{
name: "error when the server returns 400 Bad Request on the first call due to client auth, then any other error on second call",
statusCodes: []int{http.StatusBadRequest, http.StatusForbidden},
returnErrBodies: []string{`{ "error":"invalid_client", "error_description":"unhappy" }`, ""},
wantErr: "server responded with status 403",
wantNumRequests: 2,
},
{
name: "error when server returns any other 400 error on first call",
statusCodes: []int{http.StatusBadRequest},
returnErrBodies: []string{`{ "error":"anything_else", "error_description":"unhappy" }`},
wantErr: `server responded with status 400 with body: { "error":"anything_else", "error_description":"unhappy" }`,
wantNumRequests: 1,
},
{
name: "error when server returns any other error aside from 400 on first call",
statusCodes: []int{http.StatusForbidden},
returnErrBodies: []string{""},
wantErr: "server responded with status 403",
wantNumRequests: 1,
},
}
for _, tt := range tests {
tt := tt
numRequests := 0
t.Run(tt.name, func(t *testing.T) {
tokenServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
numRequests++
require.LessOrEqual(t, numRequests, 2)
require.Equal(t, http.MethodPost, r.Method)
require.NoError(t, r.ParseForm())
if numRequests == 1 {
// First request should use client_id/client_secret params.
require.Equal(t, 4, len(r.Form))
require.Equal(t, "test-client-id", r.Form.Get("client_id"))
require.Equal(t, "test-client-secret", r.Form.Get("client_secret"))
require.Equal(t, "refresh_token", r.Form.Get("token_type_hint"))
require.Equal(t, "test-initial-refresh-token", r.Form.Get("token"))
} else {
// Second request, if there is one, should use basic auth.
require.Equal(t, 2, len(r.Form))
require.Equal(t, "refresh_token", r.Form.Get("token_type_hint"))
require.Equal(t, "test-initial-refresh-token", r.Form.Get("token"))
username, password, hasBasicAuth := r.BasicAuth()
require.True(t, hasBasicAuth, "request should have had basic auth but did not")
require.Equal(t, "test-client-id", username)
require.Equal(t, "test-client-secret", password)
}
if tt.statusCodes[numRequests-1] != http.StatusOK {
w.Header().Set("content-type", "application/json")
http.Error(w, tt.returnErrBodies[numRequests-1], tt.statusCodes[numRequests-1])
}
// Otherwise, responds with 200 OK and empty body by default.
}))
t.Cleanup(tokenServer.Close)
tokenURL, err := url.Parse(tokenServer.URL)
require.NoError(t, err)
p := ProviderConfig{
Name: "test-name",
Config: &oauth2.Config{
ClientID: "test-client-id",
ClientSecret: "test-client-secret",
},
RevocationURL: tokenURL,
Client: http.DefaultClient,
}
if tt.nilRevocationURL {
p.RevocationURL = nil
}
err = p.RevokeRefreshToken(
context.Background(),
"test-initial-refresh-token",
)
require.Equal(t, tt.wantNumRequests, numRequests,
"did not make expected number of requests to revocation endpoint")
if tt.wantErr != "" {
require.EqualError(t, err, tt.wantErr)
} else {
require.NoError(t, err)
}
})
}
})
t.Run("ExchangeAuthcodeAndValidateTokens", func(t *testing.T) {
tests := []struct {
name string