[PAAS-559] Add --no-browser flag to SSO login flow

[Elizabeth Bennett (zzbennett)]

What

Adds the --no-browser flag to the SSO login flow. This prints the SSO login url and the user copies and pastes it into their browser. After successfully logging in via their browser, they paste the resultant code into the CLI, which awaits their input.

Some refactoring of the auth_server code was necessary. I broke up the auth_server into two pieces: auth_server, which handles only the local server logic that is used in the browser based flow, and auth_state, which handles all SSO related state that is required by both flows.

Added unit tests for the two new classes. Still need some kind of system test that will test with a real auth0 account and real browser. That will be handled in the coming weeks with the rest of the SSO related system testing work.

References

This requires a new endpoint in CCloud which does not yet exist, but is being tracked here: https://confluentinc.atlassian.net/browse/MMA-5601

Test&Review

Unit tested. Tested locally with my SSO enabled CPD account and manually simulating the CCloud endpoint that does not yet exist using a local http server.

[David Hyde (DABH)]

Generally looks good up to some potential spots for clean up / organization.

Note, since CLI deploys new versions on merge, we cannot merge this PR until the backend is all in place. I am happy to +1 this and get it merged once we've seen that the backend is all working.

[David Hyde (DABH)]

Thanks for splitting this into two files, it needed it 👍

[David Hyde (DABH)]

Nice tests

[David Hyde (DABH)]

This is getting to be a lot of code in this file that's specific to sso. In particular I wanted to avoid imports like bufio, browser, etc. in auth.go. Maybe we can factor out the body of this huge if statement into a small separate file like ssoAuthFlow.go or something? What do you think?

[Elizabeth Bennett (zzbennett)]

Yea sure, I think that makes sense.

[Elizabeth Bennett (zzbennett)]

Okay will hold off on merging until the UI pieces are in place and we're ready for the SSO GA

[laura-brouckman]

make sure this gets updated to https://confluent.cloud/cli_callback

[Elizabeth Bennett (zzbennett)]

good call, will do

[Elizabeth Bennett (zzbennett)]

mkay done

[Elizabeth Bennett (zzbennett)]

David Hyde (@DABH) Cody A. Ray (@codyaray) I've finally finished the browser tests for the CLI sso auth flow. They're in the last couple of commits in this PR. Might want to take a look at those too. They didn't end up being too crazy but they're kind of weird and they also make chrome a dependency for building the CLI which I don't like, but there is a flag to skip the browsers tests if needed.

[David Hyde (DABH)]

Elizabeth Bennett (@zzbennett) does the chrome dependency make the CLI binaries bigger? And/or, does it materially impact compile time for the CLIs if we're just building normally (make build-go)? A little concerned about that though I get why it may be needed.

[Elizabeth Bennett (zzbennett)]

The chrome dependency doesn't affect the binary, it just means that chrome has to be installed on the machine in order to run the CLI tests. It's working well in semaphore since semaphore's environment does have chrome installed and most devs will have chrome installed on their laptops. It just might cause headaches later on if we want to set up CI pipelines on servers without chrome installed, but I added a flag to skip the browser tests, so worst case we'd end up needing to have two CI jobs, one for the browser tests and one for the rest of the tests.

Re: build time, it does add about 6 seconds to the build time when the test is passing. The test is configured with a 10 second timeout so at most it will add 10 seconds to the build time. The overall build time will go from about 20 seconds to 25-30 seconds. it's not ideal, but also not too awful.

edit: i should say, the chrome dependency shouldn't affect the binary, although I'm not 100% sure how go modules work. I don't know if there's a way to specify test only dependenecies

edit #2: it seems that go automagically does some analysis to figure out which dependencies are test dependencies and which are runtime dependencies so the cli binaries shouldn't get bloated

[David Hyde (DABH)]

Thanks for following up on that. This all sounds good.

We are switching to Azure Pipelines for CI. Can you merge master into your branch, push, and see if AP passes/fails? I guess the only risk is chrome not being installed; if so we can modify azure-pipelines.yml to install chrome as a pre-test step on each OS image (linux/mac/windows so far). Let me know. Thanks - excited to get this in!