klesh commented on code in PR #1994:
URL: https://github.com/apache/incubator-devlake/pull/1994#discussion_r886284152
##########
plugins/helper/connection_util.go:
##########
@@ -0,0 +1,160 @@
+package helper
+
+import (
+ "fmt"
+ "github.com/apache/incubator-devlake/config"
+ "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"
+)
+
+func RefreshAndSaveConnection(connection interface{}, data
map[string]interface{}, db *gorm.DB) error {
Review Comment:
Please add `godoc` to explain what would this method do, including its PATCH
and VALIDATION nature
##########
plugins/helper/connection_util.go:
##########
@@ -0,0 +1,160 @@
+package helper
+
+import (
+ "fmt"
+ "github.com/apache/incubator-devlake/config"
+ "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"
+)
+
+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")
+ fmt.Printf("fieldName : %s\n", fieldName)
Review Comment:
I think this line should be removed.
##########
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 {
- jiraConnections[i].BasicAuthEncoded, err = core.Decrypt(encKey,
jiraConnections[i].BasicAuthEncoded)
+ jiraConnections[i].Password, err = core.Decrypt(encKey,
jiraConnections[i].Password)
Review Comment:
Since we have already let helper encrypt fields for use, we should ask
helper to decrypt them as well
##########
plugins/jira/tasks/api_client.go:
##########
@@ -29,11 +30,13 @@ import (
func NewJiraApiClient(taskCtx core.TaskContext, connection
*models.JiraConnection) (*helper.ApiAsyncClient, error) {
// load configuration
encKey := taskCtx.GetConfig(core.EncodeKeyEnvStr)
- auth, err := core.Decrypt(encKey, connection.BasicAuthEncoded)
+ decodedPassword, err := core.Decrypt(encKey, connection.Password)
if err != nil {
return nil, fmt.Errorf("Failed to decrypt Auth Token: %w", err)
}
+ auth := utils.GetEncodedToken(connection.Username, decodedPassword)
Review Comment:
It would make sense to let `BasicAuth` struct implement a method to return
`encodeToken`
##########
plugins/helper/common_models.go:
##########
@@ -0,0 +1,24 @@
+package helper
Review Comment:
The filename is vague, I suggest that we name it `connection.go` and merge
`connection_util.go` altogether since they are highly coupling
##########
plugins/helper/common_models.go:
##########
@@ -0,0 +1,24 @@
+package helper
+
+import "github.com/apache/incubator-devlake/models/common"
+
+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"`
+}
+
+type Token struct {
Review Comment:
Token is a bit vague, I would suggest to use `AccessToken` instead
##########
plugins/jira/api/connection.go:
##########
@@ -52,11 +49,12 @@ func TestConnection(input *core.ApiResourceInput)
(*core.ApiResourceOutput, erro
if err != nil {
return nil, err
}
+
// test connection
apiClient, err := helper.NewApiClient(
connection.Endpoint,
map[string]string{
- "Authorization": fmt.Sprintf("Basic %v",
connection.Auth),
+ "Authorization": "",
Review Comment:
How does this work? I think this effectively makes all user/pass combination
pass the test 😂
--
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]
