There's currently two almost identical methods in the user resource - there's the real crud-function for creating a user, but that's only accessible to administrators, and there's an action, 'register', that does the exact same thing - only that anonymous users has access to it. These two should be consolidated into the crud-function.

I'm attaching a patch which does this and which preserves the register action for compatibility.

CommentFileSizeAuthor
services_dryer_user_resource.patch2.3 KBvoxpelli

Comments

marcingy’s picture

marcingy commented

Looks good from a quick eyeball, I'll test it locally later.

ygerasimov’s picture

ygerasimov commented
Status: Needs review » Needs work

@voxpelli nice idea to do these changes as they will make code cleaner.

What I found not nice is that now anonymous user can create user that is activated even if administrative approval is required. In order to avoid this we need to have separate callback for register action that will just set 'status' property to 0 or 1 depending on value of variable_get('user_register') variable.

I will provide patch tomorrow for this.

voxpelli’s picture

voxpelli commented

Actually: Isn't it kind of silly that we check if a user is anonymous? If an anonymous user is allowed to do something - then shouldn't any user be allowed to do it? Not allowing registered user to registered new users makes sense in a UI - but does it make sense in an API?

@ygeramisov: The patch changes nothing in any behavior - it only merges the access checks of the two functions. No one gets allowed to do more than they can already do with the current user resource.

ygerasimov’s picture

ygerasimov commented
Status: Needs work » Reviewed & tested by the community

@voxpelli you are right. Services shouldn't not completely repeat UI. I am completely fine with these changes.

Marking this issue as RTBC.

voxpelli’s picture

voxpelli commented

@ygeramisov: Should we remove the need to be anonymous? If so - this is "needs work" - the current patch doesn't include that.

ygerasimov’s picture

ygerasimov commented

I would commit this patch as it is. Anonymous user might not have permission 'administer users' but still should be able to register new user if it is allowed.

What I would change in the next patch is to allow anonymous users to create users only if it is allowed to register new users without administer approval. But that is minor issue I believe.

kylebrowning’s picture

kylebrowning commented
Status: Reviewed & tested by the community » Fixed
kylebrowning’s picture

kylebrowning commented
Version: 7.x-3.x-dev » 6.x-3.x-dev
Status: Fixed » Patch (to be ported)
voxpelli’s picture

voxpelli commented

Created a follow up issue: #1171036: Don't require anonymous user or admin rights to register users

kylebrowning’s picture

kylebrowning commented
Status: Patch (to be ported) » Fixed

Status: Fixed » Closed (fixed)

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