add :strategy option and protocol to middleware #16
Conversation
This allows other CSRF-protection strategies to be employed.
|
Sorry, can you explain the purpose of this PR? It looks like you've just copied the code from the anti-forgery middleware, but I don't understand why. |
|
Ah, sorry, my fault: I'm so deep into that right now I'm not aware that the requirements are not obvious ;) Yes, it's almost (unfortunately not exactly) a copy of the protocol from ring-anti-forgery. Reason is, the state parameter in the OAuth flow is just a CSRF-protection. Thus, the same requirements here lead to a similar (but not exactly identical) solution: Again: Introducing other strategies will allow for implementations without server state. It's not exactly the same, as we need the profile/id. Hope this clarifies things. Cheers, Chris |
|
What use is this when the OAuth access token needs to be stored across state anyway? |
|
Oh, it does not need to:
- if you fetch all the data required immediately in the success handler.
- alternatively, you need to store it in a DB or another persistent storage (again, without the need of in-memory server state).
|
Where are you going to store that data? Or are you saying that the success handler would just do whatever it needs to do, and then just discard the access token and any other user data?
A session doesn't need to be in-memory, and it usually isn't in production because that wouldn't work behind a load balancer. |
|
Am 29.01.2018 um 18:15 schrieb James Reeves ***@***.***>:
if you fetch all the data required immediately in the success handler.
Where are you going to store that data? Or are you saying that the success handler would do whatever it needs to do, and store nothing?
Exactly. That might be enough in some use-cases (it is, e.g. where one just needs to authenticate the user, grabbing his/her eMail once)
alternatively, you need to store it in a DB or another persistent storage (again, without the need of in-memory server state).
A session doesn't need to be in-memory, and it usually isn't in production because that wouldn't work behind a load balancer
Yes, but a more or less transient ‚state‘ to prevent CSRF attacks is something different than a long lived user identity, that‘s why I made that difference. Not having (session) state simplifies balancing despite the possibility to persist stuff to DB or memcached or whatever.
|
|
I'll think about this, but my initial reaction is that this adds too much complexity for what seems like a fairly uncommon use case. It's possible to use OAuth entirely statelessly, but my guess is that most of the time applications going to be persisting at least some state across requests, either stored on the server, or via a cookie. In such cases, there seems no advantage to avoiding sessions. |
|
I'll think about this, but my initial reaction is that this adds too much complexity for what seems like a fairly uncommon use case. It's possible to use OAuth entirely statelessly, but my guess is that most of the time applications going to be persisting at least some state across requests, either stored on the server, or via a cookie. In such cases, there seems no advantage to avoiding sessions.
That‘s fine. I‘ll be able to run my fork of ring-oauth for my very special requirements. Feel free to take it leave it just as you wish.
|
Hi,
I'm not sure whether you're interested in this PR or not ... it's almost the same (and made for the same reasons) as the one on ring-anti-forgery.
As this library is much more "isolated", I could move forward with my own fork, it's completely up to you.
Yes, there still are some whitespace issues... I'm removing them by hand (as my tooling has different defaults obviously) if you're interested. Just let me know.
This change is