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

Request Uri is wrong if flarum installed in subfolder #778

Closed
sijad opened this issue Feb 8, 2016 · 15 comments
Closed

Request Uri is wrong if flarum installed in subfolder #778

sijad opened this issue Feb 8, 2016 · 15 comments

Comments

@sijad
Copy link
Contributor

sijad commented Feb 8, 2016

if flarum installed in a subfolder (e.g. http://localhost/flarum)
in a Controller (TwitterAuthController for example) (string) $request->getUri()->withQuery('') will return http://localhost/auth/twitter instead of http://localhost/flarum/auth/twitter

I trace down the issue here

please keep in mind this issue is not related to login extensions (twitter, facebook and github).

@sijad
Copy link
Contributor Author

sijad commented Feb 8, 2016

(string) $request->getOriginalRequest()->getUri()->withQuery('') returns the correct path (http://localhost/flarum/auth/twitter for example)

I'm not sure, maybe I'm wrong and this is related to login extensions

@luceos
Copy link
Member

luceos commented Feb 8, 2016

Is this in the latest dev-master?

@sijad
Copy link
Contributor Author

sijad commented Feb 8, 2016

Yes it's I always keep my installation updated

@tobyzerner
Copy link
Contributor

@sijad (string) $request->getOriginalRequest()->getUri()->withQuery('') returns the correct path

Spot on. Or alternatively, $request->getAttribute('originalUri')->withQuery(''). It needs to be replaced both in flarum-ext-auth-twitter's TwitterAuthController, and core's AbstractOAuth2Controller (which the other auth extensions extend)

@franzliedke
Copy link
Contributor

Hmm, well, that method isn't part of the ServerRequestInterface, though. Another option would be to tell our URL generator about the correct prefix in the corresponding middleware.

@tobyzerner
Copy link
Contributor

@franzliedke In this case, the URL generator isn't involved; we just want to get the original request URL (before the prefix is stripped as part of the middleware pipe). Perhaps we could just get the originalUri attribute and use the current request object if it's not set?

@franzliedke
Copy link
Contributor

Yeah, I think I'd prefer that.

sijad added a commit to sijad/core that referenced this issue Feb 9, 2016
sijad added a commit to sijad/core that referenced this issue Feb 9, 2016
@clarkwinkelmann
Copy link
Member

Regression time ! Since beta 8, this issue is again present. After the many reports of people having issue with this on Discuss I finally investigated.

The issue presents the same symptoms as the 2016 issue. The redirect url sent by Flarum to the provider does not include the subfolder. It impacts all core login providers.

This is because in stragility 1.3.0 (released in November 2016!) zendframework/zend-stratigility#70 the originalUri attribute is no longer set by default. It's now part of a separate middleware (Zend\Stratigility\Middleware\OriginalMessages). Look for PR #70 in the changedlog for details on the change https://github.com/zendframework/zend-stratigility/blob/master/CHANGELOG.md

The regression must have been present since beta 7 inclusive, when stragility dependency was bumbed from ^1.1 to ^1.3.

Because of this, the call to $request->getAttribute('originalUri', $request->getUri()) always actually returns $request->getUri() because the attribute doesn't exist (in https://github.com/flarum/auth-facebook/blob/1088a2dd77384fe6e725367ab66e5d2b64094724/src/FacebookAuthController.php#L54 and similar issue with the other auth extensions)

The solution would be to use the new Zend\Stratigility\Middleware\OriginalMessages middleware at the right place in Flarum to preserve the full path (not sure where that right place is though). Or we could actually manually construct the redirect url with the url builder now that every auth provider has its own call to create the url anyway (this wasn't the case <=beta7, so at this time it made sense to have a way to generate the redirect url independently of which provider actually finished handling the request)

@tobyzerner
Copy link
Contributor

Thanks @clarkwinkelmann for investigating!

I think we should add the OriginalMessages middleware regardless of whether we choose to manually construct redirect URLs or not, as having the original URI (including subfolder) available for general use seems like a good idea.

@Likqez
Copy link

Likqez commented May 30, 2019

How to fix this error now? Or are you guys implementing it into Flarum?

@clarkwinkelmann
Copy link
Member

clarkwinkelmann commented May 30, 2019

A workaround that I confirmed working. You can add the following to your .htaccess file in the root folder (the Wordpress htaccess file for example):

<IfModule mod_rewrite.c>
  RewriteEngine on
  RewriteRule ^(auth/.*)$ /flarum/$1 [R,L]
</IfModule>

EDIT: added L flag as this will probably be necessary in most cases.

It might require some tweaking depending what is already in the .htaccess file. For Wordpress for example you could add the 3rd line only inside the existing mod_rewrite block of Wordpress.

This will redirect all domain.tld/auth/* to domain.tld/flarum/auth/*. Substitute flarum with your Flarum subfolder name. In Facebook/Twitter/GitHub, whitelist domain.tld/auth/facebook (or twitter or github) without the subfolder. You can whitelist both urls (with and without the subfolder) so you won't have anything to change once this is fixed in Flarum.

If your root website uses urls starting with /auth, you might be able to replace .* with (facebook|twitter|github) to not have such a broad catch-all rule.

@Likqez
Copy link

Likqez commented May 30, 2019

Thanks for the reply, i changed my .htaccess but it'll still redirect me to my wordpress page.
My htacces looks like:
https://hastebin.com/uyaruqafuy.apache

@clarkwinkelmann
Copy link
Member

@Likqez Seems like an L flag is required for the workaround to work with Wordpress. Try

RewriteRule ^(auth/.*)$ /forum/$1 [R,L]

@Likqez
Copy link

Likqez commented Jun 1, 2019

Thanks, its working now

@franzliedke
Copy link
Contributor

Fixed by adding the middleware to the InstalledApp request handler.

I will also change the three OAuth extensions to use the URL generator to determine the callback URI, as that feels more correct. Good suggestion.

@clarkwinkelmann Let's open new issues when encountering regressions in later versions (issues can only be assigned to one milestone etc.)

franzliedke added a commit to flarum/auth-facebook that referenced this issue Jun 1, 2019
franzliedke added a commit to flarum/auth-github that referenced this issue Jun 1, 2019
franzliedke added a commit to flarum/auth-twitter that referenced this issue Jun 1, 2019
@luceos luceos modified the milestones: 0.1.0-beta.5, 0.1.0-beta.9 Jun 4, 2019
wzdiyb pushed a commit to wzdiyb/core that referenced this issue Feb 16, 2020
This is helpful when Flarum is installed in subfolders.

Fixes flarum#778.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants