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 833cf5446227c11e0fbd594d37d38d9e32391aad..b6a3efc285e03129031d7ca4da1aaf6da190b492 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 df8bf4f3fe49b031b3906a48ade3705872b21694..ea7d87383de6fa1846306f056cb552700b7112e4 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 de6ecf5c65c7ba978386fa9507cbb7f00f566e8b..ddbac12ceca411a85f3efaca7c014d777cf736c1 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 84227d155a4ed78f18a2e83c5a8cfad5f72ebf34..a405f5ef5d616dc69f5525804309439ace895916 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 d031afc26e69defe2a291927ad9fff335f2f7154..3d5c35878a0129884354ecae8c261419fbdf1a35 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())