Return newly created error if user already exists (#1002)

Signed-off-by: Till Faelligen <tfaelligen@gmail.com>

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,13 +119,13 @@ 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 {
-		return err
-	} else if acc == nil {
-		// This account already exists
+		if errors.Is(err, internal.ErrSQLUserExists) { // This account already exists
 			return nil
 		}
+		return err
+	}
 
 	// Create a dummy device with a dummy token for the application service
 	_, err = deviceDB.CreateDevice(ctx, as.SenderLocalpart, nil, as.ASToken, &as.SenderLocalpart)

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
 	}

clientapi/routing/register.go

@@ -830,16 +830,17 @@ func completeRegistration(
 
 	acc, err := accountDB.CreateAccount(ctx, username, password, appserviceID)
 	if err != nil {
-		return util.JSONResponse{
-			Code: http.StatusInternalServerError,
-			JSON: jsonerror.Unknown("failed to create account: " + err.Error()),
-		}
-	} else if acc == 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()),
+		}
+	}
 
 	// Increment prometheus counter for created users
 	amtRegUsers.Inc()

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