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

Permanent file system errors can result in hung connections #288

Closed
slifty opened this issue Oct 26, 2023 · 0 comments · Fixed by #305
Closed

Permanent file system errors can result in hung connections #288

slifty opened this issue Oct 26, 2023 · 0 comments · Fixed by #305
Assignees

Comments

@slifty
Copy link
Contributor

slifty commented Oct 26, 2023

As part of #192 we updated the retrieval of a permanent file system to become an async method. Part of that change involved the insertion of catches in case there is an issue accessing the file system. Unfortunately that caught error does not result in a returned SFTP status, meaning the connection will hang in the event that the error is thrown.

(I'm going to blame the github UX for this -- it hid all of those changes behind a "we don't show diffs of large changes" and so unfortunately I missed that entire file in my review).

This is a bug! It needs to be fixed!

The fix could come in the form of adding failure status responses to those handlers, but really the mistake is that a static auth token should not be passed to the permanent file system -- the token MANAGER should be passed to the file system, and the file system should be generating the token at the moment it is needed.

This would mean the filesystem lookup becomes static again.

@slifty slifty self-assigned this Oct 26, 2023
slifty added a commit that referenced this issue Nov 7, 2023
We had organically coupled token management with the initial
authentication flow, but they don't actually belong together.

This separates token management (e.g. utilization of refresh tokens)
from the SSH authentication system.  It also refactors the sftp session
handler to use the token manager rather than the authentication session.

Finally, the tokens are now retrieved just-in-time by the permanent file
system (rather than being passed during the creation of the permanent
file system).  This is a critical fix because (1) it prevents certain
paths that would lead to stale tokens but also (2) it means that
creating a permanent file system becomes a synchronous operation.  This
also resolves a bug where the failure to generate a token could result
in a hanging sftp connection.

While doing these refactors we took out a redundant environment
variable.

Issue #288 Permanent file system errors can result in hung connections
Issue #289 Duplicated FusionAuth env vars
slifty added a commit that referenced this issue Nov 7, 2023
Auth tokens are now retrieved just-in-time by the permanent file
system (rather than being passed during the creation of the permanent
file system).  This is a critical fix because (1) it prevents certain
paths that would lead to stale tokens but also (2) it means that
creating a permanent file system becomes a synchronous operation.  This
also resolves a bug where the failure to generate a token could result
in a hanging sftp connection.

Issue #288 Permanent file system errors can result in hung connections
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant