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

Public calendar iframe has issues on some browsers #169

Open
tcitworld opened this issue Nov 7, 2016 · 42 comments
Open

Public calendar iframe has issues on some browsers #169

tcitworld opened this issue Nov 7, 2016 · 42 comments
Assignees
Milestone

Comments

@tcitworld
Copy link
Member

@tcitworld tcitworld commented Nov 7, 2016

OS Linux Windows
Firefox Works Works
Chrome 54 Displays the grid, not the events. When refreshed, redirect issue. Displays the grid, not the events. When refreshed, redirect issue.
Chromium 53 Works Not tested
Chromium 54 Not tested Displays the grid, not the events. When refreshed, redirect issue.
Edge Not available Works
IE 12 Not available Displays the grid, not the events
@tcitworld tcitworld added the bug label Nov 7, 2016
@tcitworld tcitworld self-assigned this Nov 7, 2016
@tcitworld

This comment has been minimized.

Copy link
Member Author

@tcitworld tcitworld commented Nov 7, 2016

@georgehrke Can you test on Safari ?

@tcitworld

This comment has been minimized.

Copy link
Member Author

@tcitworld tcitworld commented Nov 7, 2016

The Chrome error is ERR_TOO_MANY_REDIRECTS

@georgehrke

This comment has been minimized.

Copy link
Member

@georgehrke georgehrke commented Dec 6, 2016

@tcitworld Do you have some example code for me?
Did you just include the iframe into an otherwise blank html file?

@georgehrke

This comment has been minimized.

Copy link
Member

@georgehrke georgehrke commented Dec 17, 2016

ping @tcitworld ^ :)

@tcitworld

This comment has been minimized.

Copy link
Member Author

@tcitworld tcitworld commented Dec 17, 2016

For instance this one works with Firefox and Chromium but not Chrome on Linux : http://www.berrylab36.org/Ouverture

Here it's embeed within SPIP but I've heard of the same issue with a Wordpress. Will try myself.

@georgehrke

This comment has been minimized.

Copy link
Member

@georgehrke georgehrke commented Dec 21, 2016

Safari 10 on macOS works just fine for me.

@georgehrke

This comment has been minimized.

Copy link
Member

@georgehrke georgehrke commented Dec 21, 2016

planning d ouverture - berrylab36 chromium today at 8 45 44 am

planning d ouverture - berrylab36 chromium today at 8 56 09 am

planning d ouverture - berrylab36 chromium today at 8 47 24 am

I see a few issues here.
To my knowledge there is no X-FRAME-OPTIONS: ALLOW. There is only DENY, SAMEORIGIN and ALLOW-FROM https://...

Further Chrom(e|ium) doesn't support X-Frame-Options.
It expects something like Content-Security-Policy: frame-ancestors my-trusty-site.com
Source: https://www.owasp.org/index.php/Content_Security_Policy_Cheat_Sheet#Preventing_Clickjacking

Also Chrom(e|ium) got more strict about https lately.
Maybe it's related to the fact, the berrylab36.org is http, but framagenda.org is https. (at least when sending requests like PROPFIND or so)

Maybe @LukasReschke has more insights here

@georgehrke

This comment has been minimized.

Copy link
Member

@georgehrke georgehrke commented Dec 24, 2016

IE doesn't work for the same reason the calendar app itself doesn't work in Internet explorer.

Should we display some overlay asking the user to use a different webbrowser?

@tcitworld

This comment has been minimized.

Copy link
Member Author

@tcitworld tcitworld commented Dec 27, 2016

I guess this needs some tweaking.

@tcitworld

This comment has been minimized.

Copy link
Member Author

@tcitworld tcitworld commented Dec 27, 2016

Will see how nextcloud/server#1472 handles things.

@enoch85

This comment has been minimized.

Copy link
Member

@enoch85 enoch85 commented Mar 12, 2017

I can confirm this. Got redirected to many times Chrome version 56.0.2924.87 (64-bit)

@enoch85

This comment has been minimized.

Copy link
Member

@enoch85 enoch85 commented Mar 12, 2017

Maybe my Ǹextcloud is to secure?

Firefox

Load denied by X-Frame-Options: https://cloud.techandme.se/apps/calendar/public/G1ZXEIKC3AGL8X4S does not permit cross-origin framing.
@georgehrke

This comment has been minimized.

Copy link
Member

@georgehrke georgehrke commented Mar 19, 2017

Maybe @LukasReschke has more insights here

@LukasReschke ping :)

@georgehrke

This comment has been minimized.

Copy link
Member

@georgehrke georgehrke commented Mar 19, 2017

IE doesn't work for the same reason the calendar app itself doesn't work in Internet explorer.
Should we display some overlay asking the user to use a different webbrowser?

At least IE 11 should be fixed by now

@flips

This comment has been minimized.

Copy link

@flips flips commented Mar 21, 2017

Seeing the same " redirected you too many times" in both Opera 43.0.2442.1165 and Chrome 57.0.2987.110. In Firefox 52.0.1 only seeing blank iframe. (macOS 10.12, all 64 bit browsers)

