Merge pull request harness#1887 from AlekSi/master

Expose OAuth2 errors, avoid redirect loop.

remote/bitbucket/bitbucket.go

@@ -39,13 +39,23 @@ func New(client, secret string) remote.Remote {
 
 // Login authenticates an account with Bitbucket using the oauth2 protocol. The
 // Bitbucket account details are returned when the user is successfully authenticated.
-func (c *config) Login(w http.ResponseWriter, r *http.Request) (*model.User, error) {
-	redirect := httputil.GetURL(r)
+func (c *config) Login(w http.ResponseWriter, req *http.Request) (*model.User, error) {
+	redirect := httputil.GetURL(req)
 	config := c.newConfig(redirect)
 
-	code := r.FormValue("code")
+	// get the OAuth errors
+	if err := req.FormValue("error"); err != "" {
+		return nil, &remote.AuthError{
+			Err:         err,
+			Description: req.FormValue("error_description"),
+			URI:         req.FormValue("error_uri"),
+		}
+	}
+
+	// get the OAuth code
+	code := req.FormValue("code")
 	if len(code) == 0 {
-		http.Redirect(w, r, config.AuthCodeURL("drone"), http.StatusSeeOther)
+		http.Redirect(w, req, config.AuthCodeURL("drone"), http.StatusSeeOther)
 		return nil, nil
 	}
 
@@ -237,8 +247,8 @@ func (c *config) Netrc(u *model.User, r *model.Repo) (*model.Netrc, error) {
 
 // Hook parses the incoming Bitbucket hook and returns the Repository and
 // Build details. If the hook is unsupported nil values are returned.
-func (c *config) Hook(r *http.Request) (*model.Repo, *model.Build, error) {
-	return parseHook(r)
+func (c *config) Hook(req *http.Request) (*model.Repo, *model.Build, error) {
+	return parseHook(req)
 }
 
 // helper function to return the bitbucket oauth2 client

remote/bitbucket/bitbucket_test.go

@@ -70,6 +70,11 @@ func Test_bitbucket(t *testing.T) {
 				_, err := c.Login(nil, r)
 				g.Assert(err != nil).IsTrue()
 			})
+			g.It("Should handle authentication errors", func() {
+				r, _ := http.NewRequest("GET", "?error=invalid_scope", nil)
+				_, err := c.Login(nil, r)
+				g.Assert(err != nil).IsTrue()
+			})
 		})
 
 		g.Describe("Given an access token", func() {

remote/bitbucket/fixtures/handler.go

@@ -27,6 +27,11 @@ func Handler() http.Handler {
 }
 
 func getOauth(c *gin.Context) {
+	switch c.PostForm("error") {
+	case "invalid_scope":
+		c.String(500, "")
+	}
+
 	switch c.PostForm("code") {
 	case "code_bad_request":
 		c.String(500, "")

remote/errors.go

@@ -0,0 +1,23 @@
+package remote
+
+// AuthError represents remote authentication error.
+type AuthError struct {
+	Err         string
+	Description string
+	URI         string
+}
+
+// Error implements error interface.
+func (ae *AuthError) Error() string {
+	err := ae.Err
+	if ae.Description != "" {
+		err += " " + ae.Description
+	}
+	if ae.URI != "" {
+		err += " " + ae.URI
+	}
+	return err
+}
+
+// check interface
+var _ error = new(AuthError)

remote/github/github.go

@@ -92,6 +92,16 @@ type client struct {
 func (c *client) Login(res http.ResponseWriter, req *http.Request) (*model.User, error) {
 	config := c.newConfig(httputil.GetURL(req))
 
+	// get the OAuth errors
+	if err := req.FormValue("error"); err != "" {
+		return nil, &remote.AuthError{
+			Err:         err,
+			Description: req.FormValue("error_description"),
+			URI:         req.FormValue("error_uri"),
+		}
+	}
+
+	// get the OAuth code
 	code := req.FormValue("code")
 	if len(code) == 0 {
 		// TODO(bradrydzewski) we really should be using a random value here and

remote/github/github_test.go

@@ -140,6 +140,7 @@ func Test_github(t *testing.T) {
 			g.It("Should create an access token")
 			g.It("Should handle an access token error")
 			g.It("Should return the authenticated user")
+			g.It("Should handle authentication errors")
 		})
 	})
 }

remote/gitlab/gitlab.go

@@ -115,6 +115,15 @@ func (g *Gitlab) Login(res http.ResponseWriter, req *http.Request) (*model.User,
 		TLSClientConfig: &tls.Config{InsecureSkipVerify: g.SkipVerify},
 	}
 
+	// get the OAuth errors
+	if err := req.FormValue("error"); err != "" {
+		return nil, &remote.AuthError{
+			Err:         err,
+			Description: req.FormValue("error_description"),
+			URI:         req.FormValue("error_uri"),
+		}
+	}
+
 	// get the OAuth code
 	var code = req.FormValue("code")
 	if len(code) == 0 {

shared/oauth2/oauth2.go

@@ -218,6 +218,9 @@ func (c *Config) AuthCodeURL(state string) string {
 	if err != nil {
 		panic("AuthURL malformed: " + err.Error())
 	}
+	if err := url_.Query().Get("error"); err != "" {
+		panic("AuthURL contains error: " + err)
+	}
 	q := url.Values{
 		"response_type":   {"code"},
 		"client_id":       {c.ClientId},