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

refactor(auth): improve authentication flow for oauth 2.0 with local webserver #325

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 7 additions & 13 deletions cmd/gateclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,10 +388,14 @@ func authenticateOAuth2(output func(string), httpClient *http.Client, endpoint s
return false, errors.New("incorrect OAuth2 auth configuration")
}

redirectURL := url.URL{
Scheme: "http",
Host: localWebServer,
}
config := &oauth2.Config{
ClientID: OAuth2.ClientId,
ClientSecret: OAuth2.ClientSecret,
RedirectURL: "http://localhost:8085",
RedirectURL: redirectURL.String(),
Scopes: OAuth2.Scopes,
Endpoint: oauth2.Endpoint{
AuthURL: OAuth2.AuthUrl,
Expand All @@ -409,14 +413,6 @@ func authenticateOAuth2(output func(string), httpClient *http.Client, endpoint s
return false, errors.Wrapf(err, "Could not refresh token from source: %v", tokenSource)
}
} else {
// Do roundtrip.
http.Handle("/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
code := r.FormValue("code")
fmt.Fprintln(w, code)
}))
go http.ListenAndServe(":8085", nil)
// Note: leaving server connection open for scope of request, will be reaped on exit.

verifier, verifierCode, err := generateCodeVerifier()
if err != nil {
return false, err
Expand All @@ -427,10 +423,8 @@ func authenticateOAuth2(output func(string), httpClient *http.Client, endpoint s
challengeMethod := oauth2.SetAuthURLParam("code_challenge_method", "S256")

authURL := config.AuthCodeURL("state-token", oauth2.AccessTypeOffline, oauth2.ApprovalForce, challengeMethod, codeChallenge)
output(fmt.Sprintf("Navigate to %s and authenticate", authURL))
code := prompt(output, "Paste authorization code:")

newToken, err = config.Exchange(context.Background(), code, codeVerifier)
output("Trying to get token from web")
newToken, err = getTokenFromWeb(output, config, authURL, codeVerifier)
if err != nil {
return false, err
}
Expand Down
85 changes: 85 additions & 0 deletions cmd/gateclient/oauth.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package gateclient

import (
"context"
"fmt"
"log"
"net"
"net/http"
"os/exec"
"runtime"

"golang.org/x/oauth2"
)

const localWebServer = "localhost:8085"

func startWebServer() (codeCh chan string, err error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a test for this please? - see: https://pkg.go.dev/net/http/httptest
We can validate:

  1. code FormValue is supplied in test and plumbed through to codeCh
  2. http response recorder has correct output
  3. listener closed
  4. error FormValue (per next comment - if part of this PR).

listener, err := net.Listen("tcp", localWebServer)
if err != nil {
return nil, err
}
codeCh = make(chan string)

go http.Serve(listener, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// TODO: add handle of `error`
Copy link
Contributor

Choose a reason for hiding this comment

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

This is commented but new in this PR. Can you give an example of what error is here?
Could be something for a follow up PR.

//error := r.FormValue("error")
code := r.FormValue("code")
codeCh <- code // send code to OAuth flow
listener.Close()
w.Header().Set("Content-Type", "text/plain")
fmt.Fprintf(w, "Received code: %v\r\nYou can now safely close this browser window.", code)
}))

return codeCh, nil
}

// getTokenFromWeb uses Config to request a Token.
// It returns the retrieved Token.
func getTokenFromWeb(output func(string), config *oauth2.Config, authURL string, opts ...oauth2.AuthCodeOption) (*oauth2.Token, error) {
codeCh, err := startWebServer()
if err != nil {
output("Unable to start a web server.")
return nil, err
}

err = openURL(authURL)
if err != nil {
log.Fatalf("Unable to open authorization URL in web server: %v", err)
} else {
output("Your browser has been opened to an authorization URL.\n" +
" This program will resume once authorization has been provided.\n")
output(authURL)
}

// Wait for the web server to get the code.
code := <-codeCh
return exchangeToken(config, code, opts...)
}

// Exchange the authorization code for an access token
func exchangeToken(config *oauth2.Config, code string, opts ...oauth2.AuthCodeOption) (*oauth2.Token, error) {
tok, err := config.Exchange(context.Background(), code, opts...)
if err != nil {
return nil, fmt.Errorf("Unable to retrieve token %v", err)
}
return tok, nil
}

// openURL opens a browser window to the specified location.
// This code originally appeared at:
// http://stackoverflow.com/questions/10377243/how-can-i-launch-a-process-that-is-not-a-file-in-go
func openURL(url string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Using a local web server or prompt is optional in the linked code and this seems quite robust. If local web server doesn't work then use the prompt. https://github.com/youtube/api-samples/blob/master/go/oauth2.go

I'm not sure what the failure modes are for the various openURL() commands, like exec hanging with no feedback. I've managed to misconfigure Linux for xdg-open more than once 😅

If an error is returned we bail out at L48 with log.Fatalf and there is no way for a user to progress unless they fix the exec issue.

I wonder if we might want to do something like:
a) fall back to prompt for code?
b) make localWebServer selectable via spin config (default enabled?) similer to https://github.com/youtube/api-samples/blob/07263305b59a7c3275bc7e925f9ce6cabf774022/go/oauth2.go#L44

WDYT?

var err error
switch runtime.GOOS {
case "linux":
err = exec.Command("xdg-open", url).Start()
case "windows":
err = exec.Command("rundll32", "url.dll,FileProtocolHandler", url).Start()
case "darwin":
err = exec.Command("open", url).Start()
default:
err = fmt.Errorf("Cannot open URL %s on this platform", url)
}
return err
}