From f906c9ba422eb720514721b559c01f840ca34a0c Mon Sep 17 00:00:00 2001
From: Niall Sheridan <nsheridan@gmail.com>
Date: Mon, 20 Aug 2018 17:18:27 +0100
Subject: [PATCH] Remove authprovider.Session

---
 server/auth/github/github.go             |  6 ++----
 server/auth/github/github_test.go        |  6 +++---
 server/auth/gitlab/gitlab.go             |  7 ++-----
 server/auth/gitlab/gitlab_test.go        | 12 ++++++------
 server/auth/google/google.go             |  6 ++----
 server/auth/google/google_test.go        |  8 ++++----
 server/auth/microsoft/microsoft.go       | 10 ++++------
 server/auth/microsoft/microsoft_test.go  |  2 +-
 server/auth/provider.go                  | 19 +------------------
 server/auth/testprovider/testprovider.go |  6 ++----
 server/handlers_test.go                  |  2 --
 server/web.go                            | 10 ++++------
 12 files changed, 31 insertions(+), 63 deletions(-)

diff --git a/server/auth/github/github.go b/server/auth/github/github.go
index d5464780..38009e10 100644
--- a/server/auth/github/github.go
+++ b/server/auth/github/github.go
@@ -97,10 +97,8 @@ func (c *Config) Revoke(token *oauth2.Token) error {
 }
 
 // StartSession retrieves an authentication endpoint from Github.
