From ca18efb1fefe25721333d630c9629eff3a8bf042 Mon Sep 17 00:00:00 2001 From: Bobby Rullo Date: Wed, 8 Jun 2016 11:31:50 -0700 Subject: [PATCH] client: load full clients w/ LoadableClient The Client object on its own doesn't fully express everything about a single client, and so when loading clients from a static configuration it's not enough to just (de)serialize clients. To that end, LoadableClient contains the full representation of a client and associated entities. --- client/client.go | 30 +++++--- client/client_test.go | 91 +++++++++++++++++-------- client/manager/manager_test.go | 20 +++--- db/client.go | 13 +++- functional/config/config_sample_test.go | 9 +-- integration/client_api_test.go | 2 +- integration/common_test.go | 2 +- integration/oidc_test.go | 9 ++- integration/user_api_test.go | 38 ++++++----- server/client_resource_test.go | 2 +- server/config.go | 2 +- server/http_test.go | 2 +- server/testutil.go | 28 +++++--- user/api/api_test.go | 2 +- 14 files changed, 165 insertions(+), 85 deletions(-) diff --git a/client/client.go b/client/client.go index 9a862ef6..84073677 100644 --- a/client/client.go +++ b/client/client.go @@ -94,16 +94,24 @@ func ValidRedirectURL(rURL *url.URL, redirectURLs []url.URL) (url.URL, error) { return url.URL{}, ErrorInvalidRedirectURL } -func ClientsFromReader(r io.Reader) ([]Client, error) { +// LoadableClient contains sufficient information for creating a Client and its related entities. +type LoadableClient struct { + Client Client + TrustedPeers []string +} + +func ClientsFromReader(r io.Reader) ([]LoadableClient, error) { var c []struct { ID string `json:"id"` Secret string `json:"secret"` RedirectURLs []string `json:"redirectURLs"` + Admin bool `json:"admin"` + TrustedPeers []string `json:"trustedPeers"` } if err := json.NewDecoder(r).Decode(&c); err != nil { return nil, err } - clients := make([]Client, len(c)) + clients := make([]LoadableClient, len(c)) for i, client := range c { if client.ID == "" { return nil, errors.New("clients must have an ID") @@ -120,14 +128,18 @@ func ClientsFromReader(r io.Reader) ([]Client, error) { redirectURIs[j] = *uri } - clients[i] = Client{ - Credentials: oidc.ClientCredentials{ - ID: client.ID, - Secret: client.Secret, - }, - Metadata: oidc.ClientMetadata{ - RedirectURIs: redirectURIs, + clients[i] = LoadableClient{ + Client: Client{ + Credentials: oidc.ClientCredentials{ + ID: client.ID, + Secret: client.Secret, + }, + Metadata: oidc.ClientMetadata{ + RedirectURIs: redirectURIs, + }, + Admin: client.Admin, }, + TrustedPeers: client.TrustedPeers, } } return clients, nil diff --git a/client/client_test.go b/client/client_test.go index 2c7b74bd..289bc101 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -13,11 +13,13 @@ import ( var ( goodSecret1 = base64.URLEncoding.EncodeToString([]byte("my_secret")) goodSecret2 = base64.URLEncoding.EncodeToString([]byte("my_other_secret")) + goodSecret3 = base64.URLEncoding.EncodeToString([]byte("yet_another_secret")) goodClient1 = `{ "id": "my_id", "secret": "` + goodSecret1 + `", - "redirectURLs": ["https://client.example.com"] + "redirectURLs": ["https://client.example.com"], + "admin": true }` goodClient2 = `{ @@ -26,6 +28,13 @@ var ( "redirectURLs": ["https://client2.example.com","https://client2_a.example.com"] }` + goodClient3 = `{ + "id": "yet_another_id", + "secret": "` + goodSecret3 + `", + "redirectURLs": ["https://client3.example.com","https://client3_a.example.com"], + "trustedPeers":["goodClient1", "goodClient2"] +}` + badURLClient = `{ "id": "my_id", "secret": "` + goodSecret1 + `", @@ -51,57 +60,85 @@ var ( func TestClientsFromReader(t *testing.T) { tests := []struct { json string - want []Client + want []LoadableClient wantErr bool }{ { json: "[]", - want: []Client{}, + want: []LoadableClient{}, }, { json: "[" + goodClient1 + "]", - want: []Client{ + want: []LoadableClient{ { - Credentials: oidc.ClientCredentials{ - ID: "my_id", - Secret: goodSecret1, - }, - Metadata: oidc.ClientMetadata{ - RedirectURIs: []url.URL{ - mustParseURL(t, "https://client.example.com"), + Client: Client{ + Credentials: oidc.ClientCredentials{ + ID: "my_id", + Secret: goodSecret1, }, + Metadata: oidc.ClientMetadata{ + RedirectURIs: []url.URL{ + mustParseURL(t, "https://client.example.com"), + }, + }, + Admin: true, }, }, }, }, { json: "[" + strings.Join([]string{goodClient1, goodClient2}, ",") + "]", - want: []Client{ + want: []LoadableClient{ { - Credentials: oidc.ClientCredentials{ - ID: "my_id", - Secret: goodSecret1, - }, - Metadata: oidc.ClientMetadata{ - RedirectURIs: []url.URL{ - mustParseURL(t, "https://client.example.com"), + Client: Client{ + Credentials: oidc.ClientCredentials{ + ID: "my_id", + Secret: goodSecret1, }, + Metadata: oidc.ClientMetadata{ + RedirectURIs: []url.URL{ + mustParseURL(t, "https://client.example.com"), + }, + }, + Admin: true, }, }, { - Credentials: oidc.ClientCredentials{ - ID: "my_other_id", - Secret: goodSecret2, - }, - Metadata: oidc.ClientMetadata{ - RedirectURIs: []url.URL{ - mustParseURL(t, "https://client2.example.com"), - mustParseURL(t, "https://client2_a.example.com"), + Client: Client{ + Credentials: oidc.ClientCredentials{ + ID: "my_other_id", + Secret: goodSecret2, + }, + Metadata: oidc.ClientMetadata{ + RedirectURIs: []url.URL{ + mustParseURL(t, "https://client2.example.com"), + mustParseURL(t, "https://client2_a.example.com"), + }, }, }, }, }, }, + { + json: "[" + goodClient3 + "]", + want: []LoadableClient{ + { + Client: Client{ + Credentials: oidc.ClientCredentials{ + ID: "yet_another_id", + Secret: goodSecret3, + }, + Metadata: oidc.ClientMetadata{ + RedirectURIs: []url.URL{ + mustParseURL(t, "https://client3.example.com"), + mustParseURL(t, "https://client3_a.example.com"), + }, + }, + }, + TrustedPeers: []string{"goodClient1", "goodClient2"}, + }, + }, + }, { json: "[" + badURLClient + "]", wantErr: true, diff --git a/client/manager/manager_test.go b/client/manager/manager_test.go index b00a4d22..1d489631 100644 --- a/client/manager/manager_test.go +++ b/client/manager/manager_test.go @@ -24,18 +24,20 @@ func makeTestFixtures() *testFixtures { f := &testFixtures{} dbMap := db.NewMemDB() - clients := []client.Client{ + clients := []client.LoadableClient{ { - Credentials: oidc.ClientCredentials{ - ID: "client.example.com", - Secret: goodSecret, - }, - Metadata: oidc.ClientMetadata{ - RedirectURIs: []url.URL{ - {Scheme: "http", Host: "client.example.com", Path: "/"}, + Client: client.Client{ + Credentials: oidc.ClientCredentials{ + ID: "client.example.com", + Secret: goodSecret, }, + Metadata: oidc.ClientMetadata{ + RedirectURIs: []url.URL{ + {Scheme: "http", Host: "client.example.com", Path: "/"}, + }, + }, + Admin: true, }, - Admin: true, }, } clientIDGenerator := func(hostport string) (string, error) { diff --git a/db/client.go b/db/client.go index 895e9e79..4a2ec754 100644 --- a/db/client.go +++ b/db/client.go @@ -212,14 +212,23 @@ func (r *clientRepo) All(tx repo.Transaction) ([]client.Client, error) { return cs, nil } -func NewClientRepoFromClients(dbm *gorp.DbMap, cs []client.Client) (client.ClientRepo, error) { +func NewClientRepoFromClients(dbm *gorp.DbMap, cs []client.LoadableClient) (client.ClientRepo, error) { repo := NewClientRepo(dbm).(*clientRepo) for _, c := range cs { - cm, err := newClientModel(c) + cm, err := newClientModel(c.Client) if err != nil { return nil, err } err = repo.executor(nil).Insert(cm) + if err != nil { + return nil, err + } + + err = repo.SetTrustedPeers(nil, c.Client.Credentials.ID, c.TrustedPeers) + if err != nil { + return nil, err + } + } return repo, nil } diff --git a/functional/config/config_sample_test.go b/functional/config/config_sample_test.go index af6b55b5..f25bf3d9 100644 --- a/functional/config/config_sample_test.go +++ b/functional/config/config_sample_test.go @@ -27,14 +27,15 @@ func TestClientSample(t *testing.T) { } memDB := db.NewMemDB() - repo := db.NewClientRepo(memDB) - for _, c := range clients { - repo.New(nil, c) + repo, err := db.NewClientRepoFromClients(memDB, clients) + if err != nil { + t.Fatalf("Error creating Clients: %v", err) } + mgr := manager.NewClientManager(repo, db.TransactionFactory(memDB), manager.ManagerOptions{}) for i, c := range clients { - ok, err := mgr.Authenticate(c.Credentials) + ok, err := mgr.Authenticate(c.Client.Credentials) if !ok { t.Errorf("case %d: couldn't authenticate", i) } diff --git a/integration/client_api_test.go b/integration/client_api_test.go index 8d69a487..4a3ad195 100644 --- a/integration/client_api_test.go +++ b/integration/client_api_test.go @@ -25,7 +25,7 @@ func TestClientCreate(t *testing.T) { }, }, } - cis := []client.Client{ci} + cis := []client.LoadableClient{{Client: ci}} srv, err := mockServer(cis) if err != nil { diff --git a/integration/common_test.go b/integration/common_test.go index eb018776..69418c08 100644 --- a/integration/common_test.go +++ b/integration/common_test.go @@ -82,7 +82,7 @@ func makeUserObjects(users []user.UserWithRemoteIdentities, passwords []user.Pas return dbMap, ur, pwr, um } -func makeClientRepoAndManager(dbMap *gorp.DbMap, clients []client.Client) (client.ClientRepo, *clientmanager.ClientManager, error) { +func makeClientRepoAndManager(dbMap *gorp.DbMap, clients []client.LoadableClient) (client.ClientRepo, *clientmanager.ClientManager, error) { clientIDGenerator := func(hostport string) (string, error) { return hostport, nil } diff --git a/integration/oidc_test.go b/integration/oidc_test.go index 6cd54da9..4d8ac072 100644 --- a/integration/oidc_test.go +++ b/integration/oidc_test.go @@ -24,7 +24,7 @@ import ( "github.com/coreos/dex/user" ) -func mockServer(cis []client.Client) (*server.Server, error) { +func mockServer(cis []client.LoadableClient) (*server.Server, error) { dbMap := db.NewMemDB() k, err := key.GeneratePrivateKey() if err != nil { @@ -144,7 +144,10 @@ func TestHTTPExchangeTokenRefreshToken(t *testing.T) { } dbMap := db.NewMemDB() - clientRepo, clientManager, err := makeClientRepoAndManager(dbMap, []client.Client{ci}) + clientRepo, clientManager, err := makeClientRepoAndManager(dbMap, + []client.LoadableClient{{ + Client: ci, + }}) if err != nil { t.Fatalf("Failed to create client identity manager: " + err.Error()) } @@ -300,7 +303,7 @@ func TestHTTPClientCredsToken(t *testing.T) { }, }, } - cis := []client.Client{ci} + cis := []client.LoadableClient{{Client: ci}} srv, err := mockServer(cis) if err != nil { diff --git a/integration/user_api_test.go b/integration/user_api_test.go index 9daf3143..163a1382 100644 --- a/integration/user_api_test.go +++ b/integration/user_api_test.go @@ -101,26 +101,30 @@ func makeUserAPITestFixtures() *userAPITestFixtures { f := &userAPITestFixtures{} dbMap, _, _, um := makeUserObjects(userUsers, userPasswords) - clients := []client.Client{ - client.Client{ - Credentials: oidc.ClientCredentials{ - ID: testClientID, - Secret: testClientSecret, - }, - Metadata: oidc.ClientMetadata{ - RedirectURIs: []url.URL{ - testRedirectURL, + clients := []client.LoadableClient{ + { + Client: client.Client{ + Credentials: oidc.ClientCredentials{ + ID: testClientID, + Secret: testClientSecret, + }, + Metadata: oidc.ClientMetadata{ + RedirectURIs: []url.URL{ + testRedirectURL, + }, }, }, }, - client.Client{ - Credentials: oidc.ClientCredentials{ - ID: userBadClientID, - Secret: base64.URLEncoding.EncodeToString([]byte("secret")), - }, - Metadata: oidc.ClientMetadata{ - RedirectURIs: []url.URL{ - testBadRedirectURL, + { + Client: client.Client{ + Credentials: oidc.ClientCredentials{ + ID: userBadClientID, + Secret: base64.URLEncoding.EncodeToString([]byte("secret")), + }, + Metadata: oidc.ClientMetadata{ + RedirectURIs: []url.URL{ + testBadRedirectURL, + }, }, }, }, diff --git a/server/client_resource_test.go b/server/client_resource_test.go index 35475fa4..7fffa78a 100644 --- a/server/client_resource_test.go +++ b/server/client_resource_test.go @@ -245,7 +245,7 @@ func TestList(t *testing.T) { for i, tt := range tests { f, err := makeTestFixturesWithOptions(testFixtureOptions{ - clients: tt.cs, + clients: clientsToLoadableClients(tt.cs), }) if err != nil { t.Fatalf("error making test fixtures: %v", err) diff --git a/server/config.go b/server/config.go index 525867ce..06232528 100644 --- a/server/config.go +++ b/server/config.go @@ -222,7 +222,7 @@ func loadUsersFromReader(r io.Reader) (users []user.UserWithRemoteIdentities, pw } // loadClients parses the clients.json file and returns a list of clients. -func loadClients(filepath string) ([]client.Client, error) { +func loadClients(filepath string) ([]client.LoadableClient, error) { f, err := os.Open(filepath) if err != nil { return nil, err diff --git a/server/http_test.go b/server/http_test.go index 180993d3..1a2de802 100644 --- a/server/http_test.go +++ b/server/http_test.go @@ -223,7 +223,7 @@ func TestHandleAuthFuncResponsesMultipleRedirectURLs(t *testing.T) { }, } f, err := makeTestFixturesWithOptions(testFixtureOptions{ - clients: clients, + clients: clientsToLoadableClients(clients), }) if err != nil { t.Fatalf("error making test fixtures: %v", err) diff --git a/server/testutil.go b/server/testutil.go index 2a4ce254..06979b03 100644 --- a/server/testutil.go +++ b/server/testutil.go @@ -103,7 +103,7 @@ type testFixtures struct { } type testFixtureOptions struct { - clients []client.Client + clients []client.LoadableClient } func sequentialGenerateCodeFunc() sessionmanager.GenerateCodeFunc { @@ -167,14 +167,16 @@ func makeTestFixturesWithOptions(options testFixtureOptions) (*testFixtures, err return nil, err } - var clients []client.Client + var clients []client.LoadableClient if options.clients == nil { - clients = []client.Client{ - client.Client{ - Credentials: testClientCredentials, - Metadata: oidc.ClientMetadata{ - RedirectURIs: []url.URL{ - testRedirectURL, + clients = []client.LoadableClient{ + { + Client: client.Client{ + Credentials: testClientCredentials, + Metadata: oidc.ClientMetadata{ + RedirectURIs: []url.URL{ + testRedirectURL, + }, }, }, }, @@ -258,3 +260,13 @@ func makeTestFixturesWithOptions(options testFixtureOptions) (*testFixtures, err }, }, nil } + +func clientsToLoadableClients(cs []client.Client) []client.LoadableClient { + lcs := make([]client.LoadableClient, len(cs), len(cs)) + for i, c := range cs { + lcs[i] = client.LoadableClient{ + Client: c, + } + } + return lcs +} diff --git a/user/api/api_test.go b/user/api/api_test.go index d7e15506..2fe97189 100644 --- a/user/api/api_test.go +++ b/user/api/api_test.go @@ -176,7 +176,7 @@ func makeTestFixtures() (*UsersAPI, *testEmailer) { secGen := func() ([]byte, error) { return []byte("secret"), nil } - clientRepo, err := db.NewClientRepoFromClients(dbMap, []client.Client{ci}) + clientRepo, err := db.NewClientRepoFromClients(dbMap, []client.LoadableClient{{Client: ci}}) if err != nil { panic("Failed to create client manager: " + err.Error()) }