Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

move caching out of memory #715

Closed
chadwhitacre opened this issue Mar 9, 2013 · 32 comments · Fixed by #3226
Closed

move caching out of memory #715

chadwhitacre opened this issue Mar 9, 2013 · 32 comments · Fixed by #3226

Comments

@chadwhitacre
Copy link
Contributor

There's one or two places in the app where we have an in-memory cache that will prevent us from scaling. I believe the two places are:

  • /on/twitter/ - we store state in memory during the round-trip to Twitter for oauth
  • /on/confirm.html - something about "connect tokens"
@bruceadams
Copy link
Contributor

/on/bitbucket also uses website.oauth_cache. I am very curious how it is that our tie to Github avoids this. Someday I'll have to learn our elsewhere related code.

@zwn
Copy link
Contributor

zwn commented Oct 22, 2013

Let's just hope that #505 (#1369) will make these go away :)

@seanlinsley
Copy link
Contributor

IRC

@seanlinsley
Copy link
Contributor

@whit537 do we currently have a mechanism to store a secure cookie in the browser?

@chadwhitacre
Copy link
Contributor Author

@seanlinsley Not really?

@bruceadams
Copy link
Contributor

I was just peaking at the oauth_cache stuff. We send a bunch of cookies already. I'm not quite sure what the sequence of events is where we need the cache, but it appears to be just hanging on to a little bit of data during the out-and-back to the "elsewhere" OAuth service. It's the sort of thing we should be able to do with a cookie, maybe even a short-lived cookie. We do need to be careful that we don't create a security hole.

@seanlinsley
Copy link
Contributor

Yeah I've been re-learning how OAuth works to see what exactly is needed at each stage.

@chadwhitacre
Copy link
Contributor Author

@seanlinsley @bruceadams Would you two be willing to own this ticket and land it before the retreat (Jan 3)?

@bruceadams
Copy link
Contributor

@whit537 yes!

@seanlinsley
Copy link
Contributor

of course :)

@chadwhitacre
Copy link
Contributor Author

Awesome! Thank you! :-)

❤️ 🚀

@ghost ghost assigned bruceadams Dec 2, 2013
@bruceadams
Copy link
Contributor

Looks like i cannot assign this to two people. I assigned it to me, mostly to show my focus. I don't intend to show exclusive ownership.

@bruceadams
Copy link
Contributor

Thinking and looking at this a bit more. We can make it work with a cookie, but I still don't know what the security implications are there. We could do server-side caching into the database instead into memory. That keeps security clean (as least as clean as it is now). We'd want to keep whatever little cache table we setup in the database clean. Each secret we need to hang on to is very short lived, no more than a few minutes, typically just a few seconds.

@chadwhitacre
Copy link
Contributor Author

@bruceadams Let's understand what exactly it is we're storing. If I remember right it's only a redirect URL or something non-sensitive like that.

@chadwhitacre
Copy link
Contributor Author

... in which case storing it in a non-secure cookie would be fine. Even if it's sensitive I like the idea of storing it in a secure cookie somehow instead of cluttering up the database.

@zwn
Copy link
Contributor

zwn commented Dec 4, 2013

I don't know how OAuth works but is this thing that we store in memory secret from the user? If it is not sensitive we can just plug it to the url as a param and be done.

@chadwhitacre
Copy link
Contributor Author

@zwn For GitHub I was able to pass the data through the URL, for Twitter I wasn't for some reason.

@chadwhitacre
Copy link
Contributor Author

Obviously if that can be figured out it's preferable.

@bruceadams
Copy link
Contributor

I certainly agree with both of that carrying it along in the URL or using a cookie is preferable to using the database (or any other server-side mechanism) so long as we don't mess up security. I'm (slowly) learning more about OAuth.

@seanlinsley
Copy link
Contributor

stealing this from @bruceadams :octocat: (IRC)

@ghost ghost assigned seanlinsley Dec 16, 2013
@chadwhitacre
Copy link
Contributor Author

@seanlinsley Looking at Huboard, I see this in Ready to Start with your name on it. I believe I closed the item you had under Work in Progress (#986), as discussed at the retreat (apologies). Does that mean you'll be going here next, or over to #986? As mentioned on #986, I think we may be able take care of that with a simple configuration change.

@chadwhitacre
Copy link
Contributor Author

@seanlinsley I guess what I'm saying is that I'd love to see this ticket get done. :-)

@seanlinsley
Copy link
Contributor

Sure, this can be the next thing I do.

@chadwhitacre
Copy link
Contributor Author

Thanks @seanlinsley! :-)

@zwn
Copy link
Contributor

zwn commented Jan 7, 2014

I've nominated this to the 'DevX ★' label.

@zwn
Copy link
Contributor

zwn commented Jan 8, 2014

We now have the same issue with OpenStreetMap #1848. I merged it anyway thinking once we know what to do we will just do it for both.

I've been trying to read up on OAuth. Could the solution be just to cache the oauth_token_secret in the database? I am getting the feeling that this might be done just once per application and not for each sign in. The other stuff could be just passed around in some nonsafe way since it is not security related (action and then).

@seanlinsley I'd like to be your coworker on this one. Feel free to ping me.

@zwn
Copy link
Contributor

zwn commented Jan 9, 2014

Check out http://requests-oauthlib.readthedocs.org/en/latest/examples/bitbucket.html. It seems that no state is carried from step 3 to step 4. The only thing there is the OAuth1Session object that is constructed from a static data.

@zwn
Copy link
Contributor

zwn commented Jan 9, 2014

OAuth is so hard to grok 😓. Now I think that request token secret really needs to be stored securely on our side. I'd do this in the db for now, since we do not have any memcached or redis going but I'd model the schema like we had (just creating a cache table with a key and data, the data possibly being hstore type?).

@zwn
Copy link
Contributor

zwn commented Jan 13, 2014

Maybe we could use tokenlib to store the token secret in a cookie or redirect url? I am afraid there is still something I am not getting about OAuth1. Why would someone design a protocol that forces the use of server side permanent storage if it could be done without it?

@zwn
Copy link
Contributor

zwn commented Jan 17, 2014

Nice overview for Twitter OAuth1 flow https://github.com/inueni/birdy#great-what-about-authorization-how-do-i-get-my-access-tokens that is somewhat readable (the best I have found so far).

It seems there is really no way to add custom params to any of the requests (nowhere to hide even encrypted token_secret). So secure cookie with tokenlib or cache in db are the two available options. I am reasonably sure there are no other.

@seanlinsley
Copy link
Contributor

I was looking into using tokenlib for this, but I can't make sense of the code as it is. I'm biting the bullet, diving in to #1369

@chadwhitacre
Copy link
Contributor Author

It's okay to store oauth_token and the related thing from OAuth2 in an unsecured cookie (IRC). On #1369 we're doing this for the oauth caches, but for this ticket we still need to address the merge confirmation workflow.

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

Successfully merging a pull request may close this issue.

5 participants