Default 'User Workflow' issue

[varontron]

Hi,Executed the following:
 In 2.0.9-SNAPSHOT (no extensions), created some users, some in ldap and
pulled, some in Syncope and pushed. All have status = 'Created'  
 Rebuilt and installed 2.0.9-SNAPSHOT with '-P all' to include extensions  
 Created a new user successfully in console, assuming workflow took over,
and user has status = 'Active'
 Could not change any original user's status to 'Active' (how?)  
 Attempted to delete users in order to re-add, and incurred error alerts in
console  
 Tried a few things, eventually noticed users were successfully deleted from
ldap  
 Users still exist in syncope with status = 'Created' and no user-task is
executable  
 Can't delete users in syncope, due to 'Empty workflow id'  
 Pulling changes from ldap isn't working either perhaps bc of configs.  
Is there any option other than dropping db and reloading?  
What is the prescribed method if deploying without Activiti, then say,
adding 100s of users, and then enabling Activiti?  Is there a prescribed
import/export or something?
>From console.logfrom core-rest.log

-
http://analgesicsolutions.com
https://github.com/Novartis/YADA

--
Sent from: http://syncope-dev.1063484.n5.nabble.com/

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope issue #70: [SYNCOPE-1301] fixed

[IsurangaPerera]

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)


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope issue #70: [SYNCOPE-1301] fixed

[ilgrosso]

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.



---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope issue #70: [SYNCOPE-1301] fixed

[IsurangaPerera]

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.


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope issue #70: [SYNCOPE-1301] fixed

[ilgrosso]

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`.


---

[GitHub] syncope issue #70: [SYNCOPE-1301] fixed

[IsurangaPerera]

Github user IsurangaPerera commented on the issue:

https://github.com/apache/syncope/pull/70
  
Yes, What if we revert it to the initial state?


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope issue #70: [SYNCOPE-1301] fixed

[ilgrosso]

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?


---

[GitHub] syncope issue #70: [SYNCOPE-1301] fixed

[IsurangaPerera]

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)


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope issue #70: [SYNCOPE-1301] fixed

[ilgrosso]

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?


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope issue #70: [SYNCOPE-1301] fixed

[IsurangaPerera]

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?


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope issue #70: [SYNCOPE-1301] fixed

[ilgrosso]

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?


---

[GitHub] syncope issue #70: [SYNCOPE-1301] fixed

[ilgrosso]

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`.


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope issue #70: [SYNCOPE-1301] fixed

[IsurangaPerera]

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?


---

[jira] [Resolved] (SYNCOPE-1302) New expression model in mapping for internal attributes to access user relationships

[Matteo Alessandroni (JIRA)]

 [ 
https://issues.apache.org/jira/browse/SYNCOPE-1302?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Matteo Alessandroni resolved SYNCOPE-1302.
--
Resolution: Fixed

> New expression model in mapping for internal attributes to access user 
> relationships
> 
>
> Key: SYNCOPE-1302
> URL: https://issues.apache.org/jira/browse/SYNCOPE-1302
> Project: Syncope
>  Issue Type: Improvement
>  Components: console, core
>Reporter: Matteo Alessandroni
>Assignee: Matteo Alessandroni
>Priority: Minor
> Fix For: 2.0.9, 2.1.0
>
>
> It would be:
> {{relationships[relationshipType][relationshipAnyType].schema}}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope issue #70: [SYNCOPE-1301] fixed

[ilgrosso]

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.


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope issue #70: [SYNCOPE-1301] fixed

[IsurangaPerera]

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


---

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[ilgrosso]

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;`


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[ilgrosso]

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) {`


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[ilgrosso]

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


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[IsurangaPerera]

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?


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[ilgrosso]

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


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[ilgrosso]

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)`


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[ilgrosso]

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.


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[IsurangaPerera]

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). 


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[ilgrosso]

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.


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[ilgrosso]

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).


---

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[IsurangaPerera]

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)


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[ilgrosso]

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.


---

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[ilgrosso]

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);
}
```


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[ilgrosso]

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


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[IsurangaPerera]

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?


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[IsurangaPerera]

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


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[ilgrosso]

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.


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[ilgrosso]

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.


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[IsurangaPerera]

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?


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[ilgrosso]

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.


