[incubator-devlake] branch main updated: feat: add warning to github test connection (#5205)

[mi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

mintsweet pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-devlake.git


The following commit(s) were added to refs/heads/main by this push:
     new 6f23b37a3 feat: add warning to github test connection (#5205)
6f23b37a3 is described below

commit 6f23b37a32430f65f1b6fe27b287e64fe2c7ea42
Author: Klesh Wong <zh...@merico.dev>
AuthorDate: Mon May 22 11:07:05 2023 +0800

    feat: add warning to github test connection (#5205)
---
 backend/plugins/github/api/connection.go      | 75 ++++++++++++++++++---------
 backend/plugins/github/api/connection_test.go | 50 ++++++++++++++++++
 2 files changed, 101 insertions(+), 24 deletions(-)

diff --git a/backend/plugins/github/api/connection.go b/backend/plugins/github/api/connection.go
index 158eac572..76e612701 100644
--- a/backend/plugins/github/api/connection.go
+++ b/backend/plugins/github/api/connection.go
@@ -19,6 +19,7 @@ package api
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 	"strings"
 
@@ -29,11 +30,31 @@ import (
 	"github.com/apache/incubator-devlake/server/api/shared"
 )
 
-var requirePermission = []string{"repo:status", "repo_deployment", "read:user", "read:org"}
+var publicPermissions = []string{"repo:status", "repo_deployment", "read:user", "read:org"}
+var privatePermissions = []string{"repo"}
+var parentPermissions = map[string]string{
+	"repo:status":     "repo",
+	"repo_deployment": "repo",
+	"read:user":       "user",
+	"read:org":        "admin:org",
+}
+
+// findMissingPerms returns the missing required permissions from the given user permissions
+func findMissingPerms(userPerms map[string]bool, requiredPerms []string) []string {
+	missingPerms := make([]string, 0)
+	for _, pp := range requiredPerms {
+		// either the specific permission or its parent permission(larger) is granted
+		if !userPerms[pp] && !userPerms[parentPermissions[pp]] {
+			missingPerms = append(missingPerms, pp)
+		}
+	}
+	return missingPerms
+}
 
 type GithubTestConnResponse struct {
 	shared.ApiBody
-	Login string `json:"login"`
+	Login   string `json:"login"`
+	Warning bool   `json:"warning"`
 }
 
 // @Summary test github connection
@@ -77,35 +98,41 @@ func TestConnection(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput,
 		return nil, errors.BadInput.Wrap(err, "invalid token")
 	}
 
+	success := false
+	warning := false
+	messages := []string{}
 	// for github classic token, check permission
 	if strings.HasPrefix(conn.Token, "ghp_") {
 		scopes := res.Header.Get("X-OAuth-Scopes")
-		for _, permission := range requirePermission {
-			if !strings.Contains(scopes, permission) {
-				if permission == "repo:status" || permission == "repo_deployment" {
-					// If the missing permission is repo:status or repo_deployment, check if the repo permission is present
-					if strings.Contains(scopes, "repo") {
-						continue
-					}
-				}
-				if permission == "read:user" {
-					if strings.Contains(scopes, "user") {
-						continue
-					}
-				}
-				if permission == "read:org" {
-					if strings.Contains(scopes, "admin:org") {
-						continue
-					}
-				}
-				return nil, errors.BadInput.New("insufficient token permission")
-			}
+		// convert "X-OAuth-Scopes" header to user permissions map
+		userPerms := map[string]bool{}
+		for _, userPerm := range strings.Split(scopes, ", ") {
+			userPerms[userPerm] = true
+		}
+		// check public repo permission
+		missingPubPerms := findMissingPerms(userPerms, publicPermissions)
+		success = len(missingPubPerms) == 0
+		if !success {
+			messages = append(messages, fmt.Sprintf(
+				"%s is/are required to collect data from Public Repos",
+				strings.Join(missingPubPerms, ", "),
+			))
+		}
+		// check private repo permission
+		missingPriPerms := findMissingPerms(userPerms, privatePermissions)
+		warning = len(missingPriPerms) > 0
+		if warning {
+			messages = append(messages, fmt.Sprintf(
+				"%s is/are required to collect data from Private Repos",
+				strings.Join(missingPriPerms, ", "),
+			))
 		}
 	}
 
 	githubApiResponse := &GithubTestConnResponse{}
-	githubApiResponse.Success = true
-	githubApiResponse.Message = "success"
+	githubApiResponse.Success = success
+	githubApiResponse.Warning = warning
+	githubApiResponse.Message = strings.Join(messages, ";\n")
 	githubApiResponse.Login = githubUserOfToken.Login
 	return &plugin.ApiResourceOutput{Body: githubApiResponse, Status: http.StatusOK}, nil
 }
diff --git a/backend/plugins/github/api/connection_test.go b/backend/plugins/github/api/connection_test.go
new file mode 100644
index 000000000..5f9a239f3
--- /dev/null
+++ b/backend/plugins/github/api/connection_test.go
@@ -0,0 +1,50 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestFindMissingPerms(t *testing.T) {
+	// permission is larger than required
+	userPerms := map[string]bool{
+		"repo": true,
+	}
+	requiredPerms := []string{"repo:status"}
+	missingPerms := findMissingPerms(userPerms, requiredPerms)
+	assert.Empty(t, missingPerms)
+
+	// permission equals to required
+	userPerms = map[string]bool{
+		"repo:status": true,
+	}
+	requiredPerms = []string{"repo:status"}
+	missingPerms = findMissingPerms(userPerms, requiredPerms)
+	assert.Empty(t, missingPerms)
+
+	// permissions are missing
+	userPerms = map[string]bool{
+		"admin:org": true,
+	}
+	requiredPerms = []string{"repo:status", "read:user"}
+	missingPerms = findMissingPerms(userPerms, requiredPerms)
+	assert.Equal(t, []string{"repo:status", "read:user"}, missingPerms)
+}