ServiceStack's authentication mechanism: too many queries for auth request?

[aicl]

Hi Demis,

I am using  ServiceStack's authentication mechanism  CredentialsAuthProvider + OrmLiteAuthRepository,

and it works fine, but checking  log ouput I find that for every

authentication request  is executed the same query to the database:

DEBUG: SELECT "Id","UserName","Email","PrimaryEmail","FirstName","LastName","DisplayName","Salt","PasswordHash","Roles","Permissions","CreatedDate","ModifiedDate","Meta" FROM "UserAuth" WHERE UserName = 'admin'

DEBUG: SELECT "Id","UserName","Email","PrimaryEmail","FirstName","LastName","DisplayName","Salt","PasswordHash","Roles","Permissions","CreatedDate","ModifiedDate","Meta" FROM "UserAuth" WHERE UserName = 'admin'

DEBUG: SELECT "Id","UserAuthId","Provider","UserId","UserName","DisplayName","FirstName","LastName","Email","RequestToken","RequestTokenSecret","Items","AccessToken","AccessTokenSecret","CreatedDate","ModifiedDate" FROM "UserOAuthProvider" WHERE UserAuthId = '1'

DEBUG: SELECT "Id","UserName","Email","PrimaryEmail","FirstName","LastName","DisplayName","Salt","PasswordHash","Roles","Permissions","CreatedDate","ModifiedDate","Meta" FROM "UserAuth" WHERE "Id" = '1'

DEBUG: SELECT "Id","UserName","Email","PrimaryEmail","FirstName","LastName","DisplayName","Salt","PasswordHash","Roles","Permissions","CreatedDate","ModifiedDate","Meta" FROM "UserAuth" WHERE "Id" = 1

and finally runs an update:

DEBUG: UPDATE "UserAuth" SET "UserName" = 'admin',"Email" = 'ad...@mail.com',"PrimaryEmail" = NULL,"FirstName" = '',"LastName" = '',"DisplayName" = 'admin',"Salt" = 'JiRQ6w==',"PasswordHash" = 'Lm9QmMBPbX76f4BWXORnlCfS7eN5e0BEJhthkDjmqRI=',"Roles" = '[Admin]',"Permissions" = '[]',"CreatedDate" = '2012-02-27T17:11:36.035902Z',"ModifiedDate" = '2012-02-27T17:11:36.035902Z',"Meta" = NULL WHERE "Id" = 1

My question is :

is this correct behavior?

or am I doing something wrong?

[Demis Bellot]

4 identical requests seems a bit much, so I'll have a look at what's going on.

Thanks

--
- Demis

http://twitter.com/demisbellot

http://www.servicestack.net/mythz_blog

[aicl]

hi,

testing other dialects (postres, mysql) i found that they do not  work !

can you change :

? dbCmd.FirstOrDefault<UserAuth>("Email = {0}", userNameOrEmail)

: dbCmd.FirstOrDefault<UserAuth>("UserName = {0}", userNameOrEmail);

for something like:

?dbCmd.FirstOrDefault<UserAuth>(OrmLiteConfig.DialectProvider.GetNameDelimited("Email")+ " = {0}", userNameOrEmail):

dbCmd.FirstOrDefault<UserAuth>OrmLiteConfig.DialectProvider.GetNameDelimited("UserName")+" = {0}", userNameOrEmail);

this way  columnNames will be quoted correctly !

( or you can use sqlexpressions )

El lunes 27 de febrero de 2012 12:23:03 UTC-5, mythz escribió:

4 identical requests seems a bit much, so I'll have a look at what's going on.

Thanks

[Demis Bellot]

I think SqlExpressions will work better in this case - so I'll change it to that.

Thanks for the heads-up!

[Demis Bellot]

Hi Angel,

I've changed the OrmLite UserAuth queries to use your SQL Expressions so they should escape the columns correctly now :)

OrmLite SqlExpressions FTW!

Still haven't looked into reducing the multiple queries just yet, might do so on the weekend.

[aicl]

thanks Demis,

just note that latest Dlls  ( downloads v 3.59) do not contain this feature

 

 

[Demis Bellot]

Ok that's strange I thought I pushed it in time, but just downloaded and decompiled the latest version from NuGet and it looks like it's not there.

I'll push again tonight.

On Wed, Feb 29, 2012 at 4:10 PM, aicl <angel.ignaci...@gmail.com> wrote:

thanks Demis,

just note that latest Dlls  ( downloads v 3.59) do not contain this feature

 

 

[Demis Bellot]

v3.60 is now up on NuGet + GitHub!

[aicl]

hi demis,

