• Remember Mailing List

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
Return to date view: threaded or flat