May 27, 2022 · May 26, 2022 · May 26, 2022 · May 26, 2022 · May 26, 2022 · May 26, 2022
diff --git a/cli/logout.go b/cli/logout.go
 import (
 "fmt"
 "os"
 "strings"

 "github.com/spf13/cobra"
 "golang.org/x/xerrors"
 Use:   "logout",
 Short: "Remove the local authenticated session",
 RunE: func(cmd *cobra.Command, args []string) error {
 var isLoggedOut bool
 client, err := createClient(cmd)
 if err != nil {
 return err
 }

 var errors []error

 config := createConfig(cmd)

 _, err:= cliui.Prompt(cmd, cliui.PromptOptions{
 _, err = cliui.Prompt(cmd, cliui.PromptOptions{
 Text:      "Are you sure you want to logout?",
 IsConfirm: true,
 Default:   "yes",
 return err
 }

 err =config.URL().Delete()
 err =client.Logout(cmd.Context())
 if err != nil {
 // Only throw error if the URL configuration file is present,
 // otherwise the user is already logged out, and we proceed
 if !os.IsNotExist(err) {
 return xerrors.Errorf("remove URL file: %w", err)
 }
 isLoggedOut = true
 errors = append(errors, xerrors.Errorf("logout api: %w", err))
 }

 err = config.URL().Delete()
 // Only throw error if the URL configuration file is present,
 // otherwise the user is already logged out, and we proceed
 if err != nil && !os.IsNotExist(err) {
 errors = append(errors, xerrors.Errorf("remove URL file: %w", err))
 }

 err = config.Session().Delete()
 if err != nil {
 // Only throw error if the session configuration file is present,
 // otherwise the user is already logged out, and we proceed
 if !os.IsNotExist(err) {
 return xerrors.Errorf("remove session file: %w", err)
 }
 isLoggedOut = true
 // Only throw error if the session configuration file is present,
 // otherwise the user is already logged out, and we proceed
 if err != nil && !os.IsNotExist(err) {
 errors = append(errors, xerrors.Errorf("remove session file: %w", err))
 }

 err = config.Organization().Delete()
 // If the organization configuration file is absent, we still proceed
 if err != nil && !os.IsNotExist(err) {
 returnxerrors.Errorf("remove organization file: %w", err)
 errors = append(errors,xerrors.Errorf("remove organization file: %w", err))
 }

 // If the user was already logged out, we show them a different message
 if isLoggedOut {
 _, _ = fmt.Fprintf(cmd.OutOrStdout(), notLoggedInMessage+"\n")
 } else {
 _, _ = fmt.Fprintf(cmd.OutOrStdout(), caret+"Successfully logged out.\n")
 if len(errors) > 0 {
 var errorStringBuilder strings.Builder
 for _, err := range errors {
 _, _ = fmt.Fprint(&errorStringBuilder, "\t"+err.Error()+"\n")
 }
 errorString := strings.TrimRight(errorStringBuilder.String(), "\n")
 return xerrors.New("Failed to log out.\n" + errorString)
 }
 _, _ = fmt.Fprintf(cmd.OutOrStdout(), caret+"You are no longer logged in. You can log in using 'coder login <url>'.\n")
 return nil
 },
 }
diff --git a/cli/logout_test.go b/cli/logout_test.go
 package cli_test

 import (
 "fmt"
 "os"
 "regexp"
 "runtime"
 "testing"

 "github.com/stretchr/testify/assert"
 pty := ptytest.New(t)
 config := login(t, pty)

 //ensure session files exist
 //Ensure session files exist.
 require.FileExists(t, string(config.URL()))
 require.FileExists(t, string(config.Session()))


 pty.ExpectMatch("Are you sure you want to logout?")
 pty.WriteLine("yes")
 pty.ExpectMatch("Successfullyloggedout")
 pty.ExpectMatch("You are no longerloggedin. You can log in using 'coder login <url>'.")
 <-logoutChan
 })
 t.Run("SkipPrompt", func(t *testing.T) {
 pty := ptytest.New(t)
 config := login(t, pty)

 //ensure session files exist
 //Ensure session files exist.
 require.FileExists(t, string(config.URL()))
 require.FileExists(t, string(config.Session()))

 assert.NoFileExists(t, string(config.Session()))
 }()

 pty.ExpectMatch("Successfullyloggedout")
 pty.ExpectMatch("You are no longerloggedin. You can log in using 'coder login <url>'.")
 <-logoutChan
 })
 t.Run("NoURLFile", func(t *testing.T) {
 pty := ptytest.New(t)
 config := login(t, pty)

 //ensure session files exist
 //Ensure session files exist.
 require.FileExists(t, string(config.URL()))
 require.FileExists(t, string(config.Session()))

 go func() {
 defer close(logoutChan)
 err := logout.Execute()
 assert.NoError(t, err)
 assert.NoFileExists(t, string(config.URL()))
 assert.NoFileExists(t, string(config.Session()))
 assert.EqualError(t, err, "You are not logged in. Try logging in using 'coder login <url>'.")
 }()

 pty.ExpectMatch("Are you sure you want to logout?")
 pty.WriteLine("yes")
 pty.ExpectMatch("You are not logged in. Try logging in using 'coder login <url>'.")
 <-logoutChan
 })
 t.Run("NoSessionFile", func(t *testing.T) {
 pty := ptytest.New(t)
 config := login(t, pty)

 //ensure session files exist
 //Ensure session files exist.
 require.FileExists(t, string(config.URL()))
 require.FileExists(t, string(config.Session()))

 go func() {
 defer close(logoutChan)
 err = logout.Execute()
 assert.NoError(t, err)
 assert.NoFileExists(t, string(config.URL()))
 assert.NoFileExists(t, string(config.Session()))
 assert.EqualError(t, err, "You are not logged in. Try logging in using 'coder login <url>'.")
 }()

 <-logoutChan
 })
 t.Run("CannotDeleteFiles", func(t *testing.T) {
 t.Parallel()

 pty := ptytest.New(t)
 config := login(t, pty)

 // Ensure session files exist.
 require.FileExists(t, string(config.URL()))
 require.FileExists(t, string(config.Session()))

 var (
 err         error
 urlFile     *os.File
 sessionFile *os.File
 )
 if runtime.GOOS == "windows" {
 // Opening the files so Windows does not allow deleting them.
 urlFile, err = os.Open(string(config.URL()))
 require.NoError(t, err)
 sessionFile, err = os.Open(string(config.Session()))
 require.NoError(t, err)
 } else {
 // Changing the permissions to throw error during deletion.
 err = os.Chmod(string(config), 0500)
 require.NoError(t, err)
 }
 t.Cleanup(func() {
 if runtime.GOOS == "windows" {
 // Closing the opened files for cleanup.
 err = urlFile.Close()
 require.NoError(t, err)
 err = sessionFile.Close()
 require.NoError(t, err)
 } else {
 // Setting the permissions back for cleanup.
 err = os.Chmod(string(config), 0700)
 require.NoError(t, err)
 }
 })

 logoutChan := make(chan struct{})
 logout, _ := clitest.New(t, "logout", "--global-config", string(config))

 logout.SetIn(pty.Input())
 logout.SetOut(pty.Output())

 go func() {
 defer close(logoutChan)
 err := logout.Execute()
 assert.NotNil(t, err)
 var errorMessage string
 if runtime.GOOS == "windows" {
 errorMessage = "The process cannot access the file because it is being used by another process."
 } else {
 errorMessage = "permission denied"
 }
 errRegex := regexp.MustCompile(fmt.Sprintf("Failed to log out.\n\tremove URL file: .+: %s\n\tremove session file: .+: %s", errorMessage, errorMessage))
 assert.Regexp(t, errRegex, err.Error())
 }()

 pty.ExpectMatch("Are you sure you want to logout?")
 pty.WriteLine("yes")
 pty.ExpectMatch("You are not logged in. Try logging in using 'coder login <url>'.")
 <-logoutChan
 })
 }
diff --git a/coderd/coderd.go b/coderd/coderd.go
 r.Get("/first", api.firstUser)
 r.Post("/first", api.postFirstUser)
 r.Post("/login", api.postLogin)
 r.Post("/logout", api.postLogout)
 r.Get("/authmethods", api.userAuthMethods)
 r.Route("/oauth2", func(r chi.Router) {
 r.Route("/github", func(r chi.Router) {
 )
 r.Post("/", api.postUser)
 r.Get("/", api.users)
 r.Post("/logout", api.postLogout)
 // These routes query information about site wide roles.
 r.Route("/roles", func(r chi.Router) {
 r.Get("/", api.assignableSiteRoles)
diff --git a/coderd/coderd_test.go b/coderd/coderd_test.go
 workspaceRBACObj := rbac.ResourceWorkspace.InOrg(organization.ID).WithID(workspace.ID.String()).WithOwner(workspace.OwnerID.String())

 // skipRoutes allows skipping routes from being checked.
 skipRoutes := map[string]string{
 "POST:/api/v2/users/logout": "Logging out deletes the API Key for other routes",
 }

 type routeCheck struct {
 NoAuthorize  bool
 AssertAction rbac.Action
 "GET:/api/v2/users/first":       {NoAuthorize: true},
 "POST:/api/v2/users/first":      {NoAuthorize: true},
 "POST:/api/v2/users/login":      {NoAuthorize: true},
 "POST:/api/v2/users/logout":     {NoAuthorize: true},
 "GET:/api/v2/users/authmethods": {NoAuthorize: true},
 "POST:/api/v2/csp/reports":      {NoAuthorize: true},

 assertRoute[noTrailSlash] = v
 }

 for k, v := range skipRoutes {
 noTrailSlash := strings.TrimRight(k, "/")
 if _, ok := skipRoutes[noTrailSlash]; ok && noTrailSlash != k {
 t.Errorf("route %q & %q is declared twice", noTrailSlash, k)
 t.FailNow()
 }
 skipRoutes[noTrailSlash] = v
 }

 err = chi.Walk(api.Handler, func(method string, route string, handler http.Handler, middlewares ...func(http.Handler) http.Handler) error {
 name := method + ":" + route
 if _, ok := skipRoutes[strings.TrimRight(name, "/")]; ok {
 return nil
 }
 t.Run(name, func(t *testing.T) {
 authorizer.reset()
 routeAssertions, ok := assertRoute[strings.TrimRight(name, "/")]
diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go
 return database.APIKey{}, sql.ErrNoRows
 }

 func (q *fakeQuerier) DeleteAPIKeyByID(_ context.Context, id string) error {
 q.mutex.Lock()
 defer q.mutex.Unlock()

 for index, apiKey := range q.apiKeys {
 if apiKey.ID != id {
 continue
 }
 q.apiKeys[index] = q.apiKeys[len(q.apiKeys)-1]
 q.apiKeys = q.apiKeys[:len(q.apiKeys)-1]
 return nil
 }
 return sql.ErrNoRows
 }

 func (q *fakeQuerier) GetFileByHash(_ context.Context, hash string) (database.File, error) {
 q.mutex.RLock()
 defer q.mutex.RUnlock()
diff --git a/coderd/database/querier.go b/coderd/database/querier.go
diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/apikeys.sql b/coderd/database/queries/apikeys.sql
 oauth_expiry = $6
 WHERE
 id = $1;

 -- name: DeleteAPIKeyByID :exec
 DELETE
 FROM
 api_keys
 WHERE
 id = $1;
Original file line number	Diff line number	Diff line change
Expand Up		@@ -3,6 +3,7 @@ package cli
		import (
		"fmt"
		"os"
		"strings"

		"github.com/spf13/cobra"
		"golang.org/x/xerrors"
Expand All		@@ -15,11 +16,16 @@ func logout() *cobra.Command {
		Use: "logout",
		Short: "Remove the local authenticated session",
		RunE: func(cmd *cobra.Command, args []string) error {
		var isLoggedOut bool
		client, err := createClient(cmd)
		if err != nil {
		return err
		}

		var errors []error

		config := createConfig(cmd)

		_, err:= cliui.Prompt(cmd, cliui.PromptOptions{
		_, err = cliui.Prompt(cmd, cliui.PromptOptions{
		Text: "Are you sure you want to logout?",
		IsConfirm: true,
		Default: "yes",
Expand All		@@ -28,38 +34,40 @@ func logout() *cobra.Command {
		return err
		}

		err =config.URL().Delete()
		err =client.Logout(cmd.Context())
		if err != nil {
		// Only throw error if the URL configuration file is present,
		// otherwise the user is already logged out, and we proceed
		if !os.IsNotExist(err) {
		return xerrors.Errorf("remove URL file: %w", err)
		}
		isLoggedOut = true
		errors = append(errors, xerrors.Errorf("logout api: %w", err))
		}

		err = config.URL().Delete()
		// Only throw error if the URL configuration file is present,
		// otherwise the user is already logged out, and we proceed
		if err != nil && !os.IsNotExist(err) {
		errors = append(errors, xerrors.Errorf("remove URL file: %w", err))
		}

		err = config.Session().Delete()
		if err != nil {
		// Only throw error if the session configuration file is present,
		// otherwise the user is already logged out, and we proceed
		if !os.IsNotExist(err) {
		return xerrors.Errorf("remove session file: %w", err)
		}
		isLoggedOut = true
		// Only throw error if the session configuration file is present,
		// otherwise the user is already logged out, and we proceed
		if err != nil && !os.IsNotExist(err) {
		errors = append(errors, xerrors.Errorf("remove session file: %w", err))
		}

		err = config.Organization().Delete()
		// If the organization configuration file is absent, we still proceed
		if err != nil && !os.IsNotExist(err) {
		returnxerrors.Errorf("remove organization file: %w", err)
		errors = append(errors,xerrors.Errorf("remove organization file: %w", err))
		}

		// If the user was already logged out, we show them a different message
		if isLoggedOut {
		_, _ = fmt.Fprintf(cmd.OutOrStdout(), notLoggedInMessage+"\n")
		} else {
		_, _ = fmt.Fprintf(cmd.OutOrStdout(), caret+"Successfully logged out.\n")
		if len(errors) > 0 {
		var errorStringBuilder strings.Builder
		for _, err := range errors {
		_, _ = fmt.Fprint(&errorStringBuilder, "\t"+err.Error()+"\n")
		}
		errorString := strings.TrimRight(errorStringBuilder.String(), "\n")
		return xerrors.New("Failed to log out.\n" + errorString)
		}
		_, _ = fmt.Fprintf(cmd.OutOrStdout(), caret+"You are no longer logged in. You can log in using 'coder login <url>'.\n")
		return nil
		},
		}
Expand Down
Original file line number	Diff line number	Diff line change
		@@ -1,7 +1,10 @@
		package cli_test

		import (
		"fmt"
		"os"
		"regexp"
		"runtime"
		"testing"

		"github.com/stretchr/testify/assert"
Expand All		@@ -21,7 +24,7 @@ func TestLogout(t *testing.T) {
		pty := ptytest.New(t)
		config := login(t, pty)

		//ensure session files exist
		//Ensure session files exist.
		require.FileExists(t, string(config.URL()))
		require.FileExists(t, string(config.Session()))

Expand All		@@ -40,7 +43,7 @@ func TestLogout(t *testing.T) {

		pty.ExpectMatch("Are you sure you want to logout?")
		pty.WriteLine("yes")
		pty.ExpectMatch("Successfullyloggedout")
		pty.ExpectMatch("You are no longerloggedin. You can log in using 'coder login <url>'.")
		<-logoutChan
		})
		t.Run("SkipPrompt", func(t *testing.T) {
Expand All		@@ -49,7 +52,7 @@ func TestLogout(t *testing.T) {
		pty := ptytest.New(t)
		config := login(t, pty)

		//ensure session files exist
		//Ensure session files exist.
		require.FileExists(t, string(config.URL()))
		require.FileExists(t, string(config.Session()))

Expand All		@@ -66,7 +69,7 @@ func TestLogout(t *testing.T) {
		assert.NoFileExists(t, string(config.Session()))
		}()

		pty.ExpectMatch("Successfullyloggedout")
		pty.ExpectMatch("You are no longerloggedin. You can log in using 'coder login <url>'.")
		<-logoutChan
		})
		t.Run("NoURLFile", func(t *testing.T) {
Expand All		@@ -75,7 +78,7 @@ func TestLogout(t *testing.T) {
		pty := ptytest.New(t)
		config := login(t, pty)

		//ensure session files exist
		//Ensure session files exist.
		require.FileExists(t, string(config.URL()))
		require.FileExists(t, string(config.Session()))

Expand All		@@ -91,14 +94,9 @@ func TestLogout(t *testing.T) {
		go func() {
		defer close(logoutChan)
		err := logout.Execute()
		assert.NoError(t, err)
		assert.NoFileExists(t, string(config.URL()))
		assert.NoFileExists(t, string(config.Session()))
		assert.EqualError(t, err, "You are not logged in. Try logging in using 'coder login <url>'.")
		}()

		pty.ExpectMatch("Are you sure you want to logout?")
		pty.WriteLine("yes")
		pty.ExpectMatch("You are not logged in. Try logging in using 'coder login <url>'.")
		<-logoutChan
		})
		t.Run("NoSessionFile", func(t *testing.T) {
Expand All		@@ -107,7 +105,7 @@ func TestLogout(t *testing.T) {
		pty := ptytest.New(t)
		config := login(t, pty)

		//ensure session files exist
		//Ensure session files exist.
		require.FileExists(t, string(config.URL()))
		require.FileExists(t, string(config.Session()))

Expand All		@@ -123,14 +121,73 @@ func TestLogout(t *testing.T) {
		go func() {
		defer close(logoutChan)
		err = logout.Execute()
		assert.NoError(t, err)
		assert.NoFileExists(t, string(config.URL()))
		assert.NoFileExists(t, string(config.Session()))
		assert.EqualError(t, err, "You are not logged in. Try logging in using 'coder login <url>'.")
		}()

		<-logoutChan
		})
		t.Run("CannotDeleteFiles", func(t *testing.T) {
		t.Parallel()

		pty := ptytest.New(t)
		config := login(t, pty)

		// Ensure session files exist.
		require.FileExists(t, string(config.URL()))
		require.FileExists(t, string(config.Session()))

		var (
		err error
		urlFile *os.File
		sessionFile *os.File
		)
		if runtime.GOOS == "windows" {
		// Opening the files so Windows does not allow deleting them.
		urlFile, err = os.Open(string(config.URL()))
		require.NoError(t, err)
		sessionFile, err = os.Open(string(config.Session()))
		require.NoError(t, err)
		} else {
		// Changing the permissions to throw error during deletion.
		err = os.Chmod(string(config), 0500)
		require.NoError(t, err)
		}
		t.Cleanup(func() {
		if runtime.GOOS == "windows" {
		// Closing the opened files for cleanup.
		err = urlFile.Close()
		require.NoError(t, err)
		err = sessionFile.Close()
		require.NoError(t, err)
AbhineetJain marked this conversation as resolved. Show resolvedHide resolved
		} else {
		// Setting the permissions back for cleanup.
		err = os.Chmod(string(config), 0700)
		require.NoError(t, err)
		}
		})

		logoutChan := make(chan struct{})
		logout, _ := clitest.New(t, "logout", "--global-config", string(config))

		logout.SetIn(pty.Input())
		logout.SetOut(pty.Output())

		go func() {
		defer close(logoutChan)
		err := logout.Execute()
		assert.NotNil(t, err)
		var errorMessage string
		if runtime.GOOS == "windows" {
		errorMessage = "The process cannot access the file because it is being used by another process."
		} else {
		errorMessage = "permission denied"
		}
		errRegex := regexp.MustCompile(fmt.Sprintf("Failed to log out.\n\tremove URL file: .+: %s\n\tremove session file: .+: %s", errorMessage, errorMessage))
		assert.Regexp(t, errRegex, err.Error())
		}()

		pty.ExpectMatch("Are you sure you want to logout?")
		pty.WriteLine("yes")
		pty.ExpectMatch("You are not logged in. Try logging in using 'coder login <url>'.")
		<-logoutChan
		})
		}
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -219,7 +219,6 @@ func New(options Options) API {
		r.Get("/first", api.firstUser)
		r.Post("/first", api.postFirstUser)
		r.Post("/login", api.postLogin)
		r.Post("/logout", api.postLogout)
		r.Get("/authmethods", api.userAuthMethods)
		r.Route("/oauth2", func(r chi.Router) {
		r.Route("/github", func(r chi.Router) {
Expand All		@@ -234,6 +233,7 @@ func New(options Options) API {
		)
		r.Post("/", api.postUser)
		r.Get("/", api.users)
		r.Post("/logout", api.postLogout)
		// These routes query information about site wide roles.
		r.Route("/roles", func(r chi.Router) {
		r.Get("/", api.assignableSiteRoles)
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -104,6 +104,10 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
		workspaceRBACObj := rbac.ResourceWorkspace.InOrg(organization.ID).WithID(workspace.ID.String()).WithOwner(workspace.OwnerID.String())

		// skipRoutes allows skipping routes from being checked.
		skipRoutes := map[string]string{
		"POST:/api/v2/users/logout": "Logging out deletes the API Key for other routes",
		}

AbhineetJain marked this conversation as resolved. Show resolvedHide resolved
		type routeCheck struct {
		NoAuthorize bool
		AssertAction rbac.Action
Expand All		@@ -117,7 +121,6 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
		"GET:/api/v2/users/first": {NoAuthorize: true},
		"POST:/api/v2/users/first": {NoAuthorize: true},
		"POST:/api/v2/users/login": {NoAuthorize: true},
		"POST:/api/v2/users/logout": {NoAuthorize: true},
		"GET:/api/v2/users/authmethods": {NoAuthorize: true},
		"POST:/api/v2/csp/reports": {NoAuthorize: true},

Expand DownExpand Up		@@ -310,8 +313,20 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
		assertRoute[noTrailSlash] = v
		}

		for k, v := range skipRoutes {
		noTrailSlash := strings.TrimRight(k, "/")
		if _, ok := skipRoutes[noTrailSlash]; ok && noTrailSlash != k {
		t.Errorf("route %q & %q is declared twice", noTrailSlash, k)
		t.FailNow()
		}
		skipRoutes[noTrailSlash] = v
		}

		err = chi.Walk(api.Handler, func(method string, route string, handler http.Handler, middlewares ...func(http.Handler) http.Handler) error {
		name := method + ":" + route
		if _, ok := skipRoutes[strings.TrimRight(name, "/")]; ok {
		return nil
		}
		t.Run(name, func(t *testing.T) {
		authorizer.reset()
		routeAssertions, ok := assertRoute[strings.TrimRight(name, "/")]
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -126,6 +126,21 @@ func (q *fakeQuerier) GetAPIKeyByID(_ context.Context, id string) (database.APIK
		return database.APIKey{}, sql.ErrNoRows
		}

		func (q *fakeQuerier) DeleteAPIKeyByID(_ context.Context, id string) error {
		q.mutex.Lock()
		defer q.mutex.Unlock()

		for index, apiKey := range q.apiKeys {
		if apiKey.ID != id {
		continue
		}
		q.apiKeys[index] = q.apiKeys[len(q.apiKeys)-1]
		q.apiKeys = q.apiKeys[:len(q.apiKeys)-1]
		return nil
		}
AbhineetJain marked this conversation as resolved. Show resolvedHide resolved
		return sql.ErrNoRows
		}

		func (q *fakeQuerier) GetFileByHash(_ context.Context, hash string) (database.File, error) {
		q.mutex.RLock()
		defer q.mutex.RUnlock()
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -38,3 +38,10 @@ SET
		oauth_expiry = $6
		WHERE
		id = $1;

		-- name: DeleteAPIKeyByID :exec
		DELETE
		FROM
		api_keys
		WHERE
		id = $1;