Implement logout #123
Implement logout #123
Conversation
internal/commands/logout.go
Outdated
| if err := store.Erase(); err != nil { | ||
| return err | ||
| } | ||
| fmt.Println(ansi.Info("Logout Succeeded")) |
There was a problem hiding this comment.
We use streams elsewhere, I know you don't agree but we should try to be consistent.
Signed-off-by: Djordje Lukic <[email protected]>
rumpl
force-pushed
the
feat-logout
branch
2 times, most recently
from
97b86c0
to
5056803
Compare
Signed-off-by: Djordje Lukic <[email protected]>
| @@ -47,7 +47,7 @@ func newLoginCmd(streams command.Streams, store credentials.Store, hubClient *hu | |||
| if err := login.RunLogin(cmd.Context(), streams, hubClient, store, args[0]); err != nil { | |||
| return err | |||
| } | |||
| fmt.Println(ansi.Info("Login Succeeded")) | |||
| fmt.Fprintln(streams.Out(), ansi.Info("Login Succeeded")) | |||
There was a problem hiding this comment.
Wondering; does this CLI also print a warning when not using a credentials-store?
There was a problem hiding this comment.
Yeah it does this:
~/dev/hub-tool feat-logout
❯ docker logout
Removing login credentials for https://index.docker.io/v1/
~/dev/hub-tool feat-logout*
❯ docker logout
Removing login credentials for https://index.docker.io/v1/
WARNING: could not erase credentials:
https://index.docker.io/v1/: error erasing credentials - err: exit status 1, out: `error erasing credentials - err: exit status 1, out: `The specified item could not be found in the keychain.``
There was a problem hiding this comment.
I decided to mute the error when you logout but there are no credentials, am happy to put it back if you think this is useful
| func newLogoutCmd(streams command.Streams, store credentials.Store) *cobra.Command { | ||
| cmd := &cobra.Command{ | ||
| Use: logoutName + " USERNAME", | ||
| Short: "Logout of the Hub", |
There was a problem hiding this comment.
Reminds me, we should probably update the descriptions of these. Effectively, we're not "logging in" or "logging out", but only storing/removing local credentials.
There was a problem hiding this comment.
Ah yes indeed, will do thanks
There was a problem hiding this comment.
Thinking about this a little bit more, the fact that we are storing/removing local credentials is an implementation detail (at least it feels that it is). In the end once the user does a hub-tool login they can do a bunch of authenticated calls, once they hub-tool logout they cannot any longer because the tool cannot make any authenticated calls any more. Having a description that says Remove the credentials from the store is far less useful to me than just Logout of the Hub. WDYT?
Signed-off-by: Djordje Lukic <[email protected]>
- What I did
Implemented the logout command
- How I did it
Call
Eraseon the credentials store- How to verify it
The last command should tell the user that they need to login.