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

Updating the leaking-user test to reproduce a problem I'd found earlier. #2

Closed
wants to merge 1 commit into from

Conversation

jmacdotorg
Copy link
Contributor

Per Yanick's request, constructing a simulation of an issue I ran into when using the current release of this module in a live Catalyst-based application.

Changes made include using two different $mech objects (to simulate two separate but simultaneous sessions), and having the /leaking_users action display the Twitter ID of the currently authenticated user.

Please disregard the first two no-op change-chunks in basic-app.t; not sure how those got in there.

@yanick
Copy link
Owner

yanick commented Mar 9, 2013

Sorry for the day. Got around checking out the branch tonight and, ah ah!, it does reproduce the problem here. Now that I can see it first-hand, I'll see what I can do. Thanks!

@jmacdotorg
Copy link
Contributor Author

Hooray! Glad to know it's not just me. Let me know if I can be of further help.

For what it's worth: the simple workaround that I added to my Catalyst application is to call $c->get_auth_realm('twitter')->credential->twitter_user( undef ); at the end of every request. That clears out that bit of cached information whose presence, I think, is at the heart of the problem, and it doesn't prevent login from occurring as expected. Hope that's helpful info...

@yanick yanick closed this in 6b1000c Mar 9, 2013
@yanick
Copy link
Owner

yanick commented Mar 9, 2013

I'm fairly sure I've fixed the problem. New version v1.0.0 is on its way to CPAN.

Warning: the function twitter_user now takes $c as an argument, and is stored in the per_user session object rather than globally for the plugin (one of the reason why it could end up being shared).

@jmacdotorg
Copy link
Contributor Author

Just an FYI that I've updated my in-production Catalyst::Authentication::Credential::Twitter installation and removed my app's workaround code, and everything is running smoothly so far.

Thanks again for attending to this so quickly!

On Mar 9, 2013, at 6:32 PM, Yanick Champoux notifications@github.com wrote:

I'm fairly sure I've fixed the problem. New version v1.0.0 is on its way to CPAN.

Warning: the function twitter_user now takes $c as an argument, and is stored in the per_user session object rather than globally for the plugin (one of the reason why it could end up being shared).


Reply to this email directly or view it on GitHub.

Jason McIntosh
Appleseed Software Consulting

jmac@appleseed-sc.comwww.appleseed-sc.com

@yanick
Copy link
Owner

yanick commented Mar 26, 2013

On 13-03-26 05:25 PM, Jason McIntosh wrote:

Just an FYI that I've updated my in-production
Catalyst::Authentication::Credential::Twitter installation and removed
my app's workaround code, and everything is running smoothly so far.

*victory dance*

Awesome. :-)

Thanks again for attending to this so quickly!

My pleasure. And thanks for helping with the bug hunting.

Joy,
`/anick

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