From 1bbca1d43cba18826854738577c0727947af43d4 Mon Sep 17 00:00:00 2001 From: Bobby Rullo Date: Fri, 15 Apr 2016 17:22:59 -0700 Subject: [PATCH 1/9] schema: tweaks to make Client API more regular --- schema/adminschema/mapper.go | 73 +++++++++++++++++ schema/adminschema/mapper_test.go | 129 ++++++++++++++++++++++++++++++ schema/adminschema/v1.json | 99 ++++++++++++----------- 3 files changed, 256 insertions(+), 45 deletions(-) create mode 100644 schema/adminschema/mapper.go create mode 100644 schema/adminschema/mapper_test.go diff --git a/schema/adminschema/mapper.go b/schema/adminschema/mapper.go new file mode 100644 index 00000000..3f6c32d1 --- /dev/null +++ b/schema/adminschema/mapper.go @@ -0,0 +1,73 @@ +package adminschema + +import ( + "errors" + "net/url" + + "github.com/coreos/dex/client" + "github.com/coreos/go-oidc/oidc" +) + +func MapSchemaClientToClient(sc Client) (client.Client, error) { + c := client.Client{ + Credentials: oidc.ClientCredentials{ + ID: sc.Id, + Secret: sc.Secret, + }, + Metadata: oidc.ClientMetadata{ + RedirectURIs: make([]url.URL, len(sc.RedirectURIs)), + }, + } + for i, ru := range sc.RedirectURIs { + if ru == "" { + return client.Client{}, errors.New("redirect URL empty") + } + + u, err := url.Parse(ru) + if err != nil { + return client.Client{}, errors.New("redirect URL invalid") + } + + c.Metadata.RedirectURIs[i] = *u + } + + c.Metadata.ClientName = sc.ClientName + + if sc.LogoURI != "" { + logoURI, err := url.Parse(sc.LogoURI) + if err != nil { + return client.Client{}, errors.New("logoURI invalid") + } + c.Metadata.LogoURI = logoURI + } + + if sc.ClientURI != "" { + clientURI, err := url.Parse(sc.ClientURI) + if err != nil { + return client.Client{}, errors.New("clientURI invalid") + } + c.Metadata.ClientURI = clientURI + } + return c, nil +} + +func MapClientToSchemaClient(c client.Client) Client { + cl := Client{ + Id: c.Credentials.ID, + Secret: c.Credentials.Secret, + RedirectURIs: make([]string, len(c.Metadata.RedirectURIs)), + } + for i, u := range c.Metadata.RedirectURIs { + cl.RedirectURIs[i] = u.String() + } + + cl.ClientName = c.Metadata.ClientName + + if c.Metadata.LogoURI != nil { + cl.LogoURI = c.Metadata.LogoURI.String() + } + if c.Metadata.ClientURI != nil { + cl.ClientURI = c.Metadata.ClientURI.String() + } + return cl +} diff --git a/schema/adminschema/mapper_test.go b/schema/adminschema/mapper_test.go new file mode 100644 index 00000000..608e9b53 --- /dev/null +++ b/schema/adminschema/mapper_test.go @@ -0,0 +1,129 @@ +package adminschema + +import ( + "net/url" + "testing" + + "github.com/coreos/go-oidc/oidc" + "github.com/kylelemons/godebug/pretty" + + "github.com/coreos/dex/client" +) + +func TestMapSchemaClientToClient(t *testing.T) { + tests := []struct { + sc Client + want client.Client + wantErr bool + }{ + { + sc: Client{ + Id: "123", + Secret: "sec_123", + RedirectURIs: []string{ + "https://client.example.com", + "https://client2.example.com", + }, + ClientName: "Bill", + LogoURI: "https://logo.example.com", + ClientURI: "https://clientURI.example.com", + }, + want: client.Client{ + Credentials: oidc.ClientCredentials{ + ID: "123", + Secret: "sec_123", + }, + Metadata: oidc.ClientMetadata{ + RedirectURIs: []url.URL{ + *mustParseURL(t, "https://client.example.com"), + *mustParseURL(t, "https://client2.example.com"), + }, + ClientName: "Bill", + LogoURI: mustParseURL(t, "https://logo.example.com"), + ClientURI: mustParseURL(t, "https://clientURI.example.com"), + }, + }, + }, { + sc: Client{ + Id: "123", + Secret: "sec_123", + RedirectURIs: []string{ + "ht.d://p * * *", + }, + }, + wantErr: true, + }, + } + + for i, tt := range tests { + got, err := MapSchemaClientToClient(tt.sc) + if tt.wantErr { + if err == nil { + t.Errorf("case %d: want non-nil error", i) + t.Logf(pretty.Sprint(got)) + } + continue + } + if err != nil { + t.Errorf("case %d: unexpected error mapping: %v", i, err) + } + + if diff := pretty.Compare(tt.want, got); diff != "" { + t.Errorf("case %d: Compare(want, got): %v", i, diff) + } + + } +} + +func TestMapClientToClientSchema(t *testing.T) { + tests := []struct { + c client.Client + want Client + }{ + { + want: Client{ + Id: "123", + Secret: "sec_123", + RedirectURIs: []string{ + "https://client.example.com", + "https://client2.example.com", + }, + ClientName: "Bill", + LogoURI: "https://logo.example.com", + ClientURI: "https://clientURI.example.com", + }, + c: client.Client{ + Credentials: oidc.ClientCredentials{ + ID: "123", + Secret: "sec_123", + }, + Metadata: oidc.ClientMetadata{ + RedirectURIs: []url.URL{ + *mustParseURL(t, "https://client.example.com"), + *mustParseURL(t, "https://client2.example.com"), + }, + ClientName: "Bill", + LogoURI: mustParseURL(t, "https://logo.example.com"), + ClientURI: mustParseURL(t, "https://clientURI.example.com"), + }, + }, + }, + } + + for i, tt := range tests { + got := MapClientToSchemaClient(tt.c) + + if diff := pretty.Compare(tt.want, got); diff != "" { + t.Errorf("case %d: Compare(want, got): %v", i, diff) + } + + } +} + +func mustParseURL(t *testing.T, s string) *url.URL { + u, err := url.Parse(s) + if err != nil { + t.Fatalf("Cannot parse %v as url: %v", s, err) + } + return u +} diff --git a/schema/adminschema/v1.json b/schema/adminschema/v1.json index f7719620..4aa8573c 100644 --- a/schema/adminschema/v1.json +++ b/schema/adminschema/v1.json @@ -45,53 +45,62 @@ } } }, - "ClientCreateRequest": { - "id": "ClientCreateRequest", - "type": "object", - "description": "A request to register a client with dex.", - "properties": { - "isAdmin": { - "type": "boolean" - }, - "client": { - "type": "object", - "properties": { - "redirect_uris": { - "type": "array", - "items": { - "type": "string" - }, - "description": "REQUIRED. Array of Redirection URI values used by the Client. One of these registered Redirection URI values MUST exactly match the redirect_uri parameter value used in each Authorization Request, with the matching performed as described in Section 6.2.1 of [RFC3986] ( Berners-Lee, T., Fielding, R., and L. Masinter, “Uniform Resource Identifier (URI): Generic Syntax,” January 2005. ) (Simple String Comparison)." - }, - "client_name": { - "type": "string", - "description": "OPTIONAL. Name of the Client to be presented to the End-User. If desired, representation of this Claim in different languages and scripts is represented as described in Section 2.1 ( Metadata Languages and Scripts ) ." - }, - "logo_uri": { - "type": "string", - "description": "OPTIONAL. URL that references a logo for the Client application. If present, the server SHOULD display this image to the End-User during approval. The value of this field MUST point to a valid image file. If desired, representation of this Claim in different languages and scripts is represented as described in Section 2.1 ( Metadata Languages and Scripts ) ." - }, - "client_uri": { - "type": "string", - "description": "OPTIONAL. URL of the home page of the Client. The value of this field MUST point to a valid Web page. If present, the server SHOULD display this URL to the End-User in a followable fashion. If desired, representation of this Claim in different languages and scripts is represented as described in Section 2.1 ( Metadata Languages and Scripts ) ." - } - } - } + "Client": { + "id": "Client", + "type": "object", + "properties": { + "id": { + "type": "string", + "description": "The client ID. Ignored in client create requests." + }, + "secret": { + "type": "string", + "description": "The client secret. Ignored in client create requests." + }, + "isAdmin": { + "type": "boolean" + }, + "redirectURIs": { + "type": "array", + "items": { + "type": "string" + }, + "description": "REQUIRED. Array of Redirection URI values used by the Client. One of these registered Redirection URI values MUST exactly match the redirect_uri parameter value used in each Authorization Request, with the matching performed as described in Section 6.2.1 of [RFC3986] ( Berners-Lee, T., Fielding, R., and L. Masinter, “Uniform Resource Identifier (URI): Generic Syntax,” January 2005. ) (Simple String Comparison)." + }, + "clientName": { + "type": "string", + "description": "OPTIONAL. Name of the Client to be presented to the End-User. If desired, representation of this Claim in different languages and scripts is represented as described in Section 2.1 ( Metadata Languages and Scripts ) ." + }, + "logoURI": { + "type": "string", + "description": "OPTIONAL. URL that references a logo for the Client application. If present, the server SHOULD display this image to the End-User during approval. The value of this field MUST point to a valid image file. If desired, representation of this Claim in different languages and scripts is represented as described in Section 2.1 ( Metadata Languages and Scripts ) ." + }, + "clientURI": { + "type": "string", + "description": "OPTIONAL. URL of the home page of the Client. The value of this field MUST point to a valid Web page. If present, the server SHOULD display this URL to the End-User in a followable fashion. If desired, representation of this Claim in different languages and scripts is represented as described in Section 2.1 ( Metadata Languages and Scripts ) ." } + } }, - "ClientRegistrationResponse": { - "id": "ClientRegistrationResponse", - "type": "object", - "description": "Upon successful registration, an ID and secret is assigned to the client.", - "properties": { - "client_id": { - "type": "string" - }, - "client_secret": { - "type": "string" - } - } + "ClientCreateRequest": { + "id": "ClientCreateRequest", + "type": "object", + "description": "A request to register a client with dex.", + "properties": { + "client": { + "$ref": "Client" + } } + }, + "ClientCreateResponse": { + "id": "ClientCreateResponse", + "type": "object", + "description": "Upon successful registration, an ID and secret is assigned to the client.", + "properties": { + "client":{ + "$ref": "Client" + } + } + } }, "resources": { "Admin": { @@ -154,7 +163,7 @@ "$ref": "ClientCreateRequest" }, "response": { - "$ref": "ClientRegistrationResponse" + "$ref": "ClientCreateResponse" } } } From 35cefb7da9f4e0177444a440d1c965db2c999e44 Mon Sep 17 00:00:00 2001 From: Bobby Rullo Date: Fri, 15 Apr 2016 17:23:27 -0700 Subject: [PATCH 2/9] schema: generate code --- schema/adminschema/README.md | 35 +++++++----- schema/adminschema/v1-gen.go | 47 ++++++++-------- schema/adminschema/v1-json.go | 103 +++++++++++++++++++--------------- schema/workerschema/mapper.go | 13 +++-- 4 files changed, 112 insertions(+), 86 deletions(-) diff --git a/schema/adminschema/README.md b/schema/adminschema/README.md index c16da394..48699989 100644 --- a/schema/adminschema/README.md +++ b/schema/adminschema/README.md @@ -20,32 +20,41 @@ __Version:__ v1 } ``` +### Client + + + +``` +{ + clientName: string // OPTIONAL. Name of the Client to be presented to the End-User. If desired, representation of this Claim in different languages and scripts is represented as described in Section 2.1 ( Metadata Languages and Scripts ) ., + clientURI: string // OPTIONAL. URL of the home page of the Client. The value of this field MUST point to a valid Web page. If present, the server SHOULD display this URL to the End-User in a followable fashion. If desired, representation of this Claim in different languages and scripts is represented as described in Section 2.1 ( Metadata Languages and Scripts ) ., + id: string // The client ID. Ignored in client create requests., + isAdmin: boolean, + logoURI: string // OPTIONAL. URL that references a logo for the Client application. If present, the server SHOULD display this image to the End-User during approval. The value of this field MUST point to a valid image file. If desired, representation of this Claim in different languages and scripts is represented as described in Section 2.1 ( Metadata Languages and Scripts ) ., + redirectURIs: [ + string + ], + secret: string +} +``` + ### ClientCreateRequest A request to register a client with dex. ``` { - client: { - client_name: string // OPTIONAL. Name of the Client to be presented to the End-User. If desired, representation of this Claim in different languages and scripts is represented as described in Section 2.1 ( Metadata Languages and Scripts ) ., - client_uri: string // OPTIONAL. URL of the home page of the Client. The value of this field MUST point to a valid Web page. If present, the server SHOULD display this URL to the End-User in a followable fashion. If desired, representation of this Claim in different languages and scripts is represented as described in Section 2.1 ( Metadata Languages and Scripts ) ., - logo_uri: string // OPTIONAL. URL that references a logo for the Client application. If present, the server SHOULD display this image to the End-User during approval. The value of this field MUST point to a valid image file. If desired, representation of this Claim in different languages and scripts is represented as described in Section 2.1 ( Metadata Languages and Scripts ) ., - redirect_uris: [ - string - ] - }, - isAdmin: boolean + client: Client } ``` -### ClientRegistrationResponse +### ClientCreateResponse Upon successful registration, an ID and secret is assigned to the client. ``` { - client_id: string, - client_secret: string + client: Client } ``` @@ -137,7 +146,7 @@ Upon successful registration, an ID and secret is assigned to the client. > |Code|Description|Type| |:-----|:-----|:-----| -| 200 | | [ClientRegistrationResponse](#clientregistrationresponse) | +| 200 | | [ClientCreateResponse](#clientcreateresponse) | | default | Unexpected error | | diff --git a/schema/adminschema/v1-gen.go b/schema/adminschema/v1-gen.go index 8b276158..bd49ddf3 100644 --- a/schema/adminschema/v1-gen.go +++ b/schema/adminschema/v1-gen.go @@ -97,49 +97,52 @@ type Admin struct { Password string `json:"password,omitempty"` } -type ClientCreateRequest struct { - Client *ClientCreateRequestClient `json:"client,omitempty"` - - IsAdmin bool `json:"isAdmin,omitempty"` -} - -type ClientCreateRequestClient struct { - // Client_name: OPTIONAL. Name of the Client to be presented to the +type Client struct { + // ClientName: OPTIONAL. Name of the Client to be presented to the // End-User. If desired, representation of this Claim in different // languages and scripts is represented as described in Section 2.1 ( // Metadata Languages and Scripts ) . - Client_name string `json:"client_name,omitempty"` + ClientName string `json:"clientName,omitempty"` - // Client_uri: OPTIONAL. URL of the home page of the Client. The value - // of this field MUST point to a valid Web page. If present, the server + // ClientURI: OPTIONAL. URL of the home page of the Client. The value of + // this field MUST point to a valid Web page. If present, the server // SHOULD display this URL to the End-User in a followable fashion. If // desired, representation of this Claim in different languages and // scripts is represented as described in Section 2.1 ( Metadata // Languages and Scripts ) . - Client_uri string `json:"client_uri,omitempty"` + ClientURI string `json:"clientURI,omitempty"` - // Logo_uri: OPTIONAL. URL that references a logo for the Client + // Id: The client ID. Ignored in client create requests. + Id string `json:"id,omitempty"` + + IsAdmin bool `json:"isAdmin,omitempty"` + + // LogoURI: OPTIONAL. URL that references a logo for the Client // application. If present, the server SHOULD display this image to the // End-User during approval. The value of this field MUST point to a // valid image file. If desired, representation of this Claim in // different languages and scripts is represented as described in // Section 2.1 ( Metadata Languages and Scripts ) . - Logo_uri string `json:"logo_uri,omitempty"` + LogoURI string `json:"logoURI,omitempty"` - // Redirect_uris: REQUIRED. Array of Redirection URI values used by the + // RedirectURIs: REQUIRED. Array of Redirection URI values used by the // Client. One of these registered Redirection URI values MUST exactly // match the redirect_uri parameter value used in each Authorization // Request, with the matching performed as described in Section 6.2.1 of // [RFC3986] ( Berners-Lee, T., Fielding, R., and L. Masinter, // “Uniform Resource Identifier (URI): Generic Syntax,” January // 2005. ) (Simple String Comparison). - Redirect_uris []string `json:"redirect_uris,omitempty"` + RedirectURIs []string `json:"redirectURIs,omitempty"` + + Secret string `json:"secret,omitempty"` } -type ClientRegistrationResponse struct { - Client_id string `json:"client_id,omitempty"` +type ClientCreateRequest struct { + Client *Client `json:"client,omitempty"` +} - Client_secret string `json:"client_secret,omitempty"` +type ClientCreateResponse struct { + Client *Client `json:"client,omitempty"` } type State struct { @@ -310,7 +313,7 @@ func (c *ClientCreateCall) Fields(s ...googleapi.Field) *ClientCreateCall { return c } -func (c *ClientCreateCall) Do() (*ClientRegistrationResponse, error) { +func (c *ClientCreateCall) Do() (*ClientCreateResponse, error) { var body io.Reader = nil body, err := googleapi.WithoutDataWrapper.JSONReader(c.clientcreaterequest) if err != nil { @@ -336,7 +339,7 @@ func (c *ClientCreateCall) Do() (*ClientRegistrationResponse, error) { if err := googleapi.CheckResponse(res); err != nil { return nil, err } - var ret *ClientRegistrationResponse + var ret *ClientCreateResponse if err := json.NewDecoder(res.Body).Decode(&ret); err != nil { return nil, err } @@ -350,7 +353,7 @@ func (c *ClientCreateCall) Do() (*ClientRegistrationResponse, error) { // "$ref": "ClientCreateRequest" // }, // "response": { - // "$ref": "ClientRegistrationResponse" + // "$ref": "ClientCreateResponse" // } // } diff --git a/schema/adminschema/v1-json.go b/schema/adminschema/v1-json.go index f99dfb71..64c29f61 100644 --- a/schema/adminschema/v1-json.go +++ b/schema/adminschema/v1-json.go @@ -51,53 +51,66 @@ const DiscoveryJSON = `{ } } }, - "ClientCreateRequest": { - "id": "ClientCreateRequest", - "type": "object", - "description": "A request to register a client with dex.", - "properties": { - "isAdmin": { - "type": "boolean" - }, - "client": { - "type": "object", - "properties": { - "redirect_uris": { - "type": "array", - "items": { - "type": "string" - }, - "description": "REQUIRED. Array of Redirection URI values used by the Client. One of these registered Redirection URI values MUST exactly match the redirect_uri parameter value used in each Authorization Request, with the matching performed as described in Section 6.2.1 of [RFC3986] ( Berners-Lee, T., Fielding, R., and L. Masinter, “Uniform Resource Identifier (URI): Generic Syntax,” January 2005. ) (Simple String Comparison)." - }, - "client_name": { - "type": "string", - "description": "OPTIONAL. Name of the Client to be presented to the End-User. If desired, representation of this Claim in different languages and scripts is represented as described in Section 2.1 ( Metadata Languages and Scripts ) ." - }, - "logo_uri": { - "type": "string", - "description": "OPTIONAL. URL that references a logo for the Client application. If present, the server SHOULD display this image to the End-User during approval. The value of this field MUST point to a valid image file. If desired, representation of this Claim in different languages and scripts is represented as described in Section 2.1 ( Metadata Languages and Scripts ) ." - }, - "client_uri": { - "type": "string", - "description": "OPTIONAL. URL of the home page of the Client. The value of this field MUST point to a valid Web page. If present, the server SHOULD display this URL to the End-User in a followable fashion. If desired, representation of this Claim in different languages and scripts is represented as described in Section 2.1 ( Metadata Languages and Scripts ) ." - } - } - } + "Client": { + "id": "Client", + "type": "object", + "properties": { + "id": { + "type": "string", + "description": "The client ID. Ignored in client create requests." + }, + "secret": { + "type": "string", + "description": "The client secret. Ignored in client create requests." + }, + "secret": { + "type": "string", + "format": "byte" + }, + "isAdmin": { + "type": "boolean" + }, + "redirectURIs": { + "type": "array", + "items": { + "type": "string" + }, + "description": "REQUIRED. Array of Redirection URI values used by the Client. One of these registered Redirection URI values MUST exactly match the redirect_uri parameter value used in each Authorization Request, with the matching performed as described in Section 6.2.1 of [RFC3986] ( Berners-Lee, T., Fielding, R., and L. Masinter, “Uniform Resource Identifier (URI): Generic Syntax,” January 2005. ) (Simple String Comparison)." + }, + "clientName": { + "type": "string", + "description": "OPTIONAL. Name of the Client to be presented to the End-User. If desired, representation of this Claim in different languages and scripts is represented as described in Section 2.1 ( Metadata Languages and Scripts ) ." + }, + "logoURI": { + "type": "string", + "description": "OPTIONAL. URL that references a logo for the Client application. If present, the server SHOULD display this image to the End-User during approval. The value of this field MUST point to a valid image file. If desired, representation of this Claim in different languages and scripts is represented as described in Section 2.1 ( Metadata Languages and Scripts ) ." + }, + "clientURI": { + "type": "string", + "description": "OPTIONAL. URL of the home page of the Client. The value of this field MUST point to a valid Web page. If present, the server SHOULD display this URL to the End-User in a followable fashion. If desired, representation of this Claim in different languages and scripts is represented as described in Section 2.1 ( Metadata Languages and Scripts ) ." } + } }, - "ClientRegistrationResponse": { - "id": "ClientRegistrationResponse", - "type": "object", - "description": "Upon successful registration, an ID and secret is assigned to the client.", - "properties": { - "client_id": { - "type": "string" - }, - "client_secret": { - "type": "string" - } - } + "ClientCreateRequest": { + "id": "ClientCreateRequest", + "type": "object", + "description": "A request to register a client with dex.", + "properties": { + "client": { + "$ref": "Client" + } } + }, + "ClientCreateResponse": { + "id": "ClientCreateResponse", + "type": "object", + "description": "Upon successful registration, an ID and secret is assigned to the client.", + "properties": { + "client":{ + "$ref": "Client" + } + } + } }, "resources": { "Admin": { @@ -160,7 +173,7 @@ const DiscoveryJSON = `{ "$ref": "ClientCreateRequest" }, "response": { - "$ref": "ClientRegistrationResponse" + "$ref": "ClientCreateResponse" } } } diff --git a/schema/workerschema/mapper.go b/schema/workerschema/mapper.go index ab9c16e7..7cdf4ab3 100644 --- a/schema/workerschema/mapper.go +++ b/schema/workerschema/mapper.go @@ -4,11 +4,12 @@ import ( "errors" "net/url" + "github.com/coreos/dex/client" "github.com/coreos/go-oidc/oidc" ) -func MapSchemaClientToClientIdentity(sc Client) (oidc.ClientIdentity, error) { - ci := oidc.ClientIdentity{ +func MapSchemaClientToClient(sc Client) (client.Client, error) { + ci := client.Client{ Credentials: oidc.ClientCredentials{ ID: sc.Id, }, @@ -19,12 +20,12 @@ func MapSchemaClientToClientIdentity(sc Client) (oidc.ClientIdentity, error) { for i, ru := range sc.RedirectURIs { if ru == "" { - return oidc.ClientIdentity{}, errors.New("redirect URL empty") + return client.Client{}, errors.New("redirect URL empty") } u, err := url.Parse(ru) if err != nil { - return oidc.ClientIdentity{}, errors.New("redirect URL invalid") + return client.Client{}, errors.New("redirect URL invalid") } ci.Metadata.RedirectURIs[i] = *u @@ -33,7 +34,7 @@ func MapSchemaClientToClientIdentity(sc Client) (oidc.ClientIdentity, error) { return ci, nil } -func MapClientIdentityToSchemaClient(c oidc.ClientIdentity) Client { +func MapClientToSchemaClient(c client.Client) Client { cl := Client{ Id: c.Credentials.ID, RedirectURIs: make([]string, len(c.Metadata.RedirectURIs)), @@ -44,7 +45,7 @@ func MapClientIdentityToSchemaClient(c oidc.ClientIdentity) Client { return cl } -func MapClientIdentityToSchemaClientWithSecret(c oidc.ClientIdentity) ClientWithSecret { +func MapClientToSchemaClientWithSecret(c client.Client) ClientWithSecret { cl := ClientWithSecret{ Id: c.Credentials.ID, Secret: c.Credentials.Secret, From 95757e87793208cf640a444a405977a01d39a3a5 Mon Sep 17 00:00:00 2001 From: Bobby Rullo Date: Thu, 14 Apr 2016 15:57:53 -0700 Subject: [PATCH 3/9] *: Client Repo now deals with custom Client object This is instead of oidc.ClientIdentity. This makes it easier to add new fields custom to dex to the client. --- admin/api.go | 37 ++++++++++----- client/client.go | 47 ++++++++++++++++++- db/client.go | 70 +++++++++++++++++----------- db/refresh.go | 8 ++-- functional/db_test.go | 42 ++++++++++++++--- functional/repo/client_repo_test.go | 6 +-- functional/repo/refresh_repo_test.go | 5 +- integration/admin_api_test.go | 15 +++--- integration/client_api_test.go | 5 +- integration/oidc_test.go | 15 +++--- integration/user_api_test.go | 6 +-- refresh/repo.go | 4 +- server/admin.go | 2 +- server/auth_middleware_test.go | 4 +- server/client_registration.go | 9 +++- server/client_resource.go | 10 ++-- server/client_resource_test.go | 13 +++--- server/config.go | 41 ++-------------- server/http_test.go | 8 ++-- server/server_test.go | 38 +++++++-------- server/testutil.go | 4 +- user/api/api_test.go | 8 ++-- 22 files changed, 241 insertions(+), 156 deletions(-) diff --git a/admin/api.go b/admin/api.go index d65c71f9..22875c3e 100644 --- a/admin/api.go +++ b/admin/api.go @@ -116,25 +116,38 @@ func (a *AdminAPI) GetState() (adminschema.State, error) { return state, nil } -type ClientRegistrationRequest struct { - IsAdmin bool `json:"isAdmin"` - Client oidc.ClientMetadata `json:"client"` -} +func (a *AdminAPI) CreateClient(req adminschema.ClientCreateRequest) (adminschema.ClientCreateResponse, error) { + cli, err := adminschema.MapSchemaClientToClient(*req.Client) + if err != nil { + // TODO should be 400s + return adminschema.ClientCreateResponse{}, mapError(err) + } -func (a *AdminAPI) CreateClient(req ClientRegistrationRequest) (oidc.ClientRegistrationResponse, error) { - if err := req.Client.Valid(); err != nil { - return oidc.ClientRegistrationResponse{}, mapError(err) + if err := cli.Metadata.Valid(); err != nil { + // TODO make sure this is not 500 + return adminschema.ClientCreateResponse{}, mapError(err) } + // metadata is guarenteed to have at least one redirect_uri by earlier validation. - id, err := oidc.GenClientID(req.Client.RedirectURIs[0].Host) + id, err := oidc.GenClientID(cli.Metadata.RedirectURIs[0].Host) if err != nil { - return oidc.ClientRegistrationResponse{}, mapError(err) + return adminschema.ClientCreateResponse{}, mapError(err) } - c, err := a.clientIdentityRepo.New(id, req.Client, req.IsAdmin) + + cli.Credentials.ID = id + + creds, err := a.clientIdentityRepo.New(cli) if err != nil { - return oidc.ClientRegistrationResponse{}, mapError(err) + return adminschema.ClientCreateResponse{}, mapError(err) } - return oidc.ClientRegistrationResponse{ClientID: c.ID, ClientSecret: c.Secret, ClientMetadata: req.Client}, nil + + req.Client.Id = creds.ID + req.Client.Secret = creds.Secret + return adminschema.ClientCreateResponse{ + Client: req.Client, + }, nil + + // github.com/coreos/dex/integrationoidc.ClientRegistrationResponse{ClientID: c.ID, ClientSecret: c.Secret, ClientMetadata: req.Client.Metadata}, nil } func mapError(e error) error { diff --git a/client/client.go b/client/client.go index 20531603..5a222c81 100644 --- a/client/client.go +++ b/client/client.go @@ -1,7 +1,9 @@ package client import ( + "encoding/json" "errors" + "io" "net/url" "reflect" @@ -15,7 +17,15 @@ var ( ErrorNotFound = errors.New("no data found") ) +type Client struct { + Credentials oidc.ClientCredentials + Metadata oidc.ClientMetadata + Admin bool +} + type ClientIdentityRepo interface { + Get(clientID string) (Client, error) + // Metadata returns one matching ClientMetadata if the given client // exists, otherwise nil. The returned error will be non-nil only // if the repo was unable to determine client existence. @@ -28,12 +38,12 @@ type ClientIdentityRepo interface { Authenticate(creds oidc.ClientCredentials) (bool, error) // All returns all registered Client Identities. - All() ([]oidc.ClientIdentity, error) + All() ([]Client, error) // New registers a ClientIdentity with the repo for the given metadata. // An unused ID must be provided. A corresponding secret will be returned // in a ClientCredentials struct along with the provided ID. - New(id string, meta oidc.ClientMetadata, admin bool) (*oidc.ClientCredentials, error) + New(client Client) (*oidc.ClientCredentials, error) SetDexAdmin(clientID string, isAdmin bool) error @@ -64,3 +74,36 @@ func ValidRedirectURL(rURL *url.URL, redirectURLs []url.URL) (url.URL, error) { } return url.URL{}, ErrorInvalidRedirectURL } + +func ClientsFromReader(r io.Reader) ([]Client, error) { + var c []struct { + ID string `json:"id"` + Secret string `json:"secret"` + RedirectURLs []string `json:"redirectURLs"` + } + if err := json.NewDecoder(r).Decode(&c); err != nil { + return nil, err + } + clients := make([]Client, len(c)) + for i, client := range c { + redirectURIs := make([]url.URL, len(client.RedirectURLs)) + for j, u := range client.RedirectURLs { + uri, err := url.Parse(u) + if err != nil { + return nil, err + } + redirectURIs[j] = *uri + } + + clients[i] = Client{ + Credentials: oidc.ClientCredentials{ + ID: client.ID, + Secret: client.Secret, + }, + Metadata: oidc.ClientMetadata{ + RedirectURIs: redirectURIs, + }, + } + } + return clients, nil +} diff --git a/db/client.go b/db/client.go index a0f54846..86e81afb 100644 --- a/db/client.go +++ b/db/client.go @@ -41,21 +41,29 @@ func init() { }) } -func newClientIdentityModel(id string, secret []byte, meta *oidc.ClientMetadata) (*clientIdentityModel, error) { - hashed, err := bcrypt.GenerateFromPassword(secret, bcryptHashCost) +func newClientIdentityModel(cli client.Client) (*clientIdentityModel, error) { + secretBytes, err := base64.URLEncoding.DecodeString(cli.Credentials.Secret) if err != nil { return nil, err } - bmeta, err := json.Marshal(meta) + hashed, err := bcrypt.GenerateFromPassword([]byte( + secretBytes), + bcryptHashCost) + if err != nil { + return nil, err + } + + bmeta, err := json.Marshal(&cli.Metadata) if err != nil { return nil, err } cim := clientIdentityModel{ - ID: id, + ID: cli.Credentials.ID, Secret: hashed, Metadata: string(bmeta), + DexAdmin: cli.Admin, } return &cim, nil @@ -68,12 +76,13 @@ type clientIdentityModel struct { DexAdmin bool `db:"dex_admin"` } -func (m *clientIdentityModel) ClientIdentity() (*oidc.ClientIdentity, error) { - ci := oidc.ClientIdentity{ +func (m *clientIdentityModel) Client() (*client.Client, error) { + ci := client.Client{ Credentials: oidc.ClientCredentials{ ID: m.ID, Secret: string(m.Secret), }, + Admin: m.DexAdmin, } if err := json.Unmarshal([]byte(m.Metadata), &ci.Metadata); err != nil { @@ -91,7 +100,7 @@ func newClientIdentityRepo(dbm *gorp.DbMap) *clientIdentityRepo { return &clientIdentityRepo{db: &db{dbm}} } -func NewClientIdentityRepoFromClients(dbm *gorp.DbMap, clients []oidc.ClientIdentity) (client.ClientIdentityRepo, error) { +func NewClientIdentityRepoFromClients(dbm *gorp.DbMap, clients []client.Client) (client.ClientIdentityRepo, error) { repo := newClientIdentityRepo(dbm) tx, err := repo.begin() if err != nil { @@ -103,12 +112,7 @@ func NewClientIdentityRepoFromClients(dbm *gorp.DbMap, clients []oidc.ClientIden if c.Credentials.Secret == "" { return nil, fmt.Errorf("client %q has no secret", c.Credentials.ID) } - dec, err := base64.URLEncoding.DecodeString(c.Credentials.Secret) - if err != nil { - return nil, fmt.Errorf("client secrets must be base64 decodable. See issue #337. Please consider replacing %q with %q", - c.Credentials.Secret, base64.URLEncoding.EncodeToString([]byte(c.Credentials.Secret))) - } - cm, err := newClientIdentityModel(c.Credentials.ID, dec, &c.Metadata) + cm, err := newClientIdentityModel(c) if err != nil { return nil, err } @@ -127,27 +131,37 @@ type clientIdentityRepo struct { *db } -func (r *clientIdentityRepo) Metadata(clientID string) (*oidc.ClientMetadata, error) { +func (r *clientIdentityRepo) Get(clientID string) (client.Client, error) { m, err := r.executor(nil).Get(clientIdentityModel{}, clientID) + if err == sql.ErrNoRows || m == nil { - return nil, client.ErrorNotFound + return client.Client{}, client.ErrorNotFound } if err != nil { - return nil, err + return client.Client{}, err } cim, ok := m.(*clientIdentityModel) if !ok { - log.Errorf("expected clientIdentityModel but found %v", reflect.TypeOf(m)) - return nil, errors.New("unrecognized model") + log.Errorf("expected clientModel but found %v", reflect.TypeOf(m)) + return client.Client{}, errors.New("unrecognized model") } - ci, err := cim.ClientIdentity() + ci, err := cim.Client() + if err != nil { + return client.Client{}, err + } + + return *ci, nil +} + +func (r *clientIdentityRepo) Metadata(clientID string) (*oidc.ClientMetadata, error) { + c, err := r.Get(clientID) if err != nil { return nil, err } - return &ci.Metadata, nil + return &c.Metadata, nil } func (r *clientIdentityRepo) IsDexAdmin(clientID string) (bool, error) { @@ -238,17 +252,17 @@ func isAlreadyExistsErr(err error) bool { return false } -func (r *clientIdentityRepo) New(id string, meta oidc.ClientMetadata, admin bool) (*oidc.ClientCredentials, error) { +func (r *clientIdentityRepo) New(cli client.Client) (*oidc.ClientCredentials, error) { secret, err := pcrypto.RandBytes(maxSecretLength) if err != nil { return nil, err } - cim, err := newClientIdentityModel(id, secret, &meta) + cli.Credentials.Secret = base64.URLEncoding.EncodeToString(secret) + cim, err := newClientIdentityModel(cli) if err != nil { return nil, err } - cim.DexAdmin = admin if err := r.executor(nil).Insert(cim); err != nil { if isAlreadyExistsErr(err) { @@ -258,14 +272,14 @@ func (r *clientIdentityRepo) New(id string, meta oidc.ClientMetadata, admin bool } cc := oidc.ClientCredentials{ - ID: id, - Secret: base64.URLEncoding.EncodeToString(secret), + ID: cli.Credentials.ID, + Secret: cli.Credentials.Secret, } return &cc, nil } -func (r *clientIdentityRepo) All() ([]oidc.ClientIdentity, error) { +func (r *clientIdentityRepo) All() ([]client.Client, error) { qt := r.quote(clientIdentityTableName) q := fmt.Sprintf("SELECT * FROM %s", qt) objs, err := r.executor(nil).Select(&clientIdentityModel{}, q) @@ -273,14 +287,14 @@ func (r *clientIdentityRepo) All() ([]oidc.ClientIdentity, error) { return nil, err } - cs := make([]oidc.ClientIdentity, len(objs)) + cs := make([]client.Client, len(objs)) for i, obj := range objs { m, ok := obj.(*clientIdentityModel) if !ok { return nil, errors.New("unable to cast client identity to clientIdentityModel") } - ci, err := m.ClientIdentity() + ci, err := m.Client() if err != nil { return nil, err } diff --git a/db/refresh.go b/db/refresh.go index 86495748..1e71c60f 100644 --- a/db/refresh.go +++ b/db/refresh.go @@ -11,10 +11,10 @@ import ( "github.com/go-gorp/gorp" "golang.org/x/crypto/bcrypt" + "github.com/coreos/dex/client" "github.com/coreos/dex/pkg/log" "github.com/coreos/dex/refresh" "github.com/coreos/dex/repo" - "github.com/coreos/go-oidc/oidc" ) const ( @@ -186,7 +186,7 @@ func (r *refreshTokenRepo) RevokeTokensForClient(userID, clientID string) error return err } -func (r *refreshTokenRepo) ClientsWithRefreshTokens(userID string) ([]oidc.ClientIdentity, error) { +func (r *refreshTokenRepo) ClientsWithRefreshTokens(userID string) ([]client.Client, error) { q := `SELECT c.* FROM %s as c INNER JOIN %s as r ON c.id = r.client_id WHERE r.user_id = $1;` q = fmt.Sprintf(q, r.quote(clientIdentityTableName), r.quote(refreshTokenTableName)) @@ -196,9 +196,9 @@ func (r *refreshTokenRepo) ClientsWithRefreshTokens(userID string) ([]oidc.Clien return nil, err } - c := make([]oidc.ClientIdentity, len(clients)) + c := make([]client.Client, len(clients)) for i, client := range clients { - ident, err := client.ClientIdentity() + ident, err := client.Client() if err != nil { return nil, err } diff --git a/functional/db_test.go b/functional/db_test.go index 97efdfe5..7690a5b0 100644 --- a/functional/db_test.go +++ b/functional/db_test.go @@ -191,7 +191,12 @@ func TestDBClientIdentityRepoMetadata(t *testing.T) { }, } - _, err := r.New("foo", cm, false) + _, err := r.New(client.Client{ + Credentials: oidc.ClientCredentials{ + ID: "foo", + }, + Metadata: cm, + }) if err != nil { t.Fatalf(err.Error()) } @@ -227,7 +232,12 @@ func TestDBClientIdentityRepoNewDuplicate(t *testing.T) { }, } - if _, err := r.New("foo", meta1, false); err != nil { + if _, err := r.New(client.Client{ + Credentials: oidc.ClientCredentials{ + ID: "foo", + }, + Metadata: meta1, + }); err != nil { t.Fatalf("unexpected error: %v", err) } @@ -237,7 +247,12 @@ func TestDBClientIdentityRepoNewDuplicate(t *testing.T) { }, } - if _, err := r.New("foo", meta2, false); err == nil { + if _, err := r.New(client.Client{ + Credentials: oidc.ClientCredentials{ + ID: "foo", + }, + Metadata: meta2, + }); err == nil { t.Fatalf("expected non-nil error") } } @@ -251,7 +266,12 @@ func TestDBClientIdentityRepoAuthenticate(t *testing.T) { }, } - cc, err := r.New("baz", cm, false) + cc, err := r.New(client.Client{ + Credentials: oidc.ClientCredentials{ + ID: "baz", + }, + Metadata: cm, + }) if err != nil { t.Fatalf(err.Error()) } @@ -299,7 +319,12 @@ func TestDBClientIdentityAll(t *testing.T) { }, } - _, err := r.New("foo", cm, false) + _, err := r.New(client.Client{ + Credentials: oidc.ClientCredentials{ + ID: "foo", + }, + Metadata: cm, + }) if err != nil { t.Fatalf(err.Error()) } @@ -322,7 +347,12 @@ func TestDBClientIdentityAll(t *testing.T) { url.URL{Scheme: "http", Host: "foo.com", Path: "/cb"}, }, } - _, err = r.New("bar", cm, false) + _, err = r.New(client.Client{ + Credentials: oidc.ClientCredentials{ + ID: "bar", + }, + Metadata: cm, + }) if err != nil { t.Fatalf(err.Error()) } diff --git a/functional/repo/client_repo_test.go b/functional/repo/client_repo_test.go index fff50ca1..62097b3e 100644 --- a/functional/repo/client_repo_test.go +++ b/functional/repo/client_repo_test.go @@ -14,8 +14,8 @@ import ( ) var ( - testClients = []oidc.ClientIdentity{ - oidc.ClientIdentity{ + testClients = []client.Client{ + client.Client{ Credentials: oidc.ClientCredentials{ ID: "client1", Secret: base64.URLEncoding.EncodeToString([]byte("secret-1")), @@ -30,7 +30,7 @@ var ( }, }, }, - oidc.ClientIdentity{ + client.Client{ Credentials: oidc.ClientCredentials{ ID: "client2", Secret: base64.URLEncoding.EncodeToString([]byte("secret-2")), diff --git a/functional/repo/refresh_repo_test.go b/functional/repo/refresh_repo_test.go index 197f59ad..40862663 100644 --- a/functional/repo/refresh_repo_test.go +++ b/functional/repo/refresh_repo_test.go @@ -11,12 +11,13 @@ import ( "github.com/go-gorp/gorp" "github.com/kylelemons/godebug/pretty" + "github.com/coreos/dex/client" "github.com/coreos/dex/db" "github.com/coreos/dex/refresh" "github.com/coreos/dex/user" ) -func newRefreshRepo(t *testing.T, users []user.UserWithRemoteIdentities, clients []oidc.ClientIdentity) refresh.RefreshTokenRepo { +func newRefreshRepo(t *testing.T, users []user.UserWithRemoteIdentities, clients []client.Client) refresh.RefreshTokenRepo { var dbMap *gorp.DbMap if dsn := os.Getenv("DEX_TEST_DSN"); dsn == "" { dbMap = db.NewMemDB() @@ -35,7 +36,7 @@ func newRefreshRepo(t *testing.T, users []user.UserWithRemoteIdentities, clients func TestRefreshTokenRepo(t *testing.T) { clientID := "client1" userID := "user1" - clients := []oidc.ClientIdentity{ + clients := []client.Client{ { Credentials: oidc.ClientCredentials{ ID: clientID, diff --git a/integration/admin_api_test.go b/integration/admin_api_test.go index 861e9788..0714b72f 100644 --- a/integration/admin_api_test.go +++ b/integration/admin_api_test.go @@ -276,21 +276,24 @@ func TestCreateClient(t *testing.T) { for i, tt := range tests { err := func() error { f := makeAdminAPITestFixtures() - req := &adminschema.ClientCreateRequestClient{} - for _, redirectURI := range tt.client.RedirectURIs { - req.Redirect_uris = append(req.Redirect_uris, redirectURI.String()) + req := &adminschema.ClientCreateRequest{ + Client: &adminschema.Client{}, } - resp, err := f.adClient.Client.Create(&adminschema.ClientCreateRequest{Client: req}).Do() + + for _, redirectURI := range tt.client.RedirectURIs { + req.Client.RedirectURIs = append(req.Client.RedirectURIs, redirectURI.String()) + } + resp, err := f.adClient.Client.Create(req).Do() if err != nil { if tt.wantError { return nil } return err } - if resp.Client_id == "" { + if resp.Client.Id == "" { return errors.New("no client id returned") } - if resp.Client_secret == "" { + if resp.Client.Secret == "" { return errors.New("no client secret returned") } return nil diff --git a/integration/client_api_test.go b/integration/client_api_test.go index 3b80eb33..fd3e63bb 100644 --- a/integration/client_api_test.go +++ b/integration/client_api_test.go @@ -7,12 +7,13 @@ import ( "reflect" "testing" + "github.com/coreos/dex/client" schema "github.com/coreos/dex/schema/workerschema" "github.com/coreos/go-oidc/oidc" ) func TestClientCreate(t *testing.T) { - ci := oidc.ClientIdentity{ + ci := client.Client{ Credentials: oidc.ClientCredentials{ ID: "72de74a9", Secret: base64.URLEncoding.EncodeToString([]byte("XXX")), @@ -23,7 +24,7 @@ func TestClientCreate(t *testing.T) { }, }, } - cis := []oidc.ClientIdentity{ci} + cis := []client.Client{ci} srv, err := mockServer(cis) if err != nil { diff --git a/integration/oidc_test.go b/integration/oidc_test.go index 48e7d862..1195e118 100644 --- a/integration/oidc_test.go +++ b/integration/oidc_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/coreos/dex/client" "github.com/coreos/dex/connector" "github.com/coreos/dex/db" phttp "github.com/coreos/dex/pkg/http" @@ -22,7 +23,7 @@ import ( "github.com/coreos/go-oidc/oidc" ) -func mockServer(cis []oidc.ClientIdentity) (*server.Server, error) { +func mockServer(cis []client.Client) (*server.Server, error) { dbMap := db.NewMemDB() k, err := key.GeneratePrivateKey() if err != nil { @@ -50,7 +51,7 @@ func mockServer(cis []oidc.ClientIdentity) (*server.Server, error) { return srv, nil } -func mockClient(srv *server.Server, ci oidc.ClientIdentity) (*oidc.Client, error) { +func mockClient(srv *server.Server, ci client.Client) (*oidc.Client, error) { hdlr := srv.HTTPHandler() sClient := &phttp.HandlerClient{Handler: hdlr} @@ -75,7 +76,7 @@ func mockClient(srv *server.Server, ci oidc.ClientIdentity) (*oidc.Client, error return oidc.NewClient(ccfg) } -func verifyUserClaims(claims jose.Claims, ci *oidc.ClientIdentity, user *user.User, issuerURL url.URL) error { +func verifyUserClaims(claims jose.Claims, ci *client.Client, user *user.User, issuerURL url.URL) error { expectedSub, expectedName := ci.Credentials.ID, ci.Credentials.ID if user != nil { expectedSub, expectedName = user.ID, user.DisplayName @@ -116,7 +117,7 @@ func TestHTTPExchangeTokenRefreshToken(t *testing.T) { ID: "local", } - ci := oidc.ClientIdentity{ + ci := client.Client{ Credentials: oidc.ClientCredentials{ ID: "72de74a9", Secret: base64.URLEncoding.EncodeToString([]byte("XXX")), @@ -124,7 +125,7 @@ func TestHTTPExchangeTokenRefreshToken(t *testing.T) { } dbMap := db.NewMemDB() - cir, err := db.NewClientIdentityRepoFromClients(dbMap, []oidc.ClientIdentity{ci}) + cir, err := db.NewClientIdentityRepoFromClients(dbMap, []client.Client{ci}) if err != nil { t.Fatalf("Failed to create client identity repo: " + err.Error()) } @@ -262,13 +263,13 @@ func TestHTTPExchangeTokenRefreshToken(t *testing.T) { } func TestHTTPClientCredsToken(t *testing.T) { - ci := oidc.ClientIdentity{ + ci := client.Client{ Credentials: oidc.ClientCredentials{ ID: "72de74a9", Secret: base64.URLEncoding.EncodeToString([]byte("XXX")), }, } - cis := []oidc.ClientIdentity{ci} + cis := []client.Client{ci} srv, err := mockServer(cis) if err != nil { diff --git a/integration/user_api_test.go b/integration/user_api_test.go index 14a2f161..6795f73d 100644 --- a/integration/user_api_test.go +++ b/integration/user_api_test.go @@ -102,8 +102,8 @@ func makeUserAPITestFixtures() *userAPITestFixtures { dbMap, _, _, um := makeUserObjects(userUsers, userPasswords) cir := func() client.ClientIdentityRepo { - repo, err := db.NewClientIdentityRepoFromClients(dbMap, []oidc.ClientIdentity{ - oidc.ClientIdentity{ + repo, err := db.NewClientIdentityRepoFromClients(dbMap, []client.Client{ + client.Client{ Credentials: oidc.ClientCredentials{ ID: testClientID, Secret: testClientSecret, @@ -114,7 +114,7 @@ func makeUserAPITestFixtures() *userAPITestFixtures { }, }, }, - oidc.ClientIdentity{ + client.Client{ Credentials: oidc.ClientCredentials{ ID: userBadClientID, Secret: base64.URLEncoding.EncodeToString([]byte("secret")), diff --git a/refresh/repo.go b/refresh/repo.go index 607169fe..8f196a15 100644 --- a/refresh/repo.go +++ b/refresh/repo.go @@ -4,7 +4,7 @@ import ( "crypto/rand" "errors" - "github.com/coreos/go-oidc/oidc" + "github.com/coreos/dex/client" ) const ( @@ -54,5 +54,5 @@ type RefreshTokenRepo interface { RevokeTokensForClient(userID, clientID string) error // ClientsWithRefreshTokens returns a list of all clients the user has an outstanding client with. - ClientsWithRefreshTokens(userID string) ([]oidc.ClientIdentity, error) + ClientsWithRefreshTokens(userID string) ([]client.Client, error) } diff --git a/server/admin.go b/server/admin.go index 21bd28d5..43d29c4f 100644 --- a/server/admin.go +++ b/server/admin.go @@ -116,7 +116,7 @@ func (s *AdminServer) getState(w http.ResponseWriter, r *http.Request, ps httpro } func (s *AdminServer) createClient(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { - var req admin.ClientRegistrationRequest + var req = adminschema.ClientCreateRequest{} if err := json.NewDecoder(r.Body).Decode(&req); err != nil { writeInvalidRequest(w, "cannot parse JSON body") return diff --git a/server/auth_middleware_test.go b/server/auth_middleware_test.go index 0b0f1b2a..3dfb1695 100644 --- a/server/auth_middleware_test.go +++ b/server/auth_middleware_test.go @@ -26,7 +26,7 @@ func TestClientToken(t *testing.T) { now := time.Now() tomorrow := now.Add(24 * time.Hour) validClientID := "valid-client" - ci := oidc.ClientIdentity{ + ci := client.Client{ Credentials: oidc.ClientCredentials{ ID: validClientID, Secret: base64.URLEncoding.EncodeToString([]byte("secret")), @@ -37,7 +37,7 @@ func TestClientToken(t *testing.T) { }, }, } - repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []oidc.ClientIdentity{ci}) + repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ci}) if err != nil { t.Fatalf("Failed to create client identity repo: %v", err) } diff --git a/server/client_registration.go b/server/client_registration.go index 13dacbeb..f1c21e99 100644 --- a/server/client_registration.go +++ b/server/client_registration.go @@ -4,7 +4,9 @@ import ( "encoding/json" "net/http" + "github.com/coreos/dex/client" "github.com/coreos/dex/pkg/log" + "github.com/coreos/go-oidc/oauth2" "github.com/coreos/go-oidc/oidc" ) @@ -43,7 +45,12 @@ func (s *Server) handleClientRegistrationRequest(r *http.Request) (*oidc.ClientR return nil, newAPIError(oauth2.ErrorServerError, "unable to save client metadata") } - creds, err := s.ClientIdentityRepo.New(id, clientMetadata, false) + creds, err := s.ClientIdentityRepo.New(client.Client{ + Credentials: oidc.ClientCredentials{ + ID: id, + }, + Metadata: clientMetadata, + }) if err != nil { log.Errorf("Failed to create new client identity: %v", err) return nil, newAPIError(oauth2.ErrorServerError, "unable to save client metadata") diff --git a/server/client_resource.go b/server/client_resource.go index c5134779..c69e9062 100644 --- a/server/client_resource.go +++ b/server/client_resource.go @@ -49,7 +49,7 @@ func (c *clientResource) list(w http.ResponseWriter, r *http.Request) { scs := make([]*schema.Client, len(cs)) for i, ci := range cs { - sc := schema.MapClientIdentityToSchemaClient(ci) + sc := schema.MapClientToSchemaClient(ci) scs[i] = &sc } @@ -76,7 +76,7 @@ func (c *clientResource) create(w http.ResponseWriter, r *http.Request) { return } - ci, err := schema.MapSchemaClientToClientIdentity(sc) + ci, err := schema.MapSchemaClientToClient(sc) if err != nil { log.Debugf("Invalid request data: %v", err) writeAPIError(w, http.StatusBadRequest, newAPIError(errorInvalidClientMetadata, "missing or invalid field: redirectURIs")) @@ -96,7 +96,9 @@ func (c *clientResource) create(w http.ResponseWriter, r *http.Request) { return } - creds, err := c.repo.New(clientID, ci.Metadata, false) + ci.Credentials.ID = clientID + creds, err := c.repo.New(ci) + if err != nil { log.Errorf("Failed creating client: %v", err) writeAPIError(w, http.StatusInternalServerError, newAPIError(errorServerError, "unable to create client")) @@ -104,7 +106,7 @@ func (c *clientResource) create(w http.ResponseWriter, r *http.Request) { } ci.Credentials = *creds - ssc := schema.MapClientIdentityToSchemaClientWithSecret(ci) + ssc := schema.MapClientToSchemaClientWithSecret(ci) w.Header().Add("Location", phttp.NewResourceLocation(r.URL, ci.Credentials.ID)) writeResponseWithBody(w, http.StatusCreated, ssc) } diff --git a/server/client_resource_test.go b/server/client_resource_test.go index ab527120..ceb4232e 100644 --- a/server/client_resource_test.go +++ b/server/client_resource_test.go @@ -14,6 +14,7 @@ import ( "strings" "testing" + "github.com/coreos/dex/client" "github.com/coreos/dex/db" schema "github.com/coreos/dex/schema/workerschema" "github.com/coreos/go-oidc/oidc" @@ -177,7 +178,7 @@ func TestList(t *testing.T) { } tests := []struct { - cs []oidc.ClientIdentity + cs []client.Client want []*schema.Client }{ // empty repo @@ -187,8 +188,8 @@ func TestList(t *testing.T) { }, // single client { - cs: []oidc.ClientIdentity{ - oidc.ClientIdentity{ + cs: []client.Client{ + client.Client{ Credentials: oidc.ClientCredentials{ID: "foo", Secret: b64Encode("bar")}, Metadata: oidc.ClientMetadata{ RedirectURIs: []url.URL{ @@ -206,8 +207,8 @@ func TestList(t *testing.T) { }, // multi client { - cs: []oidc.ClientIdentity{ - oidc.ClientIdentity{ + cs: []client.Client{ + client.Client{ Credentials: oidc.ClientCredentials{ID: "foo", Secret: b64Encode("bar")}, Metadata: oidc.ClientMetadata{ RedirectURIs: []url.URL{ @@ -215,7 +216,7 @@ func TestList(t *testing.T) { }, }, }, - oidc.ClientIdentity{ + client.Client{ Credentials: oidc.ClientCredentials{ID: "biz", Secret: b64Encode("bang")}, Metadata: oidc.ClientMetadata{ RedirectURIs: []url.URL{ diff --git a/server/config.go b/server/config.go index 50df485c..bb505510 100644 --- a/server/config.go +++ b/server/config.go @@ -13,10 +13,10 @@ import ( "time" "github.com/coreos/go-oidc/key" - "github.com/coreos/go-oidc/oidc" "github.com/coreos/pkg/health" "github.com/go-gorp/gorp" + "github.com/coreos/dex/client" "github.com/coreos/dex/connector" "github.com/coreos/dex/db" "github.com/coreos/dex/email" @@ -214,47 +214,14 @@ func loadUsersFromReader(r io.Reader) (users []user.UserWithRemoteIdentities, pw return } -// loadClients parses the clients.json file and returns the clients to be created. -func loadClients(filepath string) ([]oidc.ClientIdentity, error) { +// loadClients parses the clients.json file and returns a list of clients. +func loadClients(filepath string) ([]client.Client, error) { f, err := os.Open(filepath) if err != nil { return nil, err } defer f.Close() - return loadClientsFromReader(f) -} - -func loadClientsFromReader(r io.Reader) ([]oidc.ClientIdentity, error) { - var c []struct { - ID string `json:"id"` - Secret string `json:"secret"` - RedirectURLs []string `json:"redirectURLs"` - } - if err := json.NewDecoder(r).Decode(&c); err != nil { - return nil, err - } - clients := make([]oidc.ClientIdentity, len(c)) - for i, client := range c { - redirectURIs := make([]url.URL, len(client.RedirectURLs)) - for j, u := range client.RedirectURLs { - uri, err := url.Parse(u) - if err != nil { - return nil, err - } - redirectURIs[j] = *uri - } - - clients[i] = oidc.ClientIdentity{ - Credentials: oidc.ClientCredentials{ - ID: client.ID, - Secret: client.Secret, - }, - Metadata: oidc.ClientMetadata{ - RedirectURIs: redirectURIs, - }, - } - } - return clients, nil + return client.ClientsFromReader(f) } func (cfg *MultiServerConfig) Configure(srv *Server) error { diff --git a/server/http_test.go b/server/http_test.go index 319f9d91..c96a6bbb 100644 --- a/server/http_test.go +++ b/server/http_test.go @@ -79,8 +79,8 @@ func TestHandleAuthFuncResponsesSingleRedirectURL(t *testing.T) { IssuerURL: url.URL{Scheme: "http", Host: "server.example.com"}, SessionManager: manager.NewSessionManager(db.NewSessionRepo(db.NewMemDB()), db.NewSessionKeyRepo(db.NewMemDB())), ClientIdentityRepo: func() client.ClientIdentityRepo { - repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []oidc.ClientIdentity{ - oidc.ClientIdentity{ + repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ + client.Client{ Credentials: oidc.ClientCredentials{ ID: "XXX", Secret: base64.URLEncoding.EncodeToString([]byte("secrete")), @@ -231,8 +231,8 @@ func TestHandleAuthFuncResponsesMultipleRedirectURLs(t *testing.T) { IssuerURL: url.URL{Scheme: "http", Host: "server.example.com"}, SessionManager: manager.NewSessionManager(db.NewSessionRepo(db.NewMemDB()), db.NewSessionKeyRepo(db.NewMemDB())), ClientIdentityRepo: func() client.ClientIdentityRepo { - repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []oidc.ClientIdentity{ - oidc.ClientIdentity{ + repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ + client.Client{ Credentials: oidc.ClientCredentials{ ID: "XXX", Secret: base64.URLEncoding.EncodeToString([]byte("secrete")), diff --git a/server/server_test.go b/server/server_test.go index 0aec6468..17a2e814 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -130,7 +130,7 @@ func TestServerNewSession(t *testing.T) { state := "pants" nonce := "oncenay" - ci := oidc.ClientIdentity{ + ci := client.Client{ Credentials: oidc.ClientCredentials{ ID: "XXX", Secret: "secrete", @@ -179,7 +179,7 @@ func TestServerNewSession(t *testing.T) { } func TestServerLogin(t *testing.T) { - ci := oidc.ClientIdentity{ + ci := client.Client{ Credentials: oidc.ClientCredentials{ ID: "XXX", Secret: clientTestSecret, @@ -195,7 +195,7 @@ func TestServerLogin(t *testing.T) { }, } ciRepo := func() client.ClientIdentityRepo { - repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []oidc.ClientIdentity{ci}) + repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ci}) if err != nil { t.Fatalf("Failed to create client identity repo: %v", err) } @@ -245,8 +245,8 @@ func TestServerLogin(t *testing.T) { func TestServerLoginUnrecognizedSessionKey(t *testing.T) { ciRepo := func() client.ClientIdentityRepo { - repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []oidc.ClientIdentity{ - oidc.ClientIdentity{ + repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ + client.Client{ Credentials: oidc.ClientCredentials{ ID: "XXX", Secret: clientTestSecret, }, @@ -281,7 +281,7 @@ func TestServerLoginUnrecognizedSessionKey(t *testing.T) { } func TestServerLoginDisabledUser(t *testing.T) { - ci := oidc.ClientIdentity{ + ci := client.Client{ Credentials: oidc.ClientCredentials{ ID: "XXX", Secret: clientTestSecret, @@ -297,7 +297,7 @@ func TestServerLoginDisabledUser(t *testing.T) { }, } ciRepo := func() client.ClientIdentityRepo { - repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []oidc.ClientIdentity{ci}) + repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ci}) if err != nil { t.Fatalf("Failed to create client identity repo: %v", err) } @@ -355,14 +355,14 @@ func TestServerLoginDisabledUser(t *testing.T) { } func TestServerCodeToken(t *testing.T) { - ci := oidc.ClientIdentity{ + ci := client.Client{ Credentials: oidc.ClientCredentials{ ID: "XXX", Secret: clientTestSecret, }, } ciRepo := func() client.ClientIdentityRepo { - repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []oidc.ClientIdentity{ci}) + repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ci}) if err != nil { t.Fatalf("Failed to create client identity repo: %v", err) } @@ -441,14 +441,14 @@ func TestServerCodeToken(t *testing.T) { } func TestServerTokenUnrecognizedKey(t *testing.T) { - ci := oidc.ClientIdentity{ + ci := client.Client{ Credentials: oidc.ClientCredentials{ ID: "XXX", Secret: clientTestSecret, }, } ciRepo := func() client.ClientIdentityRepo { - repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []oidc.ClientIdentity{ci}) + repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ci}) if err != nil { t.Fatalf("Failed to create client identity repo: %v", err) } @@ -569,8 +569,8 @@ func TestServerTokenFail(t *testing.T) { km := &StaticKeyManager{ signer: tt.signer, } - ciRepo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []oidc.ClientIdentity{ - oidc.ClientIdentity{Credentials: ccFixture}, + ciRepo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ + client.Client{Credentials: ccFixture}, }) if err != nil { t.Errorf("case %d: failed to create client identity repo: %v", i, err) @@ -731,9 +731,9 @@ func TestServerRefreshToken(t *testing.T) { signer: tt.signer, } - ciRepo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []oidc.ClientIdentity{ - oidc.ClientIdentity{Credentials: credXXX}, - oidc.ClientIdentity{Credentials: credYYY}, + ciRepo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ + client.Client{Credentials: credXXX}, + client.Client{Credentials: credYYY}, }) if err != nil { t.Errorf("case %d: failed to create client identity repo: %v", i, err) @@ -784,9 +784,9 @@ func TestServerRefreshToken(t *testing.T) { signer: signerFixture, } - ciRepo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []oidc.ClientIdentity{ - oidc.ClientIdentity{Credentials: credXXX}, - oidc.ClientIdentity{Credentials: credYYY}, + ciRepo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ + client.Client{Credentials: credXXX}, + client.Client{Credentials: credYYY}, }) if err != nil { t.Fatalf("failed to create client identity repo: %v", err) diff --git a/server/testutil.go b/server/testutil.go index c1d5ce31..71cd05a2 100644 --- a/server/testutil.go +++ b/server/testutil.go @@ -136,8 +136,8 @@ func makeTestFixtures() (*testFixtures, error) { return nil, err } - clientIdentityRepo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []oidc.ClientIdentity{ - oidc.ClientIdentity{ + clientIdentityRepo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ + client.Client{ Credentials: oidc.ClientCredentials{ ID: "XXX", Secret: base64.URLEncoding.EncodeToString([]byte("secrete")), diff --git a/user/api/api_test.go b/user/api/api_test.go index de5b62d0..66d82552 100644 --- a/user/api/api_test.go +++ b/user/api/api_test.go @@ -11,6 +11,7 @@ import ( "github.com/jonboulle/clockwork" "github.com/kylelemons/godebug/pretty" + "github.com/coreos/dex/client" "github.com/coreos/dex/connector" "github.com/coreos/dex/db" schema "github.com/coreos/dex/schema/workerschema" @@ -155,7 +156,7 @@ func makeTestFixtures() (*UsersAPI, *testEmailer) { mgr := manager.NewUserManager(ur, pwr, ccr, db.TransactionFactory(dbMap), manager.ManagerOptions{}) mgr.Clock = clock - ci := oidc.ClientIdentity{ + ci := client.Client{ Credentials: oidc.ClientCredentials{ ID: "XXX", Secret: base64.URLEncoding.EncodeToString([]byte("secrete")), @@ -166,8 +167,9 @@ func makeTestFixtures() (*UsersAPI, *testEmailer) { }, }, } - if _, err := db.NewClientIdentityRepoFromClients(dbMap, []oidc.ClientIdentity{ci}); err != nil { - panic("Failed to create client identity repo: " + err.Error()) + + if _, err := db.NewClientIdentityRepoFromClients(dbMap, []client.Client{ci}); err != nil { + panic("Failed to create client repo: " + err.Error()) } // Used in TestRevokeRefreshToken test. From e5948ab3ce1f9df85d42f80eb63b2f17c1c34648 Mon Sep 17 00:00:00 2001 From: Bobby Rullo Date: Thu, 14 Apr 2016 16:27:57 -0700 Subject: [PATCH 4/9] *: ClientIdentityXXX -> ClientXXX Get rid of all outdated "ClientIdentity" terminology. --- admin/api.go | 22 +++--- client/client.go | 6 +- cmd/dexctl/driver_db.go | 4 +- db/client.go | 79 ++++++++++---------- db/migrate_test.go | 6 +- db/refresh.go | 4 +- functional/db_test.go | 20 ++--- functional/repo/client_repo_test.go | 6 +- functional/repo/refresh_repo_test.go | 2 +- integration/client_api_test.go | 2 +- integration/oidc_test.go | 30 ++++---- integration/user_api_test.go | 4 +- schema/adminschema/v1-json.go | 3 +- schema/workerschema/v1-json.go | 3 +- server/auth_middleware.go | 4 +- server/auth_middleware_test.go | 6 +- server/client_registration.go | 2 +- server/client_registration_test.go | 2 +- server/client_resource.go | 4 +- server/client_resource_test.go | 6 +- server/config.go | 8 +- server/email_verification.go | 4 +- server/email_verification_test.go | 2 +- server/http_test.go | 8 +- server/password.go | 2 +- server/password_test.go | 2 +- server/server.go | 20 ++--- server/server_test.go | 106 +++++++++++++-------------- server/testutil.go | 42 +++++------ server/user.go | 4 +- user/api/api.go | 24 +++--- user/api/api_test.go | 3 +- 32 files changed, 220 insertions(+), 220 deletions(-) diff --git a/admin/api.go b/admin/api.go index 22875c3e..5a821a9d 100644 --- a/admin/api.go +++ b/admin/api.go @@ -16,11 +16,11 @@ import ( // AdminAPI provides the logic necessary to implement the Admin API. type AdminAPI struct { - userManager *manager.UserManager - userRepo user.UserRepo - passwordInfoRepo user.PasswordInfoRepo - clientIdentityRepo client.ClientIdentityRepo - localConnectorID string + userManager *manager.UserManager + userRepo user.UserRepo + passwordInfoRepo user.PasswordInfoRepo + clientRepo client.ClientRepo + localConnectorID string } // TODO(ericchiang): Swap the DbMap for a storage interface. See #278 @@ -30,11 +30,11 @@ func NewAdminAPI(dbMap *gorp.DbMap, userManager *manager.UserManager, localConne panic("must specify non-blank localConnectorID") } return &AdminAPI{ - userManager: userManager, - userRepo: db.NewUserRepo(dbMap), - passwordInfoRepo: db.NewPasswordInfoRepo(dbMap), - clientIdentityRepo: db.NewClientIdentityRepo(dbMap), - localConnectorID: localConnectorID, + userManager: userManager, + userRepo: db.NewUserRepo(dbMap), + passwordInfoRepo: db.NewPasswordInfoRepo(dbMap), + clientRepo: db.NewClientRepo(dbMap), + localConnectorID: localConnectorID, } } @@ -136,7 +136,7 @@ func (a *AdminAPI) CreateClient(req adminschema.ClientCreateRequest) (adminschem cli.Credentials.ID = id - creds, err := a.clientIdentityRepo.New(cli) + creds, err := a.clientRepo.New(cli) if err != nil { return adminschema.ClientCreateResponse{}, mapError(err) } diff --git a/client/client.go b/client/client.go index 5a222c81..29e62943 100644 --- a/client/client.go +++ b/client/client.go @@ -23,7 +23,7 @@ type Client struct { Admin bool } -type ClientIdentityRepo interface { +type ClientRepo interface { Get(clientID string) (Client, error) // Metadata returns one matching ClientMetadata if the given client @@ -37,10 +37,10 @@ type ClientIdentityRepo interface { // to make these assertions will a non-nil error be returned. Authenticate(creds oidc.ClientCredentials) (bool, error) - // All returns all registered Client Identities. + // All returns all registered Clients All() ([]Client, error) - // New registers a ClientIdentity with the repo for the given metadata. + // New registers a Client with the repo. // An unused ID must be provided. A corresponding secret will be returned // in a ClientCredentials struct along with the provided ID. New(client Client) (*oidc.ClientCredentials, error) diff --git a/cmd/dexctl/driver_db.go b/cmd/dexctl/driver_db.go index fe918aa1..db1700d3 100644 --- a/cmd/dexctl/driver_db.go +++ b/cmd/dexctl/driver_db.go @@ -14,7 +14,7 @@ func newDBDriver(dsn string) (driver, error) { } drv := &dbDriver{ - ciRepo: db.NewClientIdentityRepo(dbc), + ciRepo: db.NewClientRepo(dbc), cfgRepo: db.NewConnectorConfigRepo(dbc), } @@ -22,7 +22,7 @@ func newDBDriver(dsn string) (driver, error) { } type dbDriver struct { - ciRepo client.ClientIdentityRepo + ciRepo client.ClientRepo cfgRepo *db.ConnectorConfigRepo } diff --git a/db/client.go b/db/client.go index 86e81afb..48960317 100644 --- a/db/client.go +++ b/db/client.go @@ -18,7 +18,7 @@ import ( ) const ( - clientIdentityTableName = "client_identity" + clientTableName = "client_identity" bcryptHashCost = 10 @@ -34,19 +34,18 @@ const ( func init() { register(table{ - name: clientIdentityTableName, - model: clientIdentityModel{}, + name: clientTableName, + model: clientModel{}, autoinc: false, pkey: []string{"id"}, }) } -func newClientIdentityModel(cli client.Client) (*clientIdentityModel, error) { +func newClientModel(cli client.Client) (*clientModel, error) { secretBytes, err := base64.URLEncoding.DecodeString(cli.Credentials.Secret) if err != nil { return nil, err } - hashed, err := bcrypt.GenerateFromPassword([]byte( secretBytes), bcryptHashCost) @@ -59,7 +58,7 @@ func newClientIdentityModel(cli client.Client) (*clientIdentityModel, error) { return nil, err } - cim := clientIdentityModel{ + cim := clientModel{ ID: cli.Credentials.ID, Secret: hashed, Metadata: string(bmeta), @@ -69,14 +68,14 @@ func newClientIdentityModel(cli client.Client) (*clientIdentityModel, error) { return &cim, nil } -type clientIdentityModel struct { +type clientModel struct { ID string `db:"id"` Secret []byte `db:"secret"` Metadata string `db:"metadata"` DexAdmin bool `db:"dex_admin"` } -func (m *clientIdentityModel) Client() (*client.Client, error) { +func (m *clientModel) Client() (*client.Client, error) { ci := client.Client{ Credentials: oidc.ClientCredentials{ ID: m.ID, @@ -92,16 +91,16 @@ func (m *clientIdentityModel) Client() (*client.Client, error) { return &ci, nil } -func NewClientIdentityRepo(dbm *gorp.DbMap) client.ClientIdentityRepo { - return newClientIdentityRepo(dbm) +func NewClientRepo(dbm *gorp.DbMap) client.ClientRepo { + return newClientRepo(dbm) } -func newClientIdentityRepo(dbm *gorp.DbMap) *clientIdentityRepo { - return &clientIdentityRepo{db: &db{dbm}} +func newClientRepo(dbm *gorp.DbMap) *clientRepo { + return &clientRepo{db: &db{dbm}} } -func NewClientIdentityRepoFromClients(dbm *gorp.DbMap, clients []client.Client) (client.ClientIdentityRepo, error) { - repo := newClientIdentityRepo(dbm) +func NewClientRepoFromClients(dbm *gorp.DbMap, clients []client.Client) (client.ClientRepo, error) { + repo := newClientRepo(dbm) tx, err := repo.begin() if err != nil { return nil, err @@ -112,7 +111,7 @@ func NewClientIdentityRepoFromClients(dbm *gorp.DbMap, clients []client.Client) if c.Credentials.Secret == "" { return nil, fmt.Errorf("client %q has no secret", c.Credentials.ID) } - cm, err := newClientIdentityModel(c) + cm, err := newClientModel(c) if err != nil { return nil, err } @@ -127,13 +126,12 @@ func NewClientIdentityRepoFromClients(dbm *gorp.DbMap, clients []client.Client) return repo, nil } -type clientIdentityRepo struct { +type clientRepo struct { *db } -func (r *clientIdentityRepo) Get(clientID string) (client.Client, error) { - m, err := r.executor(nil).Get(clientIdentityModel{}, clientID) - +func (r *clientRepo) Get(clientID string) (client.Client, error) { + m, err := r.executor(nil).Get(clientModel{}, clientID) if err == sql.ErrNoRows || m == nil { return client.Client{}, client.ErrorNotFound } @@ -141,7 +139,7 @@ func (r *clientIdentityRepo) Get(clientID string) (client.Client, error) { return client.Client{}, err } - cim, ok := m.(*clientIdentityModel) + cim, ok := m.(*clientModel) if !ok { log.Errorf("expected clientModel but found %v", reflect.TypeOf(m)) return client.Client{}, errors.New("unrecognized model") @@ -155,7 +153,7 @@ func (r *clientIdentityRepo) Get(clientID string) (client.Client, error) { return *ci, nil } -func (r *clientIdentityRepo) Metadata(clientID string) (*oidc.ClientMetadata, error) { +func (r *clientRepo) Metadata(clientID string) (*oidc.ClientMetadata, error) { c, err := r.Get(clientID) if err != nil { return nil, err @@ -164,22 +162,22 @@ func (r *clientIdentityRepo) Metadata(clientID string) (*oidc.ClientMetadata, er return &c.Metadata, nil } -func (r *clientIdentityRepo) IsDexAdmin(clientID string) (bool, error) { - m, err := r.executor(nil).Get(clientIdentityModel{}, clientID) +func (r *clientRepo) IsDexAdmin(clientID string) (bool, error) { + m, err := r.executor(nil).Get(clientModel{}, clientID) if m == nil || err != nil { return false, err } - cim, ok := m.(*clientIdentityModel) + cim, ok := m.(*clientModel) if !ok { - log.Errorf("expected clientIdentityModel but found %v", reflect.TypeOf(m)) + log.Errorf("expected clientModel but found %v", reflect.TypeOf(m)) return false, errors.New("unrecognized model") } return cim.DexAdmin, nil } -func (r *clientIdentityRepo) SetDexAdmin(clientID string, isAdmin bool) error { +func (r *clientRepo) SetDexAdmin(clientID string, isAdmin bool) error { tx, err := r.begin() if err != nil { return err @@ -187,14 +185,14 @@ func (r *clientIdentityRepo) SetDexAdmin(clientID string, isAdmin bool) error { defer tx.Rollback() exec := r.executor(tx) - m, err := exec.Get(clientIdentityModel{}, clientID) + m, err := exec.Get(clientModel{}, clientID) if m == nil || err != nil { return err } - cim, ok := m.(*clientIdentityModel) + cim, ok := m.(*clientModel) if !ok { - log.Errorf("expected clientIdentityModel but found %v", reflect.TypeOf(m)) + log.Errorf("expected clientModel but found %v", reflect.TypeOf(m)) return errors.New("unrecognized model") } @@ -207,15 +205,15 @@ func (r *clientIdentityRepo) SetDexAdmin(clientID string, isAdmin bool) error { return tx.Commit() } -func (r *clientIdentityRepo) Authenticate(creds oidc.ClientCredentials) (bool, error) { - m, err := r.executor(nil).Get(clientIdentityModel{}, creds.ID) +func (r *clientRepo) Authenticate(creds oidc.ClientCredentials) (bool, error) { + m, err := r.executor(nil).Get(clientModel{}, creds.ID) if m == nil || err != nil { return false, err } - cim, ok := m.(*clientIdentityModel) + cim, ok := m.(*clientModel) if !ok { - log.Errorf("expected clientIdentityModel but found %v", reflect.TypeOf(m)) + log.Errorf("expected clientModel but found %v", reflect.TypeOf(m)) return false, errors.New("unrecognized model") } @@ -252,14 +250,15 @@ func isAlreadyExistsErr(err error) bool { return false } -func (r *clientIdentityRepo) New(cli client.Client) (*oidc.ClientCredentials, error) { +func (r *clientRepo) New(cli client.Client) (*oidc.ClientCredentials, error) { secret, err := pcrypto.RandBytes(maxSecretLength) if err != nil { return nil, err } cli.Credentials.Secret = base64.URLEncoding.EncodeToString(secret) - cim, err := newClientIdentityModel(cli) + cim, err := newClientModel(cli) + if err != nil { return nil, err } @@ -279,19 +278,19 @@ func (r *clientIdentityRepo) New(cli client.Client) (*oidc.ClientCredentials, er return &cc, nil } -func (r *clientIdentityRepo) All() ([]client.Client, error) { - qt := r.quote(clientIdentityTableName) +func (r *clientRepo) All() ([]client.Client, error) { + qt := r.quote(clientTableName) q := fmt.Sprintf("SELECT * FROM %s", qt) - objs, err := r.executor(nil).Select(&clientIdentityModel{}, q) + objs, err := r.executor(nil).Select(&clientModel{}, q) if err != nil { return nil, err } cs := make([]client.Client, len(objs)) for i, obj := range objs { - m, ok := obj.(*clientIdentityModel) + m, ok := obj.(*clientModel) if !ok { - return nil, errors.New("unable to cast client identity to clientIdentityModel") + return nil, errors.New("unable to cast client identity to clientModel") } ci, err := m.Client() diff --git a/db/migrate_test.go b/db/migrate_test.go index e4de0534..1c81adc9 100644 --- a/db/migrate_test.go +++ b/db/migrate_test.go @@ -88,7 +88,7 @@ func TestMigrateClientMetadata(t *testing.T) { } for i, tt := range tests { - model := &clientIdentityModel{ + model := &clientModel{ ID: strconv.Itoa(i), Secret: []byte("verysecret"), Metadata: tt.before, @@ -108,12 +108,12 @@ func TestMigrateClientMetadata(t *testing.T) { for i, tt := range tests { id := strconv.Itoa(i) - m, err := dbMap.Get(clientIdentityModel{}, id) + m, err := dbMap.Get(clientModel{}, id) if err != nil { t.Errorf("case %d: failed to get model: %v", i, err) continue } - cim, ok := m.(*clientIdentityModel) + cim, ok := m.(*clientModel) if !ok { t.Errorf("case %d: unrecognized model type: %T", i, m) continue diff --git a/db/refresh.go b/db/refresh.go index 1e71c60f..8ebc9ce6 100644 --- a/db/refresh.go +++ b/db/refresh.go @@ -189,9 +189,9 @@ func (r *refreshTokenRepo) RevokeTokensForClient(userID, clientID string) error func (r *refreshTokenRepo) ClientsWithRefreshTokens(userID string) ([]client.Client, error) { q := `SELECT c.* FROM %s as c INNER JOIN %s as r ON c.id = r.client_id WHERE r.user_id = $1;` - q = fmt.Sprintf(q, r.quote(clientIdentityTableName), r.quote(refreshTokenTableName)) + q = fmt.Sprintf(q, r.quote(clientTableName), r.quote(refreshTokenTableName)) - var clients []clientIdentityModel + var clients []clientModel if _, err := r.executor(nil).Select(&clients, q, userID); err != nil { return nil, err } diff --git a/functional/db_test.go b/functional/db_test.go index 7690a5b0..a62bb446 100644 --- a/functional/db_test.go +++ b/functional/db_test.go @@ -181,8 +181,8 @@ func TestDBPrivateKeySetRepoSetGet(t *testing.T) { } } -func TestDBClientIdentityRepoMetadata(t *testing.T) { - r := db.NewClientIdentityRepo(connect(t)) +func TestDBClientRepoMetadata(t *testing.T) { + r := db.NewClientRepo(connect(t)) cm := oidc.ClientMetadata{ RedirectURIs: []url.URL{ @@ -211,8 +211,8 @@ func TestDBClientIdentityRepoMetadata(t *testing.T) { } } -func TestDBClientIdentityRepoMetadataNoExist(t *testing.T) { - r := db.NewClientIdentityRepo(connect(t)) +func TestDBClientRepoMetadataNoExist(t *testing.T) { + r := db.NewClientRepo(connect(t)) got, err := r.Metadata("noexist") if err != client.ErrorNotFound { @@ -223,8 +223,8 @@ func TestDBClientIdentityRepoMetadataNoExist(t *testing.T) { } } -func TestDBClientIdentityRepoNewDuplicate(t *testing.T) { - r := db.NewClientIdentityRepo(connect(t)) +func TestDBClientRepoNewDuplicate(t *testing.T) { + r := db.NewClientRepo(connect(t)) meta1 := oidc.ClientMetadata{ RedirectURIs: []url.URL{ @@ -257,8 +257,8 @@ func TestDBClientIdentityRepoNewDuplicate(t *testing.T) { } } -func TestDBClientIdentityRepoAuthenticate(t *testing.T) { - r := db.NewClientIdentityRepo(connect(t)) +func TestDBClientRepoAuthenticate(t *testing.T) { + r := db.NewClientRepo(connect(t)) cm := oidc.ClientMetadata{ RedirectURIs: []url.URL{ @@ -310,8 +310,8 @@ func TestDBClientIdentityRepoAuthenticate(t *testing.T) { } } -func TestDBClientIdentityAll(t *testing.T) { - r := db.NewClientIdentityRepo(connect(t)) +func TestDBClientAll(t *testing.T) { + r := db.NewClientRepo(connect(t)) cm := oidc.ClientMetadata{ RedirectURIs: []url.URL{ diff --git a/functional/repo/client_repo_test.go b/functional/repo/client_repo_test.go index 62097b3e..413562dd 100644 --- a/functional/repo/client_repo_test.go +++ b/functional/repo/client_repo_test.go @@ -48,7 +48,7 @@ var ( } ) -func newClientIdentityRepo(t *testing.T) client.ClientIdentityRepo { +func newClientRepo(t *testing.T) client.ClientRepo { dsn := os.Getenv("DEX_TEST_DSN") var dbMap *gorp.DbMap if dsn == "" { @@ -56,7 +56,7 @@ func newClientIdentityRepo(t *testing.T) client.ClientIdentityRepo { } else { dbMap = connect(t) } - repo, err := db.NewClientIdentityRepoFromClients(dbMap, testClients) + repo, err := db.NewClientRepoFromClients(dbMap, testClients) if err != nil { t.Fatalf("failed to create client repo from clients: %v", err) } @@ -101,7 +101,7 @@ func TestGetSetAdminClient(t *testing.T) { Tests: for i, tt := range tests { - repo := newClientIdentityRepo(t) + repo := newClientRepo(t) for _, cid := range startAdmins { err := repo.SetDexAdmin(cid, true) if err != nil { diff --git a/functional/repo/refresh_repo_test.go b/functional/repo/refresh_repo_test.go index 40862663..e723d7fe 100644 --- a/functional/repo/refresh_repo_test.go +++ b/functional/repo/refresh_repo_test.go @@ -27,7 +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 := db.NewClientIdentityRepoFromClients(dbMap, clients); err != nil { + if _, err := db.NewClientRepoFromClients(dbMap, clients); err != nil { t.Fatalf("Unable to add clients: %v", err) } return db.NewRefreshTokenRepo(dbMap) diff --git a/integration/client_api_test.go b/integration/client_api_test.go index fd3e63bb..d19b18a4 100644 --- a/integration/client_api_test.go +++ b/integration/client_api_test.go @@ -73,7 +73,7 @@ func TestClientCreate(t *testing.T) { t.Error("Expected non-empty Client Secret") } - meta, err := srv.ClientIdentityRepo.Metadata(newClient.Id) + meta, err := srv.ClientRepo.Metadata(newClient.Id) if err != nil { t.Errorf("Error looking up client metadata: %v", err) } else if meta == nil { diff --git a/integration/oidc_test.go b/integration/oidc_test.go index 1195e118..ca8f407c 100644 --- a/integration/oidc_test.go +++ b/integration/oidc_test.go @@ -35,17 +35,17 @@ func mockServer(cis []client.Client) (*server.Server, error) { if err != nil { return nil, err } - clientIdentityRepo, err := db.NewClientIdentityRepoFromClients(dbMap, cis) + clientRepo, err := db.NewClientRepoFromClients(dbMap, cis) if err != nil { return nil, err } sm := manager.NewSessionManager(db.NewSessionRepo(dbMap), db.NewSessionKeyRepo(dbMap)) srv := &server.Server{ - IssuerURL: url.URL{Scheme: "http", Host: "server.example.com"}, - KeyManager: km, - ClientIdentityRepo: clientIdentityRepo, - SessionManager: sm, + IssuerURL: url.URL{Scheme: "http", Host: "server.example.com"}, + KeyManager: km, + ClientRepo: clientRepo, + SessionManager: sm, } return srv, nil @@ -125,7 +125,7 @@ func TestHTTPExchangeTokenRefreshToken(t *testing.T) { } dbMap := db.NewMemDB() - cir, err := db.NewClientIdentityRepoFromClients(dbMap, []client.Client{ci}) + cir, err := db.NewClientRepoFromClients(dbMap, []client.Client{ci}) if err != nil { t.Fatalf("Failed to create client identity repo: " + err.Error()) } @@ -161,15 +161,15 @@ func TestHTTPExchangeTokenRefreshToken(t *testing.T) { refreshTokenRepo := refreshtest.NewTestRefreshTokenRepo() srv := &server.Server{ - IssuerURL: issuerURL, - KeyManager: km, - SessionManager: sm, - ClientIdentityRepo: cir, - Templates: template.New(connector.LoginPageTemplateName), - Connectors: []connector.Connector{}, - UserRepo: userRepo, - PasswordInfoRepo: passwordInfoRepo, - RefreshTokenRepo: refreshTokenRepo, + IssuerURL: issuerURL, + KeyManager: km, + SessionManager: sm, + ClientRepo: cir, + Templates: template.New(connector.LoginPageTemplateName), + Connectors: []connector.Connector{}, + UserRepo: userRepo, + PasswordInfoRepo: passwordInfoRepo, + RefreshTokenRepo: refreshTokenRepo, } if err = srv.AddConnector(cfg); err != nil { diff --git a/integration/user_api_test.go b/integration/user_api_test.go index 6795f73d..35022243 100644 --- a/integration/user_api_test.go +++ b/integration/user_api_test.go @@ -101,8 +101,8 @@ func makeUserAPITestFixtures() *userAPITestFixtures { f := &userAPITestFixtures{} dbMap, _, _, um := makeUserObjects(userUsers, userPasswords) - cir := func() client.ClientIdentityRepo { - repo, err := db.NewClientIdentityRepoFromClients(dbMap, []client.Client{ + cir := func() client.ClientRepo { + repo, err := db.NewClientRepoFromClients(dbMap, []client.Client{ client.Client{ Credentials: oidc.ClientCredentials{ ID: testClientID, diff --git a/schema/adminschema/v1-json.go b/schema/adminschema/v1-json.go index 64c29f61..5d943e99 100644 --- a/schema/adminschema/v1-json.go +++ b/schema/adminschema/v1-json.go @@ -1,4 +1,5 @@ package adminschema + // // This file is automatically generated by schema/generator // @@ -180,4 +181,4 @@ const DiscoveryJSON = `{ } } } -` \ No newline at end of file +` diff --git a/schema/workerschema/v1-json.go b/schema/workerschema/v1-json.go index 9e1eae21..06a5a6bf 100644 --- a/schema/workerschema/v1-json.go +++ b/schema/workerschema/v1-json.go @@ -1,4 +1,5 @@ package workerschema + // // This file is automatically generated by schema/generator // @@ -419,4 +420,4 @@ const DiscoveryJSON = `{ } } } -` \ No newline at end of file +` diff --git a/server/auth_middleware.go b/server/auth_middleware.go index 225b8b72..7c6fc789 100644 --- a/server/auth_middleware.go +++ b/server/auth_middleware.go @@ -14,7 +14,7 @@ import ( type clientTokenMiddleware struct { issuerURL string - ciRepo client.ClientIdentityRepo + ciRepo client.ClientRepo keysFunc func() ([]key.PublicKey, error) next http.Handler } @@ -31,7 +31,7 @@ func (c *clientTokenMiddleware) ServeHTTP(w http.ResponseWriter, r *http.Request } if c.ciRepo == nil { - log.Errorf("Misconfigured clientTokenMiddleware, ClientIdentityRepo is not set") + log.Errorf("Misconfigured clientTokenMiddleware, ClientRepo is not set") respondError() return } diff --git a/server/auth_middleware_test.go b/server/auth_middleware_test.go index 3dfb1695..06932eab 100644 --- a/server/auth_middleware_test.go +++ b/server/auth_middleware_test.go @@ -37,7 +37,7 @@ func TestClientToken(t *testing.T) { }, }, } - repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ci}) + repo, err := db.NewClientRepoFromClients(db.NewMemDB(), []client.Client{ci}) if err != nil { t.Fatalf("Failed to create client identity repo: %v", err) } @@ -65,7 +65,7 @@ func TestClientToken(t *testing.T) { tests := []struct { keys []key.PublicKey - repo client.ClientIdentityRepo + repo client.ClientRepo header string wantCode int }{ @@ -114,7 +114,7 @@ func TestClientToken(t *testing.T) { // empty repo { keys: []key.PublicKey{pubKey}, - repo: db.NewClientIdentityRepo(db.NewMemDB()), + repo: db.NewClientRepo(db.NewMemDB()), header: fmt.Sprintf("BEARER %s", validJWT), wantCode: http.StatusUnauthorized, }, diff --git a/server/client_registration.go b/server/client_registration.go index f1c21e99..ca0a23fb 100644 --- a/server/client_registration.go +++ b/server/client_registration.go @@ -45,7 +45,7 @@ func (s *Server) handleClientRegistrationRequest(r *http.Request) (*oidc.ClientR return nil, newAPIError(oauth2.ErrorServerError, "unable to save client metadata") } - creds, err := s.ClientIdentityRepo.New(client.Client{ + creds, err := s.ClientRepo.New(client.Client{ Credentials: oidc.ClientCredentials{ ID: id, }, diff --git a/server/client_registration_test.go b/server/client_registration_test.go index 2dc647cc..ef8a19e8 100644 --- a/server/client_registration_test.go +++ b/server/client_registration_test.go @@ -143,7 +143,7 @@ func TestClientRegistration(t *testing.T) { return fmt.Errorf("no client id in registration response") } - metadata, err := fixtures.clientIdentityRepo.Metadata(r.ClientID) + metadata, err := fixtures.clientRepo.Metadata(r.ClientID) if err != nil { return fmt.Errorf("failed to lookup client id after creation") } diff --git a/server/client_resource.go b/server/client_resource.go index c69e9062..ea348668 100644 --- a/server/client_resource.go +++ b/server/client_resource.go @@ -14,10 +14,10 @@ import ( ) type clientResource struct { - repo client.ClientIdentityRepo + repo client.ClientRepo } -func registerClientResource(prefix string, repo client.ClientIdentityRepo) (string, http.Handler) { +func registerClientResource(prefix string, repo client.ClientRepo) (string, http.Handler) { mux := http.NewServeMux() c := &clientResource{ repo: repo, diff --git a/server/client_resource_test.go b/server/client_resource_test.go index ceb4232e..63158fd1 100644 --- a/server/client_resource_test.go +++ b/server/client_resource_test.go @@ -28,7 +28,7 @@ func makeBody(s string) io.ReadCloser { func TestCreateInvalidRequest(t *testing.T) { u := &url.URL{Scheme: "http", Host: "example.com", Path: "clients"} h := http.Header{"Content-Type": []string{"application/json"}} - repo := db.NewClientIdentityRepo(db.NewMemDB()) + repo := db.NewClientRepo(db.NewMemDB()) res := &clientResource{repo: repo} tests := []struct { req *http.Request @@ -119,7 +119,7 @@ func TestCreateInvalidRequest(t *testing.T) { } func TestCreate(t *testing.T) { - repo := db.NewClientIdentityRepo(db.NewMemDB()) + repo := db.NewClientRepo(db.NewMemDB()) res := &clientResource{repo: repo} tests := [][]string{ []string{"http://example.com"}, @@ -239,7 +239,7 @@ func TestList(t *testing.T) { } for i, tt := range tests { - repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), tt.cs) + repo, err := db.NewClientRepoFromClients(db.NewMemDB(), tt.cs) if err != nil { t.Errorf("case %d: failed to create client identity repo: %v", i, err) continue diff --git a/server/config.go b/server/config.go index bb505510..5a28db5a 100644 --- a/server/config.go +++ b/server/config.go @@ -114,7 +114,7 @@ func (cfg *SingleServerConfig) Configure(srv *Server) error { if err != nil { return fmt.Errorf("unable to read clients from file %s: %v", cfg.ClientsFile, err) } - ciRepo, err := db.NewClientIdentityRepoFromClients(dbMap, clients) + ciRepo, err := db.NewClientRepoFromClients(dbMap, clients) if err != nil { return fmt.Errorf("failed to create client identity repo: %v", err) } @@ -155,7 +155,7 @@ func (cfg *SingleServerConfig) Configure(srv *Server) error { txnFactory := db.TransactionFactory(dbMap) userManager := usermanager.NewUserManager(userRepo, pwiRepo, cfgRepo, txnFactory, usermanager.ManagerOptions{}) - srv.ClientIdentityRepo = ciRepo + srv.ClientRepo = ciRepo srv.KeySetRepo = kRepo srv.ConnectorConfigRepo = cfgRepo srv.UserRepo = userRepo @@ -246,7 +246,7 @@ func (cfg *MultiServerConfig) Configure(srv *Server) error { return fmt.Errorf("unable to create PrivateKeySetRepo: %v", err) } - ciRepo := db.NewClientIdentityRepo(dbc) + ciRepo := db.NewClientRepo(dbc) sRepo := db.NewSessionRepo(dbc) skRepo := db.NewSessionKeyRepo(dbc) cfgRepo := db.NewConnectorConfigRepo(dbc) @@ -257,7 +257,7 @@ func (cfg *MultiServerConfig) Configure(srv *Server) error { sm := sessionmanager.NewSessionManager(sRepo, skRepo) - srv.ClientIdentityRepo = ciRepo + srv.ClientRepo = ciRepo srv.KeySetRepo = kRepo srv.ConnectorConfigRepo = cfgRepo srv.UserRepo = userRepo diff --git a/server/email_verification.go b/server/email_verification.go index 64fd3689..f38a9821 100644 --- a/server/email_verification.go +++ b/server/email_verification.go @@ -28,7 +28,7 @@ func handleVerifyEmailResendFunc( srvKeysFunc func() ([]key.PublicKey, error), emailer *useremail.UserEmailer, userRepo user.UserRepo, - clientIdentityRepo client.ClientIdentityRepo) http.HandlerFunc { + clientRepo client.ClientRepo) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { decoder := json.NewDecoder(r.Body) var params struct { @@ -57,7 +57,7 @@ func handleVerifyEmailResendFunc( return } - cm, err := clientIdentityRepo.Metadata(clientID) + cm, err := clientRepo.Metadata(clientID) if err == client.ErrorNotFound { log.Errorf("No such client: %v", err) writeAPIError(w, http.StatusBadRequest, diff --git a/server/email_verification_test.go b/server/email_verification_test.go index 3b0494a9..b444f21e 100644 --- a/server/email_verification_test.go +++ b/server/email_verification_test.go @@ -130,7 +130,7 @@ func TestHandleVerifyEmailResend(t *testing.T) { keysFunc, f.srv.UserEmailer, f.userRepo, - f.clientIdentityRepo) + f.clientRepo) w := httptest.NewRecorder() u := "http://example.com" diff --git a/server/http_test.go b/server/http_test.go index c96a6bbb..0399d95f 100644 --- a/server/http_test.go +++ b/server/http_test.go @@ -78,8 +78,8 @@ func TestHandleAuthFuncResponsesSingleRedirectURL(t *testing.T) { srv := &Server{ IssuerURL: url.URL{Scheme: "http", Host: "server.example.com"}, SessionManager: manager.NewSessionManager(db.NewSessionRepo(db.NewMemDB()), db.NewSessionKeyRepo(db.NewMemDB())), - ClientIdentityRepo: func() client.ClientIdentityRepo { - repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ + ClientRepo: func() client.ClientRepo { + repo, err := db.NewClientRepoFromClients(db.NewMemDB(), []client.Client{ client.Client{ Credentials: oidc.ClientCredentials{ ID: "XXX", @@ -230,8 +230,8 @@ func TestHandleAuthFuncResponsesMultipleRedirectURLs(t *testing.T) { srv := &Server{ IssuerURL: url.URL{Scheme: "http", Host: "server.example.com"}, SessionManager: manager.NewSessionManager(db.NewSessionRepo(db.NewMemDB()), db.NewSessionKeyRepo(db.NewMemDB())), - ClientIdentityRepo: func() client.ClientIdentityRepo { - repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ + ClientRepo: func() client.ClientRepo { + repo, err := db.NewClientRepoFromClients(db.NewMemDB(), []client.Client{ client.Client{ Credentials: oidc.ClientCredentials{ ID: "XXX", diff --git a/server/password.go b/server/password.go index 448dfb59..077a7a06 100644 --- a/server/password.go +++ b/server/password.go @@ -29,7 +29,7 @@ type SendResetPasswordEmailHandler struct { tpl *template.Template emailer *useremail.UserEmailer sm *sessionmanager.SessionManager - cr client.ClientIdentityRepo + cr client.ClientRepo } func (h *SendResetPasswordEmailHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { diff --git a/server/password_test.go b/server/password_test.go index a5f7e6d3..477faa2d 100644 --- a/server/password_test.go +++ b/server/password_test.go @@ -267,7 +267,7 @@ func TestSendResetPasswordEmailHandler(t *testing.T) { tpl: f.srv.SendResetPasswordEmailTemplate, emailer: f.srv.UserEmailer, sm: f.sessionManager, - cr: f.clientIdentityRepo, + cr: f.clientRepo, } w := httptest.NewRecorder() diff --git a/server/server.go b/server/server.go index afc2a2e5..1308c26b 100644 --- a/server/server.go +++ b/server/server.go @@ -60,7 +60,7 @@ type Server struct { KeyManager key.PrivateKeyManager KeySetRepo key.PrivateKeySetRepo SessionManager *sessionmanager.SessionManager - ClientIdentityRepo client.ClientIdentityRepo + ClientRepo client.ClientRepo ConnectorConfigRepo connector.ConnectorConfigRepo Templates *template.Template LoginTemplate *template.Template @@ -213,13 +213,13 @@ func (s *Server) HTTPHandler() http.Handler { s.KeyManager.PublicKeys, s.UserEmailer, s.UserRepo, - s.ClientIdentityRepo))) + s.ClientRepo))) mux.Handle(httpPathSendResetPassword, &SendResetPasswordEmailHandler{ tpl: s.SendResetPasswordEmailTemplate, emailer: s.UserEmailer, sm: s.SessionManager, - cr: s.ClientIdentityRepo, + cr: s.ClientRepo, }) mux.Handle(httpPathResetPassword, &ResetPasswordHandler{ @@ -256,11 +256,11 @@ func (s *Server) HTTPHandler() http.Handler { apiBasePath := path.Join(httpPathAPI, APIVersion) registerDiscoveryResource(apiBasePath, mux) - clientPath, clientHandler := registerClientResource(apiBasePath, s.ClientIdentityRepo) + clientPath, clientHandler := registerClientResource(apiBasePath, s.ClientRepo) mux.Handle(path.Join(apiBasePath, clientPath), s.NewClientTokenAuthHandler(clientHandler)) usersAPI := usersapi.NewUsersAPI(s.dbMap, s.UserManager, s.UserEmailer, s.localConnectorID) - handler := NewUserMgmtServer(usersAPI, s.JWTVerifierFactory(), s.UserManager, s.ClientIdentityRepo).HTTPHandler() + handler := NewUserMgmtServer(usersAPI, s.JWTVerifierFactory(), s.UserManager, s.ClientRepo).HTTPHandler() mux.Handle(apiBasePath+"/", handler) @@ -271,14 +271,14 @@ func (s *Server) HTTPHandler() http.Handler { func (s *Server) NewClientTokenAuthHandler(handler http.Handler) http.Handler { return &clientTokenMiddleware{ issuerURL: s.IssuerURL.String(), - ciRepo: s.ClientIdentityRepo, + ciRepo: s.ClientRepo, keysFunc: s.KeyManager.PublicKeys, next: handler, } } func (s *Server) ClientMetadata(clientID string) (*oidc.ClientMetadata, error) { - return s.ClientIdentityRepo.Metadata(clientID) + return s.ClientRepo.Metadata(clientID) } func (s *Server) NewSession(ipdcID, clientID, clientState string, redirectURL url.URL, nonce string, register bool, scope []string) (string, error) { @@ -365,7 +365,7 @@ func (s *Server) Login(ident oidc.Identity, key string) (string, error) { } func (s *Server) ClientCredsToken(creds oidc.ClientCredentials) (*jose.JWT, error) { - ok, err := s.ClientIdentityRepo.Authenticate(creds) + ok, err := s.ClientRepo.Authenticate(creds) if err != nil { log.Errorf("Failed fetching client %s from repo: %v", creds.ID, err) return nil, oauth2.NewError(oauth2.ErrorServerError) @@ -397,7 +397,7 @@ func (s *Server) ClientCredsToken(creds oidc.ClientCredentials) (*jose.JWT, erro } func (s *Server) CodeToken(creds oidc.ClientCredentials, sessionKey string) (*jose.JWT, string, error) { - ok, err := s.ClientIdentityRepo.Authenticate(creds) + ok, err := s.ClientRepo.Authenticate(creds) if err != nil { log.Errorf("Failed fetching client %s from repo: %v", creds.ID, err) return nil, "", oauth2.NewError(oauth2.ErrorServerError) @@ -466,7 +466,7 @@ func (s *Server) CodeToken(creds oidc.ClientCredentials, sessionKey string) (*jo } func (s *Server) RefreshToken(creds oidc.ClientCredentials, token string) (*jose.JWT, error) { - ok, err := s.ClientIdentityRepo.Authenticate(creds) + ok, err := s.ClientRepo.Authenticate(creds) if err != nil { log.Errorf("Failed fetching client %s from repo: %v", creds.ID, err) return nil, oauth2.NewError(oauth2.ErrorServerError) diff --git a/server/server_test.go b/server/server_test.go index 17a2e814..6177b272 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -194,8 +194,8 @@ func TestServerLogin(t *testing.T) { }, }, } - ciRepo := func() client.ClientIdentityRepo { - repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ci}) + ciRepo := func() client.ClientRepo { + repo, err := db.NewClientRepoFromClients(db.NewMemDB(), []client.Client{ci}) if err != nil { t.Fatalf("Failed to create client identity repo: %v", err) } @@ -219,11 +219,11 @@ func TestServerLogin(t *testing.T) { } srv := &Server{ - IssuerURL: url.URL{Scheme: "http", Host: "server.example.com"}, - KeyManager: km, - SessionManager: sm, - ClientIdentityRepo: ciRepo, - UserRepo: userRepo, + IssuerURL: url.URL{Scheme: "http", Host: "server.example.com"}, + KeyManager: km, + SessionManager: sm, + ClientRepo: ciRepo, + UserRepo: userRepo, } ident := oidc.Identity{ID: "YYY", Name: "elroy", Email: "elroy@example.com"} @@ -244,8 +244,8 @@ func TestServerLogin(t *testing.T) { } func TestServerLoginUnrecognizedSessionKey(t *testing.T) { - ciRepo := func() client.ClientIdentityRepo { - repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ + ciRepo := func() client.ClientRepo { + repo, err := db.NewClientRepoFromClients(db.NewMemDB(), []client.Client{ client.Client{ Credentials: oidc.ClientCredentials{ ID: "XXX", Secret: clientTestSecret, @@ -263,10 +263,10 @@ func TestServerLoginUnrecognizedSessionKey(t *testing.T) { } 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, - ClientIdentityRepo: ciRepo, + IssuerURL: url.URL{Scheme: "http", Host: "server.example.com"}, + KeyManager: km, + SessionManager: sm, + ClientRepo: ciRepo, } ident := oidc.Identity{ID: "YYY", Name: "elroy", Email: "elroy@example.com"} @@ -296,8 +296,8 @@ func TestServerLoginDisabledUser(t *testing.T) { }, }, } - ciRepo := func() client.ClientIdentityRepo { - repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ci}) + ciRepo := func() client.ClientRepo { + repo, err := db.NewClientRepoFromClients(db.NewMemDB(), []client.Client{ci}) if err != nil { t.Fatalf("Failed to create client identity repo: %v", err) } @@ -335,11 +335,11 @@ func TestServerLoginDisabledUser(t *testing.T) { }) srv := &Server{ - IssuerURL: url.URL{Scheme: "http", Host: "server.example.com"}, - KeyManager: km, - SessionManager: sm, - ClientIdentityRepo: ciRepo, - UserRepo: userRepo, + IssuerURL: url.URL{Scheme: "http", Host: "server.example.com"}, + KeyManager: km, + SessionManager: sm, + ClientRepo: ciRepo, + UserRepo: userRepo, } ident := oidc.Identity{ID: "disabled-connector-id", Name: "elroy", Email: "elroy@example.com"} @@ -361,8 +361,8 @@ func TestServerCodeToken(t *testing.T) { Secret: clientTestSecret, }, } - ciRepo := func() client.ClientIdentityRepo { - repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ci}) + ciRepo := func() client.ClientRepo { + repo, err := db.NewClientRepoFromClients(db.NewMemDB(), []client.Client{ci}) if err != nil { t.Fatalf("Failed to create client identity repo: %v", err) } @@ -381,12 +381,12 @@ func TestServerCodeToken(t *testing.T) { refreshTokenRepo := refreshtest.NewTestRefreshTokenRepo() srv := &Server{ - IssuerURL: url.URL{Scheme: "http", Host: "server.example.com"}, - KeyManager: km, - SessionManager: sm, - ClientIdentityRepo: ciRepo, - UserRepo: userRepo, - RefreshTokenRepo: refreshTokenRepo, + IssuerURL: url.URL{Scheme: "http", Host: "server.example.com"}, + KeyManager: km, + SessionManager: sm, + ClientRepo: ciRepo, + UserRepo: userRepo, + RefreshTokenRepo: refreshTokenRepo, } tests := []struct { @@ -447,8 +447,8 @@ func TestServerTokenUnrecognizedKey(t *testing.T) { Secret: clientTestSecret, }, } - ciRepo := func() client.ClientIdentityRepo { - repo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ci}) + ciRepo := func() client.ClientRepo { + repo, err := db.NewClientRepoFromClients(db.NewMemDB(), []client.Client{ci}) if err != nil { t.Fatalf("Failed to create client identity repo: %v", err) } @@ -460,10 +460,10 @@ func TestServerTokenUnrecognizedKey(t *testing.T) { 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, - ClientIdentityRepo: ciRepo, + IssuerURL: url.URL{Scheme: "http", Host: "server.example.com"}, + KeyManager: km, + SessionManager: sm, + ClientRepo: ciRepo, } sessionID, err := sm.NewSession("connector_id", ci.Credentials.ID, "bogus", url.URL{}, "", false, []string{"openid", "offline_access"}) @@ -569,7 +569,7 @@ func TestServerTokenFail(t *testing.T) { km := &StaticKeyManager{ signer: tt.signer, } - ciRepo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ + ciRepo, err := db.NewClientRepoFromClients(db.NewMemDB(), []client.Client{ client.Client{Credentials: ccFixture}, }) if err != nil { @@ -590,12 +590,12 @@ func TestServerTokenFail(t *testing.T) { refreshTokenRepo := refreshtest.NewTestRefreshTokenRepo() srv := &Server{ - IssuerURL: issuerURL, - KeyManager: km, - SessionManager: sm, - ClientIdentityRepo: ciRepo, - UserRepo: userRepo, - RefreshTokenRepo: refreshTokenRepo, + IssuerURL: issuerURL, + KeyManager: km, + SessionManager: sm, + ClientRepo: ciRepo, + UserRepo: userRepo, + RefreshTokenRepo: refreshTokenRepo, } _, err = sm.NewSessionKey(sessionID) @@ -731,7 +731,7 @@ func TestServerRefreshToken(t *testing.T) { signer: tt.signer, } - ciRepo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ + ciRepo, err := db.NewClientRepoFromClients(db.NewMemDB(), []client.Client{ client.Client{Credentials: credXXX}, client.Client{Credentials: credYYY}, }) @@ -748,11 +748,11 @@ func TestServerRefreshToken(t *testing.T) { refreshTokenRepo := refreshtest.NewTestRefreshTokenRepo() srv := &Server{ - IssuerURL: issuerURL, - KeyManager: km, - ClientIdentityRepo: ciRepo, - UserRepo: userRepo, - RefreshTokenRepo: refreshTokenRepo, + IssuerURL: issuerURL, + KeyManager: km, + ClientRepo: ciRepo, + UserRepo: userRepo, + RefreshTokenRepo: refreshTokenRepo, } if _, err := refreshTokenRepo.Create("testid-1", tt.clientID); err != nil { @@ -784,7 +784,7 @@ func TestServerRefreshToken(t *testing.T) { signer: signerFixture, } - ciRepo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ + ciRepo, err := db.NewClientRepoFromClients(db.NewMemDB(), []client.Client{ client.Client{Credentials: credXXX}, client.Client{Credentials: credYYY}, }) @@ -808,11 +808,11 @@ func TestServerRefreshToken(t *testing.T) { refreshTokenRepo := refreshtest.NewTestRefreshTokenRepo() srv := &Server{ - IssuerURL: issuerURL, - KeyManager: km, - ClientIdentityRepo: ciRepo, - UserRepo: userRepo, - RefreshTokenRepo: refreshTokenRepo, + IssuerURL: issuerURL, + KeyManager: km, + ClientRepo: ciRepo, + UserRepo: userRepo, + RefreshTokenRepo: refreshTokenRepo, } if _, err := refreshTokenRepo.Create("testid-2", credXXX.ID); err != nil { diff --git a/server/testutil.go b/server/testutil.go index 71cd05a2..5ed95a61 100644 --- a/server/testutil.go +++ b/server/testutil.go @@ -73,12 +73,12 @@ var ( ) type testFixtures struct { - srv *Server - userRepo user.UserRepo - sessionManager *sessionmanager.SessionManager - emailer *email.TemplatizedEmailer - redirectURL url.URL - clientIdentityRepo client.ClientIdentityRepo + srv *Server + userRepo user.UserRepo + sessionManager *sessionmanager.SessionManager + emailer *email.TemplatizedEmailer + redirectURL url.URL + clientRepo client.ClientRepo } func sequentialGenerateCodeFunc() sessionmanager.GenerateCodeFunc { @@ -136,7 +136,7 @@ func makeTestFixtures() (*testFixtures, error) { return nil, err } - clientIdentityRepo, err := db.NewClientIdentityRepoFromClients(db.NewMemDB(), []client.Client{ + clientRepo, err := db.NewClientRepoFromClients(db.NewMemDB(), []client.Client{ client.Client{ Credentials: oidc.ClientCredentials{ ID: "XXX", @@ -167,14 +167,14 @@ func makeTestFixtures() (*testFixtures, error) { } srv := &Server{ - IssuerURL: testIssuerURL, - SessionManager: sessionManager, - ClientIdentityRepo: clientIdentityRepo, - Templates: tpl, - UserRepo: userRepo, - PasswordInfoRepo: pwRepo, - UserManager: manager, - KeyManager: km, + IssuerURL: testIssuerURL, + SessionManager: sessionManager, + ClientRepo: clientRepo, + Templates: tpl, + UserRepo: userRepo, + PasswordInfoRepo: pwRepo, + UserManager: manager, + KeyManager: km, } err = setTemplates(srv, tpl) @@ -201,11 +201,11 @@ func makeTestFixtures() (*testFixtures, error) { ) return &testFixtures{ - srv: srv, - redirectURL: testRedirectURL, - userRepo: userRepo, - sessionManager: sessionManager, - emailer: emailer, - clientIdentityRepo: clientIdentityRepo, + srv: srv, + redirectURL: testRedirectURL, + userRepo: userRepo, + sessionManager: sessionManager, + emailer: emailer, + clientRepo: clientRepo, }, nil } diff --git a/server/user.go b/server/user.go index d77fe086..1b03d480 100644 --- a/server/user.go +++ b/server/user.go @@ -39,10 +39,10 @@ type UserMgmtServer struct { api *api.UsersAPI jwtvFactory JWTVerifierFactory um *manager.UserManager - cir client.ClientIdentityRepo + cir client.ClientRepo } -func NewUserMgmtServer(userMgmtAPI *api.UsersAPI, jwtvFactory JWTVerifierFactory, um *manager.UserManager, cir client.ClientIdentityRepo) *UserMgmtServer { +func NewUserMgmtServer(userMgmtAPI *api.UsersAPI, jwtvFactory JWTVerifierFactory, um *manager.UserManager, cir client.ClientRepo) *UserMgmtServer { return &UserMgmtServer{ api: userMgmtAPI, jwtvFactory: jwtvFactory, diff --git a/user/api/api.go b/user/api/api.go index 35e0e907..5d246b6d 100644 --- a/user/api/api.go +++ b/user/api/api.go @@ -88,11 +88,11 @@ func (e Error) Error() string { // calling User. It is assumed that the clientID has already validated as an // admin app before calling. type UsersAPI struct { - manager *manager.UserManager - localConnectorID string - clientIdentityRepo client.ClientIdentityRepo - refreshRepo refresh.RefreshTokenRepo - emailer Emailer + manager *manager.UserManager + localConnectorID string + clientRepo client.ClientRepo + refreshRepo refresh.RefreshTokenRepo + emailer Emailer } type Emailer interface { @@ -107,11 +107,11 @@ type Creds struct { // TODO(ericchiang): Don't pass a dbMap. See #385. func NewUsersAPI(dbMap *gorp.DbMap, userManager *manager.UserManager, emailer Emailer, localConnectorID string) *UsersAPI { return &UsersAPI{ - manager: userManager, - refreshRepo: db.NewRefreshTokenRepo(dbMap), - clientIdentityRepo: db.NewClientIdentityRepo(dbMap), - localConnectorID: localConnectorID, - emailer: emailer, + manager: userManager, + refreshRepo: db.NewRefreshTokenRepo(dbMap), + clientRepo: db.NewClientRepo(dbMap), + localConnectorID: localConnectorID, + emailer: emailer, } } @@ -157,7 +157,7 @@ func (u *UsersAPI) CreateUser(creds Creds, usr schema.User, redirURL url.URL) (s return schema.UserCreateResponse{}, mapError(err) } - metadata, err := u.clientIdentityRepo.Metadata(creds.ClientID) + metadata, err := u.clientRepo.Metadata(creds.ClientID) if err != nil { return schema.UserCreateResponse{}, mapError(err) } @@ -202,7 +202,7 @@ func (u *UsersAPI) ResendEmailInvitation(creds Creds, userID string, redirURL ur return schema.ResendEmailInvitationResponse{}, ErrorUnauthorized } - metadata, err := u.clientIdentityRepo.Metadata(creds.ClientID) + metadata, err := u.clientRepo.Metadata(creds.ClientID) if err != nil { return schema.ResendEmailInvitationResponse{}, mapError(err) } diff --git a/user/api/api_test.go b/user/api/api_test.go index 66d82552..607fa4b1 100644 --- a/user/api/api_test.go +++ b/user/api/api_test.go @@ -167,8 +167,7 @@ func makeTestFixtures() (*UsersAPI, *testEmailer) { }, }, } - - if _, err := db.NewClientIdentityRepoFromClients(dbMap, []client.Client{ci}); err != nil { + if _, err := db.NewClientRepoFromClients(dbMap, []client.Client{ci}); err != nil { panic("Failed to create client repo: " + err.Error()) } From 158bfa5ed75572d577267a6e061528be9c636d1c Mon Sep 17 00:00:00 2001 From: Bobby Rullo Date: Fri, 15 Apr 2016 11:23:48 -0700 Subject: [PATCH 5/9] client: Add tests for ClientsFromReader Also require client ID and secret. --- client/client.go | 6 ++ client/client_test.go | 149 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+) create mode 100644 client/client_test.go diff --git a/client/client.go b/client/client.go index 29e62943..f0b25b65 100644 --- a/client/client.go +++ b/client/client.go @@ -86,6 +86,12 @@ func ClientsFromReader(r io.Reader) ([]Client, error) { } clients := make([]Client, len(c)) for i, client := range c { + if client.ID == "" { + return nil, errors.New("clients must have an ID") + } + if len(client.Secret) == 0 { + return nil, errors.New("clients must have a Secret") + } redirectURIs := make([]url.URL, len(client.RedirectURLs)) for j, u := range client.RedirectURLs { uri, err := url.Parse(u) diff --git a/client/client_test.go b/client/client_test.go new file mode 100644 index 00000000..9ec43e5e --- /dev/null +++ b/client/client_test.go @@ -0,0 +1,149 @@ +package client + +import ( + "encoding/base64" + "net/url" + "strings" + "testing" + + "github.com/coreos/go-oidc/oidc" + "github.com/kylelemons/godebug/pretty" +) + +var ( + goodSecret1 = base64.URLEncoding.EncodeToString([]byte("my_secret")) + goodSecret2 = base64.URLEncoding.EncodeToString([]byte("my_other_secret")) + + goodClient1 = `{ + "id": "my_id", + "secret": "` + goodSecret1 + `", + "redirectURLs": ["https://client.example.com"] +}` + + goodClient2 = `{ + "id": "my_other_id", + "secret": "` + goodSecret2 + `", + "redirectURLs": ["https://client2.example.com","https://client2_a.example.com"] +}` + + badURLClient = `{ + "id": "my_id", + "secret": "` + goodSecret1 + `", + "redirectURLs": ["hdtp:/\(bad)(u)(r)(l)"] +}` + + badSecretClient = `{ + "id": "my_id", + "secret": "` + "****" + `", + "redirectURLs": ["https://client.example.com"] +}` + + noSecretClient = `{ + "id": "my_id", + "redirectURLs": ["https://client.example.com"] +}` + noIDClient = `{ + "secret": "` + goodSecret1 + `", + "redirectURLs": ["https://client.example.com"] +}` +) + +func TestClientsFromReader(t *testing.T) { + tests := []struct { + json string + want []Client + wantErr bool + }{ + { + json: "[]", + want: []Client{}, + }, + { + json: "[" + goodClient1 + "]", + want: []Client{ + { + Credentials: oidc.ClientCredentials{ + ID: "my_id", + Secret: "my_secret", + }, + Metadata: oidc.ClientMetadata{ + RedirectURIs: []url.URL{ + mustParseURL(t, "https://client.example.com"), + }, + }, + }, + }, + }, + { + json: "[" + strings.Join([]string{goodClient1, goodClient2}, ",") + "]", + want: []Client{ + { + Credentials: oidc.ClientCredentials{ + ID: "my_id", + Secret: "my_secret", + }, + Metadata: oidc.ClientMetadata{ + RedirectURIs: []url.URL{ + mustParseURL(t, "https://client.example.com"), + }, + }, + }, + { + Credentials: oidc.ClientCredentials{ + ID: "my_other_id", + Secret: "my_other_secret", + }, + Metadata: oidc.ClientMetadata{ + RedirectURIs: []url.URL{ + mustParseURL(t, "https://client2.example.com"), + mustParseURL(t, "https://client2_a.example.com"), + }, + }, + }, + }, + }, { + json: "[" + badURLClient + "]", + wantErr: true, + }, + { + json: "[" + badSecretClient + "]", + wantErr: true, + }, + { + json: "[" + noSecretClient + "]", + wantErr: true, + }, + { + json: "[" + noIDClient + "]", + wantErr: true, + }, + } + + for i, tt := range tests { + r := strings.NewReader(tt.json) + cs, err := ClientsFromReader(r) + if tt.wantErr { + if err == nil { + t.Errorf("case %d: want non-nil err", i) + t.Logf(pretty.Sprint(cs)) + } + continue + } + if err != nil { + t.Errorf("case %d: got unexpected error parsing clients: %v", i, err) + t.Logf(tt.json) + } + + if diff := pretty.Compare(tt.want, cs); diff != "" { + t.Errorf("case %d: Compare(want, got): %v", i, diff) + } + } +} + +func mustParseURL(t *testing.T, s string) url.URL { + u, err := url.Parse(s) + if err != nil { + t.Fatalf("Cannot parse %v as url: %v", s, err) + } + return *u +} From e7141336bc952e35c45ec13831fc571bfc355297 Mon Sep 17 00:00:00 2001 From: Bobby Rullo Date: Mon, 18 Apr 2016 16:52:40 -0700 Subject: [PATCH 6/9] db: Client() should not return the secret It's never used by downstream code, and besides, it's not really the secret but a Hash of the secret. --- db/client.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/db/client.go b/db/client.go index 48960317..84496b4b 100644 --- a/db/client.go +++ b/db/client.go @@ -78,8 +78,7 @@ type clientModel struct { func (m *clientModel) Client() (*client.Client, error) { ci := client.Client{ Credentials: oidc.ClientCredentials{ - ID: m.ID, - Secret: string(m.Secret), + ID: m.ID, }, Admin: m.DexAdmin, } From 3442a5af1c6ddfead0472f500d3e0f98db241ea1 Mon Sep 17 00:00:00 2001 From: Bobby Rullo Date: Mon, 18 Apr 2016 17:08:07 -0700 Subject: [PATCH 7/9] functional: test Admin field serialization --- functional/db_test.go | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/functional/db_test.go b/functional/db_test.go index a62bb446..ecbff9ae 100644 --- a/functional/db_test.go +++ b/functional/db_test.go @@ -257,6 +257,42 @@ func TestDBClientRepoNewDuplicate(t *testing.T) { } } +func TestDBClientRepoNewAdmin(t *testing.T) { + + for _, admin := range []bool{true, false} { + r := db.NewClientRepo(connect(t)) + if _, err := r.New(client.Client{ + Credentials: oidc.ClientCredentials{ + ID: "foo", + }, + Metadata: oidc.ClientMetadata{ + RedirectURIs: []url.URL{ + url.URL{Scheme: "http", Host: "foo.example.com"}, + }, + }, + Admin: admin, + }); err != nil { + t.Fatalf("expected non-nil error: %v", err) + } + + gotAdmin, err := r.IsDexAdmin("foo") + if err != nil { + t.Fatalf("expected non-nil error") + } + if gotAdmin != admin { + t.Errorf("want=%v, gotAdmin=%v", admin, gotAdmin) + } + + cli, err := r.Get("foo") + if err != nil { + t.Fatalf("expected non-nil error") + } + if cli.Admin != admin { + t.Errorf("want=%v, cli.Admin=%v", admin, cli.Admin) + } + } + +} func TestDBClientRepoAuthenticate(t *testing.T) { r := db.NewClientRepo(connect(t)) From 399b15abeb2021539a4893ca062c4d0390868b54 Mon Sep 17 00:00:00 2001 From: Bobby Rullo Date: Tue, 19 Apr 2016 18:27:45 -0700 Subject: [PATCH 8/9] integration, *: Improve tests for admin api * TestCreateClient was missing test coverage on error cases * Fixed bug where 500s were being reported for bad requests * changed function signature of NewAdminAPI back to old way of passing in lots of repos: passing in a DbMap made it difficult to test * added swappable ID and Secret generators when creating Clients --- admin/api.go | 40 +++++--- admin/api_test.go | 4 +- cmd/dex-overlord/main.go | 3 +- db/client.go | 21 ++++- integration/admin_api_test.go | 171 ++++++++++++++++++++++++++++------ schema/adminschema/mapper.go | 18 +++- 6 files changed, 203 insertions(+), 54 deletions(-) diff --git a/admin/api.go b/admin/api.go index 5a821a9d..965e9ce2 100644 --- a/admin/api.go +++ b/admin/api.go @@ -5,15 +5,17 @@ import ( "net/http" "github.com/coreos/go-oidc/oidc" - "github.com/go-gorp/gorp" "github.com/coreos/dex/client" - "github.com/coreos/dex/db" "github.com/coreos/dex/schema/adminschema" "github.com/coreos/dex/user" "github.com/coreos/dex/user/manager" ) +var ( + ClientIDGenerator = oidc.GenClientID +) + // AdminAPI provides the logic necessary to implement the Admin API. type AdminAPI struct { userManager *manager.UserManager @@ -23,17 +25,15 @@ type AdminAPI struct { localConnectorID string } -// TODO(ericchiang): Swap the DbMap for a storage interface. See #278 - -func NewAdminAPI(dbMap *gorp.DbMap, userManager *manager.UserManager, localConnectorID string) *AdminAPI { +func NewAdminAPI(userRepo user.UserRepo, pwiRepo user.PasswordInfoRepo, clientRepo client.ClientRepo, userManager *manager.UserManager, localConnectorID string) *AdminAPI { if localConnectorID == "" { panic("must specify non-blank localConnectorID") } return &AdminAPI{ userManager: userManager, - userRepo: db.NewUserRepo(dbMap), - passwordInfoRepo: db.NewPasswordInfoRepo(dbMap), - clientRepo: db.NewClientRepo(dbMap), + userRepo: userRepo, + passwordInfoRepo: pwiRepo, + clientRepo: clientRepo, localConnectorID: localConnectorID, } } @@ -67,10 +67,20 @@ func errorMaker(typ string, desc string, code int) func(internal error) Error { } var ( + ErrorMissingClient = errorMaker("bad_request", "The 'client' cannot be empty", http.StatusBadRequest)(nil) + + // Called when oidc.ClientMetadata.Valid() fails. + ErrorInvalidClientFunc = errorMaker("bad_request", "Your client could not be validated.", http.StatusBadRequest) + errorMap = map[error]func(error) Error{ user.ErrorNotFound: errorMaker("resource_not_found", "Resource could not be found.", http.StatusNotFound), user.ErrorDuplicateEmail: errorMaker("bad_request", "Email already in use.", http.StatusBadRequest), user.ErrorInvalidEmail: errorMaker("bad_request", "invalid email.", http.StatusBadRequest), + + adminschema.ErrorInvalidRedirectURI: errorMaker("bad_request", "invalid redirectURI.", http.StatusBadRequest), + adminschema.ErrorInvalidLogoURI: errorMaker("bad_request", "invalid logoURI.", http.StatusBadRequest), + adminschema.ErrorInvalidClientURI: errorMaker("bad_request", "invalid clientURI.", http.StatusBadRequest), + adminschema.ErrorNoRedirectURI: errorMaker("bad_request", "invalid redirectURI.", http.StatusBadRequest), } ) @@ -117,19 +127,21 @@ func (a *AdminAPI) GetState() (adminschema.State, error) { } func (a *AdminAPI) CreateClient(req adminschema.ClientCreateRequest) (adminschema.ClientCreateResponse, error) { + if req.Client == nil { + return adminschema.ClientCreateResponse{}, ErrorMissingClient + } + cli, err := adminschema.MapSchemaClientToClient(*req.Client) if err != nil { - // TODO should be 400s return adminschema.ClientCreateResponse{}, mapError(err) } if err := cli.Metadata.Valid(); err != nil { - // TODO make sure this is not 500 - return adminschema.ClientCreateResponse{}, mapError(err) + return adminschema.ClientCreateResponse{}, ErrorInvalidClientFunc(err) } - // metadata is guarenteed to have at least one redirect_uri by earlier validation. - id, err := oidc.GenClientID(cli.Metadata.RedirectURIs[0].Host) + // metadata is guaranteed to have at least one redirect_uri by earlier validation. + id, err := ClientIDGenerator(cli.Metadata.RedirectURIs[0].Host) if err != nil { return adminschema.ClientCreateResponse{}, mapError(err) } @@ -146,8 +158,6 @@ func (a *AdminAPI) CreateClient(req adminschema.ClientCreateRequest) (adminschem return adminschema.ClientCreateResponse{ Client: req.Client, }, nil - - // github.com/coreos/dex/integrationoidc.ClientRegistrationResponse{ClientID: c.ID, ClientSecret: c.Secret, ClientMetadata: req.Client.Metadata}, nil } func mapError(e error) error { diff --git a/admin/api_test.go b/admin/api_test.go index 165d8ff5..27214d0d 100644 --- a/admin/api_test.go +++ b/admin/api_test.go @@ -3,6 +3,7 @@ package admin import ( "testing" + "github.com/coreos/dex/client" "github.com/coreos/dex/connector" "github.com/coreos/dex/db" "github.com/coreos/dex/schema/adminschema" @@ -15,6 +16,7 @@ import ( type testFixtures struct { ur user.UserRepo pwr user.PasswordInfoRepo + cr client.ClientRepo mgr *manager.UserManager adAPI *AdminAPI } @@ -69,7 +71,7 @@ func makeTestFixtures() *testFixtures { }() f.mgr = manager.NewUserManager(f.ur, f.pwr, ccr, db.TransactionFactory(dbMap), manager.ManagerOptions{}) - f.adAPI = NewAdminAPI(dbMap, f.mgr, "local") + f.adAPI = NewAdminAPI(f.ur, f.pwr, f.cr, f.mgr, "local") return f } diff --git a/cmd/dex-overlord/main.go b/cmd/dex-overlord/main.go index 8dad8aa0..b1a0aecb 100644 --- a/cmd/dex-overlord/main.go +++ b/cmd/dex-overlord/main.go @@ -116,10 +116,11 @@ func main() { userRepo := db.NewUserRepo(dbc) pwiRepo := db.NewPasswordInfoRepo(dbc) connCfgRepo := db.NewConnectorConfigRepo(dbc) + clientRepo := db.NewClientRepo(dbc) userManager := manager.NewUserManager(userRepo, pwiRepo, connCfgRepo, db.TransactionFactory(dbc), manager.ManagerOptions{}) - adminAPI := admin.NewAdminAPI(dbc, userManager, *localConnectorID) + adminAPI := admin.NewAdminAPI(userRepo, pwiRepo, clientRepo, userManager, *localConnectorID) kRepo, err := db.NewPrivateKeySetRepo(dbc, *useOldFormat, keySecrets.BytesSlice()...) if err != nil { log.Fatalf(err.Error()) diff --git a/db/client.go b/db/client.go index 84496b4b..42dd5f8d 100644 --- a/db/client.go +++ b/db/client.go @@ -92,10 +92,20 @@ func (m *clientModel) Client() (*client.Client, error) { func NewClientRepo(dbm *gorp.DbMap) client.ClientRepo { return newClientRepo(dbm) + +} + +func NewClientRepoWithSecretGenerator(dbm *gorp.DbMap, secGen SecretGenerator) client.ClientRepo { + rep := newClientRepo(dbm) + rep.secretGenerator = secGen + return rep } func newClientRepo(dbm *gorp.DbMap) *clientRepo { - return &clientRepo{db: &db{dbm}} + return &clientRepo{ + db: &db{dbm}, + secretGenerator: DefaultSecretGenerator, + } } func NewClientRepoFromClients(dbm *gorp.DbMap, clients []client.Client) (client.ClientRepo, error) { @@ -127,6 +137,7 @@ func NewClientRepoFromClients(dbm *gorp.DbMap, clients []client.Client) (client. type clientRepo struct { *db + secretGenerator SecretGenerator } func (r *clientRepo) Get(clientID string) (client.Client, error) { @@ -249,8 +260,14 @@ func isAlreadyExistsErr(err error) bool { return false } +type SecretGenerator func() ([]byte, error) + +func DefaultSecretGenerator() ([]byte, error) { + return pcrypto.RandBytes(maxSecretLength) +} + func (r *clientRepo) New(cli client.Client) (*oidc.ClientCredentials, error) { - secret, err := pcrypto.RandBytes(maxSecretLength) + secret, err := r.secretGenerator() if err != nil { return nil, err } diff --git a/integration/admin_api_test.go b/integration/admin_api_test.go index 0714b72f..1348a24b 100644 --- a/integration/admin_api_test.go +++ b/integration/admin_api_test.go @@ -1,20 +1,23 @@ package integration import ( - "errors" + "encoding/base64" + "fmt" "net/http" "net/http/httptest" "net/url" "testing" + "github.com/coreos/go-oidc/oidc" "github.com/kylelemons/godebug/pretty" "google.golang.org/api/googleapi" "github.com/coreos/dex/admin" + "github.com/coreos/dex/client" + "github.com/coreos/dex/db" "github.com/coreos/dex/schema/adminschema" "github.com/coreos/dex/server" "github.com/coreos/dex/user" - "github.com/coreos/go-oidc/oidc" ) const ( @@ -24,6 +27,7 @@ const ( type adminAPITestFixtures struct { ur user.UserRepo pwr user.PasswordInfoRepo + cr client.ClientRepo adAPI *admin.AdminAPI adSrv *server.AdminServer hSrv *httptest.Server @@ -78,9 +82,17 @@ func makeAdminAPITestFixtures() *adminAPITestFixtures { f := &adminAPITestFixtures{} dbMap, ur, pwr, um := makeUserObjects(adminUsers, adminPasswords) + + var cliCount int + secGen := func() ([]byte, error) { + return []byte(fmt.Sprintf("client_%v", cliCount)), nil + } + cr := db.NewClientRepoWithSecretGenerator(dbMap, secGen) + + f.cr = cr f.ur = ur f.pwr = pwr - f.adAPI = admin.NewAdminAPI(dbMap, um, "local") + f.adAPI = admin.NewAdminAPI(ur, pwr, cr, um, "local") f.adSrv = server.NewAdminServer(f.adAPI, nil, adminAPITestSecret) f.hSrv = httptest.NewServer(f.adSrv.HTTPHandler()) f.hc = &http.Client{ @@ -256,50 +268,147 @@ func TestCreateAdmin(t *testing.T) { } func TestCreateClient(t *testing.T) { + oldGen := admin.ClientIDGenerator + admin.ClientIDGenerator = func(hostport string) (string, error) { + return fmt.Sprintf("client_%v", hostport), nil + } + defer func() { + admin.ClientIDGenerator = oldGen + }() + + mustParseURL := func(s string) *url.URL { + u, err := url.Parse(s) + if err != nil { + t.Fatalf("couldn't parse URL: %v", err) + } + return u + } + addIDAndSecret := func(cli adminschema.Client) *adminschema.Client { + cli.Id = "client_auth.example.com" + cli.Secret = base64.URLEncoding.EncodeToString([]byte("client_0")) + return &cli + } + + adminClientGood := adminschema.Client{ + RedirectURIs: []string{"https://auth.example.com/"}, + } + clientGood := client.Client{ + Credentials: oidc.ClientCredentials{ + ID: "client_auth.example.com", + }, + Metadata: oidc.ClientMetadata{ + RedirectURIs: []url.URL{*mustParseURL("https://auth.example.com/")}, + }, + } + + adminAdminClient := adminClientGood + adminAdminClient.IsAdmin = true + clientGoodAdmin := clientGood + clientGoodAdmin.Admin = true + + adminMultiRedirect := adminClientGood + adminMultiRedirect.RedirectURIs = []string{"https://auth.example.com/", "https://auth2.example.com/"} + clientMultiRedirect := clientGoodAdmin + clientMultiRedirect.Metadata.RedirectURIs = append( + clientMultiRedirect.Metadata.RedirectURIs, + *mustParseURL("https://auth2.example.com/")) + tests := []struct { - client oidc.ClientMetadata - wantError bool + req adminschema.ClientCreateRequest + want adminschema.ClientCreateResponse + wantClient client.Client + wantError int }{ { - client: oidc.ClientMetadata{}, - wantError: true, + req: adminschema.ClientCreateRequest{}, + wantError: http.StatusBadRequest, }, { - client: oidc.ClientMetadata{ - RedirectURIs: []url.URL{ - {Scheme: "https", Host: "auth.example.com", Path: "/"}, + req: adminschema.ClientCreateRequest{ + Client: &adminschema.Client{ + IsAdmin: true, }, }, + wantError: http.StatusBadRequest, + }, + { + req: adminschema.ClientCreateRequest{ + Client: &adminschema.Client{ + RedirectURIs: []string{"909090"}, + }, + }, + wantError: http.StatusBadRequest, + }, + { + req: adminschema.ClientCreateRequest{ + Client: &adminClientGood, + }, + want: adminschema.ClientCreateResponse{ + Client: addIDAndSecret(adminClientGood), + }, + wantClient: clientGood, + }, + { + req: adminschema.ClientCreateRequest{ + Client: &adminAdminClient, + }, + want: adminschema.ClientCreateResponse{ + Client: addIDAndSecret(adminAdminClient), + }, + wantClient: clientGoodAdmin, + }, + { + req: adminschema.ClientCreateRequest{ + Client: &adminMultiRedirect, + }, + want: adminschema.ClientCreateResponse{ + Client: addIDAndSecret(adminMultiRedirect), + }, + wantClient: clientMultiRedirect, }, } for i, tt := range tests { - err := func() error { - f := makeAdminAPITestFixtures() - req := &adminschema.ClientCreateRequest{ - Client: &adminschema.Client{}, + if i != 3 { + continue + } + f := makeAdminAPITestFixtures() + + resp, err := f.adClient.Client.Create(&tt.req).Do() + if tt.wantError != 0 { + if err == nil { + t.Errorf("case %d: want non-nil error.", i) + continue } - for _, redirectURI := range tt.client.RedirectURIs { - req.Client.RedirectURIs = append(req.Client.RedirectURIs, redirectURI.String()) + aErr, ok := err.(*googleapi.Error) + if !ok { + t.Errorf("case %d: could not assert as adminSchema.Error: %v", i, err) + continue } - resp, err := f.adClient.Client.Create(req).Do() - if err != nil { - if tt.wantError { - return nil - } - return err + if aErr.Code != tt.wantError { + t.Errorf("case %d: want aErr.Code=%v, got %v", i, tt.wantError, aErr.Code) + continue } - if resp.Client.Id == "" { - return errors.New("no client id returned") - } - if resp.Client.Secret == "" { - return errors.New("no client secret returned") - } - return nil - }() + continue + } + if err != nil { - t.Errorf("case %d: %v", i, err) + t.Errorf("case %d: unexpected error creating client: %v", i, err) + continue + } + + if diff := pretty.Compare(tt.want, resp); diff != "" { + t.Errorf("case %d: Compare(want, got) = %v", i, diff) + } + + repoClient, err := f.cr.Get(resp.Client.Id) + if err != nil { + t.Errorf("case %d: Unexpected error getting client: %v", i, err) + } + + if diff := pretty.Compare(tt.wantClient, repoClient); diff != "" { + t.Errorf("case %d: Compare(wantClient, repoClient) = %v", i, diff) } } } diff --git a/schema/adminschema/mapper.go b/schema/adminschema/mapper.go index 3f6c32d1..f360750a 100644 --- a/schema/adminschema/mapper.go +++ b/schema/adminschema/mapper.go @@ -8,6 +8,13 @@ import ( "github.com/coreos/go-oidc/oidc" ) +var ( + ErrorNoRedirectURI = errors.New("No Redirect URIs") + ErrorInvalidRedirectURI = errors.New("Invalid Redirect URI") + ErrorInvalidLogoURI = errors.New("Invalid Logo URI") + ErrorInvalidClientURI = errors.New("Invalid Client URI") +) + func MapSchemaClientToClient(sc Client) (client.Client, error) { c := client.Client{ Credentials: oidc.ClientCredentials{ @@ -20,12 +27,12 @@ func MapSchemaClientToClient(sc Client) (client.Client, error) { } for i, ru := range sc.RedirectURIs { if ru == "" { - return client.Client{}, errors.New("redirect URL empty") + return client.Client{}, ErrorNoRedirectURI } u, err := url.Parse(ru) if err != nil { - return client.Client{}, errors.New("redirect URL invalid") + return client.Client{}, ErrorInvalidRedirectURI } c.Metadata.RedirectURIs[i] = *u @@ -36,7 +43,7 @@ func MapSchemaClientToClient(sc Client) (client.Client, error) { if sc.LogoURI != "" { logoURI, err := url.Parse(sc.LogoURI) if err != nil { - return client.Client{}, errors.New("logoURI invalid") + return client.Client{}, ErrorInvalidLogoURI } c.Metadata.LogoURI = logoURI } @@ -44,10 +51,12 @@ func MapSchemaClientToClient(sc Client) (client.Client, error) { if sc.ClientURI != "" { clientURI, err := url.Parse(sc.ClientURI) if err != nil { - return client.Client{}, errors.New("clientURI invalid") + return client.Client{}, ErrorInvalidClientURI } c.Metadata.ClientURI = clientURI } + + c.Admin = sc.IsAdmin return c, nil } @@ -69,5 +78,6 @@ func MapClientToSchemaClient(c client.Client) Client { if c.Metadata.ClientURI != nil { cl.ClientURI = c.Metadata.ClientURI.String() } + cl.IsAdmin = c.Admin return cl } From 9c403aba41f54c588c7a7e70da2e6256954642b5 Mon Sep 17 00:00:00 2001 From: Bobby Rullo Date: Tue, 19 Apr 2016 18:42:15 -0700 Subject: [PATCH 9/9] fix dexctl --- cmd/dexctl/driver_db.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cmd/dexctl/driver_db.go b/cmd/dexctl/driver_db.go index db1700d3..eba8b4b2 100644 --- a/cmd/dexctl/driver_db.go +++ b/cmd/dexctl/driver_db.go @@ -36,7 +36,12 @@ func (d *dbDriver) NewClient(meta oidc.ClientMetadata) (*oidc.ClientCredentials, return nil, err } - return d.ciRepo.New(clientID, meta, false) + return d.ciRepo.New(client.Client{ + Credentials: oidc.ClientCredentials{ + ID: clientID, + }, + Metadata: meta, + }) } func (d *dbDriver) ConnectorConfigs() ([]connector.ConnectorConfig, error) {