Re: [Dev] Custom UserStore works on 5.1.0, 5.2.0 not working in 5.3.0 and 5.4.0-M1

[Johann Nallathamby]

As I got to know from Hasintha offline the fix we have done for this issue
as of now has some limitations. Can whoever working on this issue please
explain the new fix we are going to do?

Regards,
Johann.

On Tue, Jul 4, 2017 at 7:30 AM, Johann Nallathamby  wrote:

>
>
> On Tue, Jul 4, 2017 at 9:59 AM, Hasintha Indrajee 
> wrote:
>
>> We will be maintaining user store property to indicate whether the user
>> store supports claim operations (There may be multiple operation types, but
>> in this case claim operations). Before setting claims in pre and post
>> authentication listeners we will check whether the underlying user store is
>> supported for claim operations and will only continue if it's supported.
>> Unless returns and continues the flow.
>>
>
> +1. I think this may be the best way as of now.
>
>
>>
>> There were few options discussed
>>
>> 1) We can just turn off these event handlers which get fired on
>> authentication events. But we may only need to skip a specific user store
>> and we may still need to use identity management functionality with rest of
>> the user stores. In this case we have to introduce a user tore level
>> property/configuration.
>>
>> 2) One of the other options is to expect a predefined run time exception
>> from the methods which are not implemented in User Store Manager  level and
>> handle them at our event handlers so that it will not have an effect on the
>> rest of the flow since we handle the run time exception. We thought of not
>> going with this since altering logic and taking decisions based on the type
>> of exception and error messages is not a good practice.
>>
>>
>> 3) Another possibility was to introduce above check (checking newly
>> introduced user store property and skipping) at the
>> IdentityMgtEventListener level so that none of the handlers will be fired
>> if the user store is not supported for that particular operation. This is
>> not a proper fix since we may still need to get events to event handler
>> level even if the user store is not supported certain operations. eg - Even
>> though the user store is not supported for claim operations the deployment
>> may be using JDBC based Identity Store. In that we don't need handlers to
>> be skipped.
>>
>> Hence the best option seems to be handling the behaviour based on newly
>> introduced property at specific concrete handler level. Depending on the
>> logic of specific handler it will decide whether to handle this event or
>> skip and continue.
>>
>> On Tue, Jul 4, 2017 at 9:35 AM, Sagara Gunathunga 
>> wrote:
>>
>>>
>>> Hasintha, could you please update the thread with the solution we agreed
>>> ?
>>>
>>>
>>> Thanks !
>>>
>>> On Wed, Jun 21, 2017 at 1:01 PM, Isura Karunaratne 
>>> wrote:
>>>
 Hi

 On Wed, Jun 21, 2017 at 11:06 AM, Farasath Ahamed 
 wrote:

>
>
>
>
> On Wed, Jun 21, 2017 at 11:03 AM, Isura Karunaratne 
> wrote:
>
>>
>>
>> On Tue, Jun 20, 2017 at 11:29 PM, Johann Nallathamby > > wrote:
>>
>>> If these two handlers are disabled by default there shouldn't be any
>>> problem. According to default identity-event.properties file they are
>>> disabled. How come they get triggered then?
>>>
>>
>> Yes. By default the account lock/disabled features are disabled. If
>> it is required to use account lock/disable features, there should be a 
>> way
>> to store user properties.
>>
>
> Looks like we haven't used the property to check whether the listener
> is enabled or disabled although we have defined in
> identity-event.properties. Therefore the handlers get fired on
> pre-authentications
>

 Yes. This issue is fixed with https://wso2.org/jira/browse/I
 DENTITY-6091

 Thanks
 Isura.

>
>
>>
>> Also, if the um_user_attribute table is not there, most of the use
>> cases will be broken. (Add User/ Update User/ Get  Users ...). So, I 
>> think
>> that user store is incomplete.
>>
>> Thanks
>> Isura.
>>
>>
>>>
>>> On Tue, Jun 20, 2017 at 7:25 PM, Farasath Ahamed >> > wrote:
>>>
 Hi,

 The minimum requirement to write a custom JDBC user store manager
 so far (before IS 5.3.0) was to simply override the doAuthenticate()
 method. So a custom user store that was written for 5.0.0 worked 
 without
 any modifications (may be dependency changes).

 But when we use the same code on IS 5.3.0, the custom user store
 implementations that only override the doAuthenticate() are broken 
 because
 account disabled[1] and account locked[2] handlers introduced in IS 
 5.3.0.

 These two handlers call the getUserClaimValues() method of the
 userstore to retrieve some claims. Since we haven't overridden the 
 method
 in custom userstore implementation it c

Re: [Dev] Custom UserStore works on 5.1.0, 5.2.0 not working in 5.3.0 and 5.4.0-M1

[Johann Nallathamby]

On Tue, Jul 4, 2017 at 9:59 AM, Hasintha Indrajee  wrote:

> We will be maintaining user store property to indicate whether the user
> store supports claim operations (There may be multiple operation types, but
> in this case claim operations). Before setting claims in pre and post
> authentication listeners we will check whether the underlying user store is
> supported for claim operations and will only continue if it's supported.
> Unless returns and continues the flow.
>

+1. I think this may be the best way as of now.


