From c56814e7da56e20ef0d9acf0294a48a42dad4ba5 Mon Sep 17 00:00:00 2001 From: Philipp Heckel Date: Mon, 31 Jan 2022 11:44:58 -0500 Subject: [PATCH] Add wildcard access control --- auth/auth.go | 28 +++++++++++++++++++----- auth/auth_sqlite.go | 46 ++++++++++++++++++++++++---------------- auth/auth_sqlite_test.go | 3 +++ server/server.go | 3 ++- 4 files changed, 56 insertions(+), 24 deletions(-) diff --git a/auth/auth.go b/auth/auth.go index 377b54ce..63201e4b 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -1,6 +1,9 @@ package auth -import "errors" +import ( + "errors" + "regexp" +) // Auther is a generic interface to implement password-based authentication and authorization type Auther interface { @@ -39,12 +42,12 @@ type Manager interface { ChangeRole(username string, role Role) error // AllowAccess adds or updates an entry in th access control list for a specific user. It controls - // read/write access to a topic. - AllowAccess(username string, topic string, read bool, write bool) error + // read/write access to a topic. The parameter topicPattern may include wildcards (*). + AllowAccess(username string, topicPattern string, read bool, write bool) error // ResetAccess removes an access control list entry for a specific username/topic, or (if topic is - // empty) for an entire user. - ResetAccess(username string, topic string) error + // empty) for an entire user. The parameter topicPattern may include wildcards (*). + ResetAccess(username string, topicPattern string) error // DefaultAccess returns the default read/write access if no access control entry matches DefaultAccess() (read bool, write bool) @@ -89,11 +92,26 @@ const ( Everyone = "*" ) +var ( + allowedUsernameRegex = regexp.MustCompile(`^[-_.@a-zA-Z0-9]+$`) // Does not include Everyone (*) + allowedTopicPatternRegex = regexp.MustCompile(`^[-_*A-Za-z0-9]{1,64}$`) // Adds '*' for wildcards! +) + // AllowedRole returns true if the given role can be used for new users func AllowedRole(role Role) bool { return role == RoleUser || role == RoleAdmin } +// AllowedUsername returns true if the given username is valid +func AllowedUsername(username string) bool { + return allowedUsernameRegex.MatchString(username) +} + +// AllowedTopicPattern returns true if the given topic pattern is valid; this includes the wildcard character (*) +func AllowedTopicPattern(username string) bool { + return allowedTopicPatternRegex.MatchString(username) +} + // Error constants used by the package var ( ErrUnauthenticated = errors.New("unauthenticated") diff --git a/auth/auth_sqlite.go b/auth/auth_sqlite.go index ca0d4cd0..b4ea2ab6 100644 --- a/auth/auth_sqlite.go +++ b/auth/auth_sqlite.go @@ -4,7 +4,7 @@ import ( "database/sql" _ "github.com/mattn/go-sqlite3" // SQLite driver "golang.org/x/crypto/bcrypt" - "regexp" + "strings" ) const ( @@ -37,7 +37,7 @@ const ( selectTopicPermsQuery = ` SELECT read, write FROM access - WHERE user IN ('*', ?) AND topic = ? + WHERE user IN ('*', ?) AND ? LIKE topic ORDER BY user DESC ` ) @@ -69,10 +69,6 @@ type SQLiteAuth struct { defaultWrite bool } -var ( - allowedUsernameRegex = regexp.MustCompile(`^[-_.@a-zA-Z0-9]+$`) -) - var _ Auther = (*SQLiteAuth)(nil) var _ Manager = (*SQLiteAuth)(nil) @@ -161,7 +157,7 @@ func (a *SQLiteAuth) resolvePerms(read, write bool, perm Permission) error { // AddUser adds a user with the given username, password and role. The password should be hashed // before it is stored in a persistence layer. func (a *SQLiteAuth) AddUser(username, password string, role Role) error { - if !allowedUsernameRegex.MatchString(username) || !AllowedRole(role) { + if !AllowedUsername(username) || !AllowedRole(role) { return ErrInvalidArgument } hash, err := bcrypt.GenerateFromPassword([]byte(password), bcryptCost) @@ -177,7 +173,7 @@ func (a *SQLiteAuth) AddUser(username, password string, role Role) error { // RemoveUser deletes the user with the given username. The function returns nil on success, even // if the user did not exist in the first place. func (a *SQLiteAuth) RemoveUser(username string) error { - if !allowedUsernameRegex.MatchString(username) || username == Everyone { + if !AllowedUsername(username) { return ErrInvalidArgument } if _, err := a.db.Exec(deleteUserQuery, username); err != nil { @@ -284,7 +280,7 @@ func (a *SQLiteAuth) readGrants(username string) ([]Grant, error) { return nil, err } grants = append(grants, Grant{ - Topic: topic, + Topic: fromSQLWildcard(topic), Read: read, Write: write, }) @@ -307,7 +303,7 @@ func (a *SQLiteAuth) ChangePassword(username, password string) error { // ChangeRole changes a user's role. When a role is changed from RoleUser to RoleAdmin, // all existing access control entries (Grant) are removed, since they are no longer needed. func (a *SQLiteAuth) ChangeRole(username string, role Role) error { - if !allowedUsernameRegex.MatchString(username) || !AllowedRole(role) { + if !AllowedUsername(username) || !AllowedRole(role) { return ErrInvalidArgument } if _, err := a.db.Exec(updateUserRoleQuery, string(role), username); err != nil { @@ -322,25 +318,31 @@ func (a *SQLiteAuth) ChangeRole(username string, role Role) error { } // AllowAccess adds or updates an entry in th access control list for a specific user. It controls -// read/write access to a topic. -func (a *SQLiteAuth) AllowAccess(username string, topic string, read bool, write bool) error { - if _, err := a.db.Exec(upsertUserAccessQuery, username, topic, read, write); err != nil { +// read/write access to a topic. The parameter topicPattern may include wildcards (*). +func (a *SQLiteAuth) AllowAccess(username string, topicPattern string, read bool, write bool) error { + if (!AllowedUsername(username) && username != Everyone) || !AllowedTopicPattern(topicPattern) { + return ErrInvalidArgument + } + if _, err := a.db.Exec(upsertUserAccessQuery, username, toSQLWildcard(topicPattern), read, write); err != nil { return err } return nil } // ResetAccess removes an access control list entry for a specific username/topic, or (if topic is -// empty) for an entire user. -func (a *SQLiteAuth) ResetAccess(username string, topic string) error { - if username == "" && topic == "" { +// empty) for an entire user. The parameter topicPattern may include wildcards (*). +func (a *SQLiteAuth) ResetAccess(username string, topicPattern string) error { + if (!AllowedUsername(username) && username != Everyone) || (!AllowedTopicPattern(topicPattern) && topicPattern != "") { + return ErrInvalidArgument + } + if username == "" && topicPattern == "" { _, err := a.db.Exec(deleteAllAccessQuery, username) return err - } else if topic == "" { + } else if topicPattern == "" { _, err := a.db.Exec(deleteUserAccessQuery, username) return err } - _, err := a.db.Exec(deleteTopicAccessQuery, username, topic) + _, err := a.db.Exec(deleteTopicAccessQuery, username, toSQLWildcard(topicPattern)) return err } @@ -348,3 +350,11 @@ func (a *SQLiteAuth) ResetAccess(username string, topic string) error { func (a *SQLiteAuth) DefaultAccess() (read bool, write bool) { return a.defaultRead, a.defaultWrite } + +func toSQLWildcard(s string) string { + return strings.ReplaceAll(s, "*", "%") +} + +func fromSQLWildcard(s string) string { + return strings.ReplaceAll(s, "%", "*") +} diff --git a/auth/auth_sqlite_test.go b/auth/auth_sqlite_test.go index 761b41d2..266b624f 100644 --- a/auth/auth_sqlite_test.go +++ b/auth/auth_sqlite_test.go @@ -19,6 +19,7 @@ func TestSQLiteAuth_FullScenario_Default_DenyAll(t *testing.T) { 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)) + require.Nil(t, a.AllowAccess(auth.Everyone, "up*", false, true)) // Everyone can write to /up* phil, err := a.Authenticate("phil", "phil") require.Nil(t, err) @@ -77,6 +78,8 @@ func TestSQLiteAuth_FullScenario_Default_DenyAll(t *testing.T) { 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)) + require.Nil(t, a.Authorize(nil, "up1234", auth.PermissionWrite)) // Wildcard permission + require.Nil(t, a.Authorize(nil, "up5678", auth.PermissionWrite)) } func TestSQLiteAuth_AddUser_Invalid(t *testing.T) { diff --git a/server/server.go b/server/server.go index 7a78156a..b947cb8f 100644 --- a/server/server.go +++ b/server/server.go @@ -63,6 +63,7 @@ type indexPage struct { type handleFunc func(http.ResponseWriter, *http.Request, *visitor) error var ( + // If changed, don't forget to update Android App and auth_sqlite.go topicRegex = regexp.MustCompile(`^[-_A-Za-z0-9]{1,64}$`) // No /! topicPathRegex = regexp.MustCompile(`^/[-_A-Za-z0-9]{1,64}$`) // Regex must match JS & Android app! jsonPathRegex = regexp.MustCompile(`^/[-_A-Za-z0-9]{1,64}(,[-_A-Za-z0-9]{1,64})*/json$`) @@ -75,7 +76,7 @@ var ( staticRegex = regexp.MustCompile(`^/static/.+`) docsRegex = regexp.MustCompile(`^/docs(|/.*)$`) fileRegex = regexp.MustCompile(`^/file/([-_A-Za-z0-9]{1,64})(?:\.[A-Za-z0-9]{1,16})?$`) - disallowedTopics = []string{"docs", "static", "file"} + disallowedTopics = []string{"docs", "static", "file"} // If updated, also update in Android app attachURLRegex = regexp.MustCompile(`^https?://`) templateFnMap = template.FuncMap{