diff --git a/README.md b/README.md index d69256aed..8914db874 100644 --- a/README.md +++ b/README.md @@ -72,8 +72,8 @@ If you want to build the development version the easiest (after switching branch 1. Build and start the vite web server, to support error reporting and hot-reloading: `just web/dev` in one terminal. Keep the terminal window open. - This will launch the `vite` web server on `http://127.0.0.1:9000` 2. Within a new terminal window, build the development version of the CLI: `just build-dev`. Keep this terminal open, so that you can rebuild as needed. -3. Within a new terminal window, launch the development version of the CLI for the current platform (with these parameters): `./connect-client publish-ui --listen=127.0.0.1:9001 --open-browser-at="http://127.0.0.1:9000" --skip-browser-session-auth` - - *Where** `` above is replaced with a location of a sample project. For example, with a python project at `~/dev/connect-content/bundles/python-flaskapi`, your complete command line would become: `./connect-client publish-ui ~/dev/connect-content/bundles/python-flaskapi --listen=127.0.0.1:9001 --open-browser-at="http://127.0.0.1:9000" --skip-browser-session-auth`. +3. Within a new terminal window, launch the development version of the CLI for the current platform (with these parameters): `./connect-client publish-ui --listen=127.0.0.1:9001 --open-browser-at="http://127.0.0.1:9000"` + - *Where** `` above is replaced with a location of a sample project. For example, with a python project at `~/dev/connect-content/bundles/python-flaskapi`, your complete command line would become: `./connect-client publish-ui ~/dev/connect-content/bundles/python-flaskapi --listen=127.0.0.1:9001 --open-browser-at="http://127.0.0.1:9000"`. - This launches the CLI and configures it to listen on port `9001`, while launching a browser to the address which is being served by the `vite` web server. You now should have a Web UX loaded within the browser, which is loading from the `vite` dev server, but has its APIs serviced from the CLI backend. diff --git a/cmd/connect-client/commands/publish.go b/cmd/connect-client/commands/publish.go index 77aef67a0..5ee25156c 100644 --- a/cmd/connect-client/commands/publish.go +++ b/cmd/connect-client/commands/publish.go @@ -308,7 +308,6 @@ func (cmd *PublishUICmd) Run(args *cli_types.CommonArgs, ctx *cli_types.CLIConte "/", cmd.UIArgs, &cmd.PublishArgs, - ctx.LocalToken, ctx.Fs, ctx.Accounts, log, diff --git a/cmd/connect-client/main.go b/cmd/connect-client/main.go index 08e9dbfbc..e458e8248 100644 --- a/cmd/connect-client/main.go +++ b/cmd/connect-client/main.go @@ -13,7 +13,6 @@ import ( "github.com/rstudio/connect-client/internal/events" "github.com/rstudio/connect-client/internal/logging" "github.com/rstudio/connect-client/internal/project" - "github.com/rstudio/connect-client/internal/services" "github.com/spf13/afero" ) @@ -37,11 +36,7 @@ func logVersion(log logging.Logger) { func makeContext(log logging.Logger) (*cli_types.CLIContext, error) { fs := afero.NewOsFs() accountList := accounts.NewAccountList(fs, log) - token, err := services.NewLocalToken() - if err != nil { - return nil, err - } - ctx := cli_types.NewCLIContext(accountList, token, fs, log) + ctx := cli_types.NewCLIContext(accountList, fs, log) return ctx, nil } @@ -75,9 +70,6 @@ func main() { if cli.Debug { ctx.Logger = events.NewLogger(true) } - if cli.Token != nil { - ctx.LocalToken = *cli.Token - } cmd, ok := args.Selected().Target.Interface().(commands.StatefulCommand) if ok { // For these commands, we need to load saved deployment state diff --git a/cmd/connect-client/main_test.go b/cmd/connect-client/main_test.go index 783857a07..fbe1a9fed 100644 --- a/cmd/connect-client/main_test.go +++ b/cmd/connect-client/main_test.go @@ -24,6 +24,5 @@ func (s *MainSuite) TestMakeContext() { ctx, err := makeContext(log) s.Nil(err) s.NotNil(ctx.Accounts) - s.NotEqual(ctx.LocalToken, "") s.Equal(log, ctx.Logger) } diff --git a/extensions/positron/src/commands.ts b/extensions/positron/src/commands.ts index 25f9fa1e8..06c080d0b 100644 --- a/extensions/positron/src/commands.ts +++ b/extensions/positron/src/commands.ts @@ -3,5 +3,5 @@ const executable: string = "connect-client"; export type Command = string; export const create = (port: number, subcommand: string = "publish-ui"): Command => { - return `${executable} ${subcommand} --listen=127.0.0.1:${port} test/sample-content/fastapi-simple --skip-browser-session-auth`; + return `${executable} ${subcommand} --listen=127.0.0.1:${port} test/sample-content/fastapi-simple`; }; diff --git a/internal/cli_types/base.go b/internal/cli_types/base.go index 09d1678cd..dbe25eb82 100644 --- a/internal/cli_types/base.go +++ b/internal/cli_types/base.go @@ -5,7 +5,6 @@ package cli_types import ( "github.com/rstudio/connect-client/internal/accounts" "github.com/rstudio/connect-client/internal/logging" - "github.com/rstudio/connect-client/internal/services" "github.com/rstudio/connect-client/internal/state" "github.com/rstudio/connect-client/internal/util" @@ -13,9 +12,8 @@ import ( ) type CommonArgs struct { - Debug bool `help:"Enable debug mode." env:"CONNECT_DEBUG"` - Profile string `help:"Enable CPU profiling"` - Token *services.LocalToken `help:"Authentication token for the publishing UI. Default auto-generates a token."` + Debug bool `help:"Enable debug mode." env:"CONNECT_DEBUG"` + Profile string `help:"Enable CPU profiling"` } type Log interface { @@ -23,30 +21,27 @@ type Log interface { } type CLIContext struct { - Accounts accounts.AccountList - LocalToken services.LocalToken - Fs afero.Fs - Logger logging.Logger + Accounts accounts.AccountList + Fs afero.Fs + Logger logging.Logger } -func NewCLIContext(accountList accounts.AccountList, token services.LocalToken, fs afero.Fs, log logging.Logger) *CLIContext { +func NewCLIContext(accountList accounts.AccountList, fs afero.Fs, log logging.Logger) *CLIContext { return &CLIContext{ - Accounts: accountList, - LocalToken: token, - Fs: fs, - Logger: log, + Accounts: accountList, + Fs: fs, + Logger: log, } } type UIArgs struct { - Interactive bool `short:"i" help:"Launch a browser to show the UI at the listen address."` - OpenBrowserAt string `help:"Launch a browser to show the UI at specific network address." placeholder:"HOST[:PORT]" hidden:""` - SkipBrowserSessionAuth bool `help:"Skip Browser Token Auth Checks" hidden:""` - Theme string `help:"UI theme, 'light' or 'dark'." hidden:""` - Listen string `help:"Network address to listen on." placeholder:"HOST[:PORT]" default:"localhost:0"` - AccessLog bool `help:"Log all HTTP requests."` - TLSKeyFile string `help:"Path to TLS private key file for the UI server."` - TLSCertFile string `help:"Path to TLS certificate chain file for the UI server."` + Interactive bool `short:"i" help:"Launch a browser to show the UI at the listen address."` + OpenBrowserAt string `help:"Launch a browser to show the UI at specific network address." placeholder:"HOST[:PORT]" hidden:""` + Theme string `help:"UI theme, 'light' or 'dark'." hidden:""` + Listen string `help:"Network address to listen on." placeholder:"HOST[:PORT]" default:"localhost:0"` + AccessLog bool `help:"Log all HTTP requests."` + TLSKeyFile string `help:"Path to TLS private key file for the UI server."` + TLSCertFile string `help:"Path to TLS certificate chain file for the UI server."` } type PublishArgs struct { diff --git a/internal/cli_types/base_test.go b/internal/cli_types/base_test.go index 6ca57f61e..a151ac469 100644 --- a/internal/cli_types/base_test.go +++ b/internal/cli_types/base_test.go @@ -7,7 +7,6 @@ import ( "github.com/rstudio/connect-client/internal/accounts" "github.com/rstudio/connect-client/internal/logging" - "github.com/rstudio/connect-client/internal/services" "github.com/rstudio/connect-client/internal/util/utiltest" "github.com/stretchr/testify/suite" ) @@ -22,13 +21,11 @@ func TestCLIContextSuite(t *testing.T) { func (s *CLIContextSuite) TestNewCLIContext() { accountList := &accounts.MockAccountList{} - token := services.LocalToken("abc123") fs := utiltest.NewMockFs() log := logging.New() - ctx := NewCLIContext(accountList, token, fs, log) + ctx := NewCLIContext(accountList, fs, log) s.Equal(accountList, ctx.Accounts) - s.Equal(token, ctx.LocalToken) s.Equal(log, ctx.Logger) accountList.AssertNotCalled(s.T(), "GetAllAccounts") } diff --git a/internal/services/api/http_service.go b/internal/services/api/http_service.go index 04aba51ea..81e71dd41 100644 --- a/internal/services/api/http_service.go +++ b/internal/services/api/http_service.go @@ -12,7 +12,6 @@ import ( "github.com/rstudio/connect-client/internal/logging" "github.com/rstudio/connect-client/internal/project" - "github.com/rstudio/connect-client/internal/services" "github.com/rstudio/connect-client/internal/services/middleware" "github.com/rstudio/connect-client/internal/state" @@ -28,8 +27,6 @@ type Service struct { certFile string openBrowser bool openBrowserAt string - skipAuth bool - token services.LocalToken addr net.Addr log logging.Logger } @@ -45,19 +42,9 @@ func NewService( certFile string, openBrowser bool, openBrowserAt string, - skipAuth bool, accessLog bool, - token services.LocalToken, log logging.Logger) *Service { - if project.DevelopmentBuild() && skipAuth { - log.Warn("Service is operating in DEVELOPMENT MODE with NO browser to server authentication") - } else { - handler = middleware.AuthRequired(log, handler) - handler = middleware.CookieSession(log, handler) - handler = middleware.LocalTokenSession(token, log, handler) - } - if accessLog { handler = middleware.LogRequest("Access Log", log, handler) } @@ -72,8 +59,6 @@ func NewService( certFile: certFile, openBrowser: openBrowser, openBrowserAt: openBrowserAt, - skipAuth: skipAuth, - token: token, addr: nil, log: log, } @@ -90,7 +75,7 @@ func (svc *Service) isTLS() (bool, error) { } } -func (svc *Service) getURL(includeToken bool) *url.URL { +func (svc *Service) getURL() *url.URL { scheme := "http" isTLS, _ := svc.isTLS() if isTLS { @@ -103,11 +88,6 @@ func (svc *Service) getURL(includeToken bool) *url.URL { Path: path, Fragment: fragment, } - if includeToken { - appURL.RawQuery = url.Values{ - "token": []string{string(svc.token)}, - }.Encode() - } return appURL } @@ -124,9 +104,7 @@ func (svc *Service) Run() error { } svc.addr = listener.Addr() - - // If not development mode, then you get a token added to the URL - appURL := svc.getURL(!(project.DevelopmentBuild() && svc.skipAuth)) + appURL := svc.getURL() svc.log.Info("UI server running", "url", appURL.String()) fmt.Println(appURL.String()) diff --git a/internal/services/local_token.go b/internal/services/local_token.go deleted file mode 100644 index 90ae6a0ea..000000000 --- a/internal/services/local_token.go +++ /dev/null @@ -1,17 +0,0 @@ -package services - -// Copyright (C) 2023 by Posit Software, PBC. - -import ( - "github.com/rstudio/connect-client/internal/util" -) - -type LocalToken string - -func NewLocalToken() (LocalToken, error) { - str, err := util.RandomString(32) - if err != nil { - return LocalToken(""), err - } - return LocalToken(str), nil -} diff --git a/internal/services/local_token_test.go b/internal/services/local_token_test.go deleted file mode 100644 index 557592570..000000000 --- a/internal/services/local_token_test.go +++ /dev/null @@ -1,26 +0,0 @@ -package services - -// Copyright (C) 2023 by Posit Software, PBC. - -import ( - "testing" - - "github.com/rstudio/connect-client/internal/util/utiltest" - "github.com/stretchr/testify/suite" -) - -type LocalTokenSuite struct { - utiltest.Suite -} - -func TestLocalTokenSuite(t *testing.T) { - suite.Run(t, new(LocalTokenSuite)) -} - -func (s *LocalTokenSuite) TestNewLocalToken() { - token1, err := NewLocalToken() - s.NoError(err) - token2, err := NewLocalToken() - s.NoError(err) - s.NotEqual(token1, token2) -} diff --git a/internal/services/middleware/auth_required.go b/internal/services/middleware/auth_required.go deleted file mode 100644 index d2955d851..000000000 --- a/internal/services/middleware/auth_required.go +++ /dev/null @@ -1,35 +0,0 @@ -package middleware - -// Copyright (C) 2023 by Posit Software, PBC. - -import ( - "context" - "net/http" - - "github.com/rstudio/connect-client/internal/logging" -) - -// AuthRequired verifies that the session has been -// marked as authenticated by one of the auth middlewares. -type authenticatedContextKeyType string - -const authenticatedContextKey authenticatedContextKeyType = "authenticated" - -func AuthRequired(log logging.Logger, next http.HandlerFunc) http.HandlerFunc { - return func(w http.ResponseWriter, req *http.Request) { - if !isRequestAuthenticated(req) { - w.WriteHeader(http.StatusUnauthorized) - } else { - next(w, req) - } - } -} - -func authenticatedRequest(req *http.Request) *http.Request { - ctx := context.WithValue(req.Context(), authenticatedContextKey, true) - return req.WithContext(ctx) -} - -func isRequestAuthenticated(req *http.Request) bool { - return req.Context().Value(authenticatedContextKey) == true -} diff --git a/internal/services/middleware/cookie_auth.go b/internal/services/middleware/cookie_auth.go deleted file mode 100644 index 3e2c2062e..000000000 --- a/internal/services/middleware/cookie_auth.go +++ /dev/null @@ -1,52 +0,0 @@ -package middleware - -// Copyright (C) 2023 by Posit Software, PBC. - -import ( - "net/http" - - "github.com/chmike/securecookie" - - "github.com/rstudio/connect-client/internal/logging" - "github.com/rstudio/connect-client/internal/util" -) - -const sessionCookieName string = "posit-publish-session" - -var cookieKey []byte = securecookie.MustGenerateRandomKey() - -var cookieObj = securecookie.MustNew(sessionCookieName, cookieKey, securecookie.Params{ - HTTPOnly: true, - Secure: false, // we currently only serve over http - SameSite: securecookie.Lax, -}) - -// CookieSession looks for a posit-publish-session cookie. -// If there is a cookie, and it is valid, -// the session is marked as authenticated. -// An invalid cookie results in auth failure (401). -func CookieSession(log logging.Logger, next http.HandlerFunc) http.HandlerFunc { - return func(w http.ResponseWriter, req *http.Request) { - _, err := cookieObj.GetValue([]byte(sessionCookieName), req) - if err != nil { - // Proceed without auth. - if err != http.ErrNoCookie { - log.Error("Error checking for cookie", "name", sessionCookieName, "error", err) - } - next(w, req) - } else { - // Any valid cookie (signed with our key) is accepted. - req = authenticatedRequest(req) - next(w, req) - } - } -} - -func setCookie(w http.ResponseWriter) error { - sessionID, err := util.RandomBytes(32) - if err != nil { - return err - } - cookieObj.SetValue(w, sessionID) - return nil -} diff --git a/internal/services/middleware/local_token_auth.go b/internal/services/middleware/local_token_auth.go deleted file mode 100644 index f0f96037c..000000000 --- a/internal/services/middleware/local_token_auth.go +++ /dev/null @@ -1,45 +0,0 @@ -package middleware - -// Copyright (C) 2023 by Posit Software, PBC. - -import ( - "net/http" - - "github.com/rstudio/connect-client/internal/logging" - "github.com/rstudio/connect-client/internal/services" -) - -// LocalToken looks for a `token` query parameter. If -// present, it matches it against the (single) token -// generated at startup and included in the URL we -// present to the user. If it matches, redirect to the -// original URL without the token parameter and with a valid -// session cookie. A mismatch is an auth failure (401). -const tokenParameterName string = "token" - -func LocalTokenSession(expectedToken services.LocalToken, log logging.Logger, next http.HandlerFunc) http.HandlerFunc { - return func(w http.ResponseWriter, req *http.Request) { - target := req.URL - receivedTokens, ok := target.Query()[tokenParameterName] - if ok { - receivedToken := receivedTokens[0] - if expectedToken == services.LocalToken(receivedToken) { - // Success - log.Info("Authenticated via token, creating session") - setCookie(w) - values := target.Query() - values.Del(tokenParameterName) - target.RawQuery = values.Encode() - w.Header().Add("Location", target.String()) - w.WriteHeader(http.StatusMovedPermanently) - } else { - // Failure - log.Error("Invalid authentication token", "token", receivedToken) - w.WriteHeader(http.StatusUnauthorized) - } - } else { - // No token in the request - next(w, req) - } - } -} diff --git a/internal/services/ui/ui_service.go b/internal/services/ui/ui_service.go index ea3a66753..2d49a2470 100644 --- a/internal/services/ui/ui_service.go +++ b/internal/services/ui/ui_service.go @@ -10,7 +10,6 @@ import ( "github.com/rstudio/connect-client/internal/cli_types" "github.com/rstudio/connect-client/internal/logging" "github.com/rstudio/connect-client/internal/publish" - "github.com/rstudio/connect-client/internal/services" "github.com/rstudio/connect-client/internal/services/api" "github.com/rstudio/connect-client/internal/services/api/deployments" "github.com/rstudio/connect-client/internal/services/api/files" @@ -29,7 +28,6 @@ func NewUIService( fragment string, ui cli_types.UIArgs, publish *cli_types.PublishArgs, - token services.LocalToken, fs afero.Fs, lister accounts.AccountList, log logging.Logger, @@ -46,9 +44,7 @@ func NewUIService( ui.TLSCertFile, ui.Interactive, ui.OpenBrowserAt, - ui.SkipBrowserSessionAuth, ui.AccessLog, - token, log, ) } diff --git a/test/cy/cypress/e2e/home.cy.js b/test/cy/cypress/e2e/home.cy.js index 31319dc43..d49b17fbd 100644 --- a/test/cy/cypress/e2e/home.cy.js +++ b/test/cy/cypress/e2e/home.cy.js @@ -2,7 +2,6 @@ describe('Landing', () => { beforeEach(() => { cy.visit({ url: '/', - qs: { token: Cypress.env('token') } }); }); it('.should() - assert that is correct', () => { @@ -13,8 +12,7 @@ describe('Landing', () => { describe('Check Files', () => { beforeEach(() => { cy.visit({ - url: '/', - qs: { token: Cypress.env('token') } + url: '/' }); }); it('files should be listed', () => { diff --git a/test/cy/package.json b/test/cy/package.json index 6e824baa5..8c1beaf47 100644 --- a/test/cy/package.json +++ b/test/cy/package.json @@ -3,10 +3,10 @@ "scripts": { "lint": "eslint --ext .js,.ts,.cjs .", "fix": "eslint --ext .js,.ts,.cjs . --fix", - "start": "just ../../run publish-ui --listen 127.0.0.1:9000 --token=token ./test/sample-content/fastapi-simple", - "test": "start-server-and-test --expect 401 start http-get://127.0.0.1:9000 run", + "start": "just ../../run publish-ui --listen 127.0.0.1:9000 ./test/sample-content/fastapi-simple", + "test": "start-server-and-test --expect 200 start http-get://127.0.0.1:9000 run", "open": "cypress open", - "run": "cypress run --env token=token" + "run": "cypress run" }, "devDependencies": { "cypress": "^13.3.0",