diff --git a/test/extended/authentication/keycloak_client.go b/test/extended/authentication/keycloak_client.go index aab934f9456f..b5aade1a3403 100644 --- a/test/extended/authentication/keycloak_client.go +++ b/test/extended/authentication/keycloak_client.go @@ -104,7 +104,7 @@ func (kc *keycloakClient) CreateUser(username, password string, groups ...string Groups: groups, Credentials: []credential{ { - Temporary: true, + Temporary: false, Type: credentialTypePassword, Value: password, }, @@ -131,8 +131,10 @@ func (kc *keycloakClient) CreateUser(username, password string, groups ...string } type authenticationResponse struct { - AccessToken string `json:"access_token"` - IDToken string `json:"id_token"` + AccessToken string `json:"access_token"` + IDToken string `json:"id_token"` + Error string `json:"error,omitempty"` + ErrorDescription string `json:"error_description,omitempty"` } func (kc *keycloakClient) Authenticate(clientID, username, password string) error { @@ -159,6 +161,10 @@ func (kc *keycloakClient) Authenticate(clientID, username, password string) erro return fmt.Errorf("unmarshalling response data: %w", err) } + if respBody.Error != "" { + return fmt.Errorf("%s: %s", respBody.Error, respBody.ErrorDescription) + } + kc.accessToken = respBody.AccessToken kc.idToken = respBody.IDToken diff --git a/test/extended/authentication/keycloak_helpers.go b/test/extended/authentication/keycloak_helpers.go index 82d39e739c2b..81557e66be95 100644 --- a/test/extended/authentication/keycloak_helpers.go +++ b/test/extended/authentication/keycloak_helpers.go @@ -351,7 +351,7 @@ func createKeycloakRoute(ctx context.Context, service *corev1.Service, client ty } func waitForKeycloakAvailable(ctx context.Context, client *exutil.CLI, namespace string) error { - timeoutCtx, cancel := context.WithDeadline(ctx, time.Now().Add(5*time.Minute)) + timeoutCtx, cancel := context.WithDeadline(ctx, time.Now().Add(10*time.Minute)) defer cancel() err := wait.PollUntilContextCancel(timeoutCtx, 10*time.Second, true, func(ctx context.Context) (done bool, err error) { deploy, err := client.AdminKubeClient().AppsV1().Deployments(namespace).Get(ctx, keycloakResourceName, metav1.GetOptions{}) @@ -365,6 +365,8 @@ func waitForKeycloakAvailable(ctx context.Context, client *exutil.CLI, namespace } } + fmt.Println("keycloak deployment is not yet available. status: ", deploy.Status) + return false, nil }) diff --git a/test/extended/authentication/oidc.go b/test/extended/authentication/oidc.go index 3e834813a781..9610c0926ac6 100644 --- a/test/extended/authentication/oidc.go +++ b/test/extended/authentication/oidc.go @@ -15,6 +15,7 @@ import ( exutil "github.com/openshift/origin/test/extended/util" authnv1 "k8s.io/api/authentication/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/util/errors" @@ -112,15 +113,12 @@ var _ = g.Describe("[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow] o.Expect(apiServerArgs["authentication-config"].([]interface{})[0].(string)).To(o.Equal("/etc/kubernetes/static-pod-resources/configmaps/auth-config/auth-config.json")) }) - g.It("[Skipped] should remove the OpenShift OAuth stack", func() { - g.Skip("functionality not yet implemented") - /* - o.Eventually(func(gomega o.Gomega) { - _, err := oc.AdminKubeClient().AppsV1().Deployments("openshift-authentication").Get(ctx, "oauth-openshift", metav1.GetOptions{}) - gomega.Expect(err).NotTo(o.BeNil(), "should not be able to get the integrated oauth stack") - gomega.Expect(apierrors.IsNotFound(err)).To(o.BeTrue(), "integrated oauth stack should not be present when OIDC authentication is configured") - }).WithTimeout(20 * time.Minute).WithPolling(30 * time.Second).Should(o.Succeed()) - */ + g.It("should remove the OpenShift OAuth stack", func() { + o.Eventually(func(gomega o.Gomega) { + _, err := oc.AdminKubeClient().AppsV1().Deployments("openshift-authentication").Get(ctx, "oauth-openshift", metav1.GetOptions{}) + gomega.Expect(err).NotTo(o.BeNil(), "should not be able to get the integrated oauth stack") + gomega.Expect(apierrors.IsNotFound(err)).To(o.BeTrue(), "integrated oauth stack should not be present when OIDC authentication is configured") + }).WithTimeout(5 * time.Minute).WithPolling(10 * time.Second).Should(o.Succeed()) }) g.It("should not accept tokens provided by the OAuth server", func() { @@ -150,7 +148,8 @@ var _ = g.Describe("[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow] gomega.Expect(err).NotTo(o.HaveOccurred(), "should not encounter an error authenticating as keycloak user") copiedOC := *oc - tokenOC := copiedOC.WithToken(keycloakCli.AccessToken()) + token := keycloakCli.AccessToken() + tokenOC := copiedOC.WithToken(token) ssr, err := tokenOC.KubeClient().AuthenticationV1().SelfSubjectReviews().Create(ctx, &authnv1.SelfSubjectReview{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("%s-info", username), @@ -183,10 +182,12 @@ var _ = g.Describe("[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow] gomega.Expect(err).NotTo(o.HaveOccurred(), "should be able to create a SelfSubjectReview") }).WithTimeout(5 * time.Minute).WithPolling(10 * time.Second).Should(o.Succeed()) - err := resetAuthentication(ctx, oc, originalAuth) + err, modified := resetAuthentication(ctx, oc, originalAuth) o.Expect(err).NotTo(o.HaveOccurred(), "should not encounter an error reverting authentication to original state") - waitForRollout(ctx, oc) + if modified { + waitForRollout(ctx, oc) + } }) g.It("should rollout configuration on the kube-apiserver successfully", func() { @@ -328,45 +329,42 @@ var _ = g.Describe("[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow] }) }) - g.Describe("[Skipped] with invalid specified UID or Extra claim mappings", func() { + g.Describe("with invalid specified UID or Extra claim mappings", func() { g.It("should reject admission when UID claim expression is not compilable CEL", func() { - g.Skip("functionality not yet implemented") - /* - _, _, err := configureOIDCAuthentication(ctx, oc, func(o *configv1.OIDCProvider) { - o.ClaimMappings.UID = &configv1.TokenClaimOrExpressionMapping{ - Expression: "!@&*#^", - } - }) - o.Expect(err).To(o.HaveOccurred(), "should encounter an error configuring OIDC authentication") - */ + _, _, err := configureOIDCAuthentication(ctx, oc, keycloakNamespace, func(o *configv1.OIDCProvider) { + o.ClaimMappings.UID = &configv1.TokenClaimOrExpressionMapping{ + Expression: "!@&*#^", + } + }) + o.Expect(err).To(o.HaveOccurred(), "should encounter an error configuring OIDC authentication") }) g.It("should reject admission when Extra claim expression is not compilable CEL", func() { - g.Skip("functionality not yet implemented") - /* - _, _, err := configureOIDCAuthentication(ctx, oc, func(o *configv1.OIDCProvider) { - o.ClaimMappings.Extra = []configv1.ExtraMapping{ - { - Key: "payload/test", - ValueExpression: "!@*&#^!@(*&^", - }, - } - }) - o.Expect(err).To(o.HaveOccurred(), "should encounter an error configuring OIDC authentication") - */ + _, _, err := configureOIDCAuthentication(ctx, oc, keycloakNamespace, func(o *configv1.OIDCProvider) { + o.ClaimMappings.Extra = []configv1.ExtraMapping{ + { + Key: "payload/test", + ValueExpression: "!@*&#^!@(*&^", + }, + } + }) + o.Expect(err).To(o.HaveOccurred(), "should encounter an error configuring OIDC authentication") }) }) }) }) g.AfterAll(func() { - err := removeResources(ctx, cleanups...) - o.Expect(err).NotTo(o.HaveOccurred(), "should not encounter an error cleaning up keycloak resources") - - err = resetAuthentication(ctx, oc, originalAuth) + err, modified := resetAuthentication(ctx, oc, originalAuth) o.Expect(err).NotTo(o.HaveOccurred(), "should not encounter an error reverting authentication to original state") - waitForRollout(ctx, oc) + // Only if we modified the Authentication resource during the reset should we wait for a rollout + if modified { + waitForRollout(ctx, oc) + } + + err = removeResources(ctx, cleanups...) + o.Expect(err).NotTo(o.HaveOccurred(), "should not encounter an error cleaning up keycloak resources") }) }) @@ -477,11 +475,12 @@ func admittedURLForRoute(ctx context.Context, client *exutil.CLI, routeName, nam return fmt.Sprintf("https://%s", admittedURL), err } -func resetAuthentication(ctx context.Context, client *exutil.CLI, original *configv1.Authentication) error { +func resetAuthentication(ctx context.Context, client *exutil.CLI, original *configv1.Authentication) (error, bool) { if original == nil { - return nil + return nil, false } + modified := false timeoutCtx, cancel := context.WithDeadline(ctx, time.Now().Add(5*time.Minute)) defer cancel() cli := client.AdminConfigClient().ConfigV1().Authentications() @@ -491,17 +490,24 @@ func resetAuthentication(ctx context.Context, client *exutil.CLI, original *conf return false, fmt.Errorf("getting the current authentications.config.openshift.io/cluster: %w", err) } + if equality.Semantic.DeepEqual(current.Spec, original.Spec) { + return true, nil + } + current.Spec = original.Spec + modified = true _, err = cli.Update(ctx, current, metav1.UpdateOptions{}) if err != nil { - return false, err + // Only log the error so we continue to retry until the context has timed out + fmt.Println("updating authentication resource:", err) + return false, nil } return true, nil }) - return err + return err, modified } func waitForRollout(ctx context.Context, client *exutil.CLI) { @@ -523,8 +529,8 @@ func waitForRollout(ctx context.Context, client *exutil.CLI) { } gomega.Expect(found).To(o.BeTrue(), "should have found the NodeInstallerProgressing condition") - gomega.Expect(nipCond.Status).To(o.Equal(operatorv1.ConditionTrue), "NodeInstallerProgressing condition should be True") - }).WithTimeout(5*time.Minute).WithPolling(10*time.Second).Should(o.Succeed(), "should eventually begin rolling out a new revision") + gomega.Expect(nipCond.Status).To(o.Equal(operatorv1.ConditionTrue), "NodeInstallerProgressing condition should be True", nipCond) + }).WithTimeout(10*time.Minute).WithPolling(20*time.Second).Should(o.Succeed(), "should eventually begin rolling out a new revision") // Then wait for it to flip back o.Eventually(func(gomega o.Gomega) { @@ -542,6 +548,6 @@ func waitForRollout(ctx context.Context, client *exutil.CLI) { } gomega.Expect(found).To(o.BeTrue(), "should have found the NodeInstallerProgressing condition") - gomega.Expect(nipCond.Status).To(o.Equal(operatorv1.ConditionFalse), "NodeInstallerProgressing condition should be True") + gomega.Expect(nipCond.Status).To(o.Equal(operatorv1.ConditionFalse), "NodeInstallerProgressing condition should be False", nipCond) }).WithTimeout(30*time.Minute).WithPolling(30*time.Second).Should(o.Succeed(), "should eventually rollout out a new revision successfully") } diff --git a/test/extended/util/annotate/generated/zz_generated.annotations.go b/test/extended/util/annotate/generated/zz_generated.annotations.go index 8e9684a39042..68b05170a8da 100644 --- a/test/extended/util/annotate/generated/zz_generated.annotations.go +++ b/test/extended/util/annotate/generated/zz_generated.annotations.go @@ -455,9 +455,9 @@ var Annotations = map[string]string{ "[sig-auth][Feature:UserAPI] users can manipulate groups [apigroup:user.openshift.io][apigroup:authorization.openshift.io][apigroup:project.openshift.io]": " [Suite:openshift/conformance/parallel]", - "[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow][Disruptive] [OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] external IdP is configured [Skipped] with invalid specified UID or Extra claim mappings should reject admission when Extra claim expression is not compilable CEL": "", + "[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow][Disruptive] [OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] external IdP is configured with invalid specified UID or Extra claim mappings should reject admission when Extra claim expression is not compilable CEL": "", - "[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow][Disruptive] [OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] external IdP is configured [Skipped] with invalid specified UID or Extra claim mappings should reject admission when UID claim expression is not compilable CEL": "", + "[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow][Disruptive] [OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] external IdP is configured with invalid specified UID or Extra claim mappings should reject admission when UID claim expression is not compilable CEL": "", "[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow][Disruptive] [OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] external IdP is configured with valid specified UID or Extra claim mappings checking cluster identity mapping should map Extra correctly": "", @@ -465,8 +465,6 @@ var Annotations = map[string]string{ "[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow][Disruptive] [OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] external IdP is configured without specified UID or Extra claim mappings should default UID to the 'sub' claim in the access token from the IdP": "", - "[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow][Disruptive] [OCPFeatureGate:ExternalOIDC] external IdP is configured [Skipped] should remove the OpenShift OAuth stack": "", - "[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow][Disruptive] [OCPFeatureGate:ExternalOIDC] external IdP is configured should accept authentication via a certificate-based kubeconfig (break-glass)": "", "[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow][Disruptive] [OCPFeatureGate:ExternalOIDC] external IdP is configured should configure kube-apiserver": "", @@ -475,6 +473,8 @@ var Annotations = map[string]string{ "[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow][Disruptive] [OCPFeatureGate:ExternalOIDC] external IdP is configured should not accept tokens provided by the OAuth server": "", + "[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow][Disruptive] [OCPFeatureGate:ExternalOIDC] external IdP is configured should remove the OpenShift OAuth stack": "", + "[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow][Disruptive] [OCPFeatureGate:ExternalOIDC] reverting to IntegratedOAuth should accept tokens provided by the OpenShift OAuth server": "", "[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow][Disruptive] [OCPFeatureGate:ExternalOIDC] reverting to IntegratedOAuth should not accept tokens provided by an external IdP": "", diff --git a/zz_generated.manifests/test-reporting.yaml b/zz_generated.manifests/test-reporting.yaml index fab6d27c3f52..e236a4dd5fee 100644 --- a/zz_generated.manifests/test-reporting.yaml +++ b/zz_generated.manifests/test-reporting.yaml @@ -336,9 +336,6 @@ spec: when enabled' - featureGate: ExternalOIDC tests: - - testName: '[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow][Disruptive] - [OCPFeatureGate:ExternalOIDC] external IdP is configured [Skipped] should - remove the OpenShift OAuth stack' - testName: '[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow][Disruptive] [OCPFeatureGate:ExternalOIDC] external IdP is configured should accept authentication via a certificate-based kubeconfig (break-glass)' @@ -351,6 +348,9 @@ spec: - testName: '[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow][Disruptive] [OCPFeatureGate:ExternalOIDC] external IdP is configured should not accept tokens provided by the OAuth server' + - testName: '[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow][Disruptive] + [OCPFeatureGate:ExternalOIDC] external IdP is configured should remove the + OpenShift OAuth stack' - testName: '[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow][Disruptive] [OCPFeatureGate:ExternalOIDC] reverting to IntegratedOAuth should accept tokens provided by the OpenShift OAuth server' @@ -367,12 +367,12 @@ spec: tests: - testName: '[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow][Disruptive] [OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] external IdP is - configured [Skipped] with invalid specified UID or Extra claim mappings should - reject admission when Extra claim expression is not compilable CEL' + configured with invalid specified UID or Extra claim mappings should reject + admission when Extra claim expression is not compilable CEL' - testName: '[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow][Disruptive] [OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] external IdP is - configured [Skipped] with invalid specified UID or Extra claim mappings should - reject admission when UID claim expression is not compilable CEL' + configured with invalid specified UID or Extra claim mappings should reject + admission when UID claim expression is not compilable CEL' - testName: '[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow][Disruptive] [OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] external IdP is configured with valid specified UID or Extra claim mappings checking cluster