From a4460dd6ee2a1a29c8c90338dde12259e60e3abe Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Wed, 29 Nov 2017 01:43:03 -0800 Subject: [PATCH] Set up proper registration flow (#354) * Refactor registration to align with the spec * We now keep track of sessions and their completed registration stages. * We only complete registration if the client has completed a full flow. * New Derived section in config for data derived from config options. * New config options for captcha. * Send params back to client for each registration stage. Signed-off-by: Andrew Morgan (https://amorgan.xyz) --- .../dendrite/clientapi/auth/authtypes/flow.go | 21 +++ .../dendrite/clientapi/routing/register.go | 137 +++++++++++++++--- .../clientapi/routing/register_test.go | 134 +++++++++++++++++ .../dendrite/common/config/config.go | 40 +++++ 4 files changed, 309 insertions(+), 23 deletions(-) create mode 100644 src/github.com/matrix-org/dendrite/clientapi/auth/authtypes/flow.go create mode 100644 src/github.com/matrix-org/dendrite/clientapi/routing/register_test.go diff --git a/src/github.com/matrix-org/dendrite/clientapi/auth/authtypes/flow.go b/src/github.com/matrix-org/dendrite/clientapi/auth/authtypes/flow.go new file mode 100644 index 000000000..d5766fcc2 --- /dev/null +++ b/src/github.com/matrix-org/dendrite/clientapi/auth/authtypes/flow.go @@ -0,0 +1,21 @@ +// Copyright Andrew Morgan +// +// 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 authtypes + +// Flow represents one possible way that the client can authenticate a request. +// https://matrix.org/docs/spec/client_server/r0.3.0.html#user-interactive-authentication-api +type Flow struct { + Stages []LoginType `json:"stages"` +} diff --git a/src/github.com/matrix-org/dendrite/clientapi/routing/register.go b/src/github.com/matrix-org/dendrite/clientapi/routing/register.go index 875ceb049..ef8fbaf54 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/routing/register.go +++ b/src/github.com/matrix-org/dendrite/clientapi/routing/register.go @@ -23,8 +23,8 @@ import ( "fmt" "net/http" "regexp" + "sort" "strings" - "time" "github.com/matrix-org/dendrite/common/config" @@ -43,9 +43,14 @@ const ( minPasswordLength = 8 // http://matrix.org/docs/spec/client_server/r0.2.0.html#password-based maxPasswordLength = 512 // https://github.com/matrix-org/synapse/blob/v0.20.0/synapse/rest/client/v2_alpha/register.py#L161 maxUsernameLength = 254 // http://matrix.org/speculator/spec/HEAD/intro.html#user-identifiers TODO account for domain + sessionIDLength = 24 ) -var validUsernameRegex = regexp.MustCompile(`^[0-9a-zA-Z_\-./]+$`) +var ( + // TODO: Remove old sessions. Need to do so on a session-specific timeout. + sessions = make(map[string][]authtypes.LoginType) // Sessions and completed flow stages + validUsernameRegex = regexp.MustCompile(`^[0-9a-zA-Z_\-./]+$`) +) // registerRequest represents the submitted registration request. // It can be broken down into 2 sections: the auth dictionary and registration parameters. @@ -68,23 +73,18 @@ type authDict struct { Type authtypes.LoginType `json:"type"` Session string `json:"session"` Mac gomatrixserverlib.HexString `json:"mac"` + // TODO: Lots of custom keys depending on the type } // http://matrix.org/speculator/spec/HEAD/client_server/unstable.html#user-interactive-authentication-api type userInteractiveResponse struct { - Flows []authFlow `json:"flows"` + Flows []authtypes.Flow `json:"flows"` Completed []authtypes.LoginType `json:"completed"` Params map[string]interface{} `json:"params"` Session string `json:"session"` } -// authFlow represents one possible way that the client can authenticate a request. -// http://matrix.org/speculator/spec/HEAD/client_server/unstable.html#user-interactive-authentication-api -type authFlow struct { - Stages []authtypes.LoginType `json:"stages"` -} - // legacyRegisterRequest represents the submitted registration request for v1 API. type legacyRegisterRequest struct { Password string `json:"password"` @@ -94,9 +94,15 @@ type legacyRegisterRequest struct { Mac gomatrixserverlib.HexString `json:"mac"` } -func newUserInteractiveResponse(sessionID string, fs []authFlow) userInteractiveResponse { +// newUserInteractiveResponse will return a struct to be sent back to the client +// during registration. +func newUserInteractiveResponse( + sessionID string, + fs []authtypes.Flow, + params map[string]interface{}, +) userInteractiveResponse { return userInteractiveResponse{ - fs, []authtypes.LoginType{}, make(map[string]interface{}), sessionID, + fs, sessions[sessionID], params, sessionID, } } @@ -154,22 +160,26 @@ func Register( deviceDB *devices.Database, cfg *config.Dendrite, ) util.JSONResponse { + var r registerRequest resErr := httputil.UnmarshalJSONRequest(req, &r) if resErr != nil { return *resErr } - // All registration requests must specify what auth they are using to perform this request + // Retrieve or generate the sessionID + sessionID := r.Auth.Session + if sessionID == "" { + // Generate a new, random session ID + sessionID = util.RandomString(sessionIDLength) + } + + // If no auth type is specified by the client, send back the list of available flows if r.Auth.Type == "" { return util.JSONResponse{ Code: 401, - // TODO: Hard-coded 'dummy' auth for now with a bogus session ID. - // Server admins should be able to change things around (eg enable captcha) - JSON: newUserInteractiveResponse(time.Now().String(), []authFlow{ - {[]authtypes.LoginType{authtypes.LoginTypeDummy}}, - {[]authtypes.LoginType{authtypes.LoginTypeSharedSecret}}, - }), + JSON: newUserInteractiveResponse(sessionID, + cfg.Derived.Registration.Flows, cfg.Derived.Registration.Params), } } @@ -187,6 +197,19 @@ func Register( "session_id": r.Auth.Session, }).Info("Processing registration request") + return handleRegistrationFlow(req, r, sessionID, cfg, accountDB, deviceDB) +} + +// handleRegistrationFlow will direct and complete registration flow stages +// that the client has requested. +func handleRegistrationFlow( + req *http.Request, + r registerRequest, + sessionID string, + cfg *config.Dendrite, + accountDB *accounts.Database, + deviceDB *devices.Database, +) util.JSONResponse { // TODO: Shared secret registration (create new user scripts) // TODO: AS API registration // TODO: Enable registration config flag @@ -202,7 +225,8 @@ func Register( return util.MessageResponse(400, "Shared secret registration is disabled") } - valid, err := isValidMacLogin(r.Username, r.Password, r.Admin, r.Auth.Mac, cfg.Matrix.RegistrationSharedSecret) + valid, err := isValidMacLogin(r.Username, r.Password, r.Admin, + r.Auth.Mac, cfg.Matrix.RegistrationSharedSecret) if err != nil { return httputil.LogThenError(req, err) @@ -212,16 +236,34 @@ func Register( return util.MessageResponse(403, "HMAC incorrect") } - return completeRegistration(req.Context(), accountDB, deviceDB, r.Username, r.Password, r.InitialDisplayName) + // Add SharedSecret to the list of completed registration stages + sessions[sessionID] = append(sessions[sessionID], authtypes.LoginTypeSharedSecret) + case authtypes.LoginTypeDummy: // there is nothing to do - return completeRegistration(req.Context(), accountDB, deviceDB, r.Username, r.Password, r.InitialDisplayName) + // Add Dummy to the list of completed registration stages + sessions[sessionID] = append(sessions[sessionID], authtypes.LoginTypeDummy) + default: return util.JSONResponse{ Code: 501, JSON: jsonerror.Unknown("unknown/unimplemented auth type"), } } + + // Check if the user's registration flow has been completed successfully + if !checkFlowCompleted(sessions[sessionID], cfg.Derived.Registration.Flows) { + // There are still more stages to complete. + // Return the flows and those that have been completed. + return util.JSONResponse{ + Code: 401, + JSON: newUserInteractiveResponse(sessionID, + cfg.Derived.Registration.Flows, cfg.Derived.Registration.Params), + } + } + + return completeRegistration(req.Context(), accountDB, deviceDB, + r.Username, r.Password, r.InitialDisplayName) } // LegacyRegister process register requests from the legacy v1 API @@ -348,7 +390,7 @@ func isValidMacLogin( givenMac []byte, sharedSecret string, ) (bool, error) { - // Double check that username/passowrd don't contain the HMAC delimiters. We should have + // Double check that username/password don't contain the HMAC delimiters. We should have // already checked this. if strings.Contains(username, "\x00") { return false, errors.New("Username contains invalid character") @@ -376,11 +418,60 @@ func isValidMacLogin( return hmac.Equal(givenMac, expectedMAC), nil } +// checkFlows checks a single completed flow against another required one. If +// one contains at least all of the stages that the other does, checkFlows +// returns true. +func checkFlows( + completedStages []authtypes.LoginType, + requiredStages []authtypes.LoginType, +) bool { + // Create temporary slices so they originals will not be modified on sorting + completed := make([]authtypes.LoginType, len(completedStages)) + required := make([]authtypes.LoginType, len(requiredStages)) + copy(completed, completedStages) + copy(required, requiredStages) + + // Sort the slices for simple comparison + sort.Slice(completed, func(i, j int) bool { return completed[i] < completed[j] }) + sort.Slice(required, func(i, j int) bool { return required[i] < required[j] }) + + // Iterate through each slice, going to the next required slice only once + // we've found a match. + i, j := 0, 0 + for j < len(required) { + // Exit if we've reached the end of our input without being able to + // match all of the required stages. + if i >= len(completed) { + return false + } + + // If we've found a stage we want, move on to the next required stage. + if completed[i] == required[j] { + j++ + } + i++ + } + return true +} + +// checkFlowCompleted checks if a registration flow complies with any allowed flow +// dictated by the server. Order of stages does not matter. A user may complete +// extra stages as long as the required stages of at least one flow is met. +func checkFlowCompleted(flow []authtypes.LoginType, allowedFlows []authtypes.Flow) bool { + // Iterate through possible flows to check whether any have been fully completed. + for _, allowedFlow := range allowedFlows { + if checkFlows(flow, allowedFlow.Stages) { + return true + } + } + return false +} + type availableResponse struct { Available bool `json:"available"` } -// RegisterAvailable checks if the username is already taken or invalid +// RegisterAvailable checks if the username is already taken or invalid. func RegisterAvailable( req *http.Request, accountDB *accounts.Database, diff --git a/src/github.com/matrix-org/dendrite/clientapi/routing/register_test.go b/src/github.com/matrix-org/dendrite/clientapi/routing/register_test.go new file mode 100644 index 000000000..de18c8d2a --- /dev/null +++ b/src/github.com/matrix-org/dendrite/clientapi/routing/register_test.go @@ -0,0 +1,134 @@ +// Copyright 2017 Andrew Morgan +// +// 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 routing + +import ( + "testing" + + "github.com/matrix-org/dendrite/clientapi/auth/authtypes" +) + +var ( + // Registration Flows that the server allows. + allowedFlows []authtypes.Flow = []authtypes.Flow{ + { + []authtypes.LoginType{ + authtypes.LoginType("stage1"), + authtypes.LoginType("stage2"), + }, + }, + { + []authtypes.LoginType{ + authtypes.LoginType("stage1"), + authtypes.LoginType("stage3"), + }, + }, + } +) + +// Should return true as we're completing all the stages of a single flow in +// order. +func TestFlowCheckingCompleteFlowOrdered(t *testing.T) { + testFlow := []authtypes.LoginType{ + authtypes.LoginType("stage1"), + authtypes.LoginType("stage3"), + } + + if !checkFlowCompleted(testFlow, allowedFlows) { + t.Error("Incorrect registration flow verification: ", testFlow, ", from allowed flows: ", allowedFlows, ". Should be true.") + } +} + +// Should return false as all stages in a single flow need to be completed. +func TestFlowCheckingStagesFromDifferentFlows(t *testing.T) { + testFlow := []authtypes.LoginType{ + authtypes.LoginType("stage2"), + authtypes.LoginType("stage3"), + } + + if checkFlowCompleted(testFlow, allowedFlows) { + t.Error("Incorrect registration flow verification: ", testFlow, ", from allowed flows: ", allowedFlows, ". Should be false.") + } +} + +// Should return true as we're completing all the stages from a single flow, as +// well as some extraneous stages. +func TestFlowCheckingCompleteOrderedExtraneous(t *testing.T) { + testFlow := []authtypes.LoginType{ + authtypes.LoginType("stage1"), + authtypes.LoginType("stage3"), + authtypes.LoginType("stage4"), + authtypes.LoginType("stage5"), + } + if !checkFlowCompleted(testFlow, allowedFlows) { + t.Error("Incorrect registration flow verification: ", testFlow, ", from allowed flows: ", allowedFlows, ". Should be true.") + } +} + +// Should return false as we're submitting an empty flow. +func TestFlowCheckingEmptyFlow(t *testing.T) { + testFlow := []authtypes.LoginType{} + if checkFlowCompleted(testFlow, allowedFlows) { + t.Error("Incorrect registration flow verification: ", testFlow, ", from allowed flows: ", allowedFlows, ". Should be false.") + } +} + +// Should return false as we've completed a stage that isn't in any allowed flow. +func TestFlowCheckingInvalidStage(t *testing.T) { + testFlow := []authtypes.LoginType{ + authtypes.LoginType("stage8"), + } + if checkFlowCompleted(testFlow, allowedFlows) { + t.Error("Incorrect registration flow verification: ", testFlow, ", from allowed flows: ", allowedFlows, ". Should be false.") + } +} + +// Should return true as we complete all stages of an allowed flow, though out +// of order, as well as extraneous stages. +func TestFlowCheckingExtraneousUnordered(t *testing.T) { + testFlow := []authtypes.LoginType{ + authtypes.LoginType("stage5"), + authtypes.LoginType("stage4"), + authtypes.LoginType("stage3"), + authtypes.LoginType("stage2"), + authtypes.LoginType("stage1"), + } + if !checkFlowCompleted(testFlow, allowedFlows) { + t.Error("Incorrect registration flow verification: ", testFlow, ", from allowed flows: ", allowedFlows, ". Should be true.") + } +} + +// Should return false as we're providing fewer stages than are required. +func TestFlowCheckingShortIncorrectInput(t *testing.T) { + testFlow := []authtypes.LoginType{ + authtypes.LoginType("stage8"), + } + if checkFlowCompleted(testFlow, allowedFlows) { + t.Error("Incorrect registration flow verification: ", testFlow, ", from allowed flows: ", allowedFlows, ". Should be false.") + } +} + +// Should return false as we're providing different stages than are required. +func TestFlowCheckingExtraneousIncorrectInput(t *testing.T) { + testFlow := []authtypes.LoginType{ + authtypes.LoginType("stage8"), + authtypes.LoginType("stage9"), + authtypes.LoginType("stage10"), + authtypes.LoginType("stage11"), + } + if checkFlowCompleted(testFlow, allowedFlows) { + t.Error("Incorrect registration flow verification: ", testFlow, ", from allowed flows: ", allowedFlows, ". Should be false.") + } +} diff --git a/src/github.com/matrix-org/dendrite/common/config/config.go b/src/github.com/matrix-org/dendrite/common/config/config.go index 797889028..3c488f4e9 100644 --- a/src/github.com/matrix-org/dendrite/common/config/config.go +++ b/src/github.com/matrix-org/dendrite/common/config/config.go @@ -25,6 +25,7 @@ import ( "strings" "time" + "github.com/matrix-org/dendrite/clientapi/auth/authtypes" "github.com/matrix-org/gomatrixserverlib" "github.com/sirupsen/logrus" "golang.org/x/crypto/ed25519" @@ -189,6 +190,21 @@ type Dendrite struct { // The config for the jaeger opentracing reporter. Jaeger jaegerconfig.Configuration `yaml:"jaeger"` } + + // Any information derived from the configuration options for later use. + Derived struct { + Registration struct { + // Flows is a slice of flows, which represent one possible way that the client can authenticate a request. + // http://matrix.org/docs/spec/HEAD/client_server/r0.3.0.html#user-interactive-authentication-api + // As long as the generated flows only rely on config file options, + // we can generate them on startup and store them until needed + Flows []authtypes.Flow `json:"flows"` + + // Params that need to be returned to the client during + // registration in order to complete registration stages. + Params map[string]interface{} `json:"params"` + } + } } // A Path on the filesystem. @@ -305,9 +321,28 @@ func loadConfig( config.Media.AbsBasePath = Path(absPath(basePath, config.Media.BasePath)) + // Generate data from config options + config.derive() + return &config, nil } +// derive generates data that is derived from various values provided in +// the config file. +func (config *Dendrite) derive() { + // Determine registrations flows based off config values + + config.Derived.Registration.Params = make(map[string]interface{}) + + // TODO: Add email auth type + // TODO: Add MSISDN auth type + // TODO: Add Recaptcha auth type + + config.Derived.Registration.Flows = append(config.Derived.Registration.Flows, + authtypes.Flow{[]authtypes.LoginType{authtypes.LoginTypeDummy}}) +} + +// setDefaults sets default config values if they are not explicitly set. func (config *Dendrite) setDefaults() { if config.Matrix.KeyValidityPeriod == 0 { config.Matrix.KeyValidityPeriod = 24 * time.Hour @@ -327,6 +362,8 @@ func (config *Dendrite) setDefaults() { } } +// Error returns a string detailing how many errors were contained within an +// Error type. func (e Error) Error() string { if len(e.Problems) == 1 { return e.Problems[0] @@ -336,6 +373,8 @@ func (e Error) Error() string { ) } +// check returns an error type containing all errors found within the config +// file. func (config *Dendrite) check(monolithic bool) error { var problems []string @@ -420,6 +459,7 @@ func (config *Dendrite) check(monolithic bool) error { return nil } +// absPath returns the absolute path for a given relative or absolute path. func absPath(dir string, path Path) string { if filepath.IsAbs(string(path)) { // filepath.Join cleans the path so we should clean the absolute paths as well for consistency.