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