# Can we expose getLoginCredential(org.openmrs.User) to the service layer? **URL:** \ **Category:** Development\ **Created:** [August 23, 2015, 9:03pm UTC](https://talk.openmrs.org/t/can-we-expose-getlogincredential-org-openmrs-user-to-the-service-layer/2848 "2015-08-23T21:03:03Z")\ **Posts on this page:** 14\ **Page:** 1
**Author:** ![k.joseph](https://talk.openmrs.org/user_avatar/talk.openmrs.org/k.joseph/32/8097_2.png) [@k.joseph](https://talk.openmrs.org/u/k.joseph)\ **Post date:** [August 23, 2015, 9:03pm UTC](https://talk.openmrs.org/t/can-we-expose-getlogincredential-org-openmrs-user-to-the-service-layer/2848/1 "2015-08-23T21:03:03Z")
While working on this admin ui sprint task: > … I have noticed that `org.openmrs.api.db.UserDAO#getLoginCredential(org.openmrs.User)` is only accessed up to the UserDAO layer and not at the service layer. I needed to use this method since `user.getSecretQuestion()` is deprecated and returns null even when the secret question is saved! For now I am planning to call into the dao from the controller which is a sub standard way of doing things, so i thought of ringing this thought in the devs minds: Can we expose or write a `getLoginCredential(org.openmrs.User)` method to be accessed at the service layer? For now, what would be your advise to return such credentials like saved question into the controller? ---
**Author:** ![dkayiwa](https://talk.openmrs.org/user_avatar/talk.openmrs.org/dkayiwa/32/179_2.png) [@dkayiwa](https://talk.openmrs.org/u/dkayiwa)\ **Post date:** [August 24, 2015, 7:32am UTC](https://talk.openmrs.org/t/can-we-expose-getlogincredential-org-openmrs-user-to-the-service-layer/2848/2 "2015-08-24T07:32:21Z")
My understanding is that the AdminUI sprint is simply redoing the already existing functionality in the new reference application UI. So why not just use the exact same service layer API which is currently used by the legacy UI? Or are you trying to add new UI functionality that was not in the legacy UI? ---
**Author:** ![k.joseph](https://talk.openmrs.org/user_avatar/talk.openmrs.org/k.joseph/32/8097_2.png) [@k.joseph](https://talk.openmrs.org/u/k.joseph)\ **Post date:** [August 24, 2015, 8:09am UTC](https://talk.openmrs.org/t/can-we-expose-getlogincredential-org-openmrs-user-to-the-service-layer/2848/3 "2015-08-24T08:09:33Z")
We are doing similar stuff but adding on; for-example for the reference application; we are going to display the secret question instead of a blank field which was initially used in the legacy ui and may imply that the user has none defined ---
**Author:** ![dkayiwa](https://talk.openmrs.org/user_avatar/talk.openmrs.org/dkayiwa/32/179_2.png) [@dkayiwa](https://talk.openmrs.org/u/dkayiwa)\ **Post date:** [August 24, 2015, 8:27am UTC](https://talk.openmrs.org/t/can-we-expose-getlogincredential-org-openmrs-user-to-the-service-layer/2848/4 "2015-08-24T08:27:21Z")
The legacy UI already displays the secret question. So i still do not see any new feature in what you are supposed to do in the refapp. 🙂 ---
**Author:** ![k.joseph](https://talk.openmrs.org/user_avatar/talk.openmrs.org/k.joseph/32/8097_2.png) [@k.joseph](https://talk.openmrs.org/u/k.joseph)\ **Post date:** [August 24, 2015, 8:44am UTC](https://talk.openmrs.org/t/can-we-expose-getlogincredential-org-openmrs-user-to-the-service-layer/2848/5 "2015-08-24T08:44:37Z")
O, Right, i had looked up to a user with none. Actually i called into the UserDAO class to get it since user.getSecretQuestion() is deprecated, like; `getComponent(UserDAO.class).getLoginCredential(Context.getAuthenticatedUser()).getSecretQuestion()` I hope this does us no harm @dkayiwa! ---
**Author:** ![dkayiwa](https://talk.openmrs.org/user_avatar/talk.openmrs.org/dkayiwa/32/179_2.png) [@dkayiwa](https://talk.openmrs.org/u/dkayiwa)\ **Post date:** [August 24, 2015, 8:57am UTC](https://talk.openmrs.org/t/can-we-expose-getlogincredential-org-openmrs-user-to-the-service-layer/2848/6 "2015-08-24T08:57:47Z")
Do you have to get it differently from the legacy UI? ---
**Author:** ![burke](https://talk.openmrs.org/user_avatar/talk.openmrs.org/burke/32/17_2.png) [@burke](https://talk.openmrs.org/u/burke)\ **Post date:** [August 24, 2015, 2:25pm UTC](https://talk.openmrs.org/t/can-we-expose-getlogincredential-org-openmrs-user-to-the-service-layer/2848/7 "2015-08-24T14:25:39Z")
The short answer is “No.” he [`LoginCredential` javadoc`](http://resources.openmrs.org/doc/org/openmrs/api/db/LoginCredential.html) specifically states: “This should never be used by anything except for UserDAO and UserService methods that change passwords.” It sounds like user authentication is poorly designed. - I can understand why, for security, we would not want to populate every `User` instance with its secret question. - I recall pushing to have login credentials separated from the API (in hopes that [TRUNK-381](https://issues.openmrs.org/browse/TRUNK-381) might get done some day). - Our single secret question + secret answer approach to self-service security is pretty lame and should be replaced (e.g., with temporary password to email). While we may not solve that here, we should at least not dig ourselves deeper into the hole while solving the immediate problem. Certainly, having to bypass the service layer and go through the DAO is a red flag that something is wrong. Given the above points, I would think a `UserService.getSecretQuestionForUser(User)` would be a temporary fix (it could use the `LoginCredential` without exposing it outside the API). Unfortunately, this needs to be accessible to anonymous users, since its use case is during password recovery… which means anyone can anonymously obtain the secret question for any user for whom they can guess their username (part of the reason this approach is lame). At a mimimum, we should log any time it is used (where exposed in the web layer – e.g., a REST endpoint – we’d want to log the IP address of the requestor) _and_ we should double-check to make sure that an adminstrator can disable this feature system-wide for security (requiring an admin to reset a user’s password when forgotten). When we can implement a less embarassing self-service password reset feature (e.g., email temporary password), we can finally get rid of this user-generated question/answer pair approach altogether. ---
**Author:** ![darius](https://talk.openmrs.org/user_avatar/talk.openmrs.org/darius/32/8082_2.png) [@darius](https://talk.openmrs.org/u/darius)\ **Post date:** [August 24, 2015, 2:51pm UTC](https://talk.openmrs.org/t/can-we-expose-getlogincredential-org-openmrs-user-to-the-service-layer/2848/8 "2015-08-24T14:51:23Z")
I would just add that I recommend that whatever we do as a temporary solution, we should do this in the adminui module. (I.e. we should not make these temporary changes in openmrs-core.) ---
**Author:** ![wyclif](https://talk.openmrs.org/user_avatar/talk.openmrs.org/wyclif/32/19607_2.png) [@wyclif](https://talk.openmrs.org/u/wyclif)\ **Post date:** [August 24, 2015, 7:34pm UTC](https://talk.openmrs.org/t/can-we-expose-getlogincredential-org-openmrs-user-to-the-service-layer/2848/9 "2015-08-24T19:34:21Z")
@Darius the Burke’s proposed solutions aren’t going to be implemented in core in anytime soon, in the short run we are going to use the deprecated property accessors for question and answer on the user object. FYI on the account form am working on, i currently don’t support setting the secret question and answer for this same reason. ---
**Author:** ![wyclif](https://talk.openmrs.org/user_avatar/talk.openmrs.org/wyclif/32/19607_2.png) [@wyclif](https://talk.openmrs.org/u/wyclif)\ **Post date:** [August 24, 2015, 7:52pm UTC](https://talk.openmrs.org/t/can-we-expose-getlogincredential-org-openmrs-user-to-the-service-layer/2848/10 "2015-08-24T19:52:58Z")
I just saw @k\_joseph’s pull request, he has a work around in the adminui module where he fetches the userDAO bean from the Context and uses that to get the login credentials and it seems like a fine work around to me for now. ---
**Author:** ![darius](https://talk.openmrs.org/user_avatar/talk.openmrs.org/darius/32/8082_2.png) [@darius](https://talk.openmrs.org/u/darius)\ **Post date:** [August 24, 2015, 8:08pm UTC](https://talk.openmrs.org/t/can-we-expose-getlogincredential-org-openmrs-user-to-the-service-layer/2848/11 "2015-08-24T20:08:58Z")
Just as long as you’re only allowed to fetch your own secret question, and not anyone else’s, then it’s a fine workaround. ---
**Author:** ![k.joseph](https://talk.openmrs.org/user_avatar/talk.openmrs.org/k.joseph/32/8097_2.png) [@k.joseph](https://talk.openmrs.org/u/k.joseph)\ **Post date:** [August 24, 2015, 8:57pm UTC](https://talk.openmrs.org/t/can-we-expose-getlogincredential-org-openmrs-user-to-the-service-layer/2848/12 "2015-08-24T20:57:39Z")
> [@burke](#): > > I would think a UserService.getSecretQuestionForUser(User) would be a temporary fix Currently, that method doesn’t exist and getting the LoginCredential object into the controller means exposing all it’s other properties such as; salt, secret answer, hashed password into the controller though i had only used secret question property which i return onto the ui and nothing else ---
**Author:** ![burke](https://talk.openmrs.org/user_avatar/talk.openmrs.org/burke/32/17_2.png) [@burke](https://talk.openmrs.org/u/burke)\ **Post date:** [August 25, 2015, 1:27am UTC](https://talk.openmrs.org/t/can-we-expose-getlogincredential-org-openmrs-user-to-the-service-layer/2848/13 "2015-08-25T01:27:51Z")
> [@darius](#): > > Just as long as you’re only allowed to fetch your own secret question, and not anyone else’s, then it’s a fine workaround. How can you restrict access to something needed for password recovery (a feature usually only seen when you are not yet authenticated)? Except for managing their secret question, the user would not be authenticated when trying to reset their password. ---
**Author:** ![darius](https://talk.openmrs.org/user_avatar/talk.openmrs.org/darius/32/8082_2.png) [@darius](https://talk.openmrs.org/u/darius)\ **Post date:** [August 25, 2015, 7:26pm UTC](https://talk.openmrs.org/t/can-we-expose-getlogincredential-org-openmrs-user-to-the-service-layer/2848/14 "2015-08-25T19:26:23Z")
What I had meant to say is that if we’re directly accessing the DAO bean from the My Account page, it should be hardcoded in such a way that it only shows the secret question of the authenticated user (e.g. doesn’t respect any ?username=… in the request). Since, as Burke points out, anyone can learn anyone else’s secret question just by knowing their username and doing the “forgot password” workflow, I guess any security weakness is already there and easy to access. So, you can ignore this message of mine, and the previous one. 😄