From b978ed12ebbfd4df5234d3c3ac399c772607a78b Mon Sep 17 00:00:00 2001 From: Brad Rydzewski Date: Sat, 30 Apr 2016 23:22:30 -0700 Subject: [PATCH] increased bitbucket test coverage --- remote/bitbucket/bitbucket.go | 209 +++++++++++++++------------ remote/bitbucket/bitbucket_test.go | 104 +++++++++++-- remote/bitbucket/fixtures/handler.go | 40 +++++ remote/bitbucket/parse_test.go | 6 +- remote/remote.go | 13 +- 5 files changed, 258 insertions(+), 114 deletions(-) diff --git a/remote/bitbucket/bitbucket.go b/remote/bitbucket/bitbucket.go index 7f8470212..c7acc38ed 100644 --- a/remote/bitbucket/bitbucket.go +++ b/remote/bitbucket/bitbucket.go @@ -11,13 +11,16 @@ import ( "github.com/drone/drone/shared/httputil" "golang.org/x/oauth2" - "golang.org/x/oauth2/bitbucket" ) -// Bitbucket Server endpoint. -const Endpoint = "https://api.bitbucket.org" +// Bitbucket cloud endpoints. +const ( + DefaultAPI = "https://api.bitbucket.org" + DefaultURL = "https://bitbucket.org" +) type config struct { + API string URL string Client string Secret string @@ -27,63 +30,42 @@ type config struct { // repository hosting service at https://bitbucket.org func New(client, secret string) remote.Remote { return &config{ - URL: Endpoint, + API: DefaultAPI, + URL: DefaultURL, Client: client, Secret: secret, } } -// helper function to return the bitbucket oauth2 client -func (c *config) newClient(u *model.User) *internal.Client { - return internal.NewClientToken( - c.URL, - c.Client, - c.Secret, - &oauth2.Token{ - AccessToken: u.Token, - RefreshToken: u.Secret, - }, - ) -} +// 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) + config := c.newConfig(redirect) -func (c *config) Login(res http.ResponseWriter, req *http.Request) (*model.User, error) { - config := &oauth2.Config{ - ClientID: c.Client, - ClientSecret: c.Secret, - Endpoint: bitbucket.Endpoint, - RedirectURL: fmt.Sprintf("%s/authorize", httputil.GetURL(req)), - } - - var code = req.FormValue("code") + code := r.FormValue("code") if len(code) == 0 { - http.Redirect(res, req, config.AuthCodeURL("drone"), http.StatusSeeOther) + http.Redirect(w, r, config.AuthCodeURL("drone"), http.StatusSeeOther) return nil, nil } - var token, err = config.Exchange(oauth2.NoContext, code) + token, err := config.Exchange(oauth2.NoContext, code) if err != nil { return nil, err } - client := internal.NewClient(c.URL, config.Client(oauth2.NoContext, token)) + client := internal.NewClient(c.API, config.Client(oauth2.NoContext, token)) curr, err := client.FindCurrent() if err != nil { return nil, err } - return convertUser(curr, token), nil } +// Auth uses the Bitbucket oauth2 access token and refresh token to authenticate +// a session and return the Bitbucket account login. func (c *config) Auth(token, secret string) (string, error) { - client := internal.NewClientToken( - c.URL, - c.Client, - c.Secret, - &oauth2.Token{ - AccessToken: token, - RefreshToken: secret, - }, - ) + client := c.newClientToken(token, secret) user, err := client.FindCurrent() if err != nil { return "", err @@ -91,33 +73,25 @@ func (c *config) Auth(token, secret string) (string, error) { return user.Login, nil } +// Refresh refreshes the Bitbucket oauth2 access token. If the token is +// refreshed the user is updated and a true value is returned. func (c *config) Refresh(user *model.User) (bool, error) { - config := &oauth2.Config{ - ClientID: c.Client, - ClientSecret: c.Secret, - Endpoint: bitbucket.Endpoint, - } - - // creates a token source with just the refresh token. - // this will ensure an access token is automatically - // requested. + config := c.newConfig("") source := config.TokenSource( oauth2.NoContext, &oauth2.Token{RefreshToken: user.Secret}) - // requesting the token automatically refreshes and - // returns a new access token. token, err := source.Token() if err != nil || len(token.AccessToken) == 0 { return false, err } - // update the user to include tne new access token user.Token = token.AccessToken user.Secret = token.RefreshToken user.Expiry = token.Expiry.UTC().Unix() return true, nil } +// Teams returns a list of all team membership for the Bitbucket account. func (c *config) Teams(u *model.User) ([]*model.Team, error) { opts := &internal.ListTeamOpts{ PageLen: 100, @@ -130,6 +104,7 @@ func (c *config) Teams(u *model.User) ([]*model.Team, error) { return convertTeamList(resp.Values), nil } +// Repo returns the named Bitbucket repository. func (c *config) Repo(u *model.User, owner, name string) (*model.Repo, error) { repo, err := c.newClient(u).FindRepo(owner, name) if err != nil { @@ -138,36 +113,43 @@ func (c *config) Repo(u *model.User, owner, name string) (*model.Repo, error) { return convertRepo(repo), nil } +// Repos returns a list of all repositories for Bitbucket account, including +// organization repositories. func (c *config) Repos(u *model.User) ([]*model.RepoLite, error) { client := c.newClient(u) - var repos []*model.RepoLite + var all []*model.RepoLite - // gets a list of all accounts to query, including the - // user's account and all team accounts. - logins := []string{u.Login} - resp, err := client.ListTeams(&internal.ListTeamOpts{PageLen: 100, Role: "member"}) + accounts := []string{u.Login} + resp, err := client.ListTeams(&internal.ListTeamOpts{ + PageLen: 100, + Role: "member", + }) if err != nil { - return repos, err + return all, err } for _, team := range resp.Values { - logins = append(logins, team.Login) + accounts = append(accounts, team.Login) } // for each account, get the list of repos - for _, login := range logins { - repos_, err := client.ListReposAll(login) + for _, account := range accounts { + repos, err := client.ListReposAll(account) if err != nil { - return repos, err + return all, err } - for _, repo := range repos_ { - repos = append(repos, convertRepoLite(repo)) + for _, repo := range repos { + all = append(all, convertRepoLite(repo)) } } - return repos, nil + return all, nil } +// Perm returns the user permissions for the named repository. Because Bitbucket +// does not have an endpoint to access user permissions, we attempt to fetch +// the repository hook list, which is restricted to administrators to calculate +// administrative access to a repository. func (c *config) Perm(u *model.User, owner, name string) (*model.Perm, error) { client := c.newClient(u) @@ -177,20 +159,17 @@ func (c *config) Perm(u *model.User, owner, name string) (*model.Perm, error) { return perms, err } - // if we've gotten this far we know that the user at least has read access - // to the repository. - perms.Pull = true - - // if the user has access to the repository hooks we can deduce that the user - // has push and admin access. _, err = client.ListHooks(owner, name, &internal.ListOpts{}) if err == nil { perms.Push = true perms.Admin = true } + perms.Pull = true return perms, nil } +// File fetches the file from the Bitbucket repository and returns its contents +// in string format. func (c *config) File(u *model.User, r *model.Repo, b *model.Build, f string) ([]byte, error) { config, err := c.newClient(u).FindSource(r.Owner, r.Name, b.Commit, f) if err != nil { @@ -199,6 +178,7 @@ func (c *config) File(u *model.User, r *model.Repo, b *model.Build, f string) ([ return []byte(config.Data), err } +// Status creates a build status for the Bitbucket commit. func (c *config) Status(u *model.User, r *model.Repo, b *model.Build, link string) error { status := internal.BuildStatus{ State: convertStatus(b.Status), @@ -209,21 +189,14 @@ func (c *config) Status(u *model.User, r *model.Repo, b *model.Build, link strin return c.newClient(u).CreateStatus(r.Owner, r.Name, b.Commit, &status) } -func (c *config) Netrc(u *model.User, r *model.Repo) (*model.Netrc, error) { - return &model.Netrc{ - Machine: "bitbucket.org", - Login: "x-token-auth", - Password: u.Token, - }, nil -} - -func (c *config) Activate(u *model.User, r *model.Repo, k *model.Key, link string) error { +// Activate activates the repository by registering repository push hooks with +// the Bitbucket repository. Prior to registering hook, previously created hooks +// are deleted. +func (c *config) Activate(u *model.User, r *model.Repo, link string) error { rawurl, err := url.Parse(link) if err != nil { return err } - - // deletes any previously created hooks c.Deactivate(u, r, link) return c.newClient(u).CreateHook(r.Owner, r.Name, &internal.Hook{ @@ -234,32 +207,80 @@ func (c *config) Activate(u *model.User, r *model.Repo, k *model.Key, link strin }) } +// Deactivate deactives the repository be removing repository push hooks from +// the Bitbucket repository. func (c *config) Deactivate(u *model.User, r *model.Repo, link string) error { client := c.newClient(u) - linkurl, err := url.Parse(link) - if err != nil { - return err - } - hooks, err := client.ListHooks(r.Owner, r.Name, &internal.ListOpts{}) if err != nil { return err } - - for _, hook := range hooks.Values { - hookurl, err := url.Parse(hook.Url) - if err == nil && hookurl.Host == linkurl.Host { - return client.DeleteHook(r.Owner, r.Name, hook.Uuid) - } + hook := matchingHooks(hooks.Values, link) + if hook != nil { + return client.DeleteHook(r.Owner, r.Name, hook.Uuid) } - return nil } +// Netrc returns a netrc file capable of authenticating Bitbucket requests and +// cloning Bitbucket repositories. +func (c *config) Netrc(u *model.User, r *model.Repo) (*model.Netrc, error) { + return &model.Netrc{ + Machine: "bitbucket.org", + Login: "x-token-auth", + Password: u.Token, + }, nil +} + // Hook parses the incoming Bitbucket hook and returns the Repository and -// Build details. If the hook is unsupported nil values are returned and the -// hook should be skipped. +// 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) } + +// helper function to return the bitbucket oauth2 client +func (c *config) newClient(u *model.User) *internal.Client { + return c.newClientToken(u.Token, u.Secret) +} + +// helper function to return the bitbucket oauth2 client +func (c *config) newClientToken(token, secret string) *internal.Client { + return internal.NewClientToken( + c.API, + c.Client, + c.Secret, + &oauth2.Token{ + AccessToken: token, + RefreshToken: secret, + }, + ) +} + +// helper function to return the bitbucket oauth2 config +func (c *config) newConfig(redirect string) *oauth2.Config { + return &oauth2.Config{ + ClientID: c.Client, + ClientSecret: c.Secret, + Endpoint: oauth2.Endpoint{ + AuthURL: fmt.Sprintf("%s/site/oauth2/authorize", c.URL), + TokenURL: fmt.Sprintf("%s/site/oauth2/access_token", c.URL), + }, + RedirectURL: fmt.Sprintf("%s/authorize", redirect), + } +} + +// helper function to return matching hooks. +func matchingHooks(hooks []*internal.Hook, rawurl string) *internal.Hook { + link, err := url.Parse(rawurl) + if err != nil { + return nil + } + for _, hook := range hooks { + hookurl, err := url.Parse(hook.Url) + if err == nil && hookurl.Host == link.Host { + return hook + } + } + return nil +} diff --git a/remote/bitbucket/bitbucket_test.go b/remote/bitbucket/bitbucket_test.go index 6d621788e..ea1a8281a 100644 --- a/remote/bitbucket/bitbucket_test.go +++ b/remote/bitbucket/bitbucket_test.go @@ -8,6 +8,7 @@ import ( "github.com/drone/drone/model" "github.com/drone/drone/remote/bitbucket/fixtures" + "github.com/drone/drone/remote/bitbucket/internal" "github.com/franela/goblin" "github.com/gin-gonic/gin" @@ -17,7 +18,7 @@ func Test_bitbucket(t *testing.T) { gin.SetMode(gin.TestMode) s := httptest.NewServer(fixtures.Handler()) - c := &config{URL: s.URL} + c := &config{URL: s.URL, API: s.URL} g := goblin.Goblin(t) g.Describe("Bitbucket client", func() { @@ -28,10 +29,12 @@ func Test_bitbucket(t *testing.T) { g.It("Should return client with default endpoint", func() { remote := New("4vyW6b49Z", "a5012f6c6") - g.Assert(remote.(*config).URL).Equal("https://api.bitbucket.org") + g.Assert(remote.(*config).URL).Equal(DefaultURL) + g.Assert(remote.(*config).API).Equal(DefaultAPI) g.Assert(remote.(*config).Client).Equal("4vyW6b49Z") g.Assert(remote.(*config).Secret).Equal("a5012f6c6") }) + g.It("Should return the netrc file", func() { remote := New("", "") netrc, _ := remote.Netrc(fakeUser, nil) @@ -40,6 +43,35 @@ func Test_bitbucket(t *testing.T) { g.Assert(netrc.Password).Equal(fakeUser.Token) }) + g.Describe("Given an authorization request", func() { + g.It("Should redirect to authorize", func() { + w := httptest.NewRecorder() + r, _ := http.NewRequest("GET", "", nil) + _, err := c.Login(w, r) + g.Assert(err == nil).IsTrue() + g.Assert(w.Code).Equal(http.StatusSeeOther) + }) + g.It("Should return authenticated user", func() { + r, _ := http.NewRequest("GET", "?code=code", nil) + u, err := c.Login(nil, r) + g.Assert(err == nil).IsTrue() + g.Assert(u.Login).Equal(fakeUser.Login) + g.Assert(u.Token).Equal("2YotnFZFEjr1zCsicMWpAA") + g.Assert(u.Secret).Equal("tGzv3JOkF0XG5Qx2TlKWIA") + }) + g.It("Should handle failure to exchange code", func() { + w := httptest.NewRecorder() + r, _ := http.NewRequest("GET", "?code=code_bad_request", nil) + _, err := c.Login(w, r) + g.Assert(err != nil).IsTrue() + }) + g.It("Should handle failure to resolve user", func() { + r, _ := http.NewRequest("GET", "?code=code_user_not_found", nil) + _, err := c.Login(nil, r) + g.Assert(err != nil).IsTrue() + }) + }) + g.Describe("Given an access token", func() { g.It("Should return the authenticated user", func() { login, err := c.Auth( @@ -49,7 +81,7 @@ func Test_bitbucket(t *testing.T) { g.Assert(err == nil).IsTrue() g.Assert(login).Equal(fakeUser.Login) }) - g.It("Should return error when request fails", func() { + g.It("Should handle a failure to resolve user", func() { _, err := c.Auth( fakeUserNotFound.Token, fakeUserNotFound.Secret, @@ -58,6 +90,26 @@ func Test_bitbucket(t *testing.T) { }) }) + g.Describe("Given a refresh token", func() { + g.It("Should return a refresh access token", func() { + ok, err := c.Refresh(fakeUserRefresh) + g.Assert(err == nil).IsTrue() + g.Assert(ok).IsTrue() + g.Assert(fakeUserRefresh.Token).Equal("2YotnFZFEjr1zCsicMWpAA") + g.Assert(fakeUserRefresh.Secret).Equal("tGzv3JOkF0XG5Qx2TlKWIA") + }) + g.It("Should handle an empty access token", func() { + ok, err := c.Refresh(fakeUserRefreshEmpty) + g.Assert(err == nil).IsTrue() + g.Assert(ok).IsFalse() + }) + g.It("Should handle a failure to refresh", func() { + ok, err := c.Refresh(fakeUserRefreshFail) + g.Assert(err != nil).IsTrue() + g.Assert(ok).IsFalse() + }) + }) + g.Describe("When requesting a repository", func() { g.It("Should return the details", func() { repo, err := c.Repo( @@ -154,21 +206,16 @@ func Test_bitbucket(t *testing.T) { g.Describe("When activating a repository", func() { g.It("Should error when malformed hook", func() { - err := c.Activate(fakeUser, fakeRepo, nil, "%gh&%ij") + err := c.Activate(fakeUser, fakeRepo, "%gh&%ij") g.Assert(err != nil).IsTrue() }) g.It("Should create the hook", func() { - err := c.Activate(fakeUser, fakeRepo, nil, "http://127.0.0.1") + err := c.Activate(fakeUser, fakeRepo, "http://127.0.0.1") g.Assert(err == nil).IsTrue() }) - g.It("Should remove previous hooks") }) g.Describe("When deactivating a repository", func() { - g.It("Should error when malformed hook", func() { - err := c.Deactivate(fakeUser, fakeRepo, "%gh&%ij") - g.Assert(err != nil).IsTrue() - }) g.It("Should error when listing hooks fails", func() { err := c.Deactivate(fakeUser, fakeRepoNoHooks, "http://127.0.0.1") g.Assert(err != nil).IsTrue() @@ -183,6 +230,28 @@ func Test_bitbucket(t *testing.T) { }) }) + g.Describe("Given a list of hooks", func() { + g.It("Should return the matching hook", func() { + hooks := []*internal.Hook{ + {Url: "http://127.0.0.1/hook"}, + } + hook := matchingHooks(hooks, "http://127.0.0.1/") + g.Assert(hook).Equal(hooks[0]) + }) + g.It("Should handle no matches", func() { + hooks := []*internal.Hook{ + {Url: "http://localhost/hook"}, + } + hook := matchingHooks(hooks, "http://127.0.0.1/") + g.Assert(hook == nil).IsTrue() + }) + g.It("Should handle malformed hook urls", func() { + var hooks []*internal.Hook + hook := matchingHooks(hooks, "%gh&%ij") + g.Assert(hook == nil).IsTrue() + }) + }) + g.It("Should update the status", func() { err := c.Status(fakeUser, fakeRepo, fakeBuild, "http://127.0.0.1") g.Assert(err == nil).IsTrue() @@ -208,6 +277,21 @@ var ( Token: "cfcd2084", } + fakeUserRefresh = &model.User{ + Login: "superman", + Secret: "cfcd2084", + } + + fakeUserRefreshFail = &model.User{ + Login: "superman", + Secret: "refresh_token_not_found", + } + + fakeUserRefreshEmpty = &model.User{ + Login: "superman", + Secret: "refresh_token_is_empty", + } + fakeUserNotFound = &model.User{ Login: "superman", Token: "user_not_found", diff --git a/remote/bitbucket/fixtures/handler.go b/remote/bitbucket/fixtures/handler.go index a1ce21911..f5cbbb40e 100644 --- a/remote/bitbucket/fixtures/handler.go +++ b/remote/bitbucket/fixtures/handler.go @@ -12,6 +12,7 @@ func Handler() http.Handler { gin.SetMode(gin.TestMode) e := gin.New() + e.POST("/site/oauth2/access_token", getOauth) e.GET("/2.0/repositories/:owner/:name", getRepo) e.GET("/2.0/repositories/:owner/:name/hooks", getRepoHooks) e.GET("/1.0/repositories/:owner/:name/src/:commit/:file", getRepoFile) @@ -25,6 +26,27 @@ func Handler() http.Handler { return e } +func getOauth(c *gin.Context) { + switch c.PostForm("code") { + case "code_bad_request": + c.String(500, "") + return + case "code_user_not_found": + c.String(200, tokenNotFoundPayload) + return + } + switch c.PostForm("refresh_token") { + case "refresh_token_not_found": + c.String(404, "") + case "refresh_token_is_empty": + c.Header("Content-Type", "application/json") + c.String(200, "{}") + default: + c.Header("Content-Type", "application/json") + c.String(200, tokenPayload) + } +} + func getRepo(c *gin.Context) { switch c.Param("name") { case "not_found", "repo_unknown", "repo_not_found": @@ -103,6 +125,24 @@ func getUserRepos(c *gin.Context) { } } +const tokenPayload = ` +{ + "access_token":"2YotnFZFEjr1zCsicMWpAA", + "refresh_token":"tGzv3JOkF0XG5Qx2TlKWIA", + "token_type":"Bearer", + "expires_in":3600 +} +` + +const tokenNotFoundPayload = ` +{ + "access_token":"user_not_found", + "refresh_token":"user_not_found", + "token_type":"Bearer", + "expires_in":3600 +} +` + const repoPayload = ` { "full_name": "test_name/repo_name", diff --git a/remote/bitbucket/parse_test.go b/remote/bitbucket/parse_test.go index 7ff3657be..eee2bda36 100644 --- a/remote/bitbucket/parse_test.go +++ b/remote/bitbucket/parse_test.go @@ -13,7 +13,7 @@ import ( func Test_parser(t *testing.T) { g := goblin.Goblin(t) - g.Describe("Bitbucket hook parser", func() { + g.Describe("Bitbucket parser", func() { g.It("Should ignore unsupported hook", func() { buf := bytes.NewBufferString(fixtures.HookPush) @@ -27,7 +27,7 @@ func Test_parser(t *testing.T) { g.Assert(err == nil).IsTrue() }) - g.Describe("Given a pull request hook", func() { + g.Describe("Given a pull request hook payload", func() { g.It("Should return err when malformed", func() { buf := bytes.NewBufferString("[]") @@ -64,7 +64,7 @@ func Test_parser(t *testing.T) { }) }) - g.Describe("Given a push hook", func() { + g.Describe("Given a push hook payload", func() { g.It("Should return err when malformed", func() { buf := bytes.NewBufferString("[]") diff --git a/remote/remote.go b/remote/remote.go index a1255104d..cfcdafdf0 100644 --- a/remote/remote.go +++ b/remote/remote.go @@ -44,12 +44,11 @@ type Remote interface { // private repositories from a remote system. Netrc(u *model.User, r *model.Repo) (*model.Netrc, error) - // Activate activates a repository by creating the post-commit hook and - // adding the SSH deploy key, if applicable. - Activate(u *model.User, r *model.Repo, k *model.Key, link string) error + // Activate activates a repository by creating the post-commit hook. + Activate(u *model.User, r *model.Repo, link string) error - // Deactivate removes a repository by removing all the post-commit hooks - // which are equal to link and removing the SSH deploy key. + // Deactivate deactivates a repository by removing all previously created + // post-commit hooks matching the given link. Deactivate(u *model.User, r *model.Repo, link string) error // Hook parses the post-commit hook from the Request body and returns the @@ -116,8 +115,8 @@ func Netrc(c context.Context, u *model.User, r *model.Repo) (*model.Netrc, error // Activate activates a repository by creating the post-commit hook and // adding the SSH deploy key, if applicable. -func Activate(c context.Context, u *model.User, r *model.Repo, k *model.Key, link string) error { - return FromContext(c).Activate(u, r, k, link) +func Activate(c context.Context, u *model.User, r *model.Repo, link string) error { + return FromContext(c).Activate(u, r, link) } // Deactivate removes a repository by removing all the post-commit hooks