I found that multiple queries are done from method AuthProvider.SaveUserAuth.

AuthProvider.SaveUserAuth calls  authRepo.LoadUserAuth first and then authRepo.SaveUserAuth

authRepo.LoadUserAuth executes again this query: 

dbFactory.Exec(dbCmd => GetUserAuthByUserName(dbCmd, userNameOrEmail)).

(first time this query is execute when  authenticating user)

authRepo.LoadUserAuth also executes:

dbFactory.Exec(dbCmd => dbCmd.Select<UserOAuthProvider>(q => q.UserAuthId == id)).OrderBy(x => x.ModifiedDate).ToList();

authRepo.SaveUserAuth executes:

dbCmd.GetByIdOrDefault<UserAuth>(authSession.UserAuthId) // query 4

dbCmd.Save(userAuth); // update

dbCmd.GetByIdOrDefault<T>(id);   // query 5 : called by dbCmd.Save

I commented:

SaveUserAuth(authService, userSession, authRepo, tokens);

and

OnSaveUserAuth(authService, session);

in method AuthProvider.OnAuthenticated

and now just one query is executed:  when  authenticating user.

I wrote a small demo and runs fine.

For now I can see one side effect : SessionId is something like "j06wHNaQlUK0XJrrUt1Qwg==" ( it was before "1").

btw, datatable UserOAuthProvider is allways empty, which role UserOAuthProvider must play?

and why  dbCmd.Save(userAuth)  is executed? 

[Demis Bellot]

Hi Angel,

I'll try chase these extra calls down sometime soon, but I'm happy that it works now, more than it having an extra 1-2 db calls during Authentication (which is a once-off per user).

The UserOAuthProvider is for OAuth authentication i.e. when you login via Twitter or Facebook where you get additional metadata about the user that could be different amongst each of the services so I use the UserOAuthProvider table to store any OAuth specific metadata I get when authenticating with the OAuth provider, instead of overwriting the master table with external data losing the previous authentication information.

Cheers,

[aicl]

Hi Demis,

After doing more tests, I found that the session does not save all information about the authenticated user (id, mail, ...) (well this because I commented the call to SaveUserAuth (authService, userSession, authRepo , tokens); in AuthProvider.OnAuthenticated)

I made some more changes:

1. IUserAuthRepository:

bool TryAuthenticate(string userName, string password, out UserAuth userAuth);

2. OrmLiteAuthRepository.TryAuthenticate:

public bool TryAuthenticate(string userName, string password, out UserAuth userAuth)

{

//userId = null;

userAuth = GetUserAuthByUserName(userName);

if (userAuth == null) return false;

var saltedHash = new SaltedHash();

if (saltedHash.VerifyHashString(password, userAuth.PasswordHash, userAuth.Salt))

{

//userId = userAuth.Id.ToString(CultureInfo.InvariantCulture);

return true;

}

return false;

}

3.RedisAuthRepository.TryAuthenticate:

public bool TryAuthenticate(string userName, string password, out UserAuth userAuth)

{

//userId = null;

userAuth = GetUserAuthByUserName(userName);

if (userAuth == null) return false;

var saltedHash = new SaltedHash();

if (saltedHash.VerifyHashString(password, userAuth.PasswordHash, userAuth.Salt))

{

//userId = userAuth.Id.ToString(CultureInfo.InvariantCulture);

return true;

}

return false;

}

4.CredentialsAuthProvider:

public virtual bool TryAuthenticate(IServiceBase authService, string userName, string password)

{

var authRepo = authService.TryResolve<IUserAuthRepository>();

if (authRepo == null)

{

Log.WarnFormat("Tried to authenticate without a registered IUserAuthRepository");

return false;

}

var session = authService.GetSession();

UserAuth userAuth = null;

if (authRepo.TryAuthenticate(userName, password, out userAuth))

{

session.PopulateWith(userAuth);

            session.UserAuthId =      userAuth.Id.ToString(CultureInfo.InvariantCulture);

session.IsAuthenticated = true;

return true;

}

return false;

}

 

and now the session contains the user information.
I hope this information can be useful to you.

[Demis Bellot]

Hi Angel,

Any chance you send the changes over in a pull request? Shouldn't be much more effort if you've already made changes in your local fork and it provides a better overview to see what's changed.

Thanks,

[aicl]

hi, 

I just sent pull request

[Demis Bellot]

k kool, got it - will check it out tonight

On Mon, Mar 5, 2012 at 10:51 AM, aicl <angel.ignaci...@gmail.com> wrote:

hi, 

I just sent pull request

[Demis Bellot]

Hi Angel,

