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

Proper fix for tntvillage #72

Merged
merged 2 commits into from
Mar 4, 2016
Merged

Proper fix for tntvillage #72

merged 2 commits into from
Mar 4, 2016

Conversation

medariox
Copy link
Contributor

@medariox medariox commented Mar 2, 2016

@duramato works?

@medariox
Copy link
Contributor Author

medariox commented Mar 3, 2016

@p0psicles
While your idea is surely good and I agree this is the proper way to do it, checking the expiration date of the cookies is not always an acceptable solution. For example in the case of tntvillage, the cookies are set to expire after one year, but that is simply not the case and they will expire after a few days. That might be caused by a wrongly configured server, faulty software, etc. This might not be the case for Gazelle or TBDev based trackers (although they are often highly customized), under the assumption that the server is properly configured and the software functionality hasn't been compromised.

A quick look at tntvillage's cookies shows us this exact problem:
tnt-logged
These are the cookies while logged in. They show us an expiration of one year.

Here are the cookies after you get logged out (few days usually):
tnt-logged-out

As you can see we will need some kind of exceptions handling, for providers that are not so easy to handle by just checking the cookies validity.

@p0psicles
Copy link
Contributor

Then you could implement an exception for this prov using the custom_auth_check()

@medariox
Copy link
Contributor Author

medariox commented Mar 3, 2016

@p0psicles
Sure, just wanted to point out that checking the cookies validity might not always be the way to go. Did you check that this generic solution works for most trackers?

@p0psicles
Copy link
Contributor

No, i'm not saying I'll be able to solve all provider auth issues with one check. But this will give us much more flexibility for in the future. I do have to confess that this was @labrys idea. Haha. And the're might still be some issues with the way I implemented it tough.

medariox added a commit that referenced this pull request Mar 4, 2016
@medariox medariox merged commit 2c2992b into develop Mar 4, 2016
@fernandog fernandog deleted the medariox-tntvillage-fix branch March 4, 2016 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants