client: remove ClientManagerFromClients

Replaced by ClientRepoFromClients, which makes more sense IMO. Also, it
was doing the wrong thing: it was ignoring the client_id and client_secret
passed into it as far as I can tell.

client/manager/manager.go

@@ -2,7 +2,6 @@ package manager
 
 import (
 	"encoding/base64"
-	"fmt"
 
 	"errors"
 
@@ -64,35 +63,6 @@ func NewClientManager(clientRepo client.ClientRepo, txnFactory repo.TransactionF
 	}
 }
 
-func NewClientManagerFromClients(clientRepo client.ClientRepo, txnFactory repo.TransactionFactory, clients []client.Client, options ManagerOptions) (*ClientManager, error) {
-	clientManager := NewClientManager(clientRepo, txnFactory, options)
-	tx, err := clientManager.begin()
-	if err != nil {
-		return nil, err
-	}
-	defer tx.Rollback()
-
-	for _, c := range clients {
-		if c.Credentials.Secret == "" {
-			return nil, fmt.Errorf("client %q has no secret", c.Credentials.ID)
-		}
-
-		err := clientManager.addClientCredentials(&c)
-		if err != nil {
-			return nil, err
-		}
-
-		_, err = clientRepo.New(tx, c)
-		if err != nil {
-			return nil, err
-		}
-	}
-	if err := tx.Commit(); err != nil {
-		return nil, err
-	}
-	return clientManager, nil
-}
-
 func (m *ClientManager) New(cli client.Client) (*oidc.ClientCredentials, error) {
 	tx, err := m.begin()
 	if err != nil {

client/manager/manager_test.go

@@ -44,11 +44,14 @@ func makeTestFixtures() *testFixtures {
 	secGen := func() ([]byte, error) {
 		return []byte("secret"), nil
 	}
-	f.clientRepo = db.NewClientRepo(dbMap)
+
-	clientManager, err := NewClientManagerFromClients(f.clientRepo, db.TransactionFactory(dbMap), clients, ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen})
+	var err error
+	f.clientRepo, err = db.NewClientRepoFromClients(dbMap, clients)
 	if err != nil {
 		panic("Failed to create client manager: " + err.Error())
 	}
+
+	clientManager := NewClientManager(f.clientRepo, db.TransactionFactory(dbMap), ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen})
 	f.mgr = clientManager
 	return f
 }

db/client.go

@@ -199,6 +199,18 @@ func (r *clientRepo) All(tx repo.Transaction) ([]client.Client, error) {
 	return cs, nil
 }
 