>
> There were few options discussed
>
> 1) We can just turn off these event handlers which get fired on
> authentication events. But we may only need to skip a specific user store
> and we may still need to use identity management functionality with rest of
> the user stores. In this case we have to introduce a user tore level
> property/configuration.
>
> 2) One of the other options is to expect a predefined run time exception
> from the methods which are not implemented in User Store Manager  level and
> handle them at our event handlers so that it will not have an effect on the
> rest of the flow since we handle the run time exception. We thought of not
> going with this since altering logic and taking decisions based on the type
> of exception and error messages is not a good practice.
>
>
> 3) Another possibility was to introduce above check (checking newly
> introduced user store property and skipping) at the
> IdentityMgtEventListener level so that none of the handlers will be fired
> if the user store is not supported for that particular operation. This is
> not a proper fix since we may still need to get events to event handler
> level even if the user store is not supported certain operations. eg - Even
> though the user store is not supported for claim operations the deployment
> may be using JDBC based Identity Store. In that we don't need handlers to
> be skipped.
>
> Hence the best option seems to be handling the behaviour based on newly
> introduced property at specific concrete handler level. Depending on the
> logic of specific handler it will decide whether to handle this event or
> skip and continue.
>
> On Tue, Jul 4, 2017 at 9:35 AM, Sagara Gunathunga  wrote:
>
>>
>> Hasintha, could you please update the thread with the solution we agreed
>> ?
>>
>>
>> Thanks !
>>
>> On Wed, Jun 21, 2017 at 1:01 PM, Isura Karunaratne 
>> wrote:
>>
>>> Hi
>>>
>>> On Wed, Jun 21, 2017 at 11:06 AM, Farasath Ahamed 
>>> wrote:
>>>




 On Wed, Jun 21, 2017 at 11:03 AM, Isura Karunaratne 
 wrote:

>
>
> On Tue, Jun 20, 2017 at 11:29 PM, Johann Nallathamby 
> wrote:
>
>> If these two handlers are disabled by default there shouldn't be any
>> problem. According to default identity-event.properties file they are
>> disabled. How come they get triggered then?
>>
>
> Yes. By default the account lock/disabled features are disabled. If it
> is required to use account lock/disable features, there should be a way to
> store user properties.
>

 Looks like we haven't used the property to check whether the listener
 is enabled or disabled although we have defined in
 identity-event.properties. Therefore the handlers get fired on
 pre-authentications

>>>
>>> Yes. This issue is fixed with https://wso2.org/jira/browse/IDENTITY-6091
>>>
>>> Thanks
>>> Isura.
>>>


>
> Also, if the um_user_attribute table is not there, most of the use
> cases will be broken. (Add User/ Update User/ Get  Users ...). So, I think
> that user store is incomplete.
>
> Thanks
> Isura.
>
>
>>
>> On Tue, Jun 20, 2017 at 7:25 PM, Farasath Ahamed 
>> wrote:
>>
>>> Hi,
>>>
>>> The minimum requirement to write a custom JDBC user store manager so
>>> far (before IS 5.3.0) was to simply override the doAuthenticate() 
>>> method.
>>> So a custom user store that was written for 5.0.0 worked without any
>>> modifications (may be dependency changes).
>>>
>>> But when we use the same code on IS 5.3.0, the custom user store
>>> implementations that only override the doAuthenticate() are broken 
>>> because
>>> account disabled[1] and account locked[2] handlers introduced in IS 
>>> 5.3.0.
>>>
>>> These two handlers call the getUserClaimValues() method of the
>>> userstore to retrieve some claims. Since we haven't overridden the 
>>> method
>>> in custom userstore implementation it calls the super class. This leads 
>>> to
>>> trying to find the claims from a non-existing table[3].
>>>
>>> One way to solve is to override the getUserClaimValues() method. But
>>> in the PoV of the extension developer, this would be an unnecessary 
>>> step if
>>> the custom user store is just used for authentication only as explained 
>>> in
>>> [4].
>>>
>>> Even in the official

Re: [Dev] Custom UserStore works on 5.1.0, 5.2.0 not working in 5.3.0 and 5.4.0-M1

[Hasintha Indrajee]

We will be maintaining user store property to indicate whether the user
store supports claim operations (There may be multiple operation types, but
in this case claim operations). Before setting claims in pre and post
authentication listeners we will check whether the underlying user store is
supported for claim operations and will only continue if it's supported.
Unless returns and continues the flow.

There were few options discussed

1) We can just turn off these event handlers which get fired on
authentication events. But we may only need to skip a specific user store
and we may still need to use identity management functionality with rest of
the user stores. In this case we have to introduce a user tore level
property/configuration.

2) One of the other options is to expect a predefined run time exception
from the methods which are not implemented in User Store Manager  level and
handle them at our event handlers so that it will not have an effect on the
rest of the flow since we handle the run time exception. We thought of not
going with this since altering logic and taking decisions based on the type
of exception and error messages is not a good practice.


3) Another possibility was to introduce above check (checking newly
introduced user store property and skipping) at the
IdentityMgtEventListener level so that none of the handlers will be fired
if the user store is not supported for that particular operation. This is
not a proper fix since we may still need to get events to event handler
level even if the user store is not supported certain operations. eg - Even
though the user store is not supported for claim operations the deployment
may be using JDBC based Identity Store. In that we don't need handlers to
be skipped.

Hence the best option seems to be handling the behaviour based on newly
introduced property at specific concrete handler level. Depending on the
logic of specific handler it will decide whether to handle this event or
skip and continue.

On Tue, Jul 4, 2017 at 9:35 AM, Sagara Gunathunga  wrote:

>
> Hasintha, could you please update the thread with the solution we agreed ?
>
>
>
> Thanks !
>
> On Wed, Jun 21, 2017 at 1:01 PM, Isura Karunaratne  wrote:
>
>> Hi
>>
>> On Wed, Jun 21, 2017 at 11:06 AM, Farasath Ahamed 
>> wrote:
>>
>>>
>>>
>>>
>>>
>>> On Wed, Jun 21, 2017 at 11:03 AM, Isura Karunaratne 
>>> wrote:
>>>


 On Tue, Jun 20, 2017 at 11:29 PM, Johann Nallathamby 
 wrote:

> If these two handlers are disabled by default there shouldn't be any
> problem. According to default identity-event.properties file they are
> disabled. How come they get triggered then?
>

 Yes. By default the account lock/disabled features are disabled. If it
 is required to use account lock/disable features, there should be a way to
 store user properties.

