From 38dee179e7f12673dc726ec10ce0d35857df6796 Mon Sep 17 00:00:00 2001 From: Bill Williams Date: Thu, 30 May 2019 13:44:49 -0700 Subject: [PATCH] request new auth token on every retry, not just after 403 --- CHANGELOG.md | 1 + src/connection.c | 46 +++++++++++++++++-------------- src/main.c | 1 - tests/test_connection.c | 60 +++++++++++++++-------------------------- 4 files changed, 49 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bd2a9d..47b2e11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Rename command line options for MTLS cert and Key - Update to use nanomsg v. 1.1.4 - requestNewAuthToken will clear the token if it fails. +- request auth token on every retry, not just after 403 ## [1.0.2] - 2019-02-08 - Refactored connection.c and updated corresponding unit tests diff --git a/src/connection.c b/src/connection.c index c7b3cd8..3f137fe 100644 --- a/src/connection.c +++ b/src/connection.c @@ -296,21 +296,23 @@ void set_current_server (create_connection_ctx_t *ctx) ctx->current_server = get_current_server (&ctx->server_list); } -void set_extra_headers (create_connection_ctx_t *ctx, int reauthorize) -{ - if (reauthorize && (get_parodus_cfg()->client_cert_path !=NULL && strlen(get_parodus_cfg()->client_cert_path) >0)) - { - getAuthToken(get_parodus_cfg()); - } - - ctx->extra_headers = build_extra_hdrs (&ctx->header_info); -} - void free_extra_headers (create_connection_ctx_t *ctx) { FREE_PTR_VAR (ctx->extra_headers) } +void set_extra_headers (create_connection_ctx_t *ctx) +{ + ParodusCfg * cfg = get_parodus_cfg(); + free_extra_headers (ctx); + if ((cfg->client_cert_path != NULL) && (strlen(cfg->client_cert_path) > 0)) + { + getAuthToken(cfg); + } + + ctx->extra_headers = build_extra_hdrs (&ctx->header_info); +} + void free_connection_ctx (create_connection_ctx_t *ctx) { free_extra_headers (ctx); @@ -404,8 +406,9 @@ int nopoll_connect (create_connection_ctx_t *ctx, int is_ipv6) //-------------------------------------------------------------------- // Return codes for wait_connection_ready #define WAIT_SUCCESS 0 -#define WAIT_ACTION_RETRY 1 // if wait_status is 307, 302, 303 or 403 -#define WAIT_FAIL 2 +#define WAIT_ACTION_RETRY 1 // if wait_status is 307, 302 or 303 +#define WAIT_ACTION_BACKOFF 2 // if wait status is 403 +#define WAIT_FAIL 3 #define FREE_NON_NULL_PTR(ptr) if (NULL != ptr) free(ptr) @@ -441,9 +444,7 @@ int wait_connection_ready (create_connection_ctx_t *ctx) if(wait_status == 403) { ParodusError("Received Unauthorized response with status: %d\n", wait_status); - free_extra_headers (ctx); - set_extra_headers (ctx, true); - return WAIT_ACTION_RETRY; + return WAIT_ACTION_BACKOFF; } ParodusError("Client connection timeout\n"); ParodusError("RDK-10037 - WebPA Connection Lost\n"); @@ -453,9 +454,10 @@ int wait_connection_ready (create_connection_ctx_t *ctx) //-------------------------------------------------------------------- // Return codes for connect_and_wait -#define CONN_WAIT_SUCCESS 0 -#define CONN_WAIT_ACTION_RETRY 1 // if wait_status is 307, 302, 303 or 403 -#define CONN_WAIT_RETRY_DNS 2 +#define CONN_WAIT_SUCCESS 0 +#define CONN_WAIT_ACTION_RETRY 1 // if wait_status is 307, 302 or 303 +#define CONN_WAIT_ACTION_BACKOFF 2 // if wait status is 403 +#define CONN_WAIT_RETRY_DNS 3 int connect_and_wait (create_connection_ctx_t *ctx) { @@ -493,6 +495,9 @@ int connect_and_wait (create_connection_ctx_t *ctx) if (wait_rtn == WAIT_ACTION_RETRY) return CONN_WAIT_ACTION_RETRY; + if (wait_rtn == WAIT_ACTION_BACKOFF) + return CONN_WAIT_ACTION_BACKOFF; + // try ipv4 if we need to if ((0==force_flags) && (0==ctx->current_server->allow_insecure) && is_ipv6) { is_ipv6 = false; @@ -514,6 +519,7 @@ int keep_trying_to_connect (create_connection_ctx_t *ctx, while (true) { + set_extra_headers (ctx); rtn = connect_and_wait (ctx); if (rtn == CONN_WAIT_SUCCESS) return true; @@ -552,11 +558,11 @@ int createNopollConnection(noPollCtx *ctx) max_retry_count = (int) get_parodus_cfg()->webpa_backoff_max; ParodusPrint("max_retry_count is %d\n", max_retry_count ); - + + memset (&conn_ctx, 0, sizeof(create_connection_ctx_t)); conn_ctx.nopoll_ctx = ctx; init_expire_timer (&conn_ctx.connect_timer); init_header_info (&conn_ctx.header_info); - set_extra_headers (&conn_ctx, false); set_server_list_null (&conn_ctx.server_list); init_backoff_timer (&backoff_timer, max_retry_count); diff --git a/src/main.c b/src/main.c index b898306..b03870c 100644 --- a/src/main.c +++ b/src/main.c @@ -83,7 +83,6 @@ int main( int argc, char **argv) abort(); } curl_global_init(CURL_GLOBAL_DEFAULT); - getAuthToken(cfg); createSocketConnection( NULL); diff --git a/tests/test_connection.c b/tests/test_connection.c index b14e69f..a43a382 100644 --- a/tests/test_connection.c +++ b/tests/test_connection.c @@ -117,6 +117,8 @@ nopoll_bool nopoll_conn_wait_for_status_until_connection_ready (noPollConn * co int timeout, int *status, char ** message) { UNUSED(conn); UNUSED(timeout); + + *message = NULL; if (mock_wait_status >= 1000) { *status = mock_wait_status / 1000; mock_wait_status = mock_wait_status % 1000; @@ -375,6 +377,15 @@ void test_set_current_server() assert_ptr_equal (&ctx.server_list.defaults, ctx.current_server); } +void init_cfg_header_info (ParodusCfg *cfg) +{ + parStrncpy(cfg->hw_mac , "123567892366", sizeof(cfg->hw_mac)); + parStrncpy(cfg->hw_model, "TG1682", sizeof(cfg->hw_model)); + parStrncpy(cfg->hw_manufacturer , "ARRISGroup,Inc.", sizeof(cfg->hw_manufacturer)); + parStrncpy(cfg->fw_name , "2.364s2", sizeof(cfg->fw_name)); + parStrncpy(cfg->webpa_protocol , "WebPA-1.6", sizeof(cfg->webpa_protocol)); +} + void test_set_extra_headers () { int rtn; @@ -391,12 +402,8 @@ void test_set_extra_headers () cfg.client_cert_path = strdup("testcert"); parStrncpy(cfg.hw_serial_number, "Fer23u948590", sizeof(cfg.hw_serial_number)); - parStrncpy(cfg.hw_mac , "123567892366", sizeof(cfg.hw_mac)); - parStrncpy(cfg.hw_model, "TG1682", sizeof(cfg.hw_model)); - parStrncpy(cfg.hw_manufacturer , "ARRISGroup,Inc.", sizeof(cfg.hw_manufacturer)); - parStrncpy(cfg.fw_name , "2.364s2", sizeof(cfg.fw_name)); + init_cfg_header_info (&cfg); parStrncpy(cfg.cert_path , "/etc/ssl/certs/ca-certificates.crt", sizeof(cfg.cert_path)); - parStrncpy(cfg.webpa_protocol , "WebPA-1.6", sizeof(cfg.webpa_protocol)); set_parodus_cfg(&cfg); rtn = init_header_info (&ctx.header_info); @@ -578,19 +585,13 @@ void test_nopoll_connect () // Return codes for wait_connection_ready #define WAIT_SUCCESS 0 -#define WAIT_ACTION_RETRY 1 // if wait_status is 307, 302, 303 or 403 -#define WAIT_FAIL 2 +#define WAIT_ACTION_RETRY 1 // if wait_status is 307, 302 or 303 +#define WAIT_ACTION_BACKOFF 2 // if wait status is 403 +#define WAIT_FAIL 3 void test_wait_connection_ready () { create_connection_ctx_t ctx; - ParodusCfg Cfg; - const char *expected_extra_headers = - "\r\nAuthorization: Bearer Auth---" - "\r\nX-WebPA-Device-Name: mac:123567892366" - "\r\nX-WebPA-Device-Protocols: wrp-0.11,getset-0.1" - "\r\nUser-Agent: WebPA-1.6 (2.364s2; TG1682/ARRISGroup,Inc.;)" - "\r\nX-WebPA-Convey: WebPA-1.6 (TG1682)"; memset(&ctx,0,sizeof(ctx)); set_server_list_null (&ctx.server_list); @@ -605,7 +606,7 @@ void test_wait_connection_ready () will_return (nopoll_conn_wait_for_status_until_connection_ready, nopoll_false); expect_function_call (nopoll_conn_wait_for_status_until_connection_ready); assert_int_equal (wait_connection_ready (&ctx), WAIT_FAIL); - + mock_wait_status = 503; will_return (nopoll_conn_wait_for_status_until_connection_ready, nopoll_false); expect_function_call (nopoll_conn_wait_for_status_until_connection_ready); @@ -640,30 +641,19 @@ void test_wait_connection_ready () free_server (&ctx.server_list.redirect); mock_wait_status = 403; - memset(&Cfg, 0, sizeof(ParodusCfg)); - - parStrncpy (Cfg.webpa_auth_token, "Auth---", sizeof (Cfg.webpa_auth_token)); - parStrncpy(Cfg.hw_model, "TG1682", sizeof(Cfg.hw_model)); - parStrncpy(Cfg.hw_manufacturer , "ARRISGroup,Inc.", sizeof(Cfg.hw_manufacturer)); - parStrncpy(Cfg.hw_mac , "123567892366", sizeof(Cfg.hw_mac)); - parStrncpy(Cfg.fw_name , "2.364s2", sizeof(Cfg.fw_name)); - parStrncpy(Cfg.webpa_protocol , "WebPA-1.6", sizeof(Cfg.webpa_protocol)); - set_parodus_cfg(&Cfg); - - init_header_info (&ctx.header_info); will_return (nopoll_conn_wait_for_status_until_connection_ready, nopoll_false); expect_function_call (nopoll_conn_wait_for_status_until_connection_ready); - assert_int_equal (wait_connection_ready (&ctx), WAIT_ACTION_RETRY); - - assert_string_equal (ctx.extra_headers, expected_extra_headers); + assert_int_equal (wait_connection_ready (&ctx), WAIT_ACTION_BACKOFF); + free_extra_headers (&ctx); free_header_info (&ctx.header_info); } // Return codes for connect_and_wait #define CONN_WAIT_SUCCESS 0 -#define CONN_WAIT_ACTION_RETRY 1 // if wait_status is 307, 302, 303 or 403 -#define CONN_WAIT_RETRY_DNS 2 +#define CONN_WAIT_ACTION_RETRY 1 // if wait_status is 307, 302 or 303 +#define CONN_WAIT_ACTION_BACKOFF 2 // if wait status is 403 +#define CONN_WAIT_RETRY_DNS 3 void test_connect_and_wait () { @@ -815,22 +805,16 @@ void test_keep_trying () server_t test_server; backoff_timer_t backoff_timer; ParodusCfg Cfg; - char *test_extra_headers = - "\r\nAuthorization: Bearer SER_MAC Fer23u948590 123567892366" - "\r\nX-WebPA-Device-Name: mac:123567892366" - "\r\nX-WebPA-Device-Protocols: wrp-0.11,getset-0.1" - "\r\nUser-Agent: WebPA-1.6 (2.364s2; TG1682/ARRISGroup,Inc.;)" - "\r\nX-WebPA-Convey: WebPA-1.6 (TG1682)"; memset(&Cfg, 0, sizeof(ParodusCfg)); parStrncpy (Cfg.webpa_url, "http://mydns.mycom.net:8080", sizeof(Cfg.webpa_url)); + init_cfg_header_info (&Cfg); mock_wait_status = 0; memset(&ctx,0,sizeof(ctx)); ctx.nopoll_ctx = &test_nopoll_ctx; ctx.current_server = &test_server; - ctx.extra_headers = test_extra_headers; test_server.allow_insecure = 1; test_server.server_addr = "mydns.mycom.net";