+func NewClientRepoFromClients(dbm *gorp.DbMap, cs []client.Client) (client.ClientRepo, error) {
+	repo := NewClientRepo(dbm).(*clientRepo)
+	for _, c := range cs {
+		cm, err := newClientModel(c)
+		if err != nil {
+			return nil, err
+		}
+		err = repo.executor(nil).Insert(cm)
+	}
+	return repo, nil
+}
+
 func (r *clientRepo) get(tx repo.Transaction, clientID string) (client.Client, error) {
 	cm, err := r.getModel(tx, clientID)
 	if err != nil {

functional/repo/refresh_repo_test.go

@@ -12,7 +12,6 @@ import (
 	"github.com/kylelemons/godebug/pretty"
 
 	"github.com/coreos/dex/client"
-	"github.com/coreos/dex/client/manager"
 	"github.com/coreos/dex/db"
 	"github.com/coreos/dex/refresh"
 	"github.com/coreos/dex/user"
@@ -28,9 +27,7 @@ func newRefreshRepo(t *testing.T, users []user.UserWithRemoteIdentities, clients
 	if _, err := db.NewUserRepoFromUsers(dbMap, users); err != nil {
 		t.Fatalf("Unable to add users: %v", err)
 	}
-	if _, err := manager.NewClientManagerFromClients(db.NewClientRepo(dbMap), db.TransactionFactory(dbMap), clients, manager.ManagerOptions{}); err != nil {
+
-		t.Fatalf("Unable to add clients: %v", err)
-	}
 	return db.NewRefreshTokenRepo(dbMap)
 }
 

integration/common_test.go

@@ -12,6 +12,8 @@ import (
 	"github.com/go-gorp/gorp"
 	"github.com/jonboulle/clockwork"
 
+	"github.com/coreos/dex/client"
+	clientmanager "github.com/coreos/dex/client/manager"
 	"github.com/coreos/dex/connector"
 	"github.com/coreos/dex/db"
 	"github.com/coreos/dex/user"
@@ -79,3 +81,19 @@ func makeUserObjects(users []user.UserWithRemoteIdentities, passwords []user.Pas
 	um.Clock = clock
 	return dbMap, ur, pwr, um
 }
+
+func makeClientRepoAndManager(dbMap *gorp.DbMap, clients []client.Client) (client.ClientRepo, *clientmanager.ClientManager, error) {
+	clientIDGenerator := func(hostport string) (string, error) {
+		return hostport, nil
+	}
+	secGen := func() ([]byte, error) {
+		return []byte("secret"), nil
+	}
+	clientRepo, err := db.NewClientRepoFromClients(dbMap, clients)
+	if err != nil {
+		return nil, nil, err
+	}
+	clientManager := clientmanager.NewClientManager(clientRepo, db.TransactionFactory(dbMap), clientmanager.ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen})
+	return clientRepo, clientManager, nil
+
+}

integration/oidc_test.go

@@ -9,8 +9,12 @@ import (
 	"testing"
 	"time"
 
+	"github.com/coreos/go-oidc/jose"
+	"github.com/coreos/go-oidc/key"
+	"github.com/coreos/go-oidc/oauth2"
+	"github.com/coreos/go-oidc/oidc"
+
 	"github.com/coreos/dex/client"
-	clientmanager "github.com/coreos/dex/client/manager"
 	"github.com/coreos/dex/connector"
 	"github.com/coreos/dex/db"
 	phttp "github.com/coreos/dex/pkg/http"
@@ -18,10 +22,6 @@ import (
 	"github.com/coreos/dex/server"
 	"github.com/coreos/dex/session/manager"
 	"github.com/coreos/dex/user"
-	"github.com/coreos/go-oidc/jose"
-	"github.com/coreos/go-oidc/key"
-	"github.com/coreos/go-oidc/oauth2"
-	"github.com/coreos/go-oidc/oidc"
 )
 
 func mockServer(cis []client.Client) (*server.Server, error) {
@@ -37,14 +37,7 @@ func mockServer(cis []client.Client) (*server.Server, error) {
 		return nil, err
 	}
 
-	clientIDGenerator := func(hostport string) (string, error) {
+	clientRepo, clientManager, err := makeClientRepoAndManager(dbMap, cis)
-		return hostport, nil
-	}
-	secGen := func() ([]byte, error) {
-		return []byte("secret"), nil
-	}
-	clientRepo := db.NewClientRepo(dbMap)
-	clientManager, err := clientmanager.NewClientManagerFromClients(clientRepo, db.TransactionFactory(dbMap), cis, clientmanager.ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen})
 	if err != nil {
 		return nil, err
 	}
@@ -150,18 +143,12 @@ func TestHTTPExchangeTokenRefreshToken(t *testing.T) {
 		},
 	}
 
-	clientIDGenerator := func(hostport string) (string, error) {
-		return hostport, nil
-	}
-	secGen := func() ([]byte, error) {
-		return []byte("secret"), nil
-	}
 	dbMap := db.NewMemDB()
-	clientRepo := db.NewClientRepo(dbMap)
+	clientRepo, clientManager, err := makeClientRepoAndManager(dbMap, []client.Client{ci})
-	clientManager, err := clientmanager.NewClientManagerFromClients(clientRepo, db.TransactionFactory(dbMap), []client.Client{ci}, clientmanager.ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen})
 	if err != nil {
 		t.Fatalf("Failed to create client identity manager: " + err.Error())
 	}
+
 	passwordInfoRepo, err := db.NewPasswordInfoRepoFromPasswordInfos(db.NewMemDB(), []user.PasswordInfo{passwordInfo})
 	if err != nil {
 		t.Fatalf("Failed to create password info repo: %v", err)

integration/user_api_test.go

@@ -18,7 +18,6 @@ import (
 	"google.golang.org/api/googleapi"
 
 	"github.com/coreos/dex/client"
-	"github.com/coreos/dex/client/manager"
 	"github.com/coreos/dex/db"
 	schema "github.com/coreos/dex/schema/workerschema"
 	"github.com/coreos/dex/server"
@@ -126,14 +125,8 @@ func makeUserAPITestFixtures() *userAPITestFixtures {
 			},
 		},
 	}
-	clientIDGenerator := func(hostport string) (string, error) {
+
-		return hostport, nil
+	_, clientManager, err := makeClientRepoAndManager(dbMap, clients)
-	}
-	secGen := func() ([]byte, error) {
-		return []byte(testClientSecret), nil
-	}
-	clientRepo := db.NewClientRepo(dbMap)
-	clientManager, err := manager.NewClientManagerFromClients(clientRepo, db.TransactionFactory(dbMap), clients, manager.ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen})
 	if err != nil {
 		panic("Failed to create client identity manager: " + err.Error())
 	}

server/config.go

@@ -116,10 +116,9 @@ func (cfg *SingleServerConfig) Configure(srv *Server) error {
 		return fmt.Errorf("unable to read clients from file %s: %v", cfg.ClientsFile, err)
 	}
 
-	clientRepo := db.NewClientRepo(dbMap)
+	clientRepo, err := db.NewClientRepoFromClients(dbMap, clients)
-
+	if err != nil {
-	for _, c := range clients {
+		return err
-		clientRepo.New(nil, c)
 	}
 
 	f, err := os.Open(cfg.ConnectorsFile)
@@ -158,7 +157,7 @@ func (cfg *SingleServerConfig) Configure(srv *Server) error {
 
 	txnFactory := db.TransactionFactory(dbMap)
 	userManager := usermanager.NewUserManager(userRepo, pwiRepo, cfgRepo, txnFactory, usermanager.ManagerOptions{})
-	clientManager, err := clientmanager.NewClientManagerFromClients(clientRepo, db.TransactionFactory(dbMap), clients, clientmanager.ManagerOptions{})
+	clientManager := clientmanager.NewClientManager(clientRepo, db.TransactionFactory(dbMap), clientmanager.ManagerOptions{})
 	if err != nil {
 		return fmt.Errorf("Failed to create client identity manager: %v", err)
 	}

server/testutil.go

@@ -180,11 +180,13 @@ func makeTestFixturesWithOptions(options testFixtureOptions) (*testFixtures, err
 	secGen := func() ([]byte, error) {
 		return []byte("secret"), nil
 	}
-	clientRepo := db.NewClientRepo(dbMap)
+	clientRepo, err := db.NewClientRepoFromClients(dbMap, clients)
-	clientManager, err := clientmanager.NewClientManagerFromClients(clientRepo, db.TransactionFactory(dbMap), clients, clientmanager.ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen})
 	if err != nil {
 		return nil, err
 	}
+
+	clientManager := clientmanager.NewClientManager(clientRepo, db.TransactionFactory(dbMap), clientmanager.ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen})
+
 	km := key.NewPrivateKeyManager()
 	err = km.Set(key.NewPrivateKeySet([]*key.PrivateKey{testPrivKey}, time.Now().Add(time.Minute)))
 	if err != nil {

user/api/api_test.go

@@ -176,11 +176,11 @@ func makeTestFixtures() (*UsersAPI, *testEmailer) {
 	secGen := func() ([]byte, error) {
 		return []byte("secret"), nil
 	}
-	clientRepo := db.NewClientRepo(dbMap)
+	clientRepo, err := db.NewClientRepoFromClients(dbMap, []client.Client{ci})
-	clientManager, err := clientmanager.NewClientManagerFromClients(clientRepo, db.TransactionFactory(dbMap), []client.Client{ci}, clientmanager.ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen})
 	if err != nil {
 		panic("Failed to create client manager: " + err.Error())
 	}
+	clientManager := clientmanager.NewClientManager(clientRepo, db.TransactionFactory(dbMap), clientmanager.ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen})
 
 	// Used in TestRevokeRefreshToken test.
 	refreshTokens := []struct {