Re: Re: Plone 3 Support
from
Rob Miller
on Aug 16, 2007 09:11 PM
Ross Patterson wrote:
> Ross Patterson <me@...> writes:
>
>> Rob Miller <robm@...> writes:
>>
>>> Rob Campbell wrote:
>>>> Rob Miller wrote:
>>>>> ah, well, you must realize that the user will NOT have the Owner
>>>>> role at this point. she's not logged in, she's trying to set her
>>>>> initial password. any time the user goes through the
>>>>> PasswordResetTool process, she will be anonymous. i'm pretty sure
>>>>> this works already in Plone 2.5, although i don't remember exactly
>>>>> how it was handled. and maybe it's broken and i just don't know
>>>>> it... :-P. unfortunately, i don't have time to dig into it any
>>>>> further at the moment. if i can, i'll take a look over the next
>>>>> few days to see what i can discover. -r
>>>> Ok, that would explain the error. Would adding the Set own
>>>> password permission for Anonymous users fix it?
>>> no. at best it would do nothing (who is the "own" in "Set own
>>> password" for anonymous users, anyway), and at worst it would allow
>>> any anonymous user to change any other member's password.
>>>
>>>> Would that cause security problems allowing any Anonymous user to
>>>> change any password?
>>> it might. :-).
>>> anyway, i looked at the code a bit more and realized that it's not
>>> actually the password setting that's causing the problem. the
>>> password is set in the uf.userSetPassword() call that happens on
>>> line 142; your failure is happening on line 151, on the following
>>> call:
>>> member.setMemberProperties(dict(must_change_password=0))
>>> this is trying to set the "must_change_password" property, to mark
>>> that the password change has happened already.
>>> that field was just recently added to the remember schema 5 days
>>> ago:
>>> http://dev.plone.org/collective/changeset/47063> before this was added, there was no problem w/ PasswordResetTool,
>>> b/c when it tried to set the value, it realized there was no AT
>>> field that matched that name and it just returned. now that
>>> remember is supporting that field name, however, we've got a
>>> problem, b/c setProperties is doing an explicit security check and
>>> (of course) anonymous users are not showing up as having rights to
>>> change this value.
>>> AFAICT, the only thing to do is to make a change to
>>> PasswordResetTool to work around remember's security checks. this
>>> could possibly be done in PwRT itself, assuming the maintainer is
>>> okay w/ a remember-specific workaround in there, or we could
>>> monkeypatch it from remember (less that ideal). in any event, i've
>>> included a diff (untested) to PasswordResetTool.py at the end of
>>> this message that should do the trick.
>>> this is a bit of a recurring problem. setProperties on the member
>>> object always explicitly checks permissions so that users can't use
>>> it to circumvent the AT security settings on member schema fields.
>>> this often interferes with trusted code that wants to use this call,
>>> however. AFAICT, there's no good way for setProperties to check and
>>> see whether or not the calling context is trustable, in the Zope
>>> sense. there has to be a better way than putting security checks in
>>> all of the calling code, but i haven't found it yet.
>>>
>> If the code that calls
>> Products.remember.content.member.BaseMember.setProperties doesn't
>> expect security checks, we could do a SecurityManager workaround so
>> that the field mutators can be called as an unrestricted user.
>> Untested diff attached for demonstation purposes.
>
> That last patch didn't work at all. This one is still untested but I
> fixed one thing.
this patch isn't what we want. it's attempting to negate the explicit
security check that comes before. and it's unnecessary, anyway; the AT
mutators can be called just fine from trusted code.
i've got a fix, actually, b/c i forgot about some code i already wrote to do
the frame stack test in the getProperty method. i need to make the tests pass
before i commit, though, hope to get to it later.
-r