Skip to content

Commit

Permalink
fix: issue 102 and prevent store user pass in ext
Browse files Browse the repository at this point in the history
  • Loading branch information
shaj13 committed Mar 12, 2021
1 parent b4ee370 commit 57ca76a
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 25 deletions.
28 changes: 16 additions & 12 deletions auth/strategies/basic/cached.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (

// ExtensionKey represents a key for the password in info extensions.
// Typically used when basic strategy cache the authentication decisions.
//
// Deprecated: No longer used.
const ExtensionKey = "x-go-guardian-basic-password"

// NewCached return new auth.Strategy.
Expand All @@ -26,6 +28,11 @@ func NewCached(f AuthenticateFunc, cache auth.Cache, opts ...auth.Option) auth.S
return New(cb.authenticate, opts...)
}

type entry struct {
password string
info auth.Info
}

type cachedBasic struct {
fn AuthenticateFunc
comparator Comparator
Expand All @@ -42,18 +49,12 @@ func (c *cachedBasic) authenticate(ctx context.Context, r *http.Request, userNam
return c.authenticatAndHash(ctx, r, hash, userName, pass)
}

if _, ok := v.(auth.Info); !ok {
return nil, auth.NewTypeError("strategies/basic:", (*auth.Info)(nil), v)
}

info := v.(auth.Info)
ext := info.GetExtensions()

if !ext.Has(ExtensionKey) {
return c.authenticatAndHash(ctx, r, hash, userName, pass)
ent, ok := v.(entry)
if !ok {
return nil, auth.NewTypeError("strategies/basic:", entry{}, v)
}

return info, c.comparator.Compare(ext.Get(ExtensionKey), pass)
return ent.info, c.comparator.Compare(ent.password, pass)
}

func (c *cachedBasic) authenticatAndHash(ctx context.Context, r *http.Request, hash string, userName, pass string) (auth.Info, error) { //nolint:lll
Expand All @@ -63,8 +64,11 @@ func (c *cachedBasic) authenticatAndHash(ctx context.Context, r *http.Request, h
}

hashedPass, _ := c.comparator.Hash(pass)
info.GetExtensions().Set(ExtensionKey, hashedPass)
c.cache.Store(hash, info)
ent := entry{
password: hashedPass,
info: info,
}
c.cache.Store(hash, ent)

return info, nil
}
18 changes: 5 additions & 13 deletions auth/strategies/basic/cached_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,6 @@ func TestNewCached(t *testing.T) {
setCredentials: func(r *http.Request) { r.SetBasicAuth("predefined2", "test") },
expectedErr: false,
},
{
name: "it re-authenticate user when hash missing",
setCredentials: func(r *http.Request) { r.SetBasicAuth("predefined3", "test") },
expectedErr: false,
},
{
name: "it return error when cache hold invalid user info",
setCredentials: func(r *http.Request) { r.SetBasicAuth("predefined", "test") },
Expand Down Expand Up @@ -69,19 +64,16 @@ func TestNewCached(t *testing.T) {

cache := libcache.LRU.New(0)
cache.Store("predefined", "invalid-type")
cache.Store("predefined2", auth.NewDefaultUser(
cache.Store(
"predefined2",
"10",
nil,
map[string][]string{
ExtensionKey: {"9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08"},
entry{
password: "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08",
info: auth.NewDefaultUser("predefined2", "10", nil, nil),
},
))
cache.Store("predefined3", auth.NewDefaultUser("predefined3", "10", nil, nil))
)

opt := SetHash(crypto.SHA256)
info, err := NewCached(authFunc, cache, opt).Authenticate(r.Context(), r)

assert.Equal(t, tt.expectedErr, err != nil, "%s: Got Unexpected error %v", tt.name, err)
assert.Equal(t, !tt.expectedErr, info != nil, "%s: Expected info object, got nil", tt.name)
})
Expand Down

0 comments on commit 57ca76a

Please sign in to comment.