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

Fix tsh login issue #5380

Merged
merged 2 commits into from
Jan 21, 2021
Merged

Fix tsh login issue #5380

merged 2 commits into from
Jan 21, 2021

Conversation

r0mant
Copy link
Collaborator

@r0mant r0mant commented Jan 21, 2021

If --proxy differs from the actual proxy public addr during tsh login, it fails with the following error:

$ tsh --proxy=localhost --user=alice --insecure login
WARNING: You are using insecure connection to SSH proxy https://localhost:3080
Enter password for Teleport user alice:
WARNING: You are using insecure connection to SSH proxy https://proxy.example.com:3080
error: not logged in

With the following stack trace:

ERROR REPORT:
Original Error: *trace.NotFoundError not logged in
Stack Trace:
	/home/go/src/github.com/gravitational/teleport/lib/client/api.go:578 github.com/gravitational/teleport/lib/client.StatusCurrent
	/home/go/src/github.com/gravitational/teleport/tool/tsh/db.go:138 main.fetchDatabaseCreds
	/home/go/src/github.com/gravitational/teleport/tool/tsh/tsh.go:703 main.onLogin
	/home/go/src/github.com/gravitational/teleport/tool/tsh/tsh.go:440 main.Run
	/home/go/src/github.com/gravitational/teleport/tool/tsh/tsh.go:221 main.main
	/usr/local/go/src/runtime/proc.go:204 runtime.main
	/usr/local/go/src/runtime/asm_amd64.s:1374 runtime.goexit
User Message: not logged in

This PR fixes the issue by making sure that fetching database creds uses proxy address that's been updated after login procedure, not the one originally specified on the CLI.

@r0mant r0mant added the database-access Database access related issues and PRs label Jan 21, 2021
@r0mant r0mant requested a review from russjones January 21, 2021 20:09
@r0mant r0mant self-assigned this Jan 21, 2021
Copy link
Contributor

@russjones russjones left a comment

Choose a reason for hiding this comment

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

Fix looks fine, but can you add test coverage before merging.

@r0mant
Copy link
Collaborator Author

r0mant commented Jan 21, 2021

@russjones Sounds good, I've added a test for the changes in fetchDatabaseCreds. It also looks like tsh login is not being tested at all right now. Ideally we'd have tests for it as well, though it being a CLI driven flow might make it a bit challenging to test and require some refactoring. I'll make a ticket so we address it in one of future PRs.

@r0mant r0mant merged commit 14be0c1 into master Jan 21, 2021
@r0mant r0mant deleted the roman/fixtsh branch January 21, 2021 22:56
pierrebeaucamp pushed a commit that referenced this pull request Jan 22, 2021
…e/dynamodb-audit-fix

* 'master' of github.com:gravitational/teleport:
  Refactor db proxy/engine for easier reuse (#5325)
  Refactor postgres service file handling, add db config command (#5319)
  Fix tsh login issue when --proxy differs from actual proxy public addr (#5380)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database-access Database access related issues and PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants