[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16437160#comment-16437160 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user IsurangaPerera closed the pull request at: https://github.com/apache/syncope/pull/70 > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Assignee: Francesco Chicchiriccò >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16436947#comment-16436947 ] ASF subversion and git services commented on SYNCOPE-1301: -- Commit 6fd572119b1a91029962a7b36ca7f372ad2204a5 in syncope's branch refs/heads/2_0_X from [~ilgrosso] [ https://git-wip-us.apache.org/repos/asf?p=syncope.git;h=6fd5721 ] [SYNCOPE-1301] Reworking logic to avoid conficts > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16436948#comment-16436948 ] ASF subversion and git services commented on SYNCOPE-1301: -- Commit 24f789932141ee05fa12d81eca9d43178953f508 in syncope's branch refs/heads/master from [~ilgrosso] [ https://git-wip-us.apache.org/repos/asf?p=syncope.git;h=24f7899 ] [SYNCOPE-1301] Reworking logic to avoid conficts > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435830#comment-16435830 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user IsurangaPerera commented on the issue: https://github.com/apache/syncope/pull/70 Yes, that's why we have to see it from a different perspective. The initial state doesn't change the logic of replaceExisting == true But it partially affects replaceExisting == False (This affects only if the thread safe problems occur). This means when the issue syncope-1301 occurs instead of creating a new token for the same user this will replace the existing one (rare situation) > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435813#comment-16435813 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on the issue: https://github.com/apache/syncope/pull/70 > That way replace won't work since it saves first (2 tokens exist.violate UNIQUE constraint) and deletes next. That way it will only delete the existing one which results in no token at all. You are right, but as you can see, in the [current implementation](https://github.com/apache/syncope/blob/master/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AccessTokenDataBinderImpl.java#L138-L142) save and delete do not happen under the same conditions. > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435808#comment-16435808 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user IsurangaPerera commented on the issue: https://github.com/apache/syncope/pull/70 That way replace won't work since it saves first (2 tokens exist.violate UNIQUE constraint) and deletes next. That way it will only delete the existing one which results in no token at all. > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435801#comment-16435801 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on the issue: https://github.com/apache/syncope/pull/70 I would stick with minimal changes, so this PR should only change `JPAAccessToken`. > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435795#comment-16435795 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on the issue: https://github.com/apache/syncope/pull/70 Ok so... > Are you simply proposing to add such constraint and leave the rest of the code as is? e.g. to change again this PR by reverting all changes and adding the UNIQUE constraint? > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435798#comment-16435798 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user IsurangaPerera commented on the issue: https://github.com/apache/syncope/pull/70 Yes, What if we revert it to the initial state? > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435792#comment-16435792 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user IsurangaPerera commented on the issue: https://github.com/apache/syncope/pull/70 A validation error will be raised in the worst case(As I see the thread safe problem won't just affect the login as we discussed Saml as well. So Unique constraint will only prevent those scenarios. the advantage is the validation error is the worst case. It won't negatively affect the flow of the system like in thrad safe problem(Users being blocked from login) > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435775#comment-16435775 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on the issue: https://github.com/apache/syncope/pull/70 @IsurangaPerera the only effect of UNIQUE constraint will be that a validation error is raised whenever a second AccessToken is created with same owner of an existing one. Are you simply proposing to add such constraint and leave the rest of the code as is? > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435756#comment-16435756 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user IsurangaPerera commented on the issue: https://github.com/apache/syncope/pull/70 @ilgrosso As I understand in SAML SP logic always replaced. So even when we logged, as usual, the access token may changed by SAML SP. So I can understand the importance of what replaceExisitng flag does. After imposing the UNIQUE constraint as in my implementation replaceExisting == true works as expected(always).But sometimes even if the flag is false the token may be replaced (scenario discussed in mail thread). But this is only when the same user tries to log in at the same time & thread not safe problem aise. Anyway this approach is far better than using locks which causes performance drop and this is a rare case as well. What do you think? > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435748#comment-16435748 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on the issue: https://github.com/apache/syncope/pull/70 Ok, let's try this way: please re-introduce the UNIQUE constraint but rework the whole `AccessTokenDataBinderImpl#create` to comply with the two invocation scenarios as reported above. Do you think it could be feasible? > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435743#comment-16435743 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on the issue: https://github.com/apache/syncope/pull/70 @IsurangaPerera I don't remember the details, but what I can see from the source three is that `AccessTokenDataBinderImpl#create` is invoked in two places: 1. https://github.com/apache/syncope/blob/master/core/logic/src/main/java/org/apache/syncope/core/logic/AccessTokenLogic.java#L84 (plain login) 1. https://github.com/apache/syncope/blob/master/ext/saml2sp/logic/src/main/java/org/apache/syncope/core/logic/SAML2SPLogic.java#L549 (SAML 2.0 login) The former provides `replaceExisting` as `false`, the latter as `true`. From the code in `AccessTokenDataBinderImpl#create` I can see that: * for plain login, JWT is generated only at first invocation * for SAML 2.0 login, JWT is generated at every invocation, and existing JWT is replaced if existing Moreover, I cannot recall exactly why the UNIQUE constraint is not imposed to AccessToken's `owner`. > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435728#comment-16435728 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user IsurangaPerera commented on the issue: https://github.com/apache/syncope/pull/70 @ilgrosso Can I know the business rule associated with the access token. Can one user has more than one access token in an any given scenario? > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435715#comment-16435715 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on the issue: https://github.com/apache/syncope/pull/70 @IsurangaPerera the point is that the existence of `replaceFlag` (I did not realize that at first) is what prevents to enforce the UNIQUE constraint. > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435713#comment-16435713 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user IsurangaPerera commented on the issue: https://github.com/apache/syncope/pull/70 @ilgrosso without enforcing the UNIQUE constraint on the owner as we discussed in the mail I can't see a valid reason for merge these changes > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435686#comment-16435686 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181103559 --- Diff: core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPAAccessTokenDAO.java --- @@ -115,6 +115,16 @@ public AccessToken save(final AccessToken accessToken) { return entityManager().merge(accessToken); } +@Override +public AccessToken replace(final AccessToken accessToken) { +AccessToken existing = findByOwner(accessToken.getOwner()); +if (existing != null) { +delete(existing.getKey()); +} + +return save(accessToken); --- End diff -- `return replaceExisting ? save(accessToken) : existing;` > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435683#comment-16435683 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181103053 --- Diff: core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/dao/AccessTokenDAO.java --- @@ -34,6 +34,8 @@ AccessToken save(AccessToken accessToken); +AccessToken replace(AccessToken accessToken); --- End diff -- This should be ``` AccessToken replace(boolean replaceExisting, AccessToken accessToken); ``` > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435684#comment-16435684 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181103181 --- Diff: core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPAAccessTokenDAO.java --- @@ -115,6 +115,16 @@ public AccessToken save(final AccessToken accessToken) { return entityManager().merge(accessToken); } +@Override +public AccessToken replace(final AccessToken accessToken) { +AccessToken existing = findByOwner(accessToken.getOwner()); +if (existing != null) { --- End diff -- `if (replaceExisting && existing != null) {` > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435677#comment-16435677 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181102107 --- Diff: core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AccessTokenDataBinderImpl.java --- @@ -135,11 +135,7 @@ accessToken.setAuthorities(authorities); } -accessTokenDAO.save(accessToken); -} - -if (replaceExisting && existing != null) { -accessTokenDAO.delete(existing); +accessTokenDAO.merge(accessToken); --- End diff -- both: remove `unique = true` (e.g. revert all changes there, as said) AND keep the same code > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435673#comment-16435673 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user IsurangaPerera commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181101464 --- Diff: core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AccessTokenDataBinderImpl.java --- @@ -135,11 +135,7 @@ accessToken.setAuthorities(authorities); } -accessTokenDAO.save(accessToken); -} - -if (replaceExisting && existing != null) { -accessTokenDAO.delete(existing); +accessTokenDAO.merge(accessToken); --- End diff -- so you're suggesting keep this code segment intact right? if (replaceExisting && existing != null) { accessTokenDAO.delete(existing); } or remove unique = true from the model? > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435671#comment-16435671 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181100854 --- Diff: core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/entity/JPAAccessToken.java --- @@ -44,7 +44,7 @@ @Temporal(TemporalType.TIMESTAMP) private Date expiryTime; -@Column(nullable = true) +@Column(nullable = true, unique = true) --- End diff -- as said, revert any change here > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435668#comment-16435668 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181100663 --- Diff: core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPAAccessTokenDAO.java --- @@ -115,6 +115,16 @@ public AccessToken save(final AccessToken accessToken) { return entityManager().merge(accessToken); } +@Override +public AccessToken replace(final AccessToken accessToken) { +AccessToken existing = findByOwner(accessToken.getOwner()); +if (existing != null) { +delete(existing.getKey()); +} + +return entityManager().merge(accessToken); --- End diff -- replace `entityManager().merge(accessToken)` with `save(accesToken)` > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435665#comment-16435665 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181100023 --- Diff: core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AccessTokenDataBinderImpl.java --- @@ -135,11 +135,7 @@ accessToken.setAuthorities(authorities); } -accessTokenDAO.save(accessToken); -} - -if (replaceExisting && existing != null) { -accessTokenDAO.delete(existing); +accessTokenDAO.merge(accessToken); --- End diff -- That's why we are not enforcing the unique constraint (hence, NOT changing `@Column(nullable = true)`: the `replaceExisting` flag exists for a reason, do not alter such behavior as there might be some code relying on it. > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435663#comment-16435663 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user IsurangaPerera commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181099396 --- Diff: core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AccessTokenDataBinderImpl.java --- @@ -135,11 +135,7 @@ accessToken.setAuthorities(authorities); } -accessTokenDAO.save(accessToken); -} - -if (replaceExisting && existing != null) { -accessTokenDAO.delete(existing); +accessTokenDAO.merge(accessToken); --- End diff -- Actually that way there can be 2 tokens at a given moment. Suppose there exist an access token already. when trying to replace it in the old way it creates and saves another token (at this time there is 2 token which is against the unique constraint). > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435650#comment-16435650 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181097338 --- Diff: core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AccessTokenDataBinderImpl.java --- @@ -135,11 +135,7 @@ accessToken.setAuthorities(authorities); } -accessTokenDAO.save(accessToken); -} - -if (replaceExisting && existing != null) { -accessTokenDAO.delete(existing); +accessTokenDAO.merge(accessToken); --- End diff -- To me, it should be enough to pass the `replaceExisting` flag to the new `AccessTokenDAO#replace` method, and behave accordingly. > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435645#comment-16435645 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181096467 --- Diff: core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AccessTokenDataBinderImpl.java --- @@ -135,11 +135,7 @@ accessToken.setAuthorities(authorities); } -accessTokenDAO.save(accessToken); -} - -if (replaceExisting && existing != null) { -accessTokenDAO.delete(existing); +accessTokenDAO.merge(accessToken); --- End diff -- ...but there might be cases where such constraint should not apply (see the `@Column` annotation we've been discussing). > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435642#comment-16435642 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user IsurangaPerera commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181094994 --- Diff: core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AccessTokenDataBinderImpl.java --- @@ -135,11 +135,7 @@ accessToken.setAuthorities(authorities); } -accessTokenDAO.save(accessToken); -} - -if (replaceExisting && existing != null) { -accessTokenDAO.delete(existing); +accessTokenDAO.merge(accessToken); --- End diff -- exactly. this is to assure that there can be only one access token associated with a particular user at an any given time and also a workaround to overcome thread not safe problem without affecting the performance(without using serialized isolation) > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435631#comment-16435631 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181093708 --- Diff: core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AccessTokenDataBinderImpl.java --- @@ -135,11 +135,7 @@ accessToken.setAuthorities(authorities); } -accessTokenDAO.save(accessToken); -} - -if (replaceExisting && existing != null) { -accessTokenDAO.delete(existing); +accessTokenDAO.merge(accessToken); --- End diff -- So, the current implementation will replace the token only if `replaceExisting` is true; with your change, the token will be replaced anyway, because `JPAAccessTokenDAO#merge` will do ``` if (existing != null) { entityManager().remove(existing); } ``` while the current code does ``` if (replaceExisting && existing != null) { accessTokenDAO.delete(existing); } ``` > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435630#comment-16435630 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181093676 --- Diff: core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPAAccessTokenDAO.java --- @@ -115,6 +115,16 @@ public AccessToken save(final AccessToken accessToken) { return entityManager().merge(accessToken); } +@Override +@Transactional(rollbackFor = Throwable.class) +public void merge(final AccessToken accessToken) { +AccessToken existing = findByOwner(accessToken.getOwner()); +if (existing != null) { +entityManager().remove(existing); +} +entityManager().persist(accessToken); --- End diff -- Also, replace `entityManager().remove(existing)` with `delete(existing)`, thanks. > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435623#comment-16435623 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181091780 --- Diff: core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/dao/AccessTokenDAO.java --- @@ -34,6 +34,8 @@ AccessToken save(AccessToken accessToken); +void merge(AccessToken accessToken); --- End diff -- maybe `replace()` makes more sense > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435621#comment-16435621 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user IsurangaPerera commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181091428 --- Diff: core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/dao/AccessTokenDAO.java --- @@ -34,6 +34,8 @@ AccessToken save(AccessToken accessToken); +void merge(AccessToken accessToken); --- End diff -- Shall I keep the method name same(merge) or change it? > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435597#comment-16435597 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user IsurangaPerera commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181090557 --- Diff: core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AccessTokenDataBinderImpl.java --- @@ -135,11 +135,7 @@ accessToken.setAuthorities(authorities); } -accessTokenDAO.save(accessToken); -} - -if (replaceExisting && existing != null) { -accessTokenDAO.delete(existing); +accessTokenDAO.merge(accessToken); --- End diff -- It is not completely ignored. it is checked by the second if condition as before > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435594#comment-16435594 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181089858 --- Diff: core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/entity/JPAAccessToken.java --- @@ -44,7 +44,7 @@ @Temporal(TemporalType.TIMESTAMP) private Date expiryTime; -@Column(nullable = true) +@Column(unique = true) --- End diff -- Just revert, I know `nullable = true` is the default, but I don't see reasons to change. > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435590#comment-16435590 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181089640 --- Diff: core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AccessTokenDataBinderImpl.java --- @@ -135,11 +135,7 @@ accessToken.setAuthorities(authorities); } -accessTokenDAO.save(accessToken); -} - -if (replaceExisting && existing != null) { -accessTokenDAO.delete(existing); +accessTokenDAO.merge(accessToken); --- End diff -- `replaceExisting` is an argument of `AccesstTokenDataBinderImpl#create` that your change would simply ignore. Not good: it must be checked. > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435588#comment-16435588 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user IsurangaPerera commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181089295 --- Diff: core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/entity/JPAAccessToken.java --- @@ -44,7 +44,7 @@ @Temporal(TemporalType.TIMESTAMP) private Date expiryTime; -@Column(nullable = true) +@Column(unique = true) --- End diff -- I think in JPA it is redundant to use nullable = true this is the default scenario or all columns in JPA. Should I still change it? > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435584#comment-16435584 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181088731 --- Diff: core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/entity/JPAAccessToken.java --- @@ -44,7 +44,7 @@ @Temporal(TemporalType.TIMESTAMP) private Date expiryTime; -@Column(nullable = true) +@Column(unique = true) --- End diff -- As said during the initial mail exchange, I am not sure of the reason why owner is currently set as nullable, and I don't see enough reasons to change it, please revert. > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435585#comment-16435585 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user IsurangaPerera commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181088841 --- Diff: core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AccessTokenDataBinderImpl.java --- @@ -135,11 +135,7 @@ accessToken.setAuthorities(authorities); } -accessTokenDAO.save(accessToken); -} - -if (replaceExisting && existing != null) { -accessTokenDAO.delete(existing); +accessTokenDAO.merge(accessToken); --- End diff -- yes, merge function simply ensure the uniqueness constraint by checking if the token exists and replace it if so. otherwise, it will create a new register. so no need to perform this check here > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435582#comment-16435582 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181088154 --- Diff: core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/AccessTokenDataBinderImpl.java --- @@ -135,11 +135,7 @@ accessToken.setAuthorities(authorities); } -accessTokenDAO.save(accessToken); -} - -if (replaceExisting && existing != null) { -accessTokenDAO.delete(existing); +accessTokenDAO.merge(accessToken); --- End diff -- what about the `replaceExistingFlag`? You just removed it! > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435578#comment-16435578 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181087678 --- Diff: core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/dao/AccessTokenDAO.java --- @@ -34,6 +34,8 @@ AccessToken save(AccessToken accessToken); +void merge(AccessToken accessToken); --- End diff -- Change it into ``` AccessToken merge(AccessToken accessToken); ``` > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435576#comment-16435576 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181087221 --- Diff: core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPAAccessTokenDAO.java --- @@ -115,6 +115,16 @@ public AccessToken save(final AccessToken accessToken) { return entityManager().merge(accessToken); } +@Override +@Transactional(rollbackFor = Throwable.class) +public void merge(final AccessToken accessToken) { +AccessToken existing = findByOwner(accessToken.getOwner()); +if (existing != null) { +entityManager().remove(existing); +} +entityManager().persist(accessToken); --- End diff -- replace with `save(accessToken)` and make this method return `AccessToken` as `save()` does. > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435575#comment-16435575 ] ASF GitHub Bot commented on SYNCOPE-1301: - Github user ilgrosso commented on a diff in the pull request: https://github.com/apache/syncope/pull/70#discussion_r181086944 --- Diff: core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPAAccessTokenDAO.java --- @@ -115,6 +115,16 @@ public AccessToken save(final AccessToken accessToken) { return entityManager().merge(accessToken); } +@Override +@Transactional(rollbackFor = Throwable.class) --- End diff -- Please remove this annotation, it is redundant.. > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe
[ https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435567#comment-16435567 ] ASF GitHub Bot commented on SYNCOPE-1301: - GitHub user IsurangaPerera opened a pull request: https://github.com/apache/syncope/pull/70 [SYNCOPE-1301] fixed You can merge this pull request into a Git repository by running: $ git pull https://github.com/IsurangaPerera/syncope SYNCOPE-1301 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/syncope/pull/70.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #70 commit 0c9d91fa51544467e86d6427d9009306f05b1cde Author: Isuranga Perera Date: 2018-04-12T13:41:46Z [SYNCOPE-1301] fixed > Token creation is not threadsafe > > > Key: SYNCOPE-1301 > URL: https://issues.apache.org/jira/browse/SYNCOPE-1301 > Project: Syncope > Issue Type: Bug > Components: core >Affects Versions: 2.0.8 >Reporter: Isuranga Perera >Priority: Major > Fix For: 2.0.9, 2.1.0 > > > Token create method in AccessTokenDataBinderImpl[1] is not thread safe. 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). > [1] > [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] -- This message was sent by Atlassian JIRA (v7.6.3#76005)