registration and login whitelisting

[cyphar]

Currently when you run lgtm it will allow anyone to log in and set up hooks with your application. This is not useful for personal services (such as the one I'm running at https://lgtm.cyphar.com/). The way I got around this problem is by adding a regular-expression whitelist of usernames that can log in or register.

From aeed56fb775923e4c1bd4ff8bea1189fc7f3160e Mon Sep 17 00:00:00 2001
From: Aleksa Sarai <[email protected]>
Date: Sat, 17 Dec 2016 13:45:55 +1100
Subject: [PATCH] web: login: add whitelist to usernames

This is a bit of a questionable patch ATM, mainly because it implies
that it is scalable to create a giant regular expression that is only
checked at /login.

Ideally this check should be done at the DB level (or at the API level)
to ensure that no API will work if you're trying to do anything with a
user not in the whitelist. And it should be configurable to be based on
organisation.

But none of that really matters for my usecase, so YOLO.

Signed-off-by: Aleksa Sarai <[email protected]>
---
 web/login.go | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/web/login.go b/web/login.go
index c6088396046d..ff31fbc643da 100644
--- a/web/login.go
+++ b/web/login.go
@@ -2,6 +2,7 @@ package web
 
 import (
 	"net/http"
+	"regexp"
 	"time"
 
 	"github.com/lgtmco/lgtm/model"
@@ -12,8 +13,25 @@ import (
 
 	log "github.com/Sirupsen/logrus"
 	"github.com/gin-gonic/gin"
+	"github.com/ianschenck/envflag"
 )
 
+var (
+	whitelist      = envflag.String("USER_WHITELIST", "", "")
+	forceWhitelist = envflag.Bool("USER_WHITELIST_FORCE", false, "")
+
+	userWhitelistRegexp *regexp.Regexp
+)
+
+func init() {
+	envflag.Parse()
+
+	if *whitelist == "" {
+		*whitelist = ".*"
+	}
+	userWhitelistRegexp = regexp.MustCompile(*whitelist)
+}
+
 // Login attempts to authorize a user via GitHub oauth2. If the user does not
 // yet exist, and new account is created. Upon successful login the user is
 // redirected to the main screen.
@@ -45,7 +63,6 @@ func Login(c *gin.Context) {
 	// get the user from the database
 	u, err := store.GetUserLogin(c, tmpuser.Login)
 	if err != nil {
-
 		// create the user account
 		u = &model.User{}
 		u.Login = tmpuser.Login
@@ -53,6 +70,13 @@ func Login(c *gin.Context) {
 		u.Avatar = tmpuser.Avatar
 		u.Secret = model.Rand()
 
+		// If someone has set a whitelist, we need to obey it.
+		if !userWhitelistRegexp.MatchString(u.Login) {
+			log.Errorf("user not in whitelist tried to register: %s", u.Login)
+			c.Redirect(303, "/login?error=not_in_whitelist")
+			return
+		}
+
 		// insert the user into the database
 		if err := store.CreateUser(c, u); err != nil {
 			log.Errorf("cannot insert %s. %s", u.Login, err)
@@ -61,6 +85,13 @@ func Login(c *gin.Context) {
 		}
 	}
 
+	// If someone has set a whitelist, we need to obey it.
+	if *forceWhitelist && !userWhitelistRegexp.MatchString(u.Login) {
+		log.Errorf("user not in whitelist tried to log in: %s", u.Login)
+		c.Redirect(303, "/login?error=not_in_whitelist")
+		return
+	}
+
 	// update the user meta data and authorization
 	// data and cache in the datastore.
 	u.Token = tmpuser.Token
@@ -99,6 +130,12 @@ func LoginToken(c *gin.Context) {
 		c.String(404, "Unable to authenticate user %s. Not registered.", user.Login)
 		return
 	}
+	// If someone has set a whitelist, we need to obey it.
+	if *forceWhitelist && !userWhitelistRegexp.MatchString(user.Login) {
+		log.Errorf("user not in whitelist tried to log in: %s", user.Login)
+		c.Redirect(303, "/login?error=not_in_whitelist")
+		return
+	}
 	exp := time.Now().Add(time.Hour * 72).Unix()
 	token := token.New(token.UserToken, user.Login)
 	tokenstr, err := token.SignExpires(user.Secret, exp)
-- 
2.11.0

This was just a quick hack for me to secure my setup, but I'm sure that there's a nicer way of handling it. For example, you could configure it so that there's some organisation that you have to be a member of in order to register or log in.

[tboerger]

Thanks for the hint, let's see how we can integrate it properly