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

Teleport Connect should not replace AccessDenied error messages with generic "access denied" #32550

Closed
stevenGravy opened this issue Sep 26, 2023 · 2 comments · Fixed by #39558
Assignees
Labels
bug error-msg Improving customer facing error messages. teleport-connect Issues related to Teleport Connect.

Comments

@stevenGravy
Copy link
Contributor

Expected behavior:

Users would receive the same message in Web UI, tsh and Teleport Connect for login failures.

Current behavior:

Teleport Connect gives 7 PERMISSION_DENIED: access denied vs invalid credentials. This is potentially confusing to users that they're denied access vs they have invalid credentials.

Bug details:

  • Teleport version: 14.0.0
  • Recreation steps

Attempt a local login with invalid password in Teleport connect, Web UI and tsh.

@stevenGravy stevenGravy added bug error-msg Improving customer facing error messages. labels Sep 26, 2023
@ravicious ravicious added the teleport-connect Issues related to Teleport Connect. label Sep 26, 2023
@ravicious
Copy link
Member

ravicious commented Sep 29, 2023

This is because the gRPC middleware in the tsh daemon is hardcoded to always return just "access denied" on trace.IsAccessDenied.

// do not return a full error stack on access denied errors
if trace.IsAccessDenied(err) {
return resp, trail.ToGRPC(trace.AccessDenied("access denied"))

I'm not sure why this was done, I don't think any of our clients has similar behavior. Since error classes & gRPC status codes are not persisted by the time this error is processed by the UI, perhaps it was a workaround to be able to detect access denied errors in the UI code.

Fixing this would require addressing #30753 and returning a custom wrapper for errors so that we can inspect status codes. Otherwise we won't have a reliable way to detect access denied errors, which we do have today because of that hardcoded "access denied" string.

@ravicious ravicious changed the title Different error message in login error Teleport Connect vs WebUI/tsh Teleport Connect replaces custom messages for AccessDenied errors with "access denied" Jan 11, 2024
@ravicious ravicious changed the title Teleport Connect replaces custom messages for AccessDenied errors with "access denied" Teleport Connect should not replace AccessDenied error messages with generic "access denied" Jan 12, 2024
@ravicious
Copy link
Member

ravicious commented Jan 12, 2024

With the MFA support for kube access and administrative actions, when an incorrect TOTP is entered, the backend returns access denied with a message about TOTP being incorrect. But because Connect completely removes messages from access denied errors, the user will simply see "Access Denied" instead of a meaningful message.


Trying to approve an access request with second factor set to optional. #36664 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug error-msg Improving customer facing error messages. teleport-connect Issues related to Teleport Connect.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants