From 272b4d92175d919abf8294d8c31cc4e116e7ac34 Mon Sep 17 00:00:00 2001 From: Tak Wai Wong <64229756+tak-hntlabs@users.noreply.github.com> Date: Tue, 9 Aug 2022 15:23:56 -0700 Subject: [PATCH] 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 --- clientapi/auth/login_publickey_test.go | 2 -- clientapi/auth/user_interactive.go | 14 --------- clientapi/routing/register_publickey_test.go | 1 - .../storage/postgres/key_changes_table.go | 15 +++++++++- .../storage/sqlite3/key_changes_table.go | 3 +- roomserver/storage/postgres/storage.go | 19 ++++++++++-- roomserver/storage/sqlite3/storage.go | 10 ++++--- setup/config/config_publickey.go | 30 ++----------------- 8 files changed, 40 insertions(+), 54 deletions(-) diff --git a/clientapi/auth/login_publickey_test.go b/clientapi/auth/login_publickey_test.go index 321d8eb6c..6b95c5553 100644 --- a/clientapi/auth/login_publickey_test.go +++ b/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") } diff --git a/clientapi/auth/user_interactive.go b/clientapi/auth/user_interactive.go index ccf991f86..a6ee47454 100644 --- a/clientapi/auth/user_interactive.go +++ b/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 "" -} diff --git a/clientapi/routing/register_publickey_test.go b/clientapi/routing/register_publickey_test.go index c51ee1846..4079669b9 100644 --- a/clientapi/routing/register_publickey_test.go +++ b/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") } diff --git a/keyserver/storage/postgres/key_changes_table.go b/keyserver/storage/postgres/key_changes_table.go index 6894d7b7c..004f15d82 100644 --- a/keyserver/storage/postgres/key_changes_table.go +++ b/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 } diff --git a/keyserver/storage/sqlite3/key_changes_table.go b/keyserver/storage/sqlite3/key_changes_table.go index 1b27c3d05..217fa7a5d 100644 --- a/keyserver/storage/sqlite3/key_changes_table.go +++ b/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{ diff --git a/roomserver/storage/postgres/storage.go b/roomserver/storage/postgres/storage.go index 4c271ea9b..f47a64c80 100644 --- a/roomserver/storage/postgres/storage.go +++ b/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 } diff --git a/roomserver/storage/sqlite3/storage.go b/roomserver/storage/sqlite3/storage.go index bb9c15b5a..9f8a1b118 100644 --- a/roomserver/storage/sqlite3/storage.go +++ b/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 } diff --git a/setup/config/config_publickey.go b/setup/config/config_publickey.go index ae19d16f6..e214163e2 100644 --- a/setup/config/config_publickey.go +++ b/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"` - ChainIDs []int `json:"chain_ids"` - Nonce string `json:"nonce"` + Version uint `json:"version"` + ChainIDs []int `json:"chain_ids"` } 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) -}