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

Feature Request/Issue: URL path address #139

Open
lucianor opened this issue Jun 21, 2023 · 5 comments
Open

Feature Request/Issue: URL path address #139

lucianor opened this issue Jun 21, 2023 · 5 comments

Comments

@lucianor
Copy link

Hello

I'm running sshwifty under docker, which means I can access through the web with port 8182.
This mean I access the application by the web root / (https://server:8182/)
However, all assets, images, etc are located under the /sshwifty/ folder (https://server:8182/sshwifty/) by application design.

This means, if I want to reverse proxy sshwifty I need to proxy to port 8182 web root / as well as handle the /sshwifty/ folder on my webserver config.

Example with caddy:

handle_path /ssh/* {
   reverse_proxy server:8182
}
handle /sshwifty/* {
   reverse_proxy server:8182
}

The feature request is to eliminate the /sshwifty/ from the assets, etc, so they all can be served from the web root.

The other feature request is to have the ability to specify a folder address, so when I type server:8182 it will redirect to server:8182/sshwifty/ and server all files from there. This is a very common config for applications to support reverse proxy.

@nirui
Copy link
Owner

nirui commented Jun 22, 2023

Hi,

I appreciate your interest for pull request, but the design mentioned is actually intentional, see #10 (comment)

If the change in the PR helped you at your end, please keep it as a fork, but I can't really accept it. Sorry.

@lucianor
Copy link
Author

lucianor commented Jun 22, 2023

I read through your comment and understand the thought process, but I beg to differ.

From a sysadmin point of view, it's easy to just map a single folder for reverse proxy purposes. So in that case, a /ssh/ pointing to a web root would serve all files and websocket, without needing to map additional folders for assets etc.

I sent another PR with additional fixes after testing it locally. I also tested reverse proxy from my server with multiple options (caddy, lighttpd, traefik, nginx, apache) and all worked just fine. With this PR, issue #10 would work out of the box.

After this PR, the FAQ "Can I serve Sshwifty under a subpath such as https://my.domain/ssh?" the answer can be changed to "yes, if there is a reverse proxy", and use the examples provided on the PR #140

I hope this helps and looking forward for this being accepted. I love this tool very much! I'm available if you would like me to share the details of my testing setup for the PR.

@nirui
Copy link
Owner

nirui commented Jun 22, 2023

Hi, thank you for the effort, but that still wouldn't really work.

To summarize the change in the PR, it basically making the static assets to use relative paths. This is not really the best practice as it might create confusing when configuring. In addition to that, if you access the wrong path, say /ssh/anything-else, then suddenly the assets will stop loading because it's no longer the correct path for the server backend.

I also wanted to mention that, if you map Sshwifty to the intended path /sshwifty (rather than /ssh), then you can done that with just one map setting.

But more importantly, there are security concerns on why sharing the same HTTP hostname between Sshwifty and another application is not recommended in general (in short, Sshwifty uses localStorage to store it's data which can be accessed by all applications hosted under the same hostname). And it's the reason why hosting Sshwifty under a dedicated domain or HTTP hostname (that's hostname:port) is recommended.

Then again, if the change do work for you, it's the best that you maintain the change on your own specialized fork to resolve these quirks imposed by Sshwifty (sorry for the trouble). But unless the solution is generalized and optimal, we can't really push that to all users.

I'm sorry for the disappointment :(

@lucianor
Copy link
Author

Fair enough! No disappointment here, you put together a very good tool here. I will continue to maintain my fork.
I did have some problems with the private keys.. even after adding it to the config file, it still asks me for keys and details if I click on connect. I will open a new bug report or go through the code myself to check what's happening.

@CJendantix
Copy link
Contributor

This issue (and associated PR) has been stale for a long time, is it going to be closed?

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

No branches or pull requests

3 participants