[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