@georgehrke

This comment has been minimized.

Copy link
Member

@georgehrke georgehrke commented Mar 21, 2017

We don't support Opera. Will definitely look into the Chrome and Firefox issue

@flips

This comment has been minimized.

Copy link

@flips flips commented Mar 21, 2017

When I used http://www.tinywebgallery.com/blog/advanced-iframe/free-iframe-checker
it also complained about the X-FRAME-OPTIONS ... (As mentioned previously here)
Placing my test.html file on the same domain as my NextCloud installation, the iFrame started working in Firefox, but not in Safari (10.0.3) or Chrome (where they are now both blank, I don't get the framing or anything, and not the same too many redirects error).

@georgehrke

This comment has been minimized.

Copy link
Member

@georgehrke georgehrke commented Apr 2, 2017

This requires changes to the Nextcloud server, to be released with Nextcloud 12.
Rescheduling this ticket to Calendar 1.7.0

@law

This comment has been minimized.

Copy link

@law law commented Aug 1, 2017

Still seeing this issue on Nextcloud 12 and Chrome. tl; dr - firefox and safari work, Chrome throws a redirect error.

@Ich5003

This comment has been minimized.

Copy link

@Ich5003 Ich5003 commented Jan 28, 2018

Error still exists.
I'd really love to add the iframe to my webpage, but sadly it still doesn't work.
Any fixes or workarounds?

@georgehrke

This comment has been minimized.

Copy link
Member

@georgehrke georgehrke commented Jan 29, 2018

@Ich5003 What Browser, Nextcloud version, calendar app version?

@Ich5003

This comment has been minimized.

Copy link

@Ich5003 Ich5003 commented Jan 29, 2018

@georgehrke
Calendar app version is 1.5.7
Nextcloud Version is 12.0.5

Google Chrome (newest version) shows "too many redirects" error in iframe (mobile and Desktop)
Microsoft Edge (newest version) shows the correct IFrame.
//EDIT: Firefox (newest version) shows correct IFrame.
Opening the embedded link in chrome directly works aswell.

Do you need some more information?

@PeteKirkham

This comment has been minimized.

Copy link

@PeteKirkham PeteKirkham commented Jan 30, 2018

I'm having the problem too.
framagenda embedded in yeswiki displays correctly on Firefox 58.0 but not on Chrome 63.0
Nextcloud version 12. (where do I find the subversion?)

@Ich5003

This comment has been minimized.

Copy link

@Ich5003 Ich5003 commented Jan 30, 2018

This is my network log (excluding fonts) in Firefox
Can't see why chrome throws the redirect error.
log

@PeteKirkham

This comment has been minimized.

Copy link

@PeteKirkham PeteKirkham commented Jan 30, 2018

@Ich5003

This comment has been minimized.

Copy link

@Ich5003 Ich5003 commented Jan 30, 2018

I just tested again with Edge.
It's interesting that in the networking log of Edge the login page is called multiple times in a row.
This may be the reason for "too many redirects".

I guess the problem is something like this:
Any part of the page (maybe not even visible in foreground of the iframe) is responsible for an endless redirect loop.
While Chrome shows an error message on this, Edge and Firefox simply stop redirecting those background tasks and show the visible part.
I'll try to investigate this even further.

@Ich5003

This comment has been minimized.

Copy link

@Ich5003 Ich5003 commented Jan 30, 2018

And I found something even more interesting.
Let's say my nextcloud instance runs on aaa.example.com

Embedding the iframe on bbb.example.com (same server as aaa.example.com): Works a expected in all browsers
Embedding the iframe on example.com (different server): Works as expected
Embedding the iframe on someotherdomain.com: Does not work in Chrome.

So it mus be something domain specific.

@georgehrke

This comment has been minimized.

Copy link
Member

@georgehrke georgehrke commented Jan 30, 2018

It seems to be related to X-Frame-Options in that case 🙈

@georgehrke

This comment has been minimized.

Copy link
Member

@georgehrke georgehrke commented Jan 30, 2018

@tcitworld Does nextcloud/server#5462 help here?

It only does if the admin whitelists all the sites that may include the iframe, right?

Edit: And just setting it to * doesnt work afaik

@Ich5003

This comment has been minimized.

Copy link

@Ich5003 Ich5003 commented Jan 30, 2018

Hey @georgehrke
You're right, this may be possible.
I tried using jsloader and allow the other domain, but it still does not work.
Maybe I'll find another way to edit those X-Frame-Options.

@Ich5003

This comment has been minimized.

Copy link

@Ich5003 Ich5003 commented Jan 30, 2018

Filtered x-frame-options from header in my apache conf.
Still doesn't work. Really confusing.

@tcitworld

This comment has been minimized.

Copy link
Member Author

@tcitworld tcitworld commented Aug 7, 2018

We now have another issue.

Since we're inside an iframe, we seem to have issues with the SameSiteCookieCheck introduced at nextcloud/server#6630. For some reason the request does not pass the LaxSameSiteCookie check and it 302 redirects endlessly to itself.

This can be kinda fixed by adding @NoSameSiteCookieRequired to our public routes, but then the core css isn't delivered for the same reason, unless we have this kind of patch against server, which is rather dirty.

diff --git a/lib/private/AppFramework/Middleware/Security/SameSiteCookieMiddleware.php b/lib/private/AppFramework/Middleware/Security/SameSiteCookieMiddleware.php
index 22a9246d66..0098090cfd 100644
--- a/lib/private/AppFramework/Middleware/Security/SameSiteCookieMiddleware.php
+++ b/lib/private/AppFramework/Middleware/Security/SameSiteCookieMiddleware.php
@@ -44,9 +44,21 @@ class SameSiteCookieMiddleware extends Middleware {
        }
 
        public function beforeController($controller, $methodName) {
+               $pathInfo = $this->request->getPathInfo();
+               $parts = explode('/', $pathInfo);
+               $parts = $parts[1];
                $requestUri = $this->request->getScriptName();
                $processingScript = explode('/', $requestUri);
                $processingScript = $processingScript[count($processingScript)-1];
+
+               if ($parts === 'css') {
+                       return;
+               }
 
                if ($processingScript !== 'index.php') {
                        return;

AFAIK this is the only app that has a public page and allows embeding it (and this is quite a requested feature).

I'm asking for advice to @rullzer @LukasReschke @MorrisJobke here since they were linked to the server change.

@rullzer

This comment has been minimized.

Copy link
Member

@rullzer rullzer commented Aug 7, 2018

Do you have some steps for me to run to trigger the error? Else it is hard to see what is going on.

@tcitworld

This comment has been minimized.

Copy link
Member Author

@tcitworld tcitworld commented Aug 8, 2018

Sure, this should be it:

  • Share a public link from a calendar
  • open the public link
  • Take the embed html iframe code on the left
  • include it in any basic html page
  • access the html page
@tplusm

This comment has been minimized.

Copy link

@tplusm tplusm commented Aug 14, 2018

@tcitworld same issue here

@rullzer

This comment has been minimized.

Copy link
Member

@rullzer rullzer commented Aug 17, 2018

So I had some time to look into this.

The issue indeed our samesite cookie check. In an IFRAME the lax cookies are not send. Resulting in the check to fail because we do send the other cookies.

Now we could do a lot of nasty hacks and exceptions. But I think the cleaner approach here would be to migrate to lax samesite cookies for all our cookies. This only makes the cookies more secure. Plus it makes sure that if you embed some public page somewhere everbody will see the exact same thing. (as the IFRAME won't send any cookies then resulting in a true 'publicpage' rendering).

I'll track this in the server.

@rullzer rullzer mentioned this issue Aug 17, 2018
0 of 3 tasks complete
@tcitworld

This comment has been minimized.

Copy link
Member Author

@tcitworld tcitworld commented Oct 2, 2018

Thanks to nextcloud/server#11433 we should be good with NC15.

@rullzer

This comment has been minimized.

Copy link
Member

@rullzer rullzer commented Oct 2, 2018

Yes you should.
Would be good of course if somebody coould setup a test server and test it 😉

@tcitworld

This comment has been minimized.

Copy link
Member Author

@tcitworld tcitworld commented Oct 7, 2018

@rullzer

This comment has been minimized.

Copy link
Member

@rullzer rullzer commented Oct 16, 2018

Ah ok. So the check I had in mind doesn't fully work. As that would with the current logic log you out of your nextcloud if you embed a page.

Thinking more this would need a few adjustments

  1. calendar endpoint must of course be marked with @NoSameSiteCookieRequired
  2. The css and js endpoint should be marked as such I don't think this hurts as this is public info anyway
  3. The theming endpoints should be marked as such. Also should not be that much of an issue as they just serve generic theming info
@tcitworld

This comment has been minimized.

Copy link
Member Author

@tcitworld tcitworld commented Oct 17, 2018

Works perfectly ! Will send PRs. :)

tcitworld added a commit that referenced this issue Oct 17, 2018
Fixes #169 (will probably require NC 15 though)

Signed-off-by: Thomas Citharel <tcit@tcit.fr>
tcitworld added a commit to nextcloud/server that referenced this issue Oct 17, 2018
Which can be used for public iframe embeeding

See nextcloud/calendar#169

Signed-off-by: Thomas Citharel <tcit@tcit.fr>
weeman1337 added a commit to nextcloud/server that referenced this issue Oct 28, 2018
Which can be used for public iframe embeeding

See nextcloud/calendar#169

Signed-off-by: Thomas Citharel <tcit@tcit.fr>
@georgehrke georgehrke reopened this Oct 25, 2019
@pReya

This comment has been minimized.

Copy link

@pReya pReya commented Jan 19, 2020

Got the same problem as described in #1861

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
10 participants
You can’t perform that action at this time.