Openbravo Issue Tracking System - Openbravo ERP
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0035164Openbravo ERPA. Platformpublic2017-02-06 08:352017-09-21 16:49
Reporteralostale 
Assigned Tocaristu 
PriorityurgentSeveritymajorReproducibilityrandom
StatusclosedResolutionfixed 
PlatformOS5OS Version
Product Version 
Target VersionFixed in Version3.0PR17Q4 
Merge Request Status
Review Assigned Toalostale
OBNetwork customer
Web browser
ModulesCore
Support ticket
Regression level
Regression date
Regression introduced in release
Regression introduced by commit
Triggers an Emergency PackNo
Summary0035164: AuthenticationManager.username thread unsafe
DescriptionAuthenticationManager class has a member username which is set and accessed by different methods [1].

An instance of AuthenticationManager is kept in the HttpSecureAppServlet as a member [2] of the servlet. Therefore username can be accessed in an inconsistent state.

[1] https://code.openbravo.com/erp/devel/pi/file/0c91a26dca81/src/org/openbravo/base/secureApp/HttpSecureAppServlet.java#l83 [^]
[2] https://code.openbravo.com/erp/devel/pi/file/0c91a26dca81/src/org/openbravo/base/secureApp/HttpSecureAppServlet.java#l83 [^]
Steps To ReproduceWould need to create an artificial test to reproduce this behavior, though it possibly is reproducible in real cases.
Proposed Solution
Additional Information
TagsmultiThread
Relationships
related to design defect 0036808 new Triage Platform Base AuthenticationManager should be stateless 
depends on defect 0036807 closed caristu API Change: remove AuthenticationManager.username field 
blocks design defect 0034664 acknowledged Triage Platform Base multi thread unsafeties 
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2017-02-06 08:35alostaleNew Issue
2017-02-06 08:35alostaleAssigned To => platform
2017-02-06 08:35alostaleModules => Core
2017-02-06 08:35alostaleTriggers an Emergency Pack => No
2017-02-06 08:35alostaleRelationship addedblocks 0034664
2017-02-06 08:35alostaleTag Attached: multiThread
2017-02-06 08:35alostaleStatusnew => acknowledged
2017-09-07 11:50caristuAssigned Toplatform => caristu
2017-09-07 11:50caristuStatusacknowledged => scheduled
2017-09-07 11:59hgbotCheckin
2017-09-07 11:59hgbotNote Added: 0098883
2017-09-07 19:09caristuRelationship addeddepends on 0036807
2017-09-07 19:21caristuRelationship addedrelated to 0036808
2017-09-08 13:18caristuNote Added: 0098912
2017-09-08 13:18caristuStatusscheduled => resolved
2017-09-08 13:18caristuFixed in SCM revision => 282b45a9521e
2017-09-08 13:18caristuResolutionopen => fixed
2017-09-08 13:18caristuNote Edited: 0098912bug_revision_view_page.php?bugnote_id=0098912#r15853
2017-09-08 13:19caristuReview Assigned To => alostale
2017-09-08 13:20caristuNote Edited: 0098912bug_revision_view_page.php?bugnote_id=0098912#r15854
2017-09-08 13:32caristuFixed in SCM revision282b45a9521e => 282b45a9521ehttps://code.openbravo.com/erp/devel/pi/rev/282b45a9521e [^]
2017-09-08 13:32caristuFixed in SCM revision282b45a9521ehttps://code.openbravo.com/erp/devel/pi/rev/282b45a9521e [^] => https://code.openbravo.com/erp/devel/pi/rev/282b45a9521e [^]
2017-09-08 13:38caristuNote Edited: 0098912bug_revision_view_page.php?bugnote_id=0098912#r15855
2017-09-12 09:29hgbotCheckin
2017-09-12 09:29hgbotNote Added: 0098963
2017-09-12 09:34hgbotCheckin
2017-09-12 09:34hgbotNote Added: 0098966
2017-09-12 10:57alostaleNote Added: 0098972
2017-09-12 10:57alostaleStatusresolved => closed
2017-09-12 10:57alostaleFixed in Version => 3.0PR17Q4
2017-09-21 16:49hudsonbotCheckin
2017-09-21 16:49hudsonbotNote Added: 0099366
2017-09-21 16:49hudsonbotCheckin
2017-09-21 16:49hudsonbotNote Added: 0099370
2017-09-21 16:49hudsonbotCheckin
2017-09-21 16:49hudsonbotNote Added: 0099377
2017-09-21 16:49hudsonbotCheckin
2017-09-21 16:49hudsonbotNote Added: 0099378

