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

HTTPS connection downgraded to HTTP after social login success #1536

Closed
mariuskiessling opened this issue Jul 15, 2019 · 6 comments

Comments

@mariuskiessling
Copy link

commented Jul 15, 2019

After a user successfully authenticated using the social AzureAD login, BookStack receives Azure's login redirect (via HTTPS) but drop to HTTP when redirecting to the homepage. I could not test it using a different authentication provider but this issue should not be limited to AzureAD logins.

This issue occurs due to a faulty location header in BookStack's callback handler response.

return redirect()->intended('/');

The app URL is set to HTTPS.

Steps To Reproduce

  1. Start the authentication flow using the AzureAD authentication driver
  2. Login on the Microsoft login page
  3. Get redirected to BookStack
  4. Get redirected again to the intended page (or /)
  5. The connection is now no longer secured using HTTPS but is established via HTTP

Expected behavior
After the callback by AzureAD is processed, don't redirect the user to the insecure HTTP site.

Screenshots
Screen Shot on 2019-07-15 at 16:23:06

Configuration

  • BookStack Version: v0.26.2
  • PHP Version: 7.something
  • Hosting Method (Nginx/Apache/Docker): Docker using NGINX
@erosinger

This comment has been minimized.

Copy link

commented Jul 15, 2019

I am observing the same issue and would be interested in seeing this fixed. Especially for end users or network environments with high-security awareness, this might cause failures or losing trust to BookStack.

@ssddanbrown

This comment has been minimized.

Copy link
Member

commented Jul 15, 2019

Thanks for the clear information @mariuskiessling.

This is likely due to BookStack being behind a proxy, in which case BookStack is actually receiving the original request via HTTP which is remembered as a location for this redirect action.

It's likely related to #1459 which is due to be looked at for the next release therefore I'll also include this to be looked at.

@ssddanbrown ssddanbrown added this to the v0.27.0 milestone Jul 15, 2019

@mariuskiessling

This comment has been minimized.

Copy link
Author

commented Jul 15, 2019

@ssddanbrown That’s a really good point. We are running BookStack behind an envoy proxy as part of a service mesh. The encryption of traffic is fully transparent to the service thus every connection looks like HTTP to BookStack.

Would it make sense to split the indented path and only save the relative path to the requested ressource? If needed the application’s domain could be prefixed. This would also only require minor modifications.

@ssddanbrown

This comment has been minimized.

Copy link
Member

commented Jul 15, 2019

@mariuskiessling Yeah, That makes sense and is the general plan. It may be a little tricky though as you never know how that URL will be transformed by the time it reaches BookStack, Some people may have BookStack on a deep nested path (https://example.com/my/lovely/cat/wiki) which would need be be handled properly.

Since originally writing much of the original URL logic I've gotten a better grasp of the Laravel framework and I can now see a cleaner way to handle URLs in BookStack so I'll probably do this as part of a larger refactor for the next release.

@ssddanbrown

This comment has been minimized.

Copy link
Member

commented Aug 4, 2019

Significant changes have been made to URL generation in 4b0c4e6 which should now fix this.

These changes will be in the next release, v0.27. I will close this request in the meantime. If you continue to experience issues after that version is released please open a new issue referencing this one.

@mariuskiessling

This comment has been minimized.

Copy link
Author

commented Aug 4, 2019

@ssddanbrown Thank for addressing this issue so quickly!

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