Add User-Interactive Authentication (#1193)

* Add User-Interactive Authentication

And use it when deleting a device. With tests.

* Make remaining sytest pass

* Linting

* 403 not 401 on wrong user/pass

clientapi/auth/password.go

@@ -0,0 +1,75 @@
+// Copyright 2020 The Matrix.org Foundation C.I.C.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package auth
+
+import (
+	"context"
+	"net/http"
+
+	"github.com/matrix-org/dendrite/clientapi/jsonerror"
+	"github.com/matrix-org/dendrite/clientapi/userutil"
+	"github.com/matrix-org/dendrite/internal/config"
+	"github.com/matrix-org/dendrite/userapi/api"
+	"github.com/matrix-org/util"
+)
+
+type GetAccountByPassword func(ctx context.Context, localpart, password string) (*api.Account, error)
+
+type PasswordRequest struct {
+	Login
+	Password string `json:"password"`
+}
+
+// LoginTypePassword implements https://matrix.org/docs/spec/client_server/r0.6.1#password-based
+type LoginTypePassword struct {
+	GetAccountByPassword GetAccountByPassword
+	Config               *config.Dendrite
+}
+
+func (t *LoginTypePassword) Name() string {
+	return "m.login.password"
+}
+
+func (t *LoginTypePassword) Request() interface{} {
+	return &PasswordRequest{}
+}
+
+func (t *LoginTypePassword) Login(ctx context.Context, req interface{}) (*Login, *util.JSONResponse) {
+	r := req.(*PasswordRequest)
+	username := r.Username()
+	if username == "" {
+		return nil, &util.JSONResponse{
+			Code: http.StatusUnauthorized,
+			JSON: jsonerror.BadJSON("'user' must be supplied."),
+		}
+	}
+	localpart, err := userutil.ParseUsernameParam(username, &t.Config.Matrix.ServerName)
+	if err != nil {
+		return nil, &util.JSONResponse{
+			Code: http.StatusUnauthorized,
+			JSON: jsonerror.InvalidUsername(err.Error()),
+		}
+	}
+	_, err = t.GetAccountByPassword(ctx, localpart, r.Password)
+	if err != nil {
+		// Technically we could tell them if the user does not exist by checking if err == sql.ErrNoRows
+		// but that would leak the existence of the user.
+		return nil, &util.JSONResponse{
+			Code: http.StatusForbidden,
+			JSON: jsonerror.Forbidden("username or password was incorrect, or the account does not exist"),
+		}
+	}
+	return &r.Login, nil
+}

clientapi/auth/user_interactive.go

@@ -0,0 +1,248 @@
+// Copyright 2020 The Matrix.org Foundation C.I.C.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package auth
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+
+	"github.com/matrix-org/dendrite/clientapi/jsonerror"
+	"github.com/matrix-org/dendrite/internal/config"
+	"github.com/matrix-org/dendrite/userapi/api"
+	"github.com/matrix-org/util"
+	"github.com/sirupsen/logrus"
+	"github.com/tidwall/gjson"
+)
+
+// Type represents an auth type
+// https://matrix.org/docs/spec/client_server/r0.6.1#authentication-types
+type Type interface {
+	// Name returns the name of the auth type e.g `m.login.password`
+	Name() string
+	// Request returns a pointer to a new request body struct to unmarshal into.
+	Request() interface{}
+	// Login with the auth type, returning an error response on failure.
+	// Not all types support login, only m.login.password and m.login.token
+	// See https://matrix.org/docs/spec/client_server/r0.6.1#post-matrix-client-r0-login
+	// `req` is guaranteed to be the type returned from Request()
+	// This function will be called when doing login and when doing 'sudo' style
+	// actions e.g deleting devices. The response must be a 401 as per:
+	// "If the homeserver decides that an attempt on a stage was unsuccessful, but the
+	// client may make a second attempt, it returns the same HTTP status 401 response as above,
+	// with the addition of the standard errcode and error fields describing the error."
+	Login(ctx context.Context, req interface{}) (login *Login, errRes *util.JSONResponse)
+	// TODO: Extend to support Register() flow
+	// Register(ctx context.Context, sessionID string, req interface{})
+}
+
+// LoginIdentifier represents identifier types
+// https://matrix.org/docs/spec/client_server/r0.6.1#identifier-types
+type LoginIdentifier struct {
+	Type string `json:"type"`
+	// when type = m.id.user
+	User string `json:"user"`
+	// when type = m.id.thirdparty
+	Medium  string `json:"medium"`
+	Address string `json:"address"`
+}
+
+// Login represents the shared fields used in all forms of login/sudo endpoints.
+type Login struct {
+	Type       string          `json:"type"`
+	Identifier LoginIdentifier `json:"identifier"`
+	User       string          `json:"user"`    // deprecated in favour of identifier
+	Medium     string          `json:"medium"`  // deprecated in favour of identifier
+	Address    string          `json:"address"` // deprecated in favour of identifier
+
+	// Both DeviceID and InitialDisplayName can be omitted, or empty strings ("")
+	// Thus a pointer is needed to differentiate between the two
+	InitialDisplayName *string `json:"initial_device_display_name"`
+	DeviceID           *string `json:"device_id"`
+}
+
+// Username returns the user localpart/user_id in this request, if it exists.
+func (r *Login) Username() string {
+	if r.Identifier.Type == "m.id.user" {
+		return r.Identifier.User
+	}
+	// deprecated but without it Riot iOS won't log in
+	return r.User
+}
+
+// ThirdPartyID returns the 3PID medium and address for this login, if it exists.
+func (r *Login) ThirdPartyID() (medium, address string) {
+	if r.Identifier.Type == "m.id.thirdparty" {
+		return r.Identifier.Medium, r.Identifier.Address
+	}
+	// deprecated
+	if r.Medium == "email" {
+		return "email", r.Address
+	}
+	return "", ""
+}
+
+type userInteractiveFlow struct {
+	Stages []string `json:"stages"`
+}
+
+// UserInteractive checks that the user is who they claim to be, via a UI auth.
+// This is used for things like device deletion and password reset where
+// the user already has a valid access token, but we want to double-check
+// that it isn't stolen by re-authenticating them.
+type UserInteractive struct {
+	Flows []userInteractiveFlow
+	// Map of login type to implementation
+	Types map[string]Type
+	// Map of session ID to completed login types, will need to be extended in future
+	Sessions map[string][]string
+}
+
+func NewUserInteractive(getAccByPass GetAccountByPassword, cfg *config.Dendrite) *UserInteractive {
+	typePassword := &LoginTypePassword{
+		GetAccountByPassword: getAccByPass,
+		Config:               cfg,
+	}
+	// TODO: Add SSO login
+	return &UserInteractive{
+		Flows: []userInteractiveFlow{
+			{
+				Stages: []string{typePassword.Name()},
+			},
+		},
+		Types: map[string]Type{
+			typePassword.Name(): typePassword,
+		},
+		Sessions: make(map[string][]string),
+	}
+}
+
+func (u *UserInteractive) IsSingleStageFlow(authType string) bool {
+	for _, f := range u.Flows {
+		if len(f.Stages) == 1 && f.Stages[0] == authType {
+			return true
+		}
+	}
+	return false
+}
+
+func (u *UserInteractive) AddCompletedStage(sessionID, authType string) {
+	// TODO: Handle multi-stage flows
+	delete(u.Sessions, sessionID)
+}
+
+// Challenge returns an HTTP 401 with the supported flows for authenticating
+func (u *UserInteractive) Challenge(sessionID string) *util.JSONResponse {
+	return &util.JSONResponse{
+		Code: 401,
+		JSON: struct {
+			Flows   []userInteractiveFlow `json:"flows"`
+			Session string                `json:"session"`
+			// TODO: Return any additional `params`
+			Params map[string]interface{} `json:"params"`
+		}{
+			u.Flows,
+			sessionID,
+			make(map[string]interface{}),
+		},
+	}
+}
+
+// NewSession returns a challenge with a new session ID and remembers the session ID
+func (u *UserInteractive) NewSession() *util.JSONResponse {
+	sessionID, err := GenerateAccessToken()
+	if err != nil {
+		logrus.WithError(err).Error("failed to generate session ID")
+		res := jsonerror.InternalServerError()
+		return &res
+	}
+	u.Sessions[sessionID] = []string{}
+	return u.Challenge(sessionID)
+}
+
+// ResponseWithChallenge mixes together a JSON body (e.g an error with errcode/message) with the
+// standard challenge response.
+func (u *UserInteractive) ResponseWithChallenge(sessionID string, response interface{}) *util.JSONResponse {
+	mixedObjects := make(map[string]interface{})
+	b, err := json.Marshal(response)
+	if err != nil {
+		ise := jsonerror.InternalServerError()
+		return &ise
+	}
+	_ = json.Unmarshal(b, &mixedObjects)
+	challenge := u.Challenge(sessionID)
+	b, err = json.Marshal(challenge.JSON)
+	if err != nil {
+		ise := jsonerror.InternalServerError()
+		return &ise
+	}
+	_ = json.Unmarshal(b, &mixedObjects)
+
+	return &util.JSONResponse{
+		Code: 401,
+		JSON: mixedObjects,
+	}
+}
+
+// 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) {
+	// 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"
+	// https://matrix.org/docs/spec/client_server/r0.6.1#user-interactive-api-in-the-rest-api
+	hasResponse := gjson.GetBytes(bodyBytes, "auth").Exists()
+	if !hasResponse {
+		return nil, u.NewSession()
+	}
+
+	// extract the type so we know which login type to use
+	authType := gjson.GetBytes(bodyBytes, "auth.type").Str
+	loginType, ok := u.Types[authType]
+	if !ok {
+		return nil, &util.JSONResponse{
+			Code: http.StatusBadRequest,
+			JSON: jsonerror.BadJSON("unknown auth.type: " + authType),
+		}
+	}
+
+	// retrieve the session
+	sessionID := gjson.GetBytes(bodyBytes, "auth.session").Str
+	if _, ok = u.Sessions[sessionID]; !ok {
+		// if the login type is part of a single stage flow then allow them to omit the session ID
+		if !u.IsSingleStageFlow(authType) {
+			return nil, &util.JSONResponse{
+				Code: http.StatusBadRequest,
+				JSON: jsonerror.Unknown("missing or unknown auth.session"),
+			}
+		}
+	}
+
+	r := loginType.Request()
+	if err := json.Unmarshal([]byte(gjson.GetBytes(bodyBytes, "auth").Raw), r); err != nil {
+		return nil, &util.JSONResponse{
+			Code: http.StatusBadRequest,
+			JSON: jsonerror.BadJSON("The request body could not be decoded into valid JSON. " + err.Error()),
+		}
+	}
+	login, resErr := loginType.Login(ctx, r)
+	if resErr == nil {
+		u.AddCompletedStage(sessionID, authType)
+		// TODO: Check if there's more stages to go and return an error
+		return login, nil
+	}
+	return nil, u.ResponseWithChallenge(sessionID, resErr.JSON)
+}

