If your Drupal site forces some areas to HTTPS and others to HTTP, two cookies are created when logging in (one for each mode). The Twitter Sign-in module doesn't seem to respect this and only creates one cookie. This means that when logging out, things get confused and you end up in a sort of half-logged-in state (logged in to HTTPS and logged out of HTTP).

The attached patch *seems* to fix this, but I'm not sure if it's the best way to do it. Patched against 7.x-3.0-beta4.

CommentFileSizeAuthor
#11 twitter-n1514284-11-6x-5.x.patch2.24 KBdamienmckenna
#10 twitter-n1514284-9-6x-5.x.patch1.94 KBdamienmckenna
#7 twitter-n1514284-6-7.x-6.x.patch457 bytesdamienmckenna
#6 twitter-n1514284-5-7.x-5.x.patch457 bytesdamienmckenna
twitter-signin-beta4-patch.diff532 bytesdarrylh

Comments

juampynr’s picture

juampynr commented
Version: 7.x-3.0-beta4 » 7.x-3.x-dev
Issue tags: +https, +twitter-https

Hmmm, I have no idea to be honest. I will wait a little bit so more people can chime in while I review other issues.

13rac1’s picture

13rac1 commented
Version: 7.x-3.x-dev » 7.x-5.x-dev

Old patch applies to 7.x-5.x with offset. Unable to test further. Normally Drupal HTTP and HTTPS are separate logins. How does this patch change the login/logout process?

$ patch -p1 < twitter-signin-beta4-patch.diff 
patching file twitter_signin/twitter_signin.module
Hunk #1 succeeded at 140 (offset 13 lines).
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Issue summary: View changes
Status: Needs review » Needs work

Triggering the testbot.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Needs review

Triggering the testbot.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Issue tags: -twitter-https
Parent issue: » #2402311: Plan for Twitter v6.x-5.2 release
Related issues: +#2402307: Plan for Twitter v7.x-5.9 release, +#2402309: Plan for Twitter v7.x-6.0 release
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
StatusFileSize
new twitter-n1514284-5-7.x-5.x.patch457 bytes

I don't believe this is needed, the oauth callback function already calls user_login_finalize(). However, lets pass along the standard $form_state arguments into user_login_finalize() that user_login_submit() passes, so that the hooks can be triggered correctly.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
StatusFileSize
new twitter-n1514284-6-7.x-6.x.patch457 bytes

Ported to 7.x-6.x.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

I also suspect the cookie issue is a problem in core rather than something in the Twitter module - there are no calls to setcookie() in twitter_signup, instead it just uses the user's existing session.

  • DamienMcKenna committed f82efce on 
    Issue #1514284 by DamienMcKenna, darrylh: Ensure form state is passed to...
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Version: 7.x-5.x-dev » 6.x-5.x-dev
StatusFileSize
new twitter-n1514284-9-6x-5.x.patch1.94 KB

Some similar cleanup for the 6.x-5.x branch.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
StatusFileSize
new twitter-n1514284-11-6x-5.x.patch2.24 KB

Some additional comment improvements for the D6 branch.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs review » Fixed

Committed.

  • DamienMcKenna committed 2756b6b on 6.x-5.x 
    Issue #1514284 by DamienMcKenna: Some slight cleanup to the signup oauth...

  • DamienMcKenna committed c9fc28e on 7.x-6.x 
    Issue #1514284 by DamienMcKenna, darrylh: Ensure form state is passed to...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.