>>>
>>> Looks like we haven't used the property to check whether the listener is
>>> enabled or disabled although we have defined in
>>> identity-event.properties. Therefore the handlers get fired on
>>> pre-authentications
>>>
>>
>> Yes. This issue is fixed with https://wso2.org/jira/browse/IDENTITY-6091
>>
>> Thanks
>> Isura.
>>
>>>
>>>

 Also, if the um_user_attribute table is not there, most of the use
 cases will be broken. (Add User/ Update User/ Get  Users ...). So, I think
 that user store is incomplete.

 Thanks
 Isura.


>
> On Tue, Jun 20, 2017 at 7:25 PM, Farasath Ahamed 
> wrote:
>
>> Hi,
>>
>> The minimum requirement to write a custom JDBC user store manager so
>> far (before IS 5.3.0) was to simply override the doAuthenticate() method.
>> So a custom user store that was written for 5.0.0 worked without any
>> modifications (may be dependency changes).
>>
>> But when we use the same code on IS 5.3.0, the custom user store
>> implementations that only override the doAuthenticate() are broken 
>> because
>> account disabled[1] and account locked[2] handlers introduced in IS 
>> 5.3.0.
>>
>> These two handlers call the getUserClaimValues() method of the
>> userstore to retrieve some claims. Since we haven't overridden the method
>> in custom userstore implementation it calls the super class. This leads 
>> to
>> trying to find the claims from a non-existing table[3].
>>
>> One way to solve is to override the getUserClaimValues() method. But
>> in the PoV of the extension developer, this would be an unnecessary step 
>> if
>> the custom user store is just used for authentication only as explained 
>> in
>> [4].
>>
>> Even in the official docs[5], we do not have any mention of having to
>> implement the getUserClaimValues() method.
>>
>> What would be the correct and the most efficient way to resolve this?
>> Appreciate your thoughts.
>>
>>
>>
>> [1] https://github.com/wso2-extensions/identity-e

Re: [Dev] Custom UserStore works on 5.1.0, 5.2.0 not working in 5.3.0 and 5.4.0-M1

[Sagara Gunathunga]

Hasintha, could you please update the thread with the solution we agreed ?


Thanks !

On Wed, Jun 21, 2017 at 1:01 PM, Isura Karunaratne  wrote:

> Hi
>
> On Wed, Jun 21, 2017 at 11:06 AM, Farasath Ahamed 
> wrote:
>
>>
>>
>>
>>
>> On Wed, Jun 21, 2017 at 11:03 AM, Isura Karunaratne 
>> wrote:
>>
>>>
>>>
>>> On Tue, Jun 20, 2017 at 11:29 PM, Johann Nallathamby 
>>> wrote:
>>>
 If these two handlers are disabled by default there shouldn't be any
 problem. According to default identity-event.properties file they are
 disabled. How come they get triggered then?

>>>
>>> Yes. By default the account lock/disabled features are disabled. If it
>>> is required to use account lock/disable features, there should be a way to
>>> store user properties.
>>>
>>
>> Looks like we haven't used the property to check whether the listener is
>> enabled or disabled although we have defined in identity-event.properties.
>> Therefore the handlers get fired on pre-authentications
>>
>
> Yes. This issue is fixed with https://wso2.org/jira/browse/IDENTITY-6091
>
> Thanks
> Isura.
>
>>
>>
>>>
>>> Also, if the um_user_attribute table is not there, most of the use cases
>>> will be broken. (Add User/ Update User/ Get  Users ...). So, I think that
>>> user store is incomplete.
>>>
>>> Thanks
>>> Isura.
>>>
>>>

 On Tue, Jun 20, 2017 at 7:25 PM, Farasath Ahamed 
 wrote:

> Hi,
>
> The minimum requirement to write a custom JDBC user store manager so
> far (before IS 5.3.0) was to simply override the doAuthenticate() method.
> So a custom user store that was written for 5.0.0 worked without any
> modifications (may be dependency changes).
>
> But when we use the same code on IS 5.3.0, the custom user store
> implementations that only override the doAuthenticate() are broken because
> account disabled[1] and account locked[2] handlers introduced in IS 5.3.0.
>
> These two handlers call the getUserClaimValues() method of the
> userstore to retrieve some claims. Since we haven't overridden the method
> in custom userstore implementation it calls the super class. This leads to
> trying to find the claims from a non-existing table[3].
>
> One way to solve is to override the getUserClaimValues() method. But
> in the PoV of the extension developer, this would be an unnecessary step 
> if
> the custom user store is just used for authentication only as explained in
> [4].
>
> Even in the official docs[5], we do not have any mention of having to
> implement the getUserClaimValues() method.
>
> What would be the correct and the most efficient way to resolve this?
> Appreciate your thoughts.
>
>
>
> [1] https://github.com/wso2-extensions/identity-event-handle
> r-account-lock/blob/master/components/org.wso2.carbon.identi
> ty.handler.event.account.lock/src/main/java/org/wso2/carbon/
> identity/handler/event/account/lock/AccountDisableHandler.java#L89
>
> [2] https://github.com/wso2-extensions/identity-event-handle
> r-account-lock/blob/master/components/org.wso2.carbon.identi
> ty.handler.event.account.lock/src/main/java/org/wso2/carbon/
> identity/handler/event/account/lock/AccountLockHandler.java#L186
>
> [3] https://wso2.org/jira/browse/IDENTITY-6074?focusedCommen
> tId=134555&page=com.atlassian.jira.plugin.system.issuetabpan
> els:comment-tabpanel#comment-134555
>
> [4] https://wso2.org/jira/browse/IDENTITY-6074
>
>
>
>
> Thanks,
> Farasath Ahamed
> Software Engineer, WSO2 Inc.; http://wso2.com
> Mobile: +94777603866
> Blog: blog.farazath.com
> Twitter: @farazath619 
> 
>
>
>


 --
 Thanks & Regards,

 *Johann Dilantha Nallathamby*
 Senior Technical Lead - WSO2 Identity Server
 Governance Technologies Team
 WSO2, Inc.
 lean.enterprise.middleware

 Mobile - *+9476950*
 Blog - *http://nallaa.wordpress.com *

 ___
 Dev mailing list
 Dev@wso2.org
 http://wso2.org/cgi-bin/mailman/listinfo/dev


