From 367d024a2d8e1bc7daf61c303d337da613016232 Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Wed, 28 Dec 2022 19:55:11 -0500 Subject: [PATCH] Simplify API endpoints; add endpoint tests --- server/server.go | 30 +++++++------- server/server_account.go | 77 ++++++++--------------------------- server/server_account_test.go | 44 ++++++++++++++++++++ server/server_test.go | 6 +++ user/manager.go | 3 +- util/util.go | 28 +++++++++++++ 6 files changed, 113 insertions(+), 75 deletions(-) create mode 100644 server/server_account_test.go diff --git a/server/server.go b/server/server.go index 6b62b0ef..92ad8228 100644 --- a/server/server.go +++ b/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 { - return s.ensureAccountsEnabled(func(w http.ResponseWriter, r *http.Request, v *visitor) error { +func (s *Server) ensureUser(next handleFunc) handleFunc { + return s.ensureUserManager(func(w http.ResponseWriter, r *http.Request, v *visitor) error { if v.user == nil { return errHTTPNotFound } diff --git a/server/server_account.go b/server/server_account.go index f832cd99..b7e8ebcb 100644 --- a/server/server_account.go +++ b/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 { - return errHTTPUnauthorized - } - body, err := util.Peek(r.Body, 4096) // FIXME + newPassword, err := util.ReadJSONWithLimit[apiAccountCreateRequest](r.Body, jsonBodyBytesLimit) 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 { - 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 + newPrefs, err := util.ReadJSONWithLimit[user.Prefs](r.Body, jsonBodyBytesLimit) 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 { - return errors.New("no user") - } - newSubscription, err := readJSONBody[user.Subscription](r.Body) + newSubscription, err := util.ReadJSONWithLimit[user.Subscription](r.Body, jsonBodyBytesLimit) 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 -} diff --git a/server/server_account_test.go b/server/server_account_test.go new file mode 100644 index 00000000..629767f2 --- /dev/null +++ b/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) +} diff --git a/server/server_test.go b/server/server_test.go index e1517621..e97ec8ac 100644 --- a/server/server_test.go +++ b/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 { diff --git a/user/manager.go b/user/manager.go index 6731e4e7..17a8e0d6 100644 --- a/user/manager.go +++ b/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 -// before it is stored in a persistence layer. +// AddUser adds a user with the given username, password and role func (a *Manager) AddUser(username, password string, role Role) error { if !AllowedUsername(username) || !AllowedRole(role) { return ErrInvalidArgument diff --git a/util/util.go b/util/util.go index 86566edf..c9535e46 100644 --- a/util/util.go +++ b/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 "" 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 +}