You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by geomacy <gi...@git.apache.org> on 2017/04/04 13:17:14 UTC
[GitHub] brooklyn-client pull request #42: Improve login handling.
GitHub user geomacy opened a pull request:
https://github.com/apache/brooklyn-client/pull/42
Improve login handling.
See https://issues.apache.org/jira/browse/BROOKLYN-463.
Prompt for user name if not supplied.
Check for 401 Unauthorized and give tidier output.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/geomacy/brooklyn-client better-login
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-client/pull/42.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #42
----
commit 5df9e065b6522c418ad961025931c1cc000b0caf
Author: Geoff Macartney <ge...@cloudsoftcorp.com>
Date: 2017-04-04T13:08:18Z
Improve login handling.
See https://issues.apache.org/jira/browse/BROOKLYN-463.
Prompt for user name if not supplied.
Check for 401 Unauthorized and give tidier output.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
Thanks @geomacy. Will merge this. Printing usage when someone writes `br --skipSslChecks` or `br --json` can be a future improvement.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
`requiresLogin` should check that the command isn't `help`. It also needs to handle invocations of the program that set global options but don't give a command. I think that of the options `--version` should execute as normal and the others (`--skipSslChecks`, `--json` and `--help`) should print the help text.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request #42: Improve login handling.
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/brooklyn-client/pull/42#discussion_r109663969
--- Diff: cli/commands/login.go ---
@@ -84,7 +100,8 @@ func (cmd *Login) Run(scope scope.Scope, c *cli.Context) {
error_handler.ErrorExit(err)
}
fmt.Printf("\n")
- cmd.network.BrooklynPass = string(bytePassword)
+ password := string(bytePassword)
+ cmd.network.BrooklynPass = strings.TrimSpace(password)
--- End diff --
Is trimming spaces the right thing to do? What if my password is " password1 "?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
p.s. updated config file format looks like [this](https://github.com/geomacy/brooklyn-client/blob/b5e51e420aab89ad34cbf5581f3de520220263c7/cli/io/testconfig.json)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
@Graeme-Miller, @johnmccabe, @m4rkmckenna you may be interested to review this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
hi all,
I have made a bunch of updates to address the above comments and add some further functionality.
The main change is the simplification of the `.brooklyn_cli` file format, from
```json
{
"auth": {
"http://geoffs-macbook-pro.local:8081": {
"password": "password",
"username": "geoff"
},
"http://localhost:8081": {
"password": "password",
"username": "geoff"
}
},
"skipSslChecks": false,
"target": "http://geoffs-macbook-pro.local:8081"
}
```
to
```json
{
"credentials": "Z2VvZmY6cGFzc3dvcmQ=",
"skipSslChecks": false,
"target": "http://geoffs-macbook-pro.local:8081"
}
```
Note it now uses base64 to avoid characters in the password possibly causing problems with parsing the file.
There is also now a `logout` command that removes the `.brooklyn_cli` file, and a `server` command that shows you what server you are logged into.
Would appreciate another round of review.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request #42: Improve login handling.
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-client/pull/42#discussion_r118494407
--- Diff: cli/io/config.go ---
@@ -70,3 +98,111 @@ func (config *Config) Read() {
dec := json.NewDecoder(fileToRead)
dec.Decode(&config.Map)
}
+
+
+// getCredentials reads credentials from .brooklyn_cli data formatted for versions > 0.11.0
+// Note that the password is base64 encoded to avoid json formatting problems
+//{
+// "credentials": {
+// "password": "cGFzc3dvcmQ=",
+// "username": "geoff"
+// },
+// "skipSslChecks": false,
+// "target": "http://geoffs-macbook-pro.local:8081"
+//}
+func (config *Config) getCredentials(target string) (username string, password string, err error) {
+ credentials, found := config.Map[credentialsKey].(string)
+ if !found {
+ err = errors.New("No credentials found for " + target)
+ return
+ }
+
+ if decoded, errd := base64.StdEncoding.DecodeString(credentials); errd != nil {
+ err = errors.New("Could not decode credentials for " + target)
+ return
+ } else {
+ userAndPassword := strings.SplitN(string(decoded), ":", 2)
+ if len(userAndPassword) != 2 {
--- End diff --
Because it's a `SplitN(.., 2)` we should be ok here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request #42: Improve login handling.
Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller commented on a diff in the pull request:
https://github.com/apache/brooklyn-client/pull/42#discussion_r118455814
--- Diff: cli/commands/login.go ---
@@ -53,6 +58,30 @@ func (cmd *Login) Metadata() command_metadata.CommandMetadata {
}
}
+
+func (cmd *Login) promptAndReadUsername() (username string) {
+ for username == "" {
+ reader := bufio.NewReader(os.Stdin)
+ fmt.Print("Enter Username: ")
+ user, err := reader.ReadString('\n')
+ if err != nil {
+ error_handler.ErrorExit(err)
+ }
+ username = strings.TrimSpace(user)
+ }
+ return username
+}
+
+func (cmd *Login) promptAndReadPassword() (password string) {
+ fmt.Print("Enter Password: ")
--- End diff --
why no loop like in promptAndReadUsername?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
@robert I think the original format was done like that to allow for multiple hosts but then we didn't actually provide that ability. What do you all think, should we support the ability to have multiple credentials stored? So that @m4rkmckenna's example above would work?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
@robertgmoss et al; I have meant to implement Robert's change above but I'm really not getting any time to look at it these days. Can I suggest we accept this PR as it stands, and I can try to get back to adding support for multiple hosts another time?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
Just want to chime in with a different opinion. I think we should merge as is, and not implement the functionality to allow users to switch between multiple hosts.
For me, it is a question of ease of use vs security. I am uncomfortable caching any passwords locally as it is insecure- but I can see why it is necessary. Caching multiple passwords doesn't seem necessary to me and makes the solution much less secure (we never clean up the passwords, so we store a list of passwords for all hosts ever visited).
Also, I feel like it violates the principle of least astonishment. I was quite surprised when I checked the local cache and there was a list of passwords in there. Including a password that was not a brooklyn password that I must have entered in error. I think that users would be surprised by the behaviour of the CLI to cache all passwords entered.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request #42: Improve login handling.
Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller commented on a diff in the pull request:
https://github.com/apache/brooklyn-client/pull/42#discussion_r118458766
--- Diff: cli/br/brooklyn.go ---
@@ -30,27 +30,14 @@ import (
"path/filepath"
)
-func getNetworkCredentialsFromConfig(yamlMap map[string]interface{}) (string, string, string, bool) {
- var target, username, password string
- var skipSslChecks bool
- target, found := yamlMap["target"].(string)
- if found {
- auth, found := yamlMap["auth"].(map[string]interface{})
- if found {
- creds := auth[target].(map[string]interface{})
- username, found = creds["username"].(string)
- if found {
- password, found = creds["password"].(string)
- }
- }
- skipSslChecks, _ = yamlMap["skipSslChecks"].(bool)
- }
- return target, username, password, skipSslChecks
-}
-
func main() {
config := io.GetConfig()
- target, username, password, skipSslChecks := getNetworkCredentialsFromConfig(config.Map)
+ skipSslChecks := config.GetSkipSslChecks()
+ target, username, password, err := config.GetNetworkCredentials()
+ if err != nil && !isLogin(os.Args) {
--- End diff --
A nice to have: if the user hasn't previously called login we will error here. However, we could change the command to login here and continue. They would be prompted for a username and password.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request #42: Improve login handling.
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-client/pull/42#discussion_r111936403
--- Diff: cli/net/net.go ---
@@ -121,15 +121,15 @@ func (net *Network) SendRequestGetStatusCode(req *http.Request) ([]byte, int, er
client := net.makeClient()
resp, err := client.Do(req)
if err != nil {
- return nil, 0, err
+ return nil, resp.StatusCode, err
--- End diff --
Hm, indeed it probably is not. I will investigate this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request #42: Improve login handling.
Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller commented on a diff in the pull request:
https://github.com/apache/brooklyn-client/pull/42#discussion_r118458597
--- Diff: cli/io/config.go ---
@@ -70,3 +98,111 @@ func (config *Config) Read() {
dec := json.NewDecoder(fileToRead)
dec.Decode(&config.Map)
}
+
+
+// getCredentials reads credentials from .brooklyn_cli data formatted for versions > 0.11.0
+// Note that the password is base64 encoded to avoid json formatting problems
+//{
+// "credentials": {
+// "password": "cGFzc3dvcmQ=",
+// "username": "geoff"
+// },
+// "skipSslChecks": false,
+// "target": "http://geoffs-macbook-pro.local:8081"
+//}
+func (config *Config) getCredentials(target string) (username string, password string, err error) {
+ credentials, found := config.Map[credentialsKey].(string)
+ if !found {
+ err = errors.New("No credentials found for " + target)
+ return
+ }
+
+ if decoded, errd := base64.StdEncoding.DecodeString(credentials); errd != nil {
+ err = errors.New("Could not decode credentials for " + target)
+ return
+ } else {
+ userAndPassword := strings.SplitN(string(decoded), ":", 2)
+ if len(userAndPassword) != 2 {
--- End diff --
It is possible for a brooklyn user to have a ':' in there password.
If that is the case then they will not be able to use the BR command as we will fail at this line.
I suggest encoding/decoding username and password separately
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
@robertgmoss don't worry, I'm sure I can get round to it some time in the next while if I put my mind to it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by robertgmoss <gi...@git.apache.org>.
Github user robertgmoss commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
@Graeme-Miller I take your point about security, but I am not convinced that caching 1 context is _really_ that much more secure than caching `n`! The surprising thing to me is that we are not using OAuth with us storing a token.
I also think ease of use should not be underestimated. It is not unusual for multiple platforms to store multiple contexts, see for instance Kubernetes.
Having said that, I am happy to remove my objection if people feel this is the way we should to go.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request #42: Improve login handling.
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/brooklyn-client/pull/42#discussion_r111932687
--- Diff: cli/net/net.go ---
@@ -121,15 +121,15 @@ func (net *Network) SendRequestGetStatusCode(req *http.Request) ([]byte, int, er
client := net.makeClient()
resp, err := client.Do(req)
if err != nil {
- return nil, 0, err
+ return nil, resp.StatusCode, err
--- End diff --
Is `resp` a valid reference when `err` is not nil?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request #42: Improve login handling.
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-client/pull/42#discussion_r118494527
--- Diff: cli/commands/login.go ---
@@ -53,6 +58,30 @@ func (cmd *Login) Metadata() command_metadata.CommandMetadata {
}
}
+
+func (cmd *Login) promptAndReadUsername() (username string) {
+ for username == "" {
+ reader := bufio.NewReader(os.Stdin)
+ fmt.Print("Enter Username: ")
+ user, err := reader.ReadString('\n')
+ if err != nil {
+ error_handler.ErrorExit(err)
+ }
+ username = strings.TrimSpace(user)
+ }
+ return username
+}
+
+func (cmd *Login) promptAndReadPassword() (password string) {
+ fmt.Print("Enter Password: ")
--- End diff --
Good point, that could be added.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by robertgmoss <gi...@git.apache.org>.
Github user robertgmoss commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
@m4rkmckenna sorry, I thought I'd implemented that feature, but clearly I didn't. Yes, @geomacy the format was like that so that you could simply change the `target` in the `login` command, but I guess I didn't get round to that.
I think it is better UX so my vote would be to leave the `.brooklyn_cli` structure and fix the missing implementation. Additionally, it would make sense to add commands to output the current target and choose between them.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request #42: Improve login handling.
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-client/pull/42
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
@geomacy I was right on the cusp of merging this when I realised that logging out means you can't view help text:
```
$ br version
0.12.0-SNAPSHOT
$ br
NAME:
br - A Brooklyn command line client application
<snip>
$ br logout
$ br
Not logged in
```
And similarly:
```
$ br logout -h
NAME:
logout - Logout of brooklyn
USAGE:
br logout
$ br logout
$ br logout -h
Not logged in
```
I'd like this behaviour to be fixed before merging.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request #42: Improve login handling.
Posted by johnmccabe <gi...@git.apache.org>.
Github user johnmccabe commented on a diff in the pull request:
https://github.com/apache/brooklyn-client/pull/42#discussion_r111959919
--- Diff: cli/commands/login.go ---
@@ -76,6 +81,17 @@ func (cmd *Login) Run(scope scope.Scope, c *cli.Context) {
cmd.network.BrooklynUrl = cmd.network.BrooklynUrl[0 : len(cmd.network.BrooklynUrl)-1]
}
+ // Prompt for username if not supplied
+ if cmd.network.BrooklynUser == "" {
+ reader := bufio.NewReader(os.Stdin)
+ fmt.Print("Enter Username: ")
+ user, err := reader.ReadString('\n')
+ if err != nil {
+ error_handler.ErrorExit(err)
+ }
+ cmd.network.BrooklynUser = strings.TrimSpace(user)
--- End diff --
You can end up with an empty string for the user, should pull this out into a function and potentially retry if no name is supplied.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
Nice catch @sjcorbett thanks - have added another commit to address that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
P.S. I took it out because I thought it was cleaner to keep it simple.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
On the current master:
```
$ ./target/bin/br login http://ldsn3e
Get http://ldsn3e/v1/server/version: dial tcp: lookup ldsn3e on 66.28.0.45:53: no such host
```
With the changes in this PR:
```
$ ./target/bin/br login http://ldsn3e
Enter Username: sdsada
Enter Password:
panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x10 pc=0x647b1]
goroutine 1 [running]:
panic(0x4134c0, 0xc820012120)
/usr/local/Cellar/go/1.6/libexec/src/runtime/panic.go:464 +0x3e6
github.com/apache/brooklyn-client/cli/net.(*Network).SendRequestGetStatusCode(0xc820010600, 0xc8200e2000, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
/Users/sam/code/brooklyn.io/brooklyn/brooklyn-client/cli/target/src/github.com/apache/brooklyn-client/cli/net/net.go:124 +0x1e1
github.com/apache/brooklyn-client/cli/api/version.Version(0xc820010600, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
/Users/sam/code/brooklyn.io/brooklyn/brooklyn-client/cli/target/src/github.com/apache/brooklyn-client/cli/api/version/version.go:31 +0xb4
github.com/apache/brooklyn-client/cli/commands.(*Login).Run(0xc8200133f0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
/Users/sam/code/brooklyn.io/brooklyn/brooklyn-client/cli/target/src/github.com/apache/brooklyn-client/cli/commands/login.go:125 +0xfa8
github.com/apache/brooklyn-client/cli/command_runner.ConcreteRunner.RunCmdByName(0x7bdd60, 0xc820013410, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
/Users/sam/code/brooklyn.io/brooklyn/brooklyn-client/cli/target/src/github.com/apache/brooklyn-client/cli/command_runner/runner.go:49 +0xe7
github.com/apache/brooklyn-client/cli/command_runner.(*ConcreteRunner).RunCmdByName(0xc820016540, 0x49bb98, 0x5, 0xc8200828c0, 0x0, 0x0)
<autogenerated>:1 +0xe0
github.com/apache/brooklyn-client/cli/app.getCommand.func1(0xc8200828c0)
/Users/sam/code/brooklyn.io/brooklyn/brooklyn-client/cli/target/src/github.com/apache/brooklyn-client/cli/app/app.go:82 +0x57
github.com/apache/brooklyn-client/cli/vendor/github.com/urfave/cli.Command.Run(0x49bb98, 0x5, 0x0, 0x0, 0x0, 0x0, 0x0, 0xc8200b4fe0, 0x1e, 0x0, ...)
/Users/sam/code/brooklyn.io/brooklyn/brooklyn-client/cli/target/src/github.com/apache/brooklyn-client/cli/vendor/github.com/urfave/cli/command.go:174 +0x1399
github.com/apache/brooklyn-client/cli/vendor/github.com/urfave/cli.(*App).Run(0xc8200823c0, 0xc82000b6b0, 0x3, 0x3, 0x0, 0x0)
/Users/sam/code/brooklyn.io/brooklyn/brooklyn-client/cli/target/src/github.com/apache/brooklyn-client/cli/vendor/github.com/urfave/cli/app.go:187 +0x112b
main.main()
/Users/sam/code/brooklyn.io/brooklyn/brooklyn-client/cli/target/src/github.com/apache/brooklyn-client/cli/br/brooklyn.go:62 +0x915
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
@sjcorbett et al is this ok to merge now?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
ok thanks @sjcorbett
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request #42: Improve login handling.
Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller commented on a diff in the pull request:
https://github.com/apache/brooklyn-client/pull/42#discussion_r118458240
--- Diff: cli/br/brooklyn.go ---
@@ -30,27 +30,14 @@ import (
"path/filepath"
)
-func getNetworkCredentialsFromConfig(yamlMap map[string]interface{}) (string, string, string, bool) {
- var target, username, password string
- var skipSslChecks bool
- target, found := yamlMap["target"].(string)
- if found {
- auth, found := yamlMap["auth"].(map[string]interface{})
- if found {
- creds := auth[target].(map[string]interface{})
- username, found = creds["username"].(string)
- if found {
- password, found = creds["password"].(string)
- }
- }
- skipSslChecks, _ = yamlMap["skipSslChecks"].(bool)
- }
- return target, username, password, skipSslChecks
-}
-
func main() {
config := io.GetConfig()
- target, username, password, skipSslChecks := getNetworkCredentialsFromConfig(config.Map)
+ skipSslChecks := config.GetSkipSslChecks()
+ target, username, password, err := config.GetNetworkCredentials()
+ if err != nil && !isLogin(os.Args) {
--- End diff --
It might read nicer to skip config.GetNetworkCredentials() if it is login, rather than ignore the error
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request #42: Improve login handling.
Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller commented on a diff in the pull request:
https://github.com/apache/brooklyn-client/pull/42#discussion_r118496514
--- Diff: cli/io/config.go ---
@@ -70,3 +98,111 @@ func (config *Config) Read() {
dec := json.NewDecoder(fileToRead)
dec.Decode(&config.Map)
}
+
+
+// getCredentials reads credentials from .brooklyn_cli data formatted for versions > 0.11.0
+// Note that the password is base64 encoded to avoid json formatting problems
+//{
+// "credentials": {
+// "password": "cGFzc3dvcmQ=",
+// "username": "geoff"
+// },
+// "skipSslChecks": false,
+// "target": "http://geoffs-macbook-pro.local:8081"
+//}
+func (config *Config) getCredentials(target string) (username string, password string, err error) {
+ credentials, found := config.Map[credentialsKey].(string)
+ if !found {
+ err = errors.New("No credentials found for " + target)
+ return
+ }
+
+ if decoded, errd := base64.StdEncoding.DecodeString(credentials); errd != nil {
+ err = errors.New("Could not decode credentials for " + target)
+ return
+ } else {
+ userAndPassword := strings.SplitN(string(decoded), ":", 2)
+ if len(userAndPassword) != 2 {
--- End diff --
Ah, I see. So, in theory, it could be an issue if the user has a ':' in their username- but that is quite unlikely.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by robertgmoss <gi...@git.apache.org>.
Github user robertgmoss commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
@geomacy I'd rather this was not merged as is. It would deprecate a current format, then introduce a further deprecation further down the line.
I will make a PR into your branch for the change
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by m4rkmckenna <gi...@git.apache.org>.
Github user m4rkmckenna commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
@robertgmoss I was un aware we had that functionality ... How is it supposed to work?
I tried
```
br login <host1> <host1-username> <host1-password>
br apps # Returns host1 apps
br login <host2> <host2-username> <host2-password>
br apps # Returns host2 apps
br login <host1>
br apps # Returns nothing
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
Alternatively open a separate pull request for the `logout` command. It's tangential to the rest of the PR, which is ready to merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by robertgmoss <gi...@git.apache.org>.
Github user robertgmoss commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
@geomacy I think it is a shame to lose the caching of credentials for multiple Brooklyn servers and the ability to switch between them without re-entering credentials.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request #42: Improve login handling.
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-client/pull/42#discussion_r118498184
--- Diff: cli/io/config.go ---
@@ -70,3 +98,111 @@ func (config *Config) Read() {
dec := json.NewDecoder(fileToRead)
dec.Decode(&config.Map)
}
+
+
+// getCredentials reads credentials from .brooklyn_cli data formatted for versions > 0.11.0
+// Note that the password is base64 encoded to avoid json formatting problems
+//{
+// "credentials": {
+// "password": "cGFzc3dvcmQ=",
+// "username": "geoff"
+// },
+// "skipSslChecks": false,
+// "target": "http://geoffs-macbook-pro.local:8081"
+//}
+func (config *Config) getCredentials(target string) (username string, password string, err error) {
+ credentials, found := config.Map[credentialsKey].(string)
+ if !found {
+ err = errors.New("No credentials found for " + target)
+ return
+ }
+
+ if decoded, errd := base64.StdEncoding.DecodeString(credentials); errd != nil {
+ err = errors.New("Could not decode credentials for " + target)
+ return
+ } else {
+ userAndPassword := strings.SplitN(string(decoded), ":", 2)
+ if len(userAndPassword) != 2 {
--- End diff --
Yes, tested it and it works ok.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: DO NOT MERGE WIP - Improve login handling.
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
Hopefully that should be ok now
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request #42: Improve login handling.
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-client/pull/42#discussion_r118494358
--- Diff: cli/br/brooklyn.go ---
@@ -30,27 +30,14 @@ import (
"path/filepath"
)
-func getNetworkCredentialsFromConfig(yamlMap map[string]interface{}) (string, string, string, bool) {
- var target, username, password string
- var skipSslChecks bool
- target, found := yamlMap["target"].(string)
- if found {
- auth, found := yamlMap["auth"].(map[string]interface{})
- if found {
- creds := auth[target].(map[string]interface{})
- username, found = creds["username"].(string)
- if found {
- password, found = creds["password"].(string)
- }
- }
- skipSslChecks, _ = yamlMap["skipSslChecks"].(bool)
- }
- return target, username, password, skipSslChecks
-}
-
func main() {
config := io.GetConfig()
- target, username, password, skipSslChecks := getNetworkCredentialsFromConfig(config.Map)
+ skipSslChecks := config.GetSkipSslChecks()
+ target, username, password, err := config.GetNetworkCredentials()
+ if err != nil && !isLogin(os.Args) {
--- End diff --
I think it's just cleaner to tell them to log in, rather than change the command - don't think that's too much of an imposition on the user.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
Hmm, belay that - I seem to have managed to lose a commit along the way or something like that; best wait until I figure that out before reviewing this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
hi @Graeme-Miller, @robertgmoss, @m4rkmckenna et al.
I've finally got round to this PR again and have updated it with a couple more commits to implement the suggestion by Robert https://github.com/apache/brooklyn-client/pull/42#issuecomment-303995483. You can switch to another site if you have already logged in previously by just `br login http://foo.com:8081` without supplying the credentials, and these will be picked up from the config file if possible, otherwise you'll be prompted.
Would appreciate another round of review and test on this please.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request #42: Improve login handling.
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-client/pull/42#discussion_r109676874
--- Diff: cli/commands/login.go ---
@@ -84,7 +100,8 @@ func (cmd *Login) Run(scope scope.Scope, c *cli.Context) {
error_handler.ErrorExit(err)
}
fmt.Printf("\n")
- cmd.network.BrooklynPass = string(bytePassword)
+ password := string(bytePassword)
+ cmd.network.BrooklynPass = strings.TrimSpace(password)
--- End diff --
good point!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
ping @Graeme-Miller, @robertgmoss, @m4rkmckenna et al, can this be merged now?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #42: Improve login handling.
Posted by robertgmoss <gi...@git.apache.org>.
Github user robertgmoss commented on the issue:
https://github.com/apache/brooklyn-client/pull/42
Thanks @geomacy LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---