geomacy commented on a change in pull request #77: Allow sent user defined 
headers and omit credentials if not needed
URL: https://github.com/apache/brooklyn-client/pull/77#discussion_r289195414
 
 

 ##########
 File path: cli/commands/login.go
 ##########
 @@ -123,11 +154,13 @@ func (cmd *Login) Run(scope scope.Scope, c *cli.Context) 
{
 
        // If an argument was not supplied, it is set to empty string
        cmd.network.BrooklynUrl = c.Args().Get(0)
-       cmd.network.BrooklynUser = c.Args().Get(1)
-       cmd.network.BrooklynPass = c.Args().Get(2)
+       cmd.brooklynUser = c.Args().Get(1)
+       cmd.brooklynPass = c.Args().Get(2)
        cmd.network.SkipSslChecks = c.Bool("skipSslChecks")
-       cmd.network.CredentialsRequired = !c.Bool("noCredentials")
-       cmd.network.UserHeaders = parseHeaders(c.StringSlice("header"))
+       cmd.network.AuthorizationType = 
cmd.getAuthorizationType(c.String(authorizationParam))
 
 Review comment:
   I think if the `authorization` parameter is specified we should just load 
both type and value from it right here, rather than get the token in a separate 
step `getCredentialsFromAuthParamIfNeeded`. We would need to do it below the 
reset of credentials, like:
   
   ```go
   
        //clear Credentials credentials
        cmd.network.Credentials = ""
   
        authParamValue := c.String(authorizationParam)
        if authParamValue != "" {
                parts := strings.SplitN(authParamValue, ":", 2)
                if parts[0] == BEARER_AUTH && len(parts) == 2 {
                        cmd.network.AuthorizationType = BEARER_AUTH
                        cmd.network.Credentials = parts[1]
                } else {
                        cmd.network.AuthorizationType = BASIC_AUTH
                }
        } else {
                cmd.network.AuthorizationType = BASIC_AUTH
        }
   
   ```
   this could replace the use of separate functions 
`getCredentialsFromAuthParamIfNeeded` and `getAuthorizationType`. It also just 
splits the value without a separate `parse...` method, I think it's clear 
enough.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to