Fix bug in test

cmd/access.go

@@ -197,7 +197,7 @@ func showUsers(c *cli.Context, manager *user.Manager, users []*user.User) error
 		}
 		provisioned := ""
 		if u.Provisioned {
-			provisioned = ", provisioned user"
+			provisioned = ", server config"
 		}
 		fmt.Fprintf(c.App.ErrWriter, "user %s (role: %s, tier: %s%s)\n", u.Name, u.Role, tier, provisioned)
 		if u.Role == user.RoleAdmin {
@@ -206,7 +206,7 @@ func showUsers(c *cli.Context, manager *user.Manager, users []*user.User) error
 			for _, grant := range grants {
 				grantProvisioned := ""
 				if grant.Provisioned {
-					grantProvisioned = ", provisioned access entry"
+					grantProvisioned = " (server config)"
 				}
 				if grant.Permission.IsReadWrite() {
 					fmt.Fprintf(c.App.ErrWriter, "- read-write access to topic %s%s\n", grant.TopicPattern, grantProvisioned)

docs/releases.md

@@ -1456,7 +1456,8 @@ and the [ntfy Android app](https://github.com/binwiederhier/ntfy-android/release
 
 **Features:**
 
-* Enhanced JSON webhook support via [pre-defined](publish.md#pre-defined-templates) and [custom templates](publish.md#custom-templates) ([#1390](https://github.com/binwiederhier/ntfy/pull/1390))
+* [Declarative users and ACL entries](config.md#users-and-roles) ([#464](https://github.com/binwiederhier/ntfy/issues/464), [#1384](https://github.com/binwiederhier/ntfy/pull/1384), thanks to [pinpox](https://github.com/pinpox) for reporting, to [@wunter8](https://github.com/wunter8) for reviewing)
+* [Pre-defined templates](publish.md#pre-defined-templates) and [custom templates](publish.md#custom-templates) for enhanced JSON webhook support ([#1390](https://github.com/binwiederhier/ntfy/pull/1390))
 * Support of advanced [template functions](publish.md#template-functions) based on the [Sprig](https://github.com/Masterminds/sprig) library ([#1121](https://github.com/binwiederhier/ntfy/issues/1121), thanks to [@davidatkinsondoyle](https://github.com/davidatkinsondoyle) for reporting, to [@wunter8](https://github.com/wunter8) for implementing, and to the Sprig team for their work)
 
 ### ntfy Android app v1.16.1 (UNRELEASED)

user/manager.go

@@ -1484,19 +1484,25 @@ func (a *Manager) allowAccessTx(tx *sql.Tx, username string, topicPattern string
 // ResetAccess removes an access control list entry for a specific username/topic, or (if topic is
 // empty) for an entire user. The parameter topicPattern may include wildcards (*).
 func (a *Manager) ResetAccess(username string, topicPattern string) error {
+	return execTx(a.db, func(tx *sql.Tx) error {
+		return a.resetAccessTx(tx, username, topicPattern)
+	})
+}
+
+func (a *Manager) resetAccessTx(tx *sql.Tx, username string, topicPattern string) error {
 	if !AllowedUsername(username) && username != Everyone && username != "" {
 		return ErrInvalidArgument
 	} else if !AllowedTopicPattern(topicPattern) && topicPattern != "" {
 		return ErrInvalidArgument
 	}
 	if username == "" && topicPattern == "" {
-		_, err := a.db.Exec(deleteAllAccessQuery, username)
+		_, err := tx.Exec(deleteAllAccessQuery, username)
 		return err
 	} else if topicPattern == "" {
-		_, err := a.db.Exec(deleteUserAccessQuery, username, username)
+		_, err := tx.Exec(deleteUserAccessQuery, username, username)
 		return err
 	}
-	_, err := a.db.Exec(deleteTopicAccessQuery, username, username, toSQLWildcard(topicPattern))
+	_, err := tx.Exec(deleteTopicAccessQuery, username, username, toSQLWildcard(topicPattern))
 	return err
 }
 
@@ -1734,7 +1740,18 @@ func (a *Manager) maybeProvisionUsersAndAccess() error {
 			return err
 		}
 		for username, grants := range a.config.Access {
+			user, exists := util.Find(a.config.Users, func(u *User) bool {
+				return u.Name == username
+			})
+			if !exists && username != Everyone {
+				return fmt.Errorf("user %s is not a provisioned user, refusing to add ACL entry", username)
+			} else if user != nil && user.Role == RoleAdmin {
+				return fmt.Errorf("adding access control entries is not allowed for admin roles for user %s", username)
+			}
 			for _, grant := range grants {
+				if err := a.resetAccessTx(tx, username, grant.TopicPattern); err != nil {
+					return fmt.Errorf("failed to reset access for user %s and topic %s: %v", username, grant.TopicPattern, err)
+				}
 				if err := a.allowAccessTx(tx, username, grant.TopicPattern, grant.Permission, true); err != nil {
 					return err
 				}
@@ -1951,10 +1968,8 @@ func execTx(db *sql.DB, f func(tx *sql.Tx) error) error {
 	if err != nil {
 		return err
 	}
+	defer tx.Rollback()
 	if err := f(tx); err != nil {
-		if e := tx.Rollback(); e != nil {
-			return err
-		}
 		return err
 	}
 	return tx.Commit()

user/manager_test.go

@@ -1193,37 +1193,86 @@ func TestManager_WithProvisionedUsers(t *testing.T) {
 	require.Equal(t, "*", users[1].Name)
 }
 
-func TestManager_DoNotUpdateNonProvisionedUsers(t *testing.T) {
+func TestManager_UpdateNonProvisionedUsersToProvisionedUsers(t *testing.T) {
 	f := filepath.Join(t.TempDir(), "user.db")
 	conf := &Config{
 		Filename:         f,
 		DefaultAccess:    PermissionReadWrite,
 		ProvisionEnabled: true,
 		Users:            []*User{},
-		Access:           map[string][]*Grant{},
+		Access: map[string][]*Grant{
+			Everyone: {
+				{TopicPattern: "food", Permission: PermissionRead},
+			},
+		},
 	}
 	a, err := NewManager(conf)
 	require.Nil(t, err)
 
 	// Manually add user
 	require.Nil(t, a.AddUser("philuser", "manual", RoleUser, false))
+	require.Nil(t, a.AllowAccess("philuser", "stats", PermissionReadWrite))
+	require.Nil(t, a.AllowAccess("philuser", "food", PermissionReadWrite))
+
+	users, err := a.Users()
+	require.Nil(t, err)
+	require.Len(t, users, 2)
+	require.Equal(t, "philuser", users[0].Name)
+	require.Equal(t, RoleUser, users[0].Role)
+	require.False(t, users[0].Provisioned) // Manually added
+
+	grants, err := a.Grants("philuser")
+	require.Nil(t, err)
+	require.Equal(t, 2, len(grants))
+	require.Equal(t, "stats", grants[0].TopicPattern)
+	require.Equal(t, PermissionReadWrite, grants[0].Permission)
+	require.False(t, grants[0].Provisioned) // Manually added
+	require.Equal(t, "food", grants[1].TopicPattern)
+	require.Equal(t, PermissionReadWrite, grants[1].Permission)
+	require.False(t, grants[1].Provisioned) // Manually added
+
+	grants, err = a.Grants(Everyone)
+	require.Nil(t, err)
+	require.Equal(t, 1, len(grants))
+	require.Equal(t, "food", grants[0].TopicPattern)
+	require.Equal(t, PermissionRead, grants[0].Permission)
+	require.True(t, grants[0].Provisioned) // Provisioned entry
 
 	// Re-open the DB (second app start)
 	require.Nil(t, a.db.Close())
 	conf.Users = []*User{
-		{Name: "philuser", Hash: "$2a$10$AAAU21sX1uhZamTLJXHuxgVC0Z/GKISibrKCLohPgtG7yIxSk4C", Role: RoleAdmin},
+		{Name: "philuser", Hash: "$2a$10$AAAU21sX1uhZamTLJXHuxgVC0Z/GKISibrKCLohPgtG7yIxSk4C", Role: RoleUser},
+	}
+	conf.Access = map[string][]*Grant{
+		"philuser": {
+			{TopicPattern: "stats", Permission: PermissionReadWrite},
+		},
 	}
-	conf.Access = map[string][]*Grant{}
 	a, err = NewManager(conf)
 	require.Nil(t, err)
 
-	// Check that the provisioned users are there
-	users, err := a.Users()
+	// Check that the user was "upgraded" to a provisioned user
+	users, err = a.Users()
 	require.Nil(t, err)
 	require.Len(t, users, 2)
 	require.Equal(t, "philuser", users[0].Name)
-	require.Equal(t, RoleUser, users[0].Role) // Should not have been updated
-	require.NotEqual(t, "$2a$10$AAAU21sX1uhZamTLJXHuxgVC0Z/GKISibrKCLohPgtG7yIxSk4C", users[0].Hash)
+	require.Equal(t, RoleUser, users[0].Role)
+	require.Equal(t, "$2a$10$AAAU21sX1uhZamTLJXHuxgVC0Z/GKISibrKCLohPgtG7yIxSk4C", users[0].Hash)
+	require.True(t, users[0].Provisioned) // Updated to provisioned!
+
+	grants, err = a.Grants("philuser")
+	require.Nil(t, err)
+	require.Equal(t, 2, len(grants))
+	require.Equal(t, "stats", grants[0].TopicPattern)
+	require.Equal(t, PermissionReadWrite, grants[0].Permission)
+	require.True(t, grants[0].Provisioned) // Updated to provisioned!
+	require.Equal(t, "food", grants[1].TopicPattern)
+	require.Equal(t, PermissionReadWrite, grants[1].Permission)
+	require.False(t, grants[1].Provisioned) // Manually added grants stay!
+
+	grants, err = a.Grants(Everyone)
+	require.Nil(t, err)
+	require.Empty(t, grants)
 }
 
 func TestToFromSQLWildcard(t *testing.T) {