[SSSD] [PATCH] TESTS: extend PAM responder unit test
Pavel Reichl
preichl at redhat.com
Wed Nov 4 15:03:59 UTC 2015
On 11/04/2015 01:42 PM, Lukas Slebodnik wrote:
>>From fa082a04387ed83f1b12316d388b63a08ba1305d Mon Sep 17 00:00:00 2001
>> From: Pavel Reichl <preichl at redhat.com>
>> Date: Tue, 20 Oct 2015 09:10:30 -0400
>> Subject: [PATCH 2/3] TESTS: extend PAM responder unit test
>>
>> Extend PAM responder unit test to check 'online' cached authentication.
>>
>
>> +void test_pam_cached_auth_success(void **state)
>> +{
>> + int ret;
>> +
>> + ret = sysdb_cache_password(pam_test_ctx->tctx->dom, "pamuser", "12345");
>> + assert_int_equal(ret, EOK);
>> +
>> + ret = pam_set_last_online_auth_with_curr_token(pam_test_ctx->tctx->dom,
>> + "pamuser", time(NULL));
>> + assert_int_equal(ret, EOK);
>> +
>> + test_pam_cached_auth_common("12345");
>> +
>> + /* Back end should not be contacted */
>> + assert_false(pam_test_ctx->provider_contacted);
>> +}
> it would be also good to test two subsequent authentications.
Which two subsequent authentications do you mean? I suppose you don't mean test_pam_cached_auth_wrong_pw() and test_pam_cached_auth_opt_timeout() as both of them are already tested.
> In first case, a data provider should be contacted and in the
> next one should not be contacted.
>
> So you would not need to use pam_set_last_online_auth_with_curr_token.
I need answer to prev. question to properly answer these comments.
>
>
>> +
>> +void test_pam_cached_auth_wrong_pw(void **state)
>> +{
>> + int ret;
>> +
>> + ret = sysdb_cache_password(pam_test_ctx->tctx->dom, "pamuser", "12345");
>> + assert_int_equal(ret, EOK);
>> +
>> + ret = pam_set_last_online_auth_with_curr_token(pam_test_ctx->tctx->dom,
>> + "pamuser", time(NULL));
>> + assert_int_equal(ret, EOK);
>> +
>> + test_pam_cached_auth_common("11111");
>> +
>> + /* Back end should be contacted */
>> + assert_true(pam_test_ctx->provider_contacted);
>> +}
>> +
>> +/* test cached_auth_timeout option */
>> +void test_pam_cached_auth_opt_timeout(void **state)
>> +{
>> + int ret;
>> +
>> + ret = sysdb_cache_password(pam_test_ctx->tctx->dom, "pamuser", "12345");
>> + assert_int_equal(ret, EOK);
>> +
>> + ret = pam_set_last_online_auth_with_curr_token(pam_test_ctx->tctx->dom,
>> + "pamuser", time(NULL));
>> + sleep(4);
> It would be good to avoid sleep in unit tests. (Especially with hardcoded
> magic constant). The reason is that unit test should be fast.
Yes, but we are testing expiration here, I can call pam_set_last_online_auth_with_curr_token() as you propose but the test will be more artificial, but no doubt it will be faster.
It's just trade off between test factuality and speed.
OK, I'll do it.
>
> You decided to make the function pam_set_last_online_auth_with_curr_token
> public so you can use it to set lasst authentication to past.
>
>> + assert_int_equal(ret, EOK);
>> +
>> + test_pam_cached_auth_common("12345");
>> +
>> + /* Back end should be contacted */
>> + assert_true(pam_test_ctx->provider_contacted);
>> +}
>> +
>> +/* too long since last on-line authentication */
>> +void test_pam_cached_auth_timeout(void **state)
>> +{
>> + int ret;
>> +
>> + ret = sysdb_cache_password(pam_test_ctx->tctx->dom, "pamuser", "12345");
>> + assert_int_equal(ret, EOK);
>> +
>> + ret = pam_set_last_online_auth_with_curr_token(pam_test_ctx->tctx->dom,
>> + "pamuser", 0);
>> + assert_int_equal(ret, EOK);
>> +
>> + test_pam_cached_auth_common("12345");
>> +
>> + /* Back end should be contacted */
>> + assert_true(pam_test_ctx->provider_contacted);
>> +}
>> +
>> +void test_pam_cached_auth_success_combined_pw_with_cached_2fa(void **state)
>> +{
>> + int ret;
>> +
>> + ret = sysdb_cache_password_ex(pam_test_ctx->tctx->dom, "pamuser",
>> + "12345678", SSS_AUTHTOK_TYPE_2FA, 5);
>> + assert_int_equal(ret, EOK);
>> + ret = pam_set_last_online_auth_with_curr_token(pam_test_ctx->tctx->dom,
>> + "pamuser", time(NULL));
>> + assert_int_equal(ret, EOK);
>> +
>> + test_pam_cached_auth_common("12345678");
>> +
>> + assert_false(pam_test_ctx->provider_contacted);
>> +}
> it would be also good to test two subsequent authentications.
> In first case, a data provider should be contacted and in the
> next one should not be contacted.
>
Here you mean: test_pam_offline_auth_no_hash() and test_pam_offline_auth_success() ? If so, do you prefer this to be a separate patch as it IMO doen't relate to testing cached authentication?
>
> LS
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
More information about the sssd-devel
mailing list