Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Tolerate nil secret when tokenEndpointAuthMethod: none #53

Merged
merged 1 commit into from
Mar 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions controllers/oauth2client_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -310,15 +313,15 @@ 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 {
return nil, errors.New(`"client_id property missing"`)
}

psw, found := secret.Data[ClientSecretKey]
if !found {
if !found && authMethod != "none" {
return nil, errors.New(`"client_secret property missing"`)
}

Expand Down
80 changes: 80 additions & 0 deletions controllers/oauth2client_controller_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
})
})
})
Expand Down
4 changes: 3 additions & 1 deletion hydra/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}