Tak/pull-dendrite-changes (#245)

* Fix issues with migrations not getting executed (#2628)

* Fix issues with migrations not getting executed

* Check actual postgres error

* Return error if it's not "column does not exist"

* Remove nonce generation for eip4361 signin (#25)

Co-authored-by: Tak Wai Wong <tak@hntlabs.com>

clientapi/auth/login_publickey_test.go

@@ -61,7 +61,6 @@ func TestLoginPublicKeyNewSession(t *testing.T) {
 		"err.Code actual: %v, expected: %v", err.Code, http.StatusUnauthorized)
 	challenge := err.JSON.(Challenge)
 	assert.NotEmptyf(challenge.Session, "challenge.Session")
-	assert.NotEmptyf(challenge.Completed, "challenge.Completed")
 	assert.Truef(
 		authtypes.LoginTypePublicKeyEthereum == challenge.Flows[0].Stages[0],
 		"challenge.Flows[0].Stages[0] actual: %v, expected: %v", challenge.Flows[0].Stages[0], authtypes.LoginTypePublicKeyEthereum)
@@ -74,7 +73,6 @@ func TestLoginPublicKeyNewSession(t *testing.T) {
 		"[object]")
 	ethParams := params.(config.EthereumAuthParams)
 	assert.NotEmptyf(ethParams.ChainIDs, "ChainIDs actual: empty, expected not empty")
-	assert.NotEmptyf(ethParams.Nonce, "Nonce actual: \"\", expected: not empty")
 	assert.NotEmptyf(ethParams.Version, "Version actual: \"\", expected: not empty")
 }
 

clientapi/auth/user_interactive.go

@@ -178,12 +178,6 @@ func (u *UserInteractive) Challenge(sessionID string) *util.JSONResponse {
 			// If an auth flow has params,
 			// send it as part of the challenge.
 			paramsCopy[key] = p
-
-			// If an auth flow generated a nonce, add it to the session.
-			nonce := getAuthParamNonce(p)
-			if nonce != "" {
-				u.Sessions[sessionID] = append(u.Sessions[sessionID], nonce)
-			}
 		}
 	}
 
@@ -288,11 +282,3 @@ func GetAuthParams(params interface{}) interface{} {
 	}
 	return nil
 }
-
-func getAuthParamNonce(p interface{}) string {
-	v, ok := p.(config.AuthParams)
-	if ok {
-		return v.GetNonce()
-	}
-	return ""
-}

clientapi/routing/register_publickey_test.go

@@ -340,7 +340,6 @@ func TestNewRegistrationSession(t *testing.T) {
 		"[object]")
 	ethParams := params.(config.EthereumAuthParams)
 	assert.NotEmptyf(ethParams.ChainIDs, "ChainIDs actual: empty, expected not empty")
-	assert.NotEmptyf(ethParams.Nonce, "Nonce actual: \"\", expected: not empty")
 	assert.NotEmptyf(ethParams.Version, "Version actual: \"\", expected: not empty")
 }
 

keyserver/storage/postgres/key_changes_table.go

