Uploaded image for project: 'OFBiz'
  1. OFBiz
  2. OFBIZ-6561

GenericDelegator.store fails to clear userLogin cache on password update from ecommerce profile

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: Trunk
    • Fix Version/s: 16.11.01
    • Component/s: None
    • Labels:
      None

      Description

      Here is the steps to regenerate issue:

      Password successfully updated, while redirecting to profile page you will get following error message:


      org.ofbiz.widget.renderer.ScreenRenderException: Error rendering screen component://ecommerce/widget/CustomerScreens.xml#viewprofile: java.lang.IllegalArgumentException: Error running script at location [component://ecommerce/webapp/ecommerce/WEB-INF/actions/customer/ViewProfile.groovy]: org.ofbiz.service.ExecutionServiceException: You must be logged in to complete the [Get Summary Information About Orders for a Customer] process. (Error running script at location [component://ecommerce/webapp/ecommerce/WEB-INF/actions/customer/ViewProfile.groovy]: org.ofbiz.service.ExecutionServiceException: You must be logged in to complete the [Get Summary Information About Orders for a Customer] process.)

      1. OFBIZ-6561.patch
        0.8 kB
        Deepak Dixit
      2. Screen Shot 2015-07-22 at 2.12.47 PM.png
        112 kB
        Deepak Dixit

        Issue Links

          Activity

          Hide
          deepak.dixit Deepak Dixit added a comment -

          While digging into this issue found that In ServiceDispatcher.checkAuth method system perform security check on context userLogin, and system try to compare the context userLogin.currentPassword with db userLogin and if password does not match then it remove the userLogin from context.
          In this case system fetch the userLogin record from cache and it return the old password, hence compare password return false and it remove the userLogin from context.

          If we fetch the userLogin with cache false (ServiceDispatcher.java:897) then its working as expected.

          Show
          deepak.dixit Deepak Dixit added a comment - While digging into this issue found that In ServiceDispatcher.checkAuth method system perform security check on context userLogin, and system try to compare the context userLogin.currentPassword with db userLogin and if password does not match then it remove the userLogin from context. In this case system fetch the userLogin record from cache and it return the old password, hence compare password return false and it remove the userLogin from context. If we fetch the userLogin with cache false (ServiceDispatcher.java:897) then its working as expected.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Hi Deepak, I see no reasons why we could not change to cache false indeed

          Show
          jacques.le.roux Jacques Le Roux added a comment - Hi Deepak, I see no reasons why we could not change to cache false indeed
          Hide
          deepak.dixit Deepak Dixit added a comment - - edited

          We can do Jacques, but it will unnecessary increase an db call on each service invocation. If we set cache=false then its a for now fix not a proper fix, this issue related to cache refresh on update

          Show
          deepak.dixit Deepak Dixit added a comment - - edited We can do Jacques, but it will unnecessary increase an db call on each service invocation. If we set cache=false then its a for now fix not a proper fix, this issue related to cache refresh on update
          Hide
          deepak.dixit Deepak Dixit added a comment - - edited

          I think its due to transaction cache related issue, I added an condition in EntityCache.remove(GenericPK pk) method and its working fine, I am not sure if its the correct solution for this problem...

          Show
          deepak.dixit Deepak Dixit added a comment - - edited I think its due to transaction cache related issue, I added an condition in EntityCache.remove(GenericPK pk) method and its working fine, I am not sure if its the correct solution for this problem...
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Yes did not thought enough about indeed

          Show
          jacques.le.roux Jacques Le Roux added a comment - Yes did not thought enough about indeed
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          This issue is related to OFBIZ-5534, stable demo does not produce this error.

          It makes sense to me to not remove a pk from the tempCache when it's null and especially to not return null in this case. Only the try section is new in this method. So I believe your proposed change is safe Deepak, I did not check if it's optimal.

          Show
          jacques.le.roux Jacques Le Roux added a comment - This issue is related to OFBIZ-5534 , stable demo does not produce this error. It makes sense to me to not remove a pk from the tempCache when it's null and especially to not return null in this case. Only the try section is new in this method. So I believe your proposed change is safe Deepak, I did not check if it's optimal.
          Hide
          adrianc@hlmksw.com Adrian Crum added a comment -

          Reverting rev 1668267 will probably fix this issue. I have been meaning to do it, but I haven't had any time.

          Show
          adrianc@hlmksw.com Adrian Crum added a comment - Reverting rev 1668267 will probably fix this issue. I have been meaning to do it, but I haven't had any time.
          Hide
          deepak.dixit Deepak Dixit added a comment -

          Thanks Jacques and Adrian, I'll revert the r#1668267

          Show
          deepak.dixit Deepak Dixit added a comment - Thanks Jacques and Adrian, I'll revert the r#1668267
          Hide
          deepak.dixit Deepak Dixit added a comment -

          Reverted r1668267 and related changes done at 1674064, 1669537 at r#1693701.

          Show
          deepak.dixit Deepak Dixit added a comment - Reverted r1668267 and related changes done at 1674064, 1669537 at r#1693701.

            People

            • Assignee:
              deepak.dixit Deepak Dixit
              Reporter:
              deepak.dixit Deepak Dixit
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development