Forbid password reuse
This commit is contained in:
@@ -38,7 +38,7 @@ func (h *AuthHandlers) changePasswordProc(req *request.AuthReq) (err error) {
|
||||
case service.AuthErrInternalLoginDisabledByConfig().Is(err),
|
||||
service.AuthErrPasswordNotSecure().Is(err),
|
||||
service.AuthErrPasswordChangeFailedForUnknownUser().Is(err),
|
||||
service.AuthErrPasswodResetFailedOldPasswordCheckFailed().Is(err):
|
||||
service.AuthErrPasswordResetFailedOldPasswordCheckFailed().Is(err):
|
||||
req.SetKV(map[string]string{
|
||||
"error": err.Error(),
|
||||
})
|
||||
|
||||
@@ -102,7 +102,7 @@ func Test_changePasswordProc(t *testing.T) {
|
||||
fn: func(_ *settings.Settings) {
|
||||
authService = &authServiceMocked{
|
||||
changePassword: func(ctx context.Context, userID uint64, oldPassword, newPassword string) (err error) {
|
||||
return service.AuthErrPasswodResetFailedOldPasswordCheckFailed()
|
||||
return service.AuthErrPasswordResetFailedOldPasswordCheckFailed()
|
||||
},
|
||||
}
|
||||
},
|
||||
|
||||
+34
-8
@@ -352,7 +352,7 @@ func (svc auth) InternalSignUp(ctx context.Context, input *types.User, password
|
||||
return err
|
||||
}
|
||||
|
||||
if c = cc.CompareHashAndPassword(password); c == nil {
|
||||
if c = cc.CompareHashAndPassword(password, true); c == nil {
|
||||
return AuthErrInvalidCredentials(aam)
|
||||
}
|
||||
|
||||
@@ -491,7 +491,7 @@ func (svc auth) InternalLogin(ctx context.Context, email string, password string
|
||||
return err
|
||||
}
|
||||
|
||||
c := cc.CompareHashAndPassword(password)
|
||||
c := cc.CompareHashAndPassword(password, true)
|
||||
if c == nil {
|
||||
return AuthErrInvalidCredentials(aam)
|
||||
}
|
||||
@@ -506,8 +506,8 @@ func (svc auth) InternalLogin(ctx context.Context, email string, password string
|
||||
}
|
||||
|
||||
// checkPassword returns true if given (encrypted) password matches any of the credentials
|
||||
func (svc auth) checkPassword(password string, cc types.CredentialSet) bool {
|
||||
return cc.CompareHashAndPassword(password) != nil
|
||||
func (svc auth) CheckPassword(password string, validOnly bool, cc types.CredentialSet) bool {
|
||||
return cc.CompareHashAndPassword(password, validOnly) != nil
|
||||
}
|
||||
|
||||
// SetPassword sets new password for a user
|
||||
@@ -515,7 +515,8 @@ func (svc auth) checkPassword(password string, cc types.CredentialSet) bool {
|
||||
// This function also records an action
|
||||
func (svc auth) SetPassword(ctx context.Context, userID uint64, password string) (err error) {
|
||||
var (
|
||||
u *types.User
|
||||
u *types.User
|
||||
cc types.CredentialSet
|
||||
|
||||
aam = &authActionProps{
|
||||
user: u,
|
||||
@@ -540,6 +541,23 @@ func (svc auth) SetPassword(ctx context.Context, userID uint64, password string)
|
||||
aam.setUser(u)
|
||||
ctx = internalAuth.SetIdentityToContext(ctx, u)
|
||||
|
||||
cc, _, err = store.SearchCredentials(ctx, svc.store, types.CredentialFilter{
|
||||
Kind: credentialsTypePassword,
|
||||
OwnerID: userID,
|
||||
Deleted: filter.StateInclusive})
|
||||
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if svc.CheckPassword(password, true, cc) {
|
||||
return AuthErrPasswordResetFailedOldPasswordCheckFailed(aam)
|
||||
}
|
||||
|
||||
if svc.CheckPassword(password, false, cc) {
|
||||
return AuthErrPasswordSetFailedReusedPasswordCheckFailed(aam)
|
||||
}
|
||||
|
||||
if err != svc.SetPasswordCredentials(ctx, userID, password) {
|
||||
return err
|
||||
}
|
||||
@@ -606,13 +624,21 @@ func (svc auth) ChangePassword(ctx context.Context, userID uint64, oldPassword,
|
||||
aam.setUser(u)
|
||||
ctx = internalAuth.SetIdentityToContext(ctx, u)
|
||||
|
||||
cc, _, err = store.SearchCredentials(ctx, svc.store, types.CredentialFilter{Kind: credentialsTypePassword, OwnerID: userID})
|
||||
cc, _, err = store.SearchCredentials(ctx, svc.store, types.CredentialFilter{
|
||||
Kind: credentialsTypePassword,
|
||||
OwnerID: userID,
|
||||
Deleted: filter.StateInclusive})
|
||||
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if !svc.checkPassword(oldPassword, cc) {
|
||||
return AuthErrPasswodResetFailedOldPasswordCheckFailed(aam)
|
||||
if !svc.CheckPassword(oldPassword, true, cc) {
|
||||
return AuthErrPasswordResetFailedOldPasswordCheckFailed(aam)
|
||||
}
|
||||
|
||||
if svc.CheckPassword(newPassword, false, cc) {
|
||||
return AuthErrPasswordSetFailedReusedPasswordCheckFailed(aam)
|
||||
}
|
||||
|
||||
if err != svc.SetPasswordCredentials(ctx, userID, newPassword) {
|
||||
|
||||
Generated
+38
-4
@@ -1098,12 +1098,12 @@ func AuthErrPasswordChangeFailedForUnknownUser(mm ...*authActionProps) *errors.E
|
||||
return e
|
||||
}
|
||||
|
||||
// AuthErrPasswodResetFailedOldPasswordCheckFailed returns "system:auth.passwodResetFailedOldPasswordCheckFailed" as *errors.Error
|
||||
// AuthErrPasswordResetFailedOldPasswordCheckFailed returns "system:auth.passwordResetFailedOldPasswordCheckFailed" as *errors.Error
|
||||
//
|
||||
//
|
||||
// This function is auto-generated.
|
||||
//
|
||||
func AuthErrPasswodResetFailedOldPasswordCheckFailed(mm ...*authActionProps) *errors.Error {
|
||||
func AuthErrPasswordResetFailedOldPasswordCheckFailed(mm ...*authActionProps) *errors.Error {
|
||||
var p = &authActionProps{}
|
||||
if len(mm) > 0 {
|
||||
p = mm[0]
|
||||
@@ -1114,14 +1114,48 @@ func AuthErrPasswodResetFailedOldPasswordCheckFailed(mm ...*authActionProps) *er
|
||||
|
||||
p.Format("failed to change password, old password does not match", nil),
|
||||
|
||||
errors.Meta("type", "passwodResetFailedOldPasswordCheckFailed"),
|
||||
errors.Meta("type", "passwordResetFailedOldPasswordCheckFailed"),
|
||||
errors.Meta("resource", "system:auth"),
|
||||
|
||||
errors.Meta(authPropsMetaKey{}, p),
|
||||
|
||||
// translation namespace & key
|
||||
errors.Meta(locale.ErrorMetaNamespace{}, "system"),
|
||||
errors.Meta(locale.ErrorMetaKey{}, "auth.errors.passwodResetFailedOldPasswordCheckFailed"),
|
||||
errors.Meta(locale.ErrorMetaKey{}, "auth.errors.passwordResetFailedOldPasswordCheckFailed"),
|
||||
|
||||
errors.StackSkip(1),
|
||||
)
|
||||
|
||||
if len(mm) > 0 {
|
||||
}
|
||||
|
||||
return e
|
||||
}
|
||||
|
||||
// AuthErrPasswordSetFailedReusedPasswordCheckFailed returns "system:auth.passwordSetFailedReusedPasswordCheckFailed" as *errors.Error
|
||||
//
|
||||
//
|
||||
// This function is auto-generated.
|
||||
//
|
||||
func AuthErrPasswordSetFailedReusedPasswordCheckFailed(mm ...*authActionProps) *errors.Error {
|
||||
var p = &authActionProps{}
|
||||
if len(mm) > 0 {
|
||||
p = mm[0]
|
||||
}
|
||||
|
||||
var e = errors.New(
|
||||
errors.KindInternal,
|
||||
|
||||
p.Format("failed to set password, already used", nil),
|
||||
|
||||
errors.Meta("type", "passwordSetFailedReusedPasswordCheckFailed"),
|
||||
errors.Meta("resource", "system:auth"),
|
||||
|
||||
errors.Meta(authPropsMetaKey{}, p),
|
||||
|
||||
// translation namespace & key
|
||||
errors.Meta(locale.ErrorMetaNamespace{}, "system"),
|
||||
errors.Meta(locale.ErrorMetaKey{}, "auth.errors.passwordSetFailedReusedPasswordCheckFailed"),
|
||||
|
||||
errors.StackSkip(1),
|
||||
)
|
||||
|
||||
@@ -130,9 +130,12 @@ errors:
|
||||
- error: passwordChangeFailedForUnknownUser
|
||||
message: "failed to change password for the unknown user"
|
||||
|
||||
- error: passwodResetFailedOldPasswordCheckFailed
|
||||
- error: passwordResetFailedOldPasswordCheckFailed
|
||||
message: "failed to change password, old password does not match"
|
||||
|
||||
- error: passwordSetFailedReusedPasswordCheckFailed
|
||||
message: "failed to set password, already used"
|
||||
|
||||
- error: passwordCreateFailedForUnknownUser
|
||||
message: "failed to create password for the unknown user"
|
||||
|
||||
|
||||
@@ -446,7 +446,7 @@ func Test_auth_checkPassword(t *testing.T) {
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if tt.rval != svc.checkPassword(tt.args.password, tt.args.cc) {
|
||||
if tt.rval != svc.CheckPassword(tt.args.password, true, tt.args.cc) {
|
||||
t.Errorf("auth.checkPassword() expecting rval to be %v", tt.rval)
|
||||
}
|
||||
})
|
||||
|
||||
+21
-5
@@ -52,6 +52,7 @@ type (
|
||||
|
||||
userAuth interface {
|
||||
CheckPasswordStrength(string) bool
|
||||
CheckPassword(string, bool, types.CredentialSet) bool
|
||||
SetPasswordCredentials(context.Context, uint64, string) error
|
||||
RemovePasswordCredentials(context.Context, uint64) error
|
||||
RemoveAccessTokens(context.Context, *types.User) error
|
||||
@@ -686,7 +687,9 @@ 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
|
||||
u *types.User
|
||||
cc types.CredentialSet
|
||||
|
||||
uaProps = &userActionProps{user: &types.User{ID: userID}}
|
||||
a = UserActionSetPassword
|
||||
)
|
||||
@@ -698,14 +701,14 @@ func (svc user) SetPassword(ctx context.Context, userID uint64, newPassword stri
|
||||
|
||||
uaProps.setUser(u)
|
||||
|
||||
if u.Kind == types.SystemUser {
|
||||
return UserErrNotAllowedToUpdateSystem()
|
||||
}
|
||||
|
||||
if !svc.ac.CanUpdateUser(ctx, u) {
|
||||
return UserErrNotAllowedToUpdate()
|
||||
}
|
||||
|
||||
if u.Kind == types.SystemUser {
|
||||
return UserErrNotAllowedToUpdateSystem()
|
||||
}
|
||||
|
||||
if err = svc.auth.RemoveAccessTokens(ctx, u); err != nil {
|
||||
return
|
||||
}
|
||||
@@ -715,6 +718,19 @@ 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.CredentialFilter{
|
||||
Kind: credentialsTypePassword,
|
||||
OwnerID: userID,
|
||||
Deleted: filter.StateInclusive})
|
||||
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if svc.auth.CheckPassword(newPassword, false, cc) {
|
||||
return AuthErrPasswordSetFailedReusedPasswordCheckFailed()
|
||||
}
|
||||
|
||||
if !svc.auth.CheckPasswordStrength(newPassword) {
|
||||
return UserErrPasswordNotSecure()
|
||||
}
|
||||
|
||||
@@ -37,10 +37,9 @@ func (u *Credential) Valid() bool {
|
||||
}
|
||||
|
||||
// CompareHashAndPassword returns first valid credentials with matching hash
|
||||
func (set CredentialSet) CompareHashAndPassword(password string) *Credential {
|
||||
// We need only valid credentials (skip deleted, expired)
|
||||
for _, c := range set {
|
||||
if !c.Valid() {
|
||||
func (cc CredentialSet) CompareHashAndPassword(password string, validOnly bool) *Credential {
|
||||
for _, c := range cc {
|
||||
if validOnly && !c.Valid() {
|
||||
continue
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user