Tests

auth/auth.go

@@ -22,7 +22,7 @@ type Manager interface {
 
 type User struct {
 	Name   string
-	Pass   string // hashed
+	Hash   string // password hash (bcrypt)
 	Role   Role
 	Grants []Grant
 }
@@ -57,6 +57,8 @@ func AllowedRole(role Role) bool {
 }
 
 var (
-	ErrUnauthorized = errors.New("unauthorized")
-	ErrNotFound     = errors.New("not found")
+	ErrUnauthenticated = errors.New("unauthenticated")
+	ErrUnauthorized    = errors.New("unauthorized")
+	ErrInvalidArgument = errors.New("invalid argument")
+	ErrNotFound        = errors.New("not found")
 )

auth/auth_sqlite.go

@@ -2,24 +2,14 @@ package auth
 
 import (
 	"database/sql"
+	_ "github.com/mattn/go-sqlite3" // SQLite driver
 	"golang.org/x/crypto/bcrypt"
+	"regexp"
 )
 
-/*
-
-SELECT * FROM user;
-SELECT * FROM access;
-
-INSERT INTO user VALUES ('phil','$2a$06$.4W0LI5mcxzxhpjUvpTaNeu0MhRO0T7B.CYnmAkRnlztIy7PrSODu', 'admin');
-INSERT INTO user VALUES ('ben','$2a$06$skJK/AecWCUmiCjr69ke.Ow/hFA616RdvJJPxnI221zyohsRlyXL.', 'user');
-INSERT INTO user VALUES ('marian','$2a$10$8U90swQIatvHHI4sw0Wo7.OUy6dUwzMcoOABi6BsS4uF0x3zcSXRW', 'user');
-
-INSERT INTO access VALUES ('ben','alerts',1,1);
-INSERT INTO access VALUES ('marian','alerts',1,0);
-INSERT INTO access VALUES ('','announcements',1,0);
-INSERT INTO access VALUES ('','write-all',1,1);
-
-*/
+const (
+	bcryptCost = 11
+)
 
 // Auther-related queries
 const (
@@ -77,6 +67,10 @@ type SQLiteAuth struct {
 	defaultWrite bool
 }
 
+var (
+	allowedUsernameRegex = regexp.MustCompile(`^[-_.@a-zA-Z0-9]+$`)
+)
+
 var _ Auther = (*SQLiteAuth)(nil)
 var _ Manager = (*SQLiteAuth)(nil)
 
@@ -105,28 +99,18 @@ func setupNewAuthDB(db *sql.DB) error {
 
 func (a *SQLiteAuth) Authenticate(username, password string) (*User, error) {
 	if username == Everyone {
-		return nil, ErrUnauthorized
+		return nil, ErrUnauthenticated
 	}
-	rows, err := a.db.Query(selectUserQuery, username)
+	user, err := a.User(username)
 	if err != nil {
-		return nil, err
+		bcrypt.CompareHashAndPassword([]byte("$2a$11$eX15DeF27FwAgXt9wqJF0uAUMz74XywJcGBH3kP93pzKYv6ATk2ka"),
+			[]byte("intentional slow-down to avoid timing attacks"))
+		return nil, ErrUnauthenticated
 	}
-	defer rows.Close()
-	var hash, role string
-	if rows.Next() {
-		if err := rows.Scan(&hash, &role); err != nil {
-			return nil, err
-		} else if err := rows.Err(); err != nil {
-			return nil, err
-		}
-	}
-	if err := bcrypt.CompareHashAndPassword([]byte(hash), []byte(password)); err != nil {
-		return nil, err
+	if err := bcrypt.CompareHashAndPassword([]byte(user.Hash), []byte(password)); err != nil {
+		return nil, ErrUnauthenticated
 	}
-	return &User{
-		Name: username,
-		Role: Role(role),
-	}, nil
+	return user, nil
 }
 
 func (a *SQLiteAuth) Authorize(user *User, topic string, perm Permission) error {
@@ -167,7 +151,10 @@ func (a *SQLiteAuth) resolvePerms(read, write bool, perm Permission) error {
 }
 
 func (a *SQLiteAuth) AddUser(username, password string, role Role) error {
-	hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost)
+	if !allowedUsernameRegex.MatchString(username) || (role != RoleAdmin && role != RoleUser) {
+		return ErrInvalidArgument
+	}
+	hash, err := bcrypt.GenerateFromPassword([]byte(password), bcryptCost)
 	if err != nil {
 		return err
 	}
@@ -178,6 +165,9 @@ func (a *SQLiteAuth) AddUser(username, password string, role Role) error {
 }
 
 func (a *SQLiteAuth) RemoveUser(username string) error {
+	if !allowedUsernameRegex.MatchString(username) || username == Everyone {
+		return ErrInvalidArgument
+	}
 	if _, err := a.db.Exec(deleteUserQuery, username); err != nil {
 		return err
 	}
@@ -224,18 +214,18 @@ func (a *SQLiteAuth) User(username string) (*User, error) {
 	if username == Everyone {
 		return a.everyoneUser()
 	}
-	urows, err := a.db.Query(selectUserQuery, username)
+	rows, err := a.db.Query(selectUserQuery, username)
 	if err != nil {
 		return nil, err
 	}
-	defer urows.Close()
+	defer rows.Close()
 	var hash, role string
-	if !urows.Next() {
+	if !rows.Next() {
 		return nil, ErrNotFound
 	}
-	if err := urows.Scan(&hash, &role); err != nil {
+	if err := rows.Scan(&hash, &role); err != nil {
 		return nil, err
-	} else if err := urows.Err(); err != nil {
+	} else if err := rows.Err(); err != nil {
 		return nil, err
 	}
 	grants, err := a.readGrants(username)
@@ -244,7 +234,7 @@ func (a *SQLiteAuth) User(username string) (*User, error) {
 	}
 	return &User{
 		Name:   username,
-		Pass:   hash,
+		Hash:   hash,
 		Role:   Role(role),
 		Grants: grants,
 	}, nil
@@ -257,7 +247,7 @@ func (a *SQLiteAuth) everyoneUser() (*User, error) {
 	}
 	return &User{
 		Name:   Everyone,
-		Pass:   "",
+		Hash:   "",
 		Role:   RoleAnonymous,
 		Grants: grants,
 	}, nil
@@ -288,7 +278,7 @@ func (a *SQLiteAuth) readGrants(username string) ([]Grant, error) {
 }
 
 func (a *SQLiteAuth) ChangePassword(username, password string) error {
-	hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost)
+	hash, err := bcrypt.GenerateFromPassword([]byte(password), bcryptCost)
 	if err != nil {
 		return err
 	}
@@ -299,6 +289,9 @@ func (a *SQLiteAuth) ChangePassword(username, password string) error {
 }
 
 func (a *SQLiteAuth) ChangeRole(username string, role Role) error {
+	if !allowedUsernameRegex.MatchString(username) || (role != RoleAdmin && role != RoleUser) {
+		return ErrInvalidArgument
+	}
 	if _, err := a.db.Exec(updateUserRoleQuery, string(role), username); err != nil {
 		return err
 	}

auth/auth_sqlite_test.go

@@ -0,0 +1,238 @@
+package auth_test
+
+import (
+	"github.com/stretchr/testify/require"
+	"heckel.io/ntfy/auth"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+)
+
+func TestSQLiteAuth_FullScenario_Default_DenyAll(t *testing.T) {
+	a := newTestAuth(t, false, false)
+	require.Nil(t, a.AddUser("phil", "phil", auth.RoleAdmin))
+	require.Nil(t, a.AddUser("ben", "ben", auth.RoleUser))
+	require.Nil(t, a.AllowAccess("ben", "mytopic", true, true))
+	require.Nil(t, a.AllowAccess("ben", "readme", true, false))
+	require.Nil(t, a.AllowAccess("ben", "writeme", false, true))
+	require.Nil(t, a.AllowAccess("ben", "everyonewrite", false, false)) // How unfair!
+	require.Nil(t, a.AllowAccess(auth.Everyone, "announcements", true, false))
+	require.Nil(t, a.AllowAccess(auth.Everyone, "everyonewrite", true, true))
+
+	phil, err := a.Authenticate("phil", "phil")
+	require.Nil(t, err)
+	require.Equal(t, "phil", phil.Name)
+	require.True(t, strings.HasPrefix(phil.Hash, "$2a$11$"))
+	require.Equal(t, auth.RoleAdmin, phil.Role)
+	require.Equal(t, []auth.Grant{}, phil.Grants)
+
+	ben, err := a.Authenticate("ben", "ben")
+	require.Nil(t, err)
+	require.Equal(t, "ben", ben.Name)
+	require.True(t, strings.HasPrefix(ben.Hash, "$2a$11$"))
+	require.Equal(t, auth.RoleUser, ben.Role)
+	require.Equal(t, []auth.Grant{
+		{"mytopic", true, true},
+		{"readme", true, false},
+		{"writeme", false, true},
+		{"everyonewrite", false, false},
+	}, ben.Grants)
+
+	notben, err := a.Authenticate("ben", "this is wrong")
+	require.Nil(t, notben)
+	require.Equal(t, auth.ErrUnauthenticated, err)
+
+	// Admin can do everything
+	require.Nil(t, a.Authorize(phil, "sometopic", auth.PermissionWrite))
+	require.Nil(t, a.Authorize(phil, "mytopic", auth.PermissionRead))
+	require.Nil(t, a.Authorize(phil, "readme", auth.PermissionWrite))
+	require.Nil(t, a.Authorize(phil, "writeme", auth.PermissionWrite))
+	require.Nil(t, a.Authorize(phil, "announcements", auth.PermissionWrite))
+	require.Nil(t, a.Authorize(phil, "everyonewrite", auth.PermissionWrite))
+
+	// User cannot do everything
+	require.Nil(t, a.Authorize(ben, "mytopic", auth.PermissionWrite))
+	require.Nil(t, a.Authorize(ben, "mytopic", auth.PermissionRead))
+	require.Nil(t, a.Authorize(ben, "readme", auth.PermissionRead))
+	require.Equal(t, auth.ErrUnauthorized, a.Authorize(ben, "readme", auth.PermissionWrite))
+	require.Equal(t, auth.ErrUnauthorized, a.Authorize(ben, "writeme", auth.PermissionRead))
+	require.Nil(t, a.Authorize(ben, "writeme", auth.PermissionWrite))
+	require.Nil(t, a.Authorize(ben, "writeme", auth.PermissionWrite))
+	require.Equal(t, auth.ErrUnauthorized, a.Authorize(ben, "everyonewrite", auth.PermissionRead))
+	require.Equal(t, auth.ErrUnauthorized, a.Authorize(ben, "everyonewrite", auth.PermissionWrite))
+	require.Nil(t, a.Authorize(ben, "announcements", auth.PermissionRead))
+	require.Equal(t, auth.ErrUnauthorized, a.Authorize(ben, "announcements", auth.PermissionWrite))
+
+	// Everyone else can do barely anything
+	require.Equal(t, auth.ErrUnauthorized, a.Authorize(nil, "sometopicnotinthelist", auth.PermissionRead))
+	require.Equal(t, auth.ErrUnauthorized, a.Authorize(nil, "sometopicnotinthelist", auth.PermissionWrite))
+	require.Equal(t, auth.ErrUnauthorized, a.Authorize(nil, "mytopic", auth.PermissionRead))
+	require.Equal(t, auth.ErrUnauthorized, a.Authorize(nil, "mytopic", auth.PermissionWrite))
+	require.Equal(t, auth.ErrUnauthorized, a.Authorize(nil, "readme", auth.PermissionRead))
+	require.Equal(t, auth.ErrUnauthorized, a.Authorize(nil, "readme", auth.PermissionWrite))
+	require.Equal(t, auth.ErrUnauthorized, a.Authorize(nil, "writeme", auth.PermissionRead))
+	require.Equal(t, auth.ErrUnauthorized, a.Authorize(nil, "writeme", auth.PermissionWrite))
+	require.Equal(t, auth.ErrUnauthorized, a.Authorize(nil, "announcements", auth.PermissionWrite))
+	require.Nil(t, a.Authorize(nil, "announcements", auth.PermissionRead))
+	require.Nil(t, a.Authorize(nil, "everyonewrite", auth.PermissionRead))
+	require.Nil(t, a.Authorize(nil, "everyonewrite", auth.PermissionWrite))
+}
+
+func TestSQLiteAuth_AddUser_Invalid(t *testing.T) {
+	a := newTestAuth(t, false, false)
+	require.Equal(t, auth.ErrInvalidArgument, a.AddUser("  invalid  ", "pass", auth.RoleAdmin))
+	require.Equal(t, auth.ErrInvalidArgument, a.AddUser("validuser", "pass", "invalid-role"))
+}
+
+func TestSQLiteAuth_AddUser_Timing(t *testing.T) {
+	a := newTestAuth(t, false, false)
+	start := time.Now().UnixMilli()
+	require.Nil(t, a.AddUser("user", "pass", auth.RoleAdmin))
+	require.GreaterOrEqual(t, time.Now().UnixMilli()-start, int64(100)) // Ideally should be > 200ms, but let's not make a brittle
+}
+
+func TestSQLiteAuth_Authenticate_Timing(t *testing.T) {
+	a := newTestAuth(t, false, false)
+	require.Nil(t, a.AddUser("user", "pass", auth.RoleAdmin))
+
+	// Timing a correct attempt
+	start := time.Now().UnixMilli()
+	_, err := a.Authenticate("user", "pass")
+	require.Nil(t, err)
+	require.GreaterOrEqual(t, time.Now().UnixMilli()-start, int64(100)) // Ideally should be > 200ms, but let's not make a brittle
+
+	// Timing an incorrect attempt
+	start = time.Now().UnixMilli()
+	_, err = a.Authenticate("user", "INCORRECT")
+	require.Equal(t, auth.ErrUnauthenticated, err)
+	require.GreaterOrEqual(t, time.Now().UnixMilli()-start, int64(100)) // Ideally should be > 200ms, but let's not make a brittle
+
+	// Timing a non-existing user attempt
+	start = time.Now().UnixMilli()
+	_, err = a.Authenticate("DOES-NOT-EXIST", "hithere")
+	require.Equal(t, auth.ErrUnauthenticated, err)
+	require.GreaterOrEqual(t, time.Now().UnixMilli()-start, int64(100)) // Ideally should be > 200ms, but let's not make a brittle
+}
+
+func TestSQLiteAuth_UserManagement(t *testing.T) {
+	a := newTestAuth(t, false, false)
+	require.Nil(t, a.AddUser("phil", "phil", auth.RoleAdmin))
+	require.Nil(t, a.AddUser("ben", "ben", auth.RoleUser))
+	require.Nil(t, a.AllowAccess("ben", "mytopic", true, true))
+	require.Nil(t, a.AllowAccess("ben", "readme", true, false))
+	require.Nil(t, a.AllowAccess("ben", "writeme", false, true))
+	require.Nil(t, a.AllowAccess("ben", "everyonewrite", false, false)) // How unfair!
+	require.Nil(t, a.AllowAccess(auth.Everyone, "announcements", true, false))
+	require.Nil(t, a.AllowAccess(auth.Everyone, "everyonewrite", true, true))
+
+	// Query user details
+	phil, err := a.User("phil")
+	require.Nil(t, err)
+	require.Equal(t, "phil", phil.Name)
+	require.True(t, strings.HasPrefix(phil.Hash, "$2a$11$"))
+	require.Equal(t, auth.RoleAdmin, phil.Role)
+	require.Equal(t, []auth.Grant{}, phil.Grants)
+
+	ben, err := a.User("ben")
+	require.Nil(t, err)
+	require.Equal(t, "ben", ben.Name)
+	require.True(t, strings.HasPrefix(ben.Hash, "$2a$11$"))
+	require.Equal(t, auth.RoleUser, ben.Role)
+	require.Equal(t, []auth.Grant{
+		{"mytopic", true, true},
+		{"readme", true, false},
+		{"writeme", false, true},
+		{"everyonewrite", false, false},
+	}, ben.Grants)
+
+	everyone, err := a.User(auth.Everyone)
+	require.Nil(t, err)
+	require.Equal(t, "*", everyone.Name)
+	require.Equal(t, "", everyone.Hash)
+	require.Equal(t, auth.RoleAnonymous, everyone.Role)
+	require.Equal(t, []auth.Grant{
+		{"announcements", true, false},
+		{"everyonewrite", true, true},
+	}, everyone.Grants)
+
+	// Ben: Before revoking
+	require.Nil(t, a.AllowAccess("ben", "mytopic", true, true))
+	require.Nil(t, a.AllowAccess("ben", "readme", true, false))
+	require.Nil(t, a.AllowAccess("ben", "writeme", false, true))
+	require.Nil(t, a.Authorize(ben, "mytopic", auth.PermissionRead))
+	require.Nil(t, a.Authorize(ben, "mytopic", auth.PermissionWrite))
+	require.Nil(t, a.Authorize(ben, "readme", auth.PermissionRead))
+	require.Nil(t, a.Authorize(ben, "writeme", auth.PermissionWrite))
+
+	// Revoke access for "ben" to "mytopic", then check again
+	require.Nil(t, a.ResetAccess("ben", "mytopic"))
+	require.Equal(t, auth.ErrUnauthorized, a.Authorize(ben, "mytopic", auth.PermissionWrite)) // Revoked
+	require.Equal(t, auth.ErrUnauthorized, a.Authorize(ben, "mytopic", auth.PermissionRead))  // Revoked
+	require.Nil(t, a.Authorize(ben, "readme", auth.PermissionRead))                           // Unchanged
+	require.Nil(t, a.Authorize(ben, "writeme", auth.PermissionWrite))                         // Unchanged
+
+	// Revoke rest of the access
+	require.Nil(t, a.ResetAccess("ben", ""))
+	require.Equal(t, auth.ErrUnauthorized, a.Authorize(ben, "readme", auth.PermissionRead))    // Revoked
+	require.Equal(t, auth.ErrUnauthorized, a.Authorize(ben, "wrtiteme", auth.PermissionWrite)) // Revoked
+
+	// User list
+	users, err := a.Users()
+	require.Nil(t, err)
+	require.Equal(t, 3, len(users))
+	require.Equal(t, "phil", users[0].Name)
+	require.Equal(t, "ben", users[1].Name)
+	require.Equal(t, "*", users[2].Name)
+
+	// Remove user
+	require.Nil(t, a.RemoveUser("ben"))
+	_, err = a.User("ben")
+	require.Equal(t, auth.ErrNotFound, err)
+
+	users, err = a.Users()
+	require.Nil(t, err)
+	require.Equal(t, 2, len(users))
+	require.Equal(t, "phil", users[0].Name)
+	require.Equal(t, "*", users[1].Name)
+}
+
+func TestSQLiteAuth_ChangePassword(t *testing.T) {
+	a := newTestAuth(t, false, false)
+	require.Nil(t, a.AddUser("phil", "phil", auth.RoleAdmin))
+
+	_, err := a.Authenticate("phil", "phil")
+	require.Nil(t, err)
+
+	require.Nil(t, a.ChangePassword("phil", "newpass"))
+	_, err = a.Authenticate("phil", "phil")
+	require.Equal(t, auth.ErrUnauthenticated, err)
+	_, err = a.Authenticate("phil", "newpass")
+	require.Nil(t, err)
+}
+
+func TestSQLiteAuth_ChangeRole(t *testing.T) {
+	a := newTestAuth(t, false, false)
+	require.Nil(t, a.AddUser("ben", "ben", auth.RoleUser))
+	require.Nil(t, a.AllowAccess("ben", "mytopic", true, true))
+	require.Nil(t, a.AllowAccess("ben", "readme", true, false))
+
+	ben, err := a.User("ben")
+	require.Nil(t, err)
+	require.Equal(t, auth.RoleUser, ben.Role)
+	require.Equal(t, 2, len(ben.Grants))
+
+	require.Nil(t, a.ChangeRole("ben", auth.RoleAdmin))
+
+	ben, err = a.User("ben")
+	require.Nil(t, err)
+	require.Equal(t, auth.RoleAdmin, ben.Role)
+	require.Equal(t, 0, len(ben.Grants))
+}
+
+func newTestAuth(t *testing.T, defaultRead, defaultWrite bool) *auth.SQLiteAuth {
+	filename := filepath.Join(t.TempDir(), "user.db")
+	a, err := auth.NewSQLiteAuth(filename, defaultRead, defaultWrite)
+	require.Nil(t, err)
+	return a
+}