Closed (fixed)
Project:
Bakery Single Sign-On System
Version:
6.x-2.x-dev
Component:
Code
Priority:
Major
Category:
Feature request
Assigned:
Issue tags:
Reporter:
Created:
10 Sep 2009 at 21:52 UTC
Updated:
10 Jan 2014 at 18:48 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
stacie_a commentedCould this be a helpful place to check if updated user info has been updated across subsites? And if not, give them an option/button to update them?
Comment #2
gerhard killesreiter commentedthis should not be too hard to do, we currently store only rhe url of the slave sites for the master, though
Comment #3
juliangb commentedIf I've understood this issue correctly, the proposal is to create a list of links on the master user profile, linking to all of the user's slave user profiles (where they exist).
Personally, I do not see the value in this, because I see the benefits in linking back from the slaves to the master (as already happens). I think linking from the master to slaves would add confusion / unnecessary info (with associated cruft to maintain the list).
It could be useful as a repair tool to have a "Click to make sure user account data is up to date on slave sites" - this could simply trigger the updates as if the master user profile had just been edited.
Comment #4
drummRPC is used to block on all sites, the same sort of thing can be used here. RPC is slow, so only do once and store the result in the local DB.
"Click to make sure user account data is up to date on slave sites" sounds like unnecessary UI. If syncing needs to happen, do it without asking, in another issue.
For Drupal.org, we don't want profiles on subsites, except for administrative and user tasks. So, we want a link to edit your g.d.o account for subscriptions and such, and one for tecker there. API has neither, so needs to know not to make those links.
Comment #5
scott859 commentedsubscribing
Comment #6
coltranedrumm and I talked about this a bit at CPH and because the master site has no idea of the user IDs of sub-site accounts there's no way yet to link directly to their user edit page. Could we have a sub-site callback that redirected to that user's edit pages? Something like bakery/edit or even in the custom subsite module?
Comment #7
juliangb commentedA callback that redirects seems like a good simple way to do it.
But, we don't know on the master which slave sites the user has access or, or which slave sites have additional config. The first perhaps is trivial because bakery should create an account as soon as the site is visited. The second - either display all slave sites, or configuration as a setting?
Comment #8
greggles@drumm's final paragraph in #4: If we don't have profiles on the subsites how do we show what people are doing on those subsites?
The user/UID page is a reputation/involvement indicator of what each person is interested in. We need to get data from sub-sites up to the master to show on their profile.
That seems like a lot of work and I'm not sure the benefit is clear...
Comment #9
drummThe pages will still exist, but http://drupal.org/user/1 and http://groups.drupal.org/user/90 is just redundant. We want to bring all the general info up to the main site, and link out to specific tracker pages and edit pages.
Comment #10
drummComment #11
drummComment #12
drummBumping back up now that the redesign has launched.
Another important use case:
We want to know if Drupal.org users are Drupal Association members. For this, we query CiviCRM directly, but need to know the Association.drupal.org UID.
Comment #13
drummImplementation notes:
Create a table, which will only be populated on master, with
- (master) uid
- slave_uid
- slave_domain
When pushing changes from the master to slaves, via .../bakery/update, the slave's response is a good place to stash the UID. Since it is a small amount of data, and the existing response is only a message, a HTTP header might be the best place to stash it.
Comment #14
drummHere is a patch for review.
Comment #15
drummIt would be nice to populate this when an account is created on a slave site. As far as I can tell, user_save() happens after all communication to the master site. So, that would be a new request if needed.
I can't spot any other requests to put this UID in, but let me know if I am missing any.
Comment #17
coltraneIf we moved the table creation out of hook_install we wouldn't get it on slave sites. When the master is enabled for the first time (once admin/settings/bakery is submitted and the bakery_is_master variable is set) we could create the table. What do you think?
It's difficult to get the subsite UID from all the communication methods. During user registration we can get it in the thinmint payload to the master in _bakery_reset_submit(). I can't see how we'd do it in user login without trying to load the account first on the subsite, or making an additional server-side request to the master after returning.
Comment #18
gregglesI think it's fine if the list is refreshed on every edit on the master site and every registration on a subsite.
Seems good to me to limit the creation of that database table to the master, though not necessary.
Comment #19
coltraneDo we want the master to link to all subsites?
Comment #20
drummThe patch so far won't solve the UI problem, I intentionally left that out. Bakery should do something generally smart by default that works with hook_user() and/or other APIs for site-specific overriding.
Comment #21
drummNew patch that populates UID on thinmint exchange.
For table creation, best to keep it simple and use .install files as intended. Empty tables on slaves are okay.
For UI, feel free to leave this issue open or we can start a new one.
Comment #22
coltraneInstead of this can we get the schema from bakery_schema() and save some LOCs?
We should check that $slave is in the master's list of slave sites.
Powered by Dreditor.
Comment #23
gregglesI think getting the table definition from the schema is problematic if you change the schema in later updates. It duplicates code, but saves us from headaches should we do an update later.
Second point makes sense to me.
Comment #24
drummNew patch centralizes the call to drupal_write_record() and adds a check for a valid $slave. As greggles said, updates need to have the schema at the time the update was added, and should not include future changes to bakery_schema().
Comment #25
coltrane#24 is good but for the slave site match during user registration. Bakery needs master and slave URLs with an ending slash, so when the slave sets it's URL using base_url we need to append a slash.
greggles, if you want to give this a check I otherwise think it's RTBC
Comment #26
gregglesCode looks sane and in my testing it worked well.
Now committed - thanks guys http://drupal.org/cvs?commit=449928
Comment #27
gregglesThere's an errant watchdog in that last patch. Fixed in 6.x-2.x, but needs to be fixed when porting.
Comment #28
drummOf the 40 accounts created on association.drupal.org in the last 24 hours, drupal.org had a record of 5 of them. The watchdog logs are quickly discarded, there is not a whole lot to go on. I believe these are existing accounts on Drupal.org, visiting association.drupal.org creates a new account matching the master via bakery_request_account(). This is true for the most recent two, which we do have logs for.
Possible solution A - reserve the slave UID in advance and send it with the request to master/bakery/create. No additional HTTP requests, but may require trickery to create a placeholder account with a UID.
B - Add another request to master containing the slave UID after user_save() is successfully called.
I think I favor A, user_save() looks simple enough to not cause problems, but assuming that is risky.
Comment #29
gregglesA does seem riskier. I'd prefer B though I agree it does add complexity.
One of our cookies is for sending data from master to slave on first visit to the slave. Perhaps we can use that same cookie in reverse as well?
Comment #30
drummI went with A, since a new cookie exchange is a lot of work. It seems to work well. Results may vary with contrib using hook_user(), but I am happy with it.
The patch looks bulky because I flipped some failure conditions for less indentation.
Comment #31
drummKeeping up with CVS.
Comment #32
gregglesIt's a bit hard to review with the flipping of that failure condition, but this looks good and seems to do the trick. I tested it by logging in on a slave with an account that didn't exist on the slave. The master then had a record for that user in the bakery_user table with the right uid and slave_uid values.
Now committed - http://drupal.org/cvs?commit=460250
Comment #33
verynic commentedsubscribing
Comment #34
r13ose commentedAny updates to this ticket? I would like to know if the failure condition has disappeared so that a true test can be review.
Comment #35
gregglesI'd like to keep this focused on the task at hand.
Comment #36
gregglesI created a new issue for displaying the data: #1058794: Link to subsite profiles on master site's profile page.
Comment #37
gregglesJust a quick followup to try to catch attention of folks interested in this issue that this seems to not work for someone who upgraded to 6.x-2.x
#1161646: Persistent problem upgrading 6.x-1.x sites to 6.x-2.x
Comment #38
drummThis is working on Drupal 7.