diff --git a/appservice/appservice.go b/appservice/appservice.go index be5b30e2b..d78741983 100644 --- a/appservice/appservice.go +++ b/appservice/appservice.go @@ -16,6 +16,7 @@ package appservice import ( "context" + "errors" "net/http" "sync" "time" @@ -29,6 +30,7 @@ import ( "github.com/matrix-org/dendrite/appservice/workers" "github.com/matrix-org/dendrite/clientapi/auth/storage/accounts" "github.com/matrix-org/dendrite/clientapi/auth/storage/devices" + "github.com/matrix-org/dendrite/internal" "github.com/matrix-org/dendrite/internal/basecomponent" "github.com/matrix-org/dendrite/internal/config" "github.com/matrix-org/dendrite/internal/transactions" @@ -117,12 +119,12 @@ func generateAppServiceAccount( ctx := context.Background() // Create an account for the application service - acc, err := accountsDB.CreateAccount(ctx, as.SenderLocalpart, "", as.ID) + _, err := accountsDB.CreateAccount(ctx, as.SenderLocalpart, "", as.ID) if err != nil { + if errors.Is(err, internal.ErrSQLUserExists) { // This account already exists + return nil + } return err - } else if acc == nil { - // This account already exists - return nil } // Create a dummy device with a dummy token for the application service diff --git a/clientapi/auth/storage/accounts/postgres/storage.go b/clientapi/auth/storage/accounts/postgres/storage.go index 4a1832674..ba52ecfdc 100644 --- a/clientapi/auth/storage/accounts/postgres/storage.go +++ b/clientapi/auth/storage/accounts/postgres/storage.go @@ -164,7 +164,7 @@ func (d *Database) createAccount( } if err := d.profiles.insertProfile(ctx, txn, localpart); err != nil { if internal.IsUniqueConstraintViolationErr(err) { - return nil, nil + return nil, internal.ErrSQLUserExists } return nil, err } diff --git a/clientapi/routing/register.go b/clientapi/routing/register.go index 40a2862f8..e69d3bbfc 100644 --- a/clientapi/routing/register.go +++ b/clientapi/routing/register.go @@ -830,15 +830,16 @@ func completeRegistration( acc, err := accountDB.CreateAccount(ctx, username, password, appserviceID) if err != nil { + if errors.Is(err, internal.ErrSQLUserExists) { // user already exists + return util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.UserInUse("Desired user ID is already taken."), + } + } return util.JSONResponse{ Code: http.StatusInternalServerError, JSON: jsonerror.Unknown("failed to create account: " + err.Error()), } - } else if acc == nil { - return util.JSONResponse{ - Code: http.StatusBadRequest, - JSON: jsonerror.UserInUse("Desired user ID is already taken."), - } } // Increment prometheus counter for created users diff --git a/internal/sql.go b/internal/sql.go index d6a5a3086..e28a52a49 100644 --- a/internal/sql.go +++ b/internal/sql.go @@ -16,11 +16,15 @@ package internal import ( "database/sql" + "errors" "fmt" "runtime" "time" ) +// ErrUserExists is returned if a username already exists in the database. +var ErrSQLUserExists = errors.New("Username already exists") + // A Transaction is something that can be committed or rolledback. type Transaction interface { // Commit the transaction