Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Polish the login flow #11

Merged
merged 10 commits into from
Jan 25, 2021
Merged

Polish the login flow #11

merged 10 commits into from
Jan 25, 2021

Conversation

cyx
Copy link
Contributor

@cyx cyx commented Jan 23, 2021

Description

Adds the necessary plumbing to persist the login state.

internal/auth/auth.go Outdated Show resolved Hide resolved
@@ -42,6 +42,7 @@ type Authenticator struct {

type Result struct {
Tenant string
Domain string
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We forgot to include this -- ultimately this is the parameter we thread through to the management.New call (e.g. https://github.com/go-auth0/auth0)

Comment on lines +18 to +20
// config defines the exact set of tenants, access tokens, which only exists
// for a particular user's machine.
type config struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In retrospect data didn't seem clear enough as a type name :)

Name string `json:"name"`
Domain string `json:"domain"`
AccessToken string `json:"access_token,omitempty"`
ExpiresAt time.Time `json:"expires_at"`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're not using ExpiresAt yet -- but eventually we can use it to determine when to re-trigger a new login.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created a backlog item so we don't lose track https://auth0team.atlassian.net/browse/A0CLI-9

Comment on lines -50 to 91
if t.BearerToken != "" {
c.api, err = management.New(t.Domain,
management.WithStaticToken(t.BearerToken),
management.WithDebug(c.verbose))
} else {
if t.AccessToken != "" {
c.api, err = management.New(t.Domain,
management.WithClientCredentials(t.ClientID, t.ClientSecret),
management.WithStaticToken(t.AccessToken),
management.WithDebug(c.verbose))
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally we had an if else for M2M or a static token. Given that we've made device flow work already, we can remove this until we decide we need it.

if c.path == "" {
c.path = path.Join(os.Getenv("HOME"), ".config", "auth0", "config.json")
}
// setTenant assigns an existing, or new tenant. This is expected to be called
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setTenant is the meat of the PR. The rest of the stuff around it is cleanup and code polish.

Comment on lines -107 to +166
c.renderer = &display.Renderer{
Tenant: c.tenant,
MessageWriter: os.Stderr,
ResultWriter: os.Stdout,
Format: display.OutputFormat(format),
}
c.renderer.Format = display.OutputFormat(format)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we've already initialized a Renderer before, we don't really need to do that -- we just need to assign Format, Tenant, etc.

Comment on lines +169 to +171
// Once initialized, we'll keep returning the same err that was
// originally encountered.
return c.errOnce
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a bug fix in the UX such that you'll occasionally get Unable to find tenant errors.

internal/cli/login.go Outdated Show resolved Hide resolved
Comment on lines +75 to +76
func getLogin(cli *cli) string {
if !cli.isLoggedIn() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UX improvements: this checks if we're logged in and changes / amends the help text in the main auth0 command.

@woloski
Copy link

woloski commented Jan 23, 2021 via email

@cyx cyx merged commit 430935a into main Jan 25, 2021
@cyx cyx deleted the polish-the-login-flow branch January 25, 2021 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants