feat: disable oauth2 client deletion (#149)

api/v1alpha1/oauth2client_types.go

@@ -176,7 +176,7 @@ type OAuth2ClientSpec struct {
 
 	// +kubebuilder:validation:Enum=client_secret_basic;client_secret_post;private_key_jwt;none
 	//
-	// Indication which authentication method shoud be used for the token endpoint
+	// Indication which authentication method should be used for the token endpoint
 	TokenEndpointAuthMethod TokenEndpointAuthMethod `json:"tokenEndpointAuthMethod,omitempty"`
 
 	// TokenLifespans is the configuration to use for managing different token lifespans
@@ -219,6 +219,12 @@ type OAuth2ClientSpec struct {
 	//
 	// BackChannelLogoutURI RP URL that will cause the RP to log itself out when sent a Logout Token by the OP
 	BackChannelLogoutURI string `json:"backChannelLogoutURI,omitempty"`
+
+	// +kubebuilder:validation:Enum=1;2
+	//
+	// Indicates if a deleted OAuth2Client custom resource should delete the database row or not.
+	// Value 1 means deletion of the OAuth2 client, value 2 means keep an orphan oauth2 client.
+	DeletionPolicy OAuth2ClientDeletionPolicy `json:"deletionPolicy,omitempty"`
 }
 
 // GrantType represents an OAuth 2.0 grant type
@@ -265,6 +271,14 @@ const (
 	OAuth2ClientConditionReady = "Ready"
 )
 
+// OAuth2ClientDeletionPolicy represents if a deleted oauth2 client object should delete the database row or not.
+type OAuth2ClientDeletionPolicy int
+
+const (
+	OAuth2ClientDeletionPolicyDelete = iota + 1
+	OAuth2ClientDeletionPolicyOrphan
+)
+
 // +kubebuilder:validation:Enum=True;False;Unknown
 type ConditionStatus string
 

api/v1alpha1/oauth2client_types_test.go

@@ -109,6 +109,7 @@ func TestCreateAPI(t *testing.T) {
 				"invalid lifespan refresh token access token":       func() { created.Spec.TokenLifespans.RefreshTokenGrantAccessTokenLifespan = "invalid" },
 				"invalid lifespan refresh token id token":           func() { created.Spec.TokenLifespans.RefreshTokenGrantIdTokenLifespan = "invalid" },
 				"invalid lifespan refresh token refresh token":      func() { created.Spec.TokenLifespans.RefreshTokenGrantRefreshTokenLifespan = "invalid" },
+				"invalid deletion policy":                           func() { created.Spec.DeletionPolicy = -1 },
 			} {
 				t.Run(fmt.Sprintf("case=%s", desc), func(t *testing.T) {
 					resetTestClient()

config/crd/bases/hydra.ory.sh_oauth2clients.yaml

@@ -76,6 +76,14 @@ spec:
                     ClientName is the human-readable string name of the client
                     to be presented to the end-user during authorization.
                   type: string
+                deletionPolicy:
+                  description: |-
+                    Indicates if a deleted OAuth2Client custom resource should delete the database row or not.
+                    Value 0 means deletion of the OAuth2 client, value 1 means keep an orphan oauth2 client.
+                  enum:
+                    - 0
+                    - 1
+                  type: integer
                 frontChannelLogoutSessionRequired:
                   default: false
                   description:
@@ -238,8 +246,8 @@ spec:
                         - private_key_jwt
                         - none
                   description:
-                    Indication which authentication method shoud be used for the
+                    Indication which authentication method should be used for
-                    token endpoint
+                    the token endpoint
                   type: string
                 tokenLifespans:
                   description: |-

controllers/oauth2client_controller.go

@@ -328,7 +328,6 @@ func (r *OAuth2ClientReconciler) updateRegisteredOAuth2Client(ctx context.Contex
 }
 
 func (r *OAuth2ClientReconciler) unregisterOAuth2Clients(ctx context.Context, c *hydrav1alpha1.OAuth2Client) error {
-
 	// if a required field is empty, that means this is deleted after
 	// the finalizers have done their job, so just return
 	if c.Spec.Scope == "" || c.Spec.SecretName == "" {
@@ -347,6 +346,11 @@ func (r *OAuth2ClientReconciler) unregisterOAuth2Clients(ctx context.Context, c
 
 	for _, cJSON := range clients {
 		if cJSON.Owner == fmt.Sprintf("%s/%s", c.Name, c.Namespace) {
+			if c.Spec.DeletionPolicy == hydrav1alpha1.OAuth2ClientDeletionPolicyOrphan {
+				// Do not delete the OAuth2 client.
+				r.Log.Info("oauth2 client deletion, leave the row orphan")
+				return nil
+			}
 			if err := h.DeleteOAuth2Client(*cJSON.ClientID); err != nil {
 				return err
 			}

controllers/oauth2client_controller_integration_test.go

@@ -447,6 +447,180 @@ var _ = Describe("OAuth2Client Controller", func() {
 				//Ensure manager is stopped properly
 				stopMgr.Done()
 			})
+
+			It("not delete OAuth2 clients with Orphan deletion policy", func() {
+				tstName, tstClientID, tstSecretName := "test-orphan", "testClientID-orphan", "my-secret-orphan"
+				expectedRequest := &reconcile.Request{NamespacedName: types.NamespacedName{Name: tstName, Namespace: tstNamespace}}
+
+				s := runtime.NewScheme()
+				err := hydrav1alpha1.AddToScheme(s)
+				Expect(err).NotTo(HaveOccurred())
+
+				err = apiv1.AddToScheme(s)
+				Expect(err).NotTo(HaveOccurred())
+
+				// Setup the Manager and Controller.  Wrap the Controller Reconcile function so it writes each request to a
+				// channel when it is finished.
+				mgr, err := manager.New(cfg, manager.Options{
+					Scheme: s,
+					Metrics: server.Options{
+						BindAddress: ":8086",
+					},
+				})
+				Expect(err).NotTo(HaveOccurred())
+				c := mgr.GetClient()
+
+				deleteHasHappened := false
+				mch := &mocks.Client{}
+				mch.On("GetOAuth2Client", Anything).Return(nil, false, nil)
+				mch.On("DeleteOAuth2Client", Anything).Return(func(id string) error {
+					deleteHasHappened = true
+					return nil
+				})
+				mch.On("ListOAuth2Client", Anything).Return(func() []*hydra.OAuth2ClientJSON {
+					return []*hydra.OAuth2ClientJSON{
+						{
+							ClientID: &tstClientID,
+							Secret:   ptr.To(tstSecret),
+							Owner:    fmt.Sprintf("%s/%s", tstName, tstNamespace),
+						},
+					}
+				}, nil)
+				mch.On("PostOAuth2Client", AnythingOfType("*hydra.OAuth2ClientJSON")).Return(func(o *hydra.OAuth2ClientJSON) *hydra.OAuth2ClientJSON {
+					return &hydra.OAuth2ClientJSON{
+						ClientID:      &tstClientID,
+						Secret:        ptr.To(tstSecret),
+						GrantTypes:    o.GrantTypes,
+						ResponseTypes: o.ResponseTypes,
+						RedirectURIs:  o.RedirectURIs,
+						Scope:         o.Scope,
+						Audience:      o.Audience,
+						Owner:         o.Owner,
+					}
+				}, func(o *hydra.OAuth2ClientJSON) error {
+					return nil
+				})
+
+				recFn, requests := SetupTestReconcile(getAPIReconciler(mgr, mch))
+				Expect(add(mgr, recFn)).To(Succeed())
+
+				//Start the manager and the controller
+				stopMgr := StartTestManager(mgr)
+
+				// Create OAuth2 client with 'Orphan' policy
+				instance := testInstance(tstName, tstSecretName)
+				instance.Spec.DeletionPolicy = hydrav1alpha1.OAuth2ClientDeletionPolicyOrphan
+
+				// Call creation API, to actually create the CRD.
+				err = c.Create(context.TODO(), instance)
+				if apierrors.IsInvalid(err) {
+					Fail(fmt.Sprintf("failed to create object, got an invalid object error: %v", err))
+					return
+				}
+				Expect(err).NotTo(HaveOccurred())
+				Eventually(requests, timeout).Should(Receive(Equal(*expectedRequest)))
+
+				// Call deletion API, which should not really delete the CRD because we are in orphan mode.
+				err = c.Delete(context.TODO(), instance)
+				if apierrors.IsInvalid(err) {
+					Fail(fmt.Sprintf("failed to delete object, got an invalid object error: %v", err))
+					return
+				}
+				Expect(err).NotTo(HaveOccurred())
+				Eventually(requests, timeout).Should(Receive(Equal(*expectedRequest)))
+
+				Expect(deleteHasHappened).To(BeFalse())
+
+				//Ensure manager is stopped properly
+				stopMgr.Done()
+			})
+
+			It("delete OAuth2 clients with Delete deletion policy", func() {
+				tstName, tstClientID, tstSecretName := "test-delete", "testClientID-delete", "my-secret-delete"
+				expectedRequest := &reconcile.Request{NamespacedName: types.NamespacedName{Name: tstName, Namespace: tstNamespace}}
+
+				s := runtime.NewScheme()
+				err := hydrav1alpha1.AddToScheme(s)
+				Expect(err).NotTo(HaveOccurred())
+
+				err = apiv1.AddToScheme(s)
+				Expect(err).NotTo(HaveOccurred())
+
+				// Setup the Manager and Controller.  Wrap the Controller Reconcile function so it writes each request to a
+				// channel when it is finished.
+				mgr, err := manager.New(cfg, manager.Options{
+					Scheme: s,
+					Metrics: server.Options{
+						BindAddress: ":8087",
+					},
+				})
+				Expect(err).NotTo(HaveOccurred())
+				c := mgr.GetClient()
+
+				deleteHasHappened := false
+				mch := &mocks.Client{}
+				mch.On("GetOAuth2Client", Anything).Return(nil, false, nil)
+				mch.On("DeleteOAuth2Client", AnythingOfType("string")).Return(func(id string) error {
+					deleteHasHappened = true
+					return nil
+				})
+				mch.On("ListOAuth2Client", Anything).Return(func() []*hydra.OAuth2ClientJSON {
+					return []*hydra.OAuth2ClientJSON{
+						{
+							ClientID: &tstClientID,
+							Secret:   ptr.To(tstSecret),
+							Owner:    fmt.Sprintf("%s/%s", tstName, tstNamespace),
+						},
+					}
+				}, nil)
+				mch.On("PostOAuth2Client", AnythingOfType("*hydra.OAuth2ClientJSON")).Return(func(o *hydra.OAuth2ClientJSON) *hydra.OAuth2ClientJSON {
+					return &hydra.OAuth2ClientJSON{
+						ClientID:      &tstClientID,
+						Secret:        ptr.To(tstSecret),
+						GrantTypes:    o.GrantTypes,
+						ResponseTypes: o.ResponseTypes,
+						RedirectURIs:  o.RedirectURIs,
+						Scope:         o.Scope,
+						Audience:      o.Audience,
+						Owner:         o.Owner,
+					}
+				}, func(o *hydra.OAuth2ClientJSON) error {
+					return nil
+				})
+
+				recFn, requests := SetupTestReconcile(getAPIReconciler(mgr, mch))
+				Expect(add(mgr, recFn)).To(Succeed())
+
+				//Start the manager and the controller
+				stopMgr := StartTestManager(mgr)
+
+				// Create OAuth2 client with 'Delete' policy
+				instance := testInstance(tstName, tstSecretName)
+				instance.Spec.DeletionPolicy = hydrav1alpha1.OAuth2ClientDeletionPolicyDelete
+
+				// Call creation API, to actually create the CRD.
+				err = c.Create(context.TODO(), instance)
+				if apierrors.IsInvalid(err) {
+					Fail(fmt.Sprintf("failed to create object, got an invalid object error: %v", err))
+					return
+				}
+				Expect(err).NotTo(HaveOccurred())
+				Eventually(requests, timeout).Should(Receive(Equal(*expectedRequest)))
+
+				// Call deletion API, which should really delete the CRD because we are in orphan mode.
+				err = c.Delete(context.TODO(), instance)
+				if apierrors.IsInvalid(err) {
+					Fail(fmt.Sprintf("failed to delete object, got an invalid object error: %v", err))
+					return
+				}
+				Expect(err).NotTo(HaveOccurred())
+				Eventually(requests, timeout).Should(Receive(Equal(*expectedRequest)))
+
+				Expect(deleteHasHappened).To(BeTrue())
+
+				// Ensure manager is stopped properly.
+				stopMgr.Done()
+			})
 		})
 	})
 })