Add tests, use simple http.HandlerFunc

clientapi/routing/consent_tracking.go

@@ -24,12 +24,10 @@ import (
 	"net/http"
 
 	appserviceAPI "github.com/matrix-org/dendrite/appservice/api"
-	"github.com/matrix-org/dendrite/clientapi/jsonerror"
 	"github.com/matrix-org/dendrite/roomserver/api"
 	"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"
 	"github.com/sirupsen/logrus"
 )
 
@@ -42,9 +40,13 @@ type constentTemplateData struct {
 	ReadOnly     bool
 }
 
-func consent(writer http.ResponseWriter, req *http.Request, userAPI userapi.UserInternalAPI, cfg *config.ClientAPI) *util.JSONResponse {
+func writeHeaderAndText(w http.ResponseWriter, statusCode int) {
+	w.WriteHeader(statusCode)
+	_, _ = w.Write([]byte(http.StatusText(statusCode)))
+}
+
+func consent(writer http.ResponseWriter, req *http.Request, userAPI userapi.UserConsentPolicyAPI, cfg *config.ClientAPI) {
 	consentCfg := cfg.Matrix.UserConsentOptions
-	internalError := jsonerror.InternalServerError()
 
 	// The data used to populate the /consent request
 	data := constentTemplateData{
@@ -52,6 +54,7 @@ func consent(writer http.ResponseWriter, req *http.Request, userAPI userapi.User
 		Version:  req.FormValue("v"),
 		UserHMAC: req.FormValue("h"),
 	}
+
 	switch req.Method {
 	case http.MethodGet:
 		// display the privacy policy without a form
@@ -59,17 +62,24 @@ func consent(writer http.ResponseWriter, req *http.Request, userAPI userapi.User
 
 		// let's see if the user already consented to the current version
 		if !data.ReadOnly {
+			if ok, err := validHMAC(data.UserID, data.UserHMAC, consentCfg.FormSecret); err != nil || !ok {
+				writeHeaderAndText(writer, http.StatusForbidden)
+				return
+			}
+
 			res := &userapi.QueryPolicyVersionResponse{}
 			localpart, _, err := gomatrixserverlib.SplitID('@', data.UserID)
 			if err != nil {
 				logrus.WithError(err).Error("unable to split username")
-				return &internalError
+				writeHeaderAndText(writer, http.StatusInternalServerError)
+				return
 			}
 			if err = userAPI.QueryPolicyVersion(req.Context(), &userapi.QueryPolicyVersionRequest{
 				Localpart: localpart,
 			}, res); err != nil {
 				logrus.WithError(err).Error("unable query policy version")
-				return &internalError
+				writeHeaderAndText(writer, http.StatusInternalServerError)
+				return
 			}
 			data.HasConsented = res.PolicyVersion == consentCfg.Version
 		}
@@ -77,23 +87,24 @@ func consent(writer http.ResponseWriter, req *http.Request, userAPI userapi.User
 		err := consentCfg.Templates.ExecuteTemplate(writer, consentCfg.Version+".gohtml", data)
 		if err != nil {
 			logrus.WithError(err).Error("unable to execute consent template")
-			return nil
+			writeHeaderAndText(writer, http.StatusInternalServerError)
+			return
 		}
-		return nil
 	case http.MethodPost:
+		ok, err := validHMAC(data.UserID, data.UserHMAC, consentCfg.FormSecret)
+		if err != nil || !ok {
+			if !ok {
+				writeHeaderAndText(writer, http.StatusForbidden)
+				return
+			}
+			writeHeaderAndText(writer, http.StatusInternalServerError)
+			return
+		}
 		localpart, _, err := gomatrixserverlib.SplitID('@', data.UserID)
 		if err != nil {
 			logrus.WithError(err).Error("unable to split username")
-			return &internalError
+			writeHeaderAndText(writer, http.StatusInternalServerError)
-		}
+			return
-
-		ok, err := validHMAC(data.UserID, data.UserHMAC, consentCfg.FormSecret)
-		if err != nil || !ok {
-			_, err = writer.Write([]byte("invalid HMAC provided"))
-			if err != nil {
-				return &internalError
-			}
-			return &internalError
 		}
 		if err = userAPI.PerformUpdatePolicyVersion(
 			req.Context(),
@@ -103,11 +114,8 @@ func consent(writer http.ResponseWriter, req *http.Request, userAPI userapi.User
 			},
 			&userapi.UpdatePolicyVersionResponse{},
 		); err != nil {
-			_, err = writer.Write([]byte("unable to update database"))
+			writeHeaderAndText(writer, http.StatusInternalServerError)
-			if err != nil {
+			return
-				logrus.WithError(err).Error("unable to write to database")
-			}
-			return &internalError
 		}
 		// display the privacy policy without a form
 		data.ReadOnly = false
@@ -116,11 +124,9 @@ func consent(writer http.ResponseWriter, req *http.Request, userAPI userapi.User
 		err = consentCfg.Templates.ExecuteTemplate(writer, consentCfg.Version+".gohtml", data)
 		if err != nil {
 			logrus.WithError(err).Error("unable to print consent template")
-			return &internalError
+			writeHeaderAndText(writer, http.StatusInternalServerError)
 		}
-		return nil
 	}
-	return &util.JSONResponse{Code: http.StatusOK}
 }
 
 func sendServerNoticeForConsent(userAPI userapi.UserInternalAPI, rsAPI api.RoomserverInternalAPI,

clientapi/routing/consent_tracking_test.go

@@ -1,6 +1,18 @@
 package routing
 
-import "testing"
+import (
+	"context"
+	"fmt"
+	"html/template"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/matrix-org/dendrite/setup/config"
+	userapi "github.com/matrix-org/dendrite/userapi/api"
+)
 
 func Test_validHMAC(t *testing.T) {
 	type args struct {
@@ -20,7 +32,7 @@ func Test_validHMAC(t *testing.T) {
 			wantErr: false,
 			want:    false,
 		},
-		// $ echo -n '@alice:localhost' | openssl sha256 -hmac 'helloWorld'                                              27m ⚑ ◒  15:35:54
+		// $ echo -n '@alice:localhost' | openssl sha256 -hmac 'helloWorld'
 		//(stdin)= 121c9bab767ed87a3136db0c3002144dfe414720aa328d235199082e4757541e
 		//
 		{
@@ -32,6 +44,15 @@ func Test_validHMAC(t *testing.T) {
 			},
 			want: true,
 		},
+		{
+			name: "invalid hmac",
+			args: args{
+				username: "@bob:localhost",
+				userHMAC: "121c9bab767ed87a3136db0c3002144dfe414720aa328d235199082e4757541e",
+				secret:   "helloWorld",
+			},
+			want: false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -46,3 +67,170 @@ func Test_validHMAC(t *testing.T) {
 		})
 	}
 }
+
+type dummyAPI struct {
+	usersConsent map[string]string
+}
+
+func (d dummyAPI) QueryOutdatedPolicy(ctx context.Context, req *userapi.QueryOutdatedPolicyRequest, res *userapi.QueryOutdatedPolicyResponse) error {
+	return nil
+}
+
+func (d dummyAPI) PerformUpdatePolicyVersion(ctx context.Context, req *userapi.UpdatePolicyVersionRequest, res *userapi.UpdatePolicyVersionResponse) error {
+	d.usersConsent[req.Localpart] = req.PolicyVersion
+	return nil
+}
+
+func (d dummyAPI) QueryPolicyVersion(ctx context.Context, req *userapi.QueryPolicyVersionRequest, res *userapi.QueryPolicyVersionResponse) error {
+	res.PolicyVersion = "v2.0"
+	return nil
+}
+
+const dummyTemplate = `
+{{ if .HasConsented }}
+Consent given.
+{{ else }}
+WithoutForm
+	{{ if not .ReadOnly }}
+    With Form.
+    {{ end }}
+{{ end }}`
+
+func Test_consent(t *testing.T) {
+	type args struct {
+		username string
+		userHMAC string
+		version  string
+		method   string
+	}
+	tests := []struct {
+		name             string
+		args             args
+		wantRespCode     int
+		wantBodyContains string
+	}{
+		{
+			name: "not a userID, valid hmac",
+			args: args{
+				username: "notAuserID",
+				userHMAC: "7578bbface5ebb250a63935cebc05ca12060f58ebdbd271ecbc25e25a3da154d",
+				version:  "v1.0",
+				method:   http.MethodGet,
+			},
+			wantRespCode: http.StatusInternalServerError,
+		},
+
+		// $ echo -n '@alice:localhost' | openssl sha256 -hmac 'helloWorld'
+		//(stdin)= 121c9bab767ed87a3136db0c3002144dfe414720aa328d235199082e4757541e
+		//
+		{
+			name: "valid hmac for alice GET, not consented",
+			args: args{
+				username: "@alice:localhost",
+				userHMAC: "121c9bab767ed87a3136db0c3002144dfe414720aa328d235199082e4757541e",
+				version:  "v1.0",
+				method:   http.MethodGet,
+			},
+			wantRespCode:     http.StatusOK,
+			wantBodyContains: "With form",
+		},
+		{
+			name: "alice consents successfully",
+			args: args{
+				username: "@alice:localhost",
+				userHMAC: "121c9bab767ed87a3136db0c3002144dfe414720aa328d235199082e4757541e",
+				version:  "v1.0",
+				method:   http.MethodPost,
+			},
+			wantRespCode:     http.StatusOK,
+			wantBodyContains: "Consent given",
+		},
+		{
+			name: "valid hmac for alice GET, new version",
+			args: args{
+				username: "@alice:localhost",
+				userHMAC: "121c9bab767ed87a3136db0c3002144dfe414720aa328d235199082e4757541e",
+				version:  "v2.0",
+				method:   http.MethodGet,
+			},
+			wantRespCode:     http.StatusOK,
+			wantBodyContains: "With form",
+		},
+		{
+			name: "no hmac provided for alice, read only should be displayed",
+			args: args{
+				username: "@alice:localhost",
+				userHMAC: "",
+				version:  "v1.0",
+				method:   http.MethodGet,
+			},
+			wantRespCode:     http.StatusOK,
+			wantBodyContains: "WithoutForm",
+		},
+		{
+			name: "alice trying to get bobs status is forbidden",
+			args: args{
+				username: "@bob:localhost",
+				userHMAC: "121c9bab767ed87a3136db0c3002144dfe414720aa328d235199082e4757541e",
+				version:  "v1.0",
+				method:   http.MethodGet,
+			},
+			wantRespCode:     http.StatusForbidden,
+			wantBodyContains: "forbidden",
+		},
+		{
+			name: "alice trying to consent for bob is forbidden",
+			args: args{
+				username: "@bob:localhost",
+				userHMAC: "121c9bab767ed87a3136db0c3002144dfe414720aa328d235199082e4757541e",
+				version:  "v1.0",
+				method:   http.MethodPost,
+			},
+			wantRespCode:     http.StatusForbidden,
+			wantBodyContains: "forbidden",
+		},
+	}
+
+	userAPI := dummyAPI{
+		usersConsent: map[string]string{},
+	}
+	consentTemplates := template.Must(template.New("v1.0.gohtml").Parse(dummyTemplate))
+	consentTemplates = template.Must(consentTemplates.New("v2.0.gohtml").Parse(dummyTemplate))
+	userconsentOpts := config.UserConsentOptions{
+		FormSecret: "helloWorld",
+		Version:    "v1.0",
+		Templates:  consentTemplates,
+		BaseURL:    "http://localhost",
+	}
+	cfg := &config.ClientAPI{
+		Matrix: &config.Global{
+			UserConsentOptions: userconsentOpts,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			url := fmt.Sprintf("%s/consent?u=%s&v=%s&h=%s",
+				userconsentOpts.BaseURL, tt.args.username, tt.args.version, tt.args.userHMAC,
+			)
+
+			req := httptest.NewRequest(tt.args.method, url, nil)
+			w := httptest.NewRecorder()
+
+			consent(w, req, userAPI, cfg)
+
+			resp := w.Result()
+			body, err := ioutil.ReadAll(resp.Body)
+			if err != nil {
+				t.Fatalf("unable to read response body: %v", err)
+			}
+			defer resp.Body.Close()
+			if resp.StatusCode != tt.wantRespCode {
+				t.Fatalf("expected http %d, got %d", tt.wantRespCode, resp.StatusCode)
+			}
+
+			if !strings.Contains(strings.ToLower(string(body)), strings.ToLower(tt.wantBodyContains)) {
+				t.Fatalf("expected body to contain %s, but got %s", tt.wantBodyContains, string(body))
+			}
+		})
+	}
+}

clientapi/routing/routing.go

@@ -191,11 +191,9 @@ func Setup(
 			// start a new go routine to send messages about consent
 			go sendServerNoticeForConsent(userAPI, rsAPI, &cfg.Matrix.ServerNotices, cfg, serverNotificationSender, asAPI)
 		}
-		publicAPIMux.Handle("/consent",
+		publicAPIMux.HandleFunc("/consent", func(writer http.ResponseWriter, request *http.Request) {
-			httputil.MakeHTMLAPI("consent", func(writer http.ResponseWriter, request *http.Request) *util.JSONResponse {
+			consent(writer, request, userAPI, cfg)
-				return consent(writer, request, userAPI, cfg)
+		}).Methods(http.MethodGet, http.MethodPost, http.MethodOptions)
-			}),
-		).Methods(http.MethodGet, http.MethodPost, http.MethodOptions)
 	}
 
 	consentRequiredCheck := httputil.WithConsentCheck(cfg.Matrix.UserConsentOptions, userAPI)