>>>
>>>
>>> --
>>>
>>> *Isura Dilhara Karunaratne*
>>> Senior Software Engineer | WSO2
>>> Email: is...@wso2.com
>>> Mob : +94 772 254 810 <+94%2077%20225%204810>
>>> Blog : http://isurad.blogspot.com/
>>>
>>>
>>>
>>>
>>
>
>
> --
>
> *Isura Dilhara Karunaratne*
> Senior Software Engineer | WSO2
> Email: is...@wso2.com
> Mob : +94 772 254 810 <+94%2077%20225%204810>
> Blog : http://isurad.blogspot.com/
>
>
>
>
> ___
> Dev mailing list
> Dev@wso2.org
> http://wso2.org/cgi-bin/mailman/listinfo/dev
>
>


-- 
Sagara Gunathunga

Associate Director / Architect; WSO2, Inc.;  http://wso2.com
V.P Apache Web Services;http://ws.apache.org/
Linkedin;

Re: [Dev] Custom UserStore works on 5.1.0, 5.2.0 not working in 5.3.0 and 5.4.0-M1

[Isura Karunaratne]

Hi

On Wed, Jun 21, 2017 at 11:06 AM, Farasath Ahamed 
wrote:

>
>
>
>
> On Wed, Jun 21, 2017 at 11:03 AM, Isura Karunaratne 
> wrote:
>
>>
>>
>> On Tue, Jun 20, 2017 at 11:29 PM, Johann Nallathamby 
>> wrote:
>>
>>> If these two handlers are disabled by default there shouldn't be any
>>> problem. According to default identity-event.properties file they are
>>> disabled. How come they get triggered then?
>>>
>>
>> Yes. By default the account lock/disabled features are disabled. If it is
>> required to use account lock/disable features, there should be a way to
>> store user properties.
>>
>
> Looks like we haven't used the property to check whether the listener is
> enabled or disabled although we have defined in identity-event.properties.
> Therefore the handlers get fired on pre-authentications
>

Yes. This issue is fixed with https://wso2.org/jira/browse/IDENTITY-6091

Thanks
Isura.

>
>
>>
>> Also, if the um_user_attribute table is not there, most of the use cases
>> will be broken. (Add User/ Update User/ Get  Users ...). So, I think that
>> user store is incomplete.
>>
>> Thanks
>> Isura.
>>
>>
>>>
>>> On Tue, Jun 20, 2017 at 7:25 PM, Farasath Ahamed 
>>> wrote:
>>>
 Hi,

 The minimum requirement to write a custom JDBC user store manager so
 far (before IS 5.3.0) was to simply override the doAuthenticate() method.
 So a custom user store that was written for 5.0.0 worked without any
 modifications (may be dependency changes).

 But when we use the same code on IS 5.3.0, the custom user store
 implementations that only override the doAuthenticate() are broken because
 account disabled[1] and account locked[2] handlers introduced in IS 5.3.0.

 These two handlers call the getUserClaimValues() method of the
 userstore to retrieve some claims. Since we haven't overridden the method
 in custom userstore implementation it calls the super class. This leads to
 trying to find the claims from a non-existing table[3].

 One way to solve is to override the getUserClaimValues() method. But in
 the PoV of the extension developer, this would be an unnecessary step if
 the custom user store is just used for authentication only as explained in
 [4].

 Even in the official docs[5], we do not have any mention of having to
 implement the getUserClaimValues() method.

 What would be the correct and the most efficient way to resolve this?
 Appreciate your thoughts.



 [1] https://github.com/wso2-extensions/identity-event-handle
 r-account-lock/blob/master/components/org.wso2.carbon.identi
 ty.handler.event.account.lock/src/main/java/org/wso2/carbon/
 identity/handler/event/account/lock/AccountDisableHandler.java#L89

 [2] https://github.com/wso2-extensions/identity-event-handle
 r-account-lock/blob/master/components/org.wso2.carbon.identi
 ty.handler.event.account.lock/src/main/java/org/wso2/carbon/
 identity/handler/event/account/lock/AccountLockHandler.java#L186

 [3] https://wso2.org/jira/browse/IDENTITY-6074?focusedCommen
 tId=134555&page=com.atlassian.jira.plugin.system.issuetabpan
 els:comment-tabpanel#comment-134555

 [4] https://wso2.org/jira/browse/IDENTITY-6074




 Thanks,
 Farasath Ahamed
 Software Engineer, WSO2 Inc.; http://wso2.com
 Mobile: +94777603866
 Blog: blog.farazath.com
 Twitter: @farazath619 
 



>>>
>>>
>>> --
>>> Thanks & Regards,
>>>
>>> *Johann Dilantha Nallathamby*
>>> Senior Technical Lead - WSO2 Identity Server
>>> Governance Technologies Team
>>> WSO2, Inc.
>>> lean.enterprise.middleware
>>>
>>> Mobile - *+9476950*
>>> Blog - *http://nallaa.wordpress.com *
>>>
>>> ___
>>> Dev mailing list
>>> Dev@wso2.org
>>> http://wso2.org/cgi-bin/mailman/listinfo/dev
>>>
>>>
>>
>>
>> --
>>
>> *Isura Dilhara Karunaratne*
>> Senior Software Engineer | WSO2
>> Email: is...@wso2.com
>> Mob : +94 772 254 810 <+94%2077%20225%204810>
>> Blog : http://isurad.blogspot.com/
>>
>>
>>
>>
>


