-
-
Notifications
You must be signed in to change notification settings - Fork 64
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
feat: browser extension, custom server support #1220
base: master
Are you sure you want to change the base?
Conversation
Fantastic - I'll take a look at this shortly, really appreciate the time and care you put into this! |
I like the fact that you're inferring here since it means less configuration for the user, but all self-hosted instances use |
I agree with you about the configuration issues bit. I do not officially support users changing the routing configuration supplied in the selfhost configs. Many other things will break if they do, and there's really no reason for them to change it. Imo it's completely fine for us to assume that the API address will always be |
); | ||
}); | ||
chrome.storage.local.set( | ||
{ token: null, api_url: null, base_url: null }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[non-blocking] Do we perhaps want to preserve their custom domain if they have an expired session? It might be a little frustrating if every time a session expires they have to re-type their custom domain
Ah, great points. I totally forgot that I configured a reverse proxy for my instance to utilize the api subdomain. I should have a chance to work on your comments in the next day or two. Thanks. |
Given that, I don't mind keeping the setting for overriding the API URL as it stands now tucked away in the settings page for users who go full-custom such as yourself. |
Hi, Congratulations!!! It's just what I need |
I actually wonder if your selfhosted instance is using that api subdomain. Did you change any of the frontend source files or is it by chance still using |
Hello, I think this is now in a good place to share! I've worked to implement custom server support into the manifest v3 webextension, per julianpoy/RecipeSage-selfhost#65. I experimented with a few ways to present this option visually to users and have worked to minimize its impact on the majority of users who will continue to use the primary
recipesage.com
instance. Totally open to suggestions on your further preferences for UI presentation. Seems to be working well on Firefox (desktop and mobile) and Chrome.Changes include:
recipesage.com
instance-- Note: the base URL is automatically inferred by stripping the
api
subdomain from the supplied API URL. That is, if the user suppliesapi.yourdomain.tld
the base URLyourdomain.tld
will be inferred. This works so long as the user's API is accessible via an API subdomain one level higher than the base URL. However, in situations where this is not the case, the Extension Settings page can be utilized to set a custom 'Base URL.'Considerations to note:
"script-src 'self'; upgrade-insecure-requests;"
, but theupgrade-insecure-requests
policy will prevent an http address to be reached. Additionally, they would need to remove the tiny form validationif (!server.startsWith("https://"))
inaction.js
that ensures that only https API addresses are supplied.