Just some more cleanup. There is no reason all user_save() calls have to endure some distributed auth code. I checked OpenID and it is unaffected by this patch.

CommentFileSizeAuthor
#11 mw.patch3.65 KBmoshe weitzman
mw_88.patch1.76 KBmoshe weitzman

Comments

moshe weitzman’s picture

moshe weitzman commented

in order to test this, just do a registration and a login via distributed auth (i.e. enable drupal.module and login with @drupal.org) and make sure all is well.

pwolanin’s picture

pwolanin commented

subscribing

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

So where does this auth information saved instead?

moshe weitzman’s picture

moshe weitzman commented

drupa.module calls user_set_authmaps() after this patch. openid calls into the authmap table directly.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Again, this patch does not touch drupal.module, so drupal.module might register with user_external_login_register() then?!

moshe weitzman’s picture

moshe weitzman commented

@Gabor - you are right. The relevant change is in user_external_login_register().

My statement in the other issue is incorrect. drupal.module is unchanged by this patch and can be booted at any time.

catch’s picture

catch
he/him
Primary language English
commented
Category: feature » task

This still applies cleanly. drupal.module has gone but I assume this is still a valid issue?

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Yes, in principle this looks good, but another set of eyes would never hurt.

moshe weitzman’s picture

moshe weitzman commented
Version: 6.x-dev » 7.x-dev

sigh.

moshe weitzman’s picture

moshe weitzman commented
Assigned: Unassigned » moshe weitzman
Category: task » feature

refreshed. i did need to change two lines in openid module so that it creates its own authmap entry instead of relying on user_save().

moshe weitzman’s picture

moshe weitzman commented
StatusFileSize
new mw.patch3.65 KB

now with patch

dries’s picture

dries commented

I think this patch might need a re-roll because of the schema API patch that got committed?

moshe weitzman’s picture

moshe weitzman commented

I rerolled after that one landed. Should be good to go.

dries’s picture

dries commented
Status: Needs review » Fixed

Looks good to me. Makes for cleaner code. Committed this patch to CVS HEAD.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.