Skip to content

Prevent reauthentication for tilde prefix expansion errors#51464

Merged
rosstimothy merged 1 commit intomasterfrom
tross/sftp_expansion_errors
Jan 29, 2025
Merged

Prevent reauthentication for tilde prefix expansion errors#51464
rosstimothy merged 1 commit intomasterfrom
tross/sftp_expansion_errors

Conversation

@rosstimothy
Copy link
Copy Markdown
Contributor

@rosstimothy rosstimothy commented Jan 24, 2025

In #24254 expansion of tilde prefixes(i.e. tsh scp CHANGELOG.md root@host:~foo/) was explicitly denied. However, because the error returned to the user is a trace.BadParameter error the reauthentication logic kicks in and attempts to resolve the issue with fresh credentials. This error is now caught and wrapped in a NonRetryableError to prevent the authentication logic from providing a weird UX.

Related to #22886.

@rosstimothy rosstimothy added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v15 backport/branch/v17 labels Jan 24, 2025
@rosstimothy rosstimothy marked this pull request as ready for review January 24, 2025 16:58
@github-actions github-actions Bot requested review from Joerger and nklaassen January 24, 2025 16:59
@rosstimothy
Copy link
Copy Markdown
Contributor Author

Friendly ping @Joerger @nklaassen

Comment thread lib/sshutils/sftp/sftp.go Outdated
@rosstimothy rosstimothy force-pushed the tross/sftp_expansion_errors branch from 735e897 to f044617 Compare January 28, 2025 18:54
@rosstimothy rosstimothy requested a review from nklaassen January 29, 2025 18:17
Comment thread lib/sshutils/sftp/sftp_test.go Outdated
In #24254 expansion
of tilde prefixes(i.e. `tsh scp CHANGELOG.md root@host:~foo/`) was
explicitly denied. However, because the error returned to the user
is a trace.BadParameter error the reauthentication logic kicks in
and attempts to resolve the issue with fresh credentials. This
error is now caught and wrapped in a NonRetryableError to prevent
the authentication logic from providing a weird UX.

Updates #22886.
@rosstimothy rosstimothy force-pushed the tross/sftp_expansion_errors branch from 3ae7afa to 75c24b0 Compare January 29, 2025 21:18
@rosstimothy rosstimothy added this pull request to the merge queue Jan 29, 2025
Merged via the queue into master with commit b25252a Jan 29, 2025
@rosstimothy rosstimothy deleted the tross/sftp_expansion_errors branch January 29, 2025 23:07
@public-teleport-github-review-bot
Copy link
Copy Markdown

@rosstimothy See the table below for backport results.

Branch Result
branch/v15 Failed
branch/v16 Failed
branch/v17 Create PR

carloscastrojumo pushed a commit to carloscastrojumo/teleport that referenced this pull request Feb 19, 2025
…onal#51464)

In gravitational#24254 expansion
of tilde prefixes(i.e. `tsh scp CHANGELOG.md root@host:~foo/`) was
explicitly denied. However, because the error returned to the user
is a trace.BadParameter error the reauthentication logic kicks in
and attempts to resolve the issue with fresh credentials. This
error is now caught and wrapped in a NonRetryableError to prevent
the authentication logic from providing a weird UX.

Updates gravitational#22886.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/branch/v17 no-changelog Indicates that a PR does not require a changelog entry size/sm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants