Prune expired auth requests + add global max limit. Closes #2490

auth/fediverse/fediverse.go

@@ -2,9 +2,13 @@ package fediverse
 
 import (
 	"crypto/rand"
+	"errors"
 	"io"
 	"strings"
+	"sync"
 	"time"
+
+	log "github.com/sirupsen/logrus"
 )
 
 // OTPRegistration represents a single OTP request.
@@ -18,19 +22,53 @@ type OTPRegistration struct {
 
 // Key by access token to limit one OTP request for a person
 // to be active at a time.
-var pendingAuthRequests = make(map[string]OTPRegistration)
+var (
+	pendingAuthRequests = make(map[string]OTPRegistration)
+	lock                = sync.Mutex{}
+)
 
-const registrationTimeout = time.Minute * 10
+const (
+	registrationTimeout = time.Minute * 10
+	maxPendingRequests  = 1000
+)
+
+func init() {
+	go setupExpiredRequestPruner()
+}
+
+// Clear out any pending requests that have been pending for greater than
+// the specified timeout value.
+func setupExpiredRequestPruner() {
+	pruneExpiredRequestsTimer := time.NewTicker(registrationTimeout)
+
+	for range pruneExpiredRequestsTimer.C {
+		lock.Lock()
+		log.Debugln("Pruning expired OTP requests.")
+		for k, v := range pendingAuthRequests {
+			if time.Since(v.Timestamp) > registrationTimeout {
+				delete(pendingAuthRequests, k)
+			}
+		}
+		lock.Unlock()
+	}
+}
 
 // RegisterFediverseOTP will start the OTP flow for a user, creating a new
 // code and returning it to be sent to a destination.
-func RegisterFediverseOTP(accessToken, userID, userDisplayName, account string) (OTPRegistration, bool) {
+func RegisterFediverseOTP(accessToken, userID, userDisplayName, account string) (OTPRegistration, bool, error) {
 	request, requestExists := pendingAuthRequests[accessToken]
 
 	// If a request is already registered and has not expired then return that
 	// existing request.
 	if requestExists && time.Since(request.Timestamp) < registrationTimeout {
-		return request, false
+		return request, false, nil
+	}
+
+	lock.Lock()
+	defer lock.Unlock()
+
+	if len(pendingAuthRequests)+1 > maxPendingRequests {
+		return request, false, errors.New("Please try again later. Too many pending requests.")
 	}
 
 	code, _ := createCode()
@@ -43,7 +81,7 @@ func RegisterFediverseOTP(accessToken, userID, userDisplayName, account string)
 	}
 	pendingAuthRequests[accessToken] = r
 
-	return r, true
+	return r, true, nil
 }
 
 // ValidateFediverseOTP will verify a OTP code for a auth request.
@@ -54,6 +92,9 @@ func ValidateFediverseOTP(accessToken, code string) (bool, *OTPRegistration) {
 		return false, nil
 	}
 
+	lock.Lock()
+	defer lock.Unlock()
+
 	delete(pendingAuthRequests, accessToken)
 	return true, &request
 }

auth/fediverse/fediverse_test.go

@@ -3,6 +3,8 @@ package fediverse
 import (
 	"strings"
 	"testing"
+
+	"github.com/owncast/owncast/utils"
 )
 
 const (
@@ -13,7 +15,10 @@ const (
 )
 
 func TestOTPFlowValidation(t *testing.T) {
-	r, success := RegisterFediverseOTP(accessToken, userID, userDisplayName, account)
+	r, success, err := RegisterFediverseOTP(accessToken, userID, userDisplayName, account)
+	if err != nil {
+		t.Error(err)
+	}
 
 	if !success {
 		t.Error("Registration should be permitted.")
@@ -50,8 +55,8 @@ func TestOTPFlowValidation(t *testing.T) {
 }
 
 func TestSingleOTPFlowRequest(t *testing.T) {
-	r1, _ := RegisterFediverseOTP(accessToken, userID, userDisplayName, account)
-	r2, s2 := RegisterFediverseOTP(accessToken, userID, userDisplayName, account)
+	r1, _, _ := RegisterFediverseOTP(accessToken, userID, userDisplayName, account)
+	r2, s2, _ := RegisterFediverseOTP(accessToken, userID, userDisplayName, account)
 
 	if r1.Code != r2.Code {
 		t.Error("Only one registration should be permitted.")
@@ -65,14 +70,42 @@ func TestSingleOTPFlowRequest(t *testing.T) {
 func TestAccountCaseInsensitive(t *testing.T) {
 	account := "Account"
 	accessToken := "another-fake-access-token"
-	r1, _ := RegisterFediverseOTP(accessToken, userID, userDisplayName, account)
+	r1, _, _ := RegisterFediverseOTP(accessToken, userID, userDisplayName, account)
 	_, reg1 := ValidateFediverseOTP(accessToken, r1.Code)
 
 	// Simulate second auth with account in different case
-	r2, _ := RegisterFediverseOTP(accessToken, userID, userDisplayName, strings.ToUpper(account))
+	r2, _, _ := RegisterFediverseOTP(accessToken, userID, userDisplayName, strings.ToUpper(account))
 	_, reg2 := ValidateFediverseOTP(accessToken, r2.Code)
 
 	if reg1.Account != reg2.Account {
 		t.Errorf("Account names should be case-insensitive: %s %s", reg1.Account, reg2.Account)
 	}
 }
+
+func TestLimitGlobalPendingRequests(t *testing.T) {
+	for i := 0; i < maxPendingRequests-1; i++ {
+		at, _ := utils.GenerateRandomString(10)
+		uid, _ := utils.GenerateRandomString(10)
+		account, _ := utils.GenerateRandomString(10)
+
+		_, success, error := RegisterFediverseOTP(at, uid, "userDisplayName", account)
+		if !success {
+			t.Error("Registration should be permitted.", i, " of ", len(pendingAuthRequests))
+		}
+		if error != nil {
+			t.Error(error)
+		}
+	}
+
+	// This one should fail
+	at, _ := utils.GenerateRandomString(10)
+	uid, _ := utils.GenerateRandomString(10)
+	account, _ := utils.GenerateRandomString(10)
+	_, success, error := RegisterFediverseOTP(at, uid, "userDisplayName", account)
+	if success {
+		t.Error("Registration should not be permitted.")
+	}
+	if error == nil {
+		t.Error("Error should be returned.")
+	}
+}