From 44b7c2f198f5171bf3f40a36b5efa2ae40402ce3 Mon Sep 17 00:00:00 2001 From: Tom Hubrecht Date: Tue, 20 May 2025 10:49:26 +0200 Subject: [PATCH] user: Allow changing the hashed password directly This adds the detection of `NTFY_PASSWORD_HASH` when creating a user or changing its passsword so that scripts don't have to manipulate the bare password. --- cmd/user.go | 37 +++++++++++------ server/server_account.go | 4 +- server/server_account_test.go | 24 +++++------ server/server_admin.go | 2 +- server/server_admin_test.go | 16 ++++---- server/server_payments_test.go | 18 ++++----- server/server_test.go | 32 +++++++-------- server/server_twilio_test.go | 8 ++-- server/server_webpush_test.go | 4 +- user/manager.go | 32 +++++++++++---- user/manager_test.go | 74 +++++++++++++++++++--------------- 11 files changed, 144 insertions(+), 107 deletions(-) diff --git a/cmd/user.go b/cmd/user.go index af3afe54..e6867b11 100644 --- a/cmd/user.go +++ b/cmd/user.go @@ -42,7 +42,7 @@ var cmdUser = &cli.Command{ Name: "add", Aliases: []string{"a"}, Usage: "Adds a new user", - UsageText: "ntfy user add [--role=admin|user] USERNAME\nNTFY_PASSWORD=... ntfy user add [--role=admin|user] USERNAME", + UsageText: "ntfy user add [--role=admin|user] USERNAME\nNTFY_PASSWORD=... ntfy user add [--role=admin|user] USERNAME\nNTFY_PASSWORD_HASH=... ntfy user add [--role=admin|user] USERNAME", Action: execUserAdd, Flags: []cli.Flag{ &cli.StringFlag{Name: "role", Aliases: []string{"r"}, Value: string(user.RoleUser), Usage: "user role"}, @@ -55,12 +55,13 @@ granted otherwise by the auth-default-access setting). An admin user has read an topics. Examples: - ntfy user add phil # Add regular user phil - ntfy user add --role=admin phil # Add admin user phil - NTFY_PASSWORD=... ntfy user add phil # Add user, using env variable to set password (for scripts) + ntfy user add phil # Add regular user phil + ntfy user add --role=admin phil # Add admin user phil + NTFY_PASSWORD=... ntfy user add phil # Add user, using env variable to set password (for scripts) + NTFY_PASSWORD_HASH=... ntfy user add phil # Add user, using env variable to set password hash (for scripts) -You may set the NTFY_PASSWORD environment variable to pass the password. This is useful if -you are creating users via scripts. +You may set the NTFY_PASSWORD environment variable to pass the password, or NTFY_PASSWORD_HASH to pass +directly the bcrypt hash. This is useful if you are creating users via scripts. `, }, { @@ -79,7 +80,7 @@ Example: Name: "change-pass", Aliases: []string{"chp"}, Usage: "Changes a user's password", - UsageText: "ntfy user change-pass USERNAME\nNTFY_PASSWORD=... ntfy user change-pass USERNAME", + UsageText: "ntfy user change-pass USERNAME\nNTFY_PASSWORD=... ntfy user change-pass USERNAME\nNTFY_PASSWORD_HASH=... ntfy user change-pass USERNAME", Action: execUserChangePass, Description: `Change the password for the given user. @@ -89,9 +90,10 @@ it twice. Example: ntfy user change-pass phil NTFY_PASSWORD=.. ntfy user change-pass phil + NTFY_PASSWORD_HASH=.. ntfy user change-pass phil -You may set the NTFY_PASSWORD environment variable to pass the new password. This is -useful if you are updating users via scripts. +You may set the NTFY_PASSWORD environment variable to pass the new password or NTFY_PASSWORD_HASH to pass +directly the bcrypt hash. This is useful if you are updating users via scripts. `, }, @@ -174,7 +176,12 @@ variable to pass the new password. This is useful if you are creating/updating u func execUserAdd(c *cli.Context) error { username := c.Args().Get(0) role := user.Role(c.String("role")) - password := os.Getenv("NTFY_PASSWORD") + password, hashed := os.LookupEnv("NTFY_PASSWORD_HASH") + + if !hashed { + password = os.Getenv("NTFY_PASSWORD") + } + if username == "" { return errors.New("username expected, type 'ntfy user add --help' for help") } else if username == userEveryone || username == user.Everyone { @@ -200,7 +207,7 @@ func execUserAdd(c *cli.Context) error { } password = p } - if err := manager.AddUser(username, password, role); err != nil { + if err := manager.AddUser(username, password, role, hashed); err != nil { return err } fmt.Fprintf(c.App.ErrWriter, "user %s added with role %s\n", username, role) @@ -230,7 +237,11 @@ func execUserDel(c *cli.Context) error { func execUserChangePass(c *cli.Context) error { username := c.Args().Get(0) - password := os.Getenv("NTFY_PASSWORD") + password, hashed := os.LookupEnv("NTFY_PASSWORD_HASH") + + if !hashed { + password = os.Getenv("NTFY_PASSWORD") + } if username == "" { return errors.New("username expected, type 'ntfy user change-pass --help' for help") } else if username == userEveryone || username == user.Everyone { @@ -249,7 +260,7 @@ func execUserChangePass(c *cli.Context) error { return err } } - if err := manager.ChangePassword(username, password); err != nil { + if err := manager.ChangePassword(username, password, hashed); err != nil { return err } fmt.Fprintf(c.App.ErrWriter, "changed password for user %s\n", username) diff --git a/server/server_account.go b/server/server_account.go index 3f2368da..acdf25ec 100644 --- a/server/server_account.go +++ b/server/server_account.go @@ -37,7 +37,7 @@ func (s *Server) handleAccountCreate(w http.ResponseWriter, r *http.Request, v * return errHTTPConflictUserExists } logvr(v, r).Tag(tagAccount).Field("user_name", newAccount.Username).Info("Creating user %s", newAccount.Username) - if err := s.userManager.AddUser(newAccount.Username, newAccount.Password, user.RoleUser); err != nil { + if err := s.userManager.AddUser(newAccount.Username, newAccount.Password, user.RoleUser, false); err != nil { if errors.Is(err, user.ErrInvalidArgument) { return errHTTPBadRequestInvalidUsername } @@ -207,7 +207,7 @@ func (s *Server) handleAccountPasswordChange(w http.ResponseWriter, r *http.Requ return errHTTPBadRequestIncorrectPasswordConfirmation } logvr(v, r).Tag(tagAccount).Debug("Changing password for user %s", u.Name) - if err := s.userManager.ChangePassword(u.Name, req.NewPassword); err != nil { + if err := s.userManager.ChangePassword(u.Name, req.NewPassword, false); err != nil { return err } return s.writeJSON(w, newSuccessResponse()) diff --git a/server/server_account_test.go b/server/server_account_test.go index 72ba55c9..91db1bc5 100644 --- a/server/server_account_test.go +++ b/server/server_account_test.go @@ -87,9 +87,9 @@ func TestAccount_Signup_AsUser(t *testing.T) { defer s.closeDatabases() log.Info("1") - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin, false)) log.Info("2") - require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser, false)) log.Info("3") rr := request(t, s, "POST", "/v1/account", `{"username":"emma", "password":"emma"}`, map[string]string{ "Authorization": util.BasicAuth("phil", "phil"), @@ -174,7 +174,7 @@ func TestAccount_ChangeSettings(t *testing.T) { s := newTestServer(t, newTestConfigWithAuthFile(t)) defer s.closeDatabases() - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) u, _ := s.userManager.User("phil") token, _ := s.userManager.CreateToken(u.ID, "", time.Unix(0, 0), netip.IPv4Unspecified()) @@ -203,7 +203,7 @@ func TestAccount_Subscription_AddUpdateDelete(t *testing.T) { s := newTestServer(t, newTestConfigWithAuthFile(t)) defer s.closeDatabases() - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) rr := request(t, s, "POST", "/v1/account/subscription", `{"base_url": "http://abc.com", "topic": "def"}`, map[string]string{ "Authorization": util.BasicAuth("phil", "phil"), @@ -254,7 +254,7 @@ func TestAccount_ChangePassword(t *testing.T) { s := newTestServer(t, newTestConfigWithAuthFile(t)) defer s.closeDatabases() - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) rr := request(t, s, "POST", "/v1/account/password", `{"password": "WRONG", "new_password": ""}`, map[string]string{ "Authorization": util.BasicAuth("phil", "phil"), @@ -296,7 +296,7 @@ func TestAccount_ExtendToken(t *testing.T) { s := newTestServer(t, newTestConfigWithAuthFile(t)) defer s.closeDatabases() - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) rr := request(t, s, "POST", "/v1/account/token", "", map[string]string{ "Authorization": util.BasicAuth("phil", "phil"), @@ -332,7 +332,7 @@ func TestAccount_ExtendToken_NoTokenProvided(t *testing.T) { s := newTestServer(t, newTestConfigWithAuthFile(t)) defer s.closeDatabases() - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) rr := request(t, s, "PATCH", "/v1/account/token", "", map[string]string{ "Authorization": util.BasicAuth("phil", "phil"), // Not Bearer! @@ -345,7 +345,7 @@ func TestAccount_DeleteToken(t *testing.T) { s := newTestServer(t, newTestConfigWithAuthFile(t)) defer s.closeDatabases() - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) rr := request(t, s, "POST", "/v1/account/token", "", map[string]string{ "Authorization": util.BasicAuth("phil", "phil"), @@ -455,14 +455,14 @@ func TestAccount_Reservation_AddAdminSuccess(t *testing.T) { Code: "pro", ReservationLimit: 2, })) - require.Nil(t, s.userManager.AddUser("noadmin1", "pass", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("noadmin1", "pass", user.RoleUser, false)) require.Nil(t, s.userManager.ChangeTier("noadmin1", "pro")) require.Nil(t, s.userManager.AddReservation("noadmin1", "mytopic", user.PermissionDenyAll)) - require.Nil(t, s.userManager.AddUser("noadmin2", "pass", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("noadmin2", "pass", user.RoleUser, false)) require.Nil(t, s.userManager.ChangeTier("noadmin2", "pro")) - require.Nil(t, s.userManager.AddUser("phil", "adminpass", user.RoleAdmin)) + require.Nil(t, s.userManager.AddUser("phil", "adminpass", user.RoleAdmin, false)) // Admin can reserve topic rr := request(t, s, "POST", "/v1/account/reservation", `{"topic":"sometopic","everyone":"deny-all"}`, map[string]string{ @@ -624,7 +624,7 @@ func TestAccount_Reservation_Delete_Messages_And_Attachments(t *testing.T) { s := newTestServer(t, conf) // Create user with tier - require.Nil(t, s.userManager.AddUser("phil", "mypass", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "mypass", user.RoleUser, false)) require.Nil(t, s.userManager.AddTier(&user.Tier{ Code: "pro", MessageLimit: 20, diff --git a/server/server_admin.go b/server/server_admin.go index ac295718..49a4d4d9 100644 --- a/server/server_admin.go +++ b/server/server_admin.go @@ -60,7 +60,7 @@ func (s *Server) handleUsersAdd(w http.ResponseWriter, r *http.Request, v *visit return err } } - if err := s.userManager.AddUser(req.Username, req.Password, user.RoleUser); err != nil { + if err := s.userManager.AddUser(req.Username, req.Password, user.RoleUser, false); err != nil { return err } if tier != nil { diff --git a/server/server_admin_test.go b/server/server_admin_test.go index c2f8f95a..a8bc87e1 100644 --- a/server/server_admin_test.go +++ b/server/server_admin_test.go @@ -14,7 +14,7 @@ func TestUser_AddRemove(t *testing.T) { defer s.closeDatabases() // Create admin, tier - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin, false)) require.Nil(t, s.userManager.AddTier(&user.Tier{ Code: "tier1", })) @@ -56,8 +56,8 @@ func TestUser_AddRemove_Failures(t *testing.T) { defer s.closeDatabases() // Create admin - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin)) - require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin, false)) + require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser, false)) // Cannot create user with invalid username rr := request(t, s, "PUT", "/v1/users", `{"username": "not valid", "password":"ben"}`, map[string]string{ @@ -97,8 +97,8 @@ func TestAccess_AllowReset(t *testing.T) { defer s.closeDatabases() // User and admin - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin)) - require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin, false)) + require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser, false)) // Subscribing not allowed rr := request(t, s, "GET", "/gold/json?poll=1", "", map[string]string{ @@ -138,7 +138,7 @@ func TestAccess_AllowReset_NonAdminAttempt(t *testing.T) { defer s.closeDatabases() // User - require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser, false)) // Grant access fails, because non-admin rr := request(t, s, "POST", "/v1/users/access", `{"username": "ben", "topic":"gold", "permission":"ro"}`, map[string]string{ @@ -154,8 +154,8 @@ func TestAccess_AllowReset_KillConnection(t *testing.T) { defer s.closeDatabases() // User and admin, grant access to "gol*" topics - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin)) - require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin, false)) + require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser, false)) require.Nil(t, s.userManager.AllowAccess("ben", "gol*", user.PermissionRead)) // Wildcard! start, timeTaken := time.Now(), atomic.Int64{} diff --git a/server/server_payments_test.go b/server/server_payments_test.go index 8da47a65..56d4cc6a 100644 --- a/server/server_payments_test.go +++ b/server/server_payments_test.go @@ -148,7 +148,7 @@ func TestPayments_SubscriptionCreate_NotAStripeCustomer_Success(t *testing.T) { Code: "pro", StripeMonthlyPriceID: "price_123", })) - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) // Create subscription response := request(t, s, "POST", "/v1/account/billing/subscription", `{"tier": "pro", "interval": "month"}`, map[string]string{ @@ -184,7 +184,7 @@ func TestPayments_SubscriptionCreate_StripeCustomer_Success(t *testing.T) { Code: "pro", StripeMonthlyPriceID: "price_123", })) - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) u, err := s.userManager.User("phil") require.Nil(t, err) @@ -226,7 +226,7 @@ func TestPayments_AccountDelete_Cancels_Subscription(t *testing.T) { Code: "pro", StripeMonthlyPriceID: "price_123", })) - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) u, err := s.userManager.User("phil") require.Nil(t, err) @@ -280,7 +280,7 @@ func TestPayments_Checkout_Success_And_Increase_Rate_Limits_Reset_Visitor(t *tes MessageLimit: 220, // 220 * 5% = 11 requests before rate limiting kicks in MessageExpiryDuration: time.Hour, })) - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) // No tier + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) // No tier u, err := s.userManager.User("phil") require.Nil(t, err) @@ -461,7 +461,7 @@ func TestPayments_Webhook_Subscription_Updated_Downgrade_From_PastDue_To_Active( AttachmentTotalSizeLimit: 1000000, AttachmentBandwidthLimit: 1000000, })) - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) require.Nil(t, s.userManager.ChangeTier("phil", "pro")) require.Nil(t, s.userManager.AddReservation("phil", "atopic", user.PermissionDenyAll)) require.Nil(t, s.userManager.AddReservation("phil", "ztopic", user.PermissionDenyAll)) @@ -570,7 +570,7 @@ func TestPayments_Webhook_Subscription_Deleted(t *testing.T) { StripeMonthlyPriceID: "price_1234", ReservationLimit: 1, })) - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) require.Nil(t, s.userManager.ChangeTier("phil", "pro")) require.Nil(t, s.userManager.AddReservation("phil", "atopic", user.PermissionDenyAll)) @@ -658,7 +658,7 @@ func TestPayments_Subscription_Update_Different_Tier(t *testing.T) { StripeMonthlyPriceID: "price_456", StripeYearlyPriceID: "price_457", })) - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) require.Nil(t, s.userManager.ChangeTier("phil", "pro")) require.Nil(t, s.userManager.ChangeBilling("phil", &user.Billing{ StripeCustomerID: "acct_123", @@ -690,7 +690,7 @@ func TestPayments_Subscription_Delete_At_Period_End(t *testing.T) { Return(&stripe.Subscription{}, nil) // Create user - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) require.Nil(t, s.userManager.ChangeBilling("phil", &user.Billing{ StripeCustomerID: "acct_123", StripeSubscriptionID: "sub_123", @@ -724,7 +724,7 @@ func TestPayments_CreatePortalSession(t *testing.T) { }, nil) // Create user - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) require.Nil(t, s.userManager.ChangeBilling("phil", &user.Billing{ StripeCustomerID: "acct_123", StripeSubscriptionID: "sub_123", diff --git a/server/server_test.go b/server/server_test.go index 75379f8f..88c88d1c 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -411,7 +411,7 @@ func TestServer_PublishAt_FromUser(t *testing.T) { t.Parallel() s := newTestServer(t, newTestConfigWithAuthFile(t)) - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin, false)) response := request(t, s, "PUT", "/mytopic", "a message", map[string]string{ "Authorization": util.BasicAuth("phil", "phil"), "In": "1h", @@ -781,7 +781,7 @@ func TestServer_Auth_Success_Admin(t *testing.T) { c := newTestConfigWithAuthFile(t) s := newTestServer(t, c) - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin, false)) response := request(t, s, "GET", "/mytopic/auth", "", map[string]string{ "Authorization": util.BasicAuth("phil", "phil"), @@ -795,7 +795,7 @@ func TestServer_Auth_Success_User(t *testing.T) { c.AuthDefault = user.PermissionDenyAll s := newTestServer(t, c) - require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser, false)) require.Nil(t, s.userManager.AllowAccess("ben", "mytopic", user.PermissionReadWrite)) response := request(t, s, "GET", "/mytopic/auth", "", map[string]string{ @@ -809,7 +809,7 @@ func TestServer_Auth_Success_User_MultipleTopics(t *testing.T) { c.AuthDefault = user.PermissionDenyAll s := newTestServer(t, c) - require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser, false)) require.Nil(t, s.userManager.AllowAccess("ben", "mytopic", user.PermissionReadWrite)) require.Nil(t, s.userManager.AllowAccess("ben", "anothertopic", user.PermissionReadWrite)) @@ -830,7 +830,7 @@ func TestServer_Auth_Fail_InvalidPass(t *testing.T) { c.AuthDefault = user.PermissionDenyAll s := newTestServer(t, c) - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin, false)) response := request(t, s, "GET", "/mytopic/auth", "", map[string]string{ "Authorization": util.BasicAuth("phil", "INVALID"), @@ -843,7 +843,7 @@ func TestServer_Auth_Fail_Unauthorized(t *testing.T) { c.AuthDefault = user.PermissionDenyAll s := newTestServer(t, c) - require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser, false)) require.Nil(t, s.userManager.AllowAccess("ben", "sometopic", user.PermissionReadWrite)) // Not mytopic! response := request(t, s, "GET", "/mytopic/auth", "", map[string]string{ @@ -857,7 +857,7 @@ func TestServer_Auth_Fail_CannotPublish(t *testing.T) { c.AuthDefault = user.PermissionReadWrite // Open by default s := newTestServer(t, c) - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin, false)) require.Nil(t, s.userManager.AllowAccess(user.Everyone, "private", user.PermissionDenyAll)) require.Nil(t, s.userManager.AllowAccess(user.Everyone, "announcements", user.PermissionRead)) @@ -906,7 +906,7 @@ func TestServer_Auth_ViaQuery(t *testing.T) { c.AuthDefault = user.PermissionDenyAll s := newTestServer(t, c) - require.Nil(t, s.userManager.AddUser("ben", "some pass", user.RoleAdmin)) + require.Nil(t, s.userManager.AddUser("ben", "some pass", user.RoleAdmin, false)) u := fmt.Sprintf("/mytopic/json?poll=1&auth=%s", base64.RawURLEncoding.EncodeToString([]byte(util.BasicAuth("ben", "some pass")))) response := request(t, s, "GET", u, "", nil) @@ -954,8 +954,8 @@ func TestServer_StatsResetter(t *testing.T) { MessageLimit: 5, MessageExpiryDuration: -5 * time.Second, // Second, what a hack! })) - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) - require.Nil(t, s.userManager.AddUser("tieruser", "tieruser", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) + require.Nil(t, s.userManager.AddUser("tieruser", "tieruser", user.RoleUser, false)) require.Nil(t, s.userManager.ChangeTier("tieruser", "test")) // Send an anonymous message @@ -1099,7 +1099,7 @@ func TestServer_DailyMessageQuotaFromDatabase(t *testing.T) { require.Nil(t, s.userManager.AddTier(&user.Tier{ Code: "test", })) - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) require.Nil(t, s.userManager.ChangeTier("phil", "test")) u, err := s.userManager.User("phil") @@ -1696,7 +1696,7 @@ func TestServer_PublishWithTierBasedMessageLimitAndExpiry(t *testing.T) { MessageLimit: 5, MessageExpiryDuration: -5 * time.Second, // Second, what a hack! })) - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) require.Nil(t, s.userManager.ChangeTier("phil", "test")) // Publish to reach message limit @@ -1932,7 +1932,7 @@ func TestServer_PublishAttachmentWithTierBasedExpiry(t *testing.T) { AttachmentExpiryDuration: sevenDays, // 7 days AttachmentBandwidthLimit: 100000, })) - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) require.Nil(t, s.userManager.ChangeTier("phil", "test")) // Publish and make sure we can retrieve it @@ -1977,7 +1977,7 @@ func TestServer_PublishAttachmentWithTierBasedBandwidthLimit(t *testing.T) { AttachmentExpiryDuration: time.Hour, AttachmentBandwidthLimit: 14000, // < 3x5000 bytes -> enough for one upload, one download })) - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) require.Nil(t, s.userManager.ChangeTier("phil", "test")) // Publish and make sure we can retrieve it @@ -2015,7 +2015,7 @@ func TestServer_PublishAttachmentWithTierBasedLimits(t *testing.T) { AttachmentExpiryDuration: 30 * time.Second, AttachmentBandwidthLimit: 1000000, })) - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) require.Nil(t, s.userManager.ChangeTier("phil", "test")) // Publish small file as anonymous @@ -2237,7 +2237,7 @@ func TestServer_AnonymousUser_And_NonTierUser_Are_Same_Visitor(t *testing.T) { defer s.closeDatabases() // Create user without tier - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) // Publish a message (anonymous user) rr := request(t, s, "POST", "/mytopic", "hi", nil) diff --git a/server/server_twilio_test.go b/server/server_twilio_test.go index 89a36051..2501916a 100644 --- a/server/server_twilio_test.go +++ b/server/server_twilio_test.go @@ -63,7 +63,7 @@ func TestServer_Twilio_Call_Add_Verify_Call_Delete_Success(t *testing.T) { MessageLimit: 10, CallLimit: 1, })) - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) require.Nil(t, s.userManager.ChangeTier("phil", "pro")) u, err := s.userManager.User("phil") require.Nil(t, err) @@ -140,7 +140,7 @@ func TestServer_Twilio_Call_Success(t *testing.T) { MessageLimit: 10, CallLimit: 1, })) - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) require.Nil(t, s.userManager.ChangeTier("phil", "pro")) u, err := s.userManager.User("phil") require.Nil(t, err) @@ -185,7 +185,7 @@ func TestServer_Twilio_Call_Success_With_Yes(t *testing.T) { MessageLimit: 10, CallLimit: 1, })) - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) require.Nil(t, s.userManager.ChangeTier("phil", "pro")) u, err := s.userManager.User("phil") require.Nil(t, err) @@ -216,7 +216,7 @@ func TestServer_Twilio_Call_UnverifiedNumber(t *testing.T) { MessageLimit: 10, CallLimit: 1, })) - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) require.Nil(t, s.userManager.ChangeTier("phil", "pro")) // Do the thing diff --git a/server/server_webpush_test.go b/server/server_webpush_test.go index c32c7bf8..ab7a20c4 100644 --- a/server/server_webpush_test.go +++ b/server/server_webpush_test.go @@ -96,7 +96,7 @@ func TestServer_WebPush_TopicSubscribeProtected_Allowed(t *testing.T) { config.AuthDefault = user.PermissionDenyAll s := newTestServer(t, config) - require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser, false)) require.Nil(t, s.userManager.AllowAccess("ben", "test-topic", user.PermissionReadWrite)) response := request(t, s, "POST", "/v1/webpush", payloadForTopics(t, []string{"test-topic"}, testWebPushEndpoint), map[string]string{ @@ -126,7 +126,7 @@ func TestServer_WebPush_DeleteAccountUnsubscribe(t *testing.T) { config := configureAuth(t, newTestConfigWithWebPush(t)) s := newTestServer(t, config) - require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser)) + require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser, false)) require.Nil(t, s.userManager.AllowAccess("ben", "test-topic", user.PermissionReadWrite)) response := request(t, s, "POST", "/v1/webpush", payloadForTopics(t, []string{"test-topic"}, testWebPushEndpoint), map[string]string{ diff --git a/user/manager.go b/user/manager.go index 9f54625f..d691d42f 100644 --- a/user/manager.go +++ b/user/manager.go @@ -864,14 +864,23 @@ func (a *Manager) resolvePerms(base, perm Permission) error { } // AddUser adds a user with the given username, password and role -func (a *Manager) AddUser(username, password string, role Role) error { +func (a *Manager) AddUser(username, password string, role Role, hashed bool) error { if !AllowedUsername(username) || !AllowedRole(role) { return ErrInvalidArgument } - hash, err := bcrypt.GenerateFromPassword([]byte(password), a.bcryptCost) - if err != nil { - return err + + var hash []byte + var err error = nil + + if hashed { + hash = []byte(password) + } else { + hash, err = bcrypt.GenerateFromPassword([]byte(password), a.bcryptCost) + if err != nil { + return err + } } + userID := util.RandomStringPrefix(userIDPrefix, userIDLength) syncTopic, now := util.RandomStringPrefix(syncTopicPrefix, syncTopicLength), time.Now().Unix() if _, err = a.db.Exec(insertUserQuery, userID, username, hash, role, syncTopic, now); err != nil { @@ -1192,10 +1201,17 @@ func (a *Manager) ReservationOwner(topic string) (string, error) { } // ChangePassword changes a user's password -func (a *Manager) ChangePassword(username, password string) error { - hash, err := bcrypt.GenerateFromPassword([]byte(password), a.bcryptCost) - if err != nil { - return err +func (a *Manager) ChangePassword(username, password string, hashed bool) error { + var hash []byte + var err error + + if hashed { + hash = []byte(password) + } else { + hash, err = bcrypt.GenerateFromPassword([]byte(password), a.bcryptCost) + if err != nil { + return err + } } if _, err := a.db.Exec(updateUserPassQuery, hash, username); err != nil { return err diff --git a/user/manager_test.go b/user/manager_test.go index e9a95b0f..c81b8cab 100644 --- a/user/manager_test.go +++ b/user/manager_test.go @@ -18,9 +18,9 @@ const minBcryptTimingMillis = int64(50) // Ideally should be >100ms, but this sh func TestManager_FullScenario_Default_DenyAll(t *testing.T) { a := newTestManagerFromFile(t, filepath.Join(t.TempDir(), "user.db"), "", PermissionDenyAll, DefaultUserPasswordBcryptCost, DefaultUserStatsQueueWriterInterval) - require.Nil(t, a.AddUser("phil", "phil", RoleAdmin)) - require.Nil(t, a.AddUser("ben", "ben", RoleUser)) - require.Nil(t, a.AddUser("john", "john", RoleUser)) + require.Nil(t, a.AddUser("phil", "phil", RoleAdmin, false)) + require.Nil(t, a.AddUser("ben", "ben", RoleUser, false)) + require.Nil(t, a.AddUser("john", "john", RoleUser, false)) require.Nil(t, a.AllowAccess("ben", "mytopic", PermissionReadWrite)) require.Nil(t, a.AllowAccess("ben", "readme", PermissionRead)) require.Nil(t, a.AllowAccess("ben", "writeme", PermissionWrite)) @@ -134,7 +134,7 @@ func TestManager_Access_Order_LengthWriteRead(t *testing.T) { // and longer ACL rules are prioritized as well. a := newTestManagerFromFile(t, filepath.Join(t.TempDir(), "user.db"), "", PermissionDenyAll, DefaultUserPasswordBcryptCost, DefaultUserStatsQueueWriterInterval) - require.Nil(t, a.AddUser("ben", "ben", RoleUser)) + require.Nil(t, a.AddUser("ben", "ben", RoleUser, false)) require.Nil(t, a.AllowAccess("ben", "test*", PermissionReadWrite)) require.Nil(t, a.AllowAccess("ben", "*", PermissionRead)) @@ -147,20 +147,20 @@ func TestManager_Access_Order_LengthWriteRead(t *testing.T) { func TestManager_AddUser_Invalid(t *testing.T) { a := newTestManager(t, PermissionDenyAll) - require.Equal(t, ErrInvalidArgument, a.AddUser(" invalid ", "pass", RoleAdmin)) - require.Equal(t, ErrInvalidArgument, a.AddUser("validuser", "pass", "invalid-role")) + require.Equal(t, ErrInvalidArgument, a.AddUser(" invalid ", "pass", RoleAdmin, false)) + require.Equal(t, ErrInvalidArgument, a.AddUser("validuser", "pass", "invalid-role", false)) } func TestManager_AddUser_Timing(t *testing.T) { a := newTestManagerFromFile(t, filepath.Join(t.TempDir(), "user.db"), "", PermissionDenyAll, DefaultUserPasswordBcryptCost, DefaultUserStatsQueueWriterInterval) start := time.Now().UnixMilli() - require.Nil(t, a.AddUser("user", "pass", RoleAdmin)) + require.Nil(t, a.AddUser("user", "pass", RoleAdmin, false)) require.GreaterOrEqual(t, time.Now().UnixMilli()-start, minBcryptTimingMillis) } func TestManager_AddUser_And_Query(t *testing.T) { a := newTestManagerFromFile(t, filepath.Join(t.TempDir(), "user.db"), "", PermissionDenyAll, DefaultUserPasswordBcryptCost, DefaultUserStatsQueueWriterInterval) - require.Nil(t, a.AddUser("user", "pass", RoleAdmin)) + require.Nil(t, a.AddUser("user", "pass", RoleAdmin, false)) require.Nil(t, a.ChangeBilling("user", &Billing{ StripeCustomerID: "acct_123", StripeSubscriptionID: "sub_123", @@ -187,7 +187,7 @@ func TestManager_MarkUserRemoved_RemoveDeletedUsers(t *testing.T) { a := newTestManager(t, PermissionDenyAll) // Create user, add reservations and token - require.Nil(t, a.AddUser("user", "pass", RoleAdmin)) + require.Nil(t, a.AddUser("user", "pass", RoleAdmin, false)) require.Nil(t, a.AddReservation("user", "mytopic", PermissionRead)) u, err := a.User("user") @@ -237,7 +237,7 @@ func TestManager_CreateToken_Only_Lower(t *testing.T) { a := newTestManager(t, PermissionDenyAll) // Create user, add reservations and token - require.Nil(t, a.AddUser("user", "pass", RoleAdmin)) + require.Nil(t, a.AddUser("user", "pass", RoleAdmin, false)) u, err := a.User("user") require.Nil(t, err) @@ -248,8 +248,8 @@ func TestManager_CreateToken_Only_Lower(t *testing.T) { func TestManager_UserManagement(t *testing.T) { a := newTestManager(t, PermissionDenyAll) - require.Nil(t, a.AddUser("phil", "phil", RoleAdmin)) - require.Nil(t, a.AddUser("ben", "ben", RoleUser)) + require.Nil(t, a.AddUser("phil", "phil", RoleAdmin, false)) + require.Nil(t, a.AddUser("ben", "ben", RoleUser, false)) require.Nil(t, a.AllowAccess("ben", "mytopic", PermissionReadWrite)) require.Nil(t, a.AllowAccess("ben", "readme", PermissionRead)) require.Nil(t, a.AllowAccess("ben", "writeme", PermissionWrite)) @@ -339,21 +339,31 @@ func TestManager_UserManagement(t *testing.T) { func TestManager_ChangePassword(t *testing.T) { a := newTestManager(t, PermissionDenyAll) - require.Nil(t, a.AddUser("phil", "phil", RoleAdmin)) + require.Nil(t, a.AddUser("phil", "phil", RoleAdmin, false)) + require.Nil(t, a.AddUser("jane", "$2b$10$OyqU72muEy7VMd1SAU2Iru5IbeSMgrtCGHu/fWLmxL1MwlijQXWbG", RoleUser, true)) _, err := a.Authenticate("phil", "phil") require.Nil(t, err) - require.Nil(t, a.ChangePassword("phil", "newpass")) + _, err = a.Authenticate("jane", "jane") + require.Nil(t, err) + + require.Nil(t, a.ChangePassword("phil", "newpass", false)) _, err = a.Authenticate("phil", "phil") require.Equal(t, ErrUnauthenticated, err) _, err = a.Authenticate("phil", "newpass") require.Nil(t, err) + + require.Nil(t, a.ChangePassword("jane", "$2b$10$CNaCW.q1R431urlbQ5Drh.zl48TiiOeJSmZgfcswkZiPbJGQ1ApSS", true)) + _, err = a.Authenticate("jane", "jane") + require.Equal(t, ErrUnauthenticated, err) + _, err = a.Authenticate("jane", "newpass") + require.Nil(t, err) } func TestManager_ChangeRole(t *testing.T) { a := newTestManager(t, PermissionDenyAll) - require.Nil(t, a.AddUser("ben", "ben", RoleUser)) + require.Nil(t, a.AddUser("ben", "ben", RoleUser, false)) require.Nil(t, a.AllowAccess("ben", "mytopic", PermissionReadWrite)) require.Nil(t, a.AllowAccess("ben", "readme", PermissionRead)) @@ -378,8 +388,8 @@ func TestManager_ChangeRole(t *testing.T) { func TestManager_Reservations(t *testing.T) { a := newTestManager(t, PermissionDenyAll) - require.Nil(t, a.AddUser("phil", "phil", RoleUser)) - require.Nil(t, a.AddUser("ben", "ben", RoleUser)) + require.Nil(t, a.AddUser("phil", "phil", RoleUser, false)) + require.Nil(t, a.AddUser("ben", "ben", RoleUser, false)) require.Nil(t, a.AddReservation("ben", "ztopic_", PermissionDenyAll)) require.Nil(t, a.AddReservation("ben", "readme", PermissionRead)) require.Nil(t, a.AllowAccess("ben", "something-else", PermissionRead)) @@ -460,7 +470,7 @@ func TestManager_ChangeRoleFromTierUserToAdmin(t *testing.T) { AttachmentTotalSizeLimit: 524288000, AttachmentExpiryDuration: 24 * time.Hour, })) - require.Nil(t, a.AddUser("ben", "ben", RoleUser)) + require.Nil(t, a.AddUser("ben", "ben", RoleUser, false)) require.Nil(t, a.ChangeTier("ben", "pro")) require.Nil(t, a.AddReservation("ben", "mytopic", PermissionDenyAll)) @@ -507,7 +517,7 @@ func TestManager_ChangeRoleFromTierUserToAdmin(t *testing.T) { func TestManager_Token_Valid(t *testing.T) { a := newTestManager(t, PermissionDenyAll) - require.Nil(t, a.AddUser("ben", "ben", RoleUser)) + require.Nil(t, a.AddUser("ben", "ben", RoleUser, false)) u, err := a.User("ben") require.Nil(t, err) @@ -551,7 +561,7 @@ func TestManager_Token_Valid(t *testing.T) { func TestManager_Token_Invalid(t *testing.T) { a := newTestManager(t, PermissionDenyAll) - require.Nil(t, a.AddUser("ben", "ben", RoleUser)) + require.Nil(t, a.AddUser("ben", "ben", RoleUser, false)) u, err := a.AuthenticateToken(strings.Repeat("x", 32)) // 32 == token length require.Nil(t, u) @@ -570,7 +580,7 @@ func TestManager_Token_NotFound(t *testing.T) { func TestManager_Token_Expire(t *testing.T) { a := newTestManager(t, PermissionDenyAll) - require.Nil(t, a.AddUser("ben", "ben", RoleUser)) + require.Nil(t, a.AddUser("ben", "ben", RoleUser, false)) u, err := a.User("ben") require.Nil(t, err) @@ -618,7 +628,7 @@ func TestManager_Token_Expire(t *testing.T) { func TestManager_Token_Extend(t *testing.T) { a := newTestManager(t, PermissionDenyAll) - require.Nil(t, a.AddUser("ben", "ben", RoleUser)) + require.Nil(t, a.AddUser("ben", "ben", RoleUser, false)) // Try to extend token for user without token u, err := a.User("ben") @@ -647,8 +657,8 @@ func TestManager_Token_MaxCount_AutoDelete(t *testing.T) { // Tests that tokens are automatically deleted when the maximum number of tokens is reached a := newTestManager(t, PermissionDenyAll) - require.Nil(t, a.AddUser("ben", "ben", RoleUser)) - require.Nil(t, a.AddUser("phil", "phil", RoleUser)) + require.Nil(t, a.AddUser("ben", "ben", RoleUser, false)) + require.Nil(t, a.AddUser("phil", "phil", RoleUser, false)) ben, err := a.User("ben") require.Nil(t, err) @@ -723,7 +733,7 @@ func TestManager_Token_MaxCount_AutoDelete(t *testing.T) { func TestManager_EnqueueStats_ResetStats(t *testing.T) { a, err := NewManager(filepath.Join(t.TempDir(), "db"), "", PermissionReadWrite, bcrypt.MinCost, 1500*time.Millisecond) require.Nil(t, err) - require.Nil(t, a.AddUser("ben", "ben", RoleUser)) + require.Nil(t, a.AddUser("ben", "ben", RoleUser, false)) // Baseline: No messages or emails u, err := a.User("ben") @@ -765,7 +775,7 @@ func TestManager_EnqueueStats_ResetStats(t *testing.T) { func TestManager_EnqueueTokenUpdate(t *testing.T) { a, err := NewManager(filepath.Join(t.TempDir(), "db"), "", PermissionReadWrite, bcrypt.MinCost, 500*time.Millisecond) require.Nil(t, err) - require.Nil(t, a.AddUser("ben", "ben", RoleUser)) + require.Nil(t, a.AddUser("ben", "ben", RoleUser, false)) // Create user and token u, err := a.User("ben") @@ -798,7 +808,7 @@ func TestManager_EnqueueTokenUpdate(t *testing.T) { func TestManager_ChangeSettings(t *testing.T) { a, err := NewManager(filepath.Join(t.TempDir(), "db"), "", PermissionReadWrite, bcrypt.MinCost, 1500*time.Millisecond) require.Nil(t, err) - require.Nil(t, a.AddUser("ben", "ben", RoleUser)) + require.Nil(t, a.AddUser("ben", "ben", RoleUser, false)) // No settings u, err := a.User("ben") @@ -866,7 +876,7 @@ func TestManager_Tier_Create_Update_List_Delete(t *testing.T) { AttachmentBandwidthLimit: 21474836480, StripeMonthlyPriceID: "price_2", })) - require.Nil(t, a.AddUser("phil", "phil", RoleUser)) + require.Nil(t, a.AddUser("phil", "phil", RoleUser, false)) require.Nil(t, a.ChangeTier("phil", "pro")) ti, err := a.Tier("pro") @@ -981,7 +991,7 @@ func TestManager_Tier_Change_And_Reset(t *testing.T) { Name: "Pro", ReservationLimit: 4, })) - require.Nil(t, a.AddUser("phil", "phil", RoleUser)) + require.Nil(t, a.AddUser("phil", "phil", RoleUser, false)) require.Nil(t, a.ChangeTier("phil", "pro")) // Add 10 reservations (pro tier allows that) @@ -1007,7 +1017,7 @@ func TestManager_Tier_Change_And_Reset(t *testing.T) { func TestUser_PhoneNumberAddListRemove(t *testing.T) { a := newTestManager(t, PermissionDenyAll) - require.Nil(t, a.AddUser("phil", "phil", RoleUser)) + require.Nil(t, a.AddUser("phil", "phil", RoleUser, false)) phil, err := a.User("phil") require.Nil(t, err) require.Nil(t, a.AddPhoneNumber(phil.ID, "+1234567890")) @@ -1032,8 +1042,8 @@ func TestUser_PhoneNumberAddListRemove(t *testing.T) { func TestUser_PhoneNumberAdd_Multiple_Users_Same_Number(t *testing.T) { a := newTestManager(t, PermissionDenyAll) - require.Nil(t, a.AddUser("phil", "phil", RoleUser)) - require.Nil(t, a.AddUser("ben", "ben", RoleUser)) + require.Nil(t, a.AddUser("phil", "phil", RoleUser, false)) + require.Nil(t, a.AddUser("ben", "ben", RoleUser, false)) phil, err := a.User("phil") require.Nil(t, err) ben, err := a.User("ben")