Apply suggestions from code review

Co-Authored-By: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
This commit is contained in:
Parminder Singh 2019-07-20 15:15:53 +05:30
parent 10a63c0c8b
commit b727b1665d
3 changed files with 19 additions and 18 deletions

View file

@ -25,8 +25,8 @@ import (
"github.com/matrix-org/util" "github.com/matrix-org/util"
) )
// RecaptchaTemplate is template for recaptcha auth // recaptchaTemplate is an HTML webpage template for recaptcha auth
const RecaptchaTemplate = ` const recaptchaTemplate = `
<html> <html>
<head> <head>
<title>Authentication</title> <title>Authentication</title>
@ -67,8 +67,9 @@ function captchaDone() {
</html> </html>
` `
// SuccessTemplate is template for success page after auth flow ends // successTemplate is an HTML template presented to the user after successful
const SuccessTemplate = ` // recaptcha completion
const successTemplate = `
<html> <html>
<head> <head>
<title>Success!</title> <title>Success!</title>
@ -92,8 +93,8 @@ if (window.onAuthDone) {
</html> </html>
` `
// ServeTemplate fills data in template and serves it in http.ResponseWriter // serveTemplate fills template data and serves it using http.ResponseWriter
func ServeTemplate(w http.ResponseWriter, templateHTML string, data map[string]string) { func serveTemplate(w http.ResponseWriter, templateHTML string, data map[string]string) {
t := template.Must(template.New("response").Parse(templateHTML)) t := template.Must(template.New("response").Parse(templateHTML))
if err := t.Execute(w, data); err != nil { if err := t.Execute(w, data); err != nil {
panic(err) panic(err)
@ -116,21 +117,21 @@ func AuthFallback(
return nil return nil
} }
ServeRecaptcha := func() { serveRecaptcha := func() {
data := map[string]string{ data := map[string]string{
"MyUrl": req.URL.String(), "MyUrl": req.URL.String(),
"Session": sessionID, "Session": sessionID,
"SiteKey": cfg.Matrix.RecaptchaPublicKey, "SiteKey": cfg.Matrix.RecaptchaPublicKey,
} }
ServeTemplate(w, RecaptchaTemplate, data) serveTemplate(w, recaptchaTemplate, data)
} }
ServeSuccess := func() { serveSuccess := func() {
data := map[string]string{} data := map[string]string{}
ServeTemplate(w, SuccessTemplate, data) serveTemplate(w, successTemplate, data)
} }
if req.Method == "GET" { if req.Method == http.MethodGET {
// Handle Recaptcha // Handle Recaptcha
if authType == authtypes.LoginTypeRecaptcha { if authType == authtypes.LoginTypeRecaptcha {
if cfg.Matrix.RecaptchaPublicKey == "" { if cfg.Matrix.RecaptchaPublicKey == "" {
@ -142,14 +143,14 @@ func AuthFallback(
return nil return nil
} }
ServeRecaptcha() serveRecaptcha()
return nil return nil
} }
return &util.JSONResponse{ return &util.JSONResponse{
Code: 404, Code: 404,
JSON: jsonerror.NotFound("Unknown auth stage type"), JSON: jsonerror.NotFound("Unknown auth stage type"),
} }
} else if req.Method == "POST" { } else if req.Method == http.MethodPost {
clientIP := req.RemoteAddr clientIP := req.RemoteAddr
err := req.ParseForm() err := req.ParseForm()
if err != nil { if err != nil {
@ -159,14 +160,14 @@ func AuthFallback(
response := req.Form.Get("g-recaptcha-response") response := req.Form.Get("g-recaptcha-response")
if err := validateRecaptcha(&cfg, response, clientIP); err != nil { if err := validateRecaptcha(&cfg, response, clientIP); err != nil {
ServeRecaptcha() serveRecaptcha()
return nil return nil
} }
// Success. Add recaptcha as a completed login flow // Success. Add recaptcha as a completed login flow
AddCompletedSessionStage(sessionID, authtypes.LoginTypeRecaptcha) AddCompletedSessionStage(sessionID, authtypes.LoginTypeRecaptcha)
ServeSuccess() serveSuccess()
return nil return nil
} }
return &util.JSONResponse{ return &util.JSONResponse{

View file

@ -237,7 +237,7 @@ func Setup(
).Methods(http.MethodGet, http.MethodPost, http.MethodOptions) ).Methods(http.MethodGet, http.MethodPost, http.MethodOptions)
r0mux.Handle("/auth/{authType}/fallback/web", 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) vars := mux.Vars(req)
return AuthFallback(w, req, vars["authType"], cfg) return AuthFallback(w, req, vars["authType"], cfg)
}), }),

View file

@ -45,7 +45,7 @@ func MakeExternalAPI(metricsName string, f func(*http.Request) util.JSONResponse
} }
// MakeHTMLAPI adds Span metrics to the HTML Handler function // 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 { func MakeHTMLAPI(metricsName string, f func(http.ResponseWriter, *http.Request) *util.JSONResponse) http.Handler {
withSpan := func(w http.ResponseWriter, req *http.Request) { withSpan := func(w http.ResponseWriter, req *http.Request) {
span := opentracing.StartSpan(metricsName) span := opentracing.StartSpan(metricsName)