-- 

*Isura Dilhara Karunaratne*
Senior Software Engineer | WSO2
Email: is...@wso2.com
Mob : +94 772 254 810
Blog : http://isurad.blogspot.com/
___
Dev mailing list
Dev@wso2.org
http://wso2.org/cgi-bin/mailman/listinfo/dev

Re: [Dev] Custom UserStore works on 5.1.0, 5.2.0 not working in 5.3.0 and 5.4.0-M1

[Sagara Gunathunga]

On Wed, Jun 21, 2017 at 11:06 AM, Farasath Ahamed 
wrote:

>
>
>
>
> On Wed, Jun 21, 2017 at 11:03 AM, Isura Karunaratne 
> wrote:
>
>>
>>
>> On Tue, Jun 20, 2017 at 11:29 PM, Johann Nallathamby 
>> wrote:
>>
>>> If these two handlers are disabled by default there shouldn't be any
>>> problem. According to default identity-event.properties file they are
>>> disabled. How come they get triggered then?
>>>
>>
>> Yes. By default the account lock/disabled features are disabled. If it is
>> required to use account lock/disable features, there should be a way to
>> store user properties.
>>
>
> Looks like we haven't used the property to check whether the listener is
> enabled or disabled although we have defined in identity-event.properties.
> Therefore the handlers get fired on pre-authentications
>

I have arranged a review meeting on Friday 1 PM,  let's discuss and try to
model above two handler logic differently  without  breaking any existing
 features.

Thanks !

>
>
>>
>> Also, if the um_user_attribute table is not there, most of the use cases
>> will be broken. (Add User/ Update User/ Get  Users ...). So, I think that
>> user store is incomplete.
>>
>> Thanks
>> Isura.
>>
>>
>>>
>>> On Tue, Jun 20, 2017 at 7:25 PM, Farasath Ahamed 
>>> wrote:
>>>
 Hi,

 The minimum requirement to write a custom JDBC user store manager so
 far (before IS 5.3.0) was to simply override the doAuthenticate() method.
 So a custom user store that was written for 5.0.0 worked without any
 modifications (may be dependency changes).

 But when we use the same code on IS 5.3.0, the custom user store
 implementations that only override the doAuthenticate() are broken because
 account disabled[1] and account locked[2] handlers introduced in IS 5.3.0.

 These two handlers call the getUserClaimValues() method of the
 userstore to retrieve some claims. Since we haven't overridden the method
 in custom userstore implementation it calls the super class. This leads to
 trying to find the claims from a non-existing table[3].

 One way to solve is to override the getUserClaimValues() method. But in
 the PoV of the extension developer, this would be an unnecessary step if
 the custom user store is just used for authentication only as explained in
 [4].

 Even in the official docs[5], we do not have any mention of having to
 implement the getUserClaimValues() method.

 What would be the correct and the most efficient way to resolve this?
 Appreciate your thoughts.



 [1] https://github.com/wso2-extensions/identity-event-handle
 r-account-lock/blob/master/components/org.wso2.carbon.identi
 ty.handler.event.account.lock/src/main/java/org/wso2/carbon/
 identity/handler/event/account/lock/AccountDisableHandler.java#L89

 [2] https://github.com/wso2-extensions/identity-event-handle
 r-account-lock/blob/master/components/org.wso2.carbon.identi
 ty.handler.event.account.lock/src/main/java/org/wso2/carbon/
 identity/handler/event/account/lock/AccountLockHandler.java#L186

 [3] https://wso2.org/jira/browse/IDENTITY-6074?focusedCommen
 tId=134555&page=com.atlassian.jira.plugin.system.issuetabpan
 els:comment-tabpanel#comment-134555

 [4] https://wso2.org/jira/browse/IDENTITY-6074




 Thanks,
 Farasath Ahamed
 Software Engineer, WSO2 Inc.; http://wso2.com
 Mobile: +94777603866
 Blog: blog.farazath.com
 Twitter: @farazath619 
 



>>>
>>>
>>> --
>>> Thanks & Regards,
>>>
>>> *Johann Dilantha Nallathamby*
>>> Senior Technical Lead - WSO2 Identity Server
>>> Governance Technologies Team
>>> WSO2, Inc.
>>> lean.enterprise.middleware
>>>
>>> Mobile - *+9476950*
>>> Blog - *http://nallaa.wordpress.com *
>>>
>>> ___
>>> Dev mailing list
>>> Dev@wso2.org
>>> http://wso2.org/cgi-bin/mailman/listinfo/dev
>>>
>>>
>>
>>
>> --
>>
>> *Isura Dilhara Karunaratne*
>> Senior Software Engineer | WSO2
>> Email: is...@wso2.com
>> Mob : +94 772 254 810 <+94%2077%20225%204810>
>> Blog : http://isurad.blogspot.com/
>>
>>
>>
>>
>
> ___
> Dev mailing list
> Dev@wso2.org
> http://wso2.org/cgi-bin/mailman/listinfo/dev
>
>


-- 
Sagara Gunathunga

Associate Director / Architect; WSO2, Inc.;  http://wso2.com
V.P Apache Web Services;http://ws.apache.org/
Linkedin; http://www.linkedin.com/in/ssagara
Blog ;  http://ssagara.blogspot.com
___
Dev mailing list
Dev@wso2.org
http://wso2.org/cgi-bin/mailman/listinfo/dev

Re: [Dev] Custom UserStore works on 5.1.0, 5.2.0 not working in 5.3.0 and 5.4.0-M1

[Farasath Ahamed]

On Wed, Jun 21, 2017 at 11:03 AM, Isura Karunaratne  wrote:

