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/AccessTokenServiceImpl.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/AbstractTransactionalLogic.java#L29

Regards.

[1] https://github.com/apache/syncope/blob/master/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AccessTokenDataBinderImpl.java#L119

Best Regards
Isuranga Perera

On Mon, Apr 9, 2018 at 11:42 AM, Francesco Chicchiriccò <ilgro...@apache.org <mailto:ilgro...@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/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AccessTokenDataBinderImpl.java#L104
        
<https://github.com/apache/syncope/blob/master/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AccessTokenDataBinderImpl.java#L104>

        [2]
        
https://github.com/apache/syncope/blob/master/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AccessTokenDataBinderImpl.java#L113
        
<https://github.com/apache/syncope/blob/master/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/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/

Reply via email to