Add clientapi tests

.github/workflows/dendrite.yml

@@ -331,8 +331,7 @@ jobs:
             postgres: postgres
             api: full-http
     container:
-      # Temporary for debugging to see if this image is working better.
-      image: matrixdotorg/sytest-dendrite@sha256:434ad464a9f4ed3f8c3cc47200275b6ccb5c5031a8063daf4acea62be5a23c73
+      image: matrixdotorg/sytest-dendrite
       volumes:
         - ${{ github.workspace }}:/src
         - /root/.cache/go-build:/github/home/.cache/go-build

clientapi/routing/auth_fallback.go

@@ -15,6 +15,7 @@
 package routing
 
 import (
+	"fmt"
 	"html/template"
 	"net/http"
 
@@ -102,6 +103,22 @@ func AuthFallback(
 	w http.ResponseWriter, req *http.Request, authType string,
 	cfg *config.ClientAPI,
 ) *util.JSONResponse {
+	// We currently only support reCaptcha, so fail early if that's not requested
+	if authType == authtypes.LoginTypeRecaptcha {
+		if !cfg.RecaptchaEnabled {
+			return writeHTTPMessage(w, req,
+				"Recaptcha login is disabled on this Homeserver",
+				http.StatusBadRequest,
+			)
+		}
+	} else {
+		_ = writeHTTPMessage(w, req, fmt.Sprintf("Unknown authtype %q", authType), http.StatusNotImplemented)
+		return &util.JSONResponse{
+			Code: http.StatusNotFound,
+			JSON: jsonerror.NotFound("Unknown auth stage type"),
+		}
+	}
+
 	sessionID := req.URL.Query().Get("session")
 
 	if sessionID == "" {
@@ -130,72 +147,39 @@ func AuthFallback(
 
 	if req.Method == http.MethodGet {
 		// Handle Recaptcha
-		if authType == authtypes.LoginTypeRecaptcha {
-			if err := checkRecaptchaEnabled(cfg, w, req); err != nil {
-				return err
-			}
-
-			serveRecaptcha()
-			return nil
-		}
-		return &util.JSONResponse{
-			Code: http.StatusNotFound,
-			JSON: jsonerror.NotFound("Unknown auth stage type"),
-		}
+		serveRecaptcha()
+		return nil
 	} else if req.Method == http.MethodPost {
 		// Handle Recaptcha
-		if authType == authtypes.LoginTypeRecaptcha {
-			if err := checkRecaptchaEnabled(cfg, w, req); err != nil {
-				return err
-			}
-
-			clientIP := req.RemoteAddr
-			err := req.ParseForm()
-			if err != nil {
-				util.GetLogger(req.Context()).WithError(err).Error("req.ParseForm failed")
-				res := jsonerror.InternalServerError()
-				return &res
-			}
-
-			response := req.Form.Get(cfg.RecaptchaFormField)
-			if err := validateRecaptcha(cfg, response, clientIP); err != nil {
-				util.GetLogger(req.Context()).Error(err)
-				return err
-			}
-
-			// Success. Add recaptcha as a completed login flow
-			sessions.addCompletedSessionStage(sessionID, authtypes.LoginTypeRecaptcha)
-
-			serveSuccess()
-			return nil
+		clientIP := req.RemoteAddr
+		err := req.ParseForm()
+		if err != nil {
+			util.GetLogger(req.Context()).WithError(err).Error("req.ParseForm failed")
+			res := jsonerror.InternalServerError()
+			return &res
 		}
 
-		return &util.JSONResponse{
-			Code: http.StatusNotFound,
-			JSON: jsonerror.NotFound("Unknown auth stage type"),
+		response := req.Form.Get(cfg.RecaptchaFormField)
+		if err := validateRecaptcha(cfg, response, clientIP); err != nil {
+			util.GetLogger(req.Context()).Error(err)
+			w.WriteHeader(http.StatusUnauthorized)
+			serveRecaptcha() // serve the initial page again, instead of nothing
+			return err
 		}
+
+		// Success. Add recaptcha as a completed login flow
+		sessions.addCompletedSessionStage(sessionID, authtypes.LoginTypeRecaptcha)
+
+		serveSuccess()
+		return nil
 	}
+	_ = writeHTTPMessage(w, req, "Bad method", http.StatusMethodNotAllowed)
 	return &util.JSONResponse{
 		Code: http.StatusMethodNotAllowed,
 		JSON: jsonerror.NotFound("Bad method"),
 	}
 }
 
-// checkRecaptchaEnabled creates an error response if recaptcha is not usable on homeserver.
-func checkRecaptchaEnabled(
-	cfg *config.ClientAPI,
-	w http.ResponseWriter,
-	req *http.Request,
-) *util.JSONResponse {
-	if !cfg.RecaptchaEnabled {
-		return writeHTTPMessage(w, req,
-			"Recaptcha login is disabled on this Homeserver",
-			http.StatusBadRequest,
-		)
-	}
-	return nil
-}
-
 // writeHTTPMessage writes the given header and message to the HTTP response writer.
 // Returns an error JSONResponse obtained through httputil.LogThenError if the writing failed, otherwise nil.
 func writeHTTPMessage(

clientapi/routing/auth_fallback_test.go

@@ -0,0 +1,131 @@
+package routing
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strings"
+	"testing"
+
+	"github.com/matrix-org/dendrite/clientapi/auth/authtypes"
+	"github.com/matrix-org/dendrite/setup/config"
+	"github.com/matrix-org/dendrite/test/testrig"
+)
+
+func Test_AuthFallback(t *testing.T) {
+	base, _, _ := testrig.Base(nil)
+	defer base.Close()
+
+	for _, useHCaptcha := range []bool{false, true} {
+		for _, recaptchaEnabled := range []bool{false, true} {
+			for _, wantErr := range []bool{false, true} {
+				t.Run(fmt.Sprintf("useHCaptcha(%v) - recaptchaEnabled(%v) - wantErr(%v)", useHCaptcha, recaptchaEnabled, wantErr), func(t *testing.T) {
+					// Set the defaults for each test
+					base.Cfg.ClientAPI.Defaults(config.DefaultOpts{Generate: true, Monolithic: true})
+					base.Cfg.ClientAPI.RecaptchaEnabled = recaptchaEnabled
+					base.Cfg.ClientAPI.RecaptchaPublicKey = "pub"
+					base.Cfg.ClientAPI.RecaptchaPrivateKey = "priv"
+					if useHCaptcha {
+						base.Cfg.ClientAPI.RecaptchaSiteVerifyAPI = "https://hcaptcha.com/siteverify"
+						base.Cfg.ClientAPI.RecaptchaApiJsUrl = "https://js.hcaptcha.com/1/api.js"
+						base.Cfg.ClientAPI.RecaptchaFormField = "h-captcha-response"
+						base.Cfg.ClientAPI.RecaptchaSitekeyClass = "h-captcha"
+					}
+					cfgErrs := &config.ConfigErrors{}
+					base.Cfg.ClientAPI.Verify(cfgErrs, true)
+					if len(*cfgErrs) > 0 {
+						t.Fatalf("(hCaptcha=%v) unexpected config errors: %s", useHCaptcha, cfgErrs.Error())
+					}
+
+					req := httptest.NewRequest(http.MethodGet, "/?session=1337", nil)
+					rec := httptest.NewRecorder()
+
+					AuthFallback(rec, req, authtypes.LoginTypeRecaptcha, &base.Cfg.ClientAPI)
+					if !recaptchaEnabled {
+						if rec.Code != http.StatusBadRequest {
+							t.Fatalf("unexpected response code: %d, want %d", rec.Code, http.StatusBadRequest)
+						}
+						if rec.Body.String() != "Recaptcha login is disabled on this Homeserver" {
+							t.Fatalf("unexpected response body: %s", rec.Body.String())
+						}
+					} else {
+						if !strings.Contains(rec.Body.String(), base.Cfg.ClientAPI.RecaptchaSitekeyClass) {
+							t.Fatalf("body does not contain %s: %s", base.Cfg.ClientAPI.RecaptchaSitekeyClass, rec.Body.String())
+						}
+					}
+
+					srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+						if wantErr {
+							_, _ = w.Write([]byte(`{"success":false}`))
+							return
+						}
+						_, _ = w.Write([]byte(`{"success":true}`))
+					}))
+					defer srv.Close() // nolint: errcheck
+
+					base.Cfg.ClientAPI.RecaptchaSiteVerifyAPI = srv.URL
+
+					// check the result after sending the captcha
+					req = httptest.NewRequest(http.MethodPost, "/?session=1337", nil)
+					req.Form = url.Values{}
+					req.Form.Add(base.Cfg.ClientAPI.RecaptchaFormField, "someRandomValue")
+					rec = httptest.NewRecorder()
+					AuthFallback(rec, req, authtypes.LoginTypeRecaptcha, &base.Cfg.ClientAPI)
+					if recaptchaEnabled {
+						if !wantErr {
+							if rec.Code != http.StatusOK {
+								t.Fatalf("unexpected response code: %d, want %d", rec.Code, http.StatusOK)
+							}
+							if rec.Body.String() != successTemplate {
+								t.Fatalf("unexpected response: %s, want %s", rec.Body.String(), successTemplate)
+							}
+						} else {
+							if rec.Code != http.StatusUnauthorized {
+								t.Fatalf("unexpected response code: %d, want %d", rec.Code, http.StatusUnauthorized)
+							}
+							wantString := "Authentication"
+							if !strings.Contains(rec.Body.String(), wantString) {
+								t.Fatalf("expected response to contain '%s', but didn't: %s", wantString, rec.Body.String())
+							}
+						}
+					} else {
+						if rec.Code != http.StatusBadRequest {
+							t.Fatalf("unexpected response code: %d, want %d", rec.Code, http.StatusBadRequest)
+						}
+						if rec.Body.String() != "Recaptcha login is disabled on this Homeserver" {
+							t.Fatalf("unexpected response: %s, want %s", rec.Body.String(), "successTemplate")
+						}
+					}
+				})
+			}
+		}
+	}
+
+	t.Run("unknown fallbacks are handled correctly", func(t *testing.T) {
+		req := httptest.NewRequest(http.MethodPost, "/?session=1337", nil)
+		rec := httptest.NewRecorder()
+		AuthFallback(rec, req, "DoesNotExist", &base.Cfg.ClientAPI)
+		if rec.Code != http.StatusNotImplemented {
+			t.Fatalf("unexpected http status: %d, want %d", rec.Code, http.StatusNotImplemented)
+		}
+	})
+
+	t.Run("unknown methods are handled correctly", func(t *testing.T) {
+		req := httptest.NewRequest(http.MethodDelete, "/?session=1337", nil)
+		rec := httptest.NewRecorder()
+		AuthFallback(rec, req, authtypes.LoginTypeRecaptcha, &base.Cfg.ClientAPI)
+		if rec.Code != http.StatusMethodNotAllowed {
+			t.Fatalf("unexpected http status: %d, want %d", rec.Code, http.StatusMethodNotAllowed)
+		}
+	})
+
+	t.Run("missing session parameter is handled correctly", func(t *testing.T) {
+		req := httptest.NewRequest(http.MethodGet, "/", nil)
+		rec := httptest.NewRecorder()
+		AuthFallback(rec, req, authtypes.LoginTypeRecaptcha, &base.Cfg.ClientAPI)
+		if rec.Code != http.StatusBadRequest {
+			t.Fatalf("unexpected http status: %d, want %d", rec.Code, http.StatusBadRequest)
+		}
+	})
+}

clientapi/routing/login.go

@@ -23,15 +23,13 @@ import (
 	"github.com/matrix-org/dendrite/clientapi/userutil"
 	"github.com/matrix-org/dendrite/setup/config"
 	userapi "github.com/matrix-org/dendrite/userapi/api"
-	"github.com/matrix-org/gomatrixserverlib"
 	"github.com/matrix-org/util"
 )
 
 type loginResponse struct {
-	UserID      string                       `json:"user_id"`
-	AccessToken string                       `json:"access_token"`
-	HomeServer  gomatrixserverlib.ServerName `json:"home_server"`
-	DeviceID    string                       `json:"device_id"`
+	UserID      string `json:"user_id"`
+	AccessToken string `json:"access_token"`
+	DeviceID    string `json:"device_id"`
 }
 
 type flows struct {
@@ -116,7 +114,6 @@ func completeAuth(
 		JSON: loginResponse{
 			UserID:      performRes.Device.UserID,
 			AccessToken: performRes.Device.AccessToken,
-			HomeServer:  serverName,
 			DeviceID:    performRes.Device.ID,
 		},
 	}

clientapi/routing/register.go

@@ -263,10 +263,9 @@ func newUserInteractiveResponse(
 
 // http://matrix.org/speculator/spec/HEAD/client_server/unstable.html#post-matrix-client-unstable-register
 type registerResponse struct {
-	UserID      string                       `json:"user_id"`
-	AccessToken string                       `json:"access_token,omitempty"`
-	HomeServer  gomatrixserverlib.ServerName `json:"home_server"`
-	DeviceID    string                       `json:"device_id,omitempty"`
+	UserID      string `json:"user_id"`
+	AccessToken string `json:"access_token,omitempty"`
+	DeviceID    string `json:"device_id,omitempty"`
 }
 
 // recaptchaResponse represents the HTTP response from a Google Recaptcha server
@@ -715,7 +714,6 @@ func handleGuestRegistration(
 		JSON: registerResponse{
 			UserID:      devRes.Device.UserID,
 			AccessToken: devRes.Device.AccessToken,
-			HomeServer:  res.Account.ServerName,
 			DeviceID:    devRes.Device.ID,
 		},
 	}
@@ -942,8 +940,7 @@ func completeRegistration(
 		return util.JSONResponse{
 			Code: http.StatusOK,
 			JSON: registerResponse{
-				UserID:     userutil.MakeUserID(username, accRes.Account.ServerName),
-				HomeServer: accRes.Account.ServerName,
+				UserID: userutil.MakeUserID(username, accRes.Account.ServerName),
 			},
 		}
 	}
@@ -976,7 +973,6 @@ func completeRegistration(
 	result := registerResponse{
 		UserID:      devRes.Device.UserID,
 		AccessToken: devRes.Device.AccessToken,
-		HomeServer:  accRes.Account.ServerName,
 		DeviceID:    devRes.Device.ID,
 	}
 	sessions.addCompletedRegistration(sessionID, result)