Code cleanup per PR feedback. T712

config/config.go

@@ -59,19 +59,21 @@ type (
 	}
 
 	WriteAsOauthCfg struct {
-		ClientID              string `ini:"client_id"`
+		ClientID         string `ini:"client_id"`
-		ClientSecret          string `ini:"client_secret"`
+		ClientSecret     string `ini:"client_secret"`
-		AuthLocation          string `ini:"auth_location"`
+		AuthLocation     string `ini:"auth_location"`
-		TokenLocation         string `ini:"token_location"`
+		TokenLocation    string `ini:"token_location"`
-		InspectLocation       string `ini:"inspect_location"`
+		InspectLocation  string `ini:"inspect_location"`
-		StateRegisterLocation string `ini:"state_register_location"`
+		CallbackProxy    string `ini:"callback_proxy"`
+		CallbackProxyAPI string `ini:"callback_proxy_api"`
 	}
 
 	SlackOauthCfg struct {
-		ClientID              string `ini:"client_id"`
+		ClientID         string `ini:"client_id"`
-		ClientSecret          string `ini:"client_secret"`
+		ClientSecret     string `ini:"client_secret"`
-		TeamID                string `ini:"team_id"`
+		TeamID           string `ini:"team_id"`
-		StateRegisterLocation string `ini:"state_register_location"`
+		CallbackProxy    string `ini:"callback_proxy"`
+		CallbackProxyAPI string `ini:"callback_proxy_api"`
 	}
 
 	// AppCfg holds values that affect how the application functions

oauth.go

@@ -3,7 +3,6 @@ package writefreely
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"github.com/gorilla/mux"
 	"github.com/gorilla/sessions"
@@ -80,19 +79,19 @@ type oauthClient interface {
 	inspectOauthAccessToken(ctx context.Context, accessToken string) (*InspectResponse, error)
 }
 
