From ebc811aafbf0e32069de49a69bf42be865b15980 Mon Sep 17 00:00:00 2001 From: Guiheux Steven Date: Fri, 27 Mar 2020 11:14:22 +0100 Subject: [PATCH] fix(api): check and grant user before creating pullrequest or comment (#5080) --- engine/api/workflow_queue.go | 13 --- .../bitbucketserver/client_pull_request.go | 17 ++- engine/vcs/bitbucketserver/client_repos.go | 26 +++++ engine/vcs/bitbucketserver/types.go | 12 +++ engine/vcs/github/client_pull_request.go | 20 ++++ engine/vcs/github/client_repos.go | 31 ++++++ engine/vcs/github/types.go | 101 ++++++------------ 7 files changed, 140 insertions(+), 80 deletions(-) diff --git a/engine/api/workflow_queue.go b/engine/api/workflow_queue.go index 61dd5a0cfb..ad52629886 100644 --- a/engine/api/workflow_queue.go +++ b/engine/api/workflow_queue.go @@ -499,19 +499,6 @@ func postJobResult(ctx context.Context, dbFunc func(context.Context) *gorp.DbMap } go WorkflowSendEvent(context.Background(), tx, store, *proj, reportParent) - - if sdk.StatusIsTerminated(run.Status) { - //Start a goroutine to update commit statuses in repositories manager - go func(wRun *sdk.WorkflowRun) { - //The function could be called with nil project so we need to test if project is not nil - if sdk.StatusIsTerminated(wRun.Status) && proj != nil { - wRun.LastExecution = time.Now() - if err := workflow.ResyncCommitStatus(context.Background(), dbFunc(context.Background()), store, *proj, wRun); err != nil { - log.Error(ctx, "workflow.UpdateNodeJobRunStatus> %v", err) - } - } - }(run) - } } return report, nil diff --git a/engine/vcs/bitbucketserver/client_pull_request.go b/engine/vcs/bitbucketserver/client_pull_request.go index 26190214f0..2942f7e423 100644 --- a/engine/vcs/bitbucketserver/client_pull_request.go +++ b/engine/vcs/bitbucketserver/client_pull_request.go @@ -95,6 +95,15 @@ func (b *bitbucketClient) PullRequestComment(ctx context.Context, repo string, p path := fmt.Sprintf("/projects/%s/repos/%s/pull-requests/%d/comments", project, slug, prRequest.ID) + canWrite, err := b.UserHasWritePermission(ctx, repo) + if err != nil { + return err + } + if !canWrite { + if err := b.GrantWritePermission(ctx, repo); err != nil { + return err + } + } return b.do(ctx, "POST", "core", path, nil, values, nil, &options{asUser: true}) } @@ -104,9 +113,15 @@ func (b *bitbucketClient) PullRequestCreate(ctx context.Context, repo string, pr return pr, sdk.WithStack(err) } - if err := b.GrantWritePermission(ctx, repo); err != nil { + canWrite, err := b.UserHasWritePermission(ctx, repo) + if err != nil { return pr, err } + if !canWrite { + if err := b.GrantWritePermission(ctx, repo); err != nil { + return pr, err + } + } request := sdk.BitbucketServerPullRequest{ Title: pr.Title, diff --git a/engine/vcs/bitbucketserver/client_repos.go b/engine/vcs/bitbucketserver/client_repos.go index d38418f26a..9ea9afb8b3 100644 --- a/engine/vcs/bitbucketserver/client_repos.go +++ b/engine/vcs/bitbucketserver/client_repos.go @@ -114,6 +114,32 @@ func (b *bitbucketClient) RepoByFullname(ctx context.Context, fullname string) ( return repo, nil } +func (b *bitbucketClient) UserHasWritePermission(ctx context.Context, repo string) (bool, error) { + if b.username == "" { + return false, sdk.WrapError(sdk.ErrUserNotFound, "No user found in configuration") + } + + project, slug, err := getRepo(repo) + if err != nil { + return false, sdk.WithStack(err) + } + path := fmt.Sprintf("/projects/%s/repos/%s/permissions/users", project, slug) + params := url.Values{} + params.Add("filter", b.username) + + var reponse UsersPermissionResponse + if err := b.do(ctx, "GET", "core", path, params, nil, &reponse, nil); err != nil { + return false, sdk.WithStack(err) + } + + for _, v := range reponse.Values { + if v.User.Slug == b.username { + return v.Permission == "REPO_WRITE" || v.Permission == "REPO_ADMIN", nil + } + } + return false, nil +} + func (b *bitbucketClient) GrantWritePermission(ctx context.Context, repo string) error { if b.username == "" { return nil diff --git a/engine/vcs/bitbucketserver/types.go b/engine/vcs/bitbucketserver/types.go index a35b629452..5b787fcdb4 100644 --- a/engine/vcs/bitbucketserver/types.go +++ b/engine/vcs/bitbucketserver/types.go @@ -248,3 +248,15 @@ type PullRequestResponse struct { NextPageStart int `json:"nextPageStart"` IsLastPage bool `json:"isLastPage"` } + +type UsersPermissionResponse struct { + Values []UserPermission `json:"values"` + Size int `json:"size"` + NextPageStart int `json:"nextPageStart"` + IsLastPage bool `json:"isLastPage"` +} + +type UserPermission struct { + User sdk.BitbucketServerActor `json:"user"` + Permission string `json:"permission"` +} diff --git a/engine/vcs/github/client_pull_request.go b/engine/vcs/github/client_pull_request.go index 4cb1a823ff..7b5e97ed87 100644 --- a/engine/vcs/github/client_pull_request.go +++ b/engine/vcs/github/client_pull_request.go @@ -144,6 +144,16 @@ func (g *githubClient) PullRequestComment(ctx context.Context, repo string, prRe return nil } + canWrite, err := g.UserHasWritePermission(ctx, repo) + if err != nil { + return err + } + if !canWrite { + if err := g.GrantWritePermission(ctx, repo); err != nil { + return err + } + } + path := fmt.Sprintf("/repos/%s/issues/%d/comments", repo, prReq.ID) payload := map[string]string{ "body": prReq.Message, @@ -171,6 +181,16 @@ func (g *githubClient) PullRequestComment(ctx context.Context, repo string, prRe } func (g *githubClient) PullRequestCreate(ctx context.Context, repo string, pr sdk.VCSPullRequest) (sdk.VCSPullRequest, error) { + canWrite, err := g.UserHasWritePermission(ctx, repo) + if err != nil { + return sdk.VCSPullRequest{}, nil + } + if !canWrite { + if err := g.GrantWritePermission(ctx, repo); err != nil { + return sdk.VCSPullRequest{}, nil + } + } + path := fmt.Sprintf("/repos/%s/pulls", repo) payload := map[string]string{ "title": pr.Title, diff --git a/engine/vcs/github/client_repos.go b/engine/vcs/github/client_repos.go index 97f695df0b..d5c709d371 100644 --- a/engine/vcs/github/client_repos.go +++ b/engine/vcs/github/client_repos.go @@ -151,6 +151,37 @@ func (g *githubClient) repoByFullname(ctx context.Context, fullname string) (Rep return repo, nil } +func (g *githubClient) UserHasWritePermission(ctx context.Context, fullname string) (bool, error) { + owner := strings.SplitN(fullname, "/", 2)[0] + if g.username == "" || owner == g.username { + return false, sdk.WrapError(sdk.ErrUserNotFound, "No user found in configuration") + } + url := "/repos/" + fullname + "/collaborators/" + g.username + "/permission" + k := cache.Key("vcs", "github", "user-write", g.OAuthToken, url) + + status, resp, _, err := g.get(ctx, url) + if err != nil { + return false, err + } + if status >= 400 { + return false, sdk.NewError(sdk.ErrUnknownError, errorAPI(resp)) + } + var permResp UserPermissionResponse + if status == http.StatusNotModified { + if _, err := g.Cache.Get(k, &permResp); err != nil { + log.Error(ctx, "cannot get from cache %s: %v", k, err) + } + } else { + if err := json.Unmarshal(resp, &permResp); err != nil { + return false, sdk.WrapError(err, "unable to unmarshal: %s", string(resp)) + } + if err := g.Cache.SetWithTTL(k, permResp, 61*60); err != nil { + log.Error(ctx, "cannot SetWithTTL: %s: %v", k, err) + } + } + return permResp.Permission == "write" || permResp.Permission == "admin", nil +} + func (g *githubClient) GrantWritePermission(ctx context.Context, fullname string) error { owner := strings.SplitN(fullname, "/", 2)[0] if g.username == "" || owner == g.username { diff --git a/engine/vcs/github/types.go b/engine/vcs/github/types.go index 8a07307441..229b4d15b9 100644 --- a/engine/vcs/github/types.go +++ b/engine/vcs/github/types.go @@ -523,32 +523,34 @@ type ReleaseResponse struct { UploadURL string `json:"upload_url"` } +type GithubOwner struct { + Login string `json:"login"` + ID int `json:"id"` + NodeID string `json:"node_id"` + AvatarURL string `json:"avatar_url"` + GravatarID string `json:"gravatar_id"` + URL string `json:"url"` + HTMLURL string `json:"html_url"` + FollowersURL string `json:"followers_url"` + FollowingURL string `json:"following_url"` + GistsURL string `json:"gists_url"` + StarredURL string `json:"starred_url"` + SubscriptionsURL string `json:"subscriptions_url"` + OrganizationsURL string `json:"organizations_url"` + ReposURL string `json:"repos_url"` + EventsURL string `json:"events_url"` + ReceivedEventsURL string `json:"received_events_url"` + Type string `json:"type"` + SiteAdmin bool `json:"site_admin"` +} + // RepositoryInvitation allows users or external services to invite other users to collaborate on a repo. The invited users (or external services on behalf of invited users) can choose to accept or decline the invitations. type RepositoryInvitation struct { ID int `json:"id"` Repository struct { - ID int `json:"id"` - NodeID string `json:"node_id"` - Owner struct { - Login string `json:"login"` - ID int `json:"id"` - NodeID string `json:"node_id"` - AvatarURL string `json:"avatar_url"` - GravatarID string `json:"gravatar_id"` - URL string `json:"url"` - HTMLURL string `json:"html_url"` - FollowersURL string `json:"followers_url"` - FollowingURL string `json:"following_url"` - GistsURL string `json:"gists_url"` - StarredURL string `json:"starred_url"` - SubscriptionsURL string `json:"subscriptions_url"` - OrganizationsURL string `json:"organizations_url"` - ReposURL string `json:"repos_url"` - EventsURL string `json:"events_url"` - ReceivedEventsURL string `json:"received_events_url"` - Type string `json:"type"` - SiteAdmin bool `json:"site_admin"` - } `json:"owner"` + ID int `json:"id"` + NodeID string `json:"node_id"` + Owner GithubOwner `json:"owner"` Name string `json:"name"` FullName string `json:"full_name"` Description string `json:"description"` @@ -625,50 +627,12 @@ type RepositoryInvitation struct { SubscribersCount int `json:"subscribers_count"` NetworkCount int `json:"network_count"` } `json:"repository"` - Invitee struct { - Login string `json:"login"` - ID int `json:"id"` - NodeID string `json:"node_id"` - AvatarURL string `json:"avatar_url"` - GravatarID string `json:"gravatar_id"` - URL string `json:"url"` - HTMLURL string `json:"html_url"` - FollowersURL string `json:"followers_url"` - FollowingURL string `json:"following_url"` - GistsURL string `json:"gists_url"` - StarredURL string `json:"starred_url"` - SubscriptionsURL string `json:"subscriptions_url"` - OrganizationsURL string `json:"organizations_url"` - ReposURL string `json:"repos_url"` - EventsURL string `json:"events_url"` - ReceivedEventsURL string `json:"received_events_url"` - Type string `json:"type"` - SiteAdmin bool `json:"site_admin"` - } `json:"invitee"` - Inviter struct { - Login string `json:"login"` - ID int `json:"id"` - NodeID string `json:"node_id"` - AvatarURL string `json:"avatar_url"` - GravatarID string `json:"gravatar_id"` - URL string `json:"url"` - HTMLURL string `json:"html_url"` - FollowersURL string `json:"followers_url"` - FollowingURL string `json:"following_url"` - GistsURL string `json:"gists_url"` - StarredURL string `json:"starred_url"` - SubscriptionsURL string `json:"subscriptions_url"` - OrganizationsURL string `json:"organizations_url"` - ReposURL string `json:"repos_url"` - EventsURL string `json:"events_url"` - ReceivedEventsURL string `json:"received_events_url"` - Type string `json:"type"` - SiteAdmin bool `json:"site_admin"` - } `json:"inviter"` - Permissions string `json:"permissions"` - CreatedAt string `json:"created_at"` - URL string `json:"url"` - HTMLURL string `json:"html_url"` + Invitee GithubOwner `json:"invitee"` + Inviter GithubOwner `json:"inviter"` + Permissions string `json:"permissions"` + CreatedAt string `json:"created_at"` + URL string `json:"url"` + HTMLURL string `json:"html_url"` } // DiffCommits represent response from github api for a diff between commits @@ -735,3 +699,8 @@ type Ref struct { URL string `json:"url"` } `json:"object"` } + +type UserPermissionResponse struct { + Permission string `json:"permission"` + User GithubOwner `json:"user"` +}