I've heard D7 now includes some client AX support. Honestly I did not look into what that exactly contains, but I have an updated version of the openid_client_ax module which aligns the login/user creation workflow to the new core openid.module code. This also includes the latest fix from #542180: D7 version of OpenID does not deal with blocked users correctly, which fixes a user registration issue (see from comment #8: http://drupal.org/node/542180#comment-2765112).

CommentFileSizeAuthor
#4 ax-d7.patch6.68 KBgábor hojtsy
port-openid-ax-d7.patch8.58 KBgábor hojtsy

Comments

darren.ferguson’s picture

darren.ferguson commented

Gabor

I am not sure if you maintain the current openid module for drupal core, however if it were possible to add a hook_openid too that module a lot of the duplicated code in the openid_client_ax module would disappear since most of the module was re-written from the openid.module because it did not provide a way of retrieving the attributes other than the simple registration attributes it asked for name and mail i believe.

The hook for the core module would go a long way to resolving a lot of things that were re-written in this module (duplicated) because the response could not be retrieved.

If that is not possible i am happy to put this on the D7 branch and if you wish CVS access to maintain it will add you too it.

Please advise regarding this.
Respectfully,
Darren

gábor hojtsy’s picture

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

Well, the D7 module does have simple AX support, and what it does via the latest RTBC patch at #395340: Email verification not enforced with OpenID auto-registration is that it reuses the user registration form (and adds items to indicate it was an OpenID registration), so you could do the same stuff for AX data and get it saved for the user via normal user save hooks I believe. Not sure what the D7 one does for login ATM (code is changing around there :), but that could just as well do the same. So I'm not sure we'd need special hooks in D7 anymore, but keeping an eye there is definitely worthwhile.

darren.ferguson’s picture

darren.ferguson commented

I will have to download Drupal 7 to see what the module is doing since currently i have not looked at it at all version 7 wise.

If the user hooks will provide the data then that will solve the problem for retrieving the response from the openid provider.

If i get time to dig through will check how 7 has been implemented to verify if it will support the passing of the response to other modules that would require it.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
StatusFileSize
new ax-d7.patch6.68 KB

Actually, Drupal 7 core already provides hook_openid_response() for other modules to act on the response. So the huge code duplication and menu overrides can be simplified to these few lines of code:

/**
 * Implement hook_openid_response().
 */
function openid_client_ax_openid_response($response, $account) {
  // Fire hook_openid_client_ax implementations if AX is enabled.
  if (variable_get('openid_client_ax_enabled', FALSE)) {
    module_invoke_all('openid_client_ax', 'save', $response, $account);
  }
}

The patch became much leaner and needs less maintenance when core OpenID changes (as it does these days).

darren.ferguson’s picture

darren.ferguson commented

Gabor

Patch looks a lot better and will be a lot easier to maintain going forward instead of the menu alter and code duplication from the core openid module.

Do we want to create a D7 branch for this and add too there??

If you want i can open up the CVS access for you to commit on the D7 branch since i have not had any dealing currently with it.

damien tournoud’s picture

damien tournoud commented

Hi Darren, since I'm now officially maintaining the core OpenID module, would you add me as a maintainer of this one?

darren.ferguson’s picture

darren.ferguson commented

Damien, this is now done, you should have CVS access for the module.

gábor hojtsy’s picture

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

I'm totally fine with Damien reviewing and maybe even making it better :)

gábor hojtsy’s picture

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

D7 core HEAD now has openid_extract_ax_values() which is a more useful version of _openid_client_ax_get_multiple_values(), so we could also get rid of that. This reduces this module to a list of predefined AX properties and some settings as we stand.

gábor hojtsy’s picture

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

Drupal Gardens is not shipping with this module used as of public beta (last week), since Drupal code now does this stuff better for us. No need for an intermediary. I'm not entirely sure about the role of this module anymore in D7.

darren.ferguson’s picture

darren.ferguson commented
Status: Needs review » Closed (won't fix)