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

Canvas and Page Tab URLs must not end with a slash #177

Open
wants to merge 2 commits into
base: 3.0
Choose a base branch
from
Open

Canvas and Page Tab URLs must not end with a slash #177

wants to merge 2 commits into from

Conversation

apasov
Copy link

@apasov apasov commented Jun 7, 2017

While implementing Login From App Canvas as described here I encountered some problems. I think the solution I've found should be described in the Troubleshooting section. The main problem is that if Canvas or Page Tab URL has a trailing slash then web server performs a 301 redirect and the POST'ed signed_request is lost. Also I think that Route::match(['get', 'post'] is wrong (and in my case it was misleading) because both Canvas and PageTab helpers will always obtain null access token from a GET request. It should be Route::post in both examples.

While implementing Login From App Canvas as described here I encountered some problems. I think the solution I've found should be described in the Troubleshooting section. The main problem is that if Canvas or Page Tab URL has a trailing slash then web server performs a 301 redirect and the POST'ed signed_request is lost. Also I think that Route::match(['get', 'post'] is wrong (and in my case it was misleading) because both Canvas and PageTab helpers will always obtain null access token from a GET request. It should be Route::post in both examples.
@SammyK
Copy link
Owner

SammyK commented Jul 22, 2017

Hey @apasov! Thanks for the contribution! I'm certainly willing to accept this PR if you revert the route back to supporting both GET and POST. The reason is there are lots of Facebook apps and page tabs that have multiple pages. So the first hit will be a POST, then the user clicks on another link (within the canvas) and then back to home page and then it's a GET request. So the route needs to support both. :)

Hey @SammyK! Thanks, I'm glad my contribution was useful! Ok, I reverted the route back to both POST and GET. However, I still think it's not a very good solution. I believe that for the situation you described a much better approach would be to create a separate GET route without CanvasHelper token validation logic in it, rather then combine both POST and GET in a single route. But that's just my opinion. :)
@apasov
Copy link
Author

apasov commented Jul 26, 2017

Hey @SammyK! Thanks, I'm glad my contribution was useful! Ok, I reverted the route back to both POST and GET. However, I still think it's not a very good solution. I believe that for the situation you described a much better approach would be to create a separate GET route without Canvas/PageTab helpers token validation logic in it, rather than combine both POST and GET in a single route. But that's just my opinion. :)

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

Successfully merging this pull request may close these issues.

2 participants