diff --git a/remote/bitbucket/bitbucket.go b/remote/bitbucket/bitbucket.go index cff6bdfb7..a03f8ab71 100644 --- a/remote/bitbucket/bitbucket.go +++ b/remote/bitbucket/bitbucket.go @@ -1,6 +1,7 @@ package bitbucket import ( + "errors" "fmt" "net/http" "net/url" @@ -39,13 +40,27 @@ 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 != "" { + description := req.FormValue("error_description") + if description != "" { + err += " " + description + } + uri := req.FormValue("error_uri") + if uri != "" { + err += " " + uri + } + return nil, errors.New(err) + } + + // 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 +252,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 diff --git a/remote/bitbucket/bitbucket_test.go b/remote/bitbucket/bitbucket_test.go index ea1a8281a..f00b19875 100644 --- a/remote/bitbucket/bitbucket_test.go +++ b/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() { diff --git a/remote/bitbucket/fixtures/handler.go b/remote/bitbucket/fixtures/handler.go index f5cbbb40e..96aff73e0 100644 --- a/remote/bitbucket/fixtures/handler.go +++ b/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, "") diff --git a/remote/github/github.go b/remote/github/github.go index ba69cac56..fec783703 100644 --- a/remote/github/github.go +++ b/remote/github/github.go @@ -2,6 +2,7 @@ package github import ( "crypto/tls" + "errors" "fmt" "net" "net/http" @@ -92,6 +93,20 @@ 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 != "" { + description := req.FormValue("error_description") + if description != "" { + err += " " + description + } + uri := req.FormValue("error_uri") + if uri != "" { + err += " " + uri + } + return nil, errors.New(err) + } + + // get the OAuth code code := req.FormValue("code") if len(code) == 0 { // TODO(bradrydzewski) we really should be using a random value here and diff --git a/remote/github/github_test.go b/remote/github/github_test.go index 83e41f60d..8ba0a949b 100644 --- a/remote/github/github_test.go +++ b/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") }) }) } diff --git a/remote/gitlab/gitlab.go b/remote/gitlab/gitlab.go index 543b32bb8..c41993ef2 100644 --- a/remote/gitlab/gitlab.go +++ b/remote/gitlab/gitlab.go @@ -2,6 +2,7 @@ package gitlab import ( "crypto/tls" + "errors" "fmt" "io/ioutil" "net" @@ -115,6 +116,19 @@ 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 != "" { + description := req.FormValue("error_description") + if description != "" { + err += " " + description + } + uri := req.FormValue("error_uri") + if uri != "" { + err += " " + uri + } + return nil, errors.New(err) + } + // get the OAuth code var code = req.FormValue("code") if len(code) == 0 { diff --git a/shared/oauth2/oauth2.go b/shared/oauth2/oauth2.go index 97059c499..d69d74848 100644 --- a/shared/oauth2/oauth2.go +++ b/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},