Skip to content

Commit

Permalink
rely on callback for changes to app (not webhooks) to avoid race cond…
Browse files Browse the repository at this point in the history 
…itions (#1687)

* rely on callback for changes to app (not webhooks) to avoid race conditions

* remove unused functions and tests

* fix logic for repo fetching
  • Loading branch information
@motatoes
motatoes authored Sep 3, 2024
1 parent fc02440 commit ca920c5
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 609 deletions.
1 change: 0 additions & 1 deletion backend/bootstrap/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ func Bootstrap(templates embed.FS, diggerController controllers.DiggerController
}

r.POST("/github-app-webhook", diggerController.GithubAppWebHook)
r.POST("/github-app-webhook/aam", diggerController.GithubAppWebHookAfterMerge)

tenantActionsGroup := r.Group("/api/tenants")
tenantActionsGroup.Use(middleware.CORSMiddleware())
Expand Down
209 changes: 98 additions & 111 deletions backend/controllers/github.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"encoding/base64"
"encoding/json"
"errors"
"fmt"
"github.com/diggerhq/digger/backend/ci_backends"
"github.com/diggerhq/digger/backend/locking"
Expand All @@ -15,6 +16,7 @@ import (
dg_locking "github.com/diggerhq/digger/libs/locking"
orchestrator_scheduler "github.com/diggerhq/digger/libs/scheduler"
"github.com/google/uuid"
"gorm.io/gorm"
"log"
"math/rand"
"net/http"
Expand Down Expand Up @@ -67,13 +69,6 @@ func (d DiggerController) GithubAppWebHook(c *gin.Context) {
switch event := event.(type) {
case *github.InstallationEvent:
log.Printf("InstallationEvent, action: %v\n", *event.Action)
if *event.Action == "created" {
err := handleInstallationCreatedEvent(event)
if err != nil {
c.String(http.StatusInternalServerError, "Failed to handle webhook event.")
return
}
}

if *event.Action == "deleted" {
err := handleInstallationDeletedEvent(event)
Expand All @@ -82,20 +77,6 @@ func (d DiggerController) GithubAppWebHook(c *gin.Context) {
return
}
}
case *github.InstallationRepositoriesEvent:
log.Printf("InstallationRepositoriesEvent, action: %v\n", *event.Action)
if *event.Action == "added" {
err := handleInstallationRepositoriesAddedEvent(gh, event)
if err != nil {
c.String(http.StatusInternalServerError, "Failed to handle installation repo added event.")
}
}
if *event.Action == "removed" {
err := handleInstallationRepositoriesDeletedEvent(event)
if err != nil {
c.String(http.StatusInternalServerError, "Failed to handle installation repo deleted event.")
}
}
case *github.IssueCommentEvent:
log.Printf("IssueCommentEvent, action: %v\n", *event.Action)
if event.Sender.Type != nil && *event.Sender.Type == "Bot" {
Expand Down Expand Up @@ -288,19 +269,27 @@ func createOrGetDiggerRepoForGithubRepo(ghRepoFullName string, ghRepoOrganisatio

diggerRepoName := strings.ReplaceAll(ghRepoFullName, "/", "-")

repo, err := models.DB.GetRepo(orgId, diggerRepoName)
// using Unscoped because we also need to include deleted repos (and undelete them if they exist)
var existingRepo models.Repo
r := models.DB.GormDB.Unscoped().Where("organisation_id=? AND repos.name=?", orgId, diggerRepoName).Find(&existingRepo)

if err != nil {
log.Printf("Error fetching repo: %v", err)
return nil, nil, err
if r.Error != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
log.Printf("repo not found, will proceed with repo creation")
} else {
log.Printf("Error fetching repo: %v", err)
return nil, nil, err
}
}

if repo != nil {
log.Printf("Digger repo already exists: %v", repo)
return repo, org, nil
if r.RowsAffected > 0 {
existingRepo.DeletedAt = gorm.DeletedAt{}
models.DB.GormDB.Save(&existingRepo)
log.Printf("Digger repo already exists: %v", existingRepo)
return &existingRepo, org, nil
}

repo, err = models.DB.CreateRepo(diggerRepoName, ghRepoFullName, ghRepoOrganisation, ghRepoName, ghRepoUrl, org, `
repo, err := models.DB.CreateRepo(diggerRepoName, ghRepoFullName, ghRepoOrganisation, ghRepoName, ghRepoUrl, org, `
generate_projects:
include: "."
`)
Expand All @@ -312,72 +301,6 @@ generate_projects:
return repo, org, nil
}

func handleInstallationRepositoriesAddedEvent(ghClientProvider utils.GithubClientProvider, payload *github.InstallationRepositoriesEvent) error {
installationId := *payload.Installation.ID
login := *payload.Installation.Account.Login
accountId := *payload.Installation.Account.ID
appId := *payload.Installation.AppID

for _, repo := range payload.RepositoriesAdded {
repoFullName := *repo.FullName
repoOwner := strings.Split(*repo.FullName, "/")[0]
repoName := *repo.Name
repoUrl := fmt.Sprintf("https://github.com/%v", repoFullName)
_, err := models.DB.GithubRepoAdded(installationId, appId, login, accountId, repoFullName)
if err != nil {
log.Printf("GithubRepoAdded failed, error: %v\n", err)
return err
}

_, _, err = createOrGetDiggerRepoForGithubRepo(repoFullName, repoOwner, repoName, repoUrl, installationId)
if err != nil {
log.Printf("createOrGetDiggerRepoForGithubRepo failed, error: %v\n", err)
return err
}
}
return nil
}

func handleInstallationRepositoriesDeletedEvent(payload *github.InstallationRepositoriesEvent) error {
installationId := *payload.Installation.ID
appId := *payload.Installation.AppID
for _, repo := range payload.RepositoriesRemoved {
repoFullName := *repo.FullName
_, err := models.DB.GithubRepoRemoved(installationId, appId, repoFullName)
if err != nil {
return err
}

// todo: change the status of DiggerRepo to InActive
}
return nil
}

func handleInstallationCreatedEvent(installation *github.InstallationEvent) error {
installationId := *installation.Installation.ID
login := *installation.Installation.Account.Login
accountId := *installation.Installation.Account.ID
appId := *installation.Installation.AppID

for _, repo := range installation.Repositories {
repoFullName := *repo.FullName
repoOwner := strings.Split(*repo.FullName, "/")[0]
repoName := *repo.Name
repoUrl := fmt.Sprintf("https://github.com/%v", repoFullName)

log.Printf("Adding a new installation %d for repo: %s", installationId, repoFullName)
_, err := models.DB.GithubRepoAdded(installationId, appId, login, accountId, repoFullName)
if err != nil {
return err
}
_, _, err = createOrGetDiggerRepoForGithubRepo(repoFullName, repoOwner, repoName, repoUrl, installationId)
if err != nil {
return err
}
}
return nil
}

func handleInstallationDeletedEvent(installation *github.InstallationEvent) error {
installationId := *installation.Installation.ID
appId := *installation.Installation.AppID
Expand Down Expand Up @@ -1127,39 +1050,100 @@ func (d DiggerController) GithubAppCallbackPage(c *gin.Context) {
clientId := os.Getenv("GITHUB_APP_CLIENT_ID")
clientSecret := os.Getenv("GITHUB_APP_CLIENT_SECRET")

orgId, exists := c.Get(middleware.ORGANISATION_ID_KEY)
if !exists {
c.String(http.StatusForbidden, "Not allowed to access this resource")
return
}

installationId64, err := strconv.ParseInt(installationId, 10, 64)
if err != nil {
log.Printf("err: %v", err)
c.String(http.StatusInternalServerError, "Failed to parse installation_id.")
return
}

result, err := validateGithubCallback(d.GithubClientProvider, clientId, clientSecret, code, installationId64)
result, installation, err := validateGithubCallback(d.GithubClientProvider, clientId, clientSecret, code, installationId64)
if !result {
log.Printf("Failed to validated installation id, %v\n", err)
c.String(http.StatusInternalServerError, "Failed to validate installation_id.")
return
}

// TODO: Lookup org in GithubAppInstallation by installationID if found use that installationID otherwise
// create a new org for this installationID
// retrive org for current orgID
orgId, exists := c.Get(middleware.ORGANISATION_ID_KEY)
if !exists {
log.Printf("Unable to retrive orgId: %v", err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "Unable to retrive orgId"})
return
}
org, err := models.DB.GetOrganisationById(orgId)
if err != nil {
log.Printf("Error fetching organisation: %v", err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "Error fetching organisation"})
return
}

// create a github installation link (org ID matched to installation ID)
_, err = models.DB.CreateGithubInstallationLink(org, installationId64)
if err != nil {
log.Printf("Error saving CreateGithubInstallationLink to database: %v", err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "Error updating GitHub installation"})
return
}

client, _, err := d.GithubClientProvider.Get(*installation.AppID, installationId64)
if err != nil {
log.Printf("Error retriving github client: %v", err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "Error fetching organisation"})
return

}

// we get repos accessible to this installation
listRepos, _, err := client.Apps.ListRepos(context.Background(), nil)
if err != nil {
log.Printf("Failed to validated list existing repos, %v\n", err)
c.String(http.StatusInternalServerError, "Failed to list existing repos: %v", err)
return
}
repos := listRepos.Repositories

// resets all existing installations (soft delete)
var AppInstallation models.GithubAppInstallation
err = models.DB.GormDB.Model(&AppInstallation).Where("github_installation_id=?", installationId).Update("status", models.GithubAppInstallDeleted).Error
if err != nil {
log.Printf("Failed to update github installations: %v", err)
c.String(http.StatusInternalServerError, "Failed to update github installations: %v", err)
return
}

// reset all existing repos (soft delete)
var ExistingRepos []models.Repo
err = models.DB.GormDB.Delete(ExistingRepos, "organisation_id=?", orgId).Error
if err != nil {
log.Printf("could not delete repos: %v", err)
c.String(http.StatusInternalServerError, "could not delete repos: %v", err)
return
}

// here we mark repos that are available one by one
for _, repo := range repos {
repoFullName := *repo.FullName
repoOwner := strings.Split(*repo.FullName, "/")[0]
repoName := *repo.Name
repoUrl := fmt.Sprintf("https://github.com/%v", repoFullName)
_, err := models.DB.GithubRepoAdded(installationId64, *installation.AppID, *installation.Account.Login, *installation.Account.ID, repoFullName)
if err != nil {
log.Printf("github repos added error: %v", err)
c.String(http.StatusInternalServerError, "github repos added error: %v", err)
return
}

_, _, err = createOrGetDiggerRepoForGithubRepo(repoFullName, repoOwner, repoName, repoUrl, installationId64)
if err != nil {
log.Printf("createOrGetDiggerRepoForGithubRepo error: %v", err)
c.String(http.StatusInternalServerError, "createOrGetDiggerRepoForGithubRepo error: %v", err)
return
}
}

c.HTML(http.StatusOK, "github_success.tmpl", gin.H{})
}

Expand Down Expand Up @@ -1210,7 +1194,7 @@ func (d DiggerController) GithubReposPage(c *gin.Context) {

// why this validation is needed: https://roadie.io/blog/avoid-leaking-github-org-data/
// validation based on https://docs.github.com/en/apps/creating-github-apps/authenticating-with-a-github-app/generating-a-user-access-token-for-a-github-app , step 3
func validateGithubCallback(githubClientProvider utils.GithubClientProvider, clientId string, clientSecret string, code string, installationId int64) (bool, error) {
func validateGithubCallback(githubClientProvider utils.GithubClientProvider, clientId string, clientSecret string, code string, installationId int64) (bool, *github.Installation, error) {
ctx := context.Background()
type OAuthAccessResponse struct {
AccessToken string `json:"access_token"`
Expand All @@ -1221,22 +1205,22 @@ func validateGithubCallback(githubClientProvider utils.GithubClientProvider, cli
reqURL := fmt.Sprintf("https://%v/login/oauth/access_token?client_id=%s&client_secret=%s&code=%s", githubHostname, clientId, clientSecret, code)
req, err := http.NewRequest(http.MethodPost, reqURL, nil)
if err != nil {
return false, fmt.Errorf("could not create HTTP request: %v\n", err)
return false, nil, fmt.Errorf("could not create HTTP request: %v\n", err)
}
req.Header.Set("accept", "application/json")

res, err := httpClient.Do(req)
if err != nil {
return false, fmt.Errorf("request to login/oauth/access_token failed: %v\n", err)
return false, nil, fmt.Errorf("request to login/oauth/access_token failed: %v\n", err)
}

if err != nil {
return false, fmt.Errorf("Failed to read response's body: %v\n", err)
return false, nil, fmt.Errorf("Failed to read response's body: %v\n", err)
}

var t OAuthAccessResponse
if err := json.NewDecoder(res.Body).Decode(&t); err != nil {
return false, fmt.Errorf("could not parse JSON response: %v\n", err)
return false, nil, fmt.Errorf("could not parse JSON response: %v\n", err)
}

ts := oauth2.StaticTokenSource(
Expand All @@ -1253,25 +1237,28 @@ func validateGithubCallback(githubClientProvider utils.GithubClientProvider, cli
client, err := githubClientProvider.NewClient(tc)
if err != nil {
log.Printf("could create github client: %v", err)
return false, fmt.Errorf("could not create github client: %v", err)
return false, nil, fmt.Errorf("could not create github client: %v", err)
}

installationIdMatch := false
// list all installations for the user
var matchedInstallation *github.Installation
installations, _, err := client.Apps.ListUserInstallations(ctx, nil)
if err != nil {
log.Printf("could not retrieve installations: %v", err)
return false, fmt.Errorf("could not retrieve installations: %v", installationId)
return false, nil, fmt.Errorf("could not retrieve installations: %v", installationId)
}
log.Printf("installations %v", installations)
for _, v := range installations {
log.Printf("installation id: %v\n", *v.ID)
if *v.ID == installationId {
matchedInstallation = v
installationIdMatch = true
}
}
if !installationIdMatch {
return false, fmt.Errorf("InstallationId %v doesn't match any id for specified user\n", installationId)
return false, nil, fmt.Errorf("InstallationId %v doesn't match any id for specified user\n", installationId)
}
return true, nil

return true, matchedInstallation, nil
}
Loading

0 comments on commit ca920c5

Please sign in to comment.