-func (c *Config) StartSession(state string) *auth.Session {
-	return &auth.Session{
-		AuthURL: c.config.AuthCodeURL(state),
-	}
+func (c *Config) StartSession(state string) string {
+	return c.config.AuthCodeURL(state)
 }
 
 // Exchange authorizes the session and returns an access token.
diff --git a/server/auth/github/github_test.go b/server/auth/github/github_test.go
index 8c51f4f8..9e94f9a8 100644
--- a/server/auth/github/github_test.go
+++ b/server/auth/github/github_test.go
@@ -62,9 +62,9 @@ func TestStartSession(t *testing.T) {
 
 	p, _ := newGithub()
 	s := p.StartSession("test_state")
-	a.Contains(s.AuthURL, "github.com/login/oauth/authorize")
-	a.Contains(s.AuthURL, "state=test_state")
-	a.Contains(s.AuthURL, fmt.Sprintf("client_id=%s", oauthClientID))
+	a.Contains(s, "github.com/login/oauth/authorize")
+	a.Contains(s, "state=test_state")
+	a.Contains(s, fmt.Sprintf("client_id=%s", oauthClientID))
 }
 
 func newGithub() (*Config, error) {
diff --git a/server/auth/gitlab/gitlab.go b/server/auth/gitlab/gitlab.go
index 2cf2a5c2..5e1f95f7 100644
--- a/server/auth/gitlab/gitlab.go
+++ b/server/auth/gitlab/gitlab.go
@@ -4,7 +4,6 @@ import (
 	"errors"
 	"strconv"
 
-	"github.com/nsheridan/cashier/server/auth"
 	"github.com/nsheridan/cashier/server/config"
 	"github.com/nsheridan/cashier/server/metrics"
 
@@ -114,10 +113,8 @@ func (c *Config) Revoke(token *oauth2.Token) error {
 }
 
 // StartSession retrieves an authentication endpoint from Gitlab.
-func (c *Config) StartSession(state string) *auth.Session {
-	return &auth.Session{
-		AuthURL: c.config.AuthCodeURL(state),
-	}
+func (c *Config) StartSession(state string) string {
+	return c.config.AuthCodeURL(state)
 }
 
 // Exchange authorizes the session and returns an access token.
diff --git a/server/auth/gitlab/gitlab_test.go b/server/auth/gitlab/gitlab_test.go
index 39c27012..93b348b1 100644
--- a/server/auth/gitlab/gitlab_test.go
+++ b/server/auth/gitlab/gitlab_test.go
@@ -56,9 +56,9 @@ func TestGoodAllUsers(t *testing.T) {
 
 	p, _ := newGitlab()
 	s := p.StartSession("test_state")
-	a.Contains(s.AuthURL, "exampleorg/oauth/authorize")
-	a.Contains(s.AuthURL, "state=test_state")
-	a.Contains(s.AuthURL, fmt.Sprintf("client_id=%s", oauthClientID))
+	a.Contains(s, "exampleorg/oauth/authorize")
+	a.Contains(s, "state=test_state")
+	a.Contains(s, fmt.Sprintf("client_id=%s", oauthClientID))
 
 	allusers = ""
 }
@@ -78,9 +78,9 @@ func TestStartSession(t *testing.T) {
 
 	p, _ := newGitlab()
 	s := p.StartSession("test_state")
-	a.Contains(s.AuthURL, "exampleorg/oauth/authorize")
-	a.Contains(s.AuthURL, "state=test_state")
-	a.Contains(s.AuthURL, fmt.Sprintf("client_id=%s", oauthClientID))
+	a.Contains(s, "exampleorg/oauth/authorize")
+	a.Contains(s, "state=test_state")
+	a.Contains(s, fmt.Sprintf("client_id=%s", oauthClientID))
 }
 
 func newGitlab() (auth.Provider, error) {
diff --git a/server/auth/google/google.go b/server/auth/google/google.go
index 9a151f62..b7073100 100644
--- a/server/auth/google/google.go
+++ b/server/auth/google/google.go
@@ -103,10 +103,8 @@ func (c *Config) Revoke(token *oauth2.Token) error {
 }
 
 // StartSession retrieves an authentication endpoint from Google.
-func (c *Config) StartSession(state string) *auth.Session {
-	return &auth.Session{
-		AuthURL: c.config.AuthCodeURL(state, oauth2.SetAuthURLParam("hd", c.domain)),
-	}
+func (c *Config) StartSession(state string) string {
+	return c.config.AuthCodeURL(state, oauth2.SetAuthURLParam("hd", c.domain))
 }
 
 // Exchange authorizes the session and returns an access token.
diff --git a/server/auth/google/google_test.go b/server/auth/google/google_test.go
index b3d26334..92e4ca0a 100644
--- a/server/auth/google/google_test.go
+++ b/server/auth/google/google_test.go
@@ -57,10 +57,10 @@ func TestStartSession(t *testing.T) {
 	p, err := newGoogle()
 	a.NoError(err)
 	s := p.StartSession("test_state")
-	a.Contains(s.AuthURL, "accounts.google.com/o/oauth2/auth")
-	a.Contains(s.AuthURL, "state=test_state")
-	a.Contains(s.AuthURL, fmt.Sprintf("hd=%s", domain))
-	a.Contains(s.AuthURL, fmt.Sprintf("client_id=%s", oauthClientID))
+	a.Contains(s, "accounts.google.com/o/oauth2/auth")
+	a.Contains(s, "state=test_state")
+	a.Contains(s, fmt.Sprintf("hd=%s", domain))
+	a.Contains(s, fmt.Sprintf("client_id=%s", oauthClientID))
 }
 
 func newGoogle() (*Config, error) {
diff --git a/server/auth/microsoft/microsoft.go b/server/auth/microsoft/microsoft.go
index 49d9b822..8463ccfe 100644
--- a/server/auth/microsoft/microsoft.go
+++ b/server/auth/microsoft/microsoft.go
@@ -175,12 +175,10 @@ func (c *Config) Revoke(token *oauth2.Token) error {
 }
 
 // StartSession retrieves an authentication endpoint from Microsoft.
-func (c *Config) StartSession(state string) *auth.Session {
-	return &auth.Session{
-		AuthURL: c.config.AuthCodeURL(state,
-			oauth2.SetAuthURLParam("hd", c.tenant),
-			oauth2.SetAuthURLParam("prompt", "login")),
-	}
+func (c *Config) StartSession(state string) string {
+	return c.config.AuthCodeURL(state,
+		oauth2.SetAuthURLParam("hd", c.tenant),
+		oauth2.SetAuthURLParam("prompt", "login"))
 }
 
 // Exchange authorizes the session and returns an access token.
diff --git a/server/auth/microsoft/microsoft_test.go b/server/auth/microsoft/microsoft_test.go
index c2c2c172..e362ef92 100644
--- a/server/auth/microsoft/microsoft_test.go
+++ b/server/auth/microsoft/microsoft_test.go
@@ -57,7 +57,7 @@ func TestStartSession(t *testing.T) {
 	p, err := newMicrosoft()
 	a.NoError(err)
 	s := p.StartSession("test_state")
-	a.Contains(s.AuthURL, fmt.Sprintf("login.microsoftonline.com/%s/oauth2/v2.0/authorize", tenant))
+	a.Contains(s, fmt.Sprintf("login.microsoftonline.com/%s/oauth2/v2.0/authorize", tenant))
 }
 
 func newMicrosoft() (*Config, error) {
diff --git a/server/auth/provider.go b/server/auth/provider.go
index 06dc1c9a..9d1c8bd3 100644
--- a/server/auth/provider.go
+++ b/server/auth/provider.go
@@ -5,26 +5,9 @@ import "golang.org/x/oauth2"
 // Provider is an abstraction of different auth methods.
 type Provider interface {
 	Name() string
-	StartSession(string) *Session
+	StartSession(string) string
 	Exchange(string) (*oauth2.Token, error)
 	Username(*oauth2.Token) string
 	Valid(*oauth2.Token) bool
 	Revoke(*oauth2.Token) error
 }
-
-// Session stores authentication state.
-type Session struct {
-	AuthURL string
-	Token   *oauth2.Token
-}
-
-// Authorize obtains data from the provider and retains an access token that
-// can be stored for later access.
-func (s *Session) Authorize(provider Provider, code string) error {
-	t, err := provider.Exchange(code)
-	if err != nil {
-		return err
-	}
-	s.Token = t
-	return nil
-}
diff --git a/server/auth/testprovider/testprovider.go b/server/auth/testprovider/testprovider.go
index e30b04aa..f785081a 100644
--- a/server/auth/testprovider/testprovider.go
+++ b/server/auth/testprovider/testprovider.go
@@ -38,10 +38,8 @@ func (c *Config) Revoke(token *oauth2.Token) error {
 }
 
 // StartSession retrieves an authentication endpoint.
-func (c *Config) StartSession(state string) *auth.Session {
-	return &auth.Session{
-		AuthURL: "https://www.example.com/auth",
-	}
+func (c *Config) StartSession(state string) string {
+	return "https://www.example.com/auth"
 }
 
 // Exchange authorizes the session and returns an access token.
diff --git a/server/handlers_test.go b/server/handlers_test.go
index 7f314524..6dc2236c 100644
--- a/server/handlers_test.go
+++ b/server/handlers_test.go
@@ -17,7 +17,6 @@ import (
 
 	"github.com/gorilla/sessions"
 	"github.com/nsheridan/cashier/lib"
-	"github.com/nsheridan/cashier/server/auth"
 	"github.com/nsheridan/cashier/server/auth/testprovider"
 	"github.com/nsheridan/cashier/server/config"
 	"github.com/nsheridan/cashier/server/signer"
@@ -41,7 +40,6 @@ func init() {
 	certstore, _ = store.New(map[string]string{"type": "mem"})
 	ctx = &appContext{
 		cookiestore: sessions.NewCookieStore([]byte("secret")),
-		authsession: &auth.Session{AuthURL: "https://www.example.com/auth"},
 	}
 }
 
diff --git a/server/web.go b/server/web.go
index 840ce1ba..9114de1f 100644
--- a/server/web.go
+++ b/server/web.go
@@ -28,7 +28,6 @@ import (
 	"github.com/gorilla/mux"
 	"github.com/gorilla/sessions"
 	"github.com/nsheridan/cashier/lib"
-	"github.com/nsheridan/cashier/server/auth"
 	"github.com/nsheridan/cashier/server/config"
 	"github.com/nsheridan/cashier/server/templates"
 )
@@ -36,7 +35,6 @@ import (
 // appContext contains local context - cookiestore, authsession etc.
 type appContext struct {
 	cookiestore   *sessions.CookieStore
-	authsession   *auth.Session
 	requireReason bool
 }
 
@@ -172,8 +170,7 @@ func signHandler(a *appContext, w http.ResponseWriter, r *http.Request) (int, er
 func loginHandler(a *appContext, w http.ResponseWriter, r *http.Request) (int, error) {
 	state := newState()
 	a.setAuthStateCookie(w, r, state)
-	a.authsession = authprovider.StartSession(state)
-	http.Redirect(w, r, a.authsession.AuthURL, http.StatusFound)
+	http.Redirect(w, r, authprovider.StartSession(state), http.StatusFound)
 	return http.StatusFound, nil
 }
 
@@ -183,10 +180,11 @@ func callbackHandler(a *appContext, w http.ResponseWriter, r *http.Request) (int
 		return http.StatusUnauthorized, errors.New(http.StatusText(http.StatusUnauthorized))
 	}
 	code := r.FormValue("code")
-	if err := a.authsession.Authorize(authprovider, code); err != nil {
+	token, err := authprovider.Exchange(code)
+	if err != nil {
 		return http.StatusInternalServerError, err
 	}
-	a.setAuthTokenCookie(w, r, a.authsession.Token)
+	a.setAuthTokenCookie(w, r, token)
 	http.Redirect(w, r, a.getCurrentURL(r), http.StatusFound)
 	return http.StatusFound, nil
 }
-- 
GitLab