etesync / etesync-web

An EteSync web client
https://www.etesync.com
GNU Affero General Public License v3.0
245 stars 29 forks source link

Service Worker: Too broad. #207

Closed cheako closed 3 years ago

cheako commented 3 years ago

https://github.com/saierd/docker-etesync/issues/8#issuecomment-798908562 The SW is operating on /admin and I'd like it to stop. I'm using an old commit e3d6bd21e44339f158a4e62fe496d1f7afd99963 because that's what was current when the Dockerfile I'm using was created. Any tips on updating this to master are welcome... I.E. If you can say there are no, or list, installation breaking changes that would be great.

tasn commented 3 years ago

I have no idea what you are even asking. Are you talking about etesync v1 vs etesync v2?

cheako commented 3 years ago

Some things broke when I used master, they got better when using a commit from December. Is using the commit from back in December a problem?

tasn commented 3 years ago

Should be fine. Would you mind opening a ticket (or editing this one) to explain what broke?

cheako commented 3 years ago

I've upgraded to master and see no change now, it's just that I can't access /admin because the SW is intercepting.

tasn commented 3 years ago

I'm not sure why the SW is intercepting the calls, but either way, the problem seems to be that you are hosting everything on the same domain. Try using subdomains, or maybe using another docker image? As I haven't heard any such issues from other users.

cheako commented 3 years ago

Ideally it should be possible to host in a subfolder. I'll set up hosts for this, subdomains are exclusively used to delegate authority... If ppl use a subdomain when they're not delegating to another DNS server(on another IP) or even back to a server already visited, things break in subtle ways and it's important for everyone to watch out for this. If ppl want dot(s) in DNS hostnames, they should simply put dots in the name.

It's because the logic is exclusionary: Edit: service-worker.js:38

  blacklist: [/^\/_/,/\/[^/?]+\.[^/]+$/],

I'm not sure about the API, but perhaps you meant whitelist:...

tasn commented 3 years ago

Where is this line taken from? Anyhow, subdomains are also use for containing different applications, and the web client handles your encryption key, while the admin view shouldn't get access to it. Subdirectories should work, but aren't officially supported.

cheako commented 3 years ago

The term subdomain has an explicit meaning in the world of DNS servers and their configuration. If when you say subdomains but you really mean hostnames or specifically in this case I think you'd mean hostnames with dots in them(witch isn't required here, another name with or without dots workers all the same), that's confusing.

tasn commented 3 years ago

In the web's security contexts app.etesync.com, www.etesync.com and example.com all live in different security contexts. While example.com/admin and example.com/pim exist in the same security context. The latter example also shares cookies and local storage. This can cause a lot of annoying problems for JS based apps, which is why we separate them.

cheako commented 3 years ago

...instead of not using ambiguous names that might collide with other projects?

tasn commented 3 years ago

Not sure I follow

cheako commented 3 years ago

I still think this could be adjusted without any ill effects.

tasn commented 3 years ago

Open a PR and I'd happy to check it out!