From 5a29a031adbcaca85ad641bf74254d3ea985d03c Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Sun, 23 Jul 2023 12:33:17 +0200 Subject: [PATCH] [chore] Admin CLI + new account creation refactoring (#2008) * set maxPasswordLength to 72 bytes, rename validate function * refactor NewSignup * refactor admin account CLI commands * refactor oidc create user * refactor processor create * tweak password change, check old != new password --- .../action/admin/account/account.go | 236 ++++++++---------- internal/api/auth/callback.go | 97 ++++--- internal/api/client/accounts/accountcreate.go | 2 +- internal/db/admin.go | 3 +- internal/db/bundb/admin.go | 161 +++++++----- internal/gtsmodel/admin.go | 25 +- internal/processing/account/create.go | 72 +++--- internal/processing/user/password.go | 31 ++- internal/processing/user/password_test.go | 2 +- internal/validate/formvalidation.go | 4 +- internal/validate/formvalidation_test.go | 16 +- 11 files changed, 373 insertions(+), 276 deletions(-) diff --git a/cmd/gotosocial/action/admin/account/account.go b/cmd/gotosocial/action/admin/account/account.go index 3941769ec..9bb5b27c1 100644 --- a/cmd/gotosocial/action/admin/account/account.go +++ b/cmd/gotosocial/action/admin/account/account.go @@ -19,7 +19,6 @@ import ( "context" - "errors" "fmt" "os" "text/tabwriter" @@ -28,88 +27,101 @@ "github.com/superseriousbusiness/gotosocial/cmd/gotosocial/action" "github.com/superseriousbusiness/gotosocial/internal/config" "github.com/superseriousbusiness/gotosocial/internal/db/bundb" + "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/state" "github.com/superseriousbusiness/gotosocial/internal/validate" "golang.org/x/crypto/bcrypt" ) -// Create creates a new account in the database using the provided flags. -var Create action.GTSAction = func(ctx context.Context) error { +func initState(ctx context.Context) (*state.State, error) { var state state.State state.Caches.Init() + state.Caches.Start() state.Workers.Start() + // Set the state DB connection dbConn, err := bundb.NewBunDBService(ctx, &state) if err != nil { - return fmt.Errorf("error creating dbservice: %s", err) + return nil, fmt.Errorf("error creating dbConn: %w", err) } - - // Set the state DB connection state.DB = dbConn - username := config.GetAdminAccountUsername() - if username == "" { - return errors.New("no username set") + return &state, nil +} + +func stopState(ctx context.Context, state *state.State) error { + if err := state.DB.Stop(ctx); err != nil { + return fmt.Errorf("error stopping dbConn: %w", err) } + + state.Workers.Stop() + state.Caches.Stop() + + return nil +} + +// Create creates a new account and user +// in the database using the provided flags. +var Create action.GTSAction = func(ctx context.Context) error { + state, err := initState(ctx) + if err != nil { + return err + } + + username := config.GetAdminAccountUsername() if err := validate.Username(username); err != nil { return err } - usernameAvailable, err := dbConn.IsUsernameAvailable(ctx, username) + usernameAvailable, err := state.DB.IsUsernameAvailable(ctx, username) if err != nil { return err } + if !usernameAvailable { return fmt.Errorf("username %s is already in use", username) } email := config.GetAdminAccountEmail() - if email == "" { - return errors.New("no email set") - } if err := validate.Email(email); err != nil { return err } - emailAvailable, err := dbConn.IsEmailAvailable(ctx, email) + emailAvailable, err := state.DB.IsEmailAvailable(ctx, email) if err != nil { return err } + if !emailAvailable { return fmt.Errorf("email address %s is already in use", email) } password := config.GetAdminAccountPassword() - if password == "" { - return errors.New("no password set") - } - if err := validate.NewPassword(password); err != nil { + if err := validate.Password(password); err != nil { return err } - _, err = dbConn.NewSignup(ctx, username, "", false, email, password, nil, "", "", true, "", false) - if err != nil { + if _, err := state.DB.NewSignup(ctx, gtsmodel.NewSignup{ + Username: username, + Email: email, + Password: password, + EmailVerified: true, // Assume cli user wants email marked as verified already. + PreApproved: true, // Assume cli user wants account marked as approved already. + }); err != nil { return err } - return dbConn.Stop(ctx) + return stopState(ctx, state) } // List returns all existing local accounts. var List action.GTSAction = func(ctx context.Context) error { - var state state.State - state.Caches.Init() - state.Workers.Start() - - dbConn, err := bundb.NewBunDBService(ctx, &state) + state, err := initState(ctx) if err != nil { - return fmt.Errorf("error creating dbservice: %s", err) + return err } - // Set the state DB connection - state.DB = dbConn - - users, err := dbConn.GetAllUsers(ctx) + users, err := state.DB.GetAllUsers(ctx) if err != nil { return err } @@ -140,218 +152,182 @@ return nil } -// Confirm sets a user to Approved, sets Email to the current UnconfirmedEmail value, and sets ConfirmedAt to now. +// Confirm sets a user to Approved, sets Email to the current +// UnconfirmedEmail value, and sets ConfirmedAt to now. var Confirm action.GTSAction = func(ctx context.Context) error { - var state state.State - state.Caches.Init() - state.Workers.Start() - - dbConn, err := bundb.NewBunDBService(ctx, &state) + state, err := initState(ctx) if err != nil { - return fmt.Errorf("error creating dbservice: %s", err) + return err } - // Set the state DB connection - state.DB = dbConn - username := config.GetAdminAccountUsername() - if username == "" { - return errors.New("no username set") - } if err := validate.Username(username); err != nil { return err } - a, err := dbConn.GetAccountByUsernameDomain(ctx, username, "") + account, err := state.DB.GetAccountByUsernameDomain(ctx, username, "") if err != nil { return err } - u, err := dbConn.GetUserByAccountID(ctx, a.ID) + user, err := state.DB.GetUserByAccountID(ctx, account.ID) if err != nil { return err } - updatingColumns := []string{"approved", "email", "confirmed_at"} - approved := true - u.Approved = &approved - u.Email = u.UnconfirmedEmail - u.ConfirmedAt = time.Now() - if err := dbConn.UpdateUser(ctx, u, updatingColumns...); err != nil { + user.Approved = func() *bool { a := true; return &a }() + user.Email = user.UnconfirmedEmail + user.ConfirmedAt = time.Now() + if err := state.DB.UpdateUser( + ctx, user, + "approved", "email", "confirmed_at", + ); err != nil { return err } - return dbConn.Stop(ctx) + return stopState(ctx, state) } -// Promote sets a user to admin. +// Promote sets admin + moderator flags on a user to true. var Promote action.GTSAction = func(ctx context.Context) error { - var state state.State - state.Caches.Init() - state.Workers.Start() - - dbConn, err := bundb.NewBunDBService(ctx, &state) + state, err := initState(ctx) if err != nil { - return fmt.Errorf("error creating dbservice: %s", err) + return err } - // Set the state DB connection - state.DB = dbConn - username := config.GetAdminAccountUsername() - if username == "" { - return errors.New("no username set") - } if err := validate.Username(username); err != nil { return err } - a, err := dbConn.GetAccountByUsernameDomain(ctx, username, "") + account, err := state.DB.GetAccountByUsernameDomain(ctx, username, "") if err != nil { return err } - u, err := dbConn.GetUserByAccountID(ctx, a.ID) + user, err := state.DB.GetUserByAccountID(ctx, account.ID) if err != nil { return err } - admin := true - u.Admin = &admin - if err := dbConn.UpdateUser(ctx, u, "admin"); err != nil { + user.Admin = func() *bool { a := true; return &a }() + user.Moderator = func() *bool { a := true; return &a }() + if err := state.DB.UpdateUser( + ctx, user, + "admin", "moderator", + ); err != nil { return err } - return dbConn.Stop(ctx) + return stopState(ctx, state) } -// Demote sets admin on a user to false. +// Demote sets admin + moderator flags on a user to false. var Demote action.GTSAction = func(ctx context.Context) error { - var state state.State - state.Caches.Init() - state.Workers.Start() - - dbConn, err := bundb.NewBunDBService(ctx, &state) + state, err := initState(ctx) if err != nil { - return fmt.Errorf("error creating dbservice: %s", err) + return err } - // Set the state DB connection - state.DB = dbConn - username := config.GetAdminAccountUsername() - if username == "" { - return errors.New("no username set") - } if err := validate.Username(username); err != nil { return err } - a, err := dbConn.GetAccountByUsernameDomain(ctx, username, "") + a, err := state.DB.GetAccountByUsernameDomain(ctx, username, "") if err != nil { return err } - u, err := dbConn.GetUserByAccountID(ctx, a.ID) + user, err := state.DB.GetUserByAccountID(ctx, a.ID) if err != nil { return err } - admin := false - u.Admin = &admin - if err := dbConn.UpdateUser(ctx, u, "admin"); err != nil { + user.Admin = func() *bool { a := false; return &a }() + user.Moderator = func() *bool { a := false; return &a }() + if err := state.DB.UpdateUser( + ctx, user, + "admin", "moderator", + ); err != nil { return err } - return dbConn.Stop(ctx) + return stopState(ctx, state) } // Disable sets Disabled to true on a user. var Disable action.GTSAction = func(ctx context.Context) error { - var state state.State - state.Caches.Init() - state.Workers.Start() - - dbConn, err := bundb.NewBunDBService(ctx, &state) + state, err := initState(ctx) if err != nil { - return fmt.Errorf("error creating dbservice: %s", err) + return err } - // Set the state DB connection - state.DB = dbConn - username := config.GetAdminAccountUsername() - if username == "" { - return errors.New("no username set") - } if err := validate.Username(username); err != nil { return err } - a, err := dbConn.GetAccountByUsernameDomain(ctx, username, "") + account, err := state.DB.GetAccountByUsernameDomain(ctx, username, "") if err != nil { return err } - u, err := dbConn.GetUserByAccountID(ctx, a.ID) + user, err := state.DB.GetUserByAccountID(ctx, account.ID) if err != nil { return err } - disabled := true - u.Disabled = &disabled - if err := dbConn.UpdateUser(ctx, u, "disabled"); err != nil { + user.Disabled = func() *bool { d := true; return &d }() + if err := state.DB.UpdateUser( + ctx, user, + "disabled", + ); err != nil { return err } - return dbConn.Stop(ctx) + return stopState(ctx, state) } // Password sets the password of target account. var Password action.GTSAction = func(ctx context.Context) error { - var state state.State - state.Caches.Init() - state.Workers.Start() - - dbConn, err := bundb.NewBunDBService(ctx, &state) + state, err := initState(ctx) if err != nil { - return fmt.Errorf("error creating dbservice: %s", err) + return err } - // Set the state DB connection - state.DB = dbConn - username := config.GetAdminAccountUsername() - if username == "" { - return errors.New("no username set") - } if err := validate.Username(username); err != nil { return err } password := config.GetAdminAccountPassword() - if password == "" { - return errors.New("no password set") - } - if err := validate.NewPassword(password); err != nil { + if err := validate.Password(password); err != nil { return err } - a, err := dbConn.GetAccountByUsernameDomain(ctx, username, "") + account, err := state.DB.GetAccountByUsernameDomain(ctx, username, "") if err != nil { return err } - u, err := dbConn.GetUserByAccountID(ctx, a.ID) + user, err := state.DB.GetUserByAccountID(ctx, account.ID) if err != nil { return err } - pw, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) + encryptedPassword, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) if err != nil { return fmt.Errorf("error hashing password: %s", err) } - u.EncryptedPassword = string(pw) - return dbConn.UpdateUser(ctx, u, "encrypted_password") + user.EncryptedPassword = string(encryptedPassword) + if err := state.DB.UpdateUser( + ctx, user, + "encrypted_password", + ); err != nil { + return err + } + + return stopState(ctx, state) } diff --git a/internal/api/auth/callback.go b/internal/api/auth/callback.go index 05355b94d..8871fd2dc 100644 --- a/internal/api/auth/callback.go +++ b/internal/api/auth/callback.go @@ -272,50 +272,89 @@ func (m *Module) fetchUserForClaims(ctx context.Context, claims *oidc.Claims, ip } func (m *Module) createUserFromOIDC(ctx context.Context, claims *oidc.Claims, extraInfo *extraInfo, ip net.IP, appID string) (*gtsmodel.User, gtserror.WithCode) { - // check if the email address is available for use; if it's not there's nothing we can so + // Check if the claimed email address is available for use. emailAvailable, err := m.db.IsEmailAvailable(ctx, claims.Email) if err != nil { - return nil, gtserror.NewErrorBadRequest(err) + err := gtserror.Newf("db error checking email availability: %w", err) + return nil, gtserror.NewErrorInternalError(err) } + if !emailAvailable { - help := "The email address given to us by your authentication provider already exists in our records and the server administrator has not enabled account migration" - return nil, gtserror.NewErrorConflict(fmt.Errorf("email address %s is not available", claims.Email), help) + const help = "The email address given to us by your authentication provider already exists in our records and the server administrator has not enabled account migration" + err := gtserror.Newf("email address %s is not available", claims.Email) + return nil, gtserror.NewErrorConflict(err, help) } - // check if the user is in any recognised admin groups - adminGroups := config.GetOIDCAdminGroups() - var admin bool -LOOP: - for _, g := range claims.Groups { - for _, ag := range adminGroups { - if strings.EqualFold(g, ag) { - admin = true - break LOOP - } - } - } - - // We still need to set *a* password even if it's not a password the user will end up using, so set something random. - // We'll just set two uuids on top of each other, which should be long + random enough to baffle any attempts to crack. + // We still need to set something as a password, even + // if it's not a password the user will end up using. // - // If the user ever wants to log in using gts password rather than oidc flow, they'll have to request a password reset, which is fine + // We'll just set two uuids on top of each other, which + // should be long + random enough to baffle any attempts + // to crack, and which is also, conveniently, 72 bytes, + // which is the maximum length that bcrypt can handle. + // + // If the user ever wants to log in using a password + // rather than oidc flow, they'll have to request a + // password reset, which is fine. password := uuid.NewString() + uuid.NewString() - // Since this user is created via oidc, which has been set up by the admin, we can assume that the account is already - // implicitly approved, and that the email address has already been verified: otherwise, we end up in situations where - // the admin first approves the user in OIDC, and then has to approve them again in GoToSocial, which doesn't make sense. + // Since this user is created via OIDC, we can assume + // that the account should be preapproved, and the email + // address should be considered as verified already, + // since the OIDC login was successful. // - // In other words, if a user logs in via OIDC, they should be able to use their account straight away. + // If we don't assume this, we end up in a situation + // where the admin first adds a user to OIDC, then has + // to approve them again in GoToSocial when they log in + // there for the first time, which doesn't make sense. // - // See: https://github.com/superseriousbusiness/gotosocial/issues/357 - requireApproval := false - emailVerified := true + // In other words, if a user logs in via OIDC, they + // should be able to use their account straight away. + var ( + preApproved = true + emailVerified = true + ) - // create the user! this will also create an account and store it in the database so we don't need to do that here - user, err := m.db.NewSignup(ctx, extraInfo.Username, "", requireApproval, claims.Email, password, ip, "", appID, emailVerified, claims.Sub, admin) + // If one of the claimed groups corresponds to one of + // the configured admin OIDC groups, create this user + // as an admin. + admin := adminGroup(claims.Groups) + + // Create the user! This will also create an account and + // store it in the database, so we don't need to do that. + user, err := m.db.NewSignup(ctx, gtsmodel.NewSignup{ + Username: extraInfo.Username, + Email: claims.Email, + Password: password, + SignUpIP: ip, + AppID: appID, + ExternalID: claims.Sub, + PreApproved: preApproved, + EmailVerified: emailVerified, + Admin: admin, + }) if err != nil { + err := gtserror.Newf("db error doing new signup: %w", err) return nil, gtserror.NewErrorInternalError(err) } return user, nil } + +// adminGroup returns true if one of the given OIDC +// groups is equal to at least one admin OIDC group. +func adminGroup(groups []string) bool { + for _, ag := range config.GetOIDCAdminGroups() { + for _, g := range groups { + if strings.EqualFold(ag, g) { + // This is an admin group, + // ∴ user is an admin. + return true + } + } + } + + // User is in no admin groups, + // ∴ user is not an admin. + return false +} diff --git a/internal/api/client/accounts/accountcreate.go b/internal/api/client/accounts/accountcreate.go index 5999c46c1..c8247ecf2 100644 --- a/internal/api/client/accounts/accountcreate.go +++ b/internal/api/client/accounts/accountcreate.go @@ -129,7 +129,7 @@ func validateCreateAccount(form *apimodel.AccountCreateRequest) error { return err } - if err := validate.NewPassword(form.Password); err != nil { + if err := validate.Password(form.Password); err != nil { return err } diff --git a/internal/db/admin.go b/internal/db/admin.go index 29ddaa616..57ded68b1 100644 --- a/internal/db/admin.go +++ b/internal/db/admin.go @@ -19,7 +19,6 @@ import ( "context" - "net" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" ) @@ -39,7 +38,7 @@ type Admin interface { // NewSignup creates a new user in the database with the given parameters. // By the time this function is called, it should be assumed that all the parameters have passed validation! - NewSignup(ctx context.Context, username string, reason string, requireApproval bool, email string, password string, signUpIP net.IP, locale string, appID string, emailVerified bool, externalID string, admin bool) (*gtsmodel.User, Error) + NewSignup(ctx context.Context, newSignup gtsmodel.NewSignup) (*gtsmodel.User, Error) // CreateInstanceAccount creates an account in the database with the same username as the instance host value. // Ie., if the instance is hosted at 'example.org' the instance user will have a username of 'example.org'. diff --git a/internal/db/bundb/admin.go b/internal/db/bundb/admin.go index ed595a951..61ae1e044 100644 --- a/internal/db/bundb/admin.go +++ b/internal/db/bundb/admin.go @@ -21,8 +21,8 @@ "context" "crypto/rand" "crypto/rsa" + "errors" "fmt" - "net" "net/mail" "strings" "time" @@ -30,6 +30,7 @@ "github.com/superseriousbusiness/gotosocial/internal/ap" "github.com/superseriousbusiness/gotosocial/internal/config" "github.com/superseriousbusiness/gotosocial/internal/db" + "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/id" "github.com/superseriousbusiness/gotosocial/internal/log" @@ -89,106 +90,134 @@ func (a *adminDB) IsEmailAvailable(ctx context.Context, email string) (bool, db. return a.conn.NotExists(ctx, q) } -func (a *adminDB) NewSignup(ctx context.Context, username string, reason string, requireApproval bool, email string, password string, signUpIP net.IP, locale string, appID string, emailVerified bool, externalID string, admin bool) (*gtsmodel.User, db.Error) { - key, err := rsa.GenerateKey(rand.Reader, rsaKeyBits) - if err != nil { - log.Errorf(ctx, "error creating new rsa key: %s", err) +func (a *adminDB) NewSignup(ctx context.Context, newSignup gtsmodel.NewSignup) (*gtsmodel.User, db.Error) { + // If something went wrong previously while doing a new + // sign up with this username, we might already have an + // account, so check first. + account, err := a.state.DB.GetAccountByUsernameDomain(ctx, newSignup.Username, "") + if err != nil && !errors.Is(err, db.ErrNoEntries) { + // Real error occurred. + err := gtserror.Newf("error checking for existing account: %w", err) return nil, err } - // if something went wrong while creating a user, we might already have an account, so check here first... - acct := >smodel.Account{} - if err := a.conn. - NewSelect(). - Model(acct). - Where("? = ?", bun.Ident("account.username"), username). - Where("? IS NULL", bun.Ident("account.domain")). - Scan(ctx); err != nil { - err = a.conn.ProcessError(err) - if err != db.ErrNoEntries { - log.Errorf(ctx, "error checking for existing account: %s", err) - return nil, err - } + // If we didn't yet have an account + // with this username, create one now. + if account == nil { + uris := uris.GenerateURIsForAccount(newSignup.Username) - // if we have db.ErrNoEntries, we just don't have an - // account yet so create one before we proceed - accountURIs := uris.GenerateURIsForAccount(username) accountID, err := id.NewRandomULID() if err != nil { + err := gtserror.Newf("error creating new account id: %w", err) return nil, err } - acct = >smodel.Account{ + privKey, err := rsa.GenerateKey(rand.Reader, rsaKeyBits) + if err != nil { + err := gtserror.Newf("error creating new rsa private key: %w", err) + return nil, err + } + + account = >smodel.Account{ ID: accountID, - Username: username, - DisplayName: username, - Reason: reason, + Username: newSignup.Username, + DisplayName: newSignup.Username, + Reason: newSignup.Reason, Privacy: gtsmodel.VisibilityDefault, - URL: accountURIs.UserURL, - PrivateKey: key, - PublicKey: &key.PublicKey, - PublicKeyURI: accountURIs.PublicKeyURI, + URI: uris.UserURI, + URL: uris.UserURL, + InboxURI: uris.InboxURI, + OutboxURI: uris.OutboxURI, + FollowingURI: uris.FollowingURI, + FollowersURI: uris.FollowersURI, + FeaturedCollectionURI: uris.FeaturedCollectionURI, ActorType: ap.ActorPerson, - URI: accountURIs.UserURI, - InboxURI: accountURIs.InboxURI, - OutboxURI: accountURIs.OutboxURI, - FollowersURI: accountURIs.FollowersURI, - FollowingURI: accountURIs.FollowingURI, - FeaturedCollectionURI: accountURIs.FeaturedCollectionURI, + PrivateKey: privKey, + PublicKey: &privKey.PublicKey, + PublicKeyURI: uris.PublicKeyURI, } - // insert the new account! - if err := a.state.DB.PutAccount(ctx, acct); err != nil { + // Insert the new account! + if err := a.state.DB.PutAccount(ctx, account); err != nil { return nil, err } } - // we either created or already had an account by now, - // so proceed with creating a user for that account - - pw, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) - if err != nil { - return nil, fmt.Errorf("error hashing password: %s", err) + // Created or already had an account. + // Ensure user not already created. + user, err := a.state.DB.GetUserByAccountID(ctx, account.ID) + if err != nil && !errors.Is(err, db.ErrNoEntries) { + // Real error occurred. + err := gtserror.Newf("error checking for existing user: %w", err) + return nil, err } + defer func() { + // Pin account to (new) + // user before returning. + user.Account = account + }() + + if user != nil { + // Already had a user for this + // account, just return that. + return user, nil + } + + // Had no user for this account, time to create one! newUserID, err := id.NewRandomULID() if err != nil { + err := gtserror.Newf("error creating new user id: %w", err) return nil, err } - // if we don't require moderator approval, just pre-approve the user - approved := !requireApproval - u := >smodel.User{ + encryptedPassword, err := bcrypt.GenerateFromPassword( + []byte(newSignup.Password), + bcrypt.DefaultCost, + ) + if err != nil { + err := gtserror.Newf("error hashing password: %w", err) + return nil, err + } + + user = >smodel.User{ ID: newUserID, - AccountID: acct.ID, - Account: acct, - EncryptedPassword: string(pw), - SignUpIP: signUpIP.To4(), - Locale: locale, - UnconfirmedEmail: email, - CreatedByApplicationID: appID, - Approved: &approved, - ExternalID: externalID, + AccountID: account.ID, + Account: account, + EncryptedPassword: string(encryptedPassword), + SignUpIP: newSignup.SignUpIP.To4(), + Locale: newSignup.Locale, + UnconfirmedEmail: newSignup.Email, + CreatedByApplicationID: newSignup.AppID, + ExternalID: newSignup.ExternalID, } - if emailVerified { - u.ConfirmedAt = time.Now() - u.Email = email + if newSignup.EmailVerified { + // Mark given email as confirmed. + user.ConfirmedAt = time.Now() + user.Email = newSignup.Email } - if admin { - admin := true - moderator := true - u.Admin = &admin - u.Moderator = &moderator + trueBool := func() *bool { t := true; return &t } + + if newSignup.Admin { + // Make new user mod + admin. + user.Moderator = trueBool() + user.Admin = trueBool() } - // insert the user! - if err := a.state.DB.PutUser(ctx, u); err != nil { + if newSignup.PreApproved { + // Mark new user as approved. + user.Approved = trueBool() + } + + // Insert the user! + if err := a.state.DB.PutUser(ctx, user); err != nil { + err := gtserror.Newf("db error inserting user: %w", err) return nil, err } - return u, nil + return user, nil } func (a *adminDB) CreateInstanceAccount(ctx context.Context) db.Error { diff --git a/internal/gtsmodel/admin.go b/internal/gtsmodel/admin.go index f6f32a9a3..22a38f32c 100644 --- a/internal/gtsmodel/admin.go +++ b/internal/gtsmodel/admin.go @@ -17,7 +17,10 @@ package gtsmodel -import "time" +import ( + "net" + "time" +) // AdminAccountAction models an action taken by an instance administrator on an account. type AdminAccountAction struct { @@ -45,3 +48,23 @@ type AdminAccountAction struct { // AdminActionSuspend -- the account or application etc has been deleted. AdminActionSuspend AdminActionType = "suspend" ) + +// NewSignup models parameters for the creation +// of a new user + account on this instance. +// +// Aside from username, email, and password, it is +// fine to use zero values on fields of this struct. +type NewSignup struct { + Username string // Username of the new account. + Email string // Email address of the user. + Password string // Plaintext (not yet hashed) password for the user. + + Reason string // Reason given by the user when submitting a sign up request (optional). + PreApproved bool // Mark the new user/account as preapproved (optional) + SignUpIP net.IP // IP address from which the sign up request occurred (optional). + Locale string // Locale code for the new account/user (optional). + AppID string // ID of the application used to create this account (optional). + EmailVerified bool // Mark submitted email address as already verified (optional). + ExternalID string // ID of this user in external OIDC system (optional). + Admin bool // Mark new user as an admin user (optional). +} diff --git a/internal/processing/account/create.go b/internal/processing/account/create.go index 63baa713e..1a172b865 100644 --- a/internal/processing/account/create.go +++ b/internal/processing/account/create.go @@ -26,61 +26,73 @@ "github.com/superseriousbusiness/gotosocial/internal/config" "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" - "github.com/superseriousbusiness/gotosocial/internal/log" "github.com/superseriousbusiness/gotosocial/internal/messages" "github.com/superseriousbusiness/gotosocial/internal/text" "github.com/superseriousbusiness/oauth2/v4" ) -// Create processes the given form for creating a new account, returning an oauth token for that account if successful. -func (p *Processor) Create(ctx context.Context, applicationToken oauth2.TokenInfo, application *gtsmodel.Application, form *apimodel.AccountCreateRequest) (*apimodel.Token, gtserror.WithCode) { +// Create processes the given form for creating a new account, +// returning an oauth token for that account if successful. +// +// Fields on the form should have already been validated by the +// caller, before this function is called. +func (p *Processor) Create( + ctx context.Context, + appToken oauth2.TokenInfo, + app *gtsmodel.Application, + form *apimodel.AccountCreateRequest, +) (*apimodel.Token, gtserror.WithCode) { emailAvailable, err := p.state.DB.IsEmailAvailable(ctx, form.Email) if err != nil { - return nil, gtserror.NewErrorBadRequest(err) + err := fmt.Errorf("db error checking email availability: %w", err) + return nil, gtserror.NewErrorInternalError(err) } if !emailAvailable { - return nil, gtserror.NewErrorConflict(fmt.Errorf("email address %s is not available", form.Email)) + err := fmt.Errorf("email address %s is not available", form.Email) + return nil, gtserror.NewErrorConflict(err, err.Error()) } usernameAvailable, err := p.state.DB.IsUsernameAvailable(ctx, form.Username) if err != nil { - return nil, gtserror.NewErrorBadRequest(err) + err := fmt.Errorf("db error checking username availability: %w", err) + return nil, gtserror.NewErrorInternalError(err) } if !usernameAvailable { - return nil, gtserror.NewErrorConflict(fmt.Errorf("username %s in use", form.Username)) + err := fmt.Errorf("username %s is not available", form.Username) + return nil, gtserror.NewErrorConflict(err, err.Error()) } - reasonRequired := config.GetAccountsReasonRequired() - approvalRequired := config.GetAccountsApprovalRequired() - - // don't store a reason if we don't require one - reason := form.Reason - if !reasonRequired { - reason = "" + // Only store reason if one is required. + var reason string + if config.GetAccountsReasonRequired() { + reason = form.Reason } - log.Trace(ctx, "creating new username and account") - user, err := p.state.DB.NewSignup(ctx, form.Username, text.SanitizePlaintext(reason), approvalRequired, form.Email, form.Password, form.IP, form.Locale, application.ID, false, "", false) + user, err := p.state.DB.NewSignup(ctx, gtsmodel.NewSignup{ + Username: form.Username, + Email: form.Email, + Password: form.Password, + Reason: text.SanitizePlaintext(reason), + PreApproved: !config.GetAccountsApprovalRequired(), // Mark as approved if no approval required. + SignUpIP: form.IP, + Locale: form.Locale, + AppID: app.ID, + }) if err != nil { - return nil, gtserror.NewErrorInternalError(fmt.Errorf("error creating new signup in the database: %s", err)) + err := fmt.Errorf("db error creating new signup: %w", err) + return nil, gtserror.NewErrorInternalError(err) } - log.Tracef(ctx, "generating a token for user %s with account %s and application %s", user.ID, user.AccountID, application.ID) - accessToken, err := p.oauthServer.GenerateUserAccessToken(ctx, applicationToken, application.ClientSecret, user.ID) + // Generate access token *before* doing side effects; we + // don't want to process side effects if something borks. + accessToken, err := p.oauthServer.GenerateUserAccessToken(ctx, appToken, app.ClientSecret, user.ID) if err != nil { - return nil, gtserror.NewErrorInternalError(fmt.Errorf("error creating new access token for user %s: %s", user.ID, err)) + err := fmt.Errorf("error creating new access token for user %s: %w", user.ID, err) + return nil, gtserror.NewErrorInternalError(err) } - if user.Account == nil { - a, err := p.state.DB.GetAccountByID(ctx, user.AccountID) - if err != nil { - return nil, gtserror.NewErrorInternalError(fmt.Errorf("error getting new account from the database: %s", err)) - } - user.Account = a - } - - // there are side effects for creating a new account (sending confirmation emails etc) - // so pass a message to the processor so that it can do it asynchronously + // There are side effects for creating a new account + // (confirmation emails etc), perform these async. p.state.Workers.EnqueueClientAPI(ctx, messages.FromClientAPI{ APObjectType: ap.ObjectProfile, APActivityType: ap.ActivityCreate, diff --git a/internal/processing/user/password.go b/internal/processing/user/password.go index 244a5d349..68bc8ddb5 100644 --- a/internal/processing/user/password.go +++ b/internal/processing/user/password.go @@ -28,22 +28,41 @@ // PasswordChange processes a password change request for the given user. func (p *Processor) PasswordChange(ctx context.Context, user *gtsmodel.User, oldPassword string, newPassword string) gtserror.WithCode { + // Ensure provided oldPassword is the correct current password. if err := bcrypt.CompareHashAndPassword([]byte(user.EncryptedPassword), []byte(oldPassword)); err != nil { + err := gtserror.Newf("%w", err) return gtserror.NewErrorUnauthorized(err, "old password was incorrect") } - if err := validate.NewPassword(newPassword); err != nil { + // Ensure new password is strong enough. + if err := validate.Password(newPassword); err != nil { return gtserror.NewErrorBadRequest(err, err.Error()) } - newPasswordHash, err := bcrypt.GenerateFromPassword([]byte(newPassword), bcrypt.DefaultCost) - if err != nil { - return gtserror.NewErrorInternalError(err, "error hashing password") + // Ensure new password is different from old password. + if newPassword == oldPassword { + const help = "new password cannot be the same as previous password" + err := gtserror.New(help) + return gtserror.NewErrorBadRequest(err, help) } - user.EncryptedPassword = string(newPasswordHash) + // Hash the new password. + encryptedPassword, err := bcrypt.GenerateFromPassword( + []byte(newPassword), + bcrypt.DefaultCost, + ) + if err != nil { + err := gtserror.Newf("%w", err) + return gtserror.NewErrorInternalError(err) + } - if err := p.state.DB.UpdateUser(ctx, user, "encrypted_password"); err != nil { + // Set new password on user. + user.EncryptedPassword = string(encryptedPassword) + if err := p.state.DB.UpdateUser( + ctx, user, + "encrypted_password", + ); err != nil { + err := gtserror.Newf("db error updating user: %w", err) return gtserror.NewErrorInternalError(err) } diff --git a/internal/processing/user/password_test.go b/internal/processing/user/password_test.go index 785f742b5..ee30558c6 100644 --- a/internal/processing/user/password_test.go +++ b/internal/processing/user/password_test.go @@ -54,7 +54,7 @@ func (suite *ChangePasswordTestSuite) TestChangePasswordIncorrectOld() { user := suite.testUsers["local_account_1"] errWithCode := suite.user.PasswordChange(context.Background(), user, "ooooopsydoooopsy", "verygoodnewpassword") - suite.EqualError(errWithCode, "crypto/bcrypt: hashedPassword is not the hash of the given password") + suite.EqualError(errWithCode, "PasswordChange: crypto/bcrypt: hashedPassword is not the hash of the given password") suite.Equal(http.StatusUnauthorized, errWithCode.Code()) suite.Equal("Unauthorized: old password was incorrect", errWithCode.Safe()) diff --git a/internal/validate/formvalidation.go b/internal/validate/formvalidation.go index 51efa320c..c109c26d8 100644 --- a/internal/validate/formvalidation.go +++ b/internal/validate/formvalidation.go @@ -46,9 +46,9 @@ maximumListTitleLength = 200 ) -// NewPassword returns a helpful error if the given password +// Password returns a helpful error if the given password // is too short, too long, or not sufficiently strong. -func NewPassword(password string) error { +func Password(password string) error { // Ensure length is OK first. if pwLen := len(password); pwLen == 0 { return errors.New("no password provided / provided password was 0 bytes") diff --git a/internal/validate/formvalidation_test.go b/internal/validate/formvalidation_test.go index accd48ab6..534e5b849 100644 --- a/internal/validate/formvalidation_test.go +++ b/internal/validate/formvalidation_test.go @@ -43,42 +43,42 @@ func (suite *ValidationTestSuite) TestCheckPasswordStrength() { strongPassword := "3dX5@Zc%mV*W2MBNEy$@" var err error - err = validate.NewPassword(empty) + err = validate.Password(empty) if suite.Error(err) { suite.Equal(errors.New("no password provided / provided password was 0 bytes"), err) } - err = validate.NewPassword(terriblePassword) + err = validate.Password(terriblePassword) if suite.Error(err) { suite.Equal(errors.New("password is only 62% strength, try including more special characters, using uppercase letters, using numbers or using a longer password"), err) } - err = validate.NewPassword(weakPassword) + err = validate.Password(weakPassword) if suite.Error(err) { suite.Equal(errors.New("password is only 95% strength, try including more special characters, using numbers or using a longer password"), err) } - err = validate.NewPassword(shortPassword) + err = validate.Password(shortPassword) if suite.Error(err) { suite.Equal(errors.New("password is only 39% strength, try including more special characters or using a longer password"), err) } - err = validate.NewPassword(specialPassword) + err = validate.Password(specialPassword) if suite.Error(err) { suite.Equal(errors.New("password is only 53% strength, try including more special characters or using a longer password"), err) } - err = validate.NewPassword(longPassword) + err = validate.Password(longPassword) if suite.NoError(err) { suite.Equal(nil, err) } - err = validate.NewPassword(tooLong) + err = validate.Password(tooLong) if suite.Error(err) { suite.Equal(errors.New("password should be no more than 72 bytes, provided password was 571 bytes"), err) } - err = validate.NewPassword(strongPassword) + err = validate.Password(strongPassword) if suite.NoError(err) { suite.Equal(nil, err) }