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) <andrew@amorgan.xyz>

src/github.com/matrix-org/dendrite/clientapi/auth/authtypes/flow.go

@@ -0,0 +1,21 @@
+// Copyright Andrew Morgan <andrew@amorgan.xyz>
+//
+// 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"`
+}

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,

src/github.com/matrix-org/dendrite/clientapi/routing/register_test.go

@@ -0,0 +1,134 @@
+// Copyright 2017 Andrew Morgan <andrew@amorgan.xyz>
+//
+// 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.")
+	}
+}

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.