Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
To manage shared data across sites, UUID can save a lot of headache. After dealing with Bakery for a while, I feel some parts of Bakery would be much more straightforward if we had UUID.
Would a patch that introduced UUID to Bakery be acceptable? Could UUID be a dependency or just optional?
Comment | File | Size | Author |
---|---|---|---|
#16 | bakery-uuid-1341058-16.patch | 6.74 KB | bcmiller0 |
#15 | bakery-uuid-1341058-15.patch | 6.94 KB | christianchristensen |
#15 | bakery-uuid-1341058-15-D6.patch | 7.6 KB | christianchristensen |
#14 | bakery-uuid-1341058-14.patch | 6.93 KB | glennpratt |
#14 | bakery-uuid-1341058-14-D6.patch | 7.58 KB | glennpratt |
Comments
Comment #1
coltraneI would prefer to separate Bakery SSO from account data synchronization in future versions, to simplify the module.
Comment #2
glennpratt CreditAttribution: glennpratt commentedSo, I tend to agree, but the reason I'm proposing UUID is to negate the need for storage in Bakery (or data sync). In my mind, the only thing I would require from Bakery is UUID (I don't even care about syncing the user name).
Comment #3
gregglesYou want a UUID to identify the user or for what purpose?
I'm having a hard time following what you mean so maybe making the dependency on UUID and adding a basic example of how it could help as a patch would be worthwhile.
Comment #4
glennpratt CreditAttribution: glennpratt commentedYes, I want UUID to uniquely identify the user, UID will differ (and requires separate storage per slave, username and email can be changed, so that basically requires bakery to sync fields - though that is nice for d.o, it's something I already take care of elsewhere in my application).
I'll work toward a patch when I can, thanks.
Comment #5
David_Rothstein CreditAttribution: David_Rothstein commentedNot sure if this is exactly what was intended by the original issue, but it's certainly a good way to start... As far as I can tell, the attached patch is actually all that's needed to allow Bakery to share UUIDs between sites :)
The use case for this is that if you're using Deploy + UUID to push accounts between sites but also have Bakery enabled, you don't want to wind up in a situation where Bakery creates the account first but winds up giving it a different UUID. Because at that point, the UUID module won't know they're related so you won't be able to sync up any of the additional account data with Deploy.
One could make a good argument that if the UUID module is enabled this should actually be the default behavior... but that's a more complicated patch so for now I'm just making it an option.
Comment #6
glennpratt CreditAttribution: glennpratt commentedYes, that is what was intended. I'm not using the deploy module, but Services to push accounts around with other related data (often before the user ever shows up for SSO), so I have to funnel it through Bakery first and hand hold it a bit. I'd prefer it just see a user with that UUID already exists and trust me to handle the data.
Comment #7
gregglesThere's a general goal for bakery to be just about SSO and not about data sharing. If there's a way that UUID and Services or Deploy can handle that, great!
Steps to test this:
1. install a master and slave with bakery and uuid modules
2. register a user on master and then have them visit slave, confirm uuid synch
3. register a user on slave, confirm uuid synch
4. admin creates user on master, pulls to slave, confirm uuid synch
Anything else?
Comment #8
glennpratt CreditAttribution: glennpratt commentedI'd add this to be thorough:
5. Create a user with the same UUID on master and slave (without using Bakery). Confirm SSO works.
Comment #9
David_Rothstein CreditAttribution: David_Rothstein commentedThose lists look correct to me. While working on this so far I believe I already tested everything but #4. I just tried #4 quickly now (since I was easily set up to do so) and it seemed to work fine as well.
Comment #10
David_Rothstein CreditAttribution: David_Rothstein commentedTesting this with different types of user registration configurations (i.e., email confirmation required, admin approval required, etc) would also be a good idea.
This patch might not work correctly in some of those scenarios. I think I just inadvertently ran across that when testing something else, but didn't have a chance to do a full test.
Comment #11
glennpratt CreditAttribution: glennpratt commentedThis is a work in progress, but I was going toward making UUID the primary field to key off - and using it as early as possible.
Comment #12
glennpratt CreditAttribution: glennpratt commentedWe've been running the patch in #11 since August.
This changes slave update to prefer UUID if available, instead of init which is dependent on static master domain name (breaks on dev pullbacks or if user is created by other means).
The namespace patch will apply with #1369660: "Namespace" cookie names to support subdomains., please ignore.
Comment #13
glennpratt CreditAttribution: glennpratt commentedD6 roll of #11
Comment #14
glennpratt CreditAttribution: glennpratt commentedFound an issue where UUID wasn't part of the cookie sometimes. Need to add UUID to global $user in a lower weight module.
Comment #15
christianchristensen CreditAttribution: christianchristensen commentedLooks like there was a missing
(object) $cookie
for_bakery_bake_chocolatechip_cookie
on the previous patch that's been referenced that got removed. This should clean that up.Comment #16
bcmiller0 CreditAttribution: bcmiller0 commentedre-rolled d7 patch for 2.0-alpha4 version.
Comment #17
coltraneFor 2.x I think this could be handled by #556666: Sync hooks: Enable sharing of arbitrary data.
For Bakery 3.x I think we could support this issue so moving this to 3.x.
Comment #18
ckngAlternative implementation (not as primary field) is to use patch from #1966826: Allow user fields to be saved during registration from slave and configure $conf['bakery_supported_fields'].