From 466c1d3058cc853e1f3d9005298cd35ad0f70cf3 Mon Sep 17 00:00:00 2001
From: ale <ale@incal.net>
Date: Fri, 21 Jun 2019 09:20:03 +0100
Subject: [PATCH] Always use query args for the / endpoint parameters

This allows eventual future usage of 307 redirects and us accepting
POST requests without having to decode the request body.
---
 server/http.go | 51 +++++++++++++++++++++++++++++---------------------
 1 file changed, 30 insertions(+), 21 deletions(-)

diff --git a/server/http.go b/server/http.go
index 73261fa..6161cb2 100644
--- a/server/http.go
+++ b/server/http.go
@@ -262,27 +262,17 @@ func (h *Server) withAuth(f func(http.ResponseWriter, *http.Request, *authSessio
 // the original service, with the signed token.
 func (h *Server) handleHomepage(w http.ResponseWriter, req *http.Request, session *authSession) {
 	// Extract the authorization request parameters from the HTTP
-	// request.
+	// request query args.
+	//
+	// *NOTE*: we do not want to parse the request body, in case
+	// it is a POST request redirected from a 307, so we do not
+	// call req.FormValue() but look directly into request.URL
+	// instead.
 	username := session.Username
-	service := req.FormValue("s")
-	destination := req.FormValue("d")
-	nonce := req.FormValue("n")
-	var groups, reqGroups []string
-	if gstr := req.FormValue("g"); gstr != "" {
-		reqGroups = strings.Split(gstr, ",")
-		if len(reqGroups) > 0 && session.UserInfo != nil {
-			groups = intersectGroups(reqGroups, session.UserInfo.Groups)
-			// We only make this check here as a convenience to
-			// the user (we may be able to show a nicer UI): the
-			// actual group ACL must be applied on the destination
-			// service, because the 'g' parameter is untrusted at
-			// this stage.
-			if len(groups) == 0 {
-				http.Error(w, "Forbidden", http.StatusForbidden)
-				return
-			}
-		}
-	}
+	service := req.URL.Query().Get("s")
+	destination := req.URL.Query().Get("d")
+	nonce := req.URL.Query().Get("n")
+	groupsStr := req.URL.Query().Get("g")
 
 	// If the above parameters are unset, we're probably faced with a user
 	// that reached this URL by other means. Redirect them to the
@@ -297,10 +287,29 @@ func (h *Server) handleHomepage(w http.ResponseWriter, req *http.Request, sessio
 		return
 	}
 
+	// Compute the intersection of the user's groups and the
+	// requested groups, to obtain the group memberships to grant.
+	var groups []string
+	if groupsStr != "" {
+		reqGroups := strings.Split(groupsStr, ",")
+		if len(reqGroups) > 0 && session.UserInfo != nil {
+			groups = intersectGroups(reqGroups, session.UserInfo.Groups)
+			// We only make this check here as a convenience to
+			// the user (we may be able to show a nicer UI): the
+			// actual group ACL must be applied on the destination
+			// service, because the 'g' parameter is untrusted at
+			// this stage.
+			if len(groups) == 0 {
+				http.Error(w, "Forbidden", http.StatusForbidden)
+				return
+			}
+		}
+	}
+
 	// Make the authorization request.
 	token, err := h.loginService.Authorize(username, service, destination, nonce, groups)
 	if err != nil {
-		log.Printf("auth error: %v: user=%s service=%s destination=%s nonce=%s groups=%s", err, username, service, destination, nonce, req.FormValue("g"))
+		log.Printf("auth error: %v: user=%s service=%s destination=%s nonce=%s groups=%s", err, username, service, destination, nonce, groupsStr)
 		http.Error(w, err.Error(), http.StatusBadRequest)
 		return
 	}
-- 
GitLab