klesh commented on code in PR #1994:
URL: https://github.com/apache/incubator-devlake/pull/1994#discussion_r886318204
##########
plugins/gitlab/tasks/api_client.go:
##########
@@ -54,7 +54,7 @@ func NewGitlabApiClient(taskCtx core.TaskContext)
(*helper.ApiAsyncClient, error
}
apiClient.SetAfterFunction(func(res *http.Response) error {
if res.StatusCode == http.StatusUnauthorized {
- return fmt.Errorf("authentication failed, please check
your Basic Auth Token")
+ return fmt.Errorf("authentication failed, please check
your Basic Auth AccessToken")
Review Comment:
Same as above
##########
plugins/jira/api/connection.go:
##########
@@ -309,12 +203,12 @@ func ListConnections(input *core.ApiResourceInput)
(*core.ApiResourceOutput, err
if err != nil {
return nil, err
}
- encKey, err := getEncKey()
+ encKey, err := helper.GetEncKey()
if err != nil {
return nil, err
}
for i := range jiraConnections {
Review Comment:
Looks like this logic should be moved to helper function
##########
plugins/feishu/tasks/api_client.go:
##########
@@ -92,7 +92,7 @@ func NewFeishuApiClient(taskCtx core.TaskContext)
(*helper.ApiAsyncClient, error
apiClient.SetAfterFunction(func(res *http.Response) error {
if res.StatusCode == http.StatusUnauthorized {
- return fmt.Errorf("feishu authentication failed, please
check your Bearer Auth Token")
+ return fmt.Errorf("feishu authentication failed, please
check your Bearer Auth AccessToken")
Review Comment:
Doesn't sound right, maybe remove 'Bearer Auth'?
##########
plugins/helper/connection.go:
##########
@@ -0,0 +1,191 @@
+package helper
+
+import (
+ "encoding/base64"
+ "fmt"
+ "github.com/apache/incubator-devlake/config"
+ "github.com/apache/incubator-devlake/models/common"
+ "github.com/apache/incubator-devlake/plugins/core"
+ "github.com/go-playground/validator/v10"
+ "github.com/mitchellh/mapstructure"
+ "gorm.io/gorm"
+ "gorm.io/gorm/clause"
+ "reflect"
+ "strconv"
+)
+
+type BaseConnection struct {
+ Name string `gorm:"type:varchar(100);uniqueIndex" json:"name"
validate:"required"`
+ common.Model
+}
+
+type BasicAuth struct {
+ Username string `mapstructure:"username" validate:"required"
json:"username"`
+ Password string `mapstructure:"password" validate:"required"
json:"password" encrypt:"yes"`
+}
+
+func (ba BasicAuth) GetEncodedToken() string {
+ return base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%v:%v",
ba.Username, ba.Password)))
+}
+
+type AccessToken struct {
+ Token string `mapstructure:"token" validate:"required" json:"token"
encrypt:"yes"`
+}
+
+type RestConnection struct {
+ BaseConnection `mapstructure:",squash"`
+ Endpoint string `mapstructure:"endpoint" validate:"required"
json:"endpoint"`
+ Proxy string `mapstructure:"proxy" json:"proxy"`
+ RateLimit int `comment:"api request rate limt per hour"
json:"rateLimit"`
+}
+
+// RefreshAndSaveConnection populate from request input into connection which
come from REST functions to connection struct and save to DB
+// and only change value which `data` has
+// mergeFieldsToConnection merges fields from data
+// `connection` is the pointer of a plugin connection
+// `data` is http request input param
+func RefreshAndSaveConnection(connection interface{}, data
map[string]interface{}, db *gorm.DB) error {
+ var err error
+ // update fields from request body
+ err = mergeFieldsToConnection(connection, data)
+ if err != nil {
+ return err
+ }
+
+ err = saveToDb(connection, db)
+
+ if err != nil {
+ return err
+ }
+ return nil
+}
+
+func saveToDb(connection interface{}, db *gorm.DB) error {
+ dataVal := reflect.ValueOf(connection)
+ if dataVal.Kind() != reflect.Ptr {
+ panic("entityPtr is not a pointer")
+ }
+
+ encryptCode, err := GetEncKey()
+ if err != nil {
+ return err
+ }
+ dataType := reflect.Indirect(dataVal).Type()
+ fieldName := getEncryptField(dataType, "encrypt")
+ plainPwd := ""
+ if len(fieldName) > 0 {
+ plainPwd = dataVal.Elem().FieldByName(fieldName).String()
+ encyptedStr, err := core.Encrypt(encryptCode, plainPwd)
+
+ if err != nil {
+ return err
+ }
+
dataVal.Elem().FieldByName(fieldName).Set(reflect.ValueOf(encyptedStr))
+ }
+
+ err = db.Clauses(clause.OnConflict{UpdateAll:
true}).Save(connection).Error
+ if err != nil {
+ return err
+ }
+
+ dataVal.Elem().FieldByName(fieldName).Set(reflect.ValueOf(plainPwd))
+
+ return err
+}
+
+// mergeFieldsToConnection will populate all value in map to connection struct
and validate the struct
+func mergeFieldsToConnection(specificConnection interface{}, connections
...map[string]interface{}) error {
+ // decode
+ for _, connection := range connections {
+ err := mapstructure.Decode(connection, specificConnection)
+ if err != nil {
+ return err
+ }
+ }
+ // validate
+ vld := validator.New()
+ err := vld.Struct(specificConnection)
+ if err != nil {
+ return err
+ }
+
+ return nil
+}
+
+func GetEncKey() (string, error) {
+ // encrypt
+ v := config.GetConfig()
+ encKey := v.GetString(core.EncodeKeyEnvStr)
+ if encKey == "" {
+ // Randomly generate a bunch of encryption keys and set them to
config
+ encKey = core.RandomEncKey()
+ v.Set(core.EncodeKeyEnvStr, encKey)
+ err := config.WriteConfig(v)
+ if err != nil {
+ return encKey, err
+ }
+ }
+ return encKey, nil
+}
+
+func FindAuthConnectionByInput(input *core.ApiResourceInput, connection
interface{}, db *gorm.DB) error {
+ dataVal := reflect.ValueOf(connection)
+ if dataVal.Kind() != reflect.Ptr {
+ return fmt.Errorf("connection is not a pointer")
+ }
+
+ id, err := GetConnectionIdByInputParam(input)
+ if err != nil {
+ return fmt.Errorf("invalid connectionId")
+ }
+
+ err = db.First(connection, id).Error
+ if err != nil {
+ fmt.Printf("--- %s", err.Error())
+ return err
+ }
+
+ encryptCode, err := GetEncKey()
+ if err != nil {
+ return err
+ }
+ dataType := reflect.Indirect(dataVal).Type()
+
+ fieldName := getEncryptField(dataType, "encrypt")
+ if len(fieldName) > 0 {
+ decryptStr, err := core.Decrypt(encryptCode,
dataVal.Elem().FieldByName(fieldName).String())
+ if err != nil {
+ return err
Review Comment:
We should swallow this error, due to
https://github.com/apache/incubator-devlake/issues/2032 , oh, man, it was
reviewed by you.
##########
plugins/jira/models/connection.go:
##########
@@ -28,9 +28,9 @@ type EpicResponse struct {
}
type TestConnectionRequest struct {
- Endpoint string `json:"endpoint"`
- Auth string `json:"auth"`
- Proxy string `json:"proxy"`
+ Endpoint string `json:"endpoint"`
+ Proxy string `json:"proxy"`
+ helper.BasicAuth `mapstructure:",squash" encrypt:"yes"`
Review Comment:
Why do we need encryption for TestConnection when we don't need to store
them into database?
##########
plugins/jira/models/connection.go:
##########
@@ -40,15 +40,11 @@ type BoardResponse struct {
}
type JiraConnection struct {
- common.Model
- Name string `gorm:"type:varchar(100);uniqueIndex"
json:"name" validate:"required"`
- Endpoint string `json:"endpoint" validate:"required"`
- BasicAuthEncoded string `json:"basicAuthEncoded"
validate:"required"`
+ helper.RestConnection `mapstructure:",squash"`
+ helper.BasicAuth `mapstructure:",squash" encrypt:"yes"`
Review Comment:
Does `encrypt:"yes"` mean we gonna encrypt the whole struct?
##########
plugins/jira/models/migrationscripts/updateSchemas20220524.go:
##########
@@ -0,0 +1,72 @@
+/*
+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 migrationscripts
+
+import (
+ "context"
+ "github.com/apache/incubator-devlake/plugins/helper"
+ "gorm.io/gorm"
+)
+
+type JiraConnection20220524 struct {
+ helper.RestConnection
+ helper.BasicAuth
+ EpicKeyField string `gorm:"type:varchar(50);"
json:"epicKeyField"`
+ StoryPointField string `gorm:"type:varchar(50);"
json:"storyPointField"`
+ RemotelinkCommitShaPattern string
`gorm:"type:varchar(255);comment='golang regexp, the first group will be
recognized as commit sha, ref https://github.com/google/re2/wiki/Syntax'"
json:"remotelinkCommitShaPattern"`
+}
+
+func (JiraConnection20220524) TableName() string {
+ return "_tool_jira_connections"
+}
+
+type UpdateSchemas20220524 struct{}
+
+func (*UpdateSchemas20220524) Up(ctx context.Context, db *gorm.DB) error {
+ var err error
+ if !db.Migrator().HasColumn(&JiraConnection20220505{}, "password") {
+ err = db.Migrator().AddColumn(&JiraConnection20220524{},
"password")
+ if err != nil {
+ return err
+ }
+ }
+
+ if !db.Migrator().HasColumn(&JiraConnection20220505{}, "username") {
+ err = db.Migrator().AddColumn(&JiraConnection20220524{},
"username")
+ if err != nil {
+ return err
+ }
+ }
+
+ if db.Migrator().HasColumn(&JiraConnection20220505{},
"basic_auth_encoded") {
+ err = db.Migrator().DropColumn(&JiraConnection20220505{},
"basic_auth_encoded")
+ if err != nil {
+ return err
+ }
+ }
+
+ return nil
Review Comment:
I think we have to decode the `basic_auth_encoed` and spread them into
username/password accordingly before we drop them.
##########
plugins/jira/api/proxy.go:
##########
@@ -48,7 +49,14 @@ func Proxy(input *core.ApiResourceInput)
(*core.ApiResourceOutput, error) {
return nil, err
}
encKey := cfg.GetString(core.EncodeKeyEnvStr)
- basicAuth, err := core.Decrypt(encKey, jiraConnection.BasicAuthEncoded)
+
+ decodedPassword, err := core.Decrypt(encKey, jiraConnection.Password)
Review Comment:
Why not utilize `FindAuthConnectionByInput` ? and why it is called
`FindAuthConnectionByInput`? What doesn't `Auth` stand for?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
