On (05/11/15 17:01), Pavel Reichl wrote: >On 11/05/2015 09:17 AM, Lukas Slebodnik wrote: >>Let's image following use case: >>* cached authentication is enabled. >>* user "pamuser" has never authenticated to the machine and thus >> password is not cached >>* for the first time the the data provider should be contacted. >> ( storing cached password is done in DP, so you still need to >> call sysdb_cache_password) >>* cached authentication shoudl be used for second following authentication. >> and attribute lastOnlineAuthWithCurrentToken should be set be 1st >> authentication so you needn't call >> pam_set_last_online_auth_with_curr_token. >> >>This test-case would test that attribute lastOnlineAuthWithCurrentToken is >>properly set. >> >>Is it clear? > >I hope so, thanks. > >Updated patches attached.
>From 931190aa5faca0833fe285c6815ab955fd706f2d Mon Sep 17 00:00:00 2001 >From: Pavel Reichl <prei...@redhat.com> >Date: Tue, 20 Oct 2015 08:07:02 -0400 >Subject: [PATCH 1/2] pam-srv-tests: split pam_test_setup() so it can be reused > >Split pam_test_setup() so domain and pam parameters can be easily set >distinctly for each test. > >Resolves: >https://fedorahosted.org/sssd/ticket/2697 >--- ACK >From c9d24e577470f3b459c79f7653e7866754266a57 Mon Sep 17 00:00:00 2001 >From: Pavel Reichl <prei...@redhat.com> >Date: Tue, 20 Oct 2015 09:10:30 -0400 >Subject: [PATCH 2/2] pam-srv-tests: Add UT for cached 'online' auth. > >Extend PAM responder unit test to check 'online' cached authentication. > >Resolves: >https://fedorahosted.org/sssd/ticket/2697 >--- > src/responder/pam/pamsrv.h | 5 ++ > src/responder/pam/pamsrv_cmd.c | 2 +- > src/tests/cmocka/test_pam_srv.c | 195 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 201 insertions(+), 1 deletion(-) > >diff --git a/src/responder/pam/pamsrv.h b/src/responder/pam/pamsrv.h >index >59831f2e73f923053e53cad838fac715330546dd..64a7d85735ac3eb20e4504899916a49a280d4959 > 100644 >--- a/src/responder/pam/pamsrv.h >+++ b/src/responder/pam/pamsrv.h >@@ -95,4 +95,9 @@ errno_t add_pam_cert_response(struct pam_data *pd, const >char *user, > const char *token_name); > > bool may_do_cert_auth(struct pam_ctx *pctx, struct pam_data *pd); >+ >+errno_t >+pam_set_last_online_auth_with_curr_token(struct sss_domain_info *domain, >+ const char *username, >+ uint64_t value); > #endif /* __PAMSRV_H__ */ >diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c >index >4bb3e27b102584656e32546f9ad0035750eaeb06..80095cc0bf1c4c92dd04344c069b666ab76a3714 > 100644 >--- a/src/responder/pam/pamsrv_cmd.c >+++ b/src/responder/pam/pamsrv_cmd.c >@@ -1925,7 +1925,7 @@ struct sss_cmd_table *get_pam_cmds(void) > return sss_cmds; > } > >-static errno_t >+errno_t > pam_set_last_online_auth_with_curr_token(struct sss_domain_info *domain, > const char *username, > uint64_t value) >diff --git a/src/tests/cmocka/test_pam_srv.c b/src/tests/cmocka/test_pam_srv.c >index >30fbbc6eaafa7828afd7c37d5bbb581851e5bd97..8a8aa4a7e472242a7b640dd5772d6bbf39640a04 > 100644 >--- a/src/tests/cmocka/test_pam_srv.c >+++ b/src/tests/cmocka/test_pam_srv.c >@@ -71,6 +71,9 @@ > "zotpoBIZmdH+ipYsu58HohHVlM9Wi5H4QmiiXl+Soldkq7eXYlafcmT7wv8+cKwz" \ > "Nz0Tm3+eYpFqRo3skr6QzXi525Jkg3r6r+kkhxU=" \ > >+#define CACHED_AUTH_TIMEOUT_STR "2" >+#define CACHED_AUTH_TIMEOUT 2 It's more type safe to use constants. static char CACHED_AUTH_TIMEOUT_STR[] = "2"; static const int CACHED_AUTH_TIMEOUT = 2; >+ > struct pam_test_ctx { > struct sss_test_ctx *tctx; > struct sss_domain_info *subdom; >@@ -82,6 +85,7 @@ struct pam_test_ctx { > > int ncache_hits; > int exp_pam_status; >+ bool provider_contacted; > }; > > /* Must be global because it is needed in some wrappers */ >@@ -301,6 +305,26 @@ static int pam_test_setup(void **state) > return 0; > } > >+static int pam_cached_test_setup(void **state) >+{ >+ struct sss_test_conf_param dom_params[] = { >+ { "enumerate", "false" }, >+ { "cache_credentials", "true" }, >+ { "cached_auth_timeout", CACHED_AUTH_TIMEOUT_STR }, >+ { NULL, NULL }, /* Sentinel */ >+ }; >+ >+ struct sss_test_conf_param pam_params[] = { >+ { "p11_child_timeout", "30"}, ^ missing space >+ { NULL, NULL }, /* Sentinel */ >+ }; >+ >+ test_pam_setup(dom_params, pam_params, state); >+ >+ pam_test_setup_common(); >+ return 0; >+} >+ > static int pam_test_teardown(void **state) > { > int ret; >@@ -383,6 +407,7 @@ static void set_cmd_cb(cmd_cb_fn_t fn) > > int __wrap_pam_dp_send_req(struct pam_auth_req *preq, int timeout) > { >+ pam_test_ctx->provider_contacted = true; > > /* Set expected status */ > preq->pd->pam_status = pam_test_ctx->exp_pam_status; >@@ -620,6 +645,13 @@ static int >test_pam_successful_offline_auth_check(uint32_t status, > return test_pam_simple_check(status, body, blen); > } > >+static int test_pam_successful_cached_auth_check(uint32_t status, >+ uint8_t *body, size_t blen) >+{ >+ pam_test_ctx->exp_pam_status = PAM_SUCCESS; >+ return test_pam_simple_check(status, body, blen); >+} >+ > static int test_pam_wrong_pw_offline_auth_check(uint32_t status, > uint8_t *body, size_t blen) > { >@@ -814,6 +846,151 @@ void test_pam_preauth(void **state) > assert_int_equal(ret, EOK); > } > >+/* Cached on-line authentication */ >+ >+static void common_test_pam_cached_auth(const char *pwd) >+{ >+ int ret; >+ >+ mock_input_pam(pam_test_ctx, "pamuser", pwd, NULL); >+ >+ will_return(__wrap_sss_packet_get_cmd, SSS_PAM_AUTHENTICATE); >+ will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); >+ >+ pam_test_ctx->exp_pam_status = PAM_SUCCESS; >+ set_cmd_cb(test_pam_successful_cached_auth_check); >+ >+ ret = sss_cmd_execute(pam_test_ctx->cctx, SSS_PAM_AUTHENTICATE, >+ pam_test_ctx->pam_cmds); >+ assert_int_equal(ret, EOK); >+ >+ /* Wait until the test finishes with EOK */ >+ ret = test_ev_loop(pam_test_ctx->tctx); >+ assert_int_equal(ret, EOK); >+} >+ >+void test_pam_cached_auth_success(void **state) >+{ >+ int ret; >+ >+ common_test_pam_cached_auth("12345"); >+ >+ /* Back end should be contacted */ >+ assert_true(pam_test_ctx->provider_contacted); >+ >+ 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); We needn't call pam_set_last_online_auth_with_curr_token here becuase attribute was set in the first authentication. common_test_pam_cached_auth() We have to call sysdb_cache_password becuase it is done on data_provider side >+ /* Reset before next call */ >+ pam_test_ctx->provider_contacted = false; >+ >+ common_test_pam_cached_auth("12345"); >+ >+ /* Back end should not be contacted */ >+ assert_false(pam_test_ctx->provider_contacted); >+} >+ >+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); >+ >+ common_test_pam_cached_auth("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; >+ uint64_t last_online; >+ >+ ret = sysdb_cache_password(pam_test_ctx->tctx->dom, "pamuser", "12345"); >+ assert_int_equal(ret, EOK); >+ >+ last_online = time(NULL) - CACHED_AUTH_TIMEOUT - 1; >+ ret = pam_set_last_online_auth_with_curr_token(pam_test_ctx->tctx->dom, >+ "pamuser", >+ last_online); >+ assert_int_equal(ret, EOK); >+ >+ common_test_pam_cached_auth("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); >+ >+ common_test_pam_cached_auth("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; >+ >+ common_test_pam_cached_auth("12345678"); >+ >+ assert_true(pam_test_ctx->provider_contacted); >+ >+ 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); The same case as in test_pam_cached_auth_success. Calling this function is redundant here. My intention was to test that this attribute is properly stored to cache in pam responder. >+ >+ /* Reset before next call */ >+ pam_test_ctx->provider_contacted = false; >+ >+ common_test_pam_cached_auth("12345678"); >+ >+ assert_false(pam_test_ctx->provider_contacted); >+} >+ >+void test_pam_cached_auth_failed_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); >+ >+ common_test_pam_cached_auth("1111abcde"); >+ >+ assert_true(pam_test_ctx->provider_contacted); >+} >+ >+/* Off-line authentication */ >+ > void test_pam_offline_auth_no_hash(void **state) > { > int ret; >@@ -1421,6 +1598,24 @@ int main(int argc, const char *argv[]) > }; > > const struct CMUnitTest tests[] = { >+ cmocka_unit_test_setup_teardown(test_pam_cached_auth_success, >+ pam_cached_test_setup, >+ pam_test_teardown), >+ cmocka_unit_test_setup_teardown(test_pam_cached_auth_wrong_pw, >+ pam_cached_test_setup, >+ pam_test_teardown), >+ cmocka_unit_test_setup_teardown(test_pam_cached_auth_opt_timeout, >+ pam_cached_test_setup, >+ pam_test_teardown), >+ cmocka_unit_test_setup_teardown(test_pam_cached_auth_timeout, >+ pam_cached_test_setup, >+ pam_test_teardown), >+ >cmocka_unit_test_setup_teardown(test_pam_cached_auth_success_combined_pw_with_cached_2fa, >+ pam_cached_test_setup, >+ pam_test_teardown), >+ >cmocka_unit_test_setup_teardown(test_pam_cached_auth_failed_combined_pw_with_cached_2fa, >+ pam_cached_test_setup, >+ pam_test_teardown), In most cases we add unit test to the end of array. I tried it and all test passed. Please fix small issues and you have my ACK. LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
