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 <[email protected]>
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)
+}
