From b727b1665dc9c6b1b336ce4836d95d60ab85735a Mon Sep 17 00:00:00 2001 From: Parminder Singh Date: Sat, 20 Jul 2019 15:15:53 +0530 Subject: [PATCH] Apply suggestions from code review Co-Authored-By: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- clientapi/routing/auth_fallback.go | 33 +++++++++++++++--------------- clientapi/routing/routing.go | 2 +- common/httpapi.go | 2 +- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/clientapi/routing/auth_fallback.go b/clientapi/routing/auth_fallback.go index 195b891c2..9d75cc4f9 100644 --- a/clientapi/routing/auth_fallback.go +++ b/clientapi/routing/auth_fallback.go @@ -25,8 +25,8 @@ import ( "github.com/matrix-org/util" ) -// RecaptchaTemplate is template for recaptcha auth -const RecaptchaTemplate = ` +// recaptchaTemplate is an HTML webpage template for recaptcha auth +const recaptchaTemplate = ` Authentication @@ -67,8 +67,9 @@ function captchaDone() { ` -// SuccessTemplate is template for success page after auth flow ends -const SuccessTemplate = ` +// successTemplate is an HTML template presented to the user after successful +// recaptcha completion +const successTemplate = ` Success! @@ -79,7 +80,7 @@ const SuccessTemplate = ` if (window.onAuthDone) { window.onAuthDone(); } else if (window.opener && window.opener.postMessage) { - window.opener.postMessage("authDone", "*"); + window.opener.postMessage("authDone", "*"); } @@ -92,8 +93,8 @@ if (window.onAuthDone) { ` -// ServeTemplate fills data in template and serves it in http.ResponseWriter -func ServeTemplate(w http.ResponseWriter, templateHTML string, data map[string]string) { +// serveTemplate fills template data and serves it using http.ResponseWriter +func serveTemplate(w http.ResponseWriter, templateHTML string, data map[string]string) { t := template.Must(template.New("response").Parse(templateHTML)) if err := t.Execute(w, data); err != nil { panic(err) @@ -116,21 +117,21 @@ func AuthFallback( return nil } - ServeRecaptcha := func() { + serveRecaptcha := func() { data := map[string]string{ "MyUrl": req.URL.String(), "Session": sessionID, "SiteKey": cfg.Matrix.RecaptchaPublicKey, } - ServeTemplate(w, RecaptchaTemplate, data) + serveTemplate(w, recaptchaTemplate, data) } - ServeSuccess := func() { + serveSuccess := func() { data := map[string]string{} - ServeTemplate(w, SuccessTemplate, data) + serveTemplate(w, successTemplate, data) } - if req.Method == "GET" { + if req.Method == http.MethodGET { // Handle Recaptcha if authType == authtypes.LoginTypeRecaptcha { if cfg.Matrix.RecaptchaPublicKey == "" { @@ -142,14 +143,14 @@ func AuthFallback( return nil } - ServeRecaptcha() + serveRecaptcha() return nil } return &util.JSONResponse{ Code: 404, JSON: jsonerror.NotFound("Unknown auth stage type"), } - } else if req.Method == "POST" { + } else if req.Method == http.MethodPost { clientIP := req.RemoteAddr err := req.ParseForm() if err != nil { @@ -159,14 +160,14 @@ func AuthFallback( response := req.Form.Get("g-recaptcha-response") if err := validateRecaptcha(&cfg, response, clientIP); err != nil { - ServeRecaptcha() + serveRecaptcha() return nil } // Success. Add recaptcha as a completed login flow AddCompletedSessionStage(sessionID, authtypes.LoginTypeRecaptcha) - ServeSuccess() + serveSuccess() return nil } return &util.JSONResponse{ diff --git a/clientapi/routing/routing.go b/clientapi/routing/routing.go index 3a372f78a..435c11943 100644 --- a/clientapi/routing/routing.go +++ b/clientapi/routing/routing.go @@ -237,7 +237,7 @@ func Setup( ).Methods(http.MethodGet, http.MethodPost, http.MethodOptions) r0mux.Handle("/auth/{authType}/fallback/web", - common.MakeHTMLAPI("authfallback", func(w http.ResponseWriter, req *http.Request) *util.JSONResponse { + common.MakeHTMLAPI("auth_fallback", func(w http.ResponseWriter, req *http.Request) *util.JSONResponse { vars := mux.Vars(req) return AuthFallback(w, req, vars["authType"], cfg) }), diff --git a/common/httpapi.go b/common/httpapi.go index d1d7651ba..bf634ff4a 100644 --- a/common/httpapi.go +++ b/common/httpapi.go @@ -45,7 +45,7 @@ func MakeExternalAPI(metricsName string, f func(*http.Request) util.JSONResponse } // MakeHTMLAPI adds Span metrics to the HTML Handler function -// This is used to serve HTML template +// This is used to serve HTML alongside JSON error messages func MakeHTMLAPI(metricsName string, f func(http.ResponseWriter, *http.Request) *util.JSONResponse) http.Handler { withSpan := func(w http.ResponseWriter, req *http.Request) { span := opentracing.StartSpan(metricsName)