Skip to content

Commit

Permalink
client: Deprecate field id, now only client_id is to be used
Browse files Browse the repository at this point in the history
  • Loading branch information
arekkas committed Jul 22, 2018
1 parent eb70025 commit 2073298
Show file tree
Hide file tree
Showing 30 changed files with 93 additions and 158 deletions.
3 changes: 0 additions & 3 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,6 @@ type Client struct {
// ClientID is the id for this client.
ClientID string `json:"client_id"`

// ID is an alisa for client_id.
ID string `json:"id"`

// Name is the human-readable string name of the client to be presented to the
// end-user during authorization.
Name string `json:"client_name"`
Expand Down
2 changes: 1 addition & 1 deletion client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ var _ fosite.Client = new(Client)

func TestClient(t *testing.T) {
c := &Client{
ID: "foo",
ClientID: "foo",
RedirectURIs: []string{"foo"},
Scope: "foo bar",
TokenEndpointAuthMethod: "none",
Expand Down
1 change: 0 additions & 1 deletion client/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ func (h *Handler) Update(w http.ResponseWriter, r *http.Request, ps httprouter.P
secret = c.Secret
}

c.ID = ps.ByName("id")
c.ClientID = ps.ByName("id")
if err := h.Validator.Validate(&c); err != nil {
h.H.WriteError(w, r, err)
Expand Down
2 changes: 1 addition & 1 deletion client/manager_0_sql_migrations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func TestMigrations(t *testing.T) {
s := &client.SQLManager{DB: db, Hasher: &fosite.BCrypt{WorkFactor: 4}}
c, err := s.GetConcreteClient(key)
require.NoError(t, err)
assert.EqualValues(t, c.ID, key)
assert.EqualValues(t, c.GetID(), key)
})
}

Expand Down
15 changes: 6 additions & 9 deletions client/manager_memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ func (m *MemoryManager) GetConcreteClient(id string) (*Client, error) {
defer m.RUnlock()

for _, c := range m.Clients {
if c.ID == id {
c.ClientID = c.ID
if c.GetID() == id {
return &c, nil
}
}
Expand All @@ -67,7 +66,7 @@ func (m *MemoryManager) GetClient(_ context.Context, id string) (fosite.Client,
}

func (m *MemoryManager) UpdateClient(c *Client) error {
o, err := m.GetClient(context.Background(), c.ID)
o, err := m.GetClient(context.Background(), c.GetID())
if err != nil {
return err
}
Expand All @@ -88,7 +87,7 @@ func (m *MemoryManager) UpdateClient(c *Client) error {
m.Lock()
defer m.Unlock()
for k, f := range m.Clients {
if f.GetID() == c.ID {
if f.GetID() == c.GetID() {
m.Clients[k] = *c
}
}
Expand All @@ -109,13 +108,12 @@ func (m *MemoryManager) Authenticate(id string, secret []byte) (*Client, error)
return nil, errors.WithStack(err)
}

c.ClientID = c.ID
return c, nil
}

func (m *MemoryManager) CreateClient(c *Client) error {
if _, err := m.GetConcreteClient(c.ID); err == nil {
return errors.Errorf("Client %s already exists", c.ID)
if _, err := m.GetConcreteClient(c.GetID()); err == nil {
return errors.Errorf("Client %s already exists", c.GetID())
}

m.Lock()
Expand Down Expand Up @@ -152,8 +150,7 @@ func (m *MemoryManager) GetClients(limit, offset int) (clients map[string]Client

start, end := pagination.Index(limit, offset, len(m.Clients))
for _, c := range m.Clients[start:end] {
c.ClientID = c.ID
clients[c.ID] = c
clients[c.GetID()] = c
}

return clients, nil
Expand Down
5 changes: 2 additions & 3 deletions client/manager_sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ func sqlDataFromClient(d *Client) (*sqlData, error) {
}

return &sqlData{
ID: d.ID,
ID: d.GetID(),
Name: d.Name,
Secret: d.Secret,
RedirectURIs: strings.Join(d.RedirectURIs, "|"),
Expand All @@ -239,7 +239,6 @@ func sqlDataFromClient(d *Client) (*sqlData, error) {

func (d *sqlData) ToClient() (*Client, error) {
c := &Client{
ID: d.ID,
ClientID: d.ID,
Name: d.Name,
Secret: d.Secret,
Expand Down Expand Up @@ -301,7 +300,7 @@ func (m *SQLManager) GetClient(_ context.Context, id string) (fosite.Client, err
}

func (m *SQLManager) UpdateClient(c *Client) error {
o, err := m.GetClient(context.Background(), c.ID)
o, err := m.GetClient(context.Background(), c.GetID())
if err != nil {
return errors.WithStack(err)
}
Expand Down
18 changes: 9 additions & 9 deletions client/manager_test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,22 @@ func TestHelperClientAutoGenerateKey(k string, m Storage) func(t *testing.T) {
return func(t *testing.T) {
t.Parallel()
c := &Client{
ID: "foo",
ClientID: "foo",
Secret: "secret",
RedirectURIs: []string{"http://redirect"},
TermsOfServiceURI: "foo",
}
assert.NoError(t, m.CreateClient(c))
//assert.NotEmpty(t, c.ID)
assert.NoError(t, m.DeleteClient(c.ID))
assert.NoError(t, m.DeleteClient(c.GetID()))
}
}

func TestHelperClientAuthenticate(k string, m Manager) func(t *testing.T) {
return func(t *testing.T) {
t.Parallel()
m.CreateClient(&Client{
ID: "1234321",
ClientID: "1234321",
Secret: "secret",
RedirectURIs: []string{"http://redirect"},
})
Expand All @@ -59,7 +59,7 @@ func TestHelperClientAuthenticate(k string, m Manager) func(t *testing.T) {

c, err = m.Authenticate("1234321", []byte("secret"))
require.NoError(t, err)
assert.Equal(t, "1234321", c.ID)
assert.Equal(t, "1234321", c.GetID())
}
}

Expand All @@ -70,7 +70,7 @@ func TestHelperCreateGetDeleteClient(k string, m Storage) func(t *testing.T) {
assert.NotNil(t, err)

c := &Client{
ID: "1234",
ClientID: "1234",
Name: "name",
Secret: "secret",
RedirectURIs: []string{"http://redirect", "http://redirect1"},
Expand Down Expand Up @@ -100,7 +100,7 @@ func TestHelperCreateGetDeleteClient(k string, m Storage) func(t *testing.T) {
}

assert.NoError(t, m.CreateClient(&Client{
ID: "2-1234",
ClientID: "2-1234",
Name: "name",
Secret: "secret",
RedirectURIs: []string{"http://redirect"},
Expand All @@ -116,8 +116,8 @@ func TestHelperCreateGetDeleteClient(k string, m Storage) func(t *testing.T) {
ds, err := m.GetClients(100, 0)
assert.NoError(t, err)
assert.Len(t, ds, 2)
assert.NotEqual(t, ds["1234"].ID, ds["2-1234"].ID)
assert.NotEqual(t, ds["1234"].ID, ds["2-1234"].ClientID)
assert.NotEqual(t, ds["1234"].ClientID, ds["2-1234"].ClientID)
assert.NotEqual(t, ds["1234"].ClientID, ds["2-1234"].ClientID)

//test if SecretExpiresAt was set properly
assert.Equal(t, ds["1234"].SecretExpiresAt, 0)
Expand All @@ -132,7 +132,7 @@ func TestHelperCreateGetDeleteClient(k string, m Storage) func(t *testing.T) {
assert.Len(t, ds, 0)

err = m.UpdateClient(&Client{
ID: "2-1234",
ClientID: "2-1234",
Name: "name-new",
Secret: "secret-new",
RedirectURIs: []string{"http://redirect/new"},
Expand Down
11 changes: 2 additions & 9 deletions client/sdk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ func TestClientSDK(t *testing.T) {
t.Run("case=client is created and updated", func(t *testing.T) {
createClient := createTestClient("")
compareClient := createClient
compareClient.Id = compareClient.ClientId
createClient.ClientSecretExpiresAt = 10

// returned client is correct on Create
Expand Down Expand Up @@ -114,7 +113,6 @@ func TestClientSDK(t *testing.T) {
// create another client
updateClient := createTestClient("foo")
result, response, err = c.UpdateOAuth2Client(createClient.ClientId, updateClient)
updateClient.Id = updateClient.ClientId
require.NoError(t, err)
require.EqualValues(t, http.StatusOK, response.StatusCode, "%s", response.Payload)
assert.EqualValues(t, updateClient, *result)
Expand All @@ -125,7 +123,6 @@ func TestClientSDK(t *testing.T) {
result, response, err = c.GetOAuth2Client(updateClient.ClientId)
require.NoError(t, err)
require.EqualValues(t, http.StatusOK, response.StatusCode, "%s", response.Payload)
compareClient.ClientId = compareClient.Id
assert.EqualValues(t, compareClient, *result)

// client can not be found after being deleted
Expand Down Expand Up @@ -163,7 +160,7 @@ func TestClientSDK(t *testing.T) {
client: hydra.OAuth2Client{},
},
{
client: hydra.OAuth2Client{Id: "set-properly-1"},
client: hydra.OAuth2Client{ClientId: "set-properly-1"},
expectID: "set-properly-1",
},
{
Expand All @@ -176,13 +173,10 @@ func TestClientSDK(t *testing.T) {
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
id := result.ClientId
if tc.expectID != "" {
assert.EqualValues(t, tc.expectID, result.Id)
assert.EqualValues(t, tc.expectID, result.ClientId)
id = tc.expectID
}
Expand All @@ -191,7 +185,6 @@ func TestClientSDK(t *testing.T) {
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)
})
}
Expand Down
6 changes: 1 addition & 5 deletions client/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,7 @@ func NewValidator(

func (v *Validator) Validate(c *Client) error {
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."))
}
c.ClientID = stringsx.Coalesce(c.ClientID, id)

if c.TokenEndpointAuthMethod == "" {
c.TokenEndpointAuthMethod = "client_secret_basic"
Expand Down
14 changes: 5 additions & 9 deletions client/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,30 +44,26 @@ func TestValidate(t *testing.T) {
in: new(Client),
check: func(t *testing.T, c *Client) {
assert.NotEmpty(t, c.ClientID)
assert.NotEmpty(t, c.ID)
assert.Equal(t, c.ID, c.ClientID)
assert.NotEmpty(t, c.GetID())
assert.Equal(t, c.GetID(), c.ClientID)
},
},
{
in: &Client{ID: "foo"},
in: &Client{ClientID: "foo"},
check: func(t *testing.T, c *Client) {
assert.Equal(t, c.ID, c.ClientID)
assert.Equal(t, c.GetID(), c.ClientID)
},
},
{
in: &Client{ClientID: "foo"},
check: func(t *testing.T, c *Client) {
assert.Equal(t, c.ID, c.ClientID)
assert.Equal(t, c.GetID(), c.ClientID)
},
},
{
in: &Client{ClientID: "foo", UserinfoSignedResponseAlg: "foo"},
expectErr: true,
},
{
in: &Client{ClientID: "foo", ID: "bar"},
expectErr: true,
},
{
in: &Client{ClientID: "foo", TokenEndpointAuthMethod: "private_key_jwt"},
expectErr: true,
Expand Down
12 changes: 6 additions & 6 deletions consent/manager_memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (m *MemoryManager) RevokeUserClientConsentSession(user, client string) erro

var found bool
for k, c := range m.handledConsentRequests {
if c.ConsentRequest.Subject == user && (client == "" || (client != "" && c.ConsentRequest.Client.ID == client)) {
if c.ConsentRequest.Subject == user && (client == "" || (client != "" && c.ConsentRequest.Client.GetID() == client)) {
delete(m.handledConsentRequests, k)
delete(m.consentRequests, k)
if err := m.store.RevokeAccessToken(nil, c.Challenge); errors.Cause(err) == fosite.ErrNotFound {
Expand Down Expand Up @@ -124,7 +124,7 @@ func (m *MemoryManager) GetConsentRequest(challenge string) (*ConsentRequest, er
m.m["consentRequests"].RLock()
defer m.m["consentRequests"].RUnlock()
if c, ok := m.consentRequests[challenge]; ok {
c.Client.ClientID = c.Client.ID
c.Client.ClientID = c.Client.GetID()
return &c, nil
}
return nil, errors.WithStack(pkg.ErrNotFound)
Expand All @@ -151,7 +151,7 @@ func (m *MemoryManager) VerifyAndInvalidateConsentRequest(verifier string) (*Han
return nil, err
}

c.Client.ClientID = c.Client.ID
c.Client.ClientID = c.Client.GetID()
h.ConsentRequest = &c
return &h, nil
}
Expand Down Expand Up @@ -195,7 +195,7 @@ func (m *MemoryManager) FindPreviouslyGrantedConsentRequests(client string, subj
continue
}

cr.Client.ClientID = cr.Client.ID
cr.Client.ClientID = cr.Client.GetID()
c.ConsentRequest = cr
rs = append(rs, c)
}
Expand Down Expand Up @@ -246,7 +246,7 @@ func (m *MemoryManager) GetAuthenticationRequest(challenge string) (*Authenticat
m.m["authRequests"].RLock()
defer m.m["authRequests"].RUnlock()
if c, ok := m.authRequests[challenge]; ok {
c.Client.ClientID = c.Client.ID
c.Client.ClientID = c.Client.GetID()
return &c, nil
}
return nil, errors.WithStack(pkg.ErrNotFound)
Expand All @@ -273,7 +273,7 @@ func (m *MemoryManager) VerifyAndInvalidateAuthenticationRequest(verifier string
return nil, err
}

c.Client.ClientID = c.Client.ID
c.Client.ClientID = c.Client.GetID()
h.AuthenticationRequest = &c
return &h, nil
}
Expand Down
8 changes: 4 additions & 4 deletions consent/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func mockConsentRequest(key string, remember bool, rememberFor int, hasError boo
Display: "popup" + key,
},
RequestedAt: time.Now().UTC().Add(-time.Hour),
Client: &client.Client{ID: "client" + key},
Client: &client.Client{ClientID: "client" + key},
Subject: "subject" + key,
RequestURL: "https://request-url/path" + key,
Skip: skip,
Expand Down Expand Up @@ -100,7 +100,7 @@ func mockAuthRequest(key string, authAt bool) (c *AuthenticationRequest, h *Hand
Display: "popup" + key,
},
RequestedAt: time.Now().UTC().Add(-time.Hour),
Client: &client.Client{ID: "client" + key},
Client: &client.Client{ClientID: "client" + key},
Subject: "subject" + key,
RequestURL: "https://request-url/path" + key,
Skip: true,
Expand Down Expand Up @@ -482,7 +482,7 @@ func TestManagers(t *testing.T) {
}

func compareAuthenticationRequest(t *testing.T, a, b *AuthenticationRequest) {
assert.EqualValues(t, a.Client.ID, b.Client.ID)
assert.EqualValues(t, a.Client.GetID(), b.Client.GetID())
assert.EqualValues(t, a.Challenge, b.Challenge)
assert.EqualValues(t, *a.OpenIDConnectContext, *b.OpenIDConnectContext)
assert.EqualValues(t, a.Subject, b.Subject)
Expand All @@ -494,7 +494,7 @@ func compareAuthenticationRequest(t *testing.T, a, b *AuthenticationRequest) {
}

func compareConsentRequest(t *testing.T, a, b *ConsentRequest) {
assert.EqualValues(t, a.Client.ID, b.Client.ID)
assert.EqualValues(t, a.Client.GetID(), b.Client.GetID())
assert.EqualValues(t, a.Challenge, b.Challenge)
assert.EqualValues(t, *a.OpenIDConnectContext, *b.OpenIDConnectContext)
assert.EqualValues(t, a.Subject, b.Subject)
Expand Down
4 changes: 2 additions & 2 deletions consent/sdk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,12 @@ func compareSDKLoginRequest(t *testing.T, expected *AuthenticationRequest, got *
assert.EqualValues(t, expected.Challenge, got.Challenge)
assert.EqualValues(t, expected.Subject, got.Subject)
assert.EqualValues(t, expected.Skip, got.Skip)
assert.EqualValues(t, expected.Client.ID, got.Client.Id)
assert.EqualValues(t, expected.Client.GetID(), got.Client.ClientId)
}

func compareSDKConsentRequest(t *testing.T, expected *ConsentRequest, got *swagger.ConsentRequest) {
assert.EqualValues(t, expected.Challenge, got.Challenge)
assert.EqualValues(t, expected.Subject, got.Subject)
assert.EqualValues(t, expected.Skip, got.Skip)
assert.EqualValues(t, expected.Client.ID, got.Client.Id)
assert.EqualValues(t, expected.Client.GetID(), got.Client.ClientId)
}
Loading

0 comments on commit 2073298

Please sign in to comment.