From 836d5cffcf393dcf17ce09fd9acf090e9f57c3e9 Mon Sep 17 00:00:00 2001 From: davidvader Date: Tue, 29 Oct 2024 11:57:57 -0500 Subject: [PATCH] chore: constants, cleanup, permissions helper --- api/auth/get_token.go | 3 +- compiler/native/compile.go | 2 +- compiler/types/pipeline/git.go | 4 +-- compiler/types/yaml/build.go | 2 +- constants/app_install.go | 26 +++++++++++--- scm/github/app_client.go | 64 ++++++++++++++++++++++++++++++---- scm/github/repo.go | 12 +++---- scm/github/webhook.go | 4 +-- 8 files changed, 93 insertions(+), 24 deletions(-) diff --git a/api/auth/get_token.go b/api/auth/get_token.go index 197a04b90..e82da4be6 100644 --- a/api/auth/get_token.go +++ b/api/auth/get_token.go @@ -11,6 +11,7 @@ import ( "github.com/sirupsen/logrus" "github.com/go-vela/server/api/types" + "github.com/go-vela/server/constants" "github.com/go-vela/server/database" "github.com/go-vela/server/internal/token" "github.com/go-vela/server/scm" @@ -80,7 +81,7 @@ func GetAuthToken(c *gin.Context) { // handle scm setup events // setup_action==install represents the GitHub App installation callback redirect - if c.Request.FormValue("setup_action") == "install" { + if c.Request.FormValue("setup_action") == constants.AppInstallSetupActionInstall { installID, err := strconv.ParseInt(c.Request.FormValue("installation_id"), 10, 0) if err != nil { retErr := fmt.Errorf("unable to parse installation_id: %w", err) diff --git a/compiler/native/compile.go b/compiler/native/compile.go index 2e5ab3c5c..c751d9f9b 100644 --- a/compiler/native/compile.go +++ b/compiler/native/compile.go @@ -61,7 +61,7 @@ func (c *client) Compile(ctx context.Context, v interface{}) (*pipeline.Build, * } // get the netrc password from the scm - netrc, err := c.scm.GetNetrcPassword(context.Background(), c.repo, c.user, p.Git.Repositories, p.Git.Permissions) + netrc, err := c.scm.GetNetrcPassword(ctx, c.repo, c.user, p.Git.Repositories, p.Git.Permissions) if err != nil { return nil, nil, err } diff --git a/compiler/types/pipeline/git.go b/compiler/types/pipeline/git.go index eccf6f19c..a7628abf9 100644 --- a/compiler/types/pipeline/git.go +++ b/compiler/types/pipeline/git.go @@ -19,7 +19,7 @@ type Token struct { // Empty returns true if the provided struct is empty. func (g *Git) Empty() bool { - // return true if every field is empty + // return false if any of the fields are provided if g.Token != nil { if g.Token.Repositories != nil { return false @@ -30,6 +30,6 @@ func (g *Git) Empty() bool { } } - // return false if any of the fields are provided + // return true if all fields are empty return true } diff --git a/compiler/types/yaml/build.go b/compiler/types/yaml/build.go index b2a6e872b..008546fdb 100644 --- a/compiler/types/yaml/build.go +++ b/compiler/types/yaml/build.go @@ -65,7 +65,6 @@ func (b *Build) ToPipelineAPI() *api.Pipeline { func (b *Build) UnmarshalYAML(unmarshal func(interface{}) error) error { // build we try unmarshalling to build := new(struct { - Git Git Version string Metadata Metadata Environment raw.StringSliceMap @@ -75,6 +74,7 @@ func (b *Build) UnmarshalYAML(unmarshal func(interface{}) error) error { Stages StageSlice Steps StepSlice Templates TemplateSlice + Git Git }) // attempt to unmarshal as a build type diff --git a/constants/app_install.go b/constants/app_install.go index 2a2d53cb3..d9acf5f9a 100644 --- a/constants/app_install.go +++ b/constants/app_install.go @@ -5,15 +5,33 @@ package constants // see: https://docs.github.com/en/rest/authentication/permissions-required-for-github-apps?apiVersion=2022-11-28 const ( - // The string value for GitHub App install read permissions. + // GitHub App install permission 'none'. + AppInstallPermissionNone = "none" + // GitHub App install permission 'read'. AppInstallPermissionRead = "read" - // The string value for GitHub App install write permissions. + // GitHub App install permission 'write'. AppInstallPermissionWrite = "write" ) const ( - // The string value for GitHub App install contents resource. + // GitHub App install contents resource. AppInstallResourceContents = "contents" - // The string value for GitHub App install checks resource. + // GitHub App install checks resource. AppInstallResourceChecks = "checks" ) + +const ( + // GitHub App install repositories selection when "all" repositories are selected. + AppInstallRepositoriesSelectionAll = "all" + // GitHub App install repositories selection when a subset of repositories are selected. + AppInstallRepositoriesSelectionSelected = "selected" +) + +const ( + // GitHub App install setup_action type 'install'. + AppInstallSetupActionInstall = "install" + // GitHub App install event type 'created'. + AppInstallCreated = "created" + // GitHub App install event type 'deleted'. + AppInstallDeleted = "deleted" +) diff --git a/scm/github/app_client.go b/scm/github/app_client.go index 57e5e4041..f91ba4605 100644 --- a/scm/github/app_client.go +++ b/scm/github/app_client.go @@ -54,14 +54,65 @@ func (c *client) ValidateGitHubApp(ctx context.Context) error { } perms := app.GetPermissions() - if len(perms.GetContents()) == 0 || - (perms.GetContents() != constants.AppInstallPermissionRead && perms.GetContents() != constants.AppInstallPermissionWrite) { - return fmt.Errorf("github app requires contents:read permissions, found: %s", perms.GetContents()) + + type perm struct { + resource string + requiredPermission string + actualPermission string + } + + // GitHub App installation requires the following permissions + // - contents:read + // - checks:write + requiredPermissions := []perm{ + { + resource: constants.AppInstallResourceContents, + requiredPermission: constants.AppInstallPermissionRead, + actualPermission: perms.GetContents(), + }, + { + resource: constants.AppInstallResourceChecks, + requiredPermission: constants.AppInstallPermissionWrite, + actualPermission: perms.GetChecks(), + }, + } + + for _, p := range requiredPermissions { + err := hasPermission(p.resource, p.requiredPermission, p.actualPermission) + if err != nil { + return err + } + } + + return nil +} + +// hasPermission takes a resource:perm pair and checks if the actual permission matches the expected permission or is supersceded by a higher permission. +func hasPermission(resource, requiredPerm, actualPerm string) error { + if len(actualPerm) == 0 { + return fmt.Errorf("github app missing permission %s:%s", resource, requiredPerm) + } + + permitted := false + + switch requiredPerm { + case constants.AppInstallPermissionNone: + permitted = true + case constants.AppInstallPermissionRead: + if actualPerm == constants.AppInstallPermissionRead || + actualPerm == constants.AppInstallPermissionWrite { + permitted = true + } + case constants.AppInstallPermissionWrite: + if actualPerm == constants.AppInstallPermissionWrite { + permitted = true + } + default: + return fmt.Errorf("invalid required permission type: %s", requiredPerm) } - if len(perms.GetChecks()) == 0 || - perms.GetChecks() != constants.AppInstallPermissionWrite { - return fmt.Errorf("github app requires checks:write permissions, found: %s", perms.GetChecks()) + if !permitted { + return fmt.Errorf("github app requires permission %s:%s, found: %s", constants.AppInstallResourceContents, constants.AppInstallPermissionRead, actualPerm) } return nil @@ -69,7 +120,6 @@ func (c *client) ValidateGitHubApp(ctx context.Context) error { // newGithubAppClient returns the GitHub App client for authenticating as the GitHub App itself using the RoundTripper. func (c *client) newGithubAppClient() (*github.Client, error) { - // todo: create transport using context to apply tracing // create a github client based off the existing GitHub App configuration client, err := github.NewClient( &http.Client{ diff --git a/scm/github/repo.go b/scm/github/repo.go index cb45c37b7..ba10ca059 100644 --- a/scm/github/repo.go +++ b/scm/github/repo.go @@ -784,7 +784,7 @@ func (c *client) SyncRepoWithInstallation(ctx context.Context, r *api.Repo) (*ap installationCanReadRepo := false - if installation.GetRepositorySelection() != "all" { + if installation.GetRepositorySelection() != constants.AppInstallRepositoriesSelectionAll { client, err := c.newGithubAppClient() if err != nil { return r, err @@ -821,9 +821,9 @@ func (c *client) SyncRepoWithInstallation(ctx context.Context, r *api.Repo) (*ap func WithGitHubInstallationPermission(perms *github.InstallationPermissions, resource, perm string) (*github.InstallationPermissions, error) { // convert permissions from yaml string switch strings.ToLower(perm) { - case "read": - case "write": - case "none": + case constants.AppInstallPermissionNone: + case constants.AppInstallPermissionRead: + case constants.AppInstallPermissionWrite: break default: return perms, fmt.Errorf("invalid permission value given for %s: %s", resource, perm) @@ -831,9 +831,9 @@ func WithGitHubInstallationPermission(perms *github.InstallationPermissions, res // convert resource from yaml string switch strings.ToLower(resource) { - case "contents": + case constants.AppInstallResourceContents: perms.Contents = github.String(perm) - case "checks": + case constants.AppInstallResourceChecks: perms.Checks = github.String(perm) default: return perms, fmt.Errorf("invalid permission key given: %s", perm) diff --git a/scm/github/webhook.go b/scm/github/webhook.go index 54ed1dee5..31c83fb42 100644 --- a/scm/github/webhook.go +++ b/scm/github/webhook.go @@ -555,11 +555,11 @@ func (c *client) processInstallationEvent(_ context.Context, h *api.Hook, payloa install.Org = payload.GetInstallation().GetAccount().GetLogin() switch payload.GetAction() { - case "created": + case constants.AppInstallCreated: for _, repo := range payload.Repositories { install.RepositoriesAdded = append(install.RepositoriesAdded, repo.GetName()) } - case "deleted": + case constants.AppInstallDeleted: for _, repo := range payload.Repositories { install.RepositoriesRemoved = append(install.RepositoriesRemoved, repo.GetName()) }