clientapi/auth/user_interactive_test.go

@@ -0,0 +1,174 @@
+package auth
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"testing"
+
+	"github.com/matrix-org/dendrite/internal/config"
+	"github.com/matrix-org/dendrite/userapi/api"
+	"github.com/matrix-org/gomatrixserverlib"
+	"github.com/matrix-org/util"
+)
+
+var (
+	ctx        = context.Background()
+	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",
+	}
+)
+
+func getAccountByPassword(ctx context.Context, localpart, plaintextPassword string) (*api.Account, error) {
+	acc, ok := lookup[localpart+" "+plaintextPassword]
+	if !ok {
+		return nil, fmt.Errorf("unknown user/password")
+	}
+	return acc, nil
+}
+
+func setup() *UserInteractive {
+	cfg := &config.Dendrite{}
+	cfg.Matrix.ServerName = serverName
+	return NewUserInteractive(getAccountByPassword, cfg)
+}
+
+func TestUserInteractiveChallenge(t *testing.T) {
+	uia := setup()
+	// no auth key results in a challenge
+	_, errRes := uia.Verify(ctx, []byte(`{}`), device)
+	if errRes == nil {
+		t.Fatalf("Verify succeeded with {} but expected failure")
+	}
+	if errRes.Code != 401 {
+		t.Errorf("Expected HTTP 401, got %d", errRes.Code)
+	}
+}
+
+func TestUserInteractivePasswordLogin(t *testing.T) {
+	uia := setup()
+	// valid password login succeeds when an account exists
+	lookup["alice herpassword"] = &api.Account{
+		Localpart:  "alice",
+		ServerName: serverName,
+		UserID:     fmt.Sprintf("@alice:%s", serverName),
+	}
+	// valid password requests
+	testCases := []json.RawMessage{
+		// deprecated form
+		[]byte(`{
+			"auth": {
+				"type": "m.login.password",
+				"user": "alice",
+				"password": "herpassword"
+			}
+		}`),
+		// new form
+		[]byte(`{
+			"auth": {
+				"type": "m.login.password",
+				"identifier": {
+					"type": "m.id.user",
+					"user": "alice"
+				},
+				"password": "herpassword"
+			}
+		}`),
+	}
+	for _, tc := range testCases {
+		_, errRes := uia.Verify(ctx, tc, device)
+		if errRes != nil {
+			t.Errorf("Verify failed but expected success for request: %s - got %+v", string(tc), errRes)
+		}
+	}
+}
+
+func TestUserInteractivePasswordBadLogin(t *testing.T) {
+	uia := setup()
+	// password login fails when an account exists but is specced wrong
+	lookup["bob hispassword"] = &api.Account{
+		Localpart:  "bob",
+		ServerName: serverName,
+		UserID:     fmt.Sprintf("@bob:%s", serverName),
+	}
+	// invalid password requests
+	testCases := []struct {
+		body    json.RawMessage
+		wantRes util.JSONResponse
+	}{
+		{
+			// fields not in an auth dict
+			body: []byte(`{
+				"type": "m.login.password",
+				"user": "bob",
+				"password": "hispassword"
+			}`),
+			wantRes: util.JSONResponse{
+				Code: 401,
+			},
+		},
+		{
+			// wrong type
+			body: []byte(`{
+				"auth": {
+					"type": "m.login.not_password",
+					"identifier": {
+						"type": "m.id.user",
+						"user": "bob"
+					},
+					"password": "hispassword"
+				}
+			}`),
+			wantRes: util.JSONResponse{
+				Code: 400,
+			},
+		},
+		{
+			// identifier type is wrong
+			body: []byte(`{
+				"auth": {
+					"type": "m.login.password",
+					"identifier": {
+						"type": "m.id.thirdparty",
+						"user": "bob"
+					},
+					"password": "hispassword"
+				}
+			}`),
+			wantRes: util.JSONResponse{
+				Code: 401,
+			},
+		},
+		{
+			// wrong password
+			body: []byte(`{
+				"auth": {
+					"type": "m.login.password",
+					"identifier": {
+						"type": "m.id.user",
+						"user": "bob"
+					},
+					"password": "not_his_password"
+				}
+			}`),
+			wantRes: util.JSONResponse{
+				Code: 401,
+			},
+		},
+	}
+	for _, tc := range testCases {
+		_, errRes := uia.Verify(ctx, tc.body, device)
+		if errRes == nil {
+			t.Errorf("Verify succeeded but expected failure for request: %s", string(tc.body))
+			continue
+		}
+		if errRes.Code != tc.wantRes.Code {
+			t.Errorf("got code %d want code %d for request: %s", errRes.Code, tc.wantRes.Code, string(tc.body))
+		}
+	}
+}

