transport: remove SetAuth, fixes #206 (#210)

* remove SetAuth functions, implement at NewUploadPackSession/NewReceivePackSession level.
* propagate transport.Auth from Fetch/Pull/Clone options to the transport API.

Author: amlweems

options.go

@@ -22,7 +22,7 @@ var (
 type CloneOptions struct {
 	// The (possibly remote) repository URL to clone from
 	URL string
-	// Auth credentials, if required, to uses with the remote repository
+	// Auth credentials, if required, to use with the remote repository
 	Auth transport.AuthMethod
 	// Name of the remote to be added, by default `origin`
 	RemoteName string
@@ -61,6 +61,8 @@ type PullOptions struct {
 	SingleBranch bool
 	// Limit fetching to the specified number of commits.
 	Depth int
+	// Auth credentials, if required, to use with the remote repository
+	Auth transport.AuthMethod
 }
 
 // Validate validate the fields and set the default values.
@@ -84,6 +86,8 @@ type FetchOptions struct {
 	// Depth limit fetching to the specified number of commits from the tip of
 	// each remote branch history.
 	Depth int
+	// Auth credentials, if required, to use with the remote repository
+	Auth transport.AuthMethod
 }
 
 // Validate validate the fields and set the default values
@@ -108,6 +112,8 @@ type PushOptions struct {
 	// RefSpecs specify what destination ref to update with what source
 	// object. A refspec with empty src can be used to delete a reference.
 	RefSpecs []config.RefSpec
+	// Auth credentials, if required, to use with the remote repository
+	Auth transport.AuthMethod
 }
 
 // Validate validate the fields and set the default values

plumbing/transport/client/client_test.go

@@ -58,12 +58,12 @@ type dummyClient struct {
 	*http.Client
 }
 
-func (*dummyClient) NewUploadPackSession(transport.Endpoint) (
+func (*dummyClient) NewUploadPackSession(transport.Endpoint, transport.AuthMethod) (
 	transport.UploadPackSession, error) {
 	return nil, nil
 }
 
-func (*dummyClient) NewReceivePackSession(transport.Endpoint) (
+func (*dummyClient) NewReceivePackSession(transport.Endpoint, transport.AuthMethod) (
 	transport.ReceivePackSession, error) {
 	return nil, nil
 }

plumbing/transport/common.go

@@ -30,6 +30,7 @@ var (
 	ErrAuthorizationRequired  = errors.New("authorization required")
 	ErrEmptyUploadPackRequest = errors.New("empty git-upload-pack given")
 	ErrInvalidAuthMethod      = errors.New("invalid auth method")
+	ErrAlreadyConnected       = errors.New("session already established")
 )
 
 const (
@@ -41,9 +42,9 @@ const (
 // It is implemented both by the client and the server, making this a RPC.
 type Transport interface {
 	// NewUploadPackSession starts a git-upload-pack session for an endpoint.
-	NewUploadPackSession(Endpoint) (UploadPackSession, error)
+	NewUploadPackSession(Endpoint, AuthMethod) (UploadPackSession, error)
 	// NewReceivePackSession starts a git-receive-pack session for an endpoint.
-	NewReceivePackSession(Endpoint) (ReceivePackSession, error)
+	NewReceivePackSession(Endpoint, AuthMethod) (ReceivePackSession, error)
 }
 
 type Session interface {
@@ -52,8 +53,6 @@ type Session interface {
 	// If the repository does not exist, returns ErrRepositoryNotFound.
 	// If the repository exists, but is empty, returns ErrEmptyRemoteRepository.
 	AdvertisedReferences() (*packp.AdvRefs, error)
-	//TODO: Move to Client level.
-	SetAuth(auth AuthMethod) error
 	io.Closer
 }
 

plumbing/transport/file/client.go

@@ -28,13 +28,18 @@ func NewClient(uploadPackBin, receivePackBin string) transport.Transport {
 	})
 }
 
-func (r *runner) Command(cmd string, ep transport.Endpoint) (common.Command, error) {
+func (r *runner) Command(cmd string, ep transport.Endpoint, auth transport.AuthMethod) (common.Command, error) {
 	switch cmd {
 	case transport.UploadPackServiceName:
 		cmd = r.UploadPackBin
 	case transport.ReceivePackServiceName:
 		cmd = r.ReceivePackBin
 	}
+
+	if _, err := exec.LookPath(cmd); err != nil {
+		return nil, err
+	}
+
 	return &command{cmd: exec.Command(cmd, ep.Path)}, nil
 }
 
@@ -43,14 +48,6 @@ type command struct {
 	closed bool
 }
 
-func (c *command) SetAuth(auth transport.AuthMethod) error {
-	if auth != nil {
-		return transport.ErrInvalidAuthMethod
-	}
-
-	return nil
-}
-
 func (c *command) Start() error {
 	return c.cmd.Start()
 }

plumbing/transport/file/receive_pack_test.go

@@ -46,7 +46,7 @@ func (s *ReceivePackSuite) TestCommandNoOutput(c *C) {
 	}
 
 	client := NewClient("true", "true")
-	session, err := client.NewReceivePackSession(s.Endpoint)
+	session, err := client.NewReceivePackSession(s.Endpoint, s.EmptyAuth)
 	c.Assert(err, IsNil)
 	ar, err := session.AdvertisedReferences()
 	c.Assert(err, IsNil)
@@ -59,7 +59,7 @@ func (s *ReceivePackSuite) TestMalformedInputNoErrors(c *C) {
 	}
 
 	client := NewClient("yes", "yes")
-	session, err := client.NewReceivePackSession(s.Endpoint)
+	session, err := client.NewReceivePackSession(s.Endpoint, s.EmptyAuth)
 	c.Assert(err, IsNil)
 	ar, err := session.AdvertisedReferences()
 	c.Assert(err, NotNil)
@@ -69,7 +69,7 @@ func (s *ReceivePackSuite) TestMalformedInputNoErrors(c *C) {
 func (s *ReceivePackSuite) TestNonExistentCommand(c *C) {
 	cmd := "/non-existent-git"
 	client := NewClient(cmd, cmd)
-	session, err := client.NewReceivePackSession(s.Endpoint)
+	session, err := client.NewReceivePackSession(s.Endpoint, s.EmptyAuth)
 	c.Assert(err, ErrorMatches, ".*no such file or directory.*")
 	c.Assert(session, IsNil)
 }

plumbing/transport/file/server.go

@@ -19,7 +19,8 @@ func ServeUploadPack(path string) error {
 		return err
 	}
 
-	s, err := server.DefaultServer.NewUploadPackSession(ep)
+	// TODO: define and implement a server-side AuthMethod
+	s, err := server.DefaultServer.NewUploadPackSession(ep, nil)
 	if err != nil {
 		return fmt.Errorf("error creating session: %s", err)
 	}
@@ -36,7 +37,8 @@ func ServeReceivePack(path string) error {
 		return err
 	}
 
-	s, err := server.DefaultServer.NewReceivePackSession(ep)
+	// TODO: define and implement a server-side AuthMethod
+	s, err := server.DefaultServer.NewReceivePackSession(ep, nil)
 	if err != nil {
 		return fmt.Errorf("error creating session: %s", err)
 	}

plumbing/transport/file/upload_pack_test.go

@@ -52,7 +52,7 @@ func (s *UploadPackSuite) TestCommandNoOutput(c *C) {
 	}
 
 	client := NewClient("true", "true")
-	session, err := client.NewUploadPackSession(s.Endpoint)
+	session, err := client.NewUploadPackSession(s.Endpoint, s.EmptyAuth)
 	c.Assert(err, IsNil)
 	ar, err := session.AdvertisedReferences()
 	c.Assert(err, IsNil)
@@ -65,7 +65,7 @@ func (s *UploadPackSuite) TestMalformedInputNoErrors(c *C) {
 	}
 
 	client := NewClient("yes", "yes")
-	session, err := client.NewUploadPackSession(s.Endpoint)
+	session, err := client.NewUploadPackSession(s.Endpoint, s.EmptyAuth)
 	c.Assert(err, IsNil)
 	ar, err := session.AdvertisedReferences()
 	c.Assert(err, NotNil)
@@ -75,7 +75,7 @@ func (s *UploadPackSuite) TestMalformedInputNoErrors(c *C) {
 func (s *UploadPackSuite) TestNonExistentCommand(c *C) {
 	cmd := "/non-existent-git"
 	client := NewClient(cmd, cmd)
-	session, err := client.NewUploadPackSession(s.Endpoint)
+	session, err := client.NewUploadPackSession(s.Endpoint, s.EmptyAuth)
 	c.Assert(err, ErrorMatches, ".*no such file or directory.*")
 	c.Assert(session, IsNil)
 }

plumbing/transport/git/common.go

@@ -1,7 +1,6 @@
 package git
 
 import (
-	"errors"
 	"fmt"
 	"io"
 	"net"
@@ -13,22 +12,21 @@ import (
 	"gopkg.in/src-d/go-git.v4/utils/ioutil"
 )
 
-var (
-	errAlreadyConnected = errors.New("tcp connection already connected")
-)
-
 // DefaultClient is the default git client.
 var DefaultClient = common.NewClient(&runner{})
 
 type runner struct{}
 
 // Command returns a new Command for the given cmd in the given Endpoint
-func (r *runner) Command(cmd string, ep transport.Endpoint) (common.Command, error) {
+func (r *runner) Command(cmd string, ep transport.Endpoint, auth transport.AuthMethod) (common.Command, error) {
+	// auth not allowed since git protocol doesn't support authentication
+	if auth != nil {
+		return nil, transport.ErrInvalidAuthMethod
+	}
 	c := &command{command: cmd, endpoint: ep}
 	if err := c.connect(); err != nil {
 		return nil, err
 	}
-
 	return c, nil
 }
 
@@ -39,11 +37,6 @@ type command struct {
 	endpoint  transport.Endpoint
 }
 
-// SetAuth cannot be called since git protocol doesn't support authentication
-func (c *command) SetAuth(auth transport.AuthMethod) error {
-	return transport.ErrInvalidAuthMethod
-}
-
 // Start executes the command sending the required message to the TCP connection
 func (c *command) Start() error {
 	cmd := endpointToCommand(c.command, c.endpoint)
@@ -54,7 +47,7 @@ func (c *command) Start() error {
 
 func (c *command) connect() error {
 	if c.connected {
-		return errAlreadyConnected
+		return transport.ErrAlreadyConnected
 	}
 
 	var err error

plumbing/transport/http/common.go

@@ -35,16 +35,16 @@ func NewClient(c *http.Client) transport.Transport {
 	}
 }
 
-func (c *client) NewUploadPackSession(ep transport.Endpoint) (
+func (c *client) NewUploadPackSession(ep transport.Endpoint, auth transport.AuthMethod) (
 	transport.UploadPackSession, error) {
 
-	return newUploadPackSession(c.c, ep), nil
+	return newUploadPackSession(c.c, ep, auth)
 }
 
-func (c *client) NewReceivePackSession(ep transport.Endpoint) (
+func (c *client) NewReceivePackSession(ep transport.Endpoint, auth transport.AuthMethod) (
 	transport.ReceivePackSession, error) {
 
-	return newReceivePackSession(c.c, ep), nil
+	return newReceivePackSession(c.c, ep, auth)
 }
 
 type session struct {
@@ -54,16 +54,6 @@ type session struct {
 	advRefs  *packp.AdvRefs
 }
 
-func (s *session) SetAuth(auth transport.AuthMethod) error {
-	a, ok := auth.(AuthMethod)
-	if !ok {
-		return transport.ErrInvalidAuthMethod
-	}
-
-	s.auth = a
-	return nil
-}
-
 func (*session) Close() error {
 	return nil
 }

plumbing/transport/http/common_test.go

@@ -13,7 +13,8 @@ import (
 func Test(t *testing.T) { TestingT(t) }
 
 type ClientSuite struct {
-	Endpoint transport.Endpoint
+	Endpoint  transport.Endpoint
+	EmptyAuth transport.AuthMethod
 }
 
 var _ = Suite(&ClientSuite{})
@@ -76,9 +77,8 @@ func (s *ClientSuite) testNewHTTPError(c *C, code int, msg string) {
 
 func (s *ClientSuite) TestSetAuth(c *C) {
 	auth := &BasicAuth{}
-	r, err := DefaultClient.NewUploadPackSession(s.Endpoint)
+	r, err := DefaultClient.NewUploadPackSession(s.Endpoint, auth)
 	c.Assert(err, IsNil)
-	r.SetAuth(auth)
 	c.Assert(auth, Equals, r.(*upSession).auth)
 }
 
@@ -88,7 +88,6 @@ func (*mockAuth) Name() string   { return "" }
 func (*mockAuth) String() string { return "" }
 
 func (s *ClientSuite) TestSetAuthWrongType(c *C) {
-	r, err := DefaultClient.NewUploadPackSession(s.Endpoint)
-	c.Assert(err, IsNil)
-	c.Assert(r.SetAuth(&mockAuth{}), Equals, transport.ErrInvalidAuthMethod)
+	_, err := DefaultClient.NewUploadPackSession(s.Endpoint, &mockAuth{})
+	c.Assert(err, Equals, transport.ErrInvalidAuthMethod)
 }