Repository: supabase/gotrue
Pull request #641 (2022-08-28 12:53:15)

feat: Add provider_refresh_token query fragment
## What kind of change does this PR introduce?

* Retrieves the `ProviderRefreshToken` from the provider's OAuth response
* Pass the `ProviderRefreshToken` as `provider_refresh_token` in the query fragments when redirecting to SITE_URL

## What is the current behavior?

Currently the provider's refresh token is completely ignored and never passed to the client.

## What is the new behavior?

The provider's refresh token is now passed, **when it exists**, to the client as `provider_refresh_token` along the `provider_token`.

## Additional context

This feature should be backwards compatible, for a more coherent approach it could be possible to rename the current `provider_token` to `provider_access_token` but it would probably break current integrations.

Some of the OAuth providers have an expiry date on the access token they provide (e.g: Twitch). By passing the provider refresh token it allows developers to renew the provider token and build more easily and seamlessly on top of the provider's API.

This is a feature that has been requested last year already : https://github.com/supabase/gotrue-js/issues/131

Some of the providers don't pass a refresh_token because their access_token doesn't expire (example: Github Apps have a refresh token but Github OAuth apps don't). In this case I have added an if condition to not pass the refresh_token.



Patch:
diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
deleted file mode 100644
index d157515b..00000000
--- a/.github/CODEOWNERS
+++ /dev/null
@@ -1,1 +0,0 @@
-*   @netlify/opensource @netlify/backend
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
index d918fe07..2d08ff59 100644
--- a/.github/workflows/release.yml
+++ b/.github/workflows/release.yml
@@ -4,7 +4,9 @@ on:
   workflow_run:
     workflows: [Test]
     types: [completed]
-    branches: [master]
+    branches:
+      - master
+      - mfa
 
 jobs:
   release:
diff --git a/.releaserc b/.releaserc
index 32f0e451..9a1cc36c 100644
--- a/.releaserc
+++ b/.releaserc
@@ -1,6 +1,10 @@
 {
   "branches": [
-    "master"
+    "master",
+    {
+        "name": "mfa",
+        "prerelease": true
+    }
   ],
   "plugins": [
     "@semantic-release/commit-analyzer",
diff --git a/Makefile b/Makefile
index 68e6a061..48c885f0 100644
--- a/Makefile
+++ b/Makefile
@@ -15,6 +15,7 @@ build: deps ## Build the binary.
 dev-deps: deps ## Install developer dependencies
 	@go install github.com/gobuffalo/pop/soda@latest
 	@go install github.com/securego/gosec/v2/cmd/gosec@latest
+	@go install honnef.co/go/tools/cmd/staticcheck@latest
 
 deps: ## Install dependencies.
 	@go mod download
@@ -33,7 +34,17 @@ vet: # Vet the code
 	go vet $(CHECK_FILES)
 
 sec: # Check for security vulnerabilities
-	gosec -tests -exclude=G104 $(CHECK_FILES)
+	gosec -quiet $(CHECK_FILES)
+	gosec -quiet -tests -exclude=G104 $(CHECK_FILES)
+
+unused: # Look for unused code
+	@echo "Unused code:"
+	staticcheck -checks U1000 $(CHECK_FILES)
+	
+	@echo
+	
+	@echo "Code used only in _test.go (do move it in those files):"
+	staticcheck -checks U1000 -tests=false $(CHECK_FILES)
 
 dev: ## Run the development containers
 	docker-compose -f $(DEV_DOCKER_COMPOSE) up
diff --git a/api/api.go b/api/api.go
index ad9cce9f..626e34f3 100644
--- a/api/api.go
+++ b/api/api.go
@@ -2,6 +2,7 @@ package api
 
 import (
 	"context"
+	"errors"
 	"net/http"
 	"os"
 	"os/signal"
@@ -51,7 +52,10 @@ func (a *API) ListenAndServe(hostAndPort string) {
 		waitForTermination(log, done)
 		ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
 		defer cancel()
-		server.Shutdown(ctx)
+		err := server.Shutdown(ctx)
+		if err != nil && !errors.Is(err, context.Canceled) && errors.Is(err, context.DeadlineExceeded) {
+			log.WithError(err).Error("Shutdown failed")
+		}
 	}()
 
 	if err := server.ListenAndServe(); err != http.ErrServerClosed {
@@ -85,10 +89,6 @@ func (a *API) deprecationNotices(ctx context.Context) {
 		log.Warn("DEPRECATION NOTICE: GOTRUE_JWT_ADMIN_GROUP_NAME not supported by Supabase's GoTrue, will be removed soon")
 	}
 
-	if len(config.JWT.AdminRoles) > 0 {
-		log.Warn("DEPRECATION NOTICE: GOTRUE_JWT_ADMIN_ROLES not supported by Supabase's GoTrue, will be removed soon")
-	}
-
 	if config.JWT.DefaultGroupName != "" {
 		log.Warn("DEPRECATION NOTICE: GOTRUE_JWT_DEFAULT_GROUP_NAME not supported by Supabase's GoTrue, will be removed soon")
 	}
diff --git a/api/context.go b/api/context.go
index 1543b5ba..394e701e 100644
--- a/api/context.go
+++ b/api/context.go
@@ -99,16 +99,6 @@ func withSignature(ctx context.Context, id string) context.Context {
 	return context.WithValue(ctx, signatureKey, id)
 }
 
-// getSignature reads the request ID from the context.
-func getSignature(ctx context.Context) string {
-	obj := ctx.Value(signatureKey)
-	if obj == nil {
-		return ""
-	}
-
-	return obj.(string)
-}
-
 func withInviteToken(ctx context.Context, token string) context.Context {
 	return context.WithValue(ctx, inviteTokenKey, token)
 }
@@ -150,11 +140,6 @@ func getExternalReferrer(ctx context.Context) string {
 	return obj.(string)
 }
 
-// withFunctionHooks adds the provided function hooks to the context.
-func withFunctionHooks(ctx context.Context, hooks map[string][]string) context.Context {
-	return context.WithValue(ctx, functionHooksKey, hooks)
-}
-
 // getFunctionHooks reads the request ID from the context.
 func getFunctionHooks(ctx context.Context) map[string][]string {
 	obj := ctx.Value(functionHooksKey)
diff --git a/api/errors.go b/api/errors.go
index e3e619c9..ed5f24f5 100644
--- a/api/errors.go
+++ b/api/errors.go
@@ -98,10 +98,6 @@ func notFoundError(fmtString string, args ...interface{}) *HTTPError {
 	return httpError(http.StatusNotFound, fmtString, args...)
 }
 
-func acceptedTokenError(fmtString string, args ...interface{}) *HTTPError {
-	return httpError(http.StatusAccepted, fmtString, args...)
-}
-
 func expiredTokenError(fmtString string, args ...interface{}) *HTTPError {
 	return httpError(http.StatusUnauthorized, fmtString, args...)
 }
diff --git a/api/external.go b/api/external.go
index ed84659d..e005fb4e 100644
--- a/api/external.go
+++ b/api/external.go
@@ -109,7 +109,8 @@ func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Re
 
 	providerType := getExternalProviderType(ctx)
 	var userData *provider.UserProvidedData
-	var providerToken string
+	var providerAccessToken string
+	var providerRefreshToken string = ""
 	if providerType == "twitter" {
 		// future OAuth1.0 providers will use this method
 		oAuthResponseData, err := a.oAuth1Callback(ctx, r, providerType)
@@ -117,14 +118,15 @@ func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Re
 			return err
 		}
 		userData = oAuthResponseData.userData
-		providerToken = oAuthResponseData.token
+		providerAccessToken = oAuthResponseData.token
 	} else {
 		oAuthResponseData, err := a.oAuthCallback(ctx, r, providerType)
 		if err != nil {
 			return err
 		}
 		userData = oAuthResponseData.userData
-		providerToken = oAuthResponseData.token
+		providerAccessToken = oAuthResponseData.token
+		providerRefreshToken = oAuthResponseData.refreshToken
 	}
 
 	var user *models.User
@@ -286,7 +288,12 @@ func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Re
 	rurl := a.getExternalRedirectURL(r)
 	if token != nil {
 		q := url.Values{}
-		q.Set("provider_token", providerToken)
+		q.Set("provider_token", providerAccessToken)
+		// Because not all providers give out a refresh token
+		// See corresponding OAuth2 spec: <https://www.rfc-editor.org/rfc/rfc6749.html#section-5.1>
+		if providerRefreshToken != "" {
+			q.Set("provider_refresh_token", providerRefreshToken)
+		}
 		q.Set("access_token", token.Token)
 		q.Set("token_type", token.TokenType)
 		q.Set("expires_in", strconv.Itoa(token.ExpiresIn))
diff --git a/api/external_oauth.go b/api/external_oauth.go
index dc645fff..d00d9836 100644
--- a/api/external_oauth.go
+++ b/api/external_oauth.go
@@ -14,8 +14,9 @@ import (
 
 // OAuthProviderData contains the userData and token returned by the oauth provider
 type OAuthProviderData struct {
-	userData *provider.UserProvidedData
-	token    string
+	userData     *provider.UserProvidedData
+	token        string
+	refreshToken string
 }
 
 // loadOAuthState parses the `state` query parameter as a JWS payload,
@@ -96,8 +97,9 @@ func (a *API) oAuthCallback(ctx context.Context, r *http.Request, providerType s
 	}
 
 	return &OAuthProviderData{
-		userData: userData,
-		token:    token.AccessToken,
+		userData:     userData,
+		token:        token.AccessToken,
+		refreshToken: token.RefreshToken,
 	}, nil
 }
 
@@ -134,8 +136,9 @@ func (a *API) oAuth1Callback(ctx context.Context, r *http.Request, providerType
 	}
 
 	return &OAuthProviderData{
-		userData: userData,
-		token:    accessToken.Token,
+		userData:     userData,
+		token:        accessToken.Token,
+		refreshToken: "",
 	}, nil
 
 }
diff --git a/api/helpers.go b/api/helpers.go
index 55324f45..aaa5ca13 100644
--- a/api/helpers.go
+++ b/api/helpers.go
@@ -176,24 +176,6 @@ func isPrivateIP(ip net.IP) bool {
 	return false
 }
 
-func removeLocalhostFromPrivateIPBlock() *net.IPNet {
-	_, localhost, _ := net.ParseCIDR("127.0.0.0/8")
-
-	var localhostIndex int
-	for i := 0; i < len(privateIPBlocks); i++ {
-		if privateIPBlocks[i] == localhost {
-			localhostIndex = i
-		}
-	}
-	privateIPBlocks = append(privateIPBlocks[:localhostIndex], privateIPBlocks[localhostIndex+1:]...)
-
-	return localhost
-}
-
-func unshiftPrivateIPBlock(address *net.IPNet) {
-	privateIPBlocks = append([]*net.IPNet{address}, privateIPBlocks...)
-}
-
 type noLocalTransport struct {
 	inner  http.RoundTripper
 	errlog logrus.FieldLogger
diff --git a/api/hooks.go b/api/hooks.go
index 3ab7a825..66b34999 100644
--- a/api/hooks.go
+++ b/api/hooks.go
@@ -48,7 +48,6 @@ type Webhook struct {
 	jwtSecret string
 	claims    jwt.Claims
 	payload   []byte
-	headers   map[string]string
 }
 
 type WebhookResponse struct {
@@ -148,7 +147,9 @@ func (w *Webhook) generateSignature() (string, error) {
 
 func closeBody(rsp *http.Response) {
 	if rsp != nil && rsp.Body != nil {
-		rsp.Body.Close()
+		if err := rsp.Body.Close(); err != nil {
+			logrus.WithError(err).Warn("body close in hooks failed")
+		}
 	}
 }
 
@@ -231,11 +232,9 @@ func triggerHook(ctx context.Context, hookURL *url.URL, secret string, conn *sto
 	w.URL = hookURL.String()
 
 	body, err := w.trigger()
-	defer func() {
-		if body != nil {
-			body.Close()
-		}
-	}()
+	if body != nil {
+		defer body.Close()
+	}
 	if err == nil && body != nil {
 		webhookRsp := &WebhookResponse{}
 		decoder := json.NewDecoder(body)
diff --git a/api/mail.go b/api/mail.go
index cace797f..f2fbff9f 100644
--- a/api/mail.go
+++ b/api/mail.go
@@ -19,7 +19,6 @@ import (
 
 var (
 	MaxFrequencyLimitError error = errors.New("Frequency limit reached")
-	configFile                   = ""
 )
 
 type GenerateLinkParams struct {
diff --git a/api/middleware.go b/api/middleware.go
index a7122dee..69c76701 100644
--- a/api/middleware.go
+++ b/api/middleware.go
@@ -1,10 +1,8 @@
 package api
 
 import (
-	"bytes"
 	"context"
 	"encoding/json"
-	"io/ioutil"
 	"net/http"
 	"strings"
 	"time"
@@ -74,15 +72,13 @@ func (a *API) limitEmailSentHandler() middlewareHandler {
 		if config.External.Email.Enabled && !config.Mailer.Autoconfirm {
 			if req.Method == "PUT" || req.Method == "POST" {
 				res := make(map[string]interface{})
-				bodyBytes, err := ioutil.ReadAll(req.Body)
+
+				bodyBytes, err := getBodyBytes(req)
 				if err != nil {
 					return c, internalServerError("Error invalid request body").WithInternalError(err)
 				}
-				req.Body.Close()
-				req.Body = ioutil.NopCloser(bytes.NewBuffer(bodyBytes))
 
-				jsonDecoder := json.NewDecoder(bytes.NewBuffer(bodyBytes))
-				if err := jsonDecoder.Decode(&res); err != nil {
+				if err := json.Unmarshal(bodyBytes, &res); err != nil {
 					return c, badRequestError("Error invalid request body").WithInternalError(err)
 				}
 
diff --git a/api/provider/azure.go b/api/provider/azure.go
index 760660df..547bf239 100644
--- a/api/provider/azure.go
+++ b/api/provider/azure.go
@@ -25,12 +25,6 @@ type azureUser struct {
 	Sub   string `json:"sub"`
 }
 
-type azureEmail struct {
-	Email    string `json:"email"`
-	Primary  bool   `json:"is_primary"`
-	Verified bool   `json:"is_confirmed"`
-}
-
 // NewAzureProvider creates a Azure account provider.
 func NewAzureProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAuthProvider, error) {
 	if err := ext.Validate(); err != nil {
diff --git a/api/provider/notion.go b/api/provider/notion.go
index db8f84d0..c7179a65 100644
--- a/api/provider/notion.go
+++ b/api/provider/notion.go
@@ -79,18 +79,23 @@ func (g notionProvider) GetUserData(ctx context.Context, tok *oauth2.Token) (*Us
 
 	client := &http.Client{Timeout: defaultTimeout}
 	resp, err := client.Do(req)
-
 	if err != nil {
 		return nil, err
 	}
+	defer resp.Body.Close()
 
 	if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices {
 		return nil, fmt.Errorf("a %v error occurred with retrieving user from notion", resp.StatusCode)
 	}
 
-	body, _ := ioutil.ReadAll(resp.Body)
-	json.Unmarshal(body, &u)
-	defer resp.Body.Close()
+	body, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		return nil, err
+	}
+	err = json.Unmarshal(body, &u)
+	if err != nil {
+		return nil, err
+	}
 
 	if u.Bot.Owner.User.Person.Email == "" {
 		return nil, errors.New("unable to find email with notion provider")
diff --git a/api/provider/twitch.go b/api/provider/twitch.go
index 517109c7..712f7d01 100644
--- a/api/provider/twitch.go
+++ b/api/provider/twitch.go
@@ -94,18 +94,24 @@ func (t twitchProvider) GetUserData(ctx context.Context, tok *oauth2.Token) (*Us
 
 	client := &http.Client{Timeout: defaultTimeout}
 	resp, err := client.Do(req)
-
 	if err != nil {
 		return nil, err
 	}
+	defer resp.Body.Close()
 
 	if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices {
 		return nil, fmt.Errorf("a %v error occurred with retrieving user from twitch", resp.StatusCode)
 	}
 
-	body, _ := ioutil.ReadAll(resp.Body)
-	json.Unmarshal(body, &u)
-	defer resp.Body.Close()
+	body, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		return nil, err
+	}
+
+	err = json.Unmarshal(body, &u)
+	if err != nil {
+		return nil, err
+	}
 
 	if len(u.Data) == 0 {
 		return nil, errors.New("unable to find user with twitch provider")
diff --git a/api/token.go b/api/token.go
index c6f36768..cd535670 100644
--- a/api/token.go
+++ b/api/token.go
@@ -59,18 +59,7 @@ type IdTokenGrantParams struct {
 	Issuer   string `json:"issuer"`
 }
 
-type tokenType string
-
-const (
-	confirmationToken       tokenType = "confirmation_token"
-	recoveryToken           tokenType = "recovery_token"
-	emailChangeTokenNew     tokenType = "email_change_token_new"
-	emailChangeTokenCurrent tokenType = "email_change_token_current"
-	reauthenticationToken   tokenType = "reauthentication_token"
-)
-
 const useCookieHeader = "x-use-cookie"
-const useSessionCookie = "session"
 const InvalidLoginMessage = "Invalid login credentials"
 
 func (p *IdTokenGrantParams) getVerifier(ctx context.Context, config *conf.GlobalConfiguration) (*oidc.IDTokenVerifier, error) {
@@ -567,7 +556,7 @@ func (a *API) issueRefreshToken(ctx context.Context, conn *storage.Connection, u
 
 	err := conn.Transaction(func(tx *storage.Connection) error {
 		var terr error
-		refreshToken, terr = models.GrantAuthenticatedUser(tx, user)
+		refreshToken, terr = models.GrantAuthenticatedUser(tx, user, models.GrantParams{})
 		if terr != nil {
 			return internalServerError("Database error granting user").WithInternalError(terr)
 		}
diff --git a/conf/configuration.go b/conf/configuration.go
index 38e2ae7a..d87b41b1 100644
--- a/conf/configuration.go
+++ b/conf/configuration.go
@@ -2,6 +2,7 @@ package conf
 
 import (
 	"errors"
+	"net/url"
 	"os"
 	"time"
 
@@ -36,6 +37,10 @@ type DBConfiguration struct {
 	MigrationsPath string `json:"migrations_path" split_words:"true" default:"./migrations"`
 }
 
+func (c *DBConfiguration) Validate() error {
+	return nil
+}
+
 // JWTConfiguration holds all the JWT related configuration.
 type JWTConfiguration struct {
 	Secret           string   `json:"secret" required:"true"`
@@ -46,15 +51,30 @@ type JWTConfiguration struct {
 	DefaultGroupName string   `json:"default_group_name" split_words:"true"`
 }
 
+type APIConfiguration struct {
+	Host            string
+	Port            int `envconfig:"PORT" default:"8081"`
+	Endpoint        string
+	RequestIDHeader string `envconfig:"REQUEST_ID_HEADER"`
+	ExternalURL     string `json:"external_url" envconfig:"API_EXTERNAL_URL"`
+}
+
+func (a *APIConfiguration) Validate() error {
+	if a.ExternalURL != "" {
+		// sometimes, in tests, ExternalURL is empty and we regard that
+		// as a valid value
+		_, err := url.ParseRequestURI(a.ExternalURL)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 // GlobalConfiguration holds all the configuration that applies to all instances.
 type GlobalConfiguration struct {
-	API struct {
-		Host            string
-		Port            int `envconfig:"PORT" default:"8081"`
-		Endpoint        string
-		RequestIDHeader string `envconfig:"REQUEST_ID_HEADER"`
-		ExternalURL     string `json:"external_url" envconfig:"API_EXTERNAL_URL"`
-	}
+	API                   APIConfiguration
 	DB                    DBConfiguration
 	External              ProviderConfiguration
 	Logging               LoggingConfig `envconfig:"LOG"`
@@ -127,6 +147,10 @@ type SMTPConfiguration struct {
 	SenderName   string        `json:"sender_name" split_words:"true"`
 }
 
+func (c *SMTPConfiguration) Validate() error {
+	return nil
+}
+
 type MailerConfiguration struct {
 	Autoconfirm              bool                      `json:"autoconfirm"`
 	Subjects                 EmailContentConfiguration `json:"subjects"`
@@ -231,7 +255,13 @@ func LoadGlobal(filename string) (*GlobalConfiguration, error) {
 		return nil, err
 	}
 
-	config.ApplyDefaults()
+	if err := config.ApplyDefaults(); err != nil {
+		return nil, err
+	}
+
+	if err := config.Validate(); err != nil {
+		return nil, err
+	}
 
 	if _, err := ConfigureLogging(&config.Logging); err != nil {
 		return nil, err
@@ -239,14 +269,11 @@ func LoadGlobal(filename string) (*GlobalConfiguration, error) {
 
 	ConfigureTracing(&config.Tracing)
 
-	if config.SMTP.MaxFrequency == 0 {
-		config.SMTP.MaxFrequency = 1 * time.Minute
-	}
 	return config, nil
 }
 
 // ApplyDefaults sets defaults for a GlobalConfiguration
-func (config *GlobalConfiguration) ApplyDefaults() {
+func (config *GlobalConfiguration) ApplyDefaults() error {
 	if config.JWT.AdminGroupName == "" {
 		config.JWT.AdminGroupName = "admin"
 	}
@@ -320,6 +347,7 @@ func (config *GlobalConfiguration) ApplyDefaults() {
 	if config.URIAllowList == nil {
 		config.URIAllowList = []string{}
 	}
+
 	if config.URIAllowList != nil {
 		config.URIAllowListMap = make(map[string]glob.Glob)
 		for _, uri := range config.URIAllowList {
@@ -327,9 +355,32 @@ func (config *GlobalConfiguration) ApplyDefaults() {
 			config.URIAllowListMap[uri] = g
 		}
 	}
+
 	if config.PasswordMinLength < defaultMinPasswordLength {
 		config.PasswordMinLength = defaultMinPasswordLength
 	}
+
+	return nil
+}
+
+// Validate validates all of configuration.
+func (c *GlobalConfiguration) Validate() error {
+	validatables := []interface {
+		Validate() error
+	}{
+		&c.API,
+		&c.DB,
+		&c.Tracing,
+		&c.SMTP,
+	}
+
+	for _, validatable := range validatables {
+		if err := validatable.Validate(); err != nil {
+			return err
+		}
+	}
+
+	return nil
 }
 
 func (o *OAuthProviderConfiguration) Validate() error {
diff --git a/conf/tracing.go b/conf/tracing.go
index 8f51ffe9..ad01540d 100644
--- a/conf/tracing.go
+++ b/conf/tracing.go
@@ -16,6 +16,10 @@ type TracingConfig struct {
 	Tags        map[string]string
 }
 
+func (tc *TracingConfig) Validate() error {
+	return nil
+}
+
 func (tc *TracingConfig) tracingAddr() string {
 	return fmt.Sprintf("%s:%s", tc.Host, tc.Port)
 }
diff --git a/crypto/crypto.go b/crypto/crypto.go
index a6af6d8c..6912a75b 100644
--- a/crypto/crypto.go
+++ b/crypto/crypto.go
@@ -33,22 +33,3 @@ func GenerateOtp(digits int) (string, error) {
 	otp := fmt.Sprintf(expr, val.String())
 	return otp, nil
 }
-
-// GenerateOtpFromCharset generates a random n-length otp from a charset
-func GenerateOtpFromCharset(length int, charset string) (string, error) {
-	b := make([]byte, length)
-	for i := range b {
-		val, err := rand.Int(rand.Reader, big.NewInt(int64(len(charset))))
-		if err != nil {
-			return "", errors.WithMessage(err, "Error generating otp from charset")
-		}
-		b[i] = charset[val.Int64()]
-	}
-	return string(b), nil
-}
-
-// GenerateEmailOtp generates a random n-length alphanumeric otp
-func GenerateEmailOtp(length int) (string, error) {
-	const charset = "abcdefghijklmnopqrstuvwxyz"
-	return GenerateOtpFromCharset(length, charset)
-}
diff --git a/logger/log.go b/logger/log.go
index 8f5c14a9..705acfb4 100644
--- a/logger/log.go
+++ b/logger/log.go
@@ -6,6 +6,7 @@ import (
 	"time"
 
 	chimiddleware "github.com/go-chi/chi/middleware"
+	"github.com/netlify/gotrue/utilities"
 	"github.com/sirupsen/logrus"
 )
 
@@ -26,7 +27,7 @@ func (l *structuredLogger) NewLogEntry(r *http.Request) chimiddleware.LogEntry {
 		"component":   "api",
 		"method":      r.Method,
 		"path":        r.URL.Path,
-		"remote_addr": r.RemoteAddr,
+		"remote_addr": utilities.GetIPAddress(r),
 		"referer":     r.Referer(),
 		"timestamp":   time.Now().UTC().Format(time.RFC3339),
 	}
diff --git a/mailer/mailer.go b/mailer/mailer.go
index 0d63b9da..266944f3 100644
--- a/mailer/mailer.go
+++ b/mailer/mailer.go
@@ -2,7 +2,6 @@ package mailer
 
 import (
 	"net/url"
-	"regexp"
 
 	"github.com/netlify/gotrue/conf"
 	"github.com/netlify/gotrue/models"
@@ -79,9 +78,3 @@ func getSiteURL(referrerURL, siteURL, filepath, fragment string) (string, error)
 	site.RawQuery = fragment
 	return site.String(), nil
 }
-
-var urlRegexp = regexp.MustCompile(`^https?://[^/]+`)
-
-func enforceRelativeURL(url string) string {
-	return urlRegexp.ReplaceAllString(url, "")
-}
diff --git a/mailer/noop.go b/mailer/noop.go
index b35a7af1..203c19af 100644
--- a/mailer/noop.go
+++ b/mailer/noop.go
@@ -2,46 +2,8 @@ package mailer
 
 import (
 	"errors"
-
-	"github.com/netlify/gotrue/models"
 )
 
-type noopMailer struct {
-	Mailer MailClient
-}
-
-func (m noopMailer) ValidateEmail(email string) error {
-	return nil
-}
-
-func (m *noopMailer) InviteMail(user *models.User, referrerURL string) error {
-	return nil
-}
-
-func (m *noopMailer) ConfirmationMail(user *models.User, referrerURL string) error {
-	return nil
-}
-
-func (m noopMailer) RecoveryMail(user *models.User, referrerURL string) error {
-	return nil
-}
-
-func (m noopMailer) MagicLinkMail(user *models.User, referrerURL string) error {
-	return nil
-}
-
-func (m *noopMailer) EmailChangeMail(user *models.User, referrerURL string) error {
-	return nil
-}
-
-func (m noopMailer) Send(user *models.User, subject, body string, data map[string]interface{}) error {
-	return nil
-}
-
-func (m noopMailer) GetEmailActionLink(user *models.User, actionType, referrerURL string) (string, error) {
-	return "", nil
-}
-
 type noopMailClient struct{}
 
 func (m *noopMailClient) Mail(to, subjectTemplate, templateURL, defaultTemplate string, templateData map[string]interface{}) error {
diff --git a/mailer/template.go b/mailer/template.go
index c60dc86e..9555bfe3 100644
--- a/mailer/template.go
+++ b/mailer/template.go
@@ -2,7 +2,6 @@ package mailer
 
 import (
 	"fmt"
-	"strings"
 
 	"github.com/badoux/checkmail"
 	"github.com/netlify/gotrue/conf"
@@ -329,17 +328,3 @@ func (m TemplateMailer) GetEmailActionLink(user *models.User, actionType, referr
 	}
 	return url, nil
 }
-
-// formatEmailOtp separates the otp into chunks of 5 with "-" as the separator
-func formatEmailOtp(otp string) string {
-	chunkSize := 5
-	var chunks []string
-	for i := 0; i < len(otp); i += chunkSize {
-		if i+chunkSize >= len(otp) {
-			chunks = append(chunks, otp[i:])
-		} else {
-			chunks = append(chunks, otp[i:i+chunkSize])
-		}
-	}
-	return strings.Join(chunks, "-")
-}
diff --git a/models/connection.go b/models/connection.go
index ddb50b87..446c58ff 100644
--- a/models/connection.go
+++ b/models/connection.go
@@ -30,17 +30,24 @@ type SortField struct {
 	Dir  SortDirection
 }
 
+// TruncateAll deletes all data from the database, as managed by GoTrue. Not
+// intended for use outside of tests.
 func TruncateAll(conn *storage.Connection) error {
 	return conn.Transaction(func(tx *storage.Connection) error {
-		if err := tx.RawQuery("delete from " + (&pop.Model{Value: User{}}).TableName()).Exec(); err != nil {
-			return err
+		tables := []string{
+			(&pop.Model{Value: User{}}).TableName(),
+			(&pop.Model{Value: Identity{}}).TableName(),
+			(&pop.Model{Value: RefreshToken{}}).TableName(),
+			(&pop.Model{Value: AuditLogEntry{}}).TableName(),
+			(&pop.Model{Value: Session{}}).TableName(),
 		}
-		if err := tx.RawQuery("delete from " + (&pop.Model{Value: RefreshToken{}}).TableName()).Exec(); err != nil {
-			return err
-		}
-		if err := tx.RawQuery("delete from " + (&pop.Model{Value: AuditLogEntry{}}).TableName()).Exec(); err != nil {
-			return err
+
+		for _, tableName := range tables {
+			if err := tx.RawQuery("TRUNCATE " + tableName + " CASCADE").Exec(); err != nil {
+				return err
+			}
 		}
+
 		return nil
 	})
 }
diff --git a/models/refresh_token.go b/models/refresh_token.go
index 5f1bc463..eb2ab3d5 100644
--- a/models/refresh_token.go
+++ b/models/refresh_token.go
@@ -36,9 +36,14 @@ func (RefreshToken) TableName() string {
 	return tableName
 }
 
+// GrantParams is used to pass session-specific parameters when issuing a new
+// refresh token to authenticated users.
+type GrantParams struct {
+}
+
 // GrantAuthenticatedUser creates a refresh token for the provided user.
-func GrantAuthenticatedUser(tx *storage.Connection, user *User) (*RefreshToken, error) {
-	return createRefreshToken(tx, user, nil)
+func GrantAuthenticatedUser(tx *storage.Connection, user *User, params GrantParams) (*RefreshToken, error) {
+	return createRefreshToken(tx, user, nil, &params)
 }
 
 // GrantRefreshTokenSwap swaps a refresh token for a new one, revoking the provided token.
@@ -54,7 +59,7 @@ func GrantRefreshTokenSwap(r *http.Request, tx *storage.Connection, user *User,
 		if terr = tx.UpdateOnly(token, "revoked"); terr != nil {
 			return terr
 		}
-		newToken, terr = createRefreshToken(rtx, user, token)
+		newToken, terr = createRefreshToken(rtx, user, token, nil)
 		return terr
 	})
 	return newToken, err
@@ -94,7 +99,7 @@ func GetValidChildToken(tx *storage.Connection, token *RefreshToken) (*RefreshTo
 	return refreshToken, nil
 }
 
-func createRefreshToken(tx *storage.Connection, user *User, oldToken *RefreshToken) (*RefreshToken, error) {
+func createRefreshToken(tx *storage.Connection, user *User, oldToken *RefreshToken, params *GrantParams) (*RefreshToken, error) {
 	token := &RefreshToken{
 		UserID: user.ID,
 		Token:  crypto.SecureToken(),
diff --git a/security/hcaptcha.go b/security/hcaptcha.go
index 64da884d..a0014ab1 100644
--- a/security/hcaptcha.go
+++ b/security/hcaptcha.go
@@ -13,6 +13,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/netlify/gotrue/utilities"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -65,7 +66,10 @@ func VerifyRequest(r *http.Request, secretKey string) (VerificationResult, error
 	if err != nil {
 		return UserRequestFailed, err
 	}
-	r.Body.Close()
+	if err := r.Body.Close(); err != nil {
+		return UserRequestFailed, err
+	}
+
 	// re-init body so downstream route handlers don't get borked
 	r.Body = ioutil.NopCloser(bytes.NewBuffer(bodyBytes))
 
@@ -74,7 +78,7 @@ func VerifyRequest(r *http.Request, secretKey string) (VerificationResult, error
 	if err != nil || strings.TrimSpace(res.Security.Token) == "" {
 		return UserRequestFailed, errors.Wrap(err, "couldn't decode captcha info")
 	}
-	clientIP := strings.Split(r.RemoteAddr, ":")[0]
+	clientIP := utilities.GetIPAddress(r)
 	return verifyCaptchaCode(res.Security.Token, secretKey, clientIP)
 }
 
diff --git a/utilities/request.go b/utilities/request.go
new file mode 100644
index 00000000..3e244b7b
--- /dev/null
+++ b/utilities/request.go
@@ -0,0 +1,35 @@
+package utilities
+
+import (
+	"net"
+	"net/http"
+	"strings"
+)
+
+// GetIPAddress returns the real IP address of the HTTP request. It parses the
+// X-Forwarded-For header.
+func GetIPAddress(r *http.Request) string {
+	if r.Header != nil {
+		xForwardedFor := r.Header.Get("X-Forwarded-For")
+		if xForwardedFor != "" {
+			ips := strings.Split(xForwardedFor, ",")
+			for i := range ips {
+				ips[i] = strings.TrimSpace(ips[i])
+			}
+
+			for _, ip := range ips {
+				if ip != "" {
+					return ip
+				}
+			}
+		}
+	}
+
+	ipPort := r.RemoteAddr
+	ip, _, err := net.SplitHostPort(ipPort)
+	if err != nil {
+		return ipPort
+	}
+
+	return ip
+}


Test patch:
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 0923cedb..6331afc0 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -3,7 +3,10 @@ name: Test
 on:
   pull_request:
   push:
-    branches: [master]
+    branches:
+      - master
+      - mfa
+      - saml
     tags: ['*']
 
 jobs:
@@ -47,11 +50,16 @@ jobs:
         run: |
           set -x
           go vet ./...
+      - name: Check for unused code
+        run: |
+          set -x
+          go install honnef.co/go/tools/cmd/staticcheck@latest
+          make unused
       - name: Check gosec
         run: |
           set -x
           go install github.com/securego/gosec/v2/cmd/gosec@latest
-          gosec -tests -exclude=G104 ./... # exclude unhandled errors for the time being
+          make sec
       - name: Init Database
         run: psql -f hack/init_postgres.sql postgresql://postgres:root@localhost:5432/postgres
       - name: Run migrations
diff --git a/api/external_test.go b/api/external_test.go
index b582b2ba..daeadcb9 100644
--- a/api/external_test.go
+++ b/api/external_test.go
@@ -44,7 +44,6 @@ func (ts *ExternalTestSuite) createUser(providerId string, email string, name st
 		require.NoError(ts.T(), ts.API.db.Destroy(u), "Error deleting user")
 	}
 
-	// TODO: [Joel] -- refactor to take in phone
 	u, err := models.NewUser("", email, "test", ts.Config.JWT.Aud, map[string]interface{}{"provider_id": providerId, "full_name": name, "avatar_url": avatar})
 
 	if confirmationToken != "" {
diff --git a/api/helpers_test.go b/api/helpers_test.go
new file mode 100644
index 00000000..54f7beae
--- /dev/null
+++ b/api/helpers_test.go
@@ -0,0 +1,23 @@
+package api
+
+import (
+	"net"
+)
+
+func removeLocalhostFromPrivateIPBlock() *net.IPNet {
+	_, localhost, _ := net.ParseCIDR("127.0.0.0/8")
+
+	var localhostIndex int
+	for i := 0; i < len(privateIPBlocks); i++ {
+		if privateIPBlocks[i] == localhost {
+			localhostIndex = i
+		}
+	}
+	privateIPBlocks = append(privateIPBlocks[:localhostIndex], privateIPBlocks[localhostIndex+1:]...)
+
+	return localhost
+}
+
+func unshiftPrivateIPBlock(address *net.IPNet) {
+	privateIPBlocks = append([]*net.IPNet{address}, privateIPBlocks...)
+}
diff --git a/api/hook_test.go b/api/hook_test.go
index 6542143f..a000117c 100644
--- a/api/hook_test.go
+++ b/api/hook_test.go
@@ -17,6 +17,11 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// withFunctionHooks adds the provided function hooks to the context.
+func withFunctionHooks(ctx context.Context, hooks map[string][]string) context.Context {
+	return context.WithValue(ctx, functionHooksKey, hooks)
+}
+
 func TestSignupHookSendInstanceID(t *testing.T) {
 	globalConfig, err := conf.LoadGlobal(apiTestConfig)
 	require.NoError(t, err)
diff --git a/api/token_test.go b/api/token_test.go
index 3ff75803..c8d36377 100644
--- a/api/token_test.go
+++ b/api/token_test.go
@@ -50,7 +50,7 @@ func (ts *TokenTestSuite) SetupTest() {
 	u.BannedUntil = nil
 	require.NoError(ts.T(), ts.API.db.Create(u), "Error saving new test user")
 
-	ts.RefreshToken, err = models.GrantAuthenticatedUser(ts.API.db, u)
+	ts.RefreshToken, err = models.GrantAuthenticatedUser(ts.API.db, u, models.GrantParams{})
 	require.NoError(ts.T(), err, "Error creating refresh token")
 }
 
@@ -153,7 +153,7 @@ func (ts *TokenTestSuite) TestTokenRefreshTokenRotation() {
 	t := time.Now()
 	u.EmailConfirmedAt = &t
 	require.NoError(ts.T(), ts.API.db.Create(u), "Error saving foo user")
-	first, err := models.GrantAuthenticatedUser(ts.API.db, u)
+	first, err := models.GrantAuthenticatedUser(ts.API.db, u, models.GrantParams{})
 	require.NoError(ts.T(), err)
 	second, err := models.GrantRefreshTokenSwap(&http.Request{}, ts.API.db, u, first)
 	require.NoError(ts.T(), err)
@@ -245,7 +245,7 @@ func (ts *TokenTestSuite) createBannedUser() *models.User {
 	u.BannedUntil = &t
 	require.NoError(ts.T(), ts.API.db.Create(u), "Error saving new test banned user")
 
-	ts.RefreshToken, err = models.GrantAuthenticatedUser(ts.API.db, u)
+	ts.RefreshToken, err = models.GrantAuthenticatedUser(ts.API.db, u, models.GrantParams{})
 	require.NoError(ts.T(), err, "Error creating refresh token")
 
 	return u
diff --git a/mailer/mailer_test.go b/mailer/mailer_test.go
index a8a74ade..2cd565a1 100644
--- a/mailer/mailer_test.go
+++ b/mailer/mailer_test.go
@@ -1,11 +1,18 @@
 package mailer
 
 import (
+	"regexp"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 )
 
+var urlRegexp = regexp.MustCompile(`^https?://[^/]+`)
+
+func enforceRelativeURL(url string) string {
+	return urlRegexp.ReplaceAllString(url, "")
+}
+
 func TestGetSiteURL(t *testing.T) {
 	cases := []struct {
 		ReferrerURL string
diff --git a/models/refresh_token_test.go b/models/refresh_token_test.go
index cbf32af2..6da738ba 100644
--- a/models/refresh_token_test.go
+++ b/models/refresh_token_test.go
@@ -37,7 +37,7 @@ func TestRefreshToken(t *testing.T) {
 
 func (ts *RefreshTokenTestSuite) TestGrantAuthenticatedUser() {
 	u := ts.createUser()
-	r, err := GrantAuthenticatedUser(ts.db, u)
+	r, err := GrantAuthenticatedUser(ts.db, u, GrantParams{})
 	require.NoError(ts.T(), err)
 
 	require.NotEmpty(ts.T(), r.Token)
@@ -46,7 +46,7 @@ func (ts *RefreshTokenTestSuite) TestGrantAuthenticatedUser() {
 
 func (ts *RefreshTokenTestSuite) TestGrantRefreshTokenSwap() {
 	u := ts.createUser()
-	r, err := GrantAuthenticatedUser(ts.db, u)
+	r, err := GrantAuthenticatedUser(ts.db, u, GrantParams{})
 	require.NoError(ts.T(), err)
 
 	s, err := GrantRefreshTokenSwap(&http.Request{}, ts.db, u, r)
@@ -64,7 +64,7 @@ func (ts *RefreshTokenTestSuite) TestGrantRefreshTokenSwap() {
 
 func (ts *RefreshTokenTestSuite) TestLogout() {
 	u := ts.createUser()
-	r, err := GrantAuthenticatedUser(ts.db, u)
+	r, err := GrantAuthenticatedUser(ts.db, u, GrantParams{})
 	require.NoError(ts.T(), err)
 
 	require.NoError(ts.T(), Logout(ts.db, u.ID))
diff --git a/models/user_test.go b/models/user_test.go
index 8ff308a4..eb75a599 100644
--- a/models/user_test.go
+++ b/models/user_test.go
@@ -148,7 +148,7 @@ func (ts *UserTestSuite) TestFindUserByRecoveryToken() {
 
 func (ts *UserTestSuite) TestFindUserWithRefreshToken() {
 	u := ts.createUser()
-	r, err := GrantAuthenticatedUser(ts.db, u)
+	r, err := GrantAuthenticatedUser(ts.db, u, GrantParams{})
 	require.NoError(ts.T(), err)
 
 	n, nr, err := FindUserWithRefreshToken(ts.db, r.Token)
diff --git a/utilities/request_test.go b/utilities/request_test.go
new file mode 100644
index 00000000..da76042e
--- /dev/null
+++ b/utilities/request_test.go
@@ -0,0 +1,87 @@
+package utilities
+
+import (
+	"net/http"
+	tst "testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestGetIPAddress(t *tst.T) {
+	examples := []func(r *http.Request) string{
+		func(r *http.Request) string {
+			r.Header = nil
+			r.RemoteAddr = "127.0.0.1:8080"
+
+			return "127.0.0.1"
+		},
+
+		func(r *http.Request) string {
+			r.Header = nil
+			r.RemoteAddr = "incorrect"
+
+			return "incorrect"
+		},
+
+		func(r *http.Request) string {
+			r.Header = make(http.Header)
+			r.RemoteAddr = "127.0.0.1:8080"
+
+			return "127.0.0.1"
+		},
+
+		func(r *http.Request) string {
+			r.Header = make(http.Header)
+			r.RemoteAddr = "[::1]:8080"
+
+			return "::1"
+		},
+
+		func(r *http.Request) string {
+			r.Header = make(http.Header)
+			r.RemoteAddr = "127.0.0.1:8080"
+			r.Header.Add("X-Forwarded-For", "127.0.0.2")
+
+			return "127.0.0.2"
+		},
+
+		func(r *http.Request) string {
+			r.Header = make(http.Header)
+			r.RemoteAddr = "127.0.0.1:8080"
+			r.Header.Add("X-Forwarded-For", "127.0.0.2")
+
+			return "127.0.0.2"
+		},
+
+		func(r *http.Request) string {
+			r.Header = make(http.Header)
+			r.RemoteAddr = "127.0.0.1:8080"
+			r.Header.Add("X-Forwarded-For", "127.0.0.2,")
+
+			return "127.0.0.2"
+		},
+
+		func(r *http.Request) string {
+			r.Header = make(http.Header)
+			r.RemoteAddr = "127.0.0.1:8080"
+			r.Header.Add("X-Forwarded-For", "127.0.0.2,127.0.0.3")
+
+			return "127.0.0.2"
+		},
+
+		func(r *http.Request) string {
+			r.Header = make(http.Header)
+			r.RemoteAddr = "127.0.0.1:8080"
+			r.Header.Add("X-Forwarded-For", "::1,127.0.0.2")
+
+			return "::1"
+		},
+	}
+
+	for _, example := range examples {
+		req := &http.Request{}
+		expected := example(req)
+
+		require.Equal(t, GetIPAddress(req), expected)
+	}
+}