Cleanup of PR.

Updated README. Removed code duplication.

README.md

@@ -181,7 +181,7 @@ Supported options:
 | Github   | organization | If this is unset then you must whitelist individual users using `users_whitelist`. The oauth client and secrets should be issued by the specified organization. |
 | Gitlab   | allusers | Allow all valid users to get signed keys. Only allowed if siteurl set. |
 | Gitlab   | group | If `allusers` and this are unset then you must whitelist individual users using `users_whitelist`. Otherwise the user must be a member of this group. |
-| Gitlab   | siteurl | Optional. The url of the Gitlab site. Default: `https://gitlab.com/api/v3/` |
+| Gitlab   | siteurl | Optional. The url of the Gitlab site. Default: `https://gitlab.com/` |
 | Google   | domain | If this is unset then you must whitelist individual email addresses using `users_whitelist`. |
 | Microsoft | groups | Comma separated list of valid groups. |
 | Microsoft | tenant | The domain name of the Office 365 account. |

server/auth/gitlab/gitlab.go

@@ -49,7 +49,7 @@ type serviceGroupMember struct {
 	AccessLevel int    `json:"access_level"`
 }
 
-func (c *Config) logMsg(message string) {
+func (c *Config) logMsg(message error) {
 	if c.log {
 		log.Print(message)
 	}
@@ -60,26 +60,33 @@ func (c *Config) newClient(token *oauth2.Token) *http.Client {
 	return c.config.Client(oauth2.NoContext, token)
 }
 
-// Gets info on the current user.
-func (c *Config) getUser(token *oauth2.Token) *serviceUser {
+func (c *Config) getURL(token *oauth2.Token, url string) (*bytes.Buffer, error) {
 	client := c.newClient(token)
-	url := c.apiurl + "user"
 	resp, err := client.Get(url)
 	if err != nil {
-		return nil
+		return nil, fmt.Errorf("Failed to get groups: %s", err)
 	}
 	defer resp.Body.Close()
-	if resp.StatusCode != 200 {
-		if c.log {
 	var body bytes.Buffer
 	io.Copy(&body, resp.Body)
-			log.Printf("Gitlab error(http: %d) getting user: '%s'",
-				resp.StatusCode, body.String())
-			return nil
+	if resp.StatusCode != 200 {
+		return nil, fmt.Errorf("Gitlab error(http: %d) getting %s: '%s'",
+			resp.StatusCode, url, body.String())
 	}
+	return &body, nil
+}
+
+// Gets info on the current user.
+func (c *Config) getUser(token *oauth2.Token) *serviceUser {
+	url := c.apiurl + "user"
+	body, err := c.getURL(token, url)
+	if err != nil {
+		c.logMsg(err)
+		return nil
 	}
 	var user serviceUser
-	if err := json.NewDecoder(resp.Body).Decode(&user); err != nil {
+	if err := json.NewDecoder(body).Decode(&user); err != nil {
+		c.logMsg(fmt.Errorf("Failed to decode user (%s): %s", url, err))
 		return nil
 	}
 	return &user
@@ -87,27 +94,15 @@ func (c *Config) getUser(token *oauth2.Token) *serviceUser {
 
 // Gets current user group membership info.
 func (c *Config) checkGroupMembership(token *oauth2.Token, uid int, group string) bool {
-	client := c.newClient(token)
-	log.Printf("Checking group membership...")
 	url := fmt.Sprintf("%sgroups/%s/members/%d", c.apiurl, group, uid)
-	resp, err := client.Get(url)
+	body, err := c.getURL(token, url)
 	if err != nil {
-		c.logMsg(fmt.Sprintf("Failed to get groups: %s", err))
-		return false
-	}
-	defer resp.Body.Close()
-	if resp.StatusCode != 200 {
-		if c.log {
-			var body bytes.Buffer
-			io.Copy(&body, resp.Body)
-			log.Printf("Gitlab error(http: %d) getting user membership: '%s'",
-				resp.StatusCode, body.String())
+		c.logMsg(err)
 		return false
 	}
-	}
 	var m serviceGroupMember
-	if err := json.NewDecoder(resp.Body).Decode(&m); err != nil {
-		c.logMsg(fmt.Sprintf("Failed to parse groups: %s", err))
+	if err := json.NewDecoder(body).Decode(&m); err != nil {
+		c.logMsg(fmt.Errorf("Failed to parse groups (%s): %s", url, err))
 		return false
 	}
 	return m.ID == uid
@@ -180,22 +175,22 @@ func (c *Config) Valid(token *oauth2.Token) bool {
 		return false
 	}
 	if len(c.whitelist) > 0 && !c.whitelist[c.Username(token)] {
-		c.logMsg("Auth fail (not in whitelist)")
+		c.logMsg(errors.New("Auth fail (not in whitelist)"))
 		return false
 	}
 	if c.group == "" {
 		// There's no group and token is valid.  Can only reach
 		// here if user whitelist is set and user is in whitelist.
-		c.logMsg("Auth success (no groups specified in server config)")
+		c.logMsg(errors.New("Auth success (no groups specified in server config)"))
 		metrics.M.AuthValid.WithLabelValues("gitlab").Inc()
 		return true
 	}
 	if !c.checkGroupMembership(token, u.ID, c.group) {
-		c.logMsg("Auth failure (not in allowed group)")
+		c.logMsg(errors.New("Auth failure (not in allowed group)"))
 		return false
 	}
 	metrics.M.AuthValid.WithLabelValues("gitlab").Inc()
-	c.logMsg("Auth success (in allowed group)")
+	c.logMsg(errors.New("Auth success (in allowed group)"))
 	return true
 }
 

server/handlers.go

@@ -115,9 +115,7 @@ func (a *app) auth(w http.ResponseWriter, r *http.Request) {
 }
 
 func (a *app) index(w http.ResponseWriter, r *http.Request) {
-	log.Printf("Entering index handler.")
 	tok := a.getAuthToken(r)
-	log.Printf("Token found: %v\n", tok)
 	page := struct {
 		Token string
 	}{tok.AccessToken}