Refactor LoginTypePassword and Type to support m.login.token and m.login.sso.

For login token:

* m.login.token will require deleting the token after completeAuth has
  generated an access token, so a cleanup function is returned by
  Type.Login.
* Allowing different login types will require parsing the /login body
  twice: first to extract the "type" and then the type-specific parsing.
  Thus, we will have to buffer the request JSON in /login, like
  UserInteractive already does.

For SSO:

* NewUserInteractive will have to also use GetAccountByLocalpart. It
  makes more sense to just pass a (narrowed-down) accountDB interface
  to it than adding more function pointers.

Code quality:

* Passing around (and down-casting) interface{} for login request types
  has drawbacks in terms of type-safety, and no inherent benefits. We
  always decode JSON anyway. Hence renaming to Type.LoginFromJSON. Code
  that directly uses LoginTypePassword with parsed data can still use
  Login.
* Removed a TODO for SSO. This is already tracked in #1297.
* httputil.UnmarshalJSON is useful because it returns a JSONResponse.

This change is intended to have no functional changes.
This commit is contained in:
Tommie Gannert 2021-09-26 14:35:14 +02:00
parent 8e085b1164
commit 6714983349
7 changed files with 55 additions and 36 deletions

View file

@ -42,6 +42,7 @@ type DeviceDatabase interface {
type AccountDatabase interface { type AccountDatabase interface {
// Look up the account matching the given localpart. // Look up the account matching the given localpart.
GetAccountByLocalpart(ctx context.Context, localpart string) (*api.Account, error) GetAccountByLocalpart(ctx context.Context, localpart string) (*api.Account, error)
GetAccountByPassword(ctx context.Context, localpart, password string) (*api.Account, error)
} }
// VerifyUserFromRequest authenticates the HTTP request, // VerifyUserFromRequest authenticates the HTTP request,

View file

@ -19,6 +19,8 @@ import (
"net/http" "net/http"
"strings" "strings"
"github.com/matrix-org/dendrite/clientapi/auth/authtypes"
"github.com/matrix-org/dendrite/clientapi/httputil"
"github.com/matrix-org/dendrite/clientapi/jsonerror" "github.com/matrix-org/dendrite/clientapi/jsonerror"
"github.com/matrix-org/dendrite/clientapi/userutil" "github.com/matrix-org/dendrite/clientapi/userutil"
"github.com/matrix-org/dendrite/setup/config" "github.com/matrix-org/dendrite/setup/config"
@ -40,16 +42,24 @@ type LoginTypePassword struct {
} }
func (t *LoginTypePassword) Name() string { func (t *LoginTypePassword) Name() string {
return "m.login.password" return authtypes.LoginTypePassword
} }
func (t *LoginTypePassword) Request() interface{} { func (t *LoginTypePassword) LoginFromJSON(ctx context.Context, reqBytes []byte) (*Login, LoginCleanupFunc, *util.JSONResponse) {
return &PasswordRequest{} var r PasswordRequest
if err := httputil.UnmarshalJSON(reqBytes, &r); err != nil {
return nil, nil, err
}
login, err := t.Login(ctx, &r)
if err != nil {
return nil, nil, err
}
return login, func(context.Context, *util.JSONResponse) {}, nil
} }
func (t *LoginTypePassword) Login(ctx context.Context, req interface{}) (*Login, *util.JSONResponse) { func (t *LoginTypePassword) Login(ctx context.Context, r *PasswordRequest) (*Login, *util.JSONResponse) {
r := req.(*PasswordRequest)
// Squash username to all lowercase letters
username := strings.ToLower(r.Username()) username := strings.ToLower(r.Username())
if username == "" { if username == "" {
return nil, &util.JSONResponse{ return nil, &util.JSONResponse{

View file

@ -32,22 +32,24 @@ import (
type Type interface { type Type interface {
// Name returns the name of the auth type e.g `m.login.password` // Name returns the name of the auth type e.g `m.login.password`
Name() string 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. // Login with the auth type, returning an error response on failure.
// Not all types support login, only m.login.password and m.login.token // 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 // 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 // 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: // 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 // "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, // 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." // with the addition of the standard errcode and error fields describing the error."
Login(ctx context.Context, req interface{}) (login *Login, errRes *util.JSONResponse) //
// The returned cleanup function must be non-nil on success, and will be called after
// authorization has been completed. Its argument is the final result of authorization.
LoginFromJSON(ctx context.Context, reqBytes []byte) (login *Login, cleanup LoginCleanupFunc, errRes *util.JSONResponse)
// TODO: Extend to support Register() flow // TODO: Extend to support Register() flow
// Register(ctx context.Context, sessionID string, req interface{}) // Register(ctx context.Context, sessionID string, req interface{})
} }
type LoginCleanupFunc func(context.Context, *util.JSONResponse)
// LoginIdentifier represents identifier types // LoginIdentifier represents identifier types
// https://matrix.org/docs/spec/client_server/r0.6.1#identifier-types // https://matrix.org/docs/spec/client_server/r0.6.1#identifier-types
type LoginIdentifier struct { type LoginIdentifier struct {
@ -111,12 +113,11 @@ type UserInteractive struct {
Sessions map[string][]string Sessions map[string][]string
} }
func NewUserInteractive(getAccByPass GetAccountByPassword, cfg *config.ClientAPI) *UserInteractive { func NewUserInteractive(accountDB AccountDatabase, cfg *config.ClientAPI) *UserInteractive {
typePassword := &LoginTypePassword{ typePassword := &LoginTypePassword{
GetAccountByPassword: getAccByPass, GetAccountByPassword: accountDB.GetAccountByPassword,
Config: cfg, Config: cfg,
} }
// TODO: Add SSO login
return &UserInteractive{ return &UserInteractive{
Completed: []string{}, Completed: []string{},
Flows: []userInteractiveFlow{ Flows: []userInteractiveFlow{
@ -236,18 +237,13 @@ func (u *UserInteractive) Verify(ctx context.Context, bodyBytes []byte, device *
} }
} }
r := loginType.Request() login, cleanup, resErr := loginType.LoginFromJSON(ctx, []byte(gjson.GetBytes(bodyBytes, "auth").Raw))
if err := json.Unmarshal([]byte(gjson.GetBytes(bodyBytes, "auth").Raw), r); err != nil { if resErr != nil {
return nil, &util.JSONResponse{ return nil, u.ResponseWithChallenge(sessionID, resErr.JSON)
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)
u.AddCompletedStage(sessionID, authType) cleanup(ctx, nil)
// TODO: Check if there's more stages to go and return an error // TODO: Check if there's more stages to go and return an error
return login, nil return login, nil
}
return nil, u.ResponseWithChallenge(sessionID, resErr.JSON)
} }

View file

@ -24,7 +24,11 @@ var (
} }
) )
func getAccountByPassword(ctx context.Context, localpart, plaintextPassword string) (*api.Account, error) { type fakeAccountDatabase struct {
AccountDatabase
}
func (*fakeAccountDatabase) GetAccountByPassword(ctx context.Context, localpart, plaintextPassword string) (*api.Account, error) {
acc, ok := lookup[localpart+" "+plaintextPassword] acc, ok := lookup[localpart+" "+plaintextPassword]
if !ok { if !ok {
return nil, fmt.Errorf("unknown user/password") return nil, fmt.Errorf("unknown user/password")
@ -38,7 +42,7 @@ func setup() *UserInteractive {
ServerName: serverName, ServerName: serverName,
}, },
} }
return NewUserInteractive(getAccountByPassword, cfg) return NewUserInteractive(&fakeAccountDatabase{}, cfg)
} }
func TestUserInteractiveChallenge(t *testing.T) { func TestUserInteractiveChallenge(t *testing.T) {

View file

@ -36,6 +36,10 @@ func UnmarshalJSONRequest(req *http.Request, iface interface{}) *util.JSONRespon
return &resp return &resp
} }
return UnmarshalJSON(body, iface)
}
func UnmarshalJSON(body []byte, iface interface{}) *util.JSONResponse {
if !utf8.Valid(body) { if !utf8.Valid(body) {
return &util.JSONResponse{ return &util.JSONResponse{
Code: http.StatusBadRequest, Code: http.StatusBadRequest,

View file

@ -16,10 +16,10 @@ package routing
import ( import (
"context" "context"
"io/ioutil"
"net/http" "net/http"
"github.com/matrix-org/dendrite/clientapi/auth" "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/jsonerror"
"github.com/matrix-org/dendrite/clientapi/userutil" "github.com/matrix-org/dendrite/clientapi/userutil"
"github.com/matrix-org/dendrite/setup/config" "github.com/matrix-org/dendrite/setup/config"
@ -69,17 +69,21 @@ func Login(
GetAccountByPassword: accountDB.GetAccountByPassword, GetAccountByPassword: accountDB.GetAccountByPassword,
Config: cfg, Config: cfg,
} }
r := typePassword.Request() body, err := ioutil.ReadAll(req.Body)
resErr := httputil.UnmarshalJSONRequest(req, r) if err != nil {
if resErr != nil { return util.JSONResponse{
return *resErr Code: http.StatusBadRequest,
JSON: jsonerror.BadJSON("Reading request body failed: " + err.Error()),
}
} }
login, authErr := typePassword.Login(req.Context(), r) login, cleanup, authErr := typePassword.LoginFromJSON(req.Context(), body)
if authErr != nil { if authErr != nil {
return *authErr return *authErr
} }
// make a device/access token // make a device/access token
return completeAuth(req.Context(), cfg.Matrix.ServerName, userAPI, login, req.RemoteAddr, req.UserAgent()) authzErr := completeAuth(req.Context(), cfg.Matrix.ServerName, userAPI, login, req.RemoteAddr, req.UserAgent())
cleanup(req.Context(), &authzErr)
return authzErr
} }
return util.JSONResponse{ return util.JSONResponse{
Code: http.StatusMethodNotAllowed, Code: http.StatusMethodNotAllowed,

View file

@ -62,7 +62,7 @@ func Setup(
mscCfg *config.MSCs, mscCfg *config.MSCs,
) { ) {
rateLimits := httputil.NewRateLimits(&cfg.RateLimiting) rateLimits := httputil.NewRateLimits(&cfg.RateLimiting)
userInteractiveAuth := auth.NewUserInteractive(accountDB.GetAccountByPassword, cfg) userInteractiveAuth := auth.NewUserInteractive(accountDB, cfg)
unstableFeatures := map[string]bool{ unstableFeatures := map[string]bool{
"org.matrix.e2e_cross_signing": true, "org.matrix.e2e_cross_signing": true,