Forum OpenACS Development: Re: username is not unique on users table. Should we change it?

Hi Carl,

Thank you for the reply. I lost the last week digging into this problem, and I could see that your case is very difficult to solve. Looking at the specifications, they look very good. However, in my point of view (wich could possibly be wrong), they've introduced a logical error in datamodel. the users table should represent an user in OpenACS that has login access to OpenACS, and by user a mean one person. With this authority change, there can be more than one user_id referring to the same user.

I could not find any easy way to handle this problem, and I don't know if it's worthed. I guess the community majority doesn't actually see this as a real problem. However, I guess we should think about it, because there are a lot of login paths we can use in modern social networks. A small list could contain facebook, twitter, Open Social and many others, where different usernames represent the same user (I mena, one real person). If it's the best interest for everybody, I can volunteer to propose a solution that won't break everybody else's system. It's hard and it's also going to take a while, but I guess it will be worthed.

In the meantime, I added a switch to acs_user::get_by_username that allows you to search for a single username in any authority, so it will return the user_id for that username without taking authority in consideration. The following patch should do the job:


Index: packages/acs-tcl/tcl/community-core-procs.tcl
===================================================================
--- packages/acs-tcl/tcl/community-core-procs.tcl (revision 1329)
+++ packages/acs-tcl/tcl/community-core-procs.tcl (revision 1455)
@@ -394,9 +394,10 @@
{-authority_id ""}
{-username:required}
+ {-cascade:boolean}
} {
Returns user_id from authority and username. Returns the empty string if no user found.

@param authority_id The authority. Defaults to local authority.
-
+ @param username The username of the user you're trying to find.
@param username The username of the user you're trying to find.

@@ -407,9 +408,17 @@
set authority_id [auth::authority::local]
}
-
- set user_id [util_memoize [list acs_user::get_by_username_not_cached -authority_id $authority_id -username $username]]
- if {$user_id eq ""} {
- util_memoize_flush [list acs_user::get_by_username_not_cached -authority_id $authority_id -username $username]
- }
+
+ if {$cascade_p} {
+ set user_id [util_memoize [list acs_user::get_by_username_not_cached -authority_id $authority_id -username $username -cascade]]
+ if {$user_id eq ""} {
+ util_memoize_flush [list acs_user::get_by_username_not_cached -authority_id $authority_id -username $username -cascade]
+ }
+ } else {
+ set user_id [util_memoize [list acs_user::get_by_username_not_cached -authority_id $authority_id -username $username]]
+ if {$user_id eq ""} {
+ util_memoize_flush [list acs_user::get_by_username_not_cached -authority_id $authority_id -username $username]
+ }
+ }
+
return $user_id
}
@@ -418,14 +427,19 @@
{-authority_id:required}
{-username:required}
+ {-cascade:boolean}
} {
Returns user_id from authority and username. Returns the empty string if no user found.

@param authority_id The authority. Defaults to local authority.
-
@param username The username of the user you're trying to find.
+ @param cascade if cascade is set, look for username on any authority

@return user_id of the user, or the empty string if no user found.
} {
- return [db_string user_id_from_username {} -default {}]
+ if {$cascade_p} {
+ return [db_string user_id_from_username_cascade {} -default {}]
+ } else {
+ return [db_string user_id_from_username {} -default {}]
+ }
}

Index: packages/acs-tcl/tcl/community-core-procs.xql
===================================================================
--- packages/acs-tcl/tcl/community-core-procs.xql (revision 1329)
+++ packages/acs-tcl/tcl/community-core-procs.xql (revision 1455)
@@ -149,4 +149,15 @@
< /fullquery>

+< fullquery name="acs_user::get_by_username_not_cached.user_id_from_username_cascade">
+ < querytext>
+
+ select user_id
+ from users
+ where lower(username) = lower(:username)
+ limit 1
+
+ </ querytext>
+< /fullquery>
+
< fullquery name="acs_user::registered_user_p.registered_user_p">
< querytext>

Please, let me know your thoughts about it.

If you sign people up more than once, with multiple authorities, they will have multiple local accounts, but the autority data model and authentication code have no knowledge and could have no knowledge of how these users are mapped except by the username,authority_id connection.

I think what you are proposed is seperate from what the current authentication package was designed to do. It is not a flaw or a bug, it just was never designed to do that. Anything that changes that needs to make sure it doesn't change any other behavior or break any existing use cases.