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

Add the get-tokens command #56

Merged
merged 6 commits into from
Jan 28, 2021
Merged

Add the get-tokens command #56

merged 6 commits into from
Jan 28, 2021

Conversation

xer0x
Copy link
Contributor

@xer0x xer0x commented Jan 26, 2021

Description

Adds a command to get a token that can be used to access an API via curl, postman, insomnia, etc.

References

Screenshot

Screen Shot 2021-01-27 at 11 18 56 PM

Unfortunately, the output has some extra whitespace around the token. :/

Testing

  • This change adds test coverage for new/changed/fixed functionality

Checklist

  • I have added documentation for new/changed functionality in this PR or in auth0.com/docs
  • All active GitHub checks for tests, formatting, and security are passing
  • The correct base branch is being used, if not master

@xer0x xer0x mentioned this pull request Jan 26, 2021
4 tasks
Copy link
Contributor

@cyx cyx left a comment

Choose a reason for hiding this comment

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

Looking great!

Copy link
Contributor

@jfatta jfatta left a comment

Choose a reason for hiding this comment

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

just 👀

internal/cli/apis.go Outdated Show resolved Hide resolved
@xer0x
Copy link
Contributor Author

xer0x commented Jan 27, 2021

Lol, well we've got a roughed out API call here now.

I'm hoping to get to the json parsing this evening. Maybe add real values instead of the hard coded configuration.

@xer0x xer0x force-pushed the a0cli-20-token branch 2 times, most recently from 02b69f7 to 077c542 Compare January 28, 2021 07:16
@xer0x
Copy link
Contributor Author

xer0x commented Jan 28, 2021

Updated the PR to move the command into Auth0 get-token. This code triggers for M2M applications.

@cyx anything more to update here?

@xer0x xer0x requested review from cyx and jfatta January 28, 2021 18:29
Comment on lines 34 to 37
tenant, err := cli.getTenant()
if err != nil {
return tokenResponse, err
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We already do this at the top of the command itself, could we just pass the tenant in to this function perhaps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change the code to pass an extra tenant parameter to runClientCredentialsFlow()

cli.renderer.Infof("Domain: " + tenant.Domain)
cli.renderer.Infof("ClientID: " + clientID)
cli.renderer.Infof("Type: Machine to Machine")
fmt.Println()
Copy link
Contributor

Choose a reason for hiding this comment

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

This will print to stdout which might mess with piping the result to another process (like jq), have you tried that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

jq won't be confused by whitespace.

However, we could switch to cli.renderer.Infof("") if that would be prettier. I didn't use it because it would put the green triangle.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤔 actually, it would be nice if auth0 get-token --format json would output only json. Do you know if the tool already has logic like this? Seeing an example of the tool hiding output would be great.

Copy link
Contributor

Choose a reason for hiding this comment

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

So that's what this line is doing https://github.com/auth0/auth0-cli/pull/56/files#diff-32da9d2d97d78dec736cae68c2195111921918696567466ad99f78a291237298R52

fmt.Fprint(cli.renderer.MessageWriter, "\n") MessageWriter goes to stderr, while ResultWriter goes to stdout. That means we can print whatever we want to stderr but the only things that'll get piped onwards are results printed via the results writer.

A --quiet flag or similar might be a good addition though, avoid even writing the informational stuff to stderr.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Awesome! Okay that helps. Thanks Paddy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The STDOUT split seems to work

$ go run ./cmd/auth0 get-token --audience https://cli.test.auth0.fake  --client-id wgxFPH6hY3JRwYjAjiFUxU9sjxNheLOm --format json > /dev/null | jq '.access_token' | pbcopy

...token is on my clipboard!  ready to paste into Insomnia

Comment on lines 41 to 43
cli.renderer.Infof("Domain: " + tenant.Domain)
cli.renderer.Infof("ClientID: " + clientID)
cli.renderer.Infof("Type: Machine to Machine")
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be useful at the command-level, so it works for both types of flow, have you considered moving it up there?


// TODO: Check if the audience is valid, and suggest a different client if it is wrong.

payload := strings.NewReader("grant_type=client_credentials&client_id=" + clientID + "&client_secret=" + client_secret + "&audience=" + audience)
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use url.Values to avoid building up the payload string manually, it'll also take care of all sorts of encoding edge cases for you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Neat, I've switched!


var tokenResponse *authutil.TokenResponse

url := "https://" + tenant.Domain + "/oauth/token"
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use url.URL here, might be safer/cleaner than building the URL manually, there's an example at https://github.com/auth0/auth0-cli/blob/main/internal/auth/authutil/login.go#L34

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes! I was feeling like there must be a better way!

Comment on lines 48 to 52
req, _ := http.NewRequest("POST", url, payload)

req.Header.Add("content-type", "application/x-www-form-urlencoded")

res, err := http.DefaultClient.Do(req)
Copy link
Contributor

Choose a reason for hiding this comment

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

Might http.PostForm be a little cleaner here/more succinct? there's an example at https://github.com/auth0/auth0-cli/blob/main/internal/auth/auth.go#L89-L98

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PostForm?! What's this? Hello new friend.

Copy link
Contributor

@cyx cyx left a comment

Choose a reason for hiding this comment

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

Nice

@xer0x xer0x merged commit 591c7d3 into main Jan 28, 2021
@xer0x xer0x deleted the a0cli-20-token branch January 28, 2021 22:59
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.

4 participants