diff --git a/cmd/lists.go b/cmd/lists.go index 90409067..5ee518a0 100644 --- a/cmd/lists.go +++ b/cmd/lists.go @@ -1,7 +1,6 @@ package main import ( - "fmt" "net/http" "strconv" "strings" @@ -11,10 +10,6 @@ import ( "github.com/labstack/echo/v4" ) -var ( - errListPerm = echo.NewHTTPError(http.StatusForbidden, fmt.Sprintf("list permission denied")) -) - // handleGetLists retrieves lists with additional metadata like subscriber counts. func handleGetLists(c echo.Context) error { var ( @@ -33,9 +28,14 @@ func handleGetLists(c echo.Context) error { out models.PageResults ) + var permittedIDs []int + if _, ok := user.PermissionsMap["lists:get_all"]; !ok { + permittedIDs = user.GetListIDs + } + // Minimal query simply returns the list of all lists without JOIN subscriber counts. This is fast. if minimal { - res, err := app.core.GetLists("", user.GetListIDs) + res, err := app.core.GetLists("", permittedIDs) if err != nil { return err } @@ -53,7 +53,7 @@ func handleGetLists(c echo.Context) error { } // Full list query. - res, total, err := app.core.QueryLists(query, typ, optin, tags, orderBy, order, user.GetListIDs, pg.Offset, pg.Limit) + res, total, err := app.core.QueryLists(query, typ, optin, tags, orderBy, order, permittedIDs, pg.Offset, pg.Limit) if err != nil { return err } @@ -164,7 +164,8 @@ func handleDeleteLists(c echo.Context) error { func listPerm(next echo.HandlerFunc) echo.HandlerFunc { return func(c echo.Context) error { var ( - u = c.Get(auth.UserKey).(models.User) + app = c.Get("app").(*App) + user = c.Get(auth.UserKey).(models.User) id, _ = strconv.Atoi(c.Param("id")) ) @@ -179,15 +180,15 @@ func listPerm(next echo.HandlerFunc) echo.HandlerFunc { } // Check if the user has permissions for all lists or the specific list. - if _, ok := u.PermissionsMap[permAll]; ok { + if _, ok := user.PermissionsMap[permAll]; ok { return next(c) } if id > 0 { - if _, ok := u.ListPermissionsMap[id][perm]; ok { + if _, ok := user.ListPermissionsMap[id][perm]; ok { return next(c) } } - return errListPerm + return echo.NewHTTPError(http.StatusBadRequest, app.i18n.Ts("globals.messages.globals.messages.permissionDenied", "name", "list")) } } diff --git a/cmd/roles.go b/cmd/roles.go index 5813209c..9afb9d8e 100644 --- a/cmd/roles.go +++ b/cmd/roles.go @@ -1,6 +1,7 @@ package main import ( + "fmt" "net/http" "strconv" "strings" @@ -115,14 +116,14 @@ func validateRole(r models.Role, app *App) error { for _, p := range r.Permissions { if _, ok := app.constants.Permissions[p]; !ok { - return echo.NewHTTPError(http.StatusBadRequest, app.i18n.Ts("globals.messages.invalidFields", "name", "permission")) + return echo.NewHTTPError(http.StatusBadRequest, app.i18n.Ts("globals.messages.invalidFields", "name", fmt.Sprintf("permission: %s", p))) } } for _, l := range r.Lists { for _, p := range l.Permissions { if p != "list:get" && p != "list:manage" { - return echo.NewHTTPError(http.StatusBadRequest, app.i18n.Ts("globals.messages.invalidFields", "name", "list permissions")) + return echo.NewHTTPError(http.StatusBadRequest, app.i18n.Ts("globals.messages.invalidFields", "name", fmt.Sprintf("list permission: %s", p))) } } } diff --git a/cmd/subscribers.go b/cmd/subscribers.go index 46aad2db..bc801d93 100644 --- a/cmd/subscribers.go +++ b/cmd/subscribers.go @@ -10,6 +10,7 @@ import ( "strconv" "strings" + "github.com/knadh/listmonk/internal/auth" "github.com/knadh/listmonk/internal/subimporter" "github.com/knadh/listmonk/models" "github.com/labstack/echo/v4" @@ -68,12 +69,17 @@ func handleGetSubscriber(c echo.Context) error { var ( app = c.Get("app").(*App) id, _ = strconv.Atoi(c.Param("id")) + user = c.Get(auth.UserKey).(models.User) ) if id < 1 { return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("globals.messages.invalidID")) } + if err := hasSubPerm(user, []int{id}, app); err != nil { + return err + } + out, err := app.core.GetSubscriber(id, "", "") if err != nil { return err @@ -85,8 +91,9 @@ func handleGetSubscriber(c echo.Context) error { // handleQuerySubscribers handles querying subscribers based on an arbitrary SQL expression. func handleQuerySubscribers(c echo.Context) error { var ( - app = c.Get("app").(*App) - pg = app.paginator.NewFromURL(c.Request().URL.Query()) + app = c.Get("app").(*App) + user = c.Get(auth.UserKey).(models.User) + pg = app.paginator.NewFromURL(c.Request().URL.Query()) // The "WHERE ?" bit. query = sanitizeSQLExp(c.FormValue("query")) @@ -96,10 +103,10 @@ func handleQuerySubscribers(c echo.Context) error { out models.PageResults ) - // Limit the subscribers to specific lists? - listIDs, err := getQueryInts("list_id", c.QueryParams()) + // Filter list IDs by permission. + listIDs, err := filterListQeryByPerm(c.QueryParams(), user, app) if err != nil { - return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("globals.messages.invalidID")) + return err } res, total, err := app.core.QuerySubscribers(query, listIDs, subStatus, order, orderBy, pg.Offset, pg.Limit) @@ -119,16 +126,17 @@ func handleQuerySubscribers(c echo.Context) error { // handleExportSubscribers handles querying subscribers based on an arbitrary SQL expression. func handleExportSubscribers(c echo.Context) error { var ( - app = c.Get("app").(*App) + app = c.Get("app").(*App) + user = c.Get(auth.UserKey).(models.User) // The "WHERE ?" bit. query = sanitizeSQLExp(c.FormValue("query")) ) - // Limit the subscribers to specific lists? - listIDs, err := getQueryInts("list_id", c.QueryParams()) + // Filter list IDs by permission. + listIDs, err := filterListQeryByPerm(c.QueryParams(), user, app) if err != nil { - return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("globals.messages.invalidID")) + return err } // Export only specific subscriber IDs? @@ -187,7 +195,9 @@ loop: // handleCreateSubscriber handles the creation of a new subscriber. func handleCreateSubscriber(c echo.Context) error { var ( - app = c.Get("app").(*App) + app = c.Get("app").(*App) + user = c.Get(auth.UserKey).(models.User) + req subimporter.SubReq ) @@ -202,8 +212,11 @@ func handleCreateSubscriber(c echo.Context) error { return echo.NewHTTPError(http.StatusBadRequest, err.Error()) } + // Filter lists against the current user's permitted lists. + listIDs := filterListsByPerm(req.Lists, user) + // Insert the subscriber into the DB. - sub, _, err := app.core.InsertSubscriber(req.Subscriber, req.Lists, req.ListUUIDs, req.PreconfirmSubs) + sub, _, err := app.core.InsertSubscriber(req.Subscriber, listIDs, nil, req.PreconfirmSubs) if err != nil { return err } @@ -214,7 +227,9 @@ func handleCreateSubscriber(c echo.Context) error { // handleUpdateSubscriber handles modification of a subscriber. func handleUpdateSubscriber(c echo.Context) error { var ( - app = c.Get("app").(*App) + app = c.Get("app").(*App) + user = c.Get(auth.UserKey).(models.User) + id, _ = strconv.Atoi(c.Param("id")) req struct { models.Subscriber @@ -242,7 +257,10 @@ func handleUpdateSubscriber(c echo.Context) error { return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("subscribers.invalidName")) } - out, _, err := app.core.UpdateSubscriberWithLists(id, req.Subscriber, req.Lists, nil, req.PreconfirmSubs, true) + // Filter lists against the current user's permitted lists. + listIDs := filterListsByPerm(req.Lists, user) + + out, _, err := app.core.UpdateSubscriberWithLists(id, req.Subscriber, listIDs, nil, req.PreconfirmSubs, true) if err != nil { return err } @@ -318,7 +336,9 @@ func handleBlocklistSubscribers(c echo.Context) error { // It takes either an ID in the URI, or a list of IDs in the request body. func handleManageSubscriberLists(c echo.Context) error { var ( - app = c.Get("app").(*App) + app = c.Get("app").(*App) + user = c.Get(auth.UserKey).(models.User) + pID = c.Param("id") subIDs []int ) @@ -347,15 +367,18 @@ func handleManageSubscriberLists(c echo.Context) error { return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("subscribers.errorNoListsGiven")) } + // Filter lists against the current user's permitted lists. + listIDs := filterListsByPerm(req.TargetListIDs, user) + // Action. var err error switch req.Action { case "add": - err = app.core.AddSubscriptions(subIDs, req.TargetListIDs, req.Status) + err = app.core.AddSubscriptions(subIDs, listIDs, req.Status) case "remove": - err = app.core.DeleteSubscriptions(subIDs, req.TargetListIDs) + err = app.core.DeleteSubscriptions(subIDs, listIDs) case "unsubscribe": - err = app.core.UnsubscribeLists(subIDs, req.TargetListIDs, nil) + err = app.core.UnsubscribeLists(subIDs, listIDs, nil) default: return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("subscribers.invalidAction")) } @@ -446,7 +469,9 @@ func handleBlocklistSubscribersByQuery(c echo.Context) error { // from one or more lists based on an arbitrary SQL expression. func handleManageSubscriberListsByQuery(c echo.Context) error { var ( - app = c.Get("app").(*App) + app = c.Get("app").(*App) + user = c.Get(auth.UserKey).(models.User) + req subQueryReq ) @@ -458,15 +483,19 @@ func handleManageSubscriberListsByQuery(c echo.Context) error { app.i18n.T("subscribers.errorNoListsGiven")) } + // Filter lists against the current user's permitted lists. + sourceListIDs := filterListsByPerm(req.ListIDs, user) + targetListIDs := filterListsByPerm(req.TargetListIDs, user) + // Action. var err error switch req.Action { case "add": - err = app.core.AddSubscriptionsByQuery(req.Query, req.ListIDs, req.TargetListIDs, req.Status, req.SubscriptionStatus) + err = app.core.AddSubscriptionsByQuery(req.Query, sourceListIDs, targetListIDs, req.Status, req.SubscriptionStatus) case "remove": - err = app.core.DeleteSubscriptionsByQuery(req.Query, req.ListIDs, req.TargetListIDs, req.SubscriptionStatus) + err = app.core.DeleteSubscriptionsByQuery(req.Query, sourceListIDs, targetListIDs, req.SubscriptionStatus) case "unsubscribe": - err = app.core.UnsubscribeListsByQuery(req.Query, req.ListIDs, req.TargetListIDs, req.SubscriptionStatus) + err = app.core.UnsubscribeListsByQuery(req.Query, sourceListIDs, targetListIDs, req.SubscriptionStatus) default: return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("subscribers.invalidAction")) } @@ -632,3 +661,67 @@ func sendOptinConfirmationHook(app *App) func(sub models.Subscriber, listIDs []i return len(lists), nil } } + +// hasSubPerm checks whether the current user has permission to access the given list +// of subscriber IDs. +func hasSubPerm(u models.User, subIDs []int, app *App) error { + if u.RoleID == auth.SuperAdminRoleID { + return nil + } + + if _, ok := u.PermissionsMap["subscribers:get_all"]; ok { + return nil + } + + res, err := app.core.HasSubscriberLists(subIDs, u.GetListIDs) + if err != nil { + return err + } + + for id, has := range res { + if !has { + return echo.NewHTTPError(http.StatusForbidden, app.i18n.Ts("globals.messages.permissionDenied", "name", fmt.Sprintf("subscriber: %d", id))) + } + } + + return nil +} + +func filterListQeryByPerm(qp url.Values, user models.User, app *App) ([]int, error) { + var listIDs []int + + // If there are incoming list query params, filter them by permission. + if qp.Has("list_id") { + ids, err := getQueryInts("list_id", qp) + if err != nil { + return nil, echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("globals.messages.invalidID")) + } + + listIDs = []int{} + for _, id := range ids { + if _, ok := user.ListPermissionsMap[id]; ok { + listIDs = append(listIDs, id) + } + } + } else { + // There are no incoming params. If the user doesn't have permission to get all subscribers, + // filter by the lists they have access to. + if _, ok := user.PermissionsMap["subscribers:get_all"]; !ok { + listIDs = user.GetListIDs + } + } + + return listIDs, nil +} + +// filterListsByPerm filters the given list IDs against the given user's permitted lists. +func filterListsByPerm(listIDs []int, user models.User) []int { + out := make([]int, 0, len(listIDs)) + for _, id := range listIDs { + if _, ok := user.ListPermissionsMap[id]; ok { + listIDs = append(listIDs, id) + } + } + + return out +} diff --git a/frontend/src/components/Navigation.vue b/frontend/src/components/Navigation.vue index 89e799b2..4c229f1e 100644 --- a/frontend/src/components/Navigation.vue +++ b/frontend/src/components/Navigation.vue @@ -15,7 +15,7 @@ - - + + diff --git a/i18n/en.json b/i18n/en.json index 2a17880d..26fb4aa7 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -180,6 +180,7 @@ "globals.messages.errorCreating": "Error creating {name}: {error}", "globals.messages.errorDeleting": "Error deleting {name}: {error}", "globals.messages.errorFetching": "Error fetching {name}: {error}", + "globals.messages.permissionDenied": "Permission denied: {name}", "globals.messages.errorInvalidIDs": "One or more IDs are invalid: {error}", "globals.messages.errorUUID": "Error generating UUID: {error}", "globals.messages.errorUpdating": "Error updating {name}: {error}", diff --git a/internal/core/subscribers.go b/internal/core/subscribers.go index 64c0b550..759c1119 100644 --- a/internal/core/subscribers.go +++ b/internal/core/subscribers.go @@ -43,6 +43,27 @@ func (c *Core) GetSubscriber(id int, uuid, email string) (models.Subscriber, err return out[0], nil } +// HasSubscriberLists checks if the given subscribers have at least one of the given lists. +func (c *Core) HasSubscriberLists(subIDs []int, listIDs []int) (map[int]bool, error) { + res := []struct { + SubID int `db:"subscriber_id"` + Has bool `db:"has"` + }{} + + if err := c.q.HasSubscriberLists.Select(&res, pq.Array(subIDs), pq.Array(listIDs)); err != nil { + c.log.Printf("error fetching subscriber: %v", err) + return nil, echo.NewHTTPError(http.StatusInternalServerError, + c.i18n.Ts("globals.messages.errorFetching", "name", "{globals.terms.subscriber}", "error", pqErrMsg(err))) + } + + out := make(map[int]bool, len(res)) + for _, r := range res { + out[r.SubID] = r.Has + } + + return out, nil +} + // GetSubscribersByEmail fetches a subscriber by one of the given params. func (c *Core) GetSubscribersByEmail(emails []string) (models.Subscribers, error) { var out models.Subscribers diff --git a/models/queries.go b/models/queries.go index bba8e2d8..92315c0c 100644 --- a/models/queries.go +++ b/models/queries.go @@ -18,6 +18,7 @@ type Queries struct { UpsertSubscriber *sqlx.Stmt `query:"upsert-subscriber"` UpsertBlocklistSubscriber *sqlx.Stmt `query:"upsert-blocklist-subscriber"` GetSubscriber *sqlx.Stmt `query:"get-subscriber"` + HasSubscriberLists *sqlx.Stmt `query:"has-subscriber-list"` GetSubscribersByEmails *sqlx.Stmt `query:"get-subscribers-by-emails"` GetSubscriberLists *sqlx.Stmt `query:"get-subscriber-lists"` GetSubscriptions *sqlx.Stmt `query:"get-subscriptions"` diff --git a/permissions.json b/permissions.json index 235a9358..2f86958c 100644 --- a/permissions.json +++ b/permissions.json @@ -11,8 +11,8 @@ "group": "subscribers", "permissions": [ - "subscribers:get_by_list", "subscribers:get", + "subscribers:get_all", "subscribers:manage", "subscribers:import", "subscribers:sql_query", diff --git a/queries.sql b/queries.sql index b6e6c17b..e31a7702 100644 --- a/queries.sql +++ b/queries.sql @@ -8,6 +8,16 @@ SELECT * FROM subscribers WHERE WHEN $3 != '' THEN email = $3 END; +-- name: has-subscriber-list +-- Used for checking access permission by list. +SELECT s.id AS subscriber_id, + CASE + WHEN EXISTS (SELECT 1 FROM subscriber_lists sl WHERE sl.subscriber_id = s.id AND sl.list_id = ANY($2)) + THEN TRUE + ELSE FALSE + END AS has +FROM subscribers s WHERE s.id = ANY($1); + -- name: get-subscribers-by-emails -- Get subscribers by emails. SELECT * FROM subscribers WHERE email=ANY($1);