Repository: markbates/goth
Pull request #295 (2019-10-24 22:32:29)

apple -- make SecretParams properties public
A slight correction to my last contribution.


Patch:
diff --git a/providers/apple/apple.go b/providers/apple/apple.go
index 137e95a..b0e870a 100644
--- a/providers/apple/apple.go
+++ b/providers/apple/apple.go
@@ -63,12 +63,12 @@ func (p Provider) ClientId() string {
 }
 
 type SecretParams struct {
-	pkcs8PrivateKey, teamId, keyId, clientId string
-	iat, exp                                 int
+	PKCS8PrivateKey, TeamId, KeyId, ClientId string
+	Iat, Exp                                 int
 }
 
 func MakeSecret(sp SecretParams) (*string, error) {
-	block, rest := pem.Decode([]byte(strings.TrimSpace(sp.pkcs8PrivateKey)))
+	block, rest := pem.Decode([]byte(strings.TrimSpace(sp.PKCS8PrivateKey)))
 	if block == nil || len(rest) > 0 {
 		return nil, errors.New("invalid private key")
 	}
@@ -77,13 +77,13 @@ func MakeSecret(sp SecretParams) (*string, error) {
 		return nil, err
 	}
 	token := jwt.NewWithClaims(jwt.SigningMethodES256, jwt.MapClaims{
-		"iss": sp.teamId,
-		"iat": sp.iat,
-		"exp": sp.exp,
+		"iss": sp.TeamId,
+		"iat": sp.Iat,
+		"exp": sp.Exp,
 		"aud": AppleAudOrIss,
-		"sub": sp.clientId,
+		"sub": sp.ClientId,
 	})
-	token.Header["kid"] = sp.keyId
+	token.Header["kid"] = sp.KeyId
 	ss, err := token.SignedString(pk)
 	return &ss, err
 }
diff --git a/providers/slack/slack.go b/providers/slack/slack.go
index 9083637..3f0079a 100644
--- a/providers/slack/slack.go
+++ b/providers/slack/slack.go
@@ -11,17 +11,10 @@ import (
 	"net/url"
 
 	"fmt"
-
 	"github.com/markbates/goth"
 	"golang.org/x/oauth2"
 )
 