@@ -18,6 +18,8 @@ import (
 	"context"
 	"database/sql"
 
+	"github.com/lib/pq"
+
 	"github.com/matrix-org/dendrite/internal"
 	"github.com/matrix-org/dendrite/internal/sqlutil"
 	"github.com/matrix-org/dendrite/keyserver/storage/postgres/deltas"
@@ -64,7 +66,8 @@ func NewPostgresKeyChangesTable(db *sql.DB) (tables.KeyChanges, error) {
 	// TODO: Remove when we are sure we are not having goose artefacts in the db
 	// This forces an error, which indicates the migration is already applied, since the
 	// column partition was removed from the table
-	err = db.QueryRow("SELECT partition FROM keyserver_key_changes LIMIT 1;").Scan()
+	var count int
+	err = db.QueryRow("SELECT partition FROM keyserver_key_changes LIMIT 1;").Scan(&count)
 	if err == nil {
 		m := sqlutil.NewMigrator(db)
 		m.AddMigrations(sqlutil.Migration{
@@ -72,6 +75,16 @@ func NewPostgresKeyChangesTable(db *sql.DB) (tables.KeyChanges, error) {
 			Up:      deltas.UpRefactorKeyChanges,
 		})
 		return s, m.Up(context.Background())
+	} else {
+		switch e := err.(type) {
+		case *pq.Error:
+			// ignore undefined_column (42703) errors, as this is expected at this point
+			if e.Code != "42703" {
+				return nil, err
+			}
+		default:
+			return nil, err
+		}
 	}
 	return s, nil
 }

keyserver/storage/sqlite3/key_changes_table.go

@@ -61,7 +61,8 @@ func NewSqliteKeyChangesTable(db *sql.DB) (tables.KeyChanges, error) {
 	// TODO: Remove when we are sure we are not having goose artefacts in the db
 	// This forces an error, which indicates the migration is already applied, since the
 	// column partition was removed from the table
-	err = db.QueryRow("SELECT partition FROM keyserver_key_changes LIMIT 1;").Scan()
+	var count int
+	err = db.QueryRow("SELECT partition FROM keyserver_key_changes LIMIT 1;").Scan(&count)
 	if err == nil {
 		m := sqlutil.NewMigrator(db)
 		m.AddMigrations(sqlutil.Migration{

roomserver/storage/postgres/storage.go

@@ -19,8 +19,10 @@ import (
 	"database/sql"
 	"fmt"
 
+	"github.com/lib/pq"
 	// Import the postgres database driver.
 	_ "github.com/lib/pq"
+
 	"github.com/matrix-org/dendrite/internal/caching"
 	"github.com/matrix-org/dendrite/internal/sqlutil"
 	"github.com/matrix-org/dendrite/roomserver/storage/postgres/deltas"
@@ -53,21 +55,32 @@ func Open(base *base.BaseDendrite, dbProperties *config.DatabaseOptions, cache c
 	// TODO: Remove when we are sure we are not having goose artefacts in the db
 	// This forces an error, which indicates the migration is already applied, since the
 	// column event_nid was removed from the table
-	err = db.QueryRow("SELECT event_nid FROM roomserver_state_block LIMIT 1;").Scan()
+	var eventNID int
+	err = db.QueryRow("SELECT event_nid FROM roomserver_state_block LIMIT 1;").Scan(&eventNID)
 	if err == nil {
 		m := sqlutil.NewMigrator(db)
 		m.AddMigrations(sqlutil.Migration{
 			Version: "roomserver: state blocks refactor",
 			Up:      deltas.UpStateBlocksRefactor,
 		})
-		if err := m.Up(base.Context()); err != nil {
+		if err = m.Up(base.Context()); err != nil {
+			return nil, err
+		}
+	} else {
+		switch e := err.(type) {
+		case *pq.Error:
+			// ignore undefined_column (42703) errors, as this is expected at this point
+			if e.Code != "42703" {
+				return nil, err
+			}
+		default:
 			return nil, err
 		}
 	}
 
 	// Then prepare the statements. Now that the migrations have run, any columns referred
 	// to in the database code should now exist.
-	if err := d.prepare(db, writer, cache); err != nil {
+	if err = d.prepare(db, writer, cache); err != nil {
 		return nil, err
 	}
 

roomserver/storage/sqlite3/storage.go

@@ -20,6 +20,8 @@ import (
 	"database/sql"
 	"fmt"
 
+	"github.com/matrix-org/gomatrixserverlib"
+
 	"github.com/matrix-org/dendrite/internal/caching"
 	"github.com/matrix-org/dendrite/internal/sqlutil"
 	"github.com/matrix-org/dendrite/roomserver/storage/shared"
@@ -27,7 +29,6 @@ import (
 	"github.com/matrix-org/dendrite/roomserver/types"
 	"github.com/matrix-org/dendrite/setup/base"
 	"github.com/matrix-org/dendrite/setup/config"
-	"github.com/matrix-org/gomatrixserverlib"
 )
 
 // A Database is used to store room events and stream offsets.
@@ -63,21 +64,22 @@ func Open(base *base.BaseDendrite, dbProperties *config.DatabaseOptions, cache c
 	// TODO: Remove when we are sure we are not having goose artefacts in the db
 	// This forces an error, which indicates the migration is already applied, since the
 	// column event_nid was removed from the table
-	err = db.QueryRow("SELECT event_nid FROM roomserver_state_block LIMIT 1;").Scan()
+	var eventNID int
+	err = db.QueryRow("SELECT event_nid FROM roomserver_state_block LIMIT 1;").Scan(&eventNID)
 	if err == nil {
 		m := sqlutil.NewMigrator(db)
 		m.AddMigrations(sqlutil.Migration{
 			Version: "roomserver: state blocks refactor",
 			Up:      deltas.UpStateBlocksRefactor,
 		})
-		if err := m.Up(base.Context()); err != nil {
+		if err = m.Up(base.Context()); err != nil {
 			return nil, err
 		}
 	}
 
 	// Then prepare the statements. Now that the migrations have run, any columns referred
 	// to in the database code should now exist.
-	if err := d.prepare(db, writer, cache); err != nil {
+	if err = d.prepare(db, writer, cache); err != nil {
 		return nil, err
 	}
 

setup/config/config_publickey.go

@@ -1,37 +1,25 @@
 package config
 
 import (
-	"math/rand"
-	"time"
-
 	"github.com/matrix-org/dendrite/clientapi/auth/authtypes"
 )
 
-var nonceLength = 32
-
 type AuthParams interface {
 	GetParams() interface{}
-	GetNonce() string
 }
 
 type EthereumAuthParams struct {
-	Version  uint   `json:"version"`
+	Version  uint  `json:"version"`
-	ChainIDs []int  `json:"chain_ids"`
+	ChainIDs []int `json:"chain_ids"`
-	Nonce    string `json:"nonce"`
 }
 
 func (p EthereumAuthParams) GetParams() interface{} {
 	copyP := p
 	copyP.ChainIDs = make([]int, len(p.ChainIDs))
 	copy(copyP.ChainIDs, p.ChainIDs)
-	copyP.Nonce = newNonce(nonceLength)
 	return copyP
 }
 
-func (p EthereumAuthParams) GetNonce() string {
-	return p.Nonce
-}
-
 type EthereumAuthConfig struct {
 	Enabled  bool  `yaml:"enabled"`
 	Version  uint  `yaml:"version"`
@@ -61,23 +49,9 @@ func (pk *PublicKeyAuthentication) GetPublicKeyRegistrationParams() map[string]i
 		p := EthereumAuthParams{
 			Version:  pk.Ethereum.Version,
 			ChainIDs: pk.Ethereum.ChainIDs,
-			Nonce:    "",
 		}
 		params[authtypes.LoginTypePublicKeyEthereum] = p
 	}
 
 	return params
 }
-
-const lettersAndNumbers = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
-
-func newNonce(n int) string {
-	nonce := make([]byte, n)
-	rand.Seed(time.Now().UnixNano())
-
-	for i := range nonce {
-		nonce[i] = lettersAndNumbers[rand.Int63()%int64(len(lettersAndNumbers))]
-	}
-
-	return string(nonce)
-}