Refactored credential handling

This commit is contained in:
Denis Arh
2022-07-14 20:57:56 +02:00
parent b44024fb31
commit f2a4601ce3
6 changed files with 1381 additions and 1204 deletions
+25 -1063
View File
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
+198
View File
@@ -0,0 +1,198 @@
package service
import (
"github.com/cortezaproject/corteza-server/system/types"
"github.com/stretchr/testify/require"
"golang.org/x/crypto/bcrypt"
"testing"
"time"
)
func Test_isValidPassword(t *testing.T) {
var (
pwdPlain = " ... plain password ... "
pwdHashedB, _ = bcrypt.GenerateFromPassword([]byte(pwdPlain), bcrypt.DefaultCost)
pwdHashed = string(pwdHashedB)
pwdUnknown = "$2a$10$8sOZxfZinxnu3bAtpkqEx.wBBwOfci6aG1szgUyxm5.BL2WiLu.ni"
)
cases := []struct {
name string
password string
cc types.CredentialsSet
rval bool
}{
{
name: "empty set",
rval: false,
},
{
name: "bad pwd",
rval: false,
password: " foo ",
cc: types.CredentialsSet{&types.Credentials{ID: 1, Credentials: pwdHashed}},
},
{
name: "invalid credentials",
rval: false,
password: " foo ",
cc: types.CredentialsSet{&types.Credentials{ID: 0, Credentials: pwdHashed}},
},
{
name: "ok",
rval: true,
password: pwdPlain,
cc: types.CredentialsSet{&types.Credentials{ID: 1, Credentials: pwdHashed}},
},
{
name: "multipass",
rval: true,
password: pwdPlain,
cc: types.CredentialsSet{
&types.Credentials{ID: 0, Credentials: pwdHashed},
&types.Credentials{ID: 1, Credentials: pwdUnknown},
&types.Credentials{ID: 2, Credentials: pwdHashed},
&types.Credentials{ID: 3, Credentials: ""},
},
},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
var (
req = require.New(t)
rsp = isValidPassword(c.cc, c.password)
)
if c.rval {
req.True(rsp)
} else {
req.False(rsp)
}
})
}
}
func Test_isPasswordReused(t *testing.T) {
var (
pwdPlain = " ... plain password ... "
pwdHashedB, _ = bcrypt.GenerateFromPassword([]byte(pwdPlain), bcrypt.DefaultCost)
pwdHashed = string(pwdHashedB)
pwdUnknown = "$2a$10$8sOZxfZinxnu3bAtpkqEx.wBBwOfci6aG1szgUyxm5.BL2WiLu.ni"
)
cases := []struct {
name string
password string
window time.Duration
cc types.CredentialsSet
rval bool
}{
{
name: "no credentials, not reused",
rval: false,
password: pwdPlain,
cc: types.CredentialsSet{},
},
{
name: "not reused",
rval: false,
password: pwdPlain,
cc: types.CredentialsSet{
&types.Credentials{ID: 1, Credentials: pwdUnknown},
&types.Credentials{ID: 2, Credentials: ""},
},
},
{
name: "present, valid, first",
rval: true,
password: pwdPlain,
cc: types.CredentialsSet{
&types.Credentials{ID: 1, Credentials: pwdHashed},
&types.Credentials{ID: 2, Credentials: pwdUnknown},
&types.Credentials{ID: 3, Credentials: ""},
},
},
{
name: "present, but within time window",
rval: false,
password: pwdPlain,
window: 5 * time.Minute,
cc: types.CredentialsSet{
&types.Credentials{ID: 1, Credentials: pwdHashed, CreatedAt: *now()},
&types.Credentials{ID: 2, Credentials: pwdUnknown},
&types.Credentials{ID: 3, Credentials: ""},
},
},
{
name: "present, invalid, last",
rval: true,
password: pwdPlain,
cc: types.CredentialsSet{
&types.Credentials{ID: 2, Credentials: pwdUnknown},
&types.Credentials{ID: 3, Credentials: ""},
&types.Credentials{ID: 1, Credentials: pwdHashed, DeletedAt: now()},
},
},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
var (
req = require.New(t)
rsp = isPasswordReused(c.cc, c.password, c.window)
)
if c.rval {
req.True(rsp)
} else {
req.False(rsp)
}
})
}
}
func TestValidateToken(t *testing.T) {
type args struct {
token string
}
tests := []struct {
name string
args args
wantID uint64
wantCredentials string
}{
{
name: "empty",
wantID: 0,
wantCredentials: "",
args: args{token: ""}},
{
name: "foo",
wantID: 0,
wantCredentials: "",
args: args{token: "foo1"}},
{
name: "semivalid",
wantID: 0,
wantCredentials: "",
args: args{token: "foofoofoofoofoofoofoofoofoofoofo0"}},
{
name: "valid",
wantID: 1,
wantCredentials: "foofoofoofoofoofoofoofoofoofoofo",
args: args{token: "foofoofoofoofoofoofoofoofoofoofo1"}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotID, gotCredentials := validateToken(tt.args.token)
if gotID != tt.wantID {
t.Errorf("auth.validateToken() gotID = %v, want %v", gotID, tt.wantID)
}
if gotCredentials != tt.wantCredentials {
t.Errorf("auth.validateToken() gotCredentials = %v, want %v", gotCredentials, tt.wantCredentials)
}
})
}
}
-106
View File
@@ -14,7 +14,6 @@ import (
"github.com/markbates/goth"
"github.com/stretchr/testify/require"
"go.uber.org/zap"
"golang.org/x/crypto/bcrypt"
)
// Mock auth service with nil for current time, dummy provider validator and mock db
@@ -392,108 +391,3 @@ func TestAuth_multiCreateUserTokenForPasswordReset(t *testing.T) {
}
}
func Test_auth_checkPassword(t *testing.T) {
plainPassword := " ... plain password ... "
hashedPassword, _ := bcrypt.GenerateFromPassword([]byte(plainPassword), bcrypt.DefaultCost)
type args struct {
password string
cc types.CredentialsSet
}
tests := []struct {
name string
args args
rval bool
}{
{
name: "empty set",
rval: false,
args: args{}},
{
name: "bad pwd",
rval: false,
args: args{
password: " foo ",
cc: types.CredentialsSet{&types.Credentials{ID: 1, Credentials: string(hashedPassword)}}}},
{
name: "invalid credentials",
rval: false,
args: args{
password: " foo ",
cc: types.CredentialsSet{&types.Credentials{ID: 0, Credentials: string(hashedPassword)}}}},
{
name: "ok",
rval: true,
args: args{
password: plainPassword,
cc: types.CredentialsSet{&types.Credentials{ID: 1, Credentials: string(hashedPassword)}}}},
{
name: "multipass",
rval: true,
args: args{
password: plainPassword,
cc: types.CredentialsSet{
&types.Credentials{ID: 0, Credentials: string(hashedPassword)},
&types.Credentials{ID: 1, Credentials: "$2a$10$8sOZxfZinxnu3bAtpkqEx.wBBwOfci6aG1szgUyxm5.BL2WiLu.ni"},
&types.Credentials{ID: 2, Credentials: string(hashedPassword)},
&types.Credentials{ID: 3, Credentials: ""},
}}},
}
svc := auth{
settings: &types.AppSettings{},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.rval != svc.CheckPassword(tt.args.password, true, tt.args.cc) {
t.Errorf("auth.checkPassword() expecting rval to be %v", tt.rval)
}
})
}
}
func TestValidateToken(t *testing.T) {
type args struct {
token string
}
tests := []struct {
name string
args args
wantID uint64
wantCredentials string
}{
{
name: "empty",
wantID: 0,
wantCredentials: "",
args: args{token: ""}},
{
name: "foo",
wantID: 0,
wantCredentials: "",
args: args{token: "foo1"}},
{
name: "semivalid",
wantID: 0,
wantCredentials: "",
args: args{token: "foofoofoofoofoofoofoofoofoofoofo0"}},
{
name: "valid",
wantID: 1,
wantCredentials: "foofoofoofoofoofoofoofoofoofoofo",
args: args{token: "foofoofoofoofoofoofoofoofoofoofo1"}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotID, gotCredentials := validateToken(tt.args.token)
if gotID != tt.wantID {
t.Errorf("auth.validateToken() gotID = %v, want %v", gotID, tt.wantID)
}
if gotCredentials != tt.wantCredentials {
t.Errorf("auth.validateToken() gotCredentials = %v, want %v", gotCredentials, tt.wantCredentials)
}
})
}
}
+8 -15
View File
@@ -52,7 +52,6 @@ type (
userAuth interface {
CheckPasswordStrength(string) bool
CheckPassword(string, bool, types.CredentialsSet) bool
SetPasswordCredentials(context.Context, uint64, string) error
RemovePasswordCredentials(context.Context, uint64) error
RemoveAccessTokens(context.Context, *types.User) error
@@ -687,8 +686,7 @@ func (svc user) Unsuspend(ctx context.Context, userID uint64) (err error) {
// Expecting setter to have permissions to update users
func (svc user) SetPassword(ctx context.Context, userID uint64, newPassword string) (err error) {
var (
u *types.User
cc types.CredentialsSet
u *types.User
uaProps = &userActionProps{user: &types.User{ID: userID}}
a = UserActionSetPassword
@@ -718,18 +716,13 @@ func (svc user) SetPassword(ctx context.Context, userID uint64, newPassword stri
return svc.auth.RemovePasswordCredentials(ctx, userID)
}
cc, _, err = store.SearchCredentials(ctx, svc.store, types.CredentialsFilter{
Kind: credentialsTypePassword,
OwnerID: userID,
Deleted: filter.StateInclusive})
if err != nil {
return err
}
if svc.auth.CheckPassword(newPassword, false, cc) {
return AuthErrPasswordSetFailedReusedPasswordCheckFailed()
}
// note on password reuse:
//
// we do not really care if user is setting same password
// to someone else (or to self for that matter)
//
// he has rights to update the user and is doing so
// through general user management API
if !svc.auth.CheckPasswordStrength(newPassword) {
return UserErrPasswordNotSecure()
-20
View File
@@ -5,7 +5,6 @@ import (
"github.com/cortezaproject/corteza-server/pkg/filter"
"github.com/jmoiron/sqlx/types"
"golang.org/x/crypto/bcrypt"
)
type (
@@ -34,22 +33,3 @@ type (
func (u *Credentials) Valid() bool {
return u.ID > 0 && (u.ExpiresAt == nil || u.ExpiresAt.After(time.Now())) && u.DeletedAt == nil
}
// CompareHashAndPassword returns first valid credentials with matching hash
func (cc CredentialsSet) CompareHashAndPassword(password string, validOnly bool) *Credentials {
for _, c := range cc {
if validOnly && !c.Valid() {
continue
}
if len(c.Credentials) == 0 {
continue
}
if bcrypt.CompareHashAndPassword([]byte(c.Credentials), []byte(password)) == nil {
return c
}
}
return nil
}