clientapi/routing/device.go

@@ -17,8 +17,10 @@ package routing
 import (
 	"database/sql"
 	"encoding/json"
+	"io/ioutil"
 	"net/http"
 
+	"github.com/matrix-org/dendrite/clientapi/auth"
 	"github.com/matrix-org/dendrite/clientapi/jsonerror"
 	"github.com/matrix-org/dendrite/userapi/api"
 	"github.com/matrix-org/dendrite/userapi/storage/devices"
@@ -72,7 +74,8 @@ func GetDeviceByID(
 	return util.JSONResponse{
 		Code: http.StatusOK,
 		JSON: deviceJSON{
-			DeviceID: dev.ID,
+			DeviceID:    dev.ID,
+			DisplayName: dev.DisplayName,
 		},
 	}
 }
@@ -99,7 +102,8 @@ func GetDevicesByLocalpart(
 
 	for _, dev := range deviceList {
 		res.Devices = append(res.Devices, deviceJSON{
-			DeviceID: dev.ID,
+			DeviceID:    dev.ID,
+			DisplayName: dev.DisplayName,
 		})
 	}
 
@@ -161,20 +165,40 @@ func UpdateDeviceByID(
 
 // DeleteDeviceById handles DELETE requests to /devices/{deviceId}
 func DeleteDeviceById(
-	req *http.Request, deviceDB devices.Database, device *api.Device,
+	req *http.Request, userInteractiveAuth *auth.UserInteractive, deviceDB devices.Database, device *api.Device,
 	deviceID string,
 ) util.JSONResponse {
+	ctx := req.Context()
+	defer req.Body.Close() // nolint:errcheck
+	bodyBytes, err := ioutil.ReadAll(req.Body)
+	if err != nil {
+		return util.JSONResponse{
+			Code: http.StatusBadRequest,
+			JSON: jsonerror.BadJSON("The request body could not be read: " + err.Error()),
+		}
+	}
+	login, errRes := userInteractiveAuth.Verify(ctx, bodyBytes, device)
+	if errRes != nil {
+		return *errRes
+	}
+
 	localpart, _, err := gomatrixserverlib.SplitID('@', device.UserID)
 	if err != nil {
-		util.GetLogger(req.Context()).WithError(err).Error("gomatrixserverlib.SplitID failed")
+		util.GetLogger(ctx).WithError(err).Error("gomatrixserverlib.SplitID failed")
 		return jsonerror.InternalServerError()
 	}
-	ctx := req.Context()
 
-	defer req.Body.Close() // nolint: errcheck
+	// make sure that the access token being used matches the login creds used for user interactive auth, else
+	// 1 compromised access token could be used to logout all devices.
+	if login.Username() != localpart && login.Username() != device.UserID {
+		return util.JSONResponse{
+			Code: 403,
+			JSON: jsonerror.Forbidden("Cannot delete another user's device"),
+		}
+	}
 
 	if err := deviceDB.RemoveDevice(ctx, deviceID, localpart); err != nil {
-		util.GetLogger(req.Context()).WithError(err).Error("deviceDB.RemoveDevice failed")
+		util.GetLogger(ctx).WithError(err).Error("deviceDB.RemoveDevice failed")
 		return jsonerror.InternalServerError()
 	}
 

clientapi/routing/login.go

@@ -15,46 +15,20 @@
 package routing
 
 import (
-	"net/http"
-
 	"context"
+	"net/http"
 
 	"github.com/matrix-org/dendrite/clientapi/auth"
 	"github.com/matrix-org/dendrite/clientapi/httputil"
 	"github.com/matrix-org/dendrite/clientapi/jsonerror"
 	"github.com/matrix-org/dendrite/clientapi/userutil"
 	"github.com/matrix-org/dendrite/internal/config"
-	"github.com/matrix-org/dendrite/userapi/api"
 	"github.com/matrix-org/dendrite/userapi/storage/accounts"
 	"github.com/matrix-org/dendrite/userapi/storage/devices"
 	"github.com/matrix-org/gomatrixserverlib"
 	"github.com/matrix-org/util"
 )
 
-type loginFlows struct {
-	Flows []flow `json:"flows"`
-}
-
-type flow struct {
-	Type   string   `json:"type"`
-	Stages []string `json:"stages"`
-}
-
-type loginIdentifier struct {
-	Type string `json:"type"`
-	User string `json:"user"`
-}
-
-type passwordRequest struct {
-	Identifier loginIdentifier `json:"identifier"`
-	User       string          `json:"user"` // deprecated in favour of identifier
-	Password   string          `json:"password"`
-	// Both DeviceID and InitialDisplayName can be omitted, or empty strings ("")
-	// Thus a pointer is needed to differentiate between the two
-	InitialDisplayName *string `json:"initial_device_display_name"`
-	DeviceID           *string `json:"device_id"`
-}
-
 type loginResponse struct {
 	UserID      string                       `json:"user_id"`
 	AccessToken string                       `json:"access_token"`
@@ -62,9 +36,21 @@ type loginResponse struct {
 	DeviceID    string                       `json:"device_id"`
 }
 
-func passwordLogin() loginFlows {
-	f := loginFlows{}
-	s := flow{"m.login.password", []string{"m.login.password"}}
+type flows struct {
+	Flows []flow `json:"flows"`
+}
+
+type flow struct {
+	Type   string   `json:"type"`
+	Stages []string `json:"stages"`
+}
+
+func passwordLogin() flows {
+	f := flows{}
+	s := flow{
+		Type:   "m.login.password",
+		Stages: []string{"m.login.password"},
+	}
 	f.Flows = append(f.Flows, s)
 	return f
 }
@@ -74,69 +60,28 @@ func Login(
 	req *http.Request, accountDB accounts.Database, deviceDB devices.Database,
 	cfg *config.Dendrite,
 ) util.JSONResponse {
-	if req.Method == http.MethodGet { // TODO: support other forms of login other than password, depending on config options
+	if req.Method == http.MethodGet {
+		// TODO: support other forms of login other than password, depending on config options
 		return util.JSONResponse{
 			Code: http.StatusOK,
 			JSON: passwordLogin(),
 		}
 	} else if req.Method == http.MethodPost {
-		var r passwordRequest
-		var acc *api.Account
-		var errJSON *util.JSONResponse
-		resErr := httputil.UnmarshalJSONRequest(req, &r)
+		typePassword := auth.LoginTypePassword{
+			GetAccountByPassword: accountDB.GetAccountByPassword,
+			Config:               cfg,
+		}
+		r := typePassword.Request()
+		resErr := httputil.UnmarshalJSONRequest(req, r)
 		if resErr != nil {
 			return *resErr
 		}
-		switch r.Identifier.Type {
-		case "m.id.user":
-			if r.Identifier.User == "" {
-				return util.JSONResponse{
-					Code: http.StatusBadRequest,
-					JSON: jsonerror.BadJSON("'user' must be supplied."),
-				}
-			}
-			acc, errJSON = r.processUsernamePasswordLoginRequest(req, accountDB, cfg, r.Identifier.User)
-			if errJSON != nil {
-				return *errJSON
-			}
-		default:
-			// TODO: The below behaviour is deprecated but without it Riot iOS won't log in
-			if r.User != "" {
-				acc, errJSON = r.processUsernamePasswordLoginRequest(req, accountDB, cfg, r.User)
-				if errJSON != nil {
-					return *errJSON
-				}
-			} else {
-				return util.JSONResponse{
-					Code: http.StatusBadRequest,
-					JSON: jsonerror.BadJSON("login identifier '" + r.Identifier.Type + "' not supported"),
-				}
-			}
-		}
-
-		token, err := auth.GenerateAccessToken()
-		if err != nil {
-			util.GetLogger(req.Context()).WithError(err).Error("auth.GenerateAccessToken failed")
-			return jsonerror.InternalServerError()
-		}
-
-		dev, err := getDevice(req.Context(), r, deviceDB, acc, token)
-		if err != nil {
-			return util.JSONResponse{
-				Code: http.StatusInternalServerError,
-				JSON: jsonerror.Unknown("failed to create device: " + err.Error()),
-			}
-		}
-
-		return util.JSONResponse{
-			Code: http.StatusOK,
-			JSON: loginResponse{
-				UserID:      dev.UserID,
-				AccessToken: dev.AccessToken,
-				HomeServer:  cfg.Matrix.ServerName,
-				DeviceID:    dev.ID,
-			},
+		login, authErr := typePassword.Login(req.Context(), r)
+		if authErr != nil {
+			return *authErr
 		}
+		// make a device/access token
+		return completeAuth(req.Context(), cfg.Matrix.ServerName, deviceDB, login)
 	}
 	return util.JSONResponse{
 		Code: http.StatusMethodNotAllowed,
@@ -144,45 +89,38 @@ func Login(
 	}
 }
 
-// getDevice returns a new or existing device
-func getDevice(
-	ctx context.Context,
-	r passwordRequest,
-	deviceDB devices.Database,
-	acc *api.Account,
-	token string,
-) (dev *api.Device, err error) {
-	dev, err = deviceDB.CreateDevice(
-		ctx, acc.Localpart, r.DeviceID, token, r.InitialDisplayName,
+func completeAuth(
+	ctx context.Context, serverName gomatrixserverlib.ServerName, deviceDB devices.Database, login *auth.Login,
+) util.JSONResponse {
+	token, err := auth.GenerateAccessToken()
+	if err != nil {
+		util.GetLogger(ctx).WithError(err).Error("auth.GenerateAccessToken failed")
+		return jsonerror.InternalServerError()
+	}
+
+	localpart, err := userutil.ParseUsernameParam(login.Username(), &serverName)
+	if err != nil {
+		util.GetLogger(ctx).WithError(err).Error("auth.ParseUsernameParam failed")
+		return jsonerror.InternalServerError()
+	}
+
+	dev, err := deviceDB.CreateDevice(
+		ctx, localpart, login.DeviceID, token, login.InitialDisplayName,
 	)
-	return
-}
-
-func (r *passwordRequest) processUsernamePasswordLoginRequest(
-	req *http.Request, accountDB accounts.Database,
-	cfg *config.Dendrite, username string,
-) (acc *api.Account, errJSON *util.JSONResponse) {
-	util.GetLogger(req.Context()).WithField("user", username).Info("Processing login request")
-
-	localpart, err := userutil.ParseUsernameParam(username, &cfg.Matrix.ServerName)
 	if err != nil {
-		errJSON = &util.JSONResponse{
-			Code: http.StatusBadRequest,
-			JSON: jsonerror.InvalidUsername(err.Error()),
+		return util.JSONResponse{
+			Code: http.StatusInternalServerError,
+			JSON: jsonerror.Unknown("failed to create device: " + err.Error()),
 		}
-		return
 	}
 
-	acc, err = accountDB.GetAccountByPassword(req.Context(), localpart, r.Password)
-	if err != nil {
-		// Technically we could tell them if the user does not exist by checking if err == sql.ErrNoRows
-		// but that would leak the existence of the user.
-		errJSON = &util.JSONResponse{
-			Code: http.StatusForbidden,
-			JSON: jsonerror.Forbidden("username or password was incorrect, or the account does not exist"),
-		}
-		return
+	return util.JSONResponse{
+		Code: http.StatusOK,
+		JSON: loginResponse{
+			UserID:      dev.UserID,
+			AccessToken: dev.AccessToken,
+			HomeServer:  serverName,
+			DeviceID:    dev.ID,
+		},
 	}
-
-	return
 }

clientapi/routing/routing.go

@@ -22,6 +22,7 @@ import (
 	"github.com/gorilla/mux"
 	appserviceAPI "github.com/matrix-org/dendrite/appservice/api"
 	"github.com/matrix-org/dendrite/clientapi/api"
+	"github.com/matrix-org/dendrite/clientapi/auth"
 	"github.com/matrix-org/dendrite/clientapi/jsonerror"
 	"github.com/matrix-org/dendrite/clientapi/producers"
 	currentstateAPI "github.com/matrix-org/dendrite/currentstateserver/api"
@@ -63,6 +64,7 @@ func Setup(
 	stateAPI currentstateAPI.CurrentStateInternalAPI,
 	extRoomsProvider api.ExtraPublicRoomsProvider,
 ) {
+	userInteractiveAuth := auth.NewUserInteractive(accountDB.GetAccountByPassword, cfg)
 
 	publicAPIMux.Handle("/client/versions",
 		httputil.MakeExternalAPI("versions", func(req *http.Request) util.JSONResponse {
@@ -629,7 +631,7 @@ func Setup(
 			if err != nil {
 				return util.ErrorResponse(err)
 			}
-			return DeleteDeviceById(req, deviceDB, device, vars["deviceID"])
+			return DeleteDeviceById(req, userInteractiveAuth, deviceDB, device, vars["deviceID"])
 		}),
 	).Methods(http.MethodDelete, http.MethodOptions)
 

sytest-whitelist

@@ -31,6 +31,12 @@ PUT /profile/:user_id/avatar_url sets my avatar
 GET /profile/:user_id/avatar_url publicly accessible
 GET /device/{deviceId} gives a 404 for unknown devices
 PUT /device/{deviceId} gives a 404 for unknown devices
+GET /device/{deviceId}
+GET /devices
+PUT /device/{deviceId} updates device fields
+DELETE /device/{deviceId}
+DELETE /device/{deviceId} requires UI auth user to match device owner
+DELETE /device/{deviceId} with no body gives a 401
 POST /createRoom makes a public room
 POST /createRoom makes a private room
 POST /createRoom makes a private room with invites