From 62bcc3d472a982085185002666b65fd69b119aba Mon Sep 17 00:00:00 2001 From: Art Cancro Date: Mon, 2 Jun 2008 03:04:23 +0000 Subject: [PATCH] Completed the delete-user hook to remove any associated OpenID records. Also completed an auto-purger function to delete any stale OpenID associations. Still need to add dump/load code. Now I remember why I tend to avoid adding top-level database tables. --- citadel/citadel.h | 2 +- citadel/modules/expire/serv_expire.c | 62 +++++++++++++++++++++++++ citadel/modules/openid/serv_openid_rp.c | 35 ++++++++++---- libcitadel/lib/hash.c | 13 ++++++ libcitadel/lib/libcitadel.h | 5 +- 5 files changed, 106 insertions(+), 11 deletions(-) diff --git a/citadel/citadel.h b/citadel/citadel.h index ba9732542..5db1181d0 100644 --- a/citadel/citadel.h +++ b/citadel/citadel.h @@ -41,7 +41,7 @@ extern "C" { #define REV_LEVEL 736 /* This version */ #define REV_MIN 591 /* Oldest compatible database */ #define EXPORT_REV_MIN 733 /* Oldest compatible export files */ -#define LIBCITADEL_MIN 114 /* Minimum required version of libcitadel */ +#define LIBCITADEL_MIN 115 /* Minimum required version of libcitadel */ #define SERVER_TYPE 0 /* zero for stock Citadel; other developers please obtain SERVER_TYPE codes for your implementations */ diff --git a/citadel/modules/expire/serv_expire.c b/citadel/modules/expire/serv_expire.c index ea852dd49..cd30915dc 100644 --- a/citadel/modules/expire/serv_expire.c +++ b/citadel/modules/expire/serv_expire.c @@ -22,6 +22,11 @@ * * When using Berkeley DB, there's another reason for the two-phase purge: we * don't want the entire thing being done as one huge transaction. + * + * You'll also notice that we build the in-memory list of records to be deleted + * sometimes with a linked list and sometimes with a hash table. There is no + * reason for this aside from the fact that the linked list ones were written + * before we had the hash table library available. */ @@ -790,6 +795,57 @@ int PurgeEuidIndexTable(void) { } + +/* + * Purge OpenID assocations for missing users (theoretically this will never delete anything) + */ +int PurgeStaleOpenIDassociations(void) { + struct cdbdata *cdboi; + struct ctdluser usbuf; + HashList *keys = NULL; + HashPos *HashPos; + char *deleteme = NULL; + long len; + void *Value; + char *Key; + int num_deleted = 0; + + keys = NewHash(1, NULL); + if (!keys) return(0); + + + cdb_rewind(CDB_OPENID); + while (cdboi = cdb_next_item(CDB_OPENID), cdboi != NULL) { + if (cdboi->len > sizeof(long)) { + long usernum; + usernum = ((long)*(cdboi->ptr)); + if (getuserbynumber(&usbuf, usernum) != 0) { + deleteme = strdup(cdboi->ptr + sizeof(long)), + Put(keys, deleteme, strlen(deleteme), deleteme, generic_free_handler); + } + } + cdb_free(cdboi); + } + + /* Go through the hash list, deleting keys we stored in it */ + + HashPos = GetNewHashPos(); + while (GetNextHashPos(keys, HashPos, &len, &Key, &Value)!=0) + { + CtdlLogPrintf(CTDL_DEBUG, "Deleting associated OpenID <%s>\n", Value); + cdb_delete(CDB_OPENID, Value, strlen(Value)); + /* note: don't free(Value) -- deleting the hash list will handle this for us */ + ++num_deleted; + } + DeleteHashPos(&HashPos); + DeleteHash(&keys); + return num_deleted; +} + + + + + void *purge_databases(void *args) { int retval; @@ -854,6 +910,12 @@ void *purge_databases(void *args) CtdlLogPrintf(CTDL_NOTICE, "Purged %d entries from the EUID index.\n", retval); } + if (!CtdlThreadCheckStop()) + { + retval = PurgeStaleOpenIDassociations(); + CtdlLogPrintf(CTDL_NOTICE, "Purged %d stale OpenID associations.\n", retval); + } + if (!CtdlThreadCheckStop()) { retval = TDAP_ProcessAdjRefCountQueue(); diff --git a/citadel/modules/openid/serv_openid_rp.c b/citadel/modules/openid/serv_openid_rp.c index 2f8de9968..4a87abc88 100644 --- a/citadel/modules/openid/serv_openid_rp.c +++ b/citadel/modules/openid/serv_openid_rp.c @@ -67,6 +67,7 @@ struct ctdl_openid { */ + /* * Attach an OpenID to a Citadel account */ @@ -120,18 +121,39 @@ int attach_openid(struct ctdluser *who, char *claimed_id) */ void openid_purge(struct ctdluser *usbuf) { struct cdbdata *cdboi; + HashList *keys = NULL; + HashPos *HashPos; + char *deleteme = NULL; + long len; + void *Value; + char *Key; + + keys = NewHash(1, NULL); + if (!keys) return; + cdb_rewind(CDB_OPENID); while (cdboi = cdb_next_item(CDB_OPENID), cdboi != NULL) { if (cdboi->len > sizeof(long)) { if (((long)*(cdboi->ptr)) == usbuf->usernum) { - CtdlLogPrintf(CTDL_DEBUG, "FIXME we have to delete an openid\n"); + deleteme = strdup(cdboi->ptr + sizeof(long)), + Put(keys, deleteme, strlen(deleteme), deleteme, generic_free_handler); } } cdb_free(cdboi); } - /* FIXME finish this */ + /* Go through the hash list, deleting keys we stored in it */ + + HashPos = GetNewHashPos(); + while (GetNextHashPos(keys, HashPos, &len, &Key, &Value)!=0) + { + CtdlLogPrintf(CTDL_DEBUG, "Deleting associated OpenID <%s>\n", Value); + cdb_delete(CDB_OPENID, Value, strlen(Value)); + /* note: don't free(Value) -- deleting the hash list will handle this for us */ + } + DeleteHashPos(&HashPos); + DeleteHash(&keys); } @@ -469,12 +491,6 @@ void cmd_oids(char *argbuf) { -/* - * Callback function to free a pointer (used below in the hash list) - */ -void free_oid_key(void *ptr) { - free(ptr); -} /* @@ -500,7 +516,7 @@ void cmd_oidf(char *argbuf) { extract_token(thiskey, buf, 0, '|', sizeof thiskey); extract_token(thisdata, buf, 1, '|', sizeof thisdata); CtdlLogPrintf(CTDL_DEBUG, "%s: [%d] %s\n", thiskey, strlen(thisdata), thisdata); - Put(keys, thiskey, strlen(thiskey), strdup(thisdata), free_oid_key); + Put(keys, thiskey, strlen(thiskey), strdup(thisdata), generic_free_handler); } @@ -657,6 +673,7 @@ void cmd_oidf(char *argbuf) { free(Value); } DeleteHashPos(&HashPos); + DeleteHash(&keys); } diff --git a/libcitadel/lib/hash.c b/libcitadel/lib/hash.c index 937aa32e0..63c80e357 100644 --- a/libcitadel/lib/hash.c +++ b/libcitadel/lib/hash.c @@ -686,3 +686,16 @@ void SortByPayload(HashList *Hash, CompareFunc SortBy) * return strcmp (a, b); * } */ + + +/* + * Generic function to free a pointer. This can be used as a callback with the + * hash table, even on systems where free() is defined as a macro or has had other + * horrible things done to it. + */ +void generic_free_handler(void *ptr) { + free(ptr); +} + + + diff --git a/libcitadel/lib/libcitadel.h b/libcitadel/lib/libcitadel.h index f42097abd..528df6cd3 100644 --- a/libcitadel/lib/libcitadel.h +++ b/libcitadel/lib/libcitadel.h @@ -14,7 +14,7 @@ */ #include #include -#define LIBCITADEL_VERSION_NUMBER 114 +#define LIBCITADEL_VERSION_NUMBER 115 /* * Here's a bunch of stupid magic to make the MIME parser portable. @@ -315,6 +315,9 @@ void SortByHashKeyStr(HashList *Hash); int GetCount(HashList *Hash); const void *GetSearchPayload(const void *HashVoid); void SortByPayload(HashList *Hash, CompareFunc SortBy); +void generic_free_handler(void *ptr); + + void convert_spaces_to_underscores(char *str); /* -- 2.30.2