ItzNotABug / ghosler

Send newsletter emails to your Ghost CMS subscribers & members, using your own email credentials!
Apache License 2.0
40 stars 5 forks source link

Enable Hosting Ghosler at a Subdirectory #56

Closed fritz-fritz closed 1 month ago

fritz-fritz commented 1 month ago

This pull request makes a few changes to support serving Ghosler behind a reverse proxy at a subdirectory instead of hosting it at a subdomain. The existing use case should still function identically.

It accomplishes this by implementing a middleware that provides the configs.ghosler.url to all views as a variable and modifying the views to prepend all paths with this value. The result is essentially dynamically generated absolute paths to resources.

It was also necessary to modify the post function of the settings form such that it did not overwrite the ghosler.url improperly. It will still update on submission but will now detect a subdirectory via the Referer header.

Note to use Ghosler at a subdirectory you do still need to manually configure the ghosler.url before initially starting, I didn't really have a good idea on how to get around that. But now it won't overwrite this setting with the wrong data.

For reference, a minimal nginx stub looks like this:

    location /ghosler/ {
        proxy_pass              http://127.0.0.1:2370;

        # Preserve the base path
        rewrite ^/ghosler/(.*)$ /$1 break;
        proxy_redirect          / /ghosler/;
    }

An alternative approach would have been to create a new configuration variable for the baseURI for the routes. I thought it was better to not introduce a new variable but this might be a better approach as it might be more clear how to achieve such a setup and would mean even less complicated reverse proxy rules.

fritz-fritz commented 1 month ago

This is in response to #53

ItzNotABug commented 1 month ago

Alternative, it would be great if you could also do these -

  1. Add await here for safety.
  2. Remove then() here and use await.
fritz-fritz commented 1 month ago

Alternative, it would be great if you could also do these -

  1. Add await here for safety.
  2. Remove then() here and use await.

I don't see what these directly do with this pull request but have no complaints towards implementing and including

ItzNotABug commented 1 month ago

I've manually tested the PR, seems to work fine. Pl. address the review comments and we should be good to go for this.

fritz-fritz commented 1 month ago

I think we should be good to merge if you want to look it over?

ItzNotABug commented 1 month ago

LGTM!