logging in from user/login shows "Access denied" after login

I've been able to reproduce this issue on both the Drupalcon DC and Drupalcon Szeged sites, as well as any other Drupal site that I've attempted to login to using OpenID

This always happened to me on the DC Szeged site too, because their login link goes to user/login (rather than user).

OpenID modules's form_alter sends you back to drupal_get_destination. That's not safe to do upon changing someone's role. Seems better to just redirect to /user.

The problem with the patch from #4 is that it always redirects you to http://example.com/user, which is not always what we want. A specific case is when the login URL has a destination specified, like http://example.com/user/login?destination=node/123 (or http://example.com/user/login?destination=user, which I use as a temporary solution), but that destination would be ignored.

However, I do agree that this absolutely needs to be fixed. We can't ship Drupal with an OpenID client that is broken by default.

Also marked #268485: Returns to login page as a duplicate.

The attached patch fixes the issue in openid.module.

By the way, my initial thought was that it would be nice to redirect the user/login page to the user page for authenticated users, but I'm not sure how to do this when we have to deny access so users don't see a "Log in" menu item. We would need some way to deny access and redirect to another page.

oops. Removed an extra blank line.

This looks good.

This looks good to me. Wouldn't it be better to make this a generic function in user.module so that it can be reused by other authentication modules? Probably not very important, but could be helpful.

Changed openid_get_destination() to user_login_destination()

Confirming that patch in #11 works. Thanks mfb

I tested the following.
-Login with OpenID on a fresh Drupal site without content.
-Login with OpenID on a fresh Drupal site with content. Destination node/1
-Simulating user error by trying to log-in with wrong OpenID URL.

Tested with.
-Drupal 7 (September 9, 2009 - 05:11) fresh install.
-Garland theme.
-Windows XP
-Internet Explorer 7

Committed to HEAD. Thanks!

No tests.

mfb was so kind to not-really-point-me-here, but mention this addition in #578520: Make $query in url() only accept an array. ;)

Tagging.

added user/login test to the existing openid login test.

@all: What are 'tests'? I read that on a few issue queues. Could you give me link(s) where I could read more about what tests are? Does 'tests' means tests for the 'SimpleTest' module or Drupal automated patch testing output as 'Testbed results'? Or something else? Thanks.

@Onopoc: by "tests" we mean automated tests written for the Drupal SimpleTest module (in core since Drupal 7). You can find more info here: http://drupal.org/simpletest

The test addition looks good to me.

Thanks flobruit

Thanks for the tests! Committed to HEAD.

The same issue exists in Drupal 6.15. I have reworked the patch for Drupal 6.15 and am attaching it here.

6.16 is out and this wasn't fixed :(

Who tested this besides frowning that it is not in release yet?

subscribed.

i also get redirected to domain.com/user where again the login form is displayed. but without access denied. i'm just not logged in.

i dont understand why this is happening. when i login without openid i am also redirected to /user but logged in!

before i try the patch: do you think it would help in my case? running 6.16...

it could also be related to a session variable problem: http://drupal.org/node/223194

i also get redirected to domain.com/user where again the login form is displayed.

That happens to me when the server handling the OpenID account returns something that Drupal is not able to understand, or that understands to be an error.

@kiamlaluno, the strange thing is that on other drupal installations it works fine. and the openid provider i am using is also a drupal with openid provider module...

I tested this change and it does address the issue. The code looks good except I don't think the function that figures out the login destination should go into the user module. It seems like this is the only known usage, and likely the only code that should use it. That being the case, I did a minor reroll of the patch to move this functionality into the openid module and refactored the comment a bit.

Please disregard this patch

The patch in http://drupal.org/node/365597#comment-2542070 was the basis of the patch I just posted and I new see that I have undone the work suggested in comment #10. Sorry about that. I did test the previous patch and the code is fine as is, and it solves my customer's issue.

Can we get your docs additions from this last patch but keep the user_*() function space addition as in the previous patch?

Absolutely. Here is the new patch. #22 with the comment from #30. i verified the patch locally.

Looks great.

Looks great, especially glad for the testing feedback; committed.