-// Scopes
-const (
-	ScopeUserRead string = "users:read"
-)
-
-// URLs and endpoints
 const (
 	authURL         string = "https://slack.com/oauth/authorize"
 	tokenURL        string = "https://slack.com/api/oauth.access"
@@ -63,7 +56,6 @@ func (p *Provider) SetName(name string) {
 	p.providerName = name
 }
 
-// Client returns the http.Client used in the provider.
 func (p *Provider) Client() *http.Client {
 	return goth.HTTPClientWithFallBack(p.HTTPClient)
 }
@@ -96,9 +88,11 @@ func (p *Provider) FetchUser(session goth.Session) (goth.User, error) {
 	// Get the userID, slack needs userID in order to get user profile info
 	response, err := p.Client().Get(endpointUser + "?token=" + url.QueryEscape(sess.AccessToken))
 	if err != nil {
+		if response != nil {
+			response.Body.Close()
+		}
 		return user, err
 	}
-	defer response.Body.Close()
 
 	if response.StatusCode != http.StatusOK {
 		return user, fmt.Errorf("%s responded with a %d trying to fetch user information", p.providerName, response.StatusCode)
@@ -109,52 +103,34 @@ func (p *Provider) FetchUser(session goth.Session) (goth.User, error) {
 		return user, err
 	}
 
-	err = json.NewDecoder(bytes.NewReader(bits)).Decode(&user.RawData)
-	if err != nil {
-		return user, err
-	}
-
-	err = simpleUserFromReader(bytes.NewReader(bits), &user)
-
-	if p.hasScope(ScopeUserRead) {
-		// Get user profile info
-		response, err = p.Client().Get(endpointProfile + "?token=" + url.QueryEscape(sess.AccessToken) + "&user=" + user.UserID)
-		if err != nil {
-			return user, err
-		}
-		defer response.Body.Close()
-
-		if response.StatusCode != http.StatusOK {
-			return user, fmt.Errorf("%s responded with a %d trying to fetch user information", p.providerName, response.StatusCode)
-		}
+	u := struct {
+		UserID string `json:"user_id"`
+	}{}
 
-		bits, err = ioutil.ReadAll(response.Body)
-		if err != nil {
-			return user, err
-		}
+	err = json.NewDecoder(bytes.NewReader(bits)).Decode(&u)
 
-		err = json.NewDecoder(bytes.NewReader(bits)).Decode(&user.RawData)
-		if err != nil {
-			return user, err
+	// Get user profile info
+	response, err = p.Client().Get(endpointProfile + "?token=" + url.QueryEscape(sess.AccessToken) + "&user=" + u.UserID)
+	if err != nil {
+		if response != nil {
+			response.Body.Close()
 		}
-
-		err = userFromReader(bytes.NewReader(bits), &user)
+		return user, err
 	}
+	defer response.Body.Close()
 
-	return user, err
-}
-
-func (p *Provider) hasScope(scope string) bool {
-	hasScope := false
+	bits, err = ioutil.ReadAll(response.Body)
+	if err != nil {
+		return user, err
+	}
 
-	for i := range p.config.Scopes {
-		if p.config.Scopes[i] == scope {
-			hasScope = true
-			break
-		}
+	err = json.NewDecoder(bytes.NewReader(bits)).Decode(&user.RawData)
+	if err != nil {
+		return user, err
 	}
 
-	return hasScope
+	err = userFromReader(bytes.NewReader(bits), &user)
+	return user, err
 }
 
 func newConfig(provider *Provider, scopes []string) *oauth2.Config {
@@ -174,28 +150,11 @@ func newConfig(provider *Provider, scopes []string) *oauth2.Config {
 			c.Scopes = append(c.Scopes, scope)
 		}
 	} else {
-		c.Scopes = append(c.Scopes, ScopeUserRead)
+		c.Scopes = append(c.Scopes, "users:read")
 	}
 	return c
 }
 
-func simpleUserFromReader(r io.Reader, user *goth.User) error {
-	u := struct {
-		UserID string `json:"user_id"`
-		Name   string `json:"user"`
-	}{}
-
-	err := json.NewDecoder(r).Decode(&u)
-	if err != nil {
-		return err
-	}
-
-	user.UserID = u.UserID
-	user.NickName = u.Name
-
-	return nil
-}
-
 func userFromReader(r io.Reader, user *goth.User) error {
 	u := struct {
 		User struct {
@@ -206,7 +165,7 @@ func userFromReader(r io.Reader, user *goth.User) error {
 				Name      string `json:"real_name"`
 				AvatarURL string `json:"image_32"`
 				FirstName string `json:"first_name"`
-				LastName  string `json:"last_name"`
+				LastName string `json:"last_name"`
 			} `json:"profile"`
 		} `json:"user"`
 	}{}


Test patch:
diff --git a/providers/apple/apple_test.go b/providers/apple/apple_test.go
index d8e8f50..2786d6c 100644
--- a/providers/apple/apple_test.go
+++ b/providers/apple/apple_test.go
@@ -58,16 +58,16 @@ func TestMakeSecret(t *testing.T) {
 
 	iat := 1570636633
 	ss, err := MakeSecret(SecretParams{
-		pkcs8PrivateKey: `-----BEGIN PRIVATE KEY-----
+		PKCS8PrivateKey: `-----BEGIN PRIVATE KEY-----
 MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgPALVklHT2n9FNxeP
 c1+TCP+Ep7YOU7T9KB5MTVpjL1ShRANCAATXAbDMQ/URATKRoSIFMkwetLH/M2S4
 nNFzkp23qt9IJDivieB/BBJct1UvhoICg5eZDhSR+x7UH3Uhog8qgoIC
 -----END PRIVATE KEY-----`, // example
-		teamId:   "TK...",
-		keyId:    "<keyId>",
-		clientId: "<clientId>",
-		iat:      iat,
-		exp:      iat + 15777000,
+		TeamId:   "TK...",
+		KeyId:    "<keyId>",
+		ClientId: "<clientId>",
+		Iat:      iat,
+		Exp:      iat + 15777000,
 	})
 	a.NoError(err)
 	a.NotZero(ss)
diff --git a/providers/slack/slack_test.go b/providers/slack/slack_test.go
index dd6d595..eda5c75 100644
--- a/providers/slack/slack_test.go
+++ b/providers/slack/slack_test.go
@@ -1,39 +1,11 @@
 package slack_test
 
 import (
-	"context"
-	"crypto/tls"
-	"encoding/json"
-	"net"
-	"net/http"
-	"net/http/httptest"
-	"os"
-	"testing"
-
 	"github.com/markbates/goth"
 	"github.com/markbates/goth/providers/slack"
 	"github.com/stretchr/testify/assert"
-)
-
-var (
-	testAuthTestResponseData = map[string]interface{}{
-		"user":    "testuser",
-		"user_id": "user1234",
-	}
-
-	testUserInfoResponseData = map[string]interface{}{
-		"user": map[string]interface{}{
-			"id":   testAuthTestResponseData["user_id"],
-			"name": testAuthTestResponseData["user"],
-			"profile": map[string]interface{}{
-				"real_name":  "Test User",
-				"first_name": "Test",
-				"last_name":  "User",
-				"image_32":   "http://example.org/avatar.png",
-				"email":      "test@example.org",
-			},
-		},
-	}
+	"os"
+	"testing"
 )
 
 func Test_New(t *testing.T) {
@@ -62,142 +34,6 @@ func Test_BeginAuth(t *testing.T) {
 	a.Contains(s.AuthURL, "slack.com/oauth/authorize")
 }
 
-func Test_FetchUser(t *testing.T) {
-	t.Parallel()
-
-	for _, testData := range []struct {
-		name         string
-		provider     *slack.Provider
-		session      goth.Session
-		handler      http.Handler
-		expectedUser goth.User
-		expectErr    bool
-	}{
-		{
-			name:     "FetchesFullProfile",
-			provider: provider(),
-			session:  &slack.Session{AccessToken: "TOKEN"},
-			handler: http.HandlerFunc(
-				func(res http.ResponseWriter, req *http.Request) {
-					switch req.URL.Path {
-					case "/api/auth.test":
-						res.WriteHeader(http.StatusOK)
-						json.NewEncoder(res).Encode(testAuthTestResponseData)
-					case "/api/users.info":
-						res.WriteHeader(http.StatusOK)
-						json.NewEncoder(res).Encode(testUserInfoResponseData)
-					default:
-						res.WriteHeader(http.StatusNotFound)
-					}
-				},
-			),
-			expectedUser: goth.User{
-				UserID:      "user1234",
-				NickName:    "testuser",
-				Name:        "Test User",
-				FirstName:   "Test",
-				LastName:    "User",
-				AvatarURL:   "http://example.org/avatar.png",
-				Email:       "test@example.org",
-				AccessToken: "TOKEN",
-			},
-			expectErr: false,
-		},
-		{
-			name:     "FetchesBasicProfileWhenLackingUserReadScope",
-			provider: slack.New(os.Getenv("SLACK_KEY"), os.Getenv("SLACK_SECRET"), "/foo", "commands"),
-			session:  &slack.Session{AccessToken: "TOKEN"},
-			handler: http.HandlerFunc(
-				func(res http.ResponseWriter, req *http.Request) {
-					switch req.URL.Path {
-					case "/api/auth.test":
-						res.WriteHeader(http.StatusOK)
-						json.NewEncoder(res).Encode(testAuthTestResponseData)
-					default:
-						res.WriteHeader(http.StatusNotFound)
-					}
-				},
-			),
-			expectedUser: goth.User{
-				UserID:      "user1234",
-				NickName:    "testuser",
-				AccessToken: "TOKEN",
-			},
-			expectErr: false,
-		},
-		{
-			name:      "FailsWithNoAccessToken",
-			provider:  provider(),
-			session:   &slack.Session{AccessToken: ""},
-			handler:   nil,
-			expectErr: true,
-		},
-		{
-			name:     "FailsWithBadAuthTestResponse",
-			provider: provider(),
-			session:  &slack.Session{AccessToken: "TOKEN"},
-			handler: http.HandlerFunc(
-				func(res http.ResponseWriter, req *http.Request) {
-					switch req.URL.Path {
-					case "/api/auth.test":
-						res.WriteHeader(http.StatusForbidden)
-					}
-				},
-			),
-			expectedUser: goth.User{
-				AccessToken: "TOKEN",
-			},
-			expectErr: true,
-		},
-		{
-			name:     "FailsWithBadUserInfoResponse",
-			provider: provider(),
-			session:  &slack.Session{AccessToken: "TOKEN"},
-			handler: http.HandlerFunc(
-				func(res http.ResponseWriter, req *http.Request) {
-					switch req.URL.Path {
-					case "/api/auth.test":
-						res.WriteHeader(http.StatusOK)
-						json.NewEncoder(res).Encode(testAuthTestResponseData)
-					case "/api/users.info":
-						res.WriteHeader(http.StatusForbidden)
-					}
-				},
-			),
-			expectedUser: goth.User{
-				UserID:      "user1234",
-				NickName:    "testuser",
-				AccessToken: "TOKEN",
-			},
-			expectErr: true,
-		},
-	} {
-		t.Run(testData.name, func(t *testing.T) {
-			a := assert.New(t)
-
-			withMockServer(testData.provider, testData.handler, func(p *slack.Provider) {
-				user, err := p.FetchUser(testData.session)
-				a.NotZero(user)
-
-				if testData.expectErr {
-					a.Error(err)
-				} else {
-					a.NoError(err)
-				}
-
-				a.Equal(testData.expectedUser.UserID, user.UserID)
-				a.Equal(testData.expectedUser.NickName, user.NickName)
-				a.Equal(testData.expectedUser.Name, user.Name)
-				a.Equal(testData.expectedUser.FirstName, user.FirstName)
-				a.Equal(testData.expectedUser.LastName, user.LastName)
-				a.Equal(testData.expectedUser.AvatarURL, user.AvatarURL)
-				a.Equal(testData.expectedUser.Email, user.Email)
-				a.Equal(testData.expectedUser.AccessToken, user.AccessToken)
-			})
-		})
-	}
-}
-
 func Test_SessionFromJSON(t *testing.T) {
 	t.Parallel()
 	a := assert.New(t)
@@ -214,23 +50,3 @@ func Test_SessionFromJSON(t *testing.T) {
 func provider() *slack.Provider {
 	return slack.New(os.Getenv("SLACK_KEY"), os.Getenv("SLACK_SECRET"), "/foo")
 }
-
-func withMockServer(p *slack.Provider, handler http.Handler, fn func(p *slack.Provider)) {
-	server := httptest.NewTLSServer(handler)
-	defer server.Close()
-
-	httpClient := &http.Client{
-		Transport: &http.Transport{
-			DialContext: func(ctx context.Context, network, _ string) (net.Conn, error) {
-				return net.Dial(network, server.Listener.Addr().String())
-			},
-			TLSClientConfig: &tls.Config{
-				InsecureSkipVerify: true,
-			},
-		},
-	}
-
-	p.HTTPClient = httpClient
-
-	fn(p)
-}