Merge pull request #1275 from ccojocar/client-update-api

Extend the API with a function which updates the client configuration

api/api.proto

@@ -36,6 +36,20 @@ message DeleteClientResp {
   bool not_found = 1;
 }
 
+// UpdateClientReq is a request to update an exisitng client.
+message UpdateClientReq {
+    string id = 1;
+    repeated string redirect_uris = 2;
+    repeated string trusted_peers = 3;
+    string name = 4;
+    string logo_url = 5;
+}
+
+// UpdateClientResp returns the reponse form updating a client.
+message UpdateClientResp {
+    bool not_found = 1;
+}
+
 // TODO(ericchiang): expand this.
 
 // Password is an email for password mapping managed by the storage.
@@ -138,6 +152,8 @@ message RevokeRefreshResp {
 service Dex {
   // CreateClient creates a client.
   rpc CreateClient(CreateClientReq) returns (CreateClientResp) {};
+  // UpdateClient updates an existing client
+  rpc UpdateClient(UpdateClientReq) returns (UpdateClientResp) {};
   // DeleteClient deletes the provided client.
   rpc DeleteClient(DeleteClientReq) returns (DeleteClientResp) {};
   // CreatePassword creates a password.

server/api.go

@@ -80,6 +80,37 @@ func (d dexAPI) CreateClient(ctx context.Context, req *api.CreateClientReq) (*ap
 	}, nil
 }
 
+func (d dexAPI) UpdateClient(ctx context.Context, req *api.UpdateClientReq) (*api.UpdateClientResp, error) {
+	if req.Id == "" {
+		return nil, errors.New("update client: no client ID supplied")
+	}
+
+	err := d.s.UpdateClient(req.Id, func(old storage.Client) (storage.Client, error) {
+		if req.RedirectUris != nil {
+			old.RedirectURIs = req.RedirectUris
+		}
+		if req.TrustedPeers != nil {
+			old.TrustedPeers = req.TrustedPeers
+		}
+		if req.Name != "" {
+			old.Name = req.Name
+		}
+		if req.LogoUrl != "" {
+			old.LogoURL = req.LogoUrl
+		}
+		return old, nil
+	})
+
+	if err != nil {
+		if err == storage.ErrNotFound {
+			return &api.UpdateClientResp{NotFound: true}, nil
+		}
+		d.logger.Errorf("api: failed to update the client: %v", err)
+		return nil, fmt.Errorf("update client: %v", err)
+	}
+	return &api.UpdateClientResp{}, nil
+}
+
 func (d dexAPI) DeleteClient(ctx context.Context, req *api.DeleteClientReq) (*api.DeleteClientResp, error) {
 	err := d.s.DeleteClient(req.Id)
 	if err != nil {

server/api_test.go

@@ -290,3 +290,171 @@ func TestRefreshToken(t *testing.T) {
 		t.Fatalf("Refresh token returned inspite of revoking it.")
 	}
 }
+
+func TestUpdateClient(t *testing.T) {
+	logger := &logrus.Logger{
+		Out:       os.Stderr,
+		Formatter: &logrus.TextFormatter{DisableColors: true},
+		Level:     logrus.DebugLevel,
+	}
+
+	s := memory.New(logger)
+	client := newAPI(s, logger, t)
+	defer client.Close()
+	ctx := context.Background()
+
+	createClient := func(t *testing.T, clientId string) {
+		resp, err := client.CreateClient(ctx, &api.CreateClientReq{
+			Client: &api.Client{
+				Id:           clientId,
+				Secret:       "",
+				RedirectUris: []string{},
+				TrustedPeers: nil,
+				Public:       true,
+				Name:         "",
+				LogoUrl:      "",
+			},
+		})
+		if err != nil {
+			t.Fatalf("unable to create the client: %v", err)
+		}
+
+		if resp == nil {
+			t.Fatalf("create client returned no response")
+		}
+		if resp.AlreadyExists {
+			t.Error("existing client was found")
+		}
+
+		if resp.Client == nil {
+			t.Fatalf("no client created")
+		}
+	}
+
+	deleteClient := func(t *testing.T, clientId string) {
+		resp, err := client.DeleteClient(ctx, &api.DeleteClientReq{
+			Id: clientId,
+		})
+		if err != nil {
+			t.Fatalf("unable to delete the client: %v", err)
+		}
+		if resp == nil {
+			t.Fatalf("delete client delete client returned no response")
+		}
+	}
+
+	tests := map[string]struct {
+		setup   func(t *testing.T, clientId string)
+		cleanup func(t *testing.T, clientId string)
+		req     *api.UpdateClientReq
+		wantErr bool
+		want    *api.UpdateClientResp
+	}{
+		"update client": {
+			setup:   createClient,
+			cleanup: deleteClient,
+			req: &api.UpdateClientReq{
+				Id:           "test",
+				RedirectUris: []string{"https://redirect"},
+				TrustedPeers: []string{"test"},
+				Name:         "test",
+				LogoUrl:      "https://logout",
+			},
+			wantErr: false,
+			want: &api.UpdateClientResp{
+				NotFound: false,
+			},
+		},
+		"update client without ID": {
+			setup:   createClient,
+			cleanup: deleteClient,
+			req: &api.UpdateClientReq{
+				Id:           "",
+				RedirectUris: nil,
+				TrustedPeers: nil,
+				Name:         "test",
+				LogoUrl:      "test",
+			},
+			wantErr: true,
+			want: &api.UpdateClientResp{
+				NotFound: false,
+			},
+		},
+		"update client which not exists ": {
+			req: &api.UpdateClientReq{
+				Id:           "test",
+				RedirectUris: nil,
+				TrustedPeers: nil,
+				Name:         "test",
+				LogoUrl:      "test",
+			},
+			wantErr: true,
+			want: &api.UpdateClientResp{
+				NotFound: false,
+			},
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			if tc.setup != nil {
+				tc.setup(t, tc.req.Id)
+			}
+			resp, err := client.UpdateClient(ctx, tc.req)
+			if err != nil && !tc.wantErr {
+				t.Fatalf("failed to update the client: %v", err)
+			}
+
+			if !tc.wantErr {
+				if resp == nil {
+					t.Fatalf("update client response not found")
+				}
+
+				if tc.want.NotFound != resp.NotFound {
+					t.Errorf("expected in response NotFound: %t", tc.want.NotFound)
+				}
+
+				client, err := s.GetClient(tc.req.Id)
+				if err != nil {
+					t.Errorf("no client found in the storage: %v", err)
+				}
+
+				if tc.req.Id != client.ID {
+					t.Errorf("expected stored client with ID: %s, found %s", tc.req.Id, client.ID)
+				}
+				if tc.req.Name != client.Name {
+					t.Errorf("expected stored client with Name: %s, found %s", tc.req.Name, client.Name)
+				}
+				if tc.req.LogoUrl != client.LogoURL {
+					t.Errorf("expected stored client with LogoURL: %s, found %s", tc.req.LogoUrl, client.LogoURL)
+				}
+				for _, redirectURI := range tc.req.RedirectUris {
+					found := find(redirectURI, client.RedirectURIs)
+					if !found {
+						t.Errorf("expected redirect URI: %s", redirectURI)
+					}
+				}
+				for _, peer := range tc.req.TrustedPeers {
+					found := find(peer, client.TrustedPeers)
+					if !found {
+						t.Errorf("expected trusted peer: %s", peer)
+					}
+				}
+			}
+
+			if tc.cleanup != nil {
+				tc.cleanup(t, tc.req.Id)
+			}
+
+		})
+	}
+}
+
+func find(item string, items []string) bool {
+	for _, i := range items {
+		if item == i {
+			return true
+		}
+	}
+	return false
+}