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

Discard the local login server on non-desktop environment #1502

Merged
merged 1 commit into from
Oct 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 25 additions & 13 deletions pkg/login/flow.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,29 +167,39 @@ func (f *Flow) triggerMethod(provider *Provider) (acsToken string, err error) {
if provider.ID == DCOSOIDCAuth0 {
loginServer, err = loginserver.New(startFlowURL)
if err != nil {
return "", err
f.logger.Error(err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Previously we returned at this point. Why this change?

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 lets use fallback to reading from stdin in that case.

} else {
startFlowURL = loginServer.StartFlowURL()
go loginServer.Start()
defer loginServer.Close()
}
startFlowURL = loginServer.StartFlowURL()
go loginServer.Start()
}

if err := f.openBrowser(startFlowURL); err != nil {
return "", err
}
// Being unable to open the browser is not critical, a message was prompted
// to the user with the URL they should go to in order to start the login flow.
f.logger.Info(err)

if loginServer != nil {
// However we close and don't use the web server in this scenario, as the CLI might
// be running on a non-desktop environment so the we server would be stuck waiting.
// Falling back to reading from STDIN is safer here.
//
// See https://jira.mesosphere.com/browse/DCOS_OSS-5591
loginServer.Close()
} else if loginServer != nil {
bamarni marked this conversation as resolved.
Show resolved Hide resolved
token = <-loginServer.Token()
}

if token == "" {
token = f.prompt.Input("Enter token from the browser: ")
} else {
// When the token comes from the local webserver there is no need to retry. In case of an
// error, retrying isn't needed (it can't be a typo, no copy-pasting was involved).
// Thus we raise the attempt counter to 3.
attempt = 3
}
}

if token == "" {
token = f.prompt.Input("Enter token from the browser: ")
}

f.interactive = true

// methodBrowserOIDCToken relies on a login token,
Expand Down Expand Up @@ -262,10 +272,12 @@ func (f *Flow) openBrowser(startFlowURL string) error {
}
startFlowURL = req.URL.String()
}
if err := f.opener.Open(startFlowURL); err != nil {
f.logger.Error(err)
}

msg := "If your browser didn't open, please follow this link:\n\n %s\n\n"
fmt.Fprintf(f.errout, msg, startFlowURL)

if err := f.opener.Open(startFlowURL); err != nil {
return err
}
return nil
}
109 changes: 109 additions & 0 deletions pkg/login/flow_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,22 @@
package login

import (
"bytes"
"encoding/json"
"errors"
"io/ioutil"
"net/http"
"net/http/httptest"
"net/url"
"testing"

"github.com/dcos/dcos-cli/pkg/httpclient"
"github.com/dcos/dcos-cli/pkg/login/loginserver"
"github.com/dcos/dcos-cli/pkg/open"
"github.com/dcos/dcos-cli/pkg/prompt"
"github.com/sirupsen/logrus"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -39,3 +52,99 @@ func TestSelectUIDPasswordProvider(t *testing.T) {
require.NoError(t, err)
require.True(t, provider == providers["login-provider-1"] || provider == providers["login-provider-2"])
}

func TestTriggerMethodOIDCWithLoginServer(t *testing.T) {
expectedLoginToken := "dummy_login_token"
expectedACSToken := "dummy_acs_token"

opener := open.OpenerFunc(func(resource string) error {

startFlowURL, err := url.Parse(resource)
require.NoError(t, err)

loginData := &loginserver.LoginData{
Token: expectedLoginToken,
CSRFToken: startFlowURL.Query().Get("dcos_cli_csrf_token"),
}

var buf bytes.Buffer
err = json.NewEncoder(&buf).Encode(loginData)
require.NoError(t, err)

resp, err := httpclient.New("").Post(startFlowURL.Query().Get("redirect_uri"), "application/json", &buf)
require.NoError(t, err)
require.Equal(t, 200, resp.StatusCode)

return nil
})

ts := httptest.NewServer(mockLoginEndpoint(t, expectedLoginToken, expectedACSToken))
defer ts.Close()

logger := &logrus.Logger{Out: ioutil.Discard}

flow := NewFlow(FlowOpts{
Logger: logger,
Opener: opener,
})

flow.client = NewClient(httpclient.New(ts.URL), logger)

acsToken, err := flow.triggerMethod(defaultOIDCImplicitFlowProvider())
require.NoError(t, err)
require.Equal(t, expectedACSToken, acsToken)
}

func TestTriggerMethodOIDCWithoutLoginServer(t *testing.T) {

// We're simulating a container environment where the browser can't be opened.
// See https://jira.mesosphere.com/browse/DCOS_OSS-5591
opener := open.OpenerFunc(func(_ string) error {
return errors.New("couldn't open browser, I'm just a container")
})

expectedLoginToken := "dummy_login_token"
expectedACSToken := "dummy_acs_token"

ts := httptest.NewServer(mockLoginEndpoint(t, expectedLoginToken, expectedACSToken))
defer ts.Close()

logger := &logrus.Logger{Out: ioutil.Discard}

var in bytes.Buffer
in.WriteString(expectedLoginToken)

flow := NewFlow(FlowOpts{
Prompt: prompt.New(&in, ioutil.Discard),
Logger: logger,
Opener: opener,
})

flow.client = NewClient(httpclient.New(ts.URL), logger)

acsToken, err := flow.triggerMethod(defaultOIDCImplicitFlowProvider())
require.NoError(t, err)
require.Equal(t, expectedACSToken, acsToken)
}

func mockLoginEndpoint(t *testing.T, expectedLoginToken, acsToken string) http.Handler {
mux := http.NewServeMux()
mux.HandleFunc(defaultLoginEndpoint, func(w http.ResponseWriter, req *http.Request) {
assert.Equal(t, "POST", req.Method)

var credentials Credentials
if err := json.NewDecoder(req.Body).Decode(&credentials); err != nil {
w.WriteHeader(401)
return
}
if !assert.Equal(t, expectedLoginToken, credentials.Token) {
w.WriteHeader(401)
return
}
var jwt JWT
jwt.Token = acsToken
err := json.NewEncoder(w).Encode(&jwt)
assert.NoError(t, err)
})
return mux
}
4 changes: 3 additions & 1 deletion pkg/login/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"github.com/sirupsen/logrus"
)

