[SSSD] [PATCH] TESTS: extend PAM responder unit test
Lukas Slebodnik
lslebodn at redhat.com
Wed Nov 4 12:42:25 UTC 2015
On (20/10/15 15:21), Pavel Reichl wrote:
>Hello,
>
>please see first iteration of extended PAM responder unit tests.
>From 7cd365de5c306cb42901e510ff14df1b76c6bddb Mon Sep 17 00:00:00 2001
>From: Pavel Reichl <preichl at redhat.com>
>Date: Tue, 20 Oct 2015 08:07:02 -0400
>Subject: [PATCH 1/3] 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
>---
> src/tests/cmocka/test_pam_srv.c | 34 +++++++++++++++++++---------------
> 1 file changed, 19 insertions(+), 15 deletions(-)
>
>diff --git a/src/tests/cmocka/test_pam_srv.c b/src/tests/cmocka/test_pam_srv.c
>index dbdc4ae08a12914481137fe8fb5a24d242d3032f..e15a2a40428c2b12301de4690d729b4073119c2f 100644
>--- a/src/tests/cmocka/test_pam_srv.c
>+++ b/src/tests/cmocka/test_pam_srv.c
>@@ -247,22 +247,9 @@ void test_pam_setup(struct sss_test_conf_param dom_params[],
> pam_test_ctx->cctx->ev = pam_test_ctx->tctx->ev;
> }
>
>-static int pam_test_setup(void **state)
>+static void pam_test_setup_common(void)
> {
>- int ret;
>-
>- struct sss_test_conf_param dom_params[] = {
>- { "enumerate", "false" },
>- { "cache_credentials", "true" },
>- { NULL, NULL }, /* Sentinel */
>- };
>-
>- struct sss_test_conf_param pam_params[] = {
>- { "p11_child_timeout", "30"},
>- { NULL, NULL }, /* Sentinel */
>- };
>-
>- test_pam_setup(dom_params, pam_params, state);
>+ errno_t ret;
>
> /* Prime the cache with a valid user */
> ret = sysdb_add_user(pam_test_ctx->tctx->dom,
>@@ -293,7 +280,24 @@ static int pam_test_setup(void **state)
> discard_const("wronguser"),
> pam_test_ctx->pctx->id_timeout);
> assert_int_equal(ret, EOK);
>+}
>
>+static int pam_test_setup(void **state)
>+{
>+ struct sss_test_conf_param dom_params[] = {
>+ { "enumerate", "false" },
>+ { "cache_credentials", "true" },
>+ { 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;
> }
>
>--
>2.4.3
>
>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.
>
>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 | 175 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 181 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 2823f8133eb74d245be0750193ed842c0fdb26d3..ae66124b761ef301358c9b4884c5a1824708d73d 100644
>--- a/src/responder/pam/pamsrv_cmd.c
>+++ b/src/responder/pam/pamsrv_cmd.c
>@@ -1928,7 +1928,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 e15a2a40428c2b12301de4690d729b4073119c2f..8b55b48d28e916843f3899104a1263ab005769d7 100644
>--- a/src/tests/cmocka/test_pam_srv.c
>+++ b/src/tests/cmocka/test_pam_srv.c
>@@ -82,6 +82,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 +302,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", "2" },
>+ { NULL, NULL }, /* Sentinel */
>+ };
>+
>+ struct sss_test_conf_param pam_params[] = {
>+ { "p11_child_timeout", "30"},
>+ { 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 +404,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 +642,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)
^
extra space
>+{
>+ 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 +843,134 @@ void test_pam_preauth(void **state)
> assert_int_equal(ret, EOK);
> }
>
>+/* Cached on-line authentication */
>+
>+static void test_pam_cached_auth_common(const char *pwd)
^^^^
It would be good to change prefix.
The string "test_" evokes that it is called by cmocka
and maybe has setup and common function.
But the function test_pam_cached_auth_common is
not part of "const struct CMUnitTest tests[]"
>+{
>+ 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;
>+
>+ 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.
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.
>+
>+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.
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.
LS
More information about the sssd-devel
mailing list