From fba6bb8a970d761e6c8fa38f0db66ffc59dc1cea Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Wed, 24 Jan 2007 00:09:13 +0000 Subject: [PATCH] NIM Bug Fixes Document User Interface Callbacks Fix a race condition when performing renewal actions triggered by command line parameters. When importing credentials, kickoff a renewal after the credentials after the API: ccache is created. Another fix for identity expiration states. This one fixes the behavior of the system tray icon. ticket: new component: windows git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@19110 dc483132-0cff-0310-8789-dd5450dbe970 --- src/windows/identity/ui/appglobal.h | 1 + src/windows/identity/ui/credfuncs.c | 201 ++++++++++++++++++------- src/windows/identity/ui/credfuncs.h | 3 + src/windows/identity/ui/credwnd.c | 4 +- src/windows/identity/uilib/khnewcred.h | 6 +- src/windows/identity/uilib/khuidefs.h | 30 ++++ 6 files changed, 186 insertions(+), 59 deletions(-) diff --git a/src/windows/identity/ui/appglobal.h b/src/windows/identity/ui/appglobal.h index 952996107..8660de2b4 100644 --- a/src/windows/identity/ui/appglobal.h +++ b/src/windows/identity/ui/appglobal.h @@ -47,6 +47,7 @@ typedef struct tag_khm_startup_options_v1 { BOOL init; BOOL import; BOOL renew; + LONG pending_renewals; BOOL destroy; BOOL autoinit; diff --git a/src/windows/identity/ui/credfuncs.c b/src/windows/identity/ui/credfuncs.c index dcb70480e..470af4f26 100644 --- a/src/windows/identity/ui/credfuncs.c +++ b/src/windows/identity/ui/credfuncs.c @@ -367,6 +367,7 @@ kmsg_cred_completion(kmq_message *m) /* all is done. */ { khui_new_creds * nc; + khm_boolean continue_cmdline = FALSE; nc = (khui_new_creds *) m->vparam; @@ -379,13 +380,28 @@ kmsg_cred_completion(kmq_message *m) */ khm_cred_end_dialog(nc); + } else if (nc->subtype == KMSG_CRED_RENEW_CREDS) { + + /* if this is a renewal that was triggered while we + were processing the commandline, then we need to + update the pending renewal count. */ + + if (khm_startup.processing) { + LONG renewals; + renewals = InterlockedDecrement(&khm_startup.pending_renewals); + + if (renewals == 0) { + continue_cmdline = TRUE; + } + } } khui_cw_destroy_cred_blob(nc); kmq_post_message(KMSG_CRED, KMSG_CRED_REFRESH, 0, 0); - kmq_post_message(KMSG_ACT, KMSG_ACT_CONTINUE_CMDLINE, 0, 0); + if (continue_cmdline) + kmq_post_message(KMSG_ACT, KMSG_ACT_CONTINUE_CMDLINE, 0, 0); } break; @@ -416,7 +432,35 @@ kmsg_cred_completion(kmq_message *m) break; case KMSG_CRED_IMPORT: - kmq_post_message(KMSG_ACT, KMSG_ACT_CONTINUE_CMDLINE, 0, 0); + { + khm_boolean continue_cmdline = FALSE; + LONG pending_renewals; + + /* once an import operation ends, we have to trigger a + renewal so that other plug-ins that didn't participate + in the import operation can have a chance at getting + the necessary credentials. + + If we are in the middle of processing the commandline, + we have to be a little bit careful. We can't issue a + commandline conituation message right now because the + import action is still ongoing (since the renewals are + part of the action). Once the renewals have completed, + the completion handler will automatically issue a + commandline continuation message. However, if there + were no identities to renew, then we have to issue the + message ourselves. + */ + + InterlockedIncrement(&khm_startup.pending_renewals); + + khm_cred_renew_all_identities(); + + pending_renewals = InterlockedDecrement(&khm_startup.pending_renewals); + + if (pending_renewals == 0 && khm_startup.processing) + kmq_post_message(KMSG_ACT, KMSG_ACT_CONTINUE_CMDLINE, 0, 0); + } break; case KMSG_CRED_REFRESH: @@ -540,6 +584,71 @@ void khm_cred_destroy_identity(khm_handle identity) _end_task(); } +void khm_cred_renew_all_identities(void) +{ + khm_size count; + khm_size cb = 0; + khm_size n_idents = 0; + khm_int32 rv; + wchar_t * ident_names = NULL; + wchar_t * this_ident; + + kcdb_credset_get_size(NULL, &count); + + /* if there are no credentials, we just skip over the renew + action. */ + + if (count == 0) + return; + + ident_names = NULL; + + while (TRUE) { + if (ident_names) { + PFREE(ident_names); + ident_names = NULL; + } + + cb = 0; + rv = kcdb_identity_enum(KCDB_IDENT_FLAG_EMPTY, 0, + NULL, + &cb, &n_idents); + + if (n_idents == 0 || rv != KHM_ERROR_TOO_LONG || + cb == 0) + break; + + ident_names = PMALLOC(cb); + ident_names[0] = L'\0'; + + rv = kcdb_identity_enum(KCDB_IDENT_FLAG_EMPTY, 0, + ident_names, + &cb, &n_idents); + + if (KHM_SUCCEEDED(rv)) + break; + } + + if (ident_names) { + for (this_ident = ident_names; + this_ident && *this_ident; + this_ident = multi_string_next(this_ident)) { + khm_handle ident; + + if (KHM_FAILED(kcdb_identity_create(this_ident, 0, + &ident))) + continue; + + khm_cred_renew_identity(ident); + + kcdb_identity_release(ident); + } + + PFREE(ident_names); + ident_names = NULL; + } +} + void khm_cred_renew_identity(khm_handle identity) { khui_new_creds * c; @@ -558,6 +667,12 @@ void khm_cred_renew_identity(khm_handle identity) _report_sr0(KHERR_NONE, IDS_CTX_RENEW_CREDS); _describe(); + /* if we are calling this while processing startup actions, we + need to keep track of how many we have issued. */ + if (khm_startup.processing) { + InterlockedIncrement(&khm_startup.pending_renewals); + } + kmq_post_message(KMSG_CRED, KMSG_CRED_RENEW_CREDS, 0, (void *) c); _end_task(); @@ -581,6 +696,12 @@ void khm_cred_renew_cred(khm_handle cred) _report_sr0(KHERR_NONE, IDS_CTX_RENEW_CREDS); _describe(); + /* if we are calling this while processing startup actions, we + need to keep track of how many we have issued. */ + if (khm_startup.processing) { + InterlockedIncrement(&khm_startup.pending_renewals); + } + kmq_post_message(KMSG_CRED, KMSG_CRED_RENEW_CREDS, 0, (void *) c); _end_task(); @@ -599,6 +720,12 @@ void khm_cred_renew_creds(void) _report_sr0(KHERR_NONE, IDS_CTX_RENEW_CREDS); _describe(); + /* if we are calling this while processing startup actions, we + need to keep track of how many we have issued. */ + if (khm_startup.processing) { + InterlockedIncrement(&khm_startup.pending_renewals); + } + kmq_post_message(KMSG_CRED, KMSG_CRED_RENEW_CREDS, 0, (void *) c); _end_task(); @@ -947,73 +1074,37 @@ khm_cred_process_startup_actions(void) { if (khm_startup.import) { khm_cred_import(); khm_startup.import = FALSE; + + /* we also set the renew command to false here because we + trigger a renewal for all the identities at the end of + the import operation anyway. */ + khm_startup.renew = FALSE; break; } if (khm_startup.renew) { - khm_size count; - wchar_t * ident_names = NULL; - wchar_t * this_ident; - - kcdb_credset_get_size(NULL, &count); + LONG pending_renewals; /* if there are no credentials, we just skip over the renew action. */ khm_startup.renew = FALSE; - if (count != 0) { - khm_size cb = 0; - khm_size n_idents = 0; - khm_int32 rv; - - ident_names = NULL; - - while (TRUE) { - if (ident_names) { - PFREE(ident_names); - ident_names = NULL; - } - - cb = 0; - rv = kcdb_identity_enum(KCDB_IDENT_FLAG_EMPTY, 0, - NULL, - &cb, &n_idents); - - if (n_idents == 0 || rv != KHM_ERROR_TOO_LONG || - cb == 0) - break; - - ident_names = PMALLOC(cb); - - rv = kcdb_identity_enum(KCDB_IDENT_FLAG_EMPTY, 0, - ident_names, - &cb, &n_idents); + InterlockedIncrement(&khm_startup.pending_renewals); - if (KHM_SUCCEEDED(rv)) - break; - } - - if (ident_names) { - for (this_ident = ident_names; - this_ident && *this_ident; - this_ident = multi_string_next(this_ident)) { - khm_handle ident; - - if (KHM_FAILED(kcdb_identity_create(this_ident, 0, - &ident))) - continue; - - khm_cred_renew_identity(ident); + khm_cred_renew_all_identities(); - kcdb_identity_release(ident); - } + pending_renewals = InterlockedDecrement(&khm_startup.pending_renewals); - PFREE(ident_names); - ident_names = NULL; - } + if (pending_renewals != 0) break; - } + + /* if there were no pending renewals, then we just fall + through. This means that either there were no + identities to renew, or all the renewals completed. If + all the renewals completed, then the commandline + contiuation message wasn't triggered. Either way, we + must fall through if the count is zero. */ } if (khm_startup.destroy) { diff --git a/src/windows/identity/ui/credfuncs.h b/src/windows/identity/ui/credfuncs.h index 379573ff5..b3c88faa4 100644 --- a/src/windows/identity/ui/credfuncs.h +++ b/src/windows/identity/ui/credfuncs.h @@ -38,6 +38,9 @@ khm_cred_destroy_creds(khm_boolean sync, void khm_cred_destroy_identity(khm_handle identity); +void +khm_cred_renew_all_identities(void); + void khm_cred_renew_identity(khm_handle identity); diff --git a/src/windows/identity/ui/credwnd.c b/src/windows/identity/ui/credwnd.c index d8dfcc503..5bdbdb754 100644 --- a/src/windows/identity/ui/credwnd.c +++ b/src/windows/identity/ui/credwnd.c @@ -1254,6 +1254,7 @@ cw_update_outline(khui_credwnd_tbl * tbl) if (flags) { ol->flags |= flags; ol->flags |= KHUI_CW_O_SHOWFLAG; + expstate |= flags; } } } @@ -1264,7 +1265,6 @@ cw_update_outline(khui_credwnd_tbl * tbl) visible = visible && (ol->flags & KHUI_CW_O_EXPAND); flags = cw_get_buf_exp_flags(tbl, thiscred); - expstate |= flags; if(visible) { khm_int32 c_flags; @@ -1404,6 +1404,8 @@ _exit: if(grouping) PFREE(grouping); + /* note that the expstate is derived from whether or not + * we have expiration states set for any active identities */ if (n_creds == 0) khm_notify_icon_expstate(KHM_NOTIF_EMPTY); else if (expstate & CW_EXPSTATE_EXPIRED) diff --git a/src/windows/identity/uilib/khnewcred.h b/src/windows/identity/uilib/khnewcred.h index b2b014e4f..1785d5972 100644 --- a/src/windows/identity/uilib/khnewcred.h +++ b/src/windows/identity/uilib/khnewcred.h @@ -225,9 +225,9 @@ typedef struct tag_khui_new_creds { khm_int32 subtype; /*!< Subtype of the request that is being handled through this object. - One of ::KMSG_CRED_INITIAL_CREDS, - ::KMSG_CRED_NEW_CREDS or - ::KMSG_CRED_RENEW_CREDS */ + One of ::KMSG_CRED_NEW_CREDS, + ::KMSG_CRED_RENEW_CREDS or + ::KMSG_CRED_PASSWORD */ CRITICAL_SECTION cs; /*!< Internal use */ diff --git a/src/windows/identity/uilib/khuidefs.h b/src/windows/identity/uilib/khuidefs.h index 7df605cc7..845f781d2 100644 --- a/src/windows/identity/uilib/khuidefs.h +++ b/src/windows/identity/uilib/khuidefs.h @@ -88,9 +88,39 @@ khm_get_lib_version(khm_version * libver, khm_ui_4 * apiver); KHMEXP khm_ui_4 KHMAPI khm_get_commctl_version(khm_version * pdvi); +/*! \brief UI callback function + + Used with khui_request_UI_callback(). + + \see khui_request_UI_callback() + */ typedef khm_int32 (KHMAPI *khm_ui_callback)(HWND hwnd_main_wnd, void * rock); +/*! \brief Request a UI callback + + In general, plug-ins in Network Identity Manager run in their own + thread and will not be able to interact with the user directly by + creating windows of its own. There are exceptions to this, such + as when the plug-in is responding to a new credentials request or + if the plug-in provides configuration panels. However, if a + plug-in needs to provide a user interface to the user outside of + the provisions already provided by Network Identity Manager, it + needs to do so from within the user interface thread. + + To do so, a plug-in would provide a callback function of the type + ::khm_ui_callback to this function. The Network Identity Manager + will then call the callback function from within the user + interface thread. At this point, the callback function can create + any windows it wishes to create and interact with the user + directly. + + Note that when the plug-in creates any windows, it should specify + the window handle provided via the \a hwnd_main_wnd parameter as + the parent window. + + \see ::khm_ui_callback + */ KHMEXP khm_int32 KHMAPI khui_request_UI_callback(khm_ui_callback cb, void * rock); -- 2.26.2