Hi Isura, On Mon, Sep 4, 2017 at 9:35 PM, Isura Karunaratne <[email protected]> wrote:
> Hi Johann, > > On Mon, Sep 4, 2017 at 8:18 PM Johann Nallathamby <[email protected]> wrote: > >> Hi Hasanthi/Nuwandi/IAM Team, >> >> 1. Can we please add a description in the JIRA as to what this JIRA is >> for? >> >> 2. The fix has made a public enum change: >> "MAX_ATTEMTS_EXCEEDED" -> "MAX_ATTEMTS_EXCEEDED". >> Is this intentional? In any case the spelling is still wrong. >> >> 3. We have introduced a new protected method " >> setUserClaimsValuesInUserStore". Again is this intentional? And we have >> a threadlocal solution to prevent listenered being triggered twice. In that >> case do we need this new method? >> > > Here we are going to support account locking failure reason. In that case, > we need a way to identify following account lock reasons separately. > > - Admin Lock User Account > - Account not confirmed > - Account locked due to exceeding max failure attempts > > We have to check account lock claim in setUserClaimValues method to check > whether admin user is going to lock a user. Since the recursion in > UserStoreBasedIdentityDaaStore, we can't put that logic inside > setUserClaimValues method, because we use setUserClaimValues method to > store the reason for other scenarios as well. > Simply we could check for the appropriate conditions using both the account lock claim and the lock reason claim. This way we can differentiate between the different conditions. This isn't that complicated is it? I know with multiple if-else conditions this becomes a bit confusing for people, but that is a problem in our pre IS 5.3.0 where all the code was written in the listener (code to handle various scenarios and code to persist identity claims). In IS 5.3.0 we have separated out the persisting logic and the logic to handle various identity management scenarios to different handler classes. So it should be much more simpler to understand. Why I don't like the current fix is that it is purposely skipping the listeners. We had the same solutions in IS 5.0.0 and removed it to avoid skipping listeners by down casting to the specific UserStoreManagers, because then the solution doesn't work for custom user store managers. Now we are going back to the same solution. This issue is simple logic error. I don't think we need to do such a change and go back to something we removed earlier sighting other bigger concerns. I already discussed this offline with *@Farasath.* He knows what I am talking about. Shall we change the fix and not change the model altogether? Btw, did we test the same in the new implementation in IS 5.3.0? How do it do there? Regards, Johann. > > > Thanks > Isura. > > >> [1] https://wso2.org/jira/browse/IDENTITY-6324 >> >> Thanks & Regards, >> Johann. >> >> -- >> >> *Johann Dilantha Nallathamby* >> Senior Lead Solutions Engineer >> WSO2, Inc. >> lean.enterprise.middleware >> >> Mobile - *+94777776950* >> Blog - *http://nallaa.wordpress.com <http://nallaa.wordpress.com>* >> > -- > > *Isura Dilhara Karunaratne* > Associate Technical Lead | WSO2 > Email: [email protected] > Mob : +94 772 254 810 <+94%2077%20225%204810> > Blog : http://isurad.blogspot.com/ > > > > -- Thanks & Regards, *Johann Dilantha Nallathamby* Senior Lead Solutions Engineer WSO2, Inc. lean.enterprise.middleware Mobile - *+94777776950* Blog - *http://nallaa.wordpress.com <http://nallaa.wordpress.com>*
_______________________________________________ Dev mailing list [email protected] http://wso2.org/cgi-bin/mailman/listinfo/dev
