Bug fix: Include {CRYPT} in password hash so it'll decode in LDAP. Re…

…move superfluous parameter in CreateAccount and CreateSubscriber. Debug logging
robarchibald · Feb 5, 2017 · c73b58f · c73b58f
1 parent a80212f
commit c73b58f
Show file tree

Hide file tree

Showing 9 changed files with 36 additions and 30 deletions.
diff --git a/authStore.go b/authStore.go
@@ -345,7 +345,7 @@ func (s *authStore) createProfile(fullName, organization, password, picturePath
 		return newLoggedError("Error while creating profile", err)
 	}
 
-	_, err = s.createLogin(session.UserID, session.UserID, session.Email, fullName, password, mailQuota, fileQuota)
+	_, err = s.createLogin(session.UserID, session.Email, fullName, password, mailQuota, fileQuota)
 	if err != nil {
 		return newLoggedError("Unable to create login", err)
 	}
@@ -360,26 +360,26 @@ func (s *authStore) createProfile(fullName, organization, password, picturePath
 }
 
 /****************  TODO: send 0 for UID and GID numbers and empty quotas if mailQuota and fileQuota are 0 **********************/
-func (s *authStore) createLogin(userID, dbUserID int, email, fullName, password string, mailQuota, fileQuota int) (*userLogin, error) {
+func (s *authStore) createLogin(dbUserID int, email, fullName, password string, mailQuota, fileQuota int) (*userLogin, error) {
 	passwordHash, err := s.p.Hash(password)
 	if err != nil {
 		return nil, newLoggedError("Unable to create login", err)
 	}
 	if mailQuota == 0 || fileQuota == 0 {
-		return s.createAccount(userID, dbUserID, email, fullName, password)
+		return s.createAccount(dbUserID, email, fullName, password)
 	}
-	return s.createSubscriber(userID, dbUserID, email, fullName, passwordHash, mailQuota, fileQuota)
+	return s.createSubscriber(dbUserID, email, fullName, passwordHash, mailQuota, fileQuota)
 }
 
-func (s *authStore) createAccount(userID, dbUserID int, email, fullName, passwordHash string) (*userLogin, error) {
-	login, err := s.backend.CreateAccount(userID, dbUserID, email, passwordHash, fullName)
+func (s *authStore) createAccount(dbUserID int, email, fullName, passwordHash string) (*userLogin, error) {
+	login, err := s.backend.CreateAccount(dbUserID, email, passwordHash, fullName)
 	if err != nil {
 		return nil, newLoggedError("Unable to create account", err)
 	}
 	return login, nil
 }
 
-func (s *authStore) createSubscriber(userID, dbUserID int, email, fullName, passwordHash string, mailQuota, fileQuota int) (*userLogin, error) {
+func (s *authStore) createSubscriber(dbUserID int, email, fullName, passwordHash string, mailQuota, fileQuota int) (*userLogin, error) {
 	uidNumber := 10000 // vmail user
 	gidNumber := 10000 // vmail user
 	sepIndex := strings.Index(email, "@")
@@ -392,7 +392,7 @@ func (s *authStore) createSubscriber(userID, dbUserID int, email, fullName, pass
 	mQuota := fmt.Sprintf("%dGB", mailQuota)
 	fQuota := fmt.Sprintf("%dGB", fileQuota)
 
-	login, err := s.backend.CreateSubscriber(userID, dbUserID, email, passwordHash, fullName, homeDirectory, uidNumber, gidNumber, mQuota, fQuota)
+	login, err := s.backend.CreateSubscriber(dbUserID, email, passwordHash, fullName, homeDirectory, uidNumber, gidNumber, mQuota, fQuota)
 	if err != nil {
 		return nil, newLoggedError("Unable to create login", err)
 	}

diff --git a/backend.go b/backend.go
@@ -27,8 +27,8 @@ type backender interface {
 	UpdateUser(userID int, fullname string, company string, pictureURL string) error
 
 	// LoginBackender. Write out since it contains duplicate BackendCloser
-	CreateAccount(userID, dbUserID int, email, passwordHash, fullName string) (*userLogin, error)
-	CreateSubscriber(userID, dbUserID int, email, passwordHash, fullName, homeDirectory string, uidNumber, gidNumber int, mailQuota, fileQuota string) (*userLogin, error)
+	CreateAccount(dbUserID int, email, passwordHash, fullName string) (*userLogin, error)
+	CreateSubscriber(dbUserID int, email, passwordHash, fullName, homeDirectory string, uidNumber, gidNumber int, mailQuota, fileQuota string) (*userLogin, error)
 	Login(email, password string) (*userLogin, error)
 	UpdateEmail(email string, password string, newEmail string) (*loginSession, error)
 	UpdatePassword(email string, oldPassword string, newPassword string) (*loginSession, error)
@@ -48,8 +48,8 @@ type userBackender interface {
 }
 
 type loginBackender interface {
-	CreateAccount(userID, dbUserID int, email, passwordHash, fullName string) (*userLogin, error)
-	CreateSubscriber(userID, dbUserID int, email, passwordHash, fullName, homeDirectory string, uidNumber, gidNumber int, mailQuota, fileQuota string) (*userLogin, error)
+	CreateAccount(dbUserID int, email, passwordHash, fullName string) (*userLogin, error)
+	CreateSubscriber(dbUserID int, email, passwordHash, fullName, homeDirectory string, uidNumber, gidNumber int, mailQuota, fileQuota string) (*userLogin, error)
 	Login(email, password string) (*userLogin, error)
 	UpdateEmail(email string, password string, newEmail string) (*loginSession, error)
 	UpdatePassword(email string, oldPassword string, newPassword string) (*loginSession, error)
@@ -215,12 +215,12 @@ func (b *backend) UpdateUser(userID int, fullname string, company string, pictur
 	return b.u.UpdateUser(userID, fullname, company, pictureURL)
 }
 
-func (b *backend) CreateAccount(userID, dbUserID int, email, passwordHash, fullName string) (*userLogin, error) {
-	return b.l.CreateAccount(userID, dbUserID, email, passwordHash, fullName)
+func (b *backend) CreateAccount(dbUserID int, email, passwordHash, fullName string) (*userLogin, error) {
+	return b.l.CreateAccount(dbUserID, email, passwordHash, fullName)
 }
 
-func (b *backend) CreateSubscriber(userID, dbUserID int, email, passwordHash, fullName, homeDirectory string, uidNumber, gidNumber int, mailQuota, fileQuota string) (*userLogin, error) {
-	return b.l.CreateSubscriber(userID, dbUserID, email, passwordHash, fullName, homeDirectory, uidNumber, gidNumber, mailQuota, fileQuota)
+func (b *backend) CreateSubscriber(dbUserID int, email, passwordHash, fullName, homeDirectory string, uidNumber, gidNumber int, mailQuota, fileQuota string) (*userLogin, error) {
+	return b.l.CreateSubscriber(dbUserID, email, passwordHash, fullName, homeDirectory, uidNumber, gidNumber, mailQuota, fileQuota)
 }
 
 func (b *backend) UpdateEmail(email string, password string, newEmail string) (*loginSession, error) {

diff --git a/backendLDAPLogin.go b/backendLDAPLogin.go
@@ -48,7 +48,7 @@ func (l *backendLDAPLogin) Login(email, password string) (*userLogin, error) {
 }
 
 /****************  TODO: create different type of user if not using file and mail quotas  **********************/
-func (l *backendLDAPLogin) CreateAccount(userID, dbUserID int, email, passwordHash, fullName string) (*userLogin, error) {
+func (l *backendLDAPLogin) CreateAccount(dbUserID int, email, passwordHash, fullName string) (*userLogin, error) {
 	req := ldap.NewAddRequest("uid=" + email + "," + l.baseDn)
 	req.Attribute("objectClass", []string{"endfirstAccount"})
 	req.Attribute("uid", []string{email})
@@ -59,7 +59,7 @@ func (l *backendLDAPLogin) CreateAccount(userID, dbUserID int, email, passwordHa
 	return &userLogin{}, err
 }
 
-func (l *backendLDAPLogin) CreateSubscriber(userID, dbUserID int, email, passwordHash, fullName, homeDirectory string, uidNumber, gidNumber int, mailQuota, fileQuota string) (*userLogin, error) {
+func (l *backendLDAPLogin) CreateSubscriber(dbUserID int, email, passwordHash, fullName, homeDirectory string, uidNumber, gidNumber int, mailQuota, fileQuota string) (*userLogin, error) {
 	req := ldap.NewAddRequest("uid=" + email + "," + l.baseDn)
 	req.Attribute("objectClass", []string{"endfirstAccount", "endfirstSubscriber"})
 	req.Attribute("uid", []string{email})

diff --git a/backendLDAPLogin_test.go b/backendLDAPLogin_test.go
@@ -59,7 +59,7 @@ func TestLdapLogin(t *testing.T) {
 func TestLdapCreateSubscriber(t *testing.T) {
 	m := onedb.NewMock(nil, nil, nil)
 	l := backendLDAPLogin{db: m}
-	_, err := l.CreateSubscriber(1, 1, "email", "hash", "name", "homeDir", 1, 1, "mailQuota", "fileQuota")
+	_, err := l.CreateSubscriber(1, "email", "hash", "name", "homeDir", 1, 1, "mailQuota", "fileQuota")
 	if err != nil {
 		t.Error("expected success")
 	}

diff --git a/backendMemory.go b/backendMemory.go
@@ -166,18 +166,18 @@ func (m *backendMemory) UpdateUser(userID int, fullname string, company string,
 	return nil
 }
 
-func (m *backendMemory) CreateAccount(userID, dbUserID int, email, passwordHash, fullName string) (*userLogin, error) {
-	login := userLoginMemory{userID, email, fullName, passwordHash}
+func (m *backendMemory) CreateAccount(dbUserID int, email, passwordHash, fullName string) (*userLogin, error) {
+	login := userLoginMemory{dbUserID, email, fullName, passwordHash}
 	m.Logins = append(m.Logins, &login)
 
-	return &userLogin{userID, email, fullName}, nil
+	return &userLogin{dbUserID, email, fullName}, nil
 }
 
-func (m *backendMemory) CreateSubscriber(userID, dbUserID int, email, passwordHash, fullName, homeDirectory string, uidNumber, gidNumber int, mailQuota, fileQuota string) (*userLogin, error) {
-	login := userLoginMemory{userID, email, fullName, passwordHash}
+func (m *backendMemory) CreateSubscriber(dbUserID int, email, passwordHash, fullName, homeDirectory string, uidNumber, gidNumber int, mailQuota, fileQuota string) (*userLogin, error) {
+	login := userLoginMemory{dbUserID, email, fullName, passwordHash}
 	m.Logins = append(m.Logins, &login)
 
-	return &userLogin{userID, email, fullName}, nil
+	return &userLogin{dbUserID, email, fullName}, nil
 }
 
 func (m *backendMemory) UpdateEmail(email string, password string, newEmail string) (*loginSession, error) {

diff --git a/backendMemory_test.go b/backendMemory_test.go
@@ -153,7 +153,7 @@ func TestMemoryUpdateUser(t *testing.T) {
 
 func TestMemoryCreateSubscriber(t *testing.T) {
 	backend := newBackendMemory(&hashStore{}).(*backendMemory)
-	if login, err := backend.CreateSubscriber(1, 1, "email", "passwordHash", "fullName", "homeDirectory", 1, 1, "mailQuota", "fileQuota"); err != nil || login.Email != "email" {
+	if login, err := backend.CreateSubscriber(1, "email", "passwordHash", "fullName", "homeDirectory", 1, 1, "mailQuota", "fileQuota"); err != nil || login.Email != "email" {
 		t.Error("expected valid login", login)
 	}
 }

diff --git a/backend_test.go b/backend_test.go
@@ -102,7 +102,7 @@ func TestBackendUpdateUser(t *testing.T) {
 func TestBackendCreateSubscriber(t *testing.T) {
 	m := &mockBackend{CreateLoginReturn: loginErr()}
 	b := backend{u: m, l: m, s: m}
-	b.CreateSubscriber(1, 1, "email", "hash", "name", "homeDir", 1, 1, "quota", "fileQuota")
+	b.CreateSubscriber(1, "email", "hash", "name", "homeDir", 1, 1, "quota", "fileQuota")
 	if len(m.MethodsCalled) != 1 || m.MethodsCalled[0] != "CreateSubscriber" {
 		t.Error("Expected it would call backend", m.MethodsCalled)
 	}
@@ -331,15 +331,15 @@ func (b *mockBackend) UpdateUser(userID int, fullname, company, pictureURL strin
 	return b.ErrReturn
 }
 
-func (b *mockBackend) CreateAccount(userID, dbUserID int, email, passwordHash, fullName string) (*userLogin, error) {
+func (b *mockBackend) CreateAccount(dbUserID int, email, passwordHash, fullName string) (*userLogin, error) {
 	b.MethodsCalled = append(b.MethodsCalled, "CreateAccount")
 	if b.CreateLoginReturn == nil {
 		return nil, errors.New("CreateLoginReturn not initialized")
 	}
 	return b.CreateLoginReturn.Login, b.CreateLoginReturn.Err
 }
 
-func (b *mockBackend) CreateSubscriber(userID, dbUserID int, email, passwordHash, fullName, homeDirectory string, uidNumber, gidNumber int, mailQuota, fileQuota string) (*userLogin, error) {
+func (b *mockBackend) CreateSubscriber(dbUserID int, email, passwordHash, fullName, homeDirectory string, uidNumber, gidNumber int, mailQuota, fileQuota string) (*userLogin, error) {
 	b.MethodsCalled = append(b.MethodsCalled, "CreateSubscriber")
 	if b.CreateLoginReturn == nil {
 		return nil, errors.New("CreateLoginReturn not initialized")

diff --git a/cryptoStore.go b/cryptoStore.go
@@ -159,5 +159,5 @@ func cryptoHashWSalt(in, salt string) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	return hash, nil
+	return "{CRYPT}" + hash, nil
 }
diff --git a/nginxauth.go b/nginxauth.go
@@ -11,7 +11,9 @@ import (
 	"net/http"
 	"os"
 	"path"
+	"reflect"
 	"strings"
+	"time"
 )
 
 type authConf struct {
@@ -174,17 +176,20 @@ func (s *nginxauth) fileLoggerHandler(h http.Handler) http.Handler {
 
 func (s *nginxauth) method(name string, handler func(authStore authStorer, w http.ResponseWriter, r *http.Request)) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
+		starttime := time.Now()
 		if r.Method != name {
 			http.Error(w, "Unsupported method", http.StatusInternalServerError)
 			return
 		}
 		secureOnly := strings.HasPrefix(r.Referer(), "https") // proxy to back-end so if referer is secure connection, we can use secureOnly cookies
 		authStore := newAuthStore(s.backend, s.mailer, &cryptoHashStore{}, w, r, s.conf.StoragePrefix, s.cookieKey, secureOnly)
 		handler(authStore, w, r)
+		log.Println("finished with "+reflect.TypeOf(handler).Name(), time.Since(starttime))
 	}
 }
 
 func auth(authStore authStorer, w http.ResponseWriter, r *http.Request) {
+	starttime := time.Now()
 	session, err := authStore.GetSession()
 	if err != nil {
 		authErr(w, r, err)
@@ -198,6 +203,7 @@ func auth(authStore authStorer, w http.ResponseWriter, r *http.Request) {
 	}
 
 	addUserHeader(string(user), w)
+	log.Println("auth done", time.Since(starttime))
 }
 
 func authErr(w http.ResponseWriter, r *http.Request, err error) {