>
>
> On Tue, Jun 20, 2017 at 11:29 PM, Johann Nallathamby 
> wrote:
>
>> If these two handlers are disabled by default there shouldn't be any
>> problem. According to default identity-event.properties file they are
>> disabled. How come they get triggered then?
>>
>
> Yes. By default the account lock/disabled features are disabled. If it is
> required to use account lock/disable features, there should be a way to
> store user properties.
>

Looks like we haven't used the property to check whether the listener is
enabled or disabled although we have defined in identity-event.properties.
Therefore the handlers get fired on pre-authentications


>
> Also, if the um_user_attribute table is not there, most of the use cases
> will be broken. (Add User/ Update User/ Get  Users ...). So, I think that
> user store is incomplete.
>
> Thanks
> Isura.
>
>
>>
>> On Tue, Jun 20, 2017 at 7:25 PM, Farasath Ahamed 
>> wrote:
>>
>>> Hi,
>>>
>>> The minimum requirement to write a custom JDBC user store manager so far
>>> (before IS 5.3.0) was to simply override the doAuthenticate() method. So a
>>> custom user store that was written for 5.0.0 worked without any
>>> modifications (may be dependency changes).
>>>
>>> But when we use the same code on IS 5.3.0, the custom user store
>>> implementations that only override the doAuthenticate() are broken because
>>> account disabled[1] and account locked[2] handlers introduced in IS 5.3.0.
>>>
>>> These two handlers call the getUserClaimValues() method of the
>>> userstore to retrieve some claims. Since we haven't overridden the method
>>> in custom userstore implementation it calls the super class. This leads to
>>> trying to find the claims from a non-existing table[3].
>>>
>>> One way to solve is to override the getUserClaimValues() method. But in
>>> the PoV of the extension developer, this would be an unnecessary step if
>>> the custom user store is just used for authentication only as explained in
>>> [4].
>>>
>>> Even in the official docs[5], we do not have any mention of having to
>>> implement the getUserClaimValues() method.
>>>
>>> What would be the correct and the most efficient way to resolve this?
>>> Appreciate your thoughts.
>>>
>>>
>>>
>>> [1] https://github.com/wso2-extensions/identity-event-handle
>>> r-account-lock/blob/master/components/org.wso2.carbon.identi
>>> ty.handler.event.account.lock/src/main/java/org/wso2/carbon/
>>> identity/handler/event/account/lock/AccountDisableHandler.java#L89
>>>
>>> [2] https://github.com/wso2-extensions/identity-event-handle
>>> r-account-lock/blob/master/components/org.wso2.carbon.identi
>>> ty.handler.event.account.lock/src/main/java/org/wso2/carbon/
>>> identity/handler/event/account/lock/AccountLockHandler.java#L186
>>>
>>> [3] https://wso2.org/jira/browse/IDENTITY-6074?focusedCommen
>>> tId=134555&page=com.atlassian.jira.plugin.system.issuetabpan
>>> els:comment-tabpanel#comment-134555
>>>
>>> [4] https://wso2.org/jira/browse/IDENTITY-6074
>>>
>>>
>>>
>>>
>>> Thanks,
>>> Farasath Ahamed
>>> Software Engineer, WSO2 Inc.; http://wso2.com
>>> Mobile: +94777603866
>>> Blog: blog.farazath.com
>>> Twitter: @farazath619 
>>> 
>>>
>>>
>>>
>>
>>
>> --
>> Thanks & Regards,
>>
>> *Johann Dilantha Nallathamby*
>> Senior Technical Lead - WSO2 Identity Server
>> Governance Technologies Team
>> WSO2, Inc.
>> lean.enterprise.middleware
>>
>> Mobile - *+9476950*
>> Blog - *http://nallaa.wordpress.com *
>>
>> ___
>> Dev mailing list
>> Dev@wso2.org
>> http://wso2.org/cgi-bin/mailman/listinfo/dev
>>
>>
>
>
> --
>
> *Isura Dilhara Karunaratne*
> Senior Software Engineer | WSO2
> Email: is...@wso2.com
> Mob : +94 772 254 810 <+94%2077%20225%204810>
> Blog : http://isurad.blogspot.com/
>
>
>
>
___
Dev mailing list
Dev@wso2.org
http://wso2.org/cgi-bin/mailman/listinfo/dev

Re: [Dev] Custom UserStore works on 5.1.0, 5.2.0 not working in 5.3.0 and 5.4.0-M1

[Isura Karunaratne]

On Tue, Jun 20, 2017 at 11:29 PM, Johann Nallathamby 
wrote:

> If these two handlers are disabled by default there shouldn't be any
> problem. According to default identity-event.properties file they are
> disabled. How come they get triggered then?
>

Yes. By default the account lock/disabled features are disabled. If it is
required to use account lock/disable features, there should be a way to
store user properties.

Also, if the um_user_attribute table is not there, most of the use cases
will be broken. (Add User/ Update User/ Get  Users ...). So, I think that
user store is incomplete.

Thanks
Isura.


