User-owned ACL entries

This commit is contained in:
binwiederhier 2023-01-01 15:21:43 -05:00
parent 598d0bdda3
commit 2267d27c9b
9 changed files with 160 additions and 57 deletions

View file

@ -108,7 +108,7 @@ func changeAccess(c *cli.Context, manager *user.Manager, username string, topic
} else if u.Role == user.RoleAdmin {
return fmt.Errorf("user %s is an admin user, access control entries have no effect", username)
}
if err := manager.AllowAccess(username, topic, read, write); err != nil {
if err := manager.AllowAccess("", username, topic, read, write); err != nil {
return err
}
if read && write {

View file

@ -41,7 +41,7 @@ var (
errHTTPBadRequestDelayTooLarge = &errHTTP{40006, http.StatusBadRequest, "invalid delay parameter: too large, please refer to the docs", "https://ntfy.sh/docs/publish/#scheduled-delivery"}
errHTTPBadRequestPriorityInvalid = &errHTTP{40007, http.StatusBadRequest, "invalid priority parameter", "https://ntfy.sh/docs/publish/#message-priority"}
errHTTPBadRequestSinceInvalid = &errHTTP{40008, http.StatusBadRequest, "invalid since parameter", "https://ntfy.sh/docs/subscribe/api/#fetch-cached-messages"}
errHTTPBadRequestTopicInvalid = &errHTTP{40009, http.StatusBadRequest, "invalid topic: path invalid", ""}
errHTTPBadRequestTopicInvalid = &errHTTP{40009, http.StatusBadRequest, "invalid topic: topic invalid", ""}
errHTTPBadRequestTopicDisallowed = &errHTTP{40010, http.StatusBadRequest, "invalid topic: topic name is disallowed", ""}
errHTTPBadRequestMessageNotUTF8 = &errHTTP{40011, http.StatusBadRequest, "invalid message: message must be UTF-8 encoded", ""}
errHTTPBadRequestAttachmentURLInvalid = &errHTTP{40013, http.StatusBadRequest, "invalid request: attachment URL is invalid", "https://ntfy.sh/docs/publish/#attachments"}
@ -60,6 +60,7 @@ var (
errHTTPUnauthorized = &errHTTP{40101, http.StatusUnauthorized, "unauthorized", "https://ntfy.sh/docs/publish/#authentication"}
errHTTPForbidden = &errHTTP{40301, http.StatusForbidden, "forbidden", "https://ntfy.sh/docs/publish/#authentication"}
errHTTPConflictUserExists = &errHTTP{40901, http.StatusConflict, "conflict: user already exists", ""}
errHTTPConflictTopicReserved = &errHTTP{40902, http.StatusConflict, "conflict: access control entry for topic or topic pattern already exists", ""}
errHTTPEntityTooLargeAttachment = &errHTTP{41301, http.StatusRequestEntityTooLarge, "attachment too large, or bandwidth limit reached", "https://ntfy.sh/docs/publish/#limitations"}
errHTTPEntityTooLargeMatrixRequest = &errHTTP{41302, http.StatusRequestEntityTooLarge, "Matrix request is larger than the max allowed length", ""}
errHTTPEntityTooLargeJSONBody = &errHTTP{41303, http.StatusRequestEntityTooLarge, "JSON body too large", ""}
@ -70,6 +71,6 @@ var (
errHTTPTooManyRequestsAttachmentBandwidthLimit = &errHTTP{42905, http.StatusTooManyRequests, "too many requests: daily bandwidth limit reached", "https://ntfy.sh/docs/publish/#limitations"}
errHTTPTooManyRequestsAccountCreateLimit = &errHTTP{42906, http.StatusTooManyRequests, "too many requests: daily account creation limit reached", "https://ntfy.sh/docs/publish/#limitations"} // FIXME document limit
errHTTPInternalError = &errHTTP{50001, http.StatusInternalServerError, "internal server error", ""}
errHTTPInternalErrorInvalidPath = &errHTTP{50002, http.StatusInternalServerError, "internal server error: invalid file path", ""}
errHTTPInternalErrorInvalidPath = &errHTTP{50002, http.StatusInternalServerError, "internal server error: invalid path", ""}
errHTTPInternalErrorMissingBaseURL = &errHTTP{50003, http.StatusInternalServerError, "internal server error: base-url must be be configured for this feature", "https://ntfy.sh/docs/config/"}
)

View file

@ -103,6 +103,7 @@ var (
accountSettingsPath = "/v1/account/settings"
accountSubscriptionPath = "/v1/account/subscription"
accountAccessPath = "/v1/account/access"
accountAccessSingleRegex = regexp.MustCompile(`/v1/account/access/([-_A-Za-z0-9]{1,64})$`)
accountSubscriptionSingleRegex = regexp.MustCompile(`^/v1/account/subscription/([-_A-Za-z0-9]{16})$`)
matrixPushPath = "/_matrix/push/v1/notify"
staticRegex = regexp.MustCompile(`^/static/.+`)
@ -361,6 +362,8 @@ func (s *Server) handleInternal(w http.ResponseWriter, r *http.Request, v *visit
return s.ensureUser(s.handleAccountSubscriptionDelete)(w, r, v)
} else if r.Method == http.MethodPost && r.URL.Path == accountAccessPath {
return s.ensureUser(s.handleAccountAccessAdd)(w, r, v)
} else if r.Method == http.MethodDelete && accountAccessSingleRegex.MatchString(r.URL.Path) {
return s.ensureUser(s.handleAccountAccessDelete)(w, r, v)
} else if r.Method == http.MethodGet && r.URL.Path == matrixPushPath {
return s.handleMatrixDiscovery(w)
} else if r.Method == http.MethodGet && staticRegex.MatchString(r.URL.Path) {

View file

@ -91,7 +91,18 @@ func (s *Server) handleAccountGet(w http.ResponseWriter, r *http.Request, v *vis
Upgradable: true,
}
}
if len(v.user.Grants) > 0 {
response.Access = make([]*apiAccountGrant, 0)
for _, grant := range v.user.Grants {
if grant.Owner {
response.Access = append(response.Access, &apiAccountGrant{
Topic: grant.TopicPattern,
Read: grant.AllowRead,
Write: grant.AllowWrite,
})
}
}
}
} else {
response.Username = user.Everyone
response.Role = string(user.RoleAnonymous)
@ -316,13 +327,46 @@ func (s *Server) handleAccountAccessAdd(w http.ResponseWriter, r *http.Request,
if !topicRegex.MatchString(req.Topic) {
return errHTTPBadRequestTopicInvalid
}
// FIXME authorize: how do I know if v.user (= auth'd user) is allowed to write the ACL entries
if err := s.userManager.CheckAllowAccess(v.user.Name, req.Topic); err != nil {
return errHTTPConflictTopicReserved
}
owner, username := v.user.Name, v.user.Name
everyoneRead := util.Contains([]string{"read-write", "rw", "read-only", "read", "ro"}, req.Everyone)
everyoneWrite := util.Contains([]string{"read-write", "rw", "write-only", "write", "wo"}, req.Everyone)
if err := s.userManager.AllowAccess(v.user.Name, req.Topic, true, true); err != nil {
if err := s.userManager.AllowAccess(owner, username, req.Topic, true, true); err != nil {
return err
}
if err := s.userManager.AllowAccess(user.Everyone, req.Topic, everyoneRead, everyoneWrite); err != nil {
if err := s.userManager.AllowAccess(owner, user.Everyone, req.Topic, everyoneRead, everyoneWrite); err != nil {
return err
}
w.Header().Set("Content-Type", "application/json")
w.Header().Set("Access-Control-Allow-Origin", "*") // FIXME remove this
return nil
}
func (s *Server) handleAccountAccessDelete(w http.ResponseWriter, r *http.Request, v *visitor) error {
matches := accountAccessSingleRegex.FindStringSubmatch(r.URL.Path)
if len(matches) != 2 {
return errHTTPInternalErrorInvalidPath
}
topic := matches[1]
if !topicRegex.MatchString(topic) {
return errHTTPBadRequestTopicInvalid
}
authorized := false
for _, grant := range v.user.Grants {
if grant.TopicPattern == topic && grant.Owner {
authorized = true
break
}
}
if !authorized {
return errHTTPUnauthorized
}
if err := s.userManager.ResetAccess(v.user.Name, topic); err != nil {
return err
}
if err := s.userManager.ResetAccess(user.Everyone, topic); err != nil {
return err
}
w.Header().Set("Content-Type", "application/json")

View file

@ -643,7 +643,7 @@ func TestServer_Auth_Success_User(t *testing.T) {
s := newTestServer(t, c)
require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser))
require.Nil(t, s.userManager.AllowAccess("ben", "mytopic", true, true))
require.Nil(t, s.userManager.AllowAccess("", "ben", "mytopic", true, true))
response := request(t, s, "GET", "/mytopic/auth", "", map[string]string{
"Authorization": basicAuth("ben:ben"),
@ -659,8 +659,8 @@ func TestServer_Auth_Success_User_MultipleTopics(t *testing.T) {
s := newTestServer(t, c)
require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser))
require.Nil(t, s.userManager.AllowAccess("ben", "mytopic", true, true))
require.Nil(t, s.userManager.AllowAccess("ben", "anothertopic", true, true))
require.Nil(t, s.userManager.AllowAccess("", "ben", "mytopic", true, true))
require.Nil(t, s.userManager.AllowAccess("", "ben", "anothertopic", true, true))
response := request(t, s, "GET", "/mytopic,anothertopic/auth", "", map[string]string{
"Authorization": basicAuth("ben:ben"),
@ -696,7 +696,7 @@ func TestServer_Auth_Fail_Unauthorized(t *testing.T) {
s := newTestServer(t, c)
require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser))
require.Nil(t, s.userManager.AllowAccess("ben", "sometopic", true, true)) // Not mytopic!
require.Nil(t, s.userManager.AllowAccess("", "ben", "sometopic", true, true)) // Not mytopic!
response := request(t, s, "GET", "/mytopic/auth", "", map[string]string{
"Authorization": basicAuth("ben:ben"),
@ -712,8 +712,8 @@ func TestServer_Auth_Fail_CannotPublish(t *testing.T) {
s := newTestServer(t, c)
require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin))
require.Nil(t, s.userManager.AllowAccess(user.Everyone, "private", false, false))
require.Nil(t, s.userManager.AllowAccess(user.Everyone, "announcements", true, false))
require.Nil(t, s.userManager.AllowAccess("", user.Everyone, "private", false, false))
require.Nil(t, s.userManager.AllowAccess("", user.Everyone, "announcements", true, false))
response := request(t, s, "PUT", "/mytopic", "test", nil)
require.Equal(t, 200, response.Code)

View file

@ -256,12 +256,19 @@ type apiAccountStats struct {
AttachmentTotalSizeRemaining int64 `json:"attachment_total_size_remaining"`
}
type apiAccountGrant struct {
Topic string `json:"topic"`
Read bool `json:"read"`
Write bool `json:"write"`
}
type apiAccountResponse struct {
Username string `json:"username"`
Role string `json:"role,omitempty"`
Language string `json:"language,omitempty"`
Notification *user.NotificationPrefs `json:"notification,omitempty"`
Subscriptions []*user.Subscription `json:"subscriptions,omitempty"`
Access []*apiAccountGrant `json:"access,omitempty"`
Plan *apiAccountPlan `json:"plan,omitempty"`
Limits *apiAccountLimits `json:"limits,omitempty"`
Stats *apiAccountStats `json:"stats,omitempty"`

View file

@ -23,7 +23,8 @@ const (
)
var (
errNoTokenProvided = errors.New("no token provided")
errNoTokenProvided = errors.New("no token provided")
errTopicOwnedByOthers = errors.New("topic owned by others")
)
// Manager-related queries
@ -52,10 +53,10 @@ const (
CREATE UNIQUE INDEX idx_user ON user (user);
CREATE TABLE IF NOT EXISTS user_access (
user_id INT NOT NULL,
owner_user_id INT,
topic TEXT NOT NULL,
read INT NOT NULL,
write INT NOT NULL,
owner_user_id INT,
PRIMARY KEY (user_id, topic),
FOREIGN KEY (user_id) REFERENCES user (id) ON DELETE CASCADE
);
@ -115,12 +116,23 @@ const (
deleteUserQuery = `DELETE FROM user WHERE user = ?`
upsertUserAccessQuery = `
INSERT INTO user_access (user_id, topic, read, write)
VALUES ((SELECT id FROM user WHERE user = ?), ?, ?, ?)
INSERT INTO user_access (user_id, topic, read, write, owner_user_id)
VALUES ((SELECT id FROM user WHERE user = ?), ?, ?, ?, (SELECT IIF(?='',NULL,(SELECT id FROM user WHERE user=?))))
ON CONFLICT (user_id, topic)
DO UPDATE SET read=excluded.read, write=excluded.write
DO UPDATE SET read=excluded.read, write=excluded.write, owner_user_id=excluded.owner_user_id
`
selectUserAccessQuery = `
SELECT topic, read, write, IIF(owner_user_id IS NOT NULL AND user_id = owner_user_id,1,0) AS owner
FROM user_access
WHERE user_id = (SELECT id FROM user WHERE user = ?)
ORDER BY write DESC, read DESC, topic
`
selectOtherAccessCountQuery = `
SELECT count(*)
FROM user_access
WHERE (topic = ? OR ? LIKE topic)
AND (owner_user_id IS NULL OR owner_user_id != (SELECT id FROM user WHERE user = ?))
`
selectUserAccessQuery = `SELECT topic, read, write FROM user_access WHERE user_id = (SELECT id FROM user WHERE user = ?) ORDER BY write DESC, read DESC, topic`
deleteAllAccessQuery = `DELETE FROM user_access`
deleteUserAccessQuery = `DELETE FROM user_access WHERE user_id = (SELECT id FROM user WHERE user = ?)`
deleteTopicAccessQuery = `DELETE FROM user_access WHERE user_id = (SELECT id FROM user WHERE user = ?) AND topic = ?`
@ -340,8 +352,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. The query may return two
// rows (one for everyone, and one for the user), but prioritizes the user. The value for
// user.Name may be empty (= everyone).
// rows (one for everyone, and one for the user), but prioritizes the user.
rows, err := a.db.Query(selectTopicPermsQuery, username, topic)
if err != nil {
return err
@ -509,8 +520,8 @@ func (a *Manager) readGrants(username string) ([]Grant, error) {
grants := make([]Grant, 0)
for rows.Next() {
var topic string
var read, write bool
if err := rows.Scan(&topic, &read, &write); err != nil {
var read, write, owner bool
if err := rows.Scan(&topic, &read, &write, &owner); err != nil {
return nil, err
} else if err := rows.Err(); err != nil {
return nil, err
@ -519,6 +530,7 @@ func (a *Manager) readGrants(username string) ([]Grant, error) {
TopicPattern: fromSQLWildcard(topic),
AllowRead: read,
AllowWrite: write,
Owner: owner,
})
}
return grants, nil
@ -553,13 +565,42 @@ func (a *Manager) ChangeRole(username string, role Role) error {
return nil
}
// AllowAccess adds or updates an entry in th access control list for a specific user. It controls
// read/write access to a topic. The parameter topicPattern may include wildcards (*).
func (a *Manager) AllowAccess(username string, topicPattern string, read bool, write bool) error {
if (!AllowedUsername(username) && username != Everyone) || !AllowedTopicPattern(topicPattern) {
// CheckAllowAccess tests if a user may create an access control entry for the given topic.
// If there are any ACL entries that are not owned by the user, an error is returned.
func (a *Manager) CheckAllowAccess(username string, topic string) error {
if (!AllowedUsername(username) && username != Everyone) || !AllowedTopic(topic) {
return ErrInvalidArgument
}
if _, err := a.db.Exec(upsertUserAccessQuery, username, toSQLWildcard(topicPattern), read, write); err != nil {
rows, err := a.db.Query(selectOtherAccessCountQuery, topic, topic, username)
if err != nil {
return err
}
defer rows.Close()
if !rows.Next() {
return errors.New("no rows found")
}
var otherCount int
if err := rows.Scan(&otherCount); err != nil {
return err
}
if otherCount > 0 {
return errTopicOwnedByOthers
}
return nil
}
// AllowAccess adds or updates an entry in th access control list for a specific user. It controls
// read/write access to a topic. The parameter topicPattern may include wildcards (*). The ACL entry
// owner may either be a user (username), or the system (empty).
func (a *Manager) AllowAccess(owner, username string, topicPattern string, read bool, write bool) error {
if !AllowedUsername(username) && username != Everyone {
return ErrInvalidArgument
} else if owner != "" && !AllowedUsername(owner) {
return ErrInvalidArgument
} else if !AllowedTopicPattern(topicPattern) {
return ErrInvalidArgument
}
if _, err := a.db.Exec(upsertUserAccessQuery, username, toSQLWildcard(topicPattern), read, write, owner, owner); err != nil {
return err
}
return nil

View file

@ -15,13 +15,13 @@ func TestManager_FullScenario_Default_DenyAll(t *testing.T) {
a := newTestManager(t, false, false)
require.Nil(t, a.AddUser("phil", "phil", RoleAdmin))
require.Nil(t, a.AddUser("ben", "ben", 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(Everyone, "announcements", true, false))
require.Nil(t, a.AllowAccess(Everyone, "everyonewrite", true, true))
require.Nil(t, a.AllowAccess(Everyone, "up*", false, true)) // Everyone can write to /up*
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("", Everyone, "announcements", true, false))
require.Nil(t, a.AllowAccess("", Everyone, "everyonewrite", true, true))
require.Nil(t, a.AllowAccess("", Everyone, "up*", false, true)) // Everyone can write to /up*
phil, err := a.Authenticate("phil", "phil")
require.Nil(t, err)
@ -36,10 +36,10 @@ func TestManager_FullScenario_Default_DenyAll(t *testing.T) {
require.True(t, strings.HasPrefix(ben.Hash, "$2a$10$"))
require.Equal(t, RoleUser, ben.Role)
require.Equal(t, []Grant{
{"mytopic", true, true},
{"writeme", false, true},
{"readme", true, false},
{"everyonewrite", false, false},
{"mytopic", true, true, false},
{"writeme", false, true, false},
{"readme", true, false, false},
{"everyonewrite", false, false, false},
}, ben.Grants)
notben, err := a.Authenticate("ben", "this is wrong")
@ -124,12 +124,12 @@ func TestManager_UserManagement(t *testing.T) {
a := newTestManager(t, false, false)
require.Nil(t, a.AddUser("phil", "phil", RoleAdmin))
require.Nil(t, a.AddUser("ben", "ben", 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(Everyone, "announcements", true, false))
require.Nil(t, a.AllowAccess(Everyone, "everyonewrite", true, true))
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("", Everyone, "announcements", true, false))
require.Nil(t, a.AllowAccess("", Everyone, "everyonewrite", true, true))
// Query user details
phil, err := a.User("phil")
@ -145,10 +145,10 @@ func TestManager_UserManagement(t *testing.T) {
require.True(t, strings.HasPrefix(ben.Hash, "$2a$10$"))
require.Equal(t, RoleUser, ben.Role)
require.Equal(t, []Grant{
{"mytopic", true, true},
{"writeme", false, true},
{"readme", true, false},
{"everyonewrite", false, false},
{"mytopic", true, true, false},
{"writeme", false, true, false},
{"readme", true, false, false},
{"everyonewrite", false, false, false},
}, ben.Grants)
everyone, err := a.User(Everyone)
@ -157,14 +157,14 @@ func TestManager_UserManagement(t *testing.T) {
require.Equal(t, "", everyone.Hash)
require.Equal(t, RoleAnonymous, everyone.Role)
require.Equal(t, []Grant{
{"everyonewrite", true, true},
{"announcements", true, false},
{"everyonewrite", true, true, false},
{"announcements", true, false, false},
}, everyone.Grants)
// Ben: Before revoking
require.Nil(t, a.AllowAccess("ben", "mytopic", true, true)) // Overwrite!
require.Nil(t, a.AllowAccess("ben", "readme", true, false))
require.Nil(t, a.AllowAccess("ben", "writeme", false, true))
require.Nil(t, a.AllowAccess("", "ben", "mytopic", true, true)) // Overwrite!
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", PermissionRead))
require.Nil(t, a.Authorize(ben, "mytopic", PermissionWrite))
require.Nil(t, a.Authorize(ben, "readme", PermissionRead))
@ -219,8 +219,8 @@ func TestManager_ChangePassword(t *testing.T) {
func TestManager_ChangeRole(t *testing.T) {
a := newTestManager(t, false, false)
require.Nil(t, a.AddUser("ben", "ben", 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", "mytopic", true, true))
require.Nil(t, a.AllowAccess("", "ben", "readme", true, false))
ben, err := a.User("ben")
require.Nil(t, err)

View file

@ -90,6 +90,7 @@ type Grant struct {
TopicPattern string // May include wildcard (*)
AllowRead bool
AllowWrite bool
Owner bool // This user owns this ACL entry
}
// Permission represents a read or write permission to a topic
@ -118,6 +119,7 @@ const (
var (
allowedUsernameRegex = regexp.MustCompile(`^[-_.@a-zA-Z0-9]+$`) // Does not include Everyone (*)
allowedTopicRegex = regexp.MustCompile(`^[-_A-Za-z0-9]{1,64}$`) // No '*'
allowedTopicPatternRegex = regexp.MustCompile(`^[-_*A-Za-z0-9]{1,64}$`) // Adds '*' for wildcards!
)
@ -131,6 +133,11 @@ func AllowedUsername(username string) bool {
return allowedUsernameRegex.MatchString(username)
}
// AllowedTopic returns true if the given topic name is valid
func AllowedTopic(username string) bool {
return allowedTopicRegex.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)