From 94ed2d3689ea4b67c694e8875923e6a9bd4675bf Mon Sep 17 00:00:00 2001 From: Till Faelligen Date: Wed, 4 May 2022 17:57:46 +0200 Subject: [PATCH] Add tests, use simple http.HandlerFunc --- clientapi/routing/consent_tracking.go | 58 ++++--- clientapi/routing/consent_tracking_test.go | 192 ++++++++++++++++++++- clientapi/routing/routing.go | 8 +- 3 files changed, 225 insertions(+), 33 deletions(-) diff --git a/clientapi/routing/consent_tracking.go b/clientapi/routing/consent_tracking.go index a71402edd..8af8d2281 100644 --- a/clientapi/routing/consent_tracking.go +++ b/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 - } - - 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 + writeHeaderAndText(writer, http.StatusInternalServerError) + return } 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")) - if err != nil { - logrus.WithError(err).Error("unable to write to database") - } - return &internalError + writeHeaderAndText(writer, http.StatusInternalServerError) + return } // 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, diff --git a/clientapi/routing/consent_tracking_test.go b/clientapi/routing/consent_tracking_test.go index 3ddcad778..75a684eba 100644 --- a/clientapi/routing/consent_tracking_test.go +++ b/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)) + } + }) + } +} diff --git a/clientapi/routing/routing.go b/clientapi/routing/routing.go index f43ecaa80..9921e3d04 100644 --- a/clientapi/routing/routing.go +++ b/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", - httputil.MakeHTMLAPI("consent", func(writer http.ResponseWriter, request *http.Request) *util.JSONResponse { - return consent(writer, request, userAPI, cfg) - }), - ).Methods(http.MethodGet, http.MethodPost, http.MethodOptions) + publicAPIMux.HandleFunc("/consent", func(writer http.ResponseWriter, request *http.Request) { + consent(writer, request, userAPI, cfg) + }).Methods(http.MethodGet, http.MethodPost, http.MethodOptions) } consentRequiredCheck := httputil.WithConsentCheck(cfg.Matrix.UserConsentOptions, userAPI)