>
> On Tue, Jun 20, 2017 at 7:25 PM, Farasath Ahamed 
> wrote:
>
>> Hi,
>>
>> The minimum requirement to write a custom JDBC user store manager so far
>> (before IS 5.3.0) was to simply override the doAuthenticate() method. So a
>> custom user store that was written for 5.0.0 worked without any
>> modifications (may be dependency changes).
>>
>> But when we use the same code on IS 5.3.0, the custom user store
>> implementations that only override the doAuthenticate() are broken because
>> account disabled[1] and account locked[2] handlers introduced in IS 5.3.0.
>>
>> These two handlers call the getUserClaimValues() method of the
>> userstore to retrieve some claims. Since we haven't overridden the method
>> in custom userstore implementation it calls the super class. This leads to
>> trying to find the claims from a non-existing table[3].
>>
>> One way to solve is to override the getUserClaimValues() method. But in
>> the PoV of the extension developer, this would be an unnecessary step if
>> the custom user store is just used for authentication only as explained in
>> [4].
>>
>> Even in the official docs[5], we do not have any mention of having to
>> implement the getUserClaimValues() method.
>>
>> What would be the correct and the most efficient way to resolve this?
>> Appreciate your thoughts.
>>
>>
>>
>> [1] https://github.com/wso2-extensions/identity-event-handle
>> r-account-lock/blob/master/components/org.wso2.carbon.
>> identity.handler.event.account.lock/src/main/java/org/wso2/
>> carbon/identity/handler/event/account/lock/AccountDisableHandler.java#L89
>>
>> [2] https://github.com/wso2-extensions/identity-event-handle
>> r-account-lock/blob/master/components/org.wso2.carbon.
>> identity.handler.event.account.lock/src/main/java/org/wso2/
>> carbon/identity/handler/event/account/lock/AccountLockHandler.java#L186
>>
>> [3] https://wso2.org/jira/browse/IDENTITY-6074?focusedCommen
>> tId=134555&page=com.atlassian.jira.plugin.system.
>> issuetabpanels:comment-tabpanel#comment-134555
>>
>> [4] https://wso2.org/jira/browse/IDENTITY-6074
>>
>>
>>
>>
>> Thanks,
>> Farasath Ahamed
>> Software Engineer, WSO2 Inc.; http://wso2.com
>> Mobile: +94777603866
>> Blog: blog.farazath.com
>> Twitter: @farazath619 
>> 
>>
>>
>>
>
>
> --
> Thanks & Regards,
>
> *Johann Dilantha Nallathamby*
> Senior Technical Lead - WSO2 Identity Server
> Governance Technologies Team
> WSO2, Inc.
> lean.enterprise.middleware
>
> Mobile - *+9476950*
> Blog - *http://nallaa.wordpress.com *
>
> ___
> Dev mailing list
> Dev@wso2.org
> http://wso2.org/cgi-bin/mailman/listinfo/dev
>
>


-- 

*Isura Dilhara Karunaratne*
Senior Software Engineer | WSO2
Email: is...@wso2.com
Mob : +94 772 254 810
Blog : http://isurad.blogspot.com/
___
Dev mailing list
Dev@wso2.org
http://wso2.org/cgi-bin/mailman/listinfo/dev

Re: [Dev] Custom UserStore works on 5.1.0, 5.2.0 not working in 5.3.0 and 5.4.0-M1

[Johann Nallathamby]

If these two handlers are disabled by default there shouldn't be any
problem. According to default identity-event.properties file they are
disabled. How come they get triggered then?

On Tue, Jun 20, 2017 at 7:25 PM, Farasath Ahamed  wrote:

> Hi,
>
> The minimum requirement to write a custom JDBC user store manager so far
> (before IS 5.3.0) was to simply override the doAuthenticate() method. So a
> custom user store that was written for 5.0.0 worked without any
> modifications (may be dependency changes).
>
> But when we use the same code on IS 5.3.0, the custom user store
> implementations that only override the doAuthenticate() are broken because
> account disabled[1] and account locked[2] handlers introduced in IS 5.3.0.
>
> These two handlers call the getUserClaimValues() method of the
> userstore to retrieve some claims. Since we haven't overridden the method
> in custom userstore implementation it calls the super class. This leads to
> trying to find the claims from a non-existing table[3].
>
> One way to solve is to override the getUserClaimValues() method. But in
> the PoV of the extension developer, this would be an unnecessary step if
> the custom user store is just used for authentication only as explained in
> [4].
>
> Even in the official docs[5], we do not have any mention of having to
> implement the getUserClaimValues() method.
>
> What would be the correct and the most efficient way to resolve this?
> Appreciate your thoughts.
>
>
>
> [1] https://github.com/wso2-extensions/identity-event-
> handler-account-lock/blob/master/components/org.wso2.
> carbon.identity.handler.event.account.lock/src/main/java/
> org/wso2/carbon/identity/handler/event/account/lock/
> AccountDisableHandler.java#L89
>
> [2] https://github.com/wso2-extensions/identity-event-
> handler-account-lock/blob/master/components/org.wso2.
> carbon.identity.handler.event.account.lock/src/main/java/
> org/wso2/carbon/identity/handler/event/account/lock/
> AccountLockHandler.java#L186
>
> [3] https://wso2.org/jira/browse/IDENTITY-6074?
> focusedCommentId=134555&page=com.atlassian.jira.plugin.
> system.issuetabpanels:comment-tabpanel#comment-134555
>
> [4] https://wso2.org/jira/browse/IDENTITY-6074
>
>
>
>
> Thanks,
> Farasath Ahamed
> Software Engineer, WSO2 Inc.; http://wso2.com
> Mobile: +94777603866
> Blog: blog.farazath.com
> Twitter: @farazath619 
> 
>
>
>


-- 
Thanks & Regards,

*Johann Dilantha Nallathamby*
Senior Technical Lead - WSO2 Identity Server
Governance Technologies Team
WSO2, Inc.
lean.enterprise.middleware

Mobile - *+9476950*
Blog - *http://nallaa.wordpress.com *
___
Dev mailing list
Dev@wso2.org
http://wso2.org/cgi-bin/mailman/listinfo/dev

[Dev] Custom UserStore works on 5.1.0, 5.2.0 not working in 5.3.0 and 5.4.0-M1

[Farasath Ahamed]

Hi,

The minimum requirement to write a custom JDBC user store manager so far
(before IS 5.3.0) was to simply override the doAuthenticate() method. So a
custom user store that was written for 5.0.0 worked without any
modifications (may be dependency changes).

But when we use the same code on IS 5.3.0, the custom user store
implementations that only override the doAuthenticate() are broken because
account disabled[1] and account locked[2] handlers introduced in IS 5.3.0.

