On Tue, Jun 20, 2017 at 7:25 PM, Farasath Ahamed <farasa...@wso2.com> 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<String, String> 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 intorduce new API with new constrains while mark existing API
as deprecated but keep it for future releases for sometime.
- Same as above, 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.
Thanks !
>
>
>
> [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 <https://twitter.com/farazath619>
> <http://wso2.com/signature>
>
>
>
--
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
