syncope-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Francesco Chicchiriccò <ilgro...@apache.org>
Subject Re: Token creation is not thread safe
Date Mon, 09 Apr 2018 07:42:11 GMT
On 09/04/2018 09:30, Isuranga Perera wrote:
> Hi Francesco,
>
> Yes there is @Transactional annotation. But it haven't set the isolation
> property as well as the propagation property. Based on the default values
> set this thread safe problem will still occur. Please correct me if I'm
> wrong.

The transaction isolation level is set in

https://github.com/apache/syncope/blob/master/core/persistence-jpa/src/main/resources/domains/MasterDomain.xml#L57-L59

Regards.

> On Mon, Apr 9, 2018 at 12:20 PM, Francesco Chicchiriccò <ilgrosso@apache.org>
wrote:
>
>> On 09/04/2018 08:46, Isuranga Perera wrote:
>>
>>> Hi Francesco,
>>>
>>> I will take a scenario. Suppose a scenario where thread A & thread B try
>>> to login user admin.
>>>
>>>   1. thread A checks if a token exist for the user admin (suppose
>>>      currently there is no token associated with the admin)
>>>   2. Then thread A execute following logic[1] to create and save the token.
>>>   3. Before thread A save the token for user admin thread B checks if a
>>>      token exist for user admin (since the toked created by thread A is
>>>      not yet saved *exist == null*)
>>>   4. Then thread A complete the creation of token (and saving)
>>>   5. Thread B also complete the creation and saving of the token.
>>>
>>> That way there can be 2 tokens for a particular user.
>>>
>>>
>> You analysis does not take into any account the fact of the constraints
>> imposed by the @Service annotation in
>>
>> https://github.com/apache/syncope/blob/master/core/rest-cxf/
>> src/main/java/org/apache/syncope/core/rest/cxf/service/Acces
>> sTokenServiceImpl.java#L35
>>
>> (e.g. the place when external requests can come in) nor by the
>> @Transactional annotation injected into
>>
>> https://github.com/apache/syncope/blob/master/core/logic/
>> src/main/java/org/apache/syncope/core/logic/AccessTokenLogic.java#L80
>>
>> via
>>
>> https://github.com/apache/syncope/blob/master/core/logic/
>> src/main/java/org/apache/syncope/core/logic/AbstractTra
>> nsactionalLogic.java#L29
>>
>> Regards.
>>
>> [1] https://github.com/apache/syncope/blob/master/core/provision
>>> ing-java/src/main/java/org/apache/syncope/core/provisioni
>>> ng/java/data/AccessTokenDataBinderImpl.java#L119
>>>
>>> Best Regards
>>> Isuranga Perera
>>>
>>> On Mon, Apr 9, 2018 at 11:42 AM, Francesco Chicchiriccò <
>>> ilgrosso@apache.org <mailto:ilgrosso@apache.org>> wrote:
>>>
>>>      On 09/04/2018 07:07, Isuranga Perera wrote:
>>>
>>>          Hi All,
>>>
>>>          Token create method in AccessTokenDataBinderImpl[1] is not
>>>          thread safe.
>>>
>>>
>>>      Could you please explain why you're affirming this?
>>>
>>>          This could result in several problems including
>>>
>>>            * Exist 2 different access token for a particular user at a
>>>          given
>>>              time which may result in an exception thrown by method call[2]
>>>              since it expects a single token a given user.
>>>
>>>          In addition to that token replace is implemented as a
>>>          combination of 2 different functionalities. Since the method
>>>          is not thread safe this may cause some unexpected behaviors
>>>          (since there can be 2 tokens exist for a particular user. same
>>>          scenario as above).
>>>
>>>          Appreciate your insight on the $subject.
>>>
>>>
>>>          [1]
>>>          https://github.com/apache/syncope/blob/master/core/provision
>>> ing-java/src/main/java/org/apache/syncope/core/provisioni
>>> ng/java/data/AccessTokenDataBinderImpl.java#L104
>>>          <https://github.com/apache/syncope/blob/master/core/provisio
>>> ning-java/src/main/java/org/apache/syncope/core/provisioni
>>> ng/java/data/AccessTokenDataBinderImpl.java#L104>
>>>
>>>          [2]
>>>          https://github.com/apache/syncope/blob/master/core/provision
>>> ing-java/src/main/java/org/apache/syncope/core/provisioni
>>> ng/java/data/AccessTokenDataBinderImpl.java#L113
>>>          <https://github.com/apache/syncope/blob/master/core/provisio
>>> ning-java/src/main/java/org/apache/syncope/core/provisioni
>>> ng/java/data/AccessTokenDataBinderImpl.java#L113>
>>>
>>>          Best Regards
>>>          Isuranga Perera

-- 
Francesco Chicchiriccò

Tirasa - Open Source Excellence
http://www.tirasa.net/

Member at The Apache Software Foundation
Syncope, Cocoon, Olingo, CXF, OpenJPA, PonyMail
http://home.apache.org/~ilgrosso/


Mime
View raw message