---

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[IsurangaPerera]

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


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[ilgrosso]

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!


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[ilgrosso]

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);
```


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[ilgrosso]

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.


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[ilgrosso]

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..


---

[jira] [Commented] (SYNCOPE-1301) Token creation is not threadsafe

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)

[GitHub] syncope pull request #70: [SYNCOPE-1301] fixed

[IsurangaPerera]

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




---

[jira] [Commented] (SYNCOPE-1302) New expression model in mapping for internal attributes to access user relationships

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16435542#comment-16435542
 ] 

ASF subversion and git services commented on SYNCOPE-1302:
--

Commit 02b6d4eb6f6d0eaa993e00ef6d86c9f7c4411850 in syncope's branch 
refs/heads/2_0_X from [~skylark17]
[ https://git-wip-us.apache.org/repos/asf?p=syncope.git;h=02b6d4e ]

[SYNCOPE-1302] Added new expression model in mapping for internal attributes to 
access user relationships


> New expression model in mapping for internal attributes to access user 
> relationships
> 
>
> Key: SYNCOPE-1302
> URL: https://issues.apache.org/jira/browse/SYNCOPE-1302
> Project: Syncope
>  Issue Type: Improvement
>  Components: console, core
>Reporter: Matteo Alessandroni
>Assignee: Matteo Alessandroni
>Priority: Minor
> Fix For: 2.0.9, 2.1.0
>
>
> It would be:
> {{relationships[relationshipType][relationshipAnyType].schema}}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (SYNCOPE-1299) Manual reconciliation

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1299?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16435329#comment-16435329
 ] 

ASF subversion and git services commented on SYNCOPE-1299:
--

Commit e5860a76a5584edf780df180eb9229ddf44de684 in syncope's branch 
refs/heads/master from [~ilgrosso]
[ https://git-wip-us.apache.org/repos/asf?p=syncope.git;h=e5860a7 ]

[SYNCOPE-1299] Core implementation


> Manual reconciliation
> -
>
> Key: SYNCOPE-1299
> URL: https://issues.apache.org/jira/browse/SYNCOPE-1299
> Project: Syncope
>  Issue Type: Improvement
>  Components: console, core
>Reporter: Francesco Chicchiriccò
>Assignee: Francesco Chicchiriccò
>Priority: Major
> Fix For: 2.0.9, 2.1.0
>
>
> Provide the feature - in Admin Console from either an User / Group / Any 
> Object and from an External Resource (under Topology) - which, given a User / 
> Group / Any Object and an External Resource, allows to force pushing or 
> pulling values for mapped attributes.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (SYNCOPE-1299) Manual reconciliation

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/SYNCOPE-1299?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16435328#comment-16435328
 ] 

ASF subversion and git services commented on SYNCOPE-1299:
--

Commit 73fab3dee3f95029bc210a16f3c0bd57814e0920 in syncope's branch 
refs/heads/2_0_X from [~ilgrosso]
[ https://git-wip-us.apache.org/repos/asf?p=syncope.git;h=73fab3d ]

[SYNCOPE-1299] Core implementation


> Manual reconciliation
> -
>
> Key: SYNCOPE-1299
> URL: https://issues.apache.org/jira/browse/SYNCOPE-1299
> Project: Syncope
>  Issue Type: Improvement
>  Components: console, core
>Reporter: Francesco Chicchiriccò
>Assignee: Francesco Chicchiriccò
>Priority: Major
> Fix For: 2.0.9, 2.1.0
>
>
> Provide the feature - in Admin Console from either an User / Group / Any 
> Object and from an External Resource (under Topology) - which, given a User / 
> Group / Any Object and an External Resource, allows to force pushing or 
> pulling values for mapped attributes.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (SYNCOPE-1301) Token creation is not threadsafe

[Isuranga Perera (JIRA)]

Isuranga Perera created SYNCOPE-1301:


 Summary: 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
 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)