joscha / play-authenticate

An authentication plugin for Play Framework 2.x (Java)
http://joscha.github.com/play-authenticate/
Other
807 stars 367 forks source link

Best approach for getting the user, referencing play-authenticate-usage sample #300

Open Mule52 opened 8 years ago

Mule52 commented 8 years ago

I am using the play-authenticate-usage Java example. I appreciate the sample.

When I debug the sample and look at my MySQL logs, what I am seeing is that the sample will make a request twice to the database every time the page loads, on every page, once I am logged in. Is this on purpose? The requests originate from UserProvider and MyDeadboltHandler both calling User.findByAuthUserIdentity(u);

This is my MySQL log below: txn[1014] select distinct t0.id c0, t0.email c1, t0.name c2, t0.first_name c3, t0.last_name c4, t0.created_on c5, t0.updated_on c6, t0.last_login c7, t0.active c8, t0.email_validated c9 from users t0 join linked_accounts u1 on u1.user_id = t0.id where t0.active = ? and u1.provider_user_id = ? and u1.provider_key = ?

If this is on purpose, is this a recommended practice, or should I be pulling the user from session instead of hitting the DB each time on every page?

In the MyUsernamePasswordAuthProvider.java's loginUser(authUser) function, rather than getting the User from the database, shouldn't I check session for that user first? I do not have access to session in this method, but you get what I am trying to understand.

final User u = User.findByUsernamePasswordIdentity(authUser);

Better yet, the UserProvider class has a function below that will get the user from the database each time. Is this intentional for scaling, or should this use session to return the local user?

public User getUser(Session session) { final AuthUser currentAuthUser = this.auth.getUser(session); final User localUser = User.findByAuthUserIdentity(currentAuthUser); return localUser; }

Another place we get the user is in MyDeadboltHandler, on getSubject(httpContext). I would need to check session there as well. Any time the user is changed, I would need to update my user in session, now it becomes more difficult to manage.

Thanks for clarification or sugestions.

ScottBurleigh commented 8 years ago

I am have the same question about caching the user and going to the DB each time. Does the Play Authenticate sample app purposely go to the DB each time, or was it written that way for simplicity? In a large scale app, what's the suggested approach?

arvindbatra commented 8 years ago

+1 . It will be a good idea to document best practices in this case.

Bair8xiu commented 8 years ago

I did the following:

  @Nullable
    public User getUser(Session session) {
        LoggedSql.start();
        final AuthUser currentAuthUser = this.auth.getUser(session);
        final User localUser = User.findByAuthUserIdentity(currentAuthUser);
        List<String> capturedLogs = LoggedSql.stop();
        Logger.info("db hit ...");
        return localUser;
    }

note: It's SQL logging from this question

edit: It didn't show any SQL statements. edit2:

Ok got a db hit after every action, can confirm.

Mule52 commented 8 years ago

I added Play's CacheApi to this logic in my own project. Instead of getting the User each time from the database, which was seven identical database gets for my project implementation, now I just access the User from cache. When I logout, I make sure to remove any caches items I added. Now I only access the database for the User once.