diff --git a/clientapi/auth/login_publickey.go b/clientapi/auth/login_publickey.go index e999edeb7..8194df963 100644 --- a/clientapi/auth/login_publickey.go +++ b/clientapi/auth/login_publickey.go @@ -30,10 +30,10 @@ import ( type LoginPublicKeyHandler interface { AccountExists(ctx context.Context) (string, *jsonerror.MatrixError) - IsValidUserIdForRegistration(userId string) bool CreateLogin() *Login GetSession() string GetType() string + IsValidUserId(userId string) bool ValidateLoginResponse() (bool, *jsonerror.MatrixError) } diff --git a/clientapi/auth/login_publickey_ethereum.go b/clientapi/auth/login_publickey_ethereum.go index a3201a269..90de33d2b 100644 --- a/clientapi/auth/login_publickey_ethereum.go +++ b/clientapi/auth/login_publickey_ethereum.go @@ -73,6 +73,10 @@ func (pk LoginPublicKeyEthereum) AccountExists(ctx context.Context) (string, *js return "", jsonerror.Forbidden("the address is incorrect, or the account does not exist.") } + if !pk.IsValidUserId(localPart) { + return "", jsonerror.InvalidUsername("the username is not valid.") + } + res := userapi.QueryAccountAvailabilityResponse{} if err := pk.userAPI.QueryAccountAvailability(ctx, &userapi.QueryAccountAvailabilityRequest{ Localpart: localPart, @@ -80,7 +84,7 @@ func (pk LoginPublicKeyEthereum) AccountExists(ctx context.Context) (string, *js return "", jsonerror.Unknown("failed to check availability: " + err.Error()) } - if res.Available { + if localPart == "" || res.Available { return "", jsonerror.Forbidden("the address is incorrect, account does not exist") } @@ -89,7 +93,7 @@ func (pk LoginPublicKeyEthereum) AccountExists(ctx context.Context) (string, *js var validChainAgnosticIdRegex = regexp.MustCompile("^eip155=3a[0-9]+=3a0x[0-9a-fA-F]+$") -func (pk LoginPublicKeyEthereum) IsValidUserIdForRegistration(userId string) bool { +func (pk LoginPublicKeyEthereum) IsValidUserId(userId string) bool { // Verify that the user ID is a valid one according to spec. // https://github.com/ChainAgnostic/CAIPs/blob/master/CAIPs/caip-10.md @@ -100,9 +104,9 @@ func (pk LoginPublicKeyEthereum) IsValidUserIdForRegistration(userId string) boo isValid := validChainAgnosticIdRegex.MatchString(userId) - // In addition, double check that the user ID for registration + // In addition, double check that the user ID // matches the authentication data in the request. - return isValid && userId == pk.UserId + return isValid && strings.ToLower(userId) == pk.UserId } func (pk LoginPublicKeyEthereum) ValidateLoginResponse() (bool, *jsonerror.MatrixError) { diff --git a/clientapi/auth/user_interactive.go b/clientapi/auth/user_interactive.go index 9dad49a39..717e140f1 100644 --- a/clientapi/auth/user_interactive.go +++ b/clientapi/auth/user_interactive.go @@ -246,7 +246,7 @@ func (u *UserInteractive) ResponseWithChallenge(sessionID string, response inter // Verify returns an error/challenge response to send to the client, or nil if the user is authenticated. // `bodyBytes` is the HTTP request body which must contain an `auth` key. // Returns the login that was verified for additional checks if required. -func (u *UserInteractive) Verify(ctx context.Context, bodyBytes []byte, device *api.Device) (*Login, *util.JSONResponse) { +func (u *UserInteractive) Verify(ctx context.Context, bodyBytes []byte) (*Login, *util.JSONResponse) { // TODO: rate limit // "A client should first make a request with no auth parameter. The homeserver returns an HTTP 401 response, with a JSON body" diff --git a/clientapi/auth/user_interactive_test.go b/clientapi/auth/user_interactive_test.go index 3dbb9dabc..bc1239910 100644 --- a/clientapi/auth/user_interactive_test.go +++ b/clientapi/auth/user_interactive_test.go @@ -17,11 +17,6 @@ var ( serverName = gomatrixserverlib.ServerName("example.com") // space separated localpart+password -> account lookup = make(map[string]*api.Account) - device = &api.Device{ - AccessToken: "flibble", - DisplayName: "My Device", - ID: "device_id_goes_here", - } ) type fakeAccountDatabase struct { @@ -60,7 +55,7 @@ func setup() *UserInteractive { func TestUserInteractiveChallenge(t *testing.T) { uia := setup() // no auth key results in a challenge - _, errRes := uia.Verify(ctx, []byte(`{}`), device) + _, errRes := uia.Verify(ctx, []byte(`{}`)) if errRes == nil { t.Fatalf("Verify succeeded with {} but expected failure") } @@ -100,7 +95,7 @@ func TestUserInteractivePasswordLogin(t *testing.T) { }`), } for _, tc := range testCases { - _, errRes := uia.Verify(ctx, tc, device) + _, errRes := uia.Verify(ctx, tc) if errRes != nil { t.Errorf("Verify failed but expected success for request: %s - got %+v", string(tc), errRes) } @@ -181,7 +176,7 @@ func TestUserInteractivePasswordBadLogin(t *testing.T) { }, } for _, tc := range testCases { - _, errRes := uia.Verify(ctx, tc.body, device) + _, errRes := uia.Verify(ctx, tc.body) if errRes == nil { t.Errorf("Verify succeeded but expected failure for request: %s", string(tc.body)) continue diff --git a/clientapi/routing/deactivate.go b/clientapi/routing/deactivate.go index f213db7f3..9f80dff61 100644 --- a/clientapi/routing/deactivate.go +++ b/clientapi/routing/deactivate.go @@ -28,7 +28,7 @@ func Deactivate( } } - login, errRes := userInteractiveAuth.Verify(ctx, bodyBytes, deviceAPI) + login, errRes := userInteractiveAuth.Verify(ctx, bodyBytes) if errRes != nil { return *errRes } diff --git a/clientapi/routing/device.go b/clientapi/routing/device.go index e3a02661c..84e11bc7a 100644 --- a/clientapi/routing/device.go +++ b/clientapi/routing/device.go @@ -198,7 +198,7 @@ func DeleteDeviceById( sessionID = s } - login, errRes := userInteractiveAuth.Verify(ctx, bodyBytes, device) + login, errRes := userInteractiveAuth.Verify(ctx, bodyBytes) if errRes != nil { switch data := errRes.JSON.(type) { case auth.Challenge: diff --git a/clientapi/routing/register.go b/clientapi/routing/register.go index f71150da0..43242c78d 100644 --- a/clientapi/routing/register.go +++ b/clientapi/routing/register.go @@ -156,6 +156,13 @@ func (d *sessionsDict) startTimer(duration time.Duration, sessionID string) { }) } +func (d *sessionsDict) hasSession(sessionID string) bool { + d.RLock() + defer d.RUnlock() + _, ok := d.sessions[sessionID] + return ok +} + // addCompletedSessionStage records that a session has completed an auth stage // also starts a timer to delete the session once done. func (d *sessionsDict) addCompletedSessionStage(sessionID string, stage authtypes.LoginType) { diff --git a/clientapi/routing/register_publickey.go b/clientapi/routing/register_publickey.go index 258a47249..119065981 100644 --- a/clientapi/routing/register_publickey.go +++ b/clientapi/routing/register_publickey.go @@ -62,14 +62,14 @@ func handlePublicKeyRegistration( return false, authtypes.LoginStagePublicKeyNewRegistration, nil } - if _, ok := sessions.sessions[authHandler.GetSession()]; !ok { + if !sessions.hasSession(authHandler.GetSession()) { return false, "", &util.JSONResponse{ Code: http.StatusUnauthorized, JSON: jsonerror.Unknown("the session ID is missing or unknown."), } } - isValidUserId := authHandler.IsValidUserIdForRegistration(r.Username) + isValidUserId := authHandler.IsValidUserId(r.Username) if !isValidUserId { return false, "", &util.JSONResponse{ Code: http.StatusUnauthorized, diff --git a/setup/config/config_clientapi.go b/setup/config/config_clientapi.go index 7d8dc764b..ac7a5b04b 100644 --- a/setup/config/config_clientapi.go +++ b/setup/config/config_clientapi.go @@ -54,7 +54,7 @@ type ClientAPI struct { PasswordAuthenticationDisabled bool `yaml:"password_authentication_disabled"` // Public key authentication - PublicKeyAuthentication publicKeyAuthentication `yaml:"public_key_authentication"` + PublicKeyAuthentication PublicKeyAuthentication `yaml:"public_key_authentication"` } func (c *ClientAPI) Defaults(generate bool) { diff --git a/setup/config/config_publickey.go b/setup/config/config_publickey.go index 9820a5969..e214163e2 100644 --- a/setup/config/config_publickey.go +++ b/setup/config/config_publickey.go @@ -1,52 +1,40 @@ 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 { +type EthereumAuthConfig struct { Enabled bool `yaml:"enabled"` Version uint `yaml:"version"` ChainIDs []int `yaml:"chain_ids"` } -type publicKeyAuthentication struct { - Ethereum ethereumAuthConfig `yaml:"ethereum"` +type PublicKeyAuthentication struct { + Ethereum EthereumAuthConfig `yaml:"ethereum"` } -func (pk *publicKeyAuthentication) Enabled() bool { +func (pk *PublicKeyAuthentication) Enabled() bool { return pk.Ethereum.Enabled } -func (pk *publicKeyAuthentication) GetPublicKeyRegistrationFlows() []authtypes.Flow { +func (pk *PublicKeyAuthentication) GetPublicKeyRegistrationFlows() []authtypes.Flow { var flows []authtypes.Flow if pk.Ethereum.Enabled { flows = append(flows, authtypes.Flow{Stages: []authtypes.LoginType{authtypes.LoginTypePublicKeyEthereum}}) @@ -55,29 +43,15 @@ func (pk *publicKeyAuthentication) GetPublicKeyRegistrationFlows() []authtypes.F return flows } -func (pk *publicKeyAuthentication) GetPublicKeyRegistrationParams() map[string]interface{} { +func (pk *PublicKeyAuthentication) GetPublicKeyRegistrationParams() map[string]interface{} { params := make(map[string]interface{}) if pk.Ethereum.Enabled { 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) -} diff --git a/userapi/storage/tables/stats_table_test.go b/userapi/storage/tables/stats_table_test.go index a9a7e9e18..11521c8b0 100644 --- a/userapi/storage/tables/stats_table_test.go +++ b/userapi/storage/tables/stats_table_test.go @@ -300,10 +300,10 @@ func Test_UserStatistics(t *testing.T) { }, R30UsersV2: map[string]int64{ "ios": 0, - "android": 0, - "web": 0, + "android": 1, + "web": 1, "electron": 0, - "all": 0, + "all": 2, }, AllUsers: 6, NonBridgedUsers: 5,