django-oscar / django-oscar-api

RESTful JSON API for django-oscar
Other
369 stars 160 forks source link

Merging wrong baskets on login #272

Closed ladrua closed 2 years ago

ladrua commented 2 years ago

Sometimes after login the users baskets seem to be merged with "random" anonymous baskets.

This specific project has "custom inputs" for each orderline(personalizations) so it is easy to identify that they are coming from multiple baskets.

Using v2.1.0 and just using api calls from nuxt app on same domain.

Anyone got any idea what could be causing this error? The login is using JWT, and we have copied the "merging" logic from this projects login view.


def post(self, request, *args, **kwargs):
        # Use the rest framework `.data` to fake the post body of the django request.
        mutable_data = request.data.copy()
        request._request.POST = request._request.POST.copy()
        for key, value in mutable_data.items():
            request._request.POST[key] = value

        url, headers, body, status = self.create_token_response(request._request)
        response = Response(data=json.loads(body), status=status)

        ser = LoginSerializer(data=request.data)
        ser.is_valid()
        user = ser.instance
        request.user = user
        anonymous_basket = operations.get_anonymous_basket(request)
        basket = operations.get_user_basket(request.user)
        response.data['basket_is_empty'] = basket.is_empty

        if anonymous_basket is not None:
            basket.merge(anonymous_basket)

        operations.store_basket_in_session(basket, request.session)

        for k, v in headers.items():
            response[k] = v
        return response
maerteijn commented 2 years ago

Hi Ladrua,

The anonymous baskets are retrieved from the active django session, see https://github.com/django-oscar/django-oscar-api/blob/95d88a5a97e99656b3785d1d3dff1bef42d9cd12/oscarapi/basket/operations.py#L72

This assumes that a django session is unique for each anonymous user, could it be that this isn’t the case and that the same sessions are used for multiple anonymous users in your situation?

See also the documentation regarding sessions, cookies and middleware here: https://django-oscar-api.readthedocs.io/en/latest/topics/middleware.html

ladrua commented 2 years ago

Hi @maerteijn thanks for the reply! I also gather it must be a session/session-id issue somehow.

I did notice in the beginning that the middleware was missing for the project, and we added the oscarapi.middleware.ApiBasketMiddleWare and I was sure this was the cause of the error. But then the next day we got the same bug.

Could a missing middleware cause this you think?

The frontend(nuxt) is headless but reside on the same domain using reverseproxy domain.com and the api domain.com/api so it should be sharing session/id/data across the apps I would think this should make sure all sessions are unique on both nuxt and django backend. But this is the only place left to look for a bug regarding this, as I cant seem to find any other logical place look now.

Thanks again!

maerteijn commented 2 years ago

Could a missing middleware cause this you think?

I don't think so. As I think all is functioning "as should", except for the case that session cookies are somehow reused over multiple clients, which is not what you want of course.

How do you serve your Nuxt front-end? You could try to do this with a regular django view. This way you invoke the django session logic with each visit which then makes sure you'll have a unique session cookie for each client.

ladrua commented 2 years ago

So its a container setup, nuxt frontend living in a node container and django backend living in a python container both behind a nginx proxy under the same domain. Frontend domain.com and backend domain.com/api/. I do believe this is "best practice" or at least common practice.

So in my case they are both using the same cookie as they are both under the same domain. So its strange. Maybe my nuxt frontend app is not clearing out the cookies somehow? I guess this is starting to sound like a not django-oscar-api issue then.

My only bet was the middleware, and there is another middleware mentioned in the docs oscarapi.middleware.HeaderSessionMiddleware, but its not clear to me if I am suppose to use one or the other in my case.

Are you sure this can't be middleware related? Looking at the source of the middleware it seems it is doing something with cookies basket and session https://django-oscar-api.readthedocs.io/en/2.1.0/_modules/oscarapi/middleware.html

maerteijn commented 2 years ago

My only bet was the middleware, and there is another middleware mentioned in the docs

The ApiBasketMiddleWare should be used when you mix the regular django-oscar front-end with the api. This is because django-oscar uses a cookie to set the anonymous basket id, instead of using the server side session. This middleware makes sure that this won't cause any problems. This is not applicable in your case I guess as you don't use the django-oscar frontend at all.

The HeaderSessionMiddleware should be used when your front-end application already has a session mechanism so you don't want oscarapi to do so as well. In this case you would send http headers with a session id to each request of oscarapi so oscarapi will identify the session that way. You should only use this if you have some session mechanism in your front-end application and when oscarapi is NOT exposed to the web. This could be a CMS system, internal system etc.

So what is happening somehow in your situation is that session cookies are not unique per visitor. I can't really say what causes this. Some extensive testing and logging would be needed I think.

Are you using nuxt purely client side (in the browser), or also on the server? If you are using it server side, then I can already say there is your session problem as then you will have browser <-> your nuxt app <-> oscarapi with no guaranteed unique session throughout the complete stack. (at least not without making sure http headers are send back and forth between oscar-api and your nuxt application)

ladrua commented 2 years ago

Are you using nuxt purely client side (in the browser), or also on the server? This is a good point. We do both, but we never do anything basket related server-side. Just fetching product data to generate content for search engines, sharing and the usual. I'll take a deeper look to make sure there are no such cases that we fetch basket or anything like that server side, but I am pretty confident we are not.

So back to your explanation about the middlewares. So are you saying that in my case we wouldnt need a middleware at all?

maerteijn commented 2 years ago

@ladrua You’ll need the basket middleware of oscar itself, but not any of the middlewares described above, unless your situation fits as described.