From 48a6498577fb352004a3b533c443dd39117d1a4a Mon Sep 17 00:00:00 2001
From: arekkas <aeneas@ory.am>
Date: Sun, 15 Jul 2018 13:08:13 +0200
Subject: [PATCH] client: Improve handling of legacy `id` field

Closes #924

Signed-off-by: arekkas <aeneas@ory.am>
---
 Gopkg.lock               |  6 +++---
 Gopkg.toml               |  2 +-
 client/handler.go        |  1 +
 client/manager_memory.go |  2 +-
 client/sdk_test.go       | 44 ++++++++++++++++++++++++++++++++++++++++
 client/validator.go      | 13 +++++-------
 6 files changed, 55 insertions(+), 13 deletions(-)

diff --git a/Gopkg.lock b/Gopkg.lock
index 88a5fb13e46..a0c5b560394 100644
--- a/Gopkg.lock
+++ b/Gopkg.lock
@@ -316,8 +316,8 @@
     "stringsx",
     "urlx"
   ]
-  revision = "8e5660ff0e4c9c7e0cecd82ea8a87a793ff9ed23"
-  version = "v0.0.3"
+  revision = "857ebcb1de6fdd166e791d976a46c3209d8355a8"
+  version = "v0.0.4"
 
 [[projects]]
   name = "github.com/ory/graceful"
@@ -636,6 +636,6 @@
 [solve-meta]
   analyzer-name = "dep"
   analyzer-version = 1
-  inputs-digest = "c1d09891819f39bba8546871d98e9930e32f5ceaf381a48f24d383273e19e62d"
+  inputs-digest = "93ea649736b0fc7acaa3ade7c61867c793f4279abe45e48ad5af57dd90e5ae23"
   solver-name = "gps-cdcl"
   solver-version = 1
diff --git a/Gopkg.toml b/Gopkg.toml
index e60f44b3b6c..7c76497f3ef 100644
--- a/Gopkg.toml
+++ b/Gopkg.toml
@@ -131,7 +131,7 @@
 
 [[constraint]]
   name = "github.com/ory/go-convenience"
-  version = "0.0.2"
+  version = "0.0.4"
 
 [[constraint]]
   branch = "master"
diff --git a/client/handler.go b/client/handler.go
index 08ec9fb4f7a..7d1f15e0c0a 100644
--- a/client/handler.go
+++ b/client/handler.go
@@ -153,6 +153,7 @@ func (h *Handler) Update(w http.ResponseWriter, r *http.Request, ps httprouter.P
 	}
 
 	c.ID = ps.ByName("id")
+	c.ClientID = ps.ByName("id")
 	if err := h.Validator.Validate(&c); err != nil {
 		h.H.WriteError(w, r, err)
 		return
diff --git a/client/manager_memory.go b/client/manager_memory.go
index a5605faae96..af5e072b0e4 100644
--- a/client/manager_memory.go
+++ b/client/manager_memory.go
@@ -53,7 +53,7 @@ func (m *MemoryManager) GetConcreteClient(id string) (*Client, error) {
 	defer m.RUnlock()
 
 	for _, c := range m.Clients {
-		if c.GetID() == id {
+		if c.ID == id {
 			c.ClientID = c.ID
 			return &c, nil
 		}
diff --git a/client/sdk_test.go b/client/sdk_test.go
index f1505cd5f7d..40fc10b91ea 100644
--- a/client/sdk_test.go
+++ b/client/sdk_test.go
@@ -21,6 +21,7 @@
 package client_test
 
 import (
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"strings"
@@ -152,4 +153,47 @@ func TestClientSDK(t *testing.T) {
 
 		assert.Equal(t, "secret", result.ClientSecret)
 	})
+
+	t.Run("case=id should be set properly", func(t *testing.T) {
+		for k, tc := range []struct {
+			client   hydra.OAuth2Client
+			expectID string
+		}{
+			{
+				client: hydra.OAuth2Client{},
+			},
+			{
+				client:   hydra.OAuth2Client{Id: "set-properly-1"},
+				expectID: "set-properly-1",
+			},
+			{
+				client:   hydra.OAuth2Client{ClientId: "set-properly-2"},
+				expectID: "set-properly-2",
+			},
+		} {
+			t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
+				result, response, err := c.CreateOAuth2Client(tc.client)
+				require.NoError(t, err)
+				require.EqualValues(t, http.StatusCreated, response.StatusCode, "%s", response.Payload)
+
+				assert.NotEmpty(t, result.Id)
+				assert.NotEmpty(t, result.ClientId)
+				assert.EqualValues(t, result.Id, result.ClientId)
+
+				id := result.Id
+				if tc.expectID != "" {
+					assert.EqualValues(t, tc.expectID, result.Id)
+					assert.EqualValues(t, tc.expectID, result.ClientId)
+					id = tc.expectID
+				}
+
+				result, response, err = c.GetOAuth2Client(id)
+				require.NoError(t, err)
+				require.EqualValues(t, http.StatusOK, response.StatusCode, "%s", response.Payload)
+
+				assert.EqualValues(t, id, result.Id)
+				assert.EqualValues(t, id, result.ClientId)
+			})
+		}
+	})
 }
diff --git a/client/validator.go b/client/validator.go
index 92b440486fd..197f32ef086 100644
--- a/client/validator.go
+++ b/client/validator.go
@@ -29,6 +29,7 @@ import (
 
 	"github.com/ory/fosite"
 	"github.com/ory/go-convenience/stringslice"
+	"github.com/ory/go-convenience/stringsx"
 	"github.com/pborman/uuid"
 	"github.com/pkg/errors"
 )
@@ -47,14 +48,10 @@ func NewValidator(
 }
 
 func (v *Validator) Validate(c *Client) error {
-	if c.ID == "" && c.ClientID == "" {
-		c.ID = uuid.New()
-		c.ClientID = c.ID
-	} else if c.ID == "" && c.ClientID != "" {
-		c.ID = c.ClientID
-	} else if c.ID != "" && c.ClientID == "" {
-		c.ClientID = c.ID
-	} else if c.ID != c.ClientID {
+	id := uuid.New()
+	c.ID = stringsx.Coalesce(c.ID, c.ClientID, id)
+	c.ClientID = stringsx.Coalesce(c.ClientID, c.ID, id)
+	if c.ID != c.ClientID {
 		return errors.WithStack(fosite.ErrInvalidRequest.WithHint("Field id and client_id must match."))
 	}