-type oauthStateRegisterer struct {
+type callbackProxyClient struct {
-	location         string
+	server           string
 	callbackLocation string
 	httpClient       HttpClient
 }
 
 type oauthHandler struct {
-	Config               *config.Config
+	Config        *config.Config
-	DB                   OAuthDatastore
+	DB            OAuthDatastore
-	Store                sessions.Store
+	Store         sessions.Store
-	EmailKey             []byte
+	EmailKey      []byte
-	oauthClient          oauthClient
+	oauthClient   oauthClient
-	oauthStateRegisterer *oauthStateRegisterer
+	callbackProxy *callbackProxyClient
 }
 
 func (h oauthHandler) viewOauthInit(app *App, w http.ResponseWriter, r *http.Request) error {
@@ -102,9 +101,9 @@ func (h oauthHandler) viewOauthInit(app *App, w http.ResponseWriter, r *http.Req
 		return impart.HTTPError{http.StatusInternalServerError, "could not prepare oauth redirect url"}
 	}
 
-	if h.oauthStateRegisterer != nil {
+	if h.callbackProxy != nil {
-		if err := h.oauthStateRegisterer.register(ctx, state); err != nil {
+		if err := h.callbackProxy.register(ctx, state); err != nil {
-			return impart.HTTPError{http.StatusInternalServerError, "could not register state location"}
+			return impart.HTTPError{http.StatusInternalServerError, "could not register state server"}
 		}
 	}
 
@@ -117,21 +116,23 @@ func (h oauthHandler) viewOauthInit(app *App, w http.ResponseWriter, r *http.Req
 
 func configureSlackOauth(parentHandler *Handler, r *mux.Router, app *App) {
 	if app.Config().SlackOauth.ClientID != "" {
-		var stateRegisterClient *oauthStateRegisterer = nil
+		callbackLocation := app.Config().App.Host + "/oauth/callback"
-		if app.Config().SlackOauth.StateRegisterLocation != "" {
+
-			stateRegisterClient = &oauthStateRegisterer{
+		var stateRegisterClient *callbackProxyClient = nil
-				location:         app.Config().SlackOauth.StateRegisterLocation,
+		if app.Config().SlackOauth.CallbackProxyAPI != "" {
+			stateRegisterClient = &callbackProxyClient{
+				server:           app.Config().SlackOauth.CallbackProxyAPI,
 				callbackLocation: app.Config().App.Host + "/oauth/callback",
 				httpClient:       config.DefaultHTTPClient(),
 			}
+			callbackLocation = app.Config().SlackOauth.CallbackProxy
 		}
 		oauthClient := slackOauthClient{
-			ClientID:     app.Config().SlackOauth.ClientID,
+			ClientID:         app.Config().SlackOauth.ClientID,
-			ClientSecret: app.Config().SlackOauth.ClientSecret,
+			ClientSecret:     app.Config().SlackOauth.ClientSecret,
-			TeamID:       app.Config().SlackOauth.TeamID,
+			TeamID:           app.Config().SlackOauth.TeamID,
-			HttpClient:   config.DefaultHTTPClient(),
+			HttpClient:       config.DefaultHTTPClient(),
-			//CallbackLocation: app.Config().App.Host + "/oauth/callback",
+			CallbackLocation: callbackLocation,
-			CallbackLocation: "http://localhost:5000/callback",
 		}
 		configureOauthRoutes(parentHandler, r, app, oauthClient, stateRegisterClient)
 	}
@@ -139,14 +140,18 @@ func configureSlackOauth(parentHandler *Handler, r *mux.Router, app *App) {
 
 func configureWriteAsOauth(parentHandler *Handler, r *mux.Router, app *App) {
 	if app.Config().WriteAsOauth.ClientID != "" {
-		var stateRegisterClient *oauthStateRegisterer = nil
+		callbackLocation := app.Config().App.Host + "/oauth/callback"
-		if app.Config().WriteAsOauth.StateRegisterLocation != "" {
+
-			stateRegisterClient = &oauthStateRegisterer{
+		var callbackProxy *callbackProxyClient = nil
-				location:         app.Config().WriteAsOauth.StateRegisterLocation,
+		if app.Config().WriteAsOauth.CallbackProxy != "" {
+			callbackProxy = &callbackProxyClient{
+				server:           app.Config().WriteAsOauth.CallbackProxyAPI,
 				callbackLocation: app.Config().App.Host + "/oauth/callback",
 				httpClient:       config.DefaultHTTPClient(),
 			}
+			callbackLocation = app.Config().SlackOauth.CallbackProxy
 		}
+
 		oauthClient := writeAsOauthClient{
 			ClientID:         app.Config().WriteAsOauth.ClientID,
 			ClientSecret:     app.Config().WriteAsOauth.ClientSecret,
@@ -154,20 +159,20 @@ func configureWriteAsOauth(parentHandler *Handler, r *mux.Router, app *App) {
 			InspectLocation:  config.OrDefaultString(app.Config().WriteAsOauth.InspectLocation, writeAsIdentityLocation),
 			AuthLocation:     config.OrDefaultString(app.Config().WriteAsOauth.AuthLocation, writeAsAuthLocation),
 			HttpClient:       config.DefaultHTTPClient(),
-			CallbackLocation: "http://localhost:5000/callback",
+			CallbackLocation: callbackLocation,
 		}
-		configureOauthRoutes(parentHandler, r, app, oauthClient, stateRegisterClient)
+		configureOauthRoutes(parentHandler, r, app, oauthClient, callbackProxy)
 	}
 }
 
-func configureOauthRoutes(parentHandler *Handler, r *mux.Router, app *App, oauthClient oauthClient, stateRegisterClient *oauthStateRegisterer) {
+func configureOauthRoutes(parentHandler *Handler, r *mux.Router, app *App, oauthClient oauthClient, callbackProxy *callbackProxyClient) {
 	handler := &oauthHandler{
-		Config:               app.Config(),
+		Config:        app.Config(),
-		DB:                   app.DB(),
+		DB:            app.DB(),
-		Store:                app.SessionStore(),
+		Store:         app.SessionStore(),
-		oauthClient:          oauthClient,
+		oauthClient:   oauthClient,
-		EmailKey:             app.keys.EmailKey,
+		EmailKey:      app.keys.EmailKey,
-		oauthStateRegisterer: stateRegisterClient,
+		callbackProxy: callbackProxy,
 	}
 	r.HandleFunc("/oauth/"+oauthClient.GetProvider(), parentHandler.OAuth(handler.viewOauthInit)).Methods("GET")
 	r.HandleFunc("/oauth/callback", parentHandler.OAuth(handler.viewOauthCallback)).Methods("GET")
@@ -233,11 +238,11 @@ func (h oauthHandler) viewOauthCallback(app *App, w http.ResponseWriter, r *http
 	return h.showOauthSignupPage(app, w, r, tp, nil)
 }
 
-func (r *oauthStateRegisterer) register(ctx context.Context, state string) error {
+func (r *callbackProxyClient) register(ctx context.Context, state string) error {
 	form := url.Values{}
 	form.Add("state", state)
 	form.Add("location", r.callbackLocation)
-	req, err := http.NewRequestWithContext(ctx, "POST", r.location, strings.NewReader(form.Encode()))
+	req, err := http.NewRequestWithContext(ctx, "POST", r.server, strings.NewReader(form.Encode()))
 	if err != nil {
 		return err
 	}
@@ -250,7 +255,7 @@ func (r *oauthStateRegisterer) register(ctx context.Context, state string) error
 		return err
 	}
 	if resp.StatusCode != http.StatusCreated {
-		return errors.New("register state and location")
+		return fmt.Errorf("unable register state location: %d", resp.StatusCode)
 	}
 
 	return nil