Generate new devices for each new /login (#281)

5 files changed, 55 insertions, 25 deletions

diff --git a/src/github.com/matrix-org/dendrite/clientapi/auth/auth.go b/src/github.com/matrix-org/dendrite/clientapi/auth/auth.go
index 833cf544..b6a3efc2 100644
--- a/src/github.com/matrix-org/dendrite/clientapi/auth/auth.go
+++ b/src/github.com/matrix-org/dendrite/clientapi/auth/auth.go
@@ -29,17 +29,13 @@ import (
 	"github.com/matrix-org/util"
 )
 
-// UnknownDeviceID is the default device id if one is not specified.
-// This deviates from Synapse which generates a new device ID if one is not specified.
-// It's preferable to not amass a huge list of valid access tokens for an account,
-// so limiting it to 1 unknown device for now limits the number of valid tokens.
-// Clients should be giving us device IDs.
-var UnknownDeviceID = "unknown-device"
-
 // OWASP recommends at least 128 bits of entropy for tokens: https://www.owasp.org/index.php/Insufficient_Session-ID_Length
 // 32 bytes => 256 bits
 var tokenByteLength = 32
 
+// The length of generated device IDs
+var deviceIDByteLength = 6
+
 // DeviceDatabase represents a device database.
 type DeviceDatabase interface {
 	// Look up the device matching the given access token.
@@ -62,8 +58,8 @@ func VerifyAccessToken(req *http.Request, deviceDB DeviceDatabase) (device *auth
 	if err != nil {
 		if err == sql.ErrNoRows {
 			resErr = &util.JSONResponse{
-				Code: 403,
-				JSON: jsonerror.Forbidden("Invalid access token"),
+				Code: 401,
+				JSON: jsonerror.UnknownToken("Unknown token"),
 			}
 		} else {
 			resErr = &util.JSONResponse{
@@ -86,6 +82,18 @@ func GenerateAccessToken() (string, error) {
 	return base64.RawURLEncoding.EncodeToString(b), nil
 }
 
+// GenerateDeviceID creates a new device id. Returns an error if failed to generate
+// random bytes.
+func GenerateDeviceID() (string, error) {
+	b := make([]byte, deviceIDByteLength)
+	_, err := rand.Read(b)
+	if err != nil {
+		return "", err
+	}
+	// url-safe no padding
+	return base64.RawURLEncoding.EncodeToString(b), nil
+}
+
 // extractAccessToken from a request, or return an error detailing what went wrong. The
 // error message MUST be human-readable and comprehensible to the client.
 func extractAccessToken(req *http.Request) (string, error) {
diff --git a/src/github.com/matrix-org/dendrite/clientapi/auth/storage/devices/storage.go b/src/github.com/matrix-org/dendrite/clientapi/auth/storage/devices/storage.go
index df8bf4f3..ea7d8738 100644
--- a/src/github.com/matrix-org/dendrite/clientapi/auth/storage/devices/storage.go
+++ b/src/github.com/matrix-org/dendrite/clientapi/auth/storage/devices/storage.go
@@ -18,6 +18,7 @@ import (
 	"context"
 	"database/sql"
 
+	"github.com/matrix-org/dendrite/clientapi/auth"
 	"github.com/matrix-org/dendrite/clientapi/auth/authtypes"
 	"github.com/matrix-org/dendrite/common"
 	"github.com/matrix-org/gomatrixserverlib"
@@ -55,20 +56,42 @@ func (d *Database) GetDeviceByAccessToken(
 // If there is already a device with the same device ID for this user, that access token will be revoked
 // and replaced with the given accessToken. If the given accessToken is already in use for another device,
 // an error will be returned.
+// If no device ID is given one is generated.
 // Returns the device on success.
 func (d *Database) CreateDevice(
-	ctx context.Context, localpart, deviceID, accessToken string,
+	ctx context.Context, localpart string, deviceID *string, accessToken string,
 ) (dev *authtypes.Device, returnErr error) {
-	returnErr = common.WithTransaction(d.db, func(txn *sql.Tx) error {
-		var err error
-		// Revoke existing token for this device
-		if err = d.devices.deleteDevice(ctx, txn, deviceID, localpart); err != nil {
+	if deviceID != nil {
+		returnErr = common.WithTransaction(d.db, func(txn *sql.Tx) error {
+			var err error
+			// Revoke existing token for this device
+			if err = d.devices.deleteDevice(ctx, txn, *deviceID, localpart); err != nil {
+				return err
+			}
+
+			dev, err = d.devices.insertDevice(ctx, txn, *deviceID, localpart, accessToken)
 			return err
-		}
+		})
+	} else {
+		// We generate device IDs in a loop in case its already taken.
+		// We cap this at going round 5 times to ensure we don't spin forever
+		var newDeviceID string
+		for i := 1; i <= 5; i++ {
+			newDeviceID, returnErr = auth.GenerateDeviceID()
+			if returnErr != nil {
+				return
+			}
 
-		dev, err = d.devices.insertDevice(ctx, txn, deviceID, localpart, accessToken)
-		return err
-	})
+			returnErr = common.WithTransaction(d.db, func(txn *sql.Tx) error {
+				var err error
+				dev, err = d.devices.insertDevice(ctx, txn, newDeviceID, localpart, accessToken)
+				return err
+			})
+			if returnErr == nil {
+				return
+			}
+		}
+	}
 	return
 }
 
diff --git a/src/github.com/matrix-org/dendrite/clientapi/readers/login.go b/src/github.com/matrix-org/dendrite/clientapi/readers/login.go
index de6ecf5c..ddbac12c 100644
--- a/src/github.com/matrix-org/dendrite/clientapi/readers/login.go
+++ b/src/github.com/matrix-org/dendrite/clientapi/readers/login.go
@@ -46,6 +46,7 @@ type loginResponse struct {
 	UserID      string                       `json:"user_id"`
 	AccessToken string                       `json:"access_token"`
 	HomeServer  gomatrixserverlib.ServerName `json:"home_server"`
+	DeviceID    string                       `json:"device_id"`
 }
 
 func passwordLogin() loginFlows {
@@ -113,15 +114,12 @@ func Login(
 
 		token, err := auth.GenerateAccessToken()
 		if err != nil {
-			return util.JSONResponse{
-				Code: 500,
-				JSON: jsonerror.Unknown("Failed to generate access token"),
-			}
+			httputil.LogThenError(req, err)
 		}
 
 		// TODO: Use the device ID in the request
 		dev, err := deviceDB.CreateDevice(
-			req.Context(), acc.Localpart, auth.UnknownDeviceID, token,
+			req.Context(), acc.Localpart, nil, token,
 		)
 		if err != nil {
 			return util.JSONResponse{
@@ -136,6 +134,7 @@ func Login(
 				UserID:      dev.UserID,
 				AccessToken: dev.AccessToken,
 				HomeServer:  cfg.Matrix.ServerName,
+				DeviceID:    dev.ID,
 			},
 		}
 	}
diff --git a/src/github.com/matrix-org/dendrite/clientapi/writers/register.go b/src/github.com/matrix-org/dendrite/clientapi/writers/register.go
index 84227d15..a405f5ef 100644
--- a/src/github.com/matrix-org/dendrite/clientapi/writers/register.go
+++ b/src/github.com/matrix-org/dendrite/clientapi/writers/register.go
@@ -303,7 +303,7 @@ func completeRegistration(
 	}
 
 	// // TODO: Use the device ID in the request.
-	dev, err := deviceDB.CreateDevice(ctx, username, auth.UnknownDeviceID, token)
+	dev, err := deviceDB.CreateDevice(ctx, username, nil, token)
 	if err != nil {
 		return util.JSONResponse{
 			Code: 500,
diff --git a/src/github.com/matrix-org/dendrite/cmd/create-account/main.go b/src/github.com/matrix-org/dendrite/cmd/create-account/main.go
index d031afc2..3d5c3587 100644
--- a/src/github.com/matrix-org/dendrite/cmd/create-account/main.go
+++ b/src/github.com/matrix-org/dendrite/cmd/create-account/main.go
@@ -87,7 +87,7 @@ func main() {
 	}
 
 	device, err := deviceDB.CreateDevice(
-		context.Background(), *username, "create-account-script", *accessToken,
+		context.Background(), *username, nil, *accessToken,
 	)
 	if err != nil {
 		fmt.Println(err.Error())