Notes
(0098883)
hgbot   
2017-09-07 11:59   
Repository: erp/devel/pi
Changeset: d21362467e8bd529b903fff261f950bf8a75c693
Author: Carlos Aristu <carlos.aristu <at> openbravo.com>
Date: Thu Sep 07 11:56:46 2017 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/d21362467e8bd529b903fff261f950bf8a75c693 [^]

related to issue 35164: initial code improvements
  - Reorder the modifiers of the getAuthenticationManager method to comply with the Java Language Specification
  - Created new private constant for "#AD_SESSION_ID"
  - Use faster indexOf(char) method

---
M src/org/openbravo/authentication/AuthenticationManager.java
---
(0098912)
caristu   
2017-09-08 13:18   
(edited on: 2017-09-08 13:38)
Repository: erp/devel/pi
Changeset: 282b45a9521e
Author: Carlos Aristu <carlos.aristu <at> openbravo.com>
Date: Fri Sep 08 13:09:04 2017 +0200
URL: https://code.openbravo.com/erp/devel/pi/rev/282b45a9521e [^]

fixes issue 36807: remove AuthenticationManager.username field

---
M src/org/openbravo/authentication/AuthenticationManager.java
M src/org/openbravo/authentication/basic/DefaultAuthenticationManager.java
---
(0098963)
hgbot   
2017-09-12 09:29   
Repository: erp/devel/pi
Changeset: 93aa822ffff308bd1420fd928ba20c6157c579bc
Author: Carlos Aristu <carlos.aristu <at> openbravo.com>
Date: Tue Sep 12 09:21:01 2017 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/93aa822ffff308bd1420fd928ba20c6157c579bc [^]

related to issue 35164: code review improvements

  - Initialize the ThreadLocal variable (loginName) on every call to authenticate. Thus, we ensure that its value is cleaned up when Tomcat reuses a thread.
  - Use new UserLoginInfo to keep the login credentials instead of using a map.

---
M src/org/openbravo/authentication/AuthenticationManager.java
M src/org/openbravo/authentication/basic/DefaultAuthenticationManager.java
---
(0098966)
hgbot   
2017-09-12 09:34   
Repository: erp/devel/pi
Changeset: efcc5da08f96ef95133a3bfef76f08a35282dcbe
Author: Carlos Aristu <carlos.aristu <at> openbravo.com>
Date: Tue Sep 12 09:33:36 2017 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/efcc5da08f96ef95133a3bfef76f08a35282dcbe [^]

related to issue 35164: use correct param name in javadoc

---
M src/org/openbravo/authentication/AuthenticationManager.java
---
(0098972)
alostale   
2017-09-12 10:57   
reviewed + tested
(0099366)
hudsonbot   
2017-09-21 16:49   
A changeset related to this issue has been promoted main and to the
Central Repository, after passing a series of tests.

Promotion changeset: https://code.openbravo.com/erp/devel/main/rev/9750b78d3e5c [^]
Maturity status: Test
(0099370)
hudsonbot   
2017-09-21 16:49   
A changeset related to this issue has been promoted main and to the
Central Repository, after passing a series of tests.

Promotion changeset: https://code.openbravo.com/erp/devel/main/rev/9750b78d3e5c [^]
Maturity status: Test
(0099377)
hudsonbot   
2017-09-21 16:49   
A changeset related to this issue has been promoted main and to the
Central Repository, after passing a series of tests.

Promotion changeset: https://code.openbravo.com/erp/devel/main/rev/9750b78d3e5c [^]
Maturity status: Test
(0099378)
hudsonbot   
2017-09-21 16:49   
A changeset related to this issue has been promoted main and to the
Central Repository, after passing a series of tests.

Promotion changeset: https://code.openbravo.com/erp/devel/main/rev/9750b78d3e5c [^]
Maturity status: Test