Add list permission check to subscriber calls.

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"))
 	}
 }

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)))
 			}
 		}
 	}

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
+}

frontend/src/components/Navigation.vue

@@ -15,7 +15,7 @@
     <b-menu-item v-if="$can('subscribers:*')" :expanded="activeGroup.subscribers" :active="activeGroup.subscribers"
       data-cy="subscribers" @update:active="(state) => toggleGroup('subscribers', state)" icon="account-multiple"
       :label="$t('globals.terms.subscribers')">
-      <b-menu-item v-if="$can('subscribers:get')" :to="{ name: 'subscribers' }" tag="router-link"
+      <b-menu-item v-if="$can('subscribers:get_all', 'subscribers:get')" :to="{ name: 'subscribers' }" tag="router-link"
         :active="activeItem.subscribers" data-cy="all-subscribers" icon="account-multiple"
         :label="$t('menu.allSubscribers')" />
       <b-menu-item v-if="$can('subscribers:import')" :to="{ name: 'import' }" tag="router-link"

frontend/src/views/Lists.vue

@@ -79,10 +79,15 @@
 
       <b-table-column v-slot="props" field="subscriber_count" :label="$t('globals.terms.subscribers')"
         header-class="cy-subscribers" numeric sortable centered>
-        <router-link :to="`/subscribers/lists/${props.row.id}`">
+        <template v-if="$can('subscribers:get_all', 'subscribers:get')">
+          <router-link :to="`/subscribers/lists/${props.row.id}`">
+            {{ $utils.formatNumber(props.row.subscriberCount) }}
+            <span class="is-size-7 view">{{ $t('globals.buttons.view') }}</span>
+          </router-link>
+        </template>
+        <template v-else>
           {{ $utils.formatNumber(props.row.subscriberCount) }}
-          <span class="is-size-7 view">{{ $t('globals.buttons.view') }}</span>
-        </router-link>
+        </template>
       </b-table-column>
 
       <b-table-column v-slot="props" field="subscriber_counts" header-class="cy-subscribers" width="10%">

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}",

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

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"`

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",

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);