Commit 621da480 authored by ale's avatar ale

Enable authentication of privileged requests

Check passwords and recovery responses directly against backend data.
parent 59753502
Pipeline #1021 failed with stages
in 28 seconds
......@@ -173,12 +173,23 @@ func (s *AccountService) RecoverPassword(ctx context.Context, tx TX, req *Passwo
if err != nil {
return err
}
// TODO: authenticate with the secret recovery password.
// Authenticate the secret recovery password.
if !pwhash.ComparePassword(tx.GetUserRecoveryEncryptedPassword(ctx, user), req.RecoveryPassword) {
return ErrUnauthorized
}
ctx = context.WithValue(ctx, authUserCtxKey, req.Username)
return s.withRequest(ctx, tx, req, user, func(ctx context.Context) error {
s.audit.Log(ctx, ResourceID{}, "password reset via account recovery")
// Change the user password (the recovery password does not change).
return s.changeUserPasswordAndUpdateEncryptionKeys(ctx, tx, user, req.RecoveryPassword, req.Password)
if err := s.changeUserPasswordAndUpdateEncryptionKeys(ctx, tx, user, req.RecoveryPassword, req.Password); err != nil {
return err
}
// Disable 2FA.
return s.disable2FA(ctx, tx, user)
})
}
......
......@@ -5,6 +5,7 @@ import (
"errors"
"testing"
"git.autistici.org/ai3/go-common/pwhash"
sso "git.autistici.org/id/go-sso"
)
......@@ -12,6 +13,7 @@ type fakeBackend struct {
users map[string]*User
resources map[string]map[string]*Resource
passwords map[string]string
recoveryPasswords map[string]string
appSpecificPasswords map[string][]*AppSpecificPasswordInfo
encryptionKeys map[string][]*UserEncryptionKey
}
......@@ -62,9 +64,19 @@ func (b *fakeBackend) SetUserPassword(_ context.Context, user *User, password st
}
func (b *fakeBackend) SetPasswordRecoveryHint(_ context.Context, user *User, hint, response string) error {
b.users[user.Name].PasswordRecoveryHint = hint
b.recoveryPasswords[user.Name] = response
return nil
}
func (b *fakeBackend) GetUserEncryptedPassword(_ context.Context, user *User) string {
return b.passwords[user.Name]
}
func (b *fakeBackend) GetUserRecoveryEncryptedPassword(_ context.Context, user *User) string {
return b.recoveryPasswords[user.Name]
}
func (b *fakeBackend) SetResourcePassword(_ context.Context, r *Resource, password string) error {
b.passwords[r.ID.String()] = password
return nil
......@@ -138,9 +150,13 @@ func (v *fakeValidator) Validate(tkt, nonce, service string, _ []string) (*sso.T
}, nil
}
func (b *fakeBackend) addUser(user *User) {
func (b *fakeBackend) addUser(user *User, pw, rpw string) {
b.users[user.Name] = user
b.resources[user.Name] = make(map[string]*Resource)
b.passwords[user.Name] = pwhash.Encrypt(pw)
if rpw != "" {
b.recoveryPasswords[user.Name] = pwhash.Encrypt(rpw)
}
for _, r := range user.Resources {
b.resources[user.Name][r.ID.String()] = r
}
......@@ -154,6 +170,7 @@ func createFakeBackend() *fakeBackend {
"": make(map[string]*Resource),
},
passwords: make(map[string]string),
recoveryPasswords: make(map[string]string),
appSpecificPasswords: make(map[string][]*AppSpecificPasswordInfo),
encryptionKeys: make(map[string][]*UserEncryptionKey),
}
......@@ -177,7 +194,7 @@ func createFakeBackend() *fakeBackend {
},
},
},
})
}, "password", "recoverypassword")
return fb
}
......@@ -267,6 +284,9 @@ func TestService_ChangePassword(t *testing.T) {
newPassword string
expectedOk bool
}{
// First, fail cur_password authentication.
{"BADPASS", "new_password", false},
// Ordering is important as it is meant to emulate
// setting the password, failing to reset it, then
// succeeding.
......@@ -524,3 +544,27 @@ func TestService_ResetResourcePassword(t *testing.T) {
t.Fatal("oops, it appears that the password was stored in cleartext on the backend")
}
}
func TestService_Recovery(t *testing.T) {
svc, tx := testService("")
// Bad recovery response.
err := svc.RecoverPassword(context.Background(), tx, &PasswordRecoveryRequest{
Username: "testuser",
RecoveryPassword: "BADPASS",
Password: "new_password",
})
if err == nil {
t.Fatal("oops, recovered account with bad password")
}
// Successful account recovery.
err = svc.RecoverPassword(context.Background(), tx, &PasswordRecoveryRequest{
Username: "testuser",
RecoveryPassword: "recoverypassword",
Password: "new_password",
})
if err != nil {
t.Fatalf("RecoverPassword failed: %v", err)
}
}
......@@ -258,6 +258,24 @@ func (tx *backendTX) SetUserPassword(ctx context.Context, user *accountserver.Us
return nil
}
func (tx *backendTX) GetUserEncryptedPassword(ctx context.Context, user *accountserver.User) string {
values := tx.readAttributeValues(ctx, tx.getUserDN(user), passwordLDAPAttr)
if len(values) > 0 {
// Remove legacy LDAP encryption prefix.
return strings.TrimPrefix(values[0], "{crypt}")
}
return ""
}
func (tx *backendTX) GetUserRecoveryEncryptedPassword(ctx context.Context, user *accountserver.User) string {
values := tx.readAttributeValues(ctx, tx.getUserDN(user), recoveryResponseLDAPAttr)
if len(values) > 0 {
// Remove legacy LDAP encryption prefix.
return strings.TrimPrefix(values[0], "{crypt}")
}
return ""
}
func (tx *backendTX) SetPasswordRecoveryHint(ctx context.Context, user *accountserver.User, hint, response string) error {
dn := tx.getUserDN(user)
tx.setAttr(dn, recoveryHintLDAPAttr, hint)
......
......@@ -4,7 +4,7 @@ import "errors"
var (
// ErrUnauthorized means that the request failed due to lack of authorization.
ErrUnauthorized = errors.New("unauthorized")
ErrUnauthorized = newAuthError(errors.New("unauthorized"))
// ErrUserNotFound is returned when a user object is not found.
ErrUserNotFound = errors.New("user not found")
......
......@@ -198,11 +198,7 @@ func TestIntegration_ChangeUserPassword(t *testing.T) {
Username: "uno@investici.org",
SSO: c.ssoTicket("uno@investici.org"),
},
// Since the user in the test fixture does
// *not* have encryption key, and
// authenticateWithPassword is currently a
// no-op, this test will pass.
CurPassword: "ha ha, really not the current password",
CurPassword: "password",
},
Password: "new_password",
}, nil)
......
......@@ -18,9 +18,8 @@ givenName: Private
shadowLastChange: 12345
shadowWarning: 7
preferredLanguage: it
userPassword:: e2NyeXB0fSQ2JG1wVzdTZHZROG52OFJabE8kcFNqbm1hLi9CZDNIWU1hL29sMGZ
FRmZrS3pWRjAxUkkzMnpISEoxNnc2V2xaajFuSFVzMmd4ZXZIVDdoemdELnJ5Lk1BZWM3REptZVZ5
WEtkeDV0QTE=
userPassword:: e2NyeXB0fSQ2JG1wVzdTZHZROG52OFJabE8kSTRmQldkVUpFeVVsb0dpdVpnYm8yNTlVVFpJMi9y
Vk5QODdZU04zVE0ud2F5MmR2UndYNmE0NHVVV2drWC9ac25HOGF3R0RYWFRmMDVNVXhNbGlnSDA=
dn: mail=uno@investici.org,uid=uno@investici.org,ou=People,dc=example,dc=com
status: active
......
......@@ -6,6 +6,7 @@ import (
"log"
"reflect"
"git.autistici.org/ai3/go-common/pwhash"
"git.autistici.org/id/go-sso"
)
......@@ -54,6 +55,13 @@ type TX interface {
DeleteApplicationSpecificPassword(context.Context, *User, string) error
SetUserTOTPSecret(context.Context, *User, string) error
DeleteUserTOTPSecret(context.Context, *User) error
// The Get*EncryptedPassword methods allow us to perform
// authentication within the accountserver, skipping one round
// trip to the auth-server, given that we already have the
// necessary information.
GetUserEncryptedPassword(context.Context, *User) string
GetUserRecoveryEncryptedPassword(context.Context, *User) string
}
// FindResourceRequest contains parameters for searching a resource by name.
......@@ -177,7 +185,7 @@ func (s *authService) authorizeAdminGeneric(ctx context.Context, tx TX, ssoTicke
// Requests are allowed if the SSO ticket corresponds to an admin, or if
// it identifies the same user that we're querying.
if !s.isAdmin(tkt) {
return nil, newAuthError(ErrUnauthorized)
return nil, ErrUnauthorized
}
ctx = context.WithValue(ctx, authUserCtxKey, tkt.User)
......@@ -194,7 +202,7 @@ func (s *authService) authorizeAdmin(ctx context.Context, tx TX, req RequestBase
// Requests are allowed if the SSO ticket corresponds to an admin, or if
// it identifies the same user that we're querying.
if !s.isAdmin(tkt) {
return nil, nil, newAuthError(ErrUnauthorized)
return nil, nil, ErrUnauthorized
}
ctx = context.WithValue(ctx, authUserCtxKey, tkt.User)
......@@ -213,7 +221,7 @@ func (s *authService) authorizeUser(ctx context.Context, tx TX, req RequestBase)
// Requests are allowed if the SSO ticket corresponds to an admin, or if
// it identifies the same user that we're querying.
if !s.isAdmin(tkt) && tkt.User != req.Username {
return nil, nil, newAuthError(ErrUnauthorized)
return nil, nil, ErrUnauthorized
}
user, err := getUserOrDie(ctx, tx, req.Username)
......@@ -225,8 +233,14 @@ func (s *authService) authorizeUser(ctx context.Context, tx TX, req RequestBase)
// user password. Used for account-privileged operations related to
// credential manipulation.
func (s *authService) authorizeUserWithPassword(ctx context.Context, tx TX, req PrivilegedRequestBase) (context.Context, *User, error) {
// TODO: call out to the auth-server?
return s.authorizeUser(ctx, tx, req.RequestBase)
rctx, user, err := s.authorizeUser(ctx, tx, req.RequestBase)
if err != nil {
return nil, nil, err
}
if !pwhash.ComparePassword(tx.GetUserEncryptedPassword(ctx, user), req.CurPassword) {
return nil, nil, ErrUnauthorized
}
return rctx, user, nil
}
// Extended version of authorizeUser that checks access to a specific
......@@ -249,7 +263,7 @@ func (s *authService) authorizeResource(ctx context.Context, tx TX, req Resource
}
if !s.isAdmin(tkt) && !canAccessResource(tkt.User, r) {
return nil, nil, newAuthError(ErrUnauthorized)
return nil, nil, ErrUnauthorized
}
ctx = context.WithValue(ctx, authUserCtxKey, tkt.User)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment