From c19377109eebbeb2d80d50dd0134c61fbf5d301c Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Sat, 28 Feb 2026 21:15:51 -0500 Subject: [PATCH] Small refinements --- user/manager.go | 28 ++++++++-------------------- user/manager_test.go | 8 ++++---- user/types.go | 1 - 3 files changed, 12 insertions(+), 25 deletions(-) diff --git a/user/manager.go b/user/manager.go index 152f5b6f..f991d296 100644 --- a/user/manager.go +++ b/user/manager.go @@ -103,7 +103,7 @@ func (a *Manager) AuthenticateToken(token string) (*User, error) { if len(token) != tokenLength { return nil, ErrUnauthenticated } - user, err := a.UserByToken(token) + user, err := a.userByToken(token) if err != nil { log.Tag(tag).Field("token", token).Err(err).Trace("Authentication of token failed") return nil, ErrUnauthenticated @@ -114,9 +114,6 @@ func (a *Manager) AuthenticateToken(token string) (*User, error) { // AddUser adds a user with the given username, password and role func (a *Manager) AddUser(username, password string, role Role, hashed bool) error { - if !AllowedUsername(username) || !AllowedRole(role) { - return ErrInvalidArgument - } hash, err := a.maybeHashPassword(password, hashed) if err != nil { return err @@ -416,8 +413,8 @@ func (a *Manager) UserByID(id string) (*User, error) { return a.readUser(rows) } -// UserByToken returns the user with the given token if it exists and is not expired, or ErrUserNotFound otherwise -func (a *Manager) UserByToken(token string) (*User, error) { +// userByToken returns the user with the given token if it exists and is not expired, or ErrUserNotFound otherwise +func (a *Manager) userByToken(token string) (*User, error) { rows, err := a.db.Query(a.queries.selectUserByToken, token, time.Now().Unix()) if err != nil { return nil, err @@ -581,7 +578,7 @@ func (a *Manager) Authorize(user *User, topic string, perm Permission) error { username = user.Name } // Select the read/write permissions for this user/topic combo. - read, write, found, err := a.AuthorizeTopicAccess(username, topic) + read, write, found, err := a.authorizeTopicAccess(username, topic) if err != nil { return err } @@ -663,13 +660,13 @@ func (a *Manager) AllowReservation(username string, topic string) error { return nil } -// AuthorizeTopicAccess returns the read/write permissions for the given username and topic. +// authorizeTopicAccess returns the read/write permissions for the given username and topic. // The found return value indicates whether an ACL entry was found at all. // // - The query may return two rows (one for everyone, and one for the user), but prioritizes the user. // - Furthermore, the query prioritizes more specific permissions (longer!) over more generic ones, e.g. "test*" > "*" // - It also prioritizes write permissions over read permissions -func (a *Manager) AuthorizeTopicAccess(usernameOrEveryone, topic string) (read, write, found bool, err error) { +func (a *Manager) authorizeTopicAccess(usernameOrEveryone, topic string) (read, write, found bool, err error) { rows, err := a.db.Query(a.queries.selectTopicPerms, Everyone, usernameOrEveryone, topic) if err != nil { return false, false, false, err @@ -873,14 +870,6 @@ func (a *Manager) OtherAccessCount(username, topic string) (int, error) { return count, nil } -// ResetAllProvisionedAccess removes all provisioned access control entries -func (a *Manager) ResetAllProvisionedAccess() error { - if _, err := a.db.Exec(a.queries.deleteUserAccessProvisioned); err != nil { - return err - } - return nil -} - func (a *Manager) addReservationAccessTx(tx *sql.Tx, username, topic string, read, write bool, ownerUsername string) error { if !AllowedUsername(username) && username != Everyone { return ErrInvalidArgument @@ -1024,8 +1013,7 @@ func (a *Manager) Tokens(userID string) ([]*Token, error) { return tokens, nil } -// AllProvisionedTokens returns all provisioned tokens -func (a *Manager) AllProvisionedTokens() ([]*Token, error) { +func (a *Manager) allProvisionedTokens() ([]*Token, error) { rows, err := a.db.Query(a.queries.selectAllProvisionedTokens) if err != nil { return nil, err @@ -1299,7 +1287,7 @@ func (a *Manager) maybeProvisionUsersAccessAndTokens() error { provisionUsernames := util.Map(a.config.Users, func(u *User) string { return u.Name }) - existingTokens, err := a.AllProvisionedTokens() + existingTokens, err := a.allProvisionedTokens() if err != nil { return err } diff --git a/user/manager_test.go b/user/manager_test.go index 8ba6fe3b..64ea437a 100644 --- a/user/manager_test.go +++ b/user/manager_test.go @@ -1797,7 +1797,7 @@ func TestStoreUserByToken(t *testing.T) { require.Nil(t, err) require.NotEmpty(t, tk.Value) - u2, err := manager.UserByToken(tk.Value) + u2, err := manager.userByToken(tk.Value) require.Nil(t, err) require.Equal(t, "phil", u2.Name) }) @@ -2054,7 +2054,7 @@ func TestStoreAuthorizeTopicAccess(t *testing.T) { require.Nil(t, manager.AddUser("phil", "mypass", RoleUser, false)) require.Nil(t, manager.AllowAccess("phil", "mytopic", PermissionReadWrite)) - read, write, found, err := manager.AuthorizeTopicAccess("phil", "mytopic") + read, write, found, err := manager.authorizeTopicAccess("phil", "mytopic") require.Nil(t, err) require.True(t, found) require.True(t, read) @@ -2066,7 +2066,7 @@ func TestStoreAuthorizeTopicAccessNotFound(t *testing.T) { forEachStoreBackend(t, func(t *testing.T, manager *Manager) { require.Nil(t, manager.AddUser("phil", "mypass", RoleUser, false)) - _, _, found, err := manager.AuthorizeTopicAccess("phil", "other") + _, _, found, err := manager.authorizeTopicAccess("phil", "other") require.Nil(t, err) require.False(t, found) }) @@ -2077,7 +2077,7 @@ func TestStoreAuthorizeTopicAccessDenyAll(t *testing.T) { require.Nil(t, manager.AddUser("phil", "mypass", RoleUser, false)) require.Nil(t, manager.AllowAccess("phil", "secret", PermissionDenyAll)) - read, write, found, err := manager.AuthorizeTopicAccess("phil", "secret") + read, write, found, err := manager.authorizeTopicAccess("phil", "secret") require.Nil(t, err) require.True(t, found) require.False(t, read) diff --git a/user/types.go b/user/types.go index b80bc46b..58781203 100644 --- a/user/types.go +++ b/user/types.go @@ -343,4 +343,3 @@ type storeQueries struct { // Billing queries updateBilling string } -