Skip to content

Commit a606a4e

Browse files
committed
Simplify backend api and use secure comparison
1 parent f680825 commit a606a4e

6 files changed

+33
-14
lines changed

assertion_jwt_grant_type.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func (gt AssertionJWTGrantType) TokenHandler(c *Client, ew *EncoderResponseWrite
6868
return
6969
}
7070

71-
u, err := gt.persistence.GetUserByLogin(jwtTk.Subject)
71+
u, err := gt.persistence.GetUserByUsername(jwtTk.Subject)
7272
if err != nil {
7373
log.Println("JWT subject is not a valid user")
7474
ew.Encode(ErrInvalidGrant)

backend.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,7 @@ type PersistenceBackend interface {
5252
//*
5353
// User persistence
5454
//*
55-
GetUserByLogin(login string) (*User, error)
56-
GetUserByCredentials(username, password string) (*User, error)
55+
GetUserByUsername(username string) (*User, error)
5756
}
5857

5958
type AuthorizationPageData struct {

common.go

+16-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import (
2626
)
2727

2828
type User struct {
29-
Login string
29+
Username string
3030
Password string
3131
}
3232

@@ -155,3 +155,18 @@ func secureRandomBytes(bytes uint) ([]byte, error) {
155155
_, err := rand.Read(r)
156156
return r, err
157157
}
158+
159+
// secureCompare will compare two slice of bytes in constant time, ensuring no timing information
160+
// is leaked in order to prevent timing attacks.
161+
func secureCompare(a, b []byte) bool {
162+
if len(a) != len(b) {
163+
return false
164+
}
165+
166+
var result byte
167+
for i := 0; i < len(a); i++ {
168+
result |= a[i] ^ b[i]
169+
}
170+
171+
return result == 0
172+
}

in_memory_persistence.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,9 @@ func (b *InMemoryPersistence) SaveScope(s *Scope) error {
133133
return nil
134134
}
135135

136-
// GetUserByLogin lookup the user that matches the login
137-
func (b *InMemoryPersistence) GetUserByLogin(login string) (*User, error) {
138-
u, exst := b.users[login]
136+
// GetUserByUsername lookup the user that matches the login
137+
func (b *InMemoryPersistence) GetUserByUsername(username string) (*User, error) {
138+
u, exst := b.users[username]
139139
if !exst {
140140
return nil, ErrNotFound
141141
}
@@ -155,6 +155,6 @@ func (b *InMemoryPersistence) GetUserByCredentials(username, password string) (*
155155
// SaveUser persists the user in the backend, it's not part of the Backend interface
156156
// but we need a way to add users to the Backend.
157157
func (b *InMemoryPersistence) SaveUser(u *User) error {
158-
b.users[u.Login] = u
158+
b.users[u.Username] = u
159159
return nil
160160
}

integration_test.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ func setupProvider() (oauth2.PersistenceBackend, *oauth2.ClientAgent, *httptest.
9595
panic(err)
9696
}
9797

98-
user := oauth2.User{Login: "username"}
98+
user := oauth2.User{Username: "username"}
9999
if err := inMemory.SaveUser(&user); err != nil {
100100
panic(err)
101101
}
@@ -168,7 +168,7 @@ func TestAuthorizationCodeGrantType(t *testing.T) {
168168
t.Fatal(err)
169169
}
170170

171-
if persistedAuth.Client.ID != clt.ID || persistedAuth.User.Login != "username" {
171+
if persistedAuth.Client.ID != clt.ID || persistedAuth.User.Username != "username" {
172172
t.Errorf("Authorization does not match client or user")
173173
}
174174

@@ -192,7 +192,7 @@ func TestAuthorizationCodeGrantType(t *testing.T) {
192192
t.Fatal(err)
193193
}
194194

195-
if refreshedPersistedAuth.Client.ID != clt.ID || refreshedPersistedAuth.User.Login != "username" {
195+
if refreshedPersistedAuth.Client.ID != clt.ID || refreshedPersistedAuth.User.Username != "username" {
196196
t.Errorf("Authorization does not match client or user")
197197
}
198198

@@ -247,7 +247,7 @@ func TestPasswordGrantType(t *testing.T) {
247247
t.Fatal(err)
248248
}
249249

250-
if a2.Client.ID != clt.ID || a2.User.Login != "username" {
250+
if a2.Client.ID != clt.ID || a2.User.Username != "username" {
251251
t.Errorf("Authorization does not match client or user")
252252
}
253253

@@ -282,7 +282,7 @@ func TestAssertionGrantType(t *testing.T) {
282282
t.Fatal(err)
283283
}
284284

285-
if a2.Client.ID != clt.ID || a2.User.Login != "username" {
285+
if a2.Client.ID != clt.ID || a2.User.Username != "username" {
286286
t.Errorf("Authorization does not match client or user")
287287
}
288288

resource_owner_credentials_grant_type.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,17 @@ func (gt ResourceOwnerCredentialsGrantType) TokenHandler(c *Client, ew *EncoderR
5757
return
5858
}
5959

60-
u, err := gt.persistence.GetUserByCredentials(username, password)
60+
u, err := gt.persistence.GetUserByUsername(username)
6161
if err != nil {
6262
log.Println("invalid credentials")
6363
ew.Encode(ErrAccessDenied)
6464
return
6565
}
66+
if !secureCompare([]byte(username), []byte(u.Username)) {
67+
log.Println("invalid password")
68+
ew.Encode(ErrAccessDenied)
69+
return
70+
}
6671

6772
auth, err := NewAuthorization(c, u, scope, true, false)
6873
if err != nil {

0 commit comments

Comments
 (0)