I deployed the latest v3.60 of all OrmLite packages + ServiceStack.Text.

With OrmLite I've added a dialect provider of DoesTableExist which OrmLite can use to determine whether or not they should skip table creation - I haven't done this for Firebird since I don't know what it is, so when you have time can you add an implementation of DoesTableExist for firebird?

Unfortunately I wasn't able to deploy ServiceStack since your changes caused a few errors and I ran out of time to track them down and fix them tonight.

If you have a chance can you look at trying to get the tests passing in here - as atm I've got 26 failing tests:

https://github.com/ServiceStack/ServiceStack/tree/master/tests/ServiceStack.Common.Tests/OAuth

Otherwise I'll have another go at fixing them tonight.

Thanks,

Demis

[aicl]

hi Demis,

El martes 6 de marzo de 2012 04:54:58 UTC-5, mythz escribió:

Hi Angel,

1. DoesTableExist  was added

   2. I will check OAuth later...

[aicl]

hi Demis,

1. DoesTableExist  was added to FirebirdSql Dialect.

    2. I sent pull request to get tests passing in OAuth, please review

thanks, 

angel 

 

[Demis Bellot]

Hi Angel,

That did clean up a lot of the tests except this test (on all AuthProviders) is still failing:

ServiceStack.Common.Tests.OAuth.OAuthUserSessionTests 

  Logging_in_pulls_all_AuthInfo_from_repo_after_logging_in_all_AuthProviders()

https://github.com/ServiceStack/ServiceStack/blob/master/tests/ServiceStack.Common.Tests/OAuth/OAuthUserSessionTests.cs#L203

I've had a brief look at the code and I can't see anything wrong with it, except that it doesn't re-hydrate the session properly, the problem appears to be here where it wipes out the previous session because the UserNames don't match:

https://github.com/ServiceStack/ServiceStack/blob/d23abb3a083a4ab070bee87d8b521e13bd7afecc/src/ServiceStack.ServiceInterface/Auth/CredentialsAuthProvider.cs#L74 (this is the original before your changes)

But in order for the UserName to be populated the session needs to be saved in the RegistrationService but the old service didn't actually save the session anywhere:

https://github.com/ServiceStack/ServiceStack/blob/d23abb3a083a4ab070bee87d8b521e13bd7afecc/src/ServiceStack.ServiceInterface/Auth/RegistrationService.cs#L114

So I'm not sure what the problem is, if you have a better idea please have a crack at it, otherwise I'll have to pull the old branch out and debug how the old version worked.

Cheers,

[aicl]

hi Demis, 

good news: all tests pass!

please review last pull request.

angel

[Demis Bellot]

sweet, merged her in - will test her out tonight - if all is well I'll re-deploy tonight.

[aicl]

I forget to commit CredentialsAuthProvider.cs, so I did a new pull request,

my apologize.

angel

[Demis Bellot]

np - still in time for my midnight repo check/deploy :)

[Demis Bellot]

Cool thx Angel - the tests pass now!

I've deployed v3.62 of ServiceStack and ServiceStack.OrmLite to NuGet and GitHub.

Thanks for the speedy fixes :)

Cheers,

[aicl]

Here is a summary of what was done:

Tested for  combination CredentialsAuthProvider + OrmLiteAuthRepository:

1. Queries were reduced from 5 to 2:

UserAuth and UserOAuthProvider.

2. The Update statement is not executed.

In  case CredentialsAuthProvider + OrmLiteAuthRepository, I think

 second query is not necessary, but it was impossible to avoid this query without fail  test "Logging_in_pulls_all_AuthInfo_from_repo_after_logging_in_all_AuthProviders" in the following calls:

Assert.That (oAuthUserSession.TwitterUserId, Is.EqualTo (authInfo ["user_id"]));

Assert.That (oAuthUserSession.TwitterScreenName, Is.EqualTo (authInfo ["screen_name"]));

Assert.That (oAuthUserSession.ProviderOAuthAccess.Count, Is.EqualTo (2));

This "extra" query is made from CredentialAuthProvider.TryAuthenticate in the line:

session.ProviderOAuthAccess = authRepo.GetUserOAuthProviders (session.UserAuthId)

. ConvertAll (x => (IOAuthTokens) x);

Maybe later I return to check this topic,

for now, I find the best option is to implement  CustomCredentialAuthProvider: CredentialAuthProvider to overwrite TryAuthenticate

thanks, 

angel

[Demis Bellot]

Okie that's awesome Angel - always like reducing db hits, although as it's a one-off for Authentication I do prefer we try to optimize for a dev happy API.

But yeah as long as everything works I'm happy! :)