[ENG-6820] Fix n+1 queries #201
Conversation
opaduchak
force-pushed
the
feature/ENG-6820
branch
from
113f897 to
97a17c8
Compare
opaduchak
force-pushed
the
feature/ENG-6820
branch
from
23311c7 to
6e97e8d
Compare
addon_service/authentication.py
Outdated
| @@ -9,6 +9,8 @@ class GVCombinedAuthentication(drf_authentication.BaseAuthentication): | |||
| """Authentication supporting session, basic, and token methods.""" | |||
|
|
|||
| def authenticate(self, request: DrfRequest): | |||
| if request.session.get("user_reference_uri"): | |||
There was a problem hiding this comment.
I think this is a potential security problem. This session lives on gravyvalet. A sessionId cookie is set on the client side by whatever origin that hosts gravyvalet. That means when user logs out of osf.io, the sessionId cookie is not cleared on the client, nor is the session itself invalidated on the gravyvalet server side. Here's one potential scenario that could happen on staging:
- User logs into staging.osf.io.
- User makes request to gravyvalet.
- gravyvalet sets the
sessionIdcookie. - User logs out of staging.osf.io.
sessionIdcookie is not cleared on the client side, nor is the session invalidated on gravyvalet server side.- Using the same browser, an unauthenticated user successfully makes another request to gravyvalet using the old
sessionIdcookie.
Unless we can make sure that logging out of osf.io would also invalidated the corresponding session on the GV side, I don't think we can rely on just user_reference_uri on the session. Instead, we have to make sure the user provides the correct osf cookies for every request.
@opaduchak @brianjgeiger @aaxelb for a sanity check.
There was a problem hiding this comment.
Agreed, with no way to invalidate the session, we need to rely on osf.io for maintaining that information.
There was a problem hiding this comment.
Would you like me to revert this change or implement session invalidation during logout?
There was a problem hiding this comment.
Let's revert the change for this PR. If we want to implement session invalidation we can create a new ticket.
No description provided.