diff --git a/api/v1alpha1/oauth2client_types.go b/api/v1alpha1/oauth2client_types.go index 3304fd2..bf81d90 100644 --- a/api/v1alpha1/oauth2client_types.go +++ b/api/v1alpha1/oauth2client_types.go @@ -16,6 +16,8 @@ limitations under the License. package v1alpha1 import ( + "fmt" + "github.com/ory/hydra-maester/hydra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -110,10 +112,10 @@ func init() { // ToOAuth2ClientJSON converts an OAuth2Client into a OAuth2ClientJSON object that represents an OAuth2 client digestible by ORY Hydra func (c *OAuth2Client) ToOAuth2ClientJSON() *hydra.OAuth2ClientJSON { return &hydra.OAuth2ClientJSON{ - Name: c.Name, GrantTypes: grantToStringSlice(c.Spec.GrantTypes), ResponseTypes: responseToStringSlice(c.Spec.ResponseTypes), Scope: c.Spec.Scope, + Owner: fmt.Sprintf("%s/%s", c.Name, c.Namespace), } } diff --git a/controllers/mocks/HydraClientInterface.go b/controllers/mocks/HydraClientInterface.go index f8a2c52..84943a4 100644 --- a/controllers/mocks/HydraClientInterface.go +++ b/controllers/mocks/HydraClientInterface.go @@ -54,6 +54,29 @@ func (_m *HydraClientInterface) GetOAuth2Client(id string) (*hydra.OAuth2ClientJ return r0, r1, r2 } +// ListOAuth2Client provides a mock function with given fields: +func (_m *HydraClientInterface) ListOAuth2Client() ([]*hydra.OAuth2ClientJSON, error) { + ret := _m.Called() + + var r0 []*hydra.OAuth2ClientJSON + if rf, ok := ret.Get(0).(func() []*hydra.OAuth2ClientJSON); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*hydra.OAuth2ClientJSON) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func() error); ok { + r1 = rf() + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // PostOAuth2Client provides a mock function with given fields: o func (_m *HydraClientInterface) PostOAuth2Client(o *hydra.OAuth2ClientJSON) (*hydra.OAuth2ClientJSON, error) { ret := _m.Called(o) diff --git a/controllers/oauth2client_controller.go b/controllers/oauth2client_controller.go index 225e21e..e47ae6d 100644 --- a/controllers/oauth2client_controller.go +++ b/controllers/oauth2client_controller.go @@ -34,11 +34,11 @@ import ( const ( ClientIDKey = "client_id" ClientSecretKey = "client_secret" - ownerLabel = "owner" ) type HydraClientInterface interface { GetOAuth2Client(id string) (*hydra.OAuth2ClientJSON, bool, error) + ListOAuth2Client() ([]*hydra.OAuth2ClientJSON, error) PostOAuth2Client(o *hydra.OAuth2ClientJSON) (*hydra.OAuth2ClientJSON, error) PutOAuth2Client(o *hydra.OAuth2ClientJSON) (*hydra.OAuth2ClientJSON, error) DeleteOAuth2Client(id string) error @@ -62,8 +62,8 @@ func (r *OAuth2ClientReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error var oauth2client hydrav1alpha1.OAuth2Client if err := r.Get(ctx, req.NamespacedName, &oauth2client); err != nil { if apierrs.IsNotFound(err) { - if err := r.unregisterOAuth2Clients(ctx, req.Name, req.Namespace); err != nil { - return ctrl.Result{}, err + if registerErr := r.unregisterOAuth2Clients(ctx, req.Name, req.Namespace); registerErr != nil { + return ctrl.Result{}, registerErr } return ctrl.Result{}, nil } @@ -75,7 +75,10 @@ func (r *OAuth2ClientReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error var secret apiv1.Secret if err := r.Get(ctx, types.NamespacedName{Name: oauth2client.Spec.SecretName, Namespace: req.Namespace}, &secret); err != nil { if apierrs.IsNotFound(err) { - return ctrl.Result{}, r.registerOAuth2Client(ctx, &oauth2client, nil) + if registerErr := r.registerOAuth2Client(ctx, &oauth2client, nil); registerErr != nil { + return ctrl.Result{}, registerErr + } + return ctrl.Result{}, nil } return ctrl.Result{}, err } @@ -83,24 +86,36 @@ func (r *OAuth2ClientReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error credentials, err := parseSecret(secret) if err != nil { r.Log.Error(err, fmt.Sprintf("secret %s/%s is invalid", secret.Name, secret.Namespace)) - return ctrl.Result{}, r.updateReconciliationStatusError(ctx, &oauth2client, hydrav1alpha1.StatusInvalidSecret, err) - } - - if err := r.labelSecretWithOwnerReference(ctx, &oauth2client, secret); err != nil { - return ctrl.Result{}, err + if updateErr := r.updateReconciliationStatusError(ctx, &oauth2client, hydrav1alpha1.StatusInvalidSecret, err); updateErr != nil { + return ctrl.Result{}, updateErr + } + return ctrl.Result{}, nil } - _, found, err := r.HydraClient.GetOAuth2Client(string(credentials.ID)) + fetched, found, err := r.HydraClient.GetOAuth2Client(string(credentials.ID)) if err != nil { return ctrl.Result{}, err } if found { - return ctrl.Result{}, r.updateRegisteredOAuth2Client(ctx, &oauth2client, credentials) + if fetched.Owner != oauth2client.Name { + conflictErr := errors.Errorf("ID provided in secret %s/%s is assigned to another resource", secret.Name, secret.Namespace) + if updateErr := r.updateReconciliationStatusError(ctx, &oauth2client, hydrav1alpha1.StatusInvalidSecret, conflictErr); updateErr != nil { + return ctrl.Result{}, updateErr + } + return ctrl.Result{}, nil + } + + if updateErr := r.updateRegisteredOAuth2Client(ctx, &oauth2client, credentials); updateErr != nil { + return ctrl.Result{}, updateErr + } + return ctrl.Result{}, nil } - return ctrl.Result{}, r.registerOAuth2Client(ctx, &oauth2client, credentials) + if registerErr := r.registerOAuth2Client(ctx, &oauth2client, credentials); registerErr != nil { + return ctrl.Result{}, registerErr + } } return ctrl.Result{}, nil @@ -119,21 +134,25 @@ func (r *OAuth2ClientReconciler) registerOAuth2Client(ctx context.Context, c *hy if credentials != nil { if _, err := r.HydraClient.PostOAuth2Client(c.ToOAuth2ClientJSON().WithCredentials(credentials)); err != nil { - return r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusRegistrationFailed, err) + if updateErr := r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusRegistrationFailed, err); updateErr != nil { + return updateErr + } } - return nil + return r.ensureEmptyStatusError(ctx, c) } created, err := r.HydraClient.PostOAuth2Client(c.ToOAuth2ClientJSON()) if err != nil { - return r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusRegistrationFailed, err) + if updateErr := r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusRegistrationFailed, err); updateErr != nil { + return updateErr + } + return nil } clientSecret := apiv1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: c.Spec.SecretName, Namespace: c.Namespace, - Labels: map[string]string{ownerLabel: c.Name}, }, Data: map[string][]byte{ ClientIDKey: []byte(*created.ClientID), @@ -142,44 +161,35 @@ func (r *OAuth2ClientReconciler) registerOAuth2Client(ctx context.Context, c *hy } if err := r.Create(ctx, &clientSecret); err != nil { - return r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusCreateSecretFailed, err) + if updateErr := r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusCreateSecretFailed, err); updateErr != nil { + return updateErr + } } - return nil + return r.ensureEmptyStatusError(ctx, c) } func (r *OAuth2ClientReconciler) updateRegisteredOAuth2Client(ctx context.Context, c *hydrav1alpha1.OAuth2Client, credentials *hydra.Oauth2ClientCredentials) error { if _, err := r.HydraClient.PutOAuth2Client(c.ToOAuth2ClientJSON().WithCredentials(credentials)); err != nil { - return r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusUpdateFailed, err) + if updateErr := r.updateReconciliationStatusError(ctx, c, hydrav1alpha1.StatusUpdateFailed, err); updateErr != nil { + return updateErr + } } - return nil + return r.ensureEmptyStatusError(ctx, c) } func (r *OAuth2ClientReconciler) unregisterOAuth2Clients(ctx context.Context, name, namespace string) error { - var secretList apiv1.SecretList - - err := r.List( - ctx, - &secretList, - client.InNamespace(namespace), - client.MatchingLabels(map[string]string{ownerLabel: name})) + clients, err := r.HydraClient.ListOAuth2Client() if err != nil { return err } - if len(secretList.Items) == 0 { - return nil - } - - ids := make(map[string]struct{}) - for _, s := range secretList.Items { - ids[string(s.Data[ClientIDKey])] = struct{}{} - } - - for id := range ids { - if err := r.HydraClient.DeleteOAuth2Client(id); err != nil { - return err + for _, c := range clients { + if c.Owner == fmt.Sprintf("%s/%s", name, namespace) { + if err := r.HydraClient.DeleteOAuth2Client(*c.ClientID); err != nil { + return err + } } } @@ -188,16 +198,25 @@ func (r *OAuth2ClientReconciler) unregisterOAuth2Clients(ctx context.Context, na func (r *OAuth2ClientReconciler) updateReconciliationStatusError(ctx context.Context, c *hydrav1alpha1.OAuth2Client, code hydrav1alpha1.StatusCode, err error) error { r.Log.Error(err, fmt.Sprintf("error processing client %s/%s ", c.Name, c.Namespace), "oauth2client", "register") - c.Status.ObservedGeneration = c.Generation c.Status.ReconciliationError = hydrav1alpha1.ReconciliationError{ Code: code, Description: err.Error(), } - if updateErr := r.Status().Update(ctx, c); updateErr != nil { - r.Log.Error(updateErr, fmt.Sprintf("status update failed for client %s/%s ", c.Name, c.Namespace), "oauth2client", "update status") - return updateErr - } + return r.updateClientStatus(ctx, c) +} + +func (r *OAuth2ClientReconciler) ensureEmptyStatusError(ctx context.Context, c *hydrav1alpha1.OAuth2Client) error { + c.Status.ReconciliationError = hydrav1alpha1.ReconciliationError{} + return r.updateClientStatus(ctx, c) +} + +func (r *OAuth2ClientReconciler) updateClientStatus(ctx context.Context, c *hydrav1alpha1.OAuth2Client) error { + c.Status.ObservedGeneration = c.Generation + if err := r.Status().Update(ctx, c); err != nil { + r.Log.Error(err, fmt.Sprintf("status update failed for client %s/%s ", c.Name, c.Namespace), "oauth2client", "update status") + return err + } return nil } @@ -218,16 +237,3 @@ func parseSecret(secret apiv1.Secret) (*hydra.Oauth2ClientCredentials, error) { Password: psw, }, nil } - -func (r *OAuth2ClientReconciler) labelSecretWithOwnerReference(ctx context.Context, c *hydrav1alpha1.OAuth2Client, secret apiv1.Secret) error { - - if secret.Labels[ownerLabel] != c.Name { - if secret.Labels == nil { - secret.Labels = make(map[string]string, 1) - } - secret.Labels[ownerLabel] = c.Name - return r.Update(ctx, &secret) - } - - return nil -} diff --git a/controllers/oauth2client_controller_integration_test.go b/controllers/oauth2client_controller_integration_test.go index 4d6169c..1ca2e7a 100644 --- a/controllers/oauth2client_controller_integration_test.go +++ b/controllers/oauth2client_controller_integration_test.go @@ -61,14 +61,15 @@ var _ = Describe("OAuth2Client Controller", func() { mch := &mocks.HydraClientInterface{} 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: pointer.StringPtr(tstSecret), - Name: o.Name, GrantTypes: o.GrantTypes, ResponseTypes: o.ResponseTypes, Scope: o.Scope, + Owner: o.Owner, } }, func(o *hydra.OAuth2ClientJSON) error { return nil @@ -137,6 +138,7 @@ var _ = Describe("OAuth2Client Controller", func() { mch := &mocks.HydraClientInterface{} mch.On("PostOAuth2Client", Anything).Return(nil, errors.New("error")) mch.On("DeleteOAuth2Client", Anything).Return(nil) + mch.On("ListOAuth2Client", Anything).Return(nil, nil) recFn, requests := SetupTestReconcile(getAPIReconciler(mgr, mch)) @@ -201,15 +203,16 @@ var _ = Describe("OAuth2Client Controller", func() { mch := mocks.HydraClientInterface{} mch.On("DeleteOAuth2Client", Anything).Return(nil) + mch.On("ListOAuth2Client", Anything).Return(nil, nil) mch.On("GetOAuth2Client", Anything).Return(nil, false, nil) mch.On("PostOAuth2Client", AnythingOfType("*hydra.OAuth2ClientJSON")).Return(func(o *hydra.OAuth2ClientJSON) *hydra.OAuth2ClientJSON { postedClient = &hydra.OAuth2ClientJSON{ ClientID: o.ClientID, Secret: o.Secret, - Name: o.Name, GrantTypes: o.GrantTypes, ResponseTypes: o.ResponseTypes, Scope: o.Scope, + Owner: o.Owner, } return postedClient }, func(o *hydra.OAuth2ClientJSON) error { @@ -287,6 +290,7 @@ var _ = Describe("OAuth2Client Controller", func() { mch := mocks.HydraClientInterface{} mch.On("DeleteOAuth2Client", Anything).Return(nil) + mch.On("ListOAuth2Client", Anything).Return(nil, nil) recFn, requests := SetupTestReconcile(getAPIReconciler(mgr, &mch)) diff --git a/hydra/client.go b/hydra/client.go index be28dab..6f6dd33 100644 --- a/hydra/client.go +++ b/hydra/client.go @@ -39,6 +39,28 @@ func (c *Client) GetOAuth2Client(id string) (*OAuth2ClientJSON, bool, error) { } } +func (c *Client) ListOAuth2Client() ([]*OAuth2ClientJSON, error) { + + var jsonClientList []*OAuth2ClientJSON + + req, err := c.newRequest(http.MethodGet, "", nil) + if err != nil { + return nil, err + } + + resp, err := c.do(req, &jsonClientList) + if err != nil { + return nil, err + } + + switch resp.StatusCode { + case http.StatusOK: + return jsonClientList, nil + default: + return nil, fmt.Errorf("%s %s http request returned unexpected status code %s", req.Method, req.URL.String(), resp.Status) + } +} + func (c *Client) PostOAuth2Client(o *OAuth2ClientJSON) (*OAuth2ClientJSON, error) { var jsonClient *OAuth2ClientJSON @@ -57,7 +79,7 @@ func (c *Client) PostOAuth2Client(o *OAuth2ClientJSON) (*OAuth2ClientJSON, error case http.StatusCreated: return jsonClient, nil case http.StatusConflict: - return nil, fmt.Errorf(" %s %s http request failed: requested ID already exists", req.Method, req.URL) + return nil, fmt.Errorf("%s %s http request failed: requested ID already exists", req.Method, req.URL) default: return nil, fmt.Errorf("%s %s http request returned unexpected status code: %s", req.Method, req.URL, resp.Status) } diff --git a/hydra/client_test.go b/hydra/client_test.go index fa566c8..b9ceb63 100644 --- a/hydra/client_test.go +++ b/hydra/client_test.go @@ -19,10 +19,13 @@ import ( const ( testID = "test-id" schemeHTTP = "http" - testClient = `{"client_id":"test-id","client_name":"test-name","scope":"some,scopes","grant_types":["type1"]}` - testClientCreated = `{"client_id":"test-id-2", "client_secret": "TmGkvcY7k526","client_name":"test-name-2","scope":"some,other,scopes","grant_types":["type2"]}` - testClientUpdated = `{"client_id":"test-id-3", "client_secret": "xFoPPm654por","client_name":"test-name-3","scope":"yet,another,scope","grant_types":["type3"]}` + testClient = `{"client_id":"test-id","owner":"test-name","scope":"some,scopes","grant_types":["type1"]}` + testClientCreated = `{"client_id":"test-id-2","client_secret":"TmGkvcY7k526","owner":"test-name-2","scope":"some,other,scopes","grant_types":["type2"]}` + testClientUpdated = `{"client_id":"test-id-3","client_secret":"xFoPPm654por","owner":"test-name-3","scope":"yet,another,scope","grant_types":["type3"]}` + testClientList = `{"client_id":"test-id-4","owner":"test-name-4","scope":"scope1 scope2","grant_types":["type4"]}` + testClientList2 = `{"client_id":"test-id-5","owner":"test-name-5","scope":"scope3 scope4","grant_types":["type5"]}` emptyBody = `{}` + emptyBodyList = `[]` clientsEndpoint = "/clients" ) @@ -33,16 +36,16 @@ type server struct { } var testOAuthJSONPost = &hydra.OAuth2ClientJSON{ - Name: "test-name-2", Scope: "some,other,scopes", GrantTypes: []string{"type2"}, + Owner: "test-name-2", } var testOAuthJSONPut = &hydra.OAuth2ClientJSON{ ClientID: pointer.StringPtr("test-id-3"), - Name: "test-name-3", Scope: "yet,another,scope", GrantTypes: []string{"type3"}, + Owner: "test-name-3", } func TestCRUD(t *testing.T) { @@ -160,9 +163,9 @@ func TestCRUD(t *testing.T) { if new { require.NotNil(t, o) - assert.Equal(testOAuthJSONPost.Name, o.Name) assert.Equal(testOAuthJSONPost.Scope, o.Scope) assert.Equal(testOAuthJSONPost.GrantTypes, o.GrantTypes) + assert.Equal(testOAuthJSONPost.Owner, o.Owner) assert.NotNil(o.Secret) assert.NotNil(o.ClientID) } @@ -213,10 +216,10 @@ func TestCRUD(t *testing.T) { if ok { require.NotNil(t, o) - assert.Equal(testOAuthJSONPut.Name, o.Name) assert.Equal(testOAuthJSONPut.Scope, o.Scope) assert.Equal(testOAuthJSONPut.GrantTypes, o.GrantTypes) assert.Equal(testOAuthJSONPut.ClientID, o.ClientID) + assert.Equal(testOAuthJSONPut.Owner, o.Owner) assert.NotNil(o.Secret) } }) @@ -260,6 +263,61 @@ func TestCRUD(t *testing.T) { }) } }) + + t.Run("method=list", func(t *testing.T) { + + for d, tc := range map[string]server{ + "no clients": { + http.StatusOK, + emptyBodyList, + nil, + }, + "one client": { + http.StatusOK, + fmt.Sprintf("[%s]", testClientList), + nil, + }, + "more clients": { + http.StatusOK, + fmt.Sprintf("[%s,%s]", testClientList, testClientList2), + nil, + }, + "internal server error when requesting": { + http.StatusInternalServerError, + emptyBodyList, + errors.New("http request returned unexpected status code"), + }, + } { + t.Run(fmt.Sprintf("case/%s", d), func(t *testing.T) { + + //given + h := http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + assert.Equal(c.HydraURL.String(), fmt.Sprintf("%s://%s%s", schemeHTTP, req.Host, req.URL.Path)) + assert.Equal(http.MethodGet, req.Method) + w.WriteHeader(tc.statusCode) + w.Write([]byte(tc.respBody)) + w.Header().Set("Content-type", "application/json") + + }) + runServer(&c, h) + + //when + list, err := c.ListOAuth2Client() + + //then + if tc.err == nil { + require.NoError(t, err) + require.NotNil(t, list) + var expectedList []*hydra.OAuth2ClientJSON + json.Unmarshal([]byte(tc.respBody), &expectedList) + assert.Equal(expectedList, list) + } else { + require.Error(t, err) + assert.Contains(err.Error(), tc.err.Error()) + } + }) + } + }) } func runServer(c *hydra.Client, h http.HandlerFunc) { diff --git a/hydra/types.go b/hydra/types.go index fe128d8..7a14801 100644 --- a/hydra/types.go +++ b/hydra/types.go @@ -6,10 +6,10 @@ import "k8s.io/utils/pointer" type OAuth2ClientJSON struct { ClientID *string `json:"client_id,omitempty"` Secret *string `json:"client_secret,omitempty"` - Name string `json:"client_name"` GrantTypes []string `json:"grant_types"` ResponseTypes []string `json:"response_types,omitempty"` Scope string `json:"scope"` + Owner string `json:"owner"` } // Oauth2ClientCredentials represents client ID and password fetched from a Kubernetes secret