Simplify API endpoints; add endpoint tests

server/server.go

@@ -50,6 +50,8 @@ import (
 			- figure out what settings are "web" or "phone"
 		UI:
 		- Subscription dotmenu dropdown: Move to nav bar, or make same as profile dropdown
+		rate limiting:
+		- login/account endpoints
 		Pages:
 		- Home
 		- Password reset
@@ -342,28 +344,28 @@ func (s *Server) handleInternal(w http.ResponseWriter, r *http.Request, v *visit
 		return s.handleHealth(w, r, v)
 	} else if r.Method == http.MethodGet && r.URL.Path == webConfigPath {
 		return s.ensureWebEnabled(s.handleWebConfig)(w, r, v)
-	} else if r.Method == http.MethodPost && r.URL.Path == accountTokenPath {
-		return s.ensureAccountsEnabled(s.handleAccountTokenIssue)(w, r, v)
 	} else if r.Method == http.MethodPost && r.URL.Path == accountPath {
-		return s.ensureAccountsEnabled(s.handleAccountCreate)(w, r, v)
+		return s.ensureUserManager(s.handleAccountCreate)(w, r, v)
+	} else if r.Method == http.MethodPost && r.URL.Path == accountTokenPath {
+		return s.ensureUser(s.handleAccountTokenIssue)(w, r, v)
 	} else if r.Method == http.MethodGet && r.URL.Path == accountPath {
 		return s.handleAccountGet(w, r, v) // Allowed by anonymous
 	} else if r.Method == http.MethodDelete && r.URL.Path == accountPath {
-		return s.ensureWithAccount(s.handleAccountDelete)(w, r, v)
+		return s.ensureUser(s.handleAccountDelete)(w, r, v)
 	} else if r.Method == http.MethodPost && r.URL.Path == accountPasswordPath {
-		return s.ensureWithAccount(s.handleAccountPasswordChange)(w, r, v)
+		return s.ensureUser(s.handleAccountPasswordChange)(w, r, v)
 	} else if r.Method == http.MethodPatch && r.URL.Path == accountTokenPath {
-		return s.ensureWithAccount(s.handleAccountTokenExtend)(w, r, v)
+		return s.ensureUser(s.handleAccountTokenExtend)(w, r, v)
 	} else if r.Method == http.MethodDelete && r.URL.Path == accountTokenPath {
-		return s.ensureWithAccount(s.handleAccountTokenDelete)(w, r, v)
+		return s.ensureUser(s.handleAccountTokenDelete)(w, r, v)
 	} else if r.Method == http.MethodPatch && r.URL.Path == accountSettingsPath {
-		return s.ensureWithAccount(s.handleAccountSettingsChange)(w, r, v)
+		return s.ensureUser(s.handleAccountSettingsChange)(w, r, v)
 	} else if r.Method == http.MethodPost && r.URL.Path == accountSubscriptionPath {
-		return s.ensureWithAccount(s.handleAccountSubscriptionAdd)(w, r, v)
+		return s.ensureUser(s.handleAccountSubscriptionAdd)(w, r, v)
 	} else if r.Method == http.MethodPatch && accountSubscriptionSingleRegex.MatchString(r.URL.Path) {
-		return s.ensureWithAccount(s.handleAccountSubscriptionChange)(w, r, v)
+		return s.ensureUser(s.handleAccountSubscriptionChange)(w, r, v)
 	} else if r.Method == http.MethodDelete && accountSubscriptionSingleRegex.MatchString(r.URL.Path) {
-		return s.ensureWithAccount(s.handleAccountSubscriptionDelete)(w, r, v)
+		return s.ensureUser(s.handleAccountSubscriptionDelete)(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) {
@@ -1403,7 +1405,7 @@ func (s *Server) ensureWebEnabled(next handleFunc) handleFunc {
 	}
 }
 
-func (s *Server) ensureAccountsEnabled(next handleFunc) handleFunc {
+func (s *Server) ensureUserManager(next handleFunc) handleFunc {
 	return func(w http.ResponseWriter, r *http.Request, v *visitor) error {
 		if s.userManager == nil {
 			return errHTTPNotFound
@@ -1412,8 +1414,8 @@ func (s *Server) ensureAccountsEnabled(next handleFunc) handleFunc {
 	}
 }
 
-func (s *Server) ensureWithAccount(next handleFunc) handleFunc {
+func (s *Server) ensureUser(next handleFunc) handleFunc {
-	return s.ensureAccountsEnabled(func(w http.ResponseWriter, r *http.Request, v *visitor) error {
+	return s.ensureUserManager(func(w http.ResponseWriter, r *http.Request, v *visitor) error {
 		if v.user == nil {
 			return errHTTPNotFound
 		}

server/server_account.go

@@ -5,10 +5,13 @@ import (
 	"errors"
 	"heckel.io/ntfy/user"
 	"heckel.io/ntfy/util"
-	"io"
 	"net/http"
 )
 
+const (
+	jsonBodyBytesLimit = 4096
+)
+
 func (s *Server) handleAccountCreate(w http.ResponseWriter, r *http.Request, v *visitor) error {
 	admin := v.user != nil && v.user.Role == user.RoleAdmin
 	if !admin {
@@ -18,15 +21,10 @@ func (s *Server) handleAccountCreate(w http.ResponseWriter, r *http.Request, v *
 			return errHTTPUnauthorized // Cannot create account from user context
 		}
 	}
-	body, err := util.Peek(r.Body, 4096) // FIXME
+	newAccount, err := util.ReadJSONWithLimit[apiAccountCreateRequest](r.Body, jsonBodyBytesLimit)
 	if err != nil {
 		return err
 	}
-	defer r.Body.Close()
-	var newAccount apiAccountCreateRequest
-	if err := json.NewDecoder(body).Decode(&newAccount); err != nil {
-		return err
-	}
 	if existingUser, _ := s.userManager.User(newAccount.Username); existingUser != nil {
 		return errHTTPConflictUserExists
 	}
@@ -38,13 +36,10 @@ func (s *Server) handleAccountCreate(w http.ResponseWriter, r *http.Request, v *
 	}
 	w.Header().Set("Content-Type", "application/json")
 	w.Header().Set("Access-Control-Allow-Origin", "*") // FIXME remove this
-	// FIXME return something
 	return nil
 }
 
 func (s *Server) handleAccountGet(w http.ResponseWriter, r *http.Request, v *visitor) error {
-	w.Header().Set("Content-Type", "application/json")
-	w.Header().Set("Access-Control-Allow-Origin", "*") // FIXME remove this
 	stats, err := v.Info()
 	if err != nil {
 		return err
@@ -105,6 +100,8 @@ func (s *Server) handleAccountGet(w http.ResponseWriter, r *http.Request, v *vis
 			Upgradable: true,
 		}
 	}
+	w.Header().Set("Content-Type", "application/json")
+	w.Header().Set("Access-Control-Allow-Origin", "*") // FIXME remove this
 	if err := json.NewEncoder(w).Encode(response); err != nil {
 		return err
 	}
@@ -125,32 +122,20 @@ func (s *Server) handleAccountDelete(w http.ResponseWriter, r *http.Request, v *
 }
 
 func (s *Server) handleAccountPasswordChange(w http.ResponseWriter, r *http.Request, v *visitor) error {
-	if v.user == nil {
+	newPassword, err := util.ReadJSONWithLimit[apiAccountCreateRequest](r.Body, jsonBodyBytesLimit)
-		return errHTTPUnauthorized
-	}
-	body, err := util.Peek(r.Body, 4096) // FIXME
 	if err != nil {
 		return err
 	}
-	defer r.Body.Close()
-	var newPassword apiAccountCreateRequest // Re-use!
-	if err := json.NewDecoder(body).Decode(&newPassword); err != nil {
-		return err
-	}
 	if err := s.userManager.ChangePassword(v.user.Name, newPassword.Password); err != nil {
 		return err
 	}
 	w.Header().Set("Content-Type", "application/json")
 	w.Header().Set("Access-Control-Allow-Origin", "*") // FIXME remove this
-	// FIXME return something
 	return nil
 }
 
 func (s *Server) handleAccountTokenIssue(w http.ResponseWriter, r *http.Request, v *visitor) error {
 	// TODO rate limit
-	if v.user == nil {
-		return errHTTPUnauthorized
-	}
 	token, err := s.userManager.CreateToken(v.user)
 	if err != nil {
 		return err
@@ -192,7 +177,7 @@ func (s *Server) handleAccountTokenExtend(w http.ResponseWriter, r *http.Request
 
 func (s *Server) handleAccountTokenDelete(w http.ResponseWriter, r *http.Request, v *visitor) error {
 	// TODO rate limit
-	if v.user == nil || v.user.Token == "" {
+	if v.user.Token == "" {
 		return errHTTPUnauthorized
 	}
 	if err := s.userManager.RemoveToken(v.user); err != nil {
@@ -203,20 +188,10 @@ func (s *Server) handleAccountTokenDelete(w http.ResponseWriter, r *http.Request
 }
 
 func (s *Server) handleAccountSettingsChange(w http.ResponseWriter, r *http.Request, v *visitor) error {
-	if v.user == nil {
+	newPrefs, err := util.ReadJSONWithLimit[user.Prefs](r.Body, jsonBodyBytesLimit)
-		return errors.New("no user")
-	}
-	w.Header().Set("Content-Type", "application/json")
-	w.Header().Set("Access-Control-Allow-Origin", "*") // FIXME remove this
-	body, err := util.Peek(r.Body, 4096)               // FIXME
 	if err != nil {
 		return err
 	}
-	defer r.Body.Close()
-	var newPrefs user.Prefs
-	if err := json.NewDecoder(body).Decode(&newPrefs); err != nil {
-		return err
-	}
 	if v.user.Prefs == nil {
 		v.user.Prefs = &user.Prefs{}
 	}
@@ -238,14 +213,16 @@ func (s *Server) handleAccountSettingsChange(w http.ResponseWriter, r *http.Requ
 			prefs.Notification.MinPriority = newPrefs.Notification.MinPriority
 		}
 	}
-	return s.userManager.ChangeSettings(v.user)
+	if err := s.userManager.ChangeSettings(v.user); 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) handleAccountSubscriptionAdd(w http.ResponseWriter, r *http.Request, v *visitor) error {
-	if v.user == nil {
+	newSubscription, err := util.ReadJSONWithLimit[user.Subscription](r.Body, jsonBodyBytesLimit)
-		return errors.New("no user")
-	}
-	newSubscription, err := readJSONBody[user.Subscription](r.Body)
 	if err != nil {
 		return err
 	}
@@ -275,16 +252,11 @@ func (s *Server) handleAccountSubscriptionAdd(w http.ResponseWriter, r *http.Req
 }
 
 func (s *Server) handleAccountSubscriptionChange(w http.ResponseWriter, r *http.Request, v *visitor) error {
-	if v.user == nil {
-		return errors.New("no user") // FIXME s.ensureUser
-	}
-	w.Header().Set("Content-Type", "application/json")
-	w.Header().Set("Access-Control-Allow-Origin", "*") // FIXME remove this
 	matches := accountSubscriptionSingleRegex.FindStringSubmatch(r.URL.Path)
 	if len(matches) != 2 {
 		return errHTTPInternalErrorInvalidFilePath // FIXME
 	}
-	updatedSubscription, err := readJSONBody[user.Subscription](r.Body)
+	updatedSubscription, err := util.ReadJSONWithLimit[user.Subscription](r.Body, jsonBodyBytesLimit)
 	if err != nil {
 		return err
 	}
@@ -342,16 +314,3 @@ func (s *Server) handleAccountSubscriptionDelete(w http.ResponseWriter, r *http.
 	}
 	return nil
 }
-
-func readJSONBody[T any](body io.ReadCloser) (*T, error) {
-	body, err := util.Peek(body, 4096)
-	if err != nil {
-		return nil, err
-	}
-	defer body.Close()
-	var obj T
-	if err := json.NewDecoder(body).Decode(&obj); err != nil {
-		return nil, err
-	}
-	return &obj, nil
-}

server/server_account_test.go

@@ -0,0 +1,44 @@
+package server
+
+import (
+	"github.com/stretchr/testify/require"
+	"heckel.io/ntfy/util"
+	"io"
+	"testing"
+	"time"
+)
+
+func TestAccount_Create_Success(t *testing.T) {
+	conf := newTestConfigWithUsers(t)
+	conf.EnableSignup = true
+	s := newTestServer(t, conf)
+
+	rr := request(t, s, "POST", "/v1/account", `{"username":"phil", "password":"mypass"}`, nil)
+	require.Equal(t, 200, rr.Code)
+
+	rr = request(t, s, "POST", "/v1/account/token", "", map[string]string{
+		"Authorization": util.BasicAuth("phil", "mypass"),
+	})
+	require.Equal(t, 200, rr.Code)
+	token, _ := util.ReadJSON[apiAccountTokenResponse](io.NopCloser(rr.Body))
+	require.NotEmpty(t, token.Token)
+	require.True(t, time.Now().Add(71*time.Hour).Unix() < token.Expires)
+
+	rr = request(t, s, "GET", "/v1/account", "", map[string]string{
+		"Authorization": util.BearerAuth(token.Token),
+	})
+	require.Equal(t, 200, rr.Code)
+	account, _ := util.ReadJSON[apiAccountResponse](io.NopCloser(rr.Body))
+	require.Equal(t, "phil", account.Username)
+	require.Equal(t, "user", account.Role)
+}
+
+func TestAccount_Create_Disabled(t *testing.T) {
+	conf := newTestConfigWithUsers(t)
+	conf.EnableSignup = false
+	s := newTestServer(t, conf)
+
+	rr := request(t, s, "POST", "/v1/account", `{"username":"phil", "password":"mypass"}`, nil)
+	require.Equal(t, 400, rr.Code)
+	require.Equal(t, 40022, toHTTPError(t, rr.Body.String()).Code)
+}

server/server_test.go

@@ -1446,6 +1446,12 @@ func newTestConfig(t *testing.T) *Config {
 	return conf
 }
 
+func newTestConfigWithUsers(t *testing.T) *Config {
+	conf := newTestConfig(t)
+	conf.AuthFile = filepath.Join(t.TempDir(), "user.db")
+	return conf
+}
+
 func newTestServer(t *testing.T, config *Config) *Server {
 	server, err := New(config)
 	if err != nil {

user/manager.go

@@ -336,8 +336,7 @@ func (a *Manager) resolvePerms(read, write bool, perm Permission) error {
 	return ErrUnauthorized
 }
 
-// AddUser adds a user with the given username, password and role. The password should be hashed
+// AddUser adds a user with the given username, password and role
-// before it is stored in a persistence layer.
 func (a *Manager) AddUser(username, password string, role Role) error {
 	if !AllowedUsername(username) || !AllowedRole(role) {
 		return ErrInvalidArgument

util/util.go

@@ -251,6 +251,11 @@ func BasicAuth(user, pass string) string {
 	return fmt.Sprintf("Basic %s", base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", user, pass))))
 }
 
+// BearerAuth encodes the Authorization header value for a bearer/token auth
+func BearerAuth(token string) string {
+	return fmt.Sprintf("Bearer %s", token)
+}
+
 // MaybeMarshalJSON returns a JSON string of the given object, or "<cannot serialize>" if serialization failed.
 // This is useful for logging purposes where a failure doesn't matter that much.
 func MaybeMarshalJSON(v any) string {
@@ -283,3 +288,26 @@ func QuoteCommand(command []string) string {
 	}
 	return strings.Join(quoted, " ")
 }
+
+// ReadJSON reads the given io.ReadCloser into a struct
+func ReadJSON[T any](body io.ReadCloser) (*T, error) {
+	var obj T
+	if err := json.NewDecoder(body).Decode(&obj); err != nil {
+		return nil, err
+	}
+	return &obj, nil
+}
+
+// ReadJSONWithLimit reads the given io.ReadCloser into a struct, but only until limit is reached
+func ReadJSONWithLimit[T any](r io.ReadCloser, limit int) (*T, error) {
+	r, err := Peek(r, limit)
+	if err != nil {
+		return nil, err
+	}
+	defer r.Close()
+	var obj T
+	if err := json.NewDecoder(r).Decode(&obj); err != nil {
+		return nil, err
+	}
+	return &obj, nil
+}