fix: tolerate nil secret when tokenEndpointAuthMethod is none (#53)

Signed-off-by: Clément BUCHART <clement@buchart.dev>

controllers/oauth2client_controller.go

@@ -128,7 +128,7 @@ func (r *OAuth2ClientReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error
 		return ctrl.Result{}, err
 	}
 
-	credentials, err := parseSecret(secret)
+	credentials, err := parseSecret(secret, oauth2client.Spec.TokenEndpointAuthMethod)
 	if err != nil {
 		r.Log.Error(err, fmt.Sprintf("secret %s/%s is invalid", secret.Name, secret.Namespace))
 		if updateErr := r.updateReconciliationStatusError(ctx, &oauth2client, hydrav1alpha1.StatusInvalidSecret, err); updateErr != nil {
@@ -229,11 +229,14 @@ func (r *OAuth2ClientReconciler) registerOAuth2Client(ctx context.Context, c *hy
 			}},
 		},
 		Data: map[string][]byte{
-			ClientIDKey:     []byte(*created.ClientID),
-			ClientSecretKey: []byte(*created.Secret),
+			ClientIDKey: []byte(*created.ClientID),
 		},
 	}
 
+	if created.Secret != nil {
+		clientSecret.Data[ClientSecretKey] = []byte(*created.Secret)
+	}
+
 	if err := r.Create(ctx, &clientSecret); err != nil {
 		if updateErr := r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusCreateSecretFailed, err); updateErr != nil {
 			return updateErr
@@ -310,7 +313,7 @@ func (r *OAuth2ClientReconciler) updateClientStatus(ctx context.Context, c *hydr
 	return nil
 }
 
-func parseSecret(secret apiv1.Secret) (*hydra.Oauth2ClientCredentials, error) {
+func parseSecret(secret apiv1.Secret, authMethod hydrav1alpha1.TokenEndpointAuthMethod) (*hydra.Oauth2ClientCredentials, error) {
 
 	id, found := secret.Data[ClientIDKey]
 	if !found {
@@ -318,7 +321,7 @@ func parseSecret(secret apiv1.Secret) (*hydra.Oauth2ClientCredentials, error) {
 	}
 
 	psw, found := secret.Data[ClientSecretKey]
-	if !found {
+	if !found && authMethod != "none" {
 		return nil, errors.New(`"client_secret property missing"`)
 	}
 

controllers/oauth2client_controller_integration_test.go

@@ -355,6 +355,86 @@ var _ = Describe("OAuth2Client Controller", func() {
 				close(stopMgr)
 				mgrStopped.Wait()
 			})
+
+			It("tolerate nil client_secret if tokenEndpointAuthMethod is none", func() {
+				tstName, tstClientID, tstSecretName := "test5", "testClientID-5", "my-secret-without-client-secret"
+				expectedRequest := &reconcile.Request{NamespacedName: types.NamespacedName{Name: tstName, Namespace: tstNamespace}}
+
+				s := scheme.Scheme
+				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})
+				Expect(err).NotTo(HaveOccurred())
+				c := mgr.GetClient()
+
+				mch := &mocks.HydraClientInterface{}
+				mch.On("GetOAuth2Client", Anything).Return(nil, false, nil)
+				mch.On("DeleteOAuth2Client", Anything).Return(nil)
+				mch.On("ListOAuth2Client", Anything).Return(nil, nil)
+				mch.On("PostOAuth2Client", AnythingOfType("*hydra.OAuth2ClientJSON")).Return(func(o *hydra.OAuth2ClientJSON) *hydra.OAuth2ClientJSON {
+					return &hydra.OAuth2ClientJSON{
+						ClientID:      &tstClientID,
+						Secret:        nil,
+						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, mgrStopped := StartTestManager(mgr)
+
+				instance := testInstance(tstName, tstSecretName)
+				instance.Spec.TokenEndpointAuthMethod = "none"
+				err = c.Create(context.TODO(), instance)
+				// The instance object may not be a valid object because it might be missing some required fields.
+				// Please modify the instance object by adding required fields and then remove the following if statement.
+				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)))
+
+				//Verify the created CR instance status
+				var retrieved hydrav1alpha1.OAuth2Client
+				ok := client.ObjectKey{Name: tstName, Namespace: tstNamespace}
+				err = c.Get(context.TODO(), ok, &retrieved)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(retrieved.Status.ReconciliationError.Code).To(BeEmpty())
+				Expect(retrieved.Status.ReconciliationError.Description).To(BeEmpty())
+
+				//Verify the created Secret
+				var createdSecret apiv1.Secret
+				ok = client.ObjectKey{Name: tstSecretName, Namespace: tstNamespace}
+				err = k8sClient.Get(context.TODO(), ok, &createdSecret)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(createdSecret.Data[controllers.ClientIDKey]).To(Equal([]byte(tstClientID)))
+				Expect(createdSecret.Data[controllers.ClientSecretKey]).To(BeNil())
+				Expect(createdSecret.OwnerReferences).To(Equal(getOwnerReferenceTo(retrieved)))
+
+				//delete instance
+				c.Delete(context.TODO(), instance)
+
+				//Ensure manager is stopped properly
+				close(stopMgr)
+				mgrStopped.Wait()
+			})
 		})
 	})
 })

hydra/types.go

@@ -28,6 +28,8 @@ type Oauth2ClientCredentials struct {
 
 func (oj *OAuth2ClientJSON) WithCredentials(credentials *Oauth2ClientCredentials) *OAuth2ClientJSON {
 	oj.ClientID = pointer.StringPtr(string(credentials.ID))
-	oj.Secret = pointer.StringPtr(string(credentials.Password))
+	if credentials.Password != nil {
+		oj.Secret = pointer.StringPtr(string(credentials.Password))
+	}
 	return oj
 }