const defaultLoginEndpoint = "/acs/api/v1/auth/login"

var (
// ErrAuthDisabled is the error returned when attempting to get authentication providers
// from a cluster without authentication.
Expand Down Expand Up @@ -133,7 +135,7 @@ func (c *Client) sniffAuth(acsToken string) (*http.Response, error) {
// Login makes a POST requests to the login endpoint with the given credentials.
func (c *Client) Login(loginEndpoint string, credentials *Credentials) (string, error) {
if loginEndpoint == "" {
loginEndpoint = "/acs/api/v1/auth/login"
loginEndpoint = defaultLoginEndpoint
}

reqBody, err := json.Marshal(credentials)
Expand Down
21 changes: 14 additions & 7 deletions pkg/login/loginserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,11 @@ func New(startFlowURL string) (*LoginServer, error) {
u.RawQuery = q.Encode()
ls.startFlowURL = u.String()

return ls, nil
}

// Start starts the login server.
func (ls *LoginServer) Start() error {
c := cors.New(cors.Options{
AllowedOrigins: []string{"https://dcos.auth0.com"},
})

return http.Serve(ls.listener, c.Handler(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
ls.srv = &http.Server{Handler: c.Handler(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
var loginData LoginData

err := json.NewDecoder(req.Body).Decode(&loginData)
Expand All @@ -90,7 +85,19 @@ func (ls *LoginServer) Start() error {
return
}
ls.tokenCh <- loginData.Token
})))
}))}

return ls, nil
}

// Start starts the login server.
func (ls *LoginServer) Start() error {
return ls.srv.Serve(ls.listener)
}

// Close closes the login server.
func (ls *LoginServer) Close() error {
return ls.srv.Close()
}

// StartFlowURL returns the start flow URL for the login server based flow.
Expand Down
8 changes: 8 additions & 0 deletions pkg/open/open.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,11 @@ func NewOsOpener(logger *logrus.Logger) *OsOpener {
logger: logger,
}
}

// OpenerFunc is a func adapter for the Opener interface.
type OpenerFunc func(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.

This is used only in test https://github.com/dcos/dcos-cli/pull/1502/files#diff-48a7e37bb2ef223e6225547541997ea1R58 so it can be placed in _test.go file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even though it is only used in a test right now, it is meant to be exposed to other projects using this package (to provide a custom open mechanism, or for their own tests).


// Open invokes the OpenerFunc.
func (f OpenerFunc) Open(resource string) error {
return f(resource)
}