These two handlers call the getUserClaimValues() method of the userstore to
retrieve some claims. Since we haven't overridden the method in custom
userstore implementation it calls the super class. This leads to trying to
find the claims from a non-existing table[3].

One way to solve is to override the getUserClaimValues() method. But in the
PoV of the extension developer, this would be an unnecessary step if the
custom user store is just used for authentication only as explained in [4].

Even in the official docs[5], we do not have any mention of having to
implement the getUserClaimValues() method.

What would be the correct and the most efficient way to resolve this?
Appreciate your thoughts.



[1]
https://github.com/wso2-extensions/identity-event-handler-account-lock/blob/master/components/org.wso2.carbon.identity.handler.event.account.lock/src/main/java/org/wso2/carbon/identity/handler/event/account/lock/AccountDisableHandler.java#L89

[2]
https://github.com/wso2-extensions/identity-event-handler-account-lock/blob/master/components/org.wso2.carbon.identity.handler.event.account.lock/src/main/java/org/wso2/carbon/identity/handler/event/account/lock/AccountLockHandler.java#L186

[3]
https://wso2.org/jira/browse/IDENTITY-6074?focusedCommentId=134555&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-134555

[4] https://wso2.org/jira/browse/IDENTITY-6074




Thanks,
Farasath Ahamed
Software Engineer, WSO2 Inc.; http://wso2.com
Mobile: +94777603866
Blog: blog.farazath.com
Twitter: @farazath619 

___
Dev mailing list
Dev@wso2.org
http://wso2.org/cgi-bin/mailman/listinfo/dev

Re: [Dev] Custom UserStore works on 5.1.0, 5.2.0 not working in 5.3.0 and 5.4.0-M1

[Sagara Gunathunga]

On Tue, Jun 20, 2017 at 7:25 PM, Farasath Ahamed  wrote:

> Hi,
>
> The minimum requirement to write a custom JDBC user store manager so far
> (before IS 5.3.0) was to simply override the doAuthenticate() method. So a
> custom user store that was written for 5.0.0 worked without any
> modifications (may be dependency changes).
>
> But when we use the same code on IS 5.3.0, the custom user store
> implementations that only override the doAuthenticate() are broken because
> account disabled[1] and account locked[2] handlers introduced in IS 5.3.0.
>
> These two handlers call the getUserClaimValues() method of the
> userstore to retrieve some claims. Since we haven't overridden the method
> in custom userstore implementation it calls the super class. This leads to
> trying to find the claims from a non-existing table[3].
>
> One way to solve is to override the getUserClaimValues() method. But in
> the PoV of the extension developer, this would be an unnecessary step if
> the custom user store is just used for authentication only as explained in
> [4].
>
> Even in the official docs[5], we do not have any mention of having to
> implement the getUserClaimValues() method.
>
> What would be the correct and the most efficient way to resolve this?
> Appreciate your thoughts.
>

[This is the same comment I added in JIRA ticket ]

In summery, one of the very important  and widely used user-mgt extension
was broken with above handlers so correct fix is to come up with a solution
to fix the design of those 2 handlers, we can't sugguest a fix for
extension developer  side,  followings are my comments on technical
solutions we identified  earlier.

*1.) create um_user_attribute table in the database schema *

- Most of the cases the main reason for users to write custom JDBC user
stores is they just want to plug one of the existing database with user
details, usually these user databases are designed and used with some
others systems so adding a new table ( this new table does not mean
anything to this other system) may not possible or involve many
complexities. Further if users create this table they don't have any
meaningful data to put there, they just create an empty table in order to
get a WSO2 IS extension works, IMO this is very bad extension experience.

- Additionally if a user is decided to deviate from WSO2 IS user database
schema, it is not possible to force the user to create um_user_attribute
table in his own schema, in other words users can't use their own schema
100% instead they must always include one table from WSO2 IS user database
schema. If the user can forget all other IS default tables what the
specially on um_user_attribute table ? IMO there is no such speciality.


- Assuming a user wrote custom JDBC store with his own schema and works
fine on 5.1.0/5.2.0 etc. now when he migrate to 5.3.0 he is forced to
create empty table in his own database schema, that's not acceptable ( Here
I'm not talk about WSO2 database migration between two versions, that is
acceptable )


- According to our documentation [1] there is no such requirement
mentioned, we can fix WSO2 documentation but what about large number of
blogs and tutorials written by internal and external people, we can't
change them, with this all of those content become invalid.


*2.) override the getUserPropertyValues() method *

- Custom JDBC user store is not a new extension, it was there for a long
time with same API. The bare minimum practical use case is to plugin only
authentication logic through a read only database access, in such cases
overriding doAuthenticate() method is sufficient and it's perfectly make
sense for extension developers as well. But now when a user just want to
plugin read-only authentication logic the user is forced to override
getUserPropertyValues() methods as well, override of
getUserPropertyValues() method does not make sense in business POV, as you
suggested returning empty HashMap does not add any business value instead
it just required for the extension to work.

- Like in previous case if a user migrating to IS 5.3.0 from 5.1.0/5.2.0 he
must modify his extension code to include empty java method which does not
make any sense in his extension domain, additionally the extension code
should go another round of development, testing process which invloves
extra cost and extra time.

- By looking at suggested code it just return empty HashMap, then it's a
valid question why IS expect empty HashMap from users, it's just empty
container. ( this point should not get confuse with cases where users
intentionally override getUserPropertyValues() method to achieve something
useful in his business context)

@Override
public Map getUserPropertyValues(String userName, String[]
propertyNames, String profileName) throws UserStoreException {
return new HashMap<>();
}

- Assuming we want to enforce getUserPropertyValues() method for any custom
JDBC user store, this is not the right approach to modify API, the right
approach is in