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.
Patch for supporting the new path in OpenID for one that does not have the access denied message coming back if the user is not logged into the site initially.
Comment | File | Size | Author |
---|---|---|---|
#25 | 322764_openid_provider_path+func2.patch | 9.07 KB | anarcat |
#22 | 322764_openid_provider_path+func1.patch | 9.32 KB | Aron Novak |
#20 | 322764_openid_provider_path+func.patch | 8.93 KB | alex_b |
#17 | 322764_openid_provider_path+func.patch | 8.95 KB | anarcat |
#16 | openid_url.diff | 6.22 KB | anarcat |
Comments
Comment #1
Chris Johnson CreditAttribution: Chris Johnson commentedThis patch, or the patch in http://drupal.org/node/322767 is required for the OpenID provider to even really function.
Comment #2
christefano CreditAttribution: christefano commentedI think this is a duplicate of #322767 since this patch is required by (and already included in) the patch in that issue. This can be reopened if necessary.
Comment #3
sumitk CreditAttribution: sumitk commentedno its not duplicate of #322767 you need to apply this one too
Comment #4
Aron NovakI tested the stuff and it seems #322767 is not really neccessary.
As i see, to make the openid_provider work you need to apply this patch (322764) and #322769.
However while I tried to login to another Drupal site, i received a message that there are invaild characters in the username, i had to choose another one. I assume this is not a critical issue.
Comment #5
Aron NovakBetter testing, this patch is not critical, openid_provider works fine without it. However it's important to tune access privileges correctly. See openid_provider_requirements().
Comment #6
lourenzo CreditAttribution: lourenzo commentedI made a test with this patch, and works for me. I've got the invalid chars when testing with another drupal, too.
Comment #7
wundo CreditAttribution: wundo commentedThe invalid chars is another issue (not related with this patch), the guys are working on that other issue, right now.
Comment #8
walkah CreditAttribution: walkah commentedI've changed this patch slightly to change the format of the URL to be user/%/openidurl - this removes the requirement to have a patch for XRDS-simple module.
The patch also includes removing the hook_requirements implementation in .install
Comment #9
sumitk CreditAttribution: sumitk commentedAbove patch applied successfully. I added admin settings for sreg request so I am combining both patches and pasting a newer one to test.
Explaining a bit more earlier using openid_provider does not include your username and email in openid call (which was already present in core openid module) so we added those 2 parameters (niackname and email from $user object) with some admin settings.
Comment #10
alex_b CreditAttribution: alex_b commentedJust reviewed and tested. Looks good.
Not fond of 'openidurl' -
user/%/openid/identity user/%/openid/openid-identity ?
Comment #11
anarcat CreditAttribution: anarcat commentedThe patch works and i think can go in.
I am not fond of "openidurl" either at all though, so I'm marking "needs work".
In building our provider here, we are wanting something really user-friendly, something like http://id.example.com/username
Should I build a patch with that instead?
Comment #12
alex_b CreditAttribution: alex_b commented* Changed the default openid provider path to user/%/identity (as discussed at code sprint between walkah, sumitka and alex_b)
* Added pathauto support so that provider path can be aliased automatically
Comment #13
anarcat CreditAttribution: anarcat commentedSo here's a patch that further improves on the path. I enclosed the path generation in a function so we can play around (and maybe add hooks?) more easily with the URL. I chose
user/<uid>/id
to keep it simple.I thought of making the pattern a variable (variable_get('openid_provider_url') + associated changes to the settings) but that doesn't work well since the form settings submission would need to be changed to regen the menus when that value changes, which maybe possible.
I feel, however, that url aliases or thing like that would make this cleaner.
Comment #14
anarcat CreditAttribution: anarcat commentedI actually question the validity of this approach altogether: shouldn't we just allow anonymous users to access user profiles? Isn't this what OpenID is about in the first place?
Comment #15
Chris Johnson CreditAttribution: Chris Johnson commentedI'm imagining a case where a site has extensive user profiles which contain a lot more information than they provide in their OpenIDs. Hence, the data in the OpenID (name, email, ID, maybe a few others) is public, but the rest of the profile (expertise, cell phone number, address, etc.) is private. Each field in the profile has its own privacy settings. I want to avoid having OpenID bypass those privacy settings.
Comment #16
anarcat CreditAttribution: anarcat commentedVery simple update: change the canonical url to
user/%s/openid
.@Chris Johnson : agreed. This is different from
user/%s
. I don't see the controls you're mentionning in place, however... (?)Note that I resolved my "cleaner URL" issue by adding Pathauto support into the module, see #394690: pathauto support, which depends on this patch here.
Comment #17
anarcat CreditAttribution: anarcat commentedThe above patch forgot one place for the URL change, in the
.inc
file. I have fixed that and factored out the URL generation in a function so it's easier to change it module-wide.The patch otherwise works very well and I'm able to use http://id.koumbit.net/anarcat to connect to remote sites.
A bit annoying to see we had some major work duplication here, I am closing #394690: pathauto support.
Comment #18
alex_b CreditAttribution: alex_b commenteduser/%/openid is used by openid module
Comment #19
anarcat CreditAttribution: anarcat commentedThe patch uses the recommended
user/%s/identity
.Comment #20
alex_b CreditAttribution: alex_b commented#19: thanks for clarification.
* Rerolled after #394996: Initial cleanup of module code went in.
* Adjusted output on hook_user('view') to be more user module like http://skitch.com/alexbarth/b8ppm/admin-provider
This is RTBC from my point of view.
Comment #21
alex_b CreditAttribution: alex_b commenteddid not mean to reset status.
Comment #22
Aron NovakHere is a version what does not interfere with sites without pathauto.
Now it's RTBC.
Comment #23
alex_b CreditAttribution: alex_b commented#15/#16:
I just realize that in certain use cases you want to have the user profile page as the provider path. Think about rich user profiles that are meant to be passed around - much as I can bling out my OpenID page here http://lxbarth.myopenid.com/.
Unfortunately, you can't use pathauto to alias the identity path so that it points to the user profile instead of the openid page. If you try to enter the same pattern as the user profile pattern, you will get still a different path (suffixed by -0, -1, etc.).
When you think about it, you should only use the user/%id/identity page if you /can't/ access the user/%id page.
Solution:
* The path user/%id/identity should be provided in any case.
* user/%id should be the default path offered as openid identity path on user/%id
* user/%id/identity should be the fallback if anonymous users don't have 'access user profiles' permissions
* Add appropriate help text
Comment #24
Chris Johnson CreditAttribution: Chris Johnson commented@anarcat: sorry for the delay in following up on your question. I got stranded in the Chicago airport for about 18 hours.
The per-field permission controls are provided by contributed modules such as in the user_relationships module.
Comment #25
anarcat CreditAttribution: anarcat commentedI agree with #23, but still thinks the code is RTBC. This is blocking and pretty annoying, and I'm not sure there's a good way to workaround the issue mentionned.
I've rerolled the patch since it didn't apply cleanly here for some reason.
Comment #26
Aron Novak#23:
* user/%id/identity should be the fallback if anonymous users don't have 'access user profiles' permissions
I see one serious problem here.
Maybe openid experts will correct me, but if the provider path changes, clients will believe that this is a different user (result: user already exists for example at Drupal login). So if the permissions are changed, it causes headache to the users.
Comment #27
walkah CreditAttribution: walkah commented@Aron - yes, I shared this with anarcat last night - changing identity URLs will break things for people who have used their identity will no longer be able to login if their identity changes.
I think we should hold off on this for now.
the patch in #25 currently doesn't cleanly apply, but i'm going to clean it up and commit.
Comment #28
walkah CreditAttribution: walkah commentedcommitted. thanks everyone!