From 86ef34d8e212a108368f977af451a4d03a38277e Mon Sep 17 00:00:00 2001 From: Bobby Rullo Date: Thu, 2 Jun 2016 18:39:58 -0700 Subject: [PATCH 1/6] client: generateClientCreds -> addClientCreds a little easier to read this way IMO. --- client/manager/manager.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/client/manager/manager.go b/client/manager/manager.go index 7435cf2c..10b11c10 100644 --- a/client/manager/manager.go +++ b/client/manager/manager.go @@ -77,12 +77,12 @@ func NewClientManagerFromClients(clientRepo client.ClientRepo, txnFactory repo.T return nil, fmt.Errorf("client %q has no secret", c.Credentials.ID) } - cli, err := clientManager.generateClientCredentials(c) + err := clientManager.addClientCredentials(&c) if err != nil { return nil, err } - _, err = clientRepo.New(tx, cli) + _, err = clientRepo.New(tx, c) if err != nil { return nil, err } @@ -100,15 +100,15 @@ func (m *ClientManager) New(cli client.Client) (*oidc.ClientCredentials, error) } defer tx.Rollback() - c, err := m.generateClientCredentials(cli) + err = m.addClientCredentials(&cli) if err != nil { return nil, err } - creds := c.Credentials + creds := cli.Credentials // Save Client - _, err = m.clientRepo.New(tx, c) + _, err = m.clientRepo.New(tx, cli) if err != nil { return nil, err } @@ -189,25 +189,25 @@ func (m *ClientManager) Authenticate(creds oidc.ClientCredentials) (bool, error) return ok, nil } -func (m *ClientManager) generateClientCredentials(cli client.Client) (client.Client, error) { +func (m *ClientManager) addClientCredentials(cli *client.Client) error { // Generate Client ID if len(cli.Metadata.RedirectURIs) < 1 { - return cli, errors.New("no client redirect url given") + return errors.New("no client redirect url given") } clientID, err := m.clientIDGenerator(cli.Metadata.RedirectURIs[0].Host) if err != nil { - return cli, err + return err } // Generate Secret secret, err := m.secretGenerator() if err != nil { - return cli, err + return err } clientSecret := base64.URLEncoding.EncodeToString(secret) cli.Credentials = oidc.ClientCredentials{ ID: clientID, Secret: clientSecret, } - return cli, nil + return nil } From 8d1a6f2324a00b6a85ad20dc6e3c68482f0d90c9 Mon Sep 17 00:00:00 2001 From: Bobby Rullo Date: Fri, 3 Jun 2016 11:19:06 -0700 Subject: [PATCH 2/6] functional: test sample clients file is valid Also tests that it's being loaded properly (which is not the case in NewClientManagerFromClients, which will be removed in subsequent commit) --- functional/config/config_sample_test.go | 46 +++++++++++++++++++++++++ test-functional | 1 + 2 files changed, 47 insertions(+) create mode 100644 functional/config/config_sample_test.go diff --git a/functional/config/config_sample_test.go b/functional/config/config_sample_test.go new file mode 100644 index 00000000..af6b55b5 --- /dev/null +++ b/functional/config/config_sample_test.go @@ -0,0 +1,46 @@ +package config + +import ( + "os" + "testing" + + "github.com/coreos/dex/client" + "github.com/coreos/dex/client/manager" + "github.com/coreos/dex/db" +) + +const ( + clientsFile = "../../static/fixtures/clients.json.sample" +) + +// TestClientSample makes sure that the clients.json.sample file is valid and can be loaded properly. +func TestClientSample(t *testing.T) { + f, err := os.Open(clientsFile) + if err != nil { + t.Fatalf("could not open file %q: %v", clientsFile, err) + } + defer f.Close() + + clients, err := client.ClientsFromReader(f) + if err != nil { + t.Fatalf("Error loading Clients: %v", err) + } + + memDB := db.NewMemDB() + repo := db.NewClientRepo(memDB) + for _, c := range clients { + repo.New(nil, c) + } + mgr := manager.NewClientManager(repo, db.TransactionFactory(memDB), manager.ManagerOptions{}) + + for i, c := range clients { + ok, err := mgr.Authenticate(c.Credentials) + if !ok { + t.Errorf("case %d: couldn't authenticate", i) + } + if err != nil { + t.Errorf("case %d: error authenticating: %v", i, err) + } + } + +} diff --git a/test-functional b/test-functional index 5552bc27..bb703b83 100755 --- a/test-functional +++ b/test-functional @@ -4,3 +4,4 @@ source ./env go test $@ github.com/coreos/dex/functional go test $@ github.com/coreos/dex/functional/repo +go test $@ github.com/coreos/dex/functional/config From ad1d5ab253c44db28eb7c1b7d477ffb6583118b1 Mon Sep 17 00:00:00 2001 From: Bobby Rullo Date: Mon, 6 Jun 2016 13:03:07 -0700 Subject: [PATCH 3/6] server: remove boilerplate setup code Use the test fixture setup stuff in testutil instead. --- server/server_test.go | 495 ++++++++---------------------------------- server/testutil.go | 27 ++- 2 files changed, 110 insertions(+), 412 deletions(-) diff --git a/server/server_test.go b/server/server_test.go index 0a8816fc..f0eb30ce 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -10,7 +10,6 @@ import ( "time" "github.com/coreos/dex/client" - clientmanager "github.com/coreos/dex/client/manager" "github.com/coreos/dex/db" "github.com/coreos/dex/refresh/refreshtest" "github.com/coreos/dex/session/manager" @@ -22,7 +21,6 @@ import ( "github.com/kylelemons/godebug/pretty" ) -var clientTestSecret = base64.URLEncoding.EncodeToString([]byte("secret")) var validRedirURL = url.URL{ Scheme: "http", Host: "client.example.com", @@ -185,110 +183,42 @@ func TestServerNewSession(t *testing.T) { } func TestServerLogin(t *testing.T) { - ci := client.Client{ - Credentials: oidc.ClientCredentials{ - ID: testClientID, - Secret: clientTestSecret, - }, - Metadata: oidc.ClientMetadata{ - RedirectURIs: []url.URL{ - url.URL{ - Scheme: "http", - Host: "client.example.com", - Path: "/callback", - }, - }, - }, - } - - dbm := db.NewMemDB() - clientRepo := db.NewClientRepo(dbm) - clientManager, err := clientmanager.NewClientManagerFromClients(clientRepo, db.TransactionFactory(dbm), []client.Client{ci}, clientmanager.ManagerOptions{}) + f, err := makeTestFixtures() if err != nil { - t.Fatalf("Failed to create client identity manager: %v", err) + t.Fatalf("error making test fixtures: %v", err) } - km := &StaticKeyManager{ - signer: &StaticSigner{sig: []byte("beer"), err: nil}, - } + sm := f.sessionManager + sessionID, err := sm.NewSession("IDPC-1", testClientID, "bogus", testRedirectURL, "", false, []string{"openid"}) - sm := manager.NewSessionManager(db.NewSessionRepo(db.NewMemDB()), db.NewSessionKeyRepo(db.NewMemDB())) - sm.GenerateCode = staticGenerateCodeFunc("fakecode") - sessionID, err := sm.NewSession("test_connector_id", ci.Credentials.ID, "bogus", ci.Metadata.RedirectURIs[0], "", false, []string{"openid"}) - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } - - userRepo, err := makeNewUserRepo() - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } - - srv := &Server{ - IssuerURL: url.URL{Scheme: "http", Host: "server.example.com"}, - KeyManager: km, - SessionManager: sm, - ClientRepo: clientRepo, - ClientManager: clientManager, - UserRepo: userRepo, - } - - ident := oidc.Identity{ID: "YYY", Name: "elroy", Email: "elroy@example.com"} + ident := oidc.Identity{ID: testUserRemoteID1, Name: "elroy", Email: testUserEmail1} key, err := sm.NewSessionKey(sessionID) if err != nil { t.Fatalf("Unexpected error: %v", err) } - redirectURL, err := srv.Login(ident, key) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + redirectURL, err := f.srv.Login(ident, key) if err != nil { t.Fatalf("Unexpected err from Server.Login: %v", err) } - wantRedirectURL := "http://client.example.com/callback?code=fakecode&state=bogus" + wantRedirectURL := "http://client.example.com/callback?code=code-3&state=bogus" if wantRedirectURL != redirectURL { t.Fatalf("Unexpected redirectURL: want=%q, got=%q", wantRedirectURL, redirectURL) } } func TestServerLoginUnrecognizedSessionKey(t *testing.T) { - clients := []client.Client{ - client.Client{ - Credentials: oidc.ClientCredentials{ - ID: testClientID, Secret: clientTestSecret, - }, - Metadata: oidc.ClientMetadata{ - RedirectURIs: []url.URL{ - validRedirURL, - }, - }, - }, - } - dbm := db.NewMemDB() - clientIDGenerator := func(hostport string) (string, error) { - return hostport, nil - } - secGen := func() ([]byte, error) { - return []byte("secret"), nil - } - clientRepo := db.NewClientRepo(dbm) - clientManager, err := clientmanager.NewClientManagerFromClients(clientRepo, db.TransactionFactory(dbm), clients, clientmanager.ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen}) + f, err := makeTestFixtures() if err != nil { - t.Fatalf("Failed to create client identity manager: %v", err) - } - km := &StaticKeyManager{ - signer: &StaticSigner{sig: nil, err: errors.New("fail")}, - } - sm := manager.NewSessionManager(db.NewSessionRepo(db.NewMemDB()), db.NewSessionKeyRepo(db.NewMemDB())) - srv := &Server{ - IssuerURL: url.URL{Scheme: "http", Host: "server.example.com"}, - KeyManager: km, - SessionManager: sm, - ClientRepo: clientRepo, - ClientManager: clientManager, + t.Fatalf("error making test fixtures: %v", err) } - ident := oidc.Identity{ID: "YYY", Name: "elroy", Email: "elroy@example.com"} - code, err := srv.Login(ident, testClientID) + ident := oidc.Identity{ID: testUserRemoteID1, Name: "elroy", Email: testUserEmail1} + code, err := f.srv.Login(ident, testClientID) if err == nil { t.Fatalf("Expected non-nil error") } @@ -299,47 +229,12 @@ func TestServerLoginUnrecognizedSessionKey(t *testing.T) { } func TestServerLoginDisabledUser(t *testing.T) { - ci := client.Client{ - Credentials: oidc.ClientCredentials{ - ID: testClientID, - Secret: clientTestSecret, - }, - Metadata: oidc.ClientMetadata{ - RedirectURIs: []url.URL{ - validRedirURL, - }, - }, - } - clients := []client.Client{ci} - dbm := db.NewMemDB() - clientIDGenerator := func(hostport string) (string, error) { - return hostport, nil - } - secGen := func() ([]byte, error) { - return []byte("secret"), nil - } - clientRepo := db.NewClientRepo(dbm) - clientManager, err := clientmanager.NewClientManagerFromClients(clientRepo, db.TransactionFactory(dbm), clients, clientmanager.ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen}) + f, err := makeTestFixtures() if err != nil { - t.Fatalf("Failed to create client identity manager: %v", err) - } - km := &StaticKeyManager{ - signer: &StaticSigner{sig: []byte("beer"), err: nil}, + t.Fatalf("error making test fixtures: %v", err) } - sm := manager.NewSessionManager(db.NewSessionRepo(db.NewMemDB()), db.NewSessionKeyRepo(db.NewMemDB())) - sm.GenerateCode = staticGenerateCodeFunc("fakecode") - sessionID, err := sm.NewSession("test_connector_id", ci.Credentials.ID, "bogus", ci.Metadata.RedirectURIs[0], "", false, []string{"openid"}) - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } - - userRepo, err := makeNewUserRepo() - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } - - err = userRepo.Create(nil, user.User{ + err = f.userRepo.Create(nil, user.User{ ID: "disabled-1", Email: "disabled@example.com", Disabled: true, @@ -348,79 +243,29 @@ func TestServerLoginDisabledUser(t *testing.T) { t.Fatalf("Unexpected error: %v", err) } - err = userRepo.AddRemoteIdentity(nil, "disabled-1", user.RemoteIdentity{ + err = f.userRepo.AddRemoteIdentity(nil, "disabled-1", user.RemoteIdentity{ ConnectorID: "test_connector_id", ID: "disabled-connector-id", }) - srv := &Server{ - IssuerURL: url.URL{Scheme: "http", Host: "server.example.com"}, - KeyManager: km, - SessionManager: sm, - ClientRepo: clientRepo, - ClientManager: clientManager, - UserRepo: userRepo, - } - - ident := oidc.Identity{ID: "disabled-connector-id", Name: "elroy", Email: "elroy@example.com"} - key, err := sm.NewSessionKey(sessionID) + sessionID, err := f.sessionManager.NewSession("test_connector_id", testClientID, "bogus", testRedirectURL, "", false, []string{"openid"}) if err != nil { t.Fatalf("Unexpected error: %v", err) } - _, err = srv.Login(ident, key) + ident := oidc.Identity{ID: "disabled-connector-id", Name: "elroy", Email: "elroy@example.com"} + key, err := f.sessionManager.NewSessionKey(sessionID) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + _, err = f.srv.Login(ident, key) if err == nil { t.Errorf("disabled user was allowed to log in") } } func TestServerCodeToken(t *testing.T) { - ci := client.Client{ - Credentials: oidc.ClientCredentials{ - ID: testClientID, - Secret: clientTestSecret, - }, - Metadata: oidc.ClientMetadata{ - RedirectURIs: []url.URL{ - validRedirURL, - }, - }, - } - clients := []client.Client{ci} - dbm := db.NewMemDB() - clientIDGenerator := func(hostport string) (string, error) { - return hostport, nil - } - secGen := func() ([]byte, error) { - return []byte("secret"), nil - } - clientRepo := db.NewClientRepo(dbm) - clientManager, err := clientmanager.NewClientManagerFromClients(clientRepo, db.TransactionFactory(dbm), clients, clientmanager.ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen}) - if err != nil { - t.Fatalf("Failed to create client identity manager: %v", err) - } - km := &StaticKeyManager{ - signer: &StaticSigner{sig: []byte("beer"), err: nil}, - } - sm := manager.NewSessionManager(db.NewSessionRepo(db.NewMemDB()), db.NewSessionKeyRepo(db.NewMemDB())) - - userRepo, err := makeNewUserRepo() - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } - - refreshTokenRepo := refreshtest.NewTestRefreshTokenRepo() - - srv := &Server{ - IssuerURL: url.URL{Scheme: "http", Host: "server.example.com"}, - KeyManager: km, - SessionManager: sm, - ClientRepo: clientRepo, - ClientManager: clientManager, - UserRepo: userRepo, - RefreshTokenRepo: refreshTokenRepo, - } - tests := []struct { scope []string refreshToken string @@ -440,7 +285,14 @@ func TestServerCodeToken(t *testing.T) { } for i, tt := range tests { - sessionID, err := sm.NewSession("bogus_idpc", ci.Credentials.ID, "bogus", url.URL{}, "", false, tt.scope) + f, err := makeTestFixtures() + if err != nil { + t.Fatalf("error making test fixtures: %v", err) + } + f.srv.RefreshTokenRepo = refreshtest.NewTestRefreshTokenRepo() + + sm := f.sessionManager + sessionID, err := sm.NewSession("bogus_idpc", testClientID, "bogus", url.URL{}, "", false, tt.scope) if err != nil { t.Fatalf("case %d: unexpected error: %v", i, err) } @@ -449,7 +301,7 @@ func TestServerCodeToken(t *testing.T) { t.Fatalf("case %d: unexpected error: %v", i, err) } - _, err = sm.AttachUser(sessionID, "testid-1") + _, err = sm.AttachUser(sessionID, testUserID1) if err != nil { t.Fatalf("case %d: unexpected error: %v", i, err) } @@ -459,7 +311,11 @@ func TestServerCodeToken(t *testing.T) { t.Fatalf("case %d: unexpected error: %v", i, err) } - jwt, token, err := srv.CodeToken(ci.Credentials, key) + jwt, token, err := f.srv.CodeToken( + oidc.ClientCredentials{ + ID: testClientID, + Secret: clientTestSecret, + }, key) if err != nil { t.Fatalf("case %d: unexpected error: %v", i, err) } @@ -473,45 +329,13 @@ func TestServerCodeToken(t *testing.T) { } func TestServerTokenUnrecognizedKey(t *testing.T) { - ci := client.Client{ - Credentials: oidc.ClientCredentials{ - ID: testClientID, - Secret: clientTestSecret, - }, - Metadata: oidc.ClientMetadata{ - RedirectURIs: []url.URL{ - validRedirURL, - }, - }, - } - - clients := []client.Client{ci} - dbm := db.NewMemDB() - clientIDGenerator := func(hostport string) (string, error) { - return hostport, nil - } - secGen := func() ([]byte, error) { - return []byte("secret"), nil - } - clientRepo := db.NewClientRepo(dbm) - clientManager, err := clientmanager.NewClientManagerFromClients(clientRepo, db.TransactionFactory(dbm), clients, clientmanager.ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen}) + f, err := makeTestFixtures() if err != nil { - t.Fatalf("Failed to create client identity manager: %v", err) + t.Fatalf("error making test fixtures: %v", err) } - km := &StaticKeyManager{ - signer: &StaticSigner{sig: []byte("beer"), err: nil}, - } - sm := manager.NewSessionManager(db.NewSessionRepo(db.NewMemDB()), db.NewSessionKeyRepo(db.NewMemDB())) + sm := f.sessionManager - srv := &Server{ - IssuerURL: url.URL{Scheme: "http", Host: "server.example.com"}, - KeyManager: km, - SessionManager: sm, - ClientRepo: clientRepo, - ClientManager: clientManager, - } - - sessionID, err := sm.NewSession("connector_id", ci.Credentials.ID, "bogus", url.URL{}, "", false, []string{"openid", "offline_access"}) + sessionID, err := sm.NewSession("connector_id", testClientID, "bogus", url.URL{}, "", false, []string{"openid", "offline_access"}) if err != nil { t.Fatalf("Unexpected error: %v", err) } @@ -521,7 +345,7 @@ func TestServerTokenUnrecognizedKey(t *testing.T) { t.Fatalf("Unexpected error: %v", err) } - jwt, token, err := srv.CodeToken(ci.Credentials, "foo") + jwt, token, err := f.srv.CodeToken(testClientCredentials, "foo") if err == nil { t.Fatalf("Expected non-nil error") } @@ -534,12 +358,8 @@ func TestServerTokenUnrecognizedKey(t *testing.T) { } func TestServerTokenFail(t *testing.T) { - issuerURL := url.URL{Scheme: "http", Host: "server.example.com"} keyFixture := "goodkey" - ccFixture := oidc.ClientCredentials{ - ID: testClientID, - Secret: clientTestSecret, - } + signerFixture := &StaticSigner{sig: []byte("beer"), err: nil} tests := []struct { @@ -555,7 +375,7 @@ func TestServerTokenFail(t *testing.T) { // NOTE(ericchiang): This test assumes that the database ID of the first // refresh token will be "1". signer: signerFixture, - argCC: ccFixture, + argCC: testClientCredentials, argKey: keyFixture, scope: []string{"openid", "offline_access"}, refreshToken: fmt.Sprintf("1/%s", base64.URLEncoding.EncodeToString([]byte("refresh-1"))), @@ -564,7 +384,7 @@ func TestServerTokenFail(t *testing.T) { // no 'offline_access' in 'scope', should get empty refresh token { signer: signerFixture, - argCC: ccFixture, + argCC: testClientCredentials, argKey: keyFixture, scope: []string{"openid"}, }, @@ -572,7 +392,7 @@ func TestServerTokenFail(t *testing.T) { // unrecognized key { signer: signerFixture, - argCC: ccFixture, + argCC: testClientCredentials, argKey: "foo", err: oauth2.NewError(oauth2.ErrorInvalidGrant), scope: []string{"openid", "offline_access"}, @@ -590,7 +410,7 @@ func TestServerTokenFail(t *testing.T) { // signing operation fails { signer: &StaticSigner{sig: nil, err: errors.New("fail")}, - argCC: ccFixture, + argCC: testClientCredentials, argKey: keyFixture, err: oauth2.NewError(oauth2.ErrorServerError), scope: []string{"openid", "offline_access"}, @@ -598,10 +418,19 @@ func TestServerTokenFail(t *testing.T) { } for i, tt := range tests { - sm := manager.NewSessionManager(db.NewSessionRepo(db.NewMemDB()), db.NewSessionKeyRepo(db.NewMemDB())) - sm.GenerateCode = func() (string, error) { return keyFixture, nil } - sessionID, err := sm.NewSession("connector_id", ccFixture.ID, "bogus", url.URL{}, "", false, tt.scope) + f, err := makeTestFixtures() + if err != nil { + t.Fatalf("error making test fixtures: %v", err) + } + sm := f.sessionManager + sm.GenerateCode = func() (string, error) { return keyFixture, nil } + f.srv.RefreshTokenRepo = refreshtest.NewTestRefreshTokenRepo() + f.srv.KeyManager = &StaticKeyManager{ + signer: tt.signer, + } + + sessionID, err := sm.NewSession(testConnectorID1, testClientID, "bogus", url.URL{}, "", false, tt.scope) if err != nil { t.Fatalf("Unexpected error: %v", err) } @@ -611,60 +440,17 @@ func TestServerTokenFail(t *testing.T) { t.Errorf("case %d: unexpected error: %v", i, err) continue } - km := &StaticKeyManager{ - signer: tt.signer, - } - - clients := []client.Client{ - client.Client{ - Credentials: ccFixture, - Metadata: oidc.ClientMetadata{ - RedirectURIs: []url.URL{ - validRedirURL, - }, - }, - }, - } - dbm := db.NewMemDB() - clientIDGenerator := func(hostport string) (string, error) { - return hostport, nil - } - secGen := func() ([]byte, error) { - return []byte("secret"), nil - } - clientRepo := db.NewClientRepo(dbm) - clientManager, err := clientmanager.NewClientManagerFromClients(clientRepo, db.TransactionFactory(dbm), clients, clientmanager.ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen}) - if err != nil { - t.Fatalf("Failed to create client identity manager: %v", err) - } - _, err = sm.AttachUser(sessionID, "testid-1") + _, err = sm.AttachUser(sessionID, testUserID1) if err != nil { t.Fatalf("case %d: unexpected error: %v", i, err) } - userRepo, err := makeNewUserRepo() - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } - - refreshTokenRepo := refreshtest.NewTestRefreshTokenRepo() - - srv := &Server{ - IssuerURL: issuerURL, - KeyManager: km, - SessionManager: sm, - ClientRepo: clientRepo, - ClientManager: clientManager, - UserRepo: userRepo, - RefreshTokenRepo: refreshTokenRepo, - } - _, err = sm.NewSessionKey(sessionID) if err != nil { t.Fatalf("Unexpected error: %v", err) } - jwt, token, err := srv.CodeToken(tt.argCC, tt.argKey) + jwt, token, err := f.srv.CodeToken(tt.argCC, tt.argKey) if token != tt.refreshToken { fmt.Printf("case %d: expect refresh token %q, got %q\n", i, tt.refreshToken, token) t.Fatalf("case %d: expect refresh token %q, got %q", i, tt.refreshToken, token) @@ -683,18 +469,7 @@ func TestServerTokenFail(t *testing.T) { } func TestServerRefreshToken(t *testing.T) { - issuerURL := url.URL{Scheme: "http", Host: "server.example.com"} - clientA := client.Client{ - Credentials: oidc.ClientCredentials{ - ID: testClientID, - Secret: clientTestSecret, - }, - Metadata: oidc.ClientMetadata{ - RedirectURIs: []url.URL{ - url.URL{Scheme: "https", Host: "client.example.com", Path: "one/two/three"}, - }, - }, - } + clientB := client.Client{ Credentials: oidc.ClientCredentials{ ID: "example2.com", @@ -706,7 +481,6 @@ func TestServerRefreshToken(t *testing.T) { }, }, } - signerFixture := &StaticSigner{sig: []byte("beer"), err: nil} // NOTE(ericchiang): These tests assume that the database ID of the first @@ -721,39 +495,39 @@ func TestServerRefreshToken(t *testing.T) { // Everything is good. { fmt.Sprintf("1/%s", base64.URLEncoding.EncodeToString([]byte("refresh-1"))), - clientA.Credentials.ID, - clientA.Credentials, + testClientID, + testClientCredentials, signerFixture, nil, }, // Invalid refresh token(malformatted). { "invalid-token", - clientA.Credentials.ID, - clientA.Credentials, + testClientID, + testClientCredentials, signerFixture, oauth2.NewError(oauth2.ErrorInvalidRequest), }, // Invalid refresh token(invalid payload content). { fmt.Sprintf("1/%s", base64.URLEncoding.EncodeToString([]byte("refresh-2"))), - clientA.Credentials.ID, - clientA.Credentials, + testClientID, + testClientCredentials, signerFixture, oauth2.NewError(oauth2.ErrorInvalidRequest), }, // Invalid refresh token(invalid ID content). { fmt.Sprintf("0/%s", base64.URLEncoding.EncodeToString([]byte("refresh-1"))), - clientA.Credentials.ID, - clientA.Credentials, + testClientID, + testClientCredentials, signerFixture, oauth2.NewError(oauth2.ErrorInvalidRequest), }, // Invalid client(client is not associated with the token). { fmt.Sprintf("1/%s", base64.URLEncoding.EncodeToString([]byte("refresh-1"))), - clientA.Credentials.ID, + testClientID, clientB.Credentials, signerFixture, oauth2.NewError(oauth2.ErrorInvalidClient), @@ -761,7 +535,7 @@ func TestServerRefreshToken(t *testing.T) { // Invalid client(no client ID). { fmt.Sprintf("1/%s", base64.URLEncoding.EncodeToString([]byte("refresh-1"))), - clientA.Credentials.ID, + testClientID, oidc.ClientCredentials{ID: "", Secret: "aaa"}, signerFixture, oauth2.NewError(oauth2.ErrorInvalidClient), @@ -769,7 +543,7 @@ func TestServerRefreshToken(t *testing.T) { // Invalid client(no such client). { fmt.Sprintf("1/%s", base64.URLEncoding.EncodeToString([]byte("refresh-1"))), - clientA.Credentials.ID, + testClientID, oidc.ClientCredentials{ID: "AAA", Secret: "aaa"}, signerFixture, oauth2.NewError(oauth2.ErrorInvalidClient), @@ -777,7 +551,7 @@ func TestServerRefreshToken(t *testing.T) { // Invalid client(no secrets). { fmt.Sprintf("1/%s", base64.URLEncoding.EncodeToString([]byte("refresh-1"))), - clientA.Credentials.ID, + testClientID, oidc.ClientCredentials{ID: testClientID}, signerFixture, oauth2.NewError(oauth2.ErrorInvalidClient), @@ -785,7 +559,7 @@ func TestServerRefreshToken(t *testing.T) { // Invalid client(invalid secret). { fmt.Sprintf("1/%s", base64.URLEncoding.EncodeToString([]byte("refresh-1"))), - clientA.Credentials.ID, + testClientID, oidc.ClientCredentials{ID: "bad-id", Secret: "bad-secret"}, signerFixture, oauth2.NewError(oauth2.ErrorInvalidClient), @@ -793,8 +567,8 @@ func TestServerRefreshToken(t *testing.T) { // Signing operation fails. { fmt.Sprintf("1/%s", base64.URLEncoding.EncodeToString([]byte("refresh-1"))), - clientA.Credentials.ID, - clientA.Credentials, + testClientID, + testClientCredentials, &StaticSigner{sig: nil, err: errors.New("fail")}, oauth2.NewError(oauth2.ErrorServerError), }, @@ -804,45 +578,22 @@ func TestServerRefreshToken(t *testing.T) { km := &StaticKeyManager{ signer: tt.signer, } - - clients := []client.Client{ - clientA, - clientB, + f, err := makeTestFixtures() + if err != nil { + t.Fatalf("error making test fixtures: %v", err) + } + f.srv.RefreshTokenRepo = refreshtest.NewTestRefreshTokenRepo() + f.srv.KeyManager = km + _, err = f.clientRepo.New(nil, clientB) + if err != nil { + t.Errorf("case %d: error creating other client: %v", i, err) } - clientIDGenerator := func(hostport string) (string, error) { - return hostport, nil - } - secGen := func() ([]byte, error) { - return []byte("secret"), nil - } - dbm := db.NewMemDB() - clientRepo := db.NewClientRepo(dbm) - clientManager, err := clientmanager.NewClientManagerFromClients(clientRepo, db.TransactionFactory(dbm), clients, clientmanager.ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen}) - if err != nil { - t.Fatalf("Failed to create client identity manager: %v", err) - } - userRepo, err := makeNewUserRepo() - if err != nil { + if _, err := f.srv.RefreshTokenRepo.Create(testUserID1, tt.clientID); err != nil { t.Fatalf("Unexpected error: %v", err) } - refreshTokenRepo := refreshtest.NewTestRefreshTokenRepo() - - srv := &Server{ - IssuerURL: issuerURL, - KeyManager: km, - ClientRepo: clientRepo, - ClientManager: clientManager, - UserRepo: userRepo, - RefreshTokenRepo: refreshTokenRepo, - } - - if _, err := refreshTokenRepo.Create("testid-1", tt.clientID); err != nil { - t.Fatalf("Unexpected error: %v", err) - } - - jwt, err := srv.RefreshToken(tt.creds, tt.token) + jwt, err := f.srv.RefreshToken(tt.creds, tt.token) if !reflect.DeepEqual(err, tt.err) { t.Errorf("Case %d: expect: %v, got: %v", i, tt.err, err) } @@ -855,71 +606,9 @@ func TestServerRefreshToken(t *testing.T) { if err != nil { t.Errorf("Case %d: unexpected error: %v", i, err) } - if claims["iss"] != issuerURL.String() || claims["sub"] != "testid-1" || claims["aud"] != testClientID { + if claims["iss"] != testIssuerURL.String() || claims["sub"] != testUserID1 || claims["aud"] != testClientID { t.Errorf("Case %d: invalid claims: %v", i, claims) } } } - - // Test that we should return error when user cannot be found after - // verifying the token. - km := &StaticKeyManager{ - signer: signerFixture, - } - - clients := []client.Client{ - clientA, - clientB, - } - clientIDGenerator := func(hostport string) (string, error) { - return hostport, nil - } - secGen := func() ([]byte, error) { - return []byte("secret"), nil - } - dbm := db.NewMemDB() - clientRepo := db.NewClientRepo(dbm) - clientManager, err := clientmanager.NewClientManagerFromClients(clientRepo, db.TransactionFactory(dbm), clients, clientmanager.ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen}) - if err != nil { - t.Fatalf("Failed to create client identity manager: %v", err) - } - userRepo, err := makeNewUserRepo() - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } - - // Create a user that will be removed later. - if err := userRepo.Create(nil, user.User{ - ID: "testid-2", - Email: "test-2@example.com", - }); err != nil { - t.Fatalf("Unexpected error: %v", err) - } - - refreshTokenRepo := refreshtest.NewTestRefreshTokenRepo() - - srv := &Server{ - IssuerURL: issuerURL, - KeyManager: km, - ClientRepo: clientRepo, - ClientManager: clientManager, - UserRepo: userRepo, - RefreshTokenRepo: refreshTokenRepo, - } - - if _, err := refreshTokenRepo.Create("testid-2", clientA.Credentials.ID); err != nil { - t.Fatalf("Unexpected error: %v", err) - } - - // Recreate the user repo to remove the user we created. - userRepo, err = makeNewUserRepo() - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } - srv.UserRepo = userRepo - - _, err = srv.RefreshToken(clientA.Credentials, fmt.Sprintf("1/%s", base64.URLEncoding.EncodeToString([]byte("refresh-1")))) - if !reflect.DeepEqual(err, oauth2.NewError(oauth2.ErrorServerError)) { - t.Errorf("Expect: %v, got: %v", oauth2.NewError(oauth2.ErrorServerError), err) - } } diff --git a/server/testutil.go b/server/testutil.go index 3dc1ec78..cfe1d9e6 100644 --- a/server/testutil.go +++ b/server/testutil.go @@ -26,21 +26,33 @@ const ( ) var ( + testUserID1 = "ID-1" + testUserEmail1 = "Email-1@example.com" + testUserRemoteID1 = "RID-1" + testIssuerURL = url.URL{Scheme: "http", Host: "server.example.com"} - testClientID = "client.example.com" + + testClientID = "client.example.com" + clientTestSecret = base64.URLEncoding.EncodeToString([]byte("secret")) + testClientCredentials = oidc.ClientCredentials{ + ID: testClientID, + Secret: clientTestSecret, + } + + testConnectorID1 = "IDPC-1" testRedirectURL = url.URL{Scheme: "http", Host: "client.example.com", Path: "/callback"} testUsers = []user.UserWithRemoteIdentities{ { User: user.User{ - ID: "ID-1", - Email: "Email-1@example.com", + ID: testUserID1, + Email: testUserEmail1, }, RemoteIdentities: []user.RemoteIdentity{ { - ConnectorID: "IDPC-1", - ID: "RID-1", + ConnectorID: testConnectorID1, + ID: testUserRemoteID1, }, }, }, @@ -140,10 +152,7 @@ func makeTestFixtures() (*testFixtures, error) { clients := []client.Client{ client.Client{ - Credentials: oidc.ClientCredentials{ - ID: testClientID, - Secret: base64.URLEncoding.EncodeToString([]byte("secret")), - }, + Credentials: testClientCredentials, Metadata: oidc.ClientMetadata{ RedirectURIs: []url.URL{ testRedirectURL, From a33d61c8e2cd71305d992f9a1eb49a08ca730804 Mon Sep 17 00:00:00 2001 From: Bobby Rullo Date: Mon, 6 Jun 2016 16:41:11 -0700 Subject: [PATCH 4/6] server: remove boilerplate setup code part deux Use the test fixture setup stuff in testutil instead. --- server/client_resource_test.go | 20 ++++------ server/http_test.go | 67 ++++++---------------------------- server/testutil.go | 27 ++++++++++---- 3 files changed, 39 insertions(+), 75 deletions(-) diff --git a/server/client_resource_test.go b/server/client_resource_test.go index 4b11dd15..35475fa4 100644 --- a/server/client_resource_test.go +++ b/server/client_resource_test.go @@ -188,7 +188,7 @@ func TestList(t *testing.T) { }{ // empty repo { - cs: nil, + cs: []client.Client{}, want: nil, }, // single client @@ -244,20 +244,14 @@ func TestList(t *testing.T) { } for i, tt := range tests { - dbm := db.NewMemDB() - clientIDGenerator := func(hostport string) (string, error) { - return hostport, nil - } - secGen := func() ([]byte, error) { - return []byte("secret"), nil - } - clientRepo := db.NewClientRepo(dbm) - clientManager, err := manager.NewClientManagerFromClients(clientRepo, db.TransactionFactory(dbm), tt.cs, manager.ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen}) + f, err := makeTestFixturesWithOptions(testFixtureOptions{ + clients: tt.cs, + }) if err != nil { - t.Fatalf("Failed to create client identity manager: %v", err) - continue + t.Fatalf("error making test fixtures: %v", err) } - res := &clientResource{manager: clientManager} + + res := &clientResource{manager: f.clientManager} r, err := http.NewRequest("GET", "http://example.com/clients", nil) if err != nil { diff --git a/server/http_test.go b/server/http_test.go index 25af185e..0dda1847 100644 --- a/server/http_test.go +++ b/server/http_test.go @@ -17,10 +17,7 @@ import ( "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/session/manager" "github.com/coreos/go-oidc/jose" "github.com/coreos/go-oidc/oauth2" "github.com/coreos/go-oidc/oidc" @@ -76,38 +73,6 @@ func TestHandleAuthFuncResponsesSingleRedirectURL(t *testing.T) { idpcs := []connector.Connector{ &fakeConnector{loginURL: "http://fake.example.com"}, } - dbm := db.NewMemDB() - clients := []client.Client{ - client.Client{ - Credentials: oidc.ClientCredentials{ - ID: "client.example.com", - Secret: base64.URLEncoding.EncodeToString([]byte("secret")), - }, - Metadata: oidc.ClientMetadata{ - RedirectURIs: []url.URL{ - url.URL{Scheme: "http", Host: "client.example.com", Path: "/callback"}, - }, - }, - }, - } - - clientIDGenerator := func(hostport string) (string, error) { - return hostport, nil - } - secGen := func() ([]byte, error) { - return []byte("secret"), nil - } - clientRepo := db.NewClientRepo(dbm) - clientManager, err := clientmanager.NewClientManagerFromClients(clientRepo, db.TransactionFactory(dbm), clients, clientmanager.ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen}) - if err != nil { - t.Fatalf("Failed to create client identity manager: %v", err) - } - srv := &Server{ - IssuerURL: url.URL{Scheme: "http", Host: "server.example.com"}, - SessionManager: manager.NewSessionManager(db.NewSessionRepo(db.NewMemDB()), db.NewSessionKeyRepo(db.NewMemDB())), - ClientRepo: clientRepo, - ClientManager: clientManager, - } tests := []struct { query url.Values @@ -118,7 +83,7 @@ func TestHandleAuthFuncResponsesSingleRedirectURL(t *testing.T) { { query: url.Values{ "response_type": []string{"code"}, - "client_id": []string{"client.example.com"}, + "client_id": []string{testClientID}, "connector_id": []string{"fake"}, "scope": []string{"openid"}, }, @@ -210,7 +175,12 @@ func TestHandleAuthFuncResponsesSingleRedirectURL(t *testing.T) { } for i, tt := range tests { - hdlr := handleAuthFunc(srv, idpcs, nil, true) + f, err := makeTestFixtures() + if err != nil { + t.Fatalf("error making test fixtures: %v", err) + } + + hdlr := handleAuthFunc(f.srv, idpcs, nil, true) w := httptest.NewRecorder() u := fmt.Sprintf("http://server.example.com?%s", tt.query.Encode()) req, err := http.NewRequest("GET", u, nil) @@ -237,7 +207,6 @@ func TestHandleAuthFuncResponsesMultipleRedirectURLs(t *testing.T) { &fakeConnector{loginURL: "http://fake.example.com"}, } - dbm := db.NewMemDB() clients := []client.Client{ client.Client{ Credentials: oidc.ClientCredentials{ @@ -252,23 +221,11 @@ func TestHandleAuthFuncResponsesMultipleRedirectURLs(t *testing.T) { }, }, } - - clientIDGenerator := func(hostport string) (string, error) { - return hostport, nil - } - secGen := func() ([]byte, error) { - return []byte("secret"), nil - } - clientRepo := db.NewClientRepo(dbm) - clientManager, err := clientmanager.NewClientManagerFromClients(clientRepo, db.TransactionFactory(dbm), clients, clientmanager.ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen}) + f, err := makeTestFixturesWithOptions(testFixtureOptions{ + clients: clients, + }) if err != nil { - t.Fatalf("Failed to create client identity manager: %v", err) - } - srv := &Server{ - IssuerURL: url.URL{Scheme: "http", Host: "server.example.com"}, - SessionManager: manager.NewSessionManager(db.NewSessionRepo(db.NewMemDB()), db.NewSessionKeyRepo(db.NewMemDB())), - ClientRepo: clientRepo, - ClientManager: clientManager, + t.Fatalf("error making test fixtures: %v", err) } tests := []struct { @@ -327,7 +284,7 @@ func TestHandleAuthFuncResponsesMultipleRedirectURLs(t *testing.T) { } for i, tt := range tests { - hdlr := handleAuthFunc(srv, idpcs, nil, true) + hdlr := handleAuthFunc(f.srv, idpcs, nil, true) w := httptest.NewRecorder() u := fmt.Sprintf("http://server.example.com?%s", tt.query.Encode()) req, err := http.NewRequest("GET", u, nil) diff --git a/server/testutil.go b/server/testutil.go index cfe1d9e6..ae1e51b7 100644 --- a/server/testutil.go +++ b/server/testutil.go @@ -95,6 +95,10 @@ type testFixtures struct { clientManager *clientmanager.ClientManager } +type testFixtureOptions struct { + clients []client.Client +} + func sequentialGenerateCodeFunc() sessionmanager.GenerateCodeFunc { x := 0 return func() (string, error) { @@ -104,6 +108,10 @@ func sequentialGenerateCodeFunc() sessionmanager.GenerateCodeFunc { } func makeTestFixtures() (*testFixtures, error) { + return makeTestFixturesWithOptions(testFixtureOptions{}) +} + +func makeTestFixturesWithOptions(options testFixtureOptions) (*testFixtures, error) { dbMap := db.NewMemDB() userRepo, err := db.NewUserRepoFromUsers(dbMap, testUsers) if err != nil { @@ -150,15 +158,20 @@ func makeTestFixtures() (*testFixtures, error) { return nil, err } - clients := []client.Client{ - client.Client{ - Credentials: testClientCredentials, - Metadata: oidc.ClientMetadata{ - RedirectURIs: []url.URL{ - testRedirectURL, + var clients []client.Client + if options.clients == nil { + clients = []client.Client{ + client.Client{ + Credentials: testClientCredentials, + Metadata: oidc.ClientMetadata{ + RedirectURIs: []url.URL{ + testRedirectURL, + }, }, }, - }, + } + } else { + clients = options.clients } clientIDGenerator := func(hostport string) (string, error) { From 1b4dca80d74e6662594e2885ccce488c7c784a3a Mon Sep 17 00:00:00 2001 From: Bobby Rullo Date: Mon, 6 Jun 2016 17:58:16 -0700 Subject: [PATCH 5/6] 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 | 30 ---------------------------- client/manager/manager_test.go | 7 +++++-- db/client.go | 12 +++++++++++ functional/repo/refresh_repo_test.go | 5 +---- integration/common_test.go | 18 +++++++++++++++++ integration/oidc_test.go | 29 ++++++++------------------- integration/user_api_test.go | 11 ++-------- server/config.go | 9 ++++----- server/testutil.go | 6 ++++-- user/api/api_test.go | 4 ++-- 10 files changed, 56 insertions(+), 75 deletions(-) diff --git a/client/manager/manager.go b/client/manager/manager.go index 10b11c10..28335936 100644 --- a/client/manager/manager.go +++ b/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 { diff --git a/client/manager/manager_test.go b/client/manager/manager_test.go index 62c4c520..2ab2083c 100644 --- a/client/manager/manager_test.go +++ b/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 } diff --git a/db/client.go b/db/client.go index da43e2e6..edbd87c0 100644 --- a/db/client.go +++ b/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 { diff --git a/functional/repo/refresh_repo_test.go b/functional/repo/refresh_repo_test.go index eaacf6a7..9df9aa8a 100644 --- a/functional/repo/refresh_repo_test.go +++ b/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) } diff --git a/integration/common_test.go b/integration/common_test.go index f4cc2449..eb018776 100644 --- a/integration/common_test.go +++ b/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 + +} diff --git a/integration/oidc_test.go b/integration/oidc_test.go index 82f08e93..6cd54da9 100644 --- a/integration/oidc_test.go +++ b/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) { - 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}) + clientRepo, clientManager, err := makeClientRepoAndManager(dbMap, cis) 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) - clientManager, err := clientmanager.NewClientManagerFromClients(clientRepo, db.TransactionFactory(dbMap), []client.Client{ci}, clientmanager.ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen}) + clientRepo, clientManager, err := makeClientRepoAndManager(dbMap, []client.Client{ci}) 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) diff --git a/integration/user_api_test.go b/integration/user_api_test.go index d0296549..3b381c0e 100644 --- a/integration/user_api_test.go +++ b/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 - } - 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}) + + _, clientManager, err := makeClientRepoAndManager(dbMap, clients) if err != nil { panic("Failed to create client identity manager: " + err.Error()) } diff --git a/server/config.go b/server/config.go index 0e002a3b..525867ce 100644 --- a/server/config.go +++ b/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) - - for _, c := range clients { - clientRepo.New(nil, c) + clientRepo, err := db.NewClientRepoFromClients(dbMap, clients) + if err != nil { + return err } 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) } diff --git a/server/testutil.go b/server/testutil.go index ae1e51b7..be794a44 100644 --- a/server/testutil.go +++ b/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) - clientManager, err := clientmanager.NewClientManagerFromClients(clientRepo, db.TransactionFactory(dbMap), clients, clientmanager.ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen}) + clientRepo, err := db.NewClientRepoFromClients(dbMap, clients) 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 { diff --git a/user/api/api_test.go b/user/api/api_test.go index 494d1d0a..d7e15506 100644 --- a/user/api/api_test.go +++ b/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) - clientManager, err := clientmanager.NewClientManagerFromClients(clientRepo, db.TransactionFactory(dbMap), []client.Client{ci}, clientmanager.ManagerOptions{ClientIDGenerator: clientIDGenerator, SecretGenerator: secGen}) + clientRepo, err := db.NewClientRepoFromClients(dbMap, []client.Client{ci}) 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 { From 182e8af420e6aed6fdbea1ac2b8e3bc476dedee4 Mon Sep 17 00:00:00 2001 From: Bobby Rullo Date: Tue, 7 Jun 2016 16:46:08 -0700 Subject: [PATCH 6/6] test: alphabetize tests --- test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test b/test index ffab9c5d..f66c7323 100755 --- a/test +++ b/test @@ -18,7 +18,7 @@ if [ ! -d $GOPATH/pkg ]; then echo "WARNING: No cached builds detected. Please run the ./build script to speed up future tests." fi -TESTABLE="connector db integration pkg/crypto pkg/flag pkg/http pkg/time pkg/html functional/repo server session session/manager user user/api user/manager user/email email admin client client/manager" +TESTABLE="admin client client/manager connector db email functional/repo integration pkg/crypto pkg/flag pkg/http pkg/time pkg/html server session session/manager user user/api user/manager user/email" FORMATTABLE="$TESTABLE cmd/dexctl cmd/dex-worker cmd/dex-overlord examples/app functional pkg/log" # user has not provided PKG override