Update memory and cached creds when changing password from gdm or xdm

[boyang]

Jeremy Allison wrote:
> On Fri, Jul 04, 2008 at 11:00:45AM +0800, boyang wrote:
>
>   
>> (!(NT_STATUS_IS_OK(cache_ret)
>>     || NT_STATUS_EQUAL(cache_ret, NT_STATUS_OBJECT_NAME_NOT_FOUND)))
>> is any error except NT_STATUS_OBJECT_NAME_NOT_FOUND. The pairs of () seems
>> to lead misunderstanding.
>>     
>
> Ah, yes - now I see - thanks. I missed the placing of the
> ()'s. I prefer my way of doing it though :-). Less likely
> to lead to misunderstandings :-).
>
>   
>> Reauthentication happens after changing password. Therefore,
>> when we change password, there is no memory creds. *BUT*
>> after changing password, we have to update memory creds.
>> In winbindd_dual_pam_chauthtok(), update memory creds
>> will definitely fail if password is changed from gdm or xdm
>> because there is no memory creds at that time.
>>
>> So, I think you've looked the way around. It worked as:
>> changing password ---> updated memory creds fail ---->
>> reauthentication to update memory creds successfully.
>> *BUT* you look it as:
>> authentication(Oh, yes. memory creds exists) ---->
>> changing password ----> update memory creds(memory
>> creds is there, so update succeed.)
>> :-).
>> Did I understand you correctly?
>>     
>
> No, I thought the logic looked like :
>
> change password, followed by authentication,
> followed by update memory creds.
>
> but your explaination makes sense instead. Thanks.
>
>   
>> @About patches:
>>       I think there is only one careless mistake. cache_ret is not
>> replaced with result. :-)
>> here:
>>
>> +			if (NT_STATUS_EQUAL(cache_ret, NT_STATUS_NO_SUCH_USER)) {
>> +				result = NT_STATUS_OK;
>> +			}
>>     
>
> Yes - well caught. That should be "result" instead of
> cache_ret.
>
>   
>> @Plus:
>>       There seems to be some duplicate in pam_sm_chauthtok() and
>> winbindd_dual_pam_chauthtok(). That is to say, why we not abandon
>> memory creds and cached creds updating in winbindd_dual_pam
>> _chauthtok()? And just force one reauthentication after password change?
>>
>>       The idea behind the duplication is password can be changed by user
>> other than the user itself, eg, administrator. So, we give a chance to
>> update
>> cached creds if it is admin that change the password. (even the updating
>> of cached creds is not quite reliable....ugly?)
>>     
>
> Don't want to mess with this. It's complicated :-). In order to
> do so I'd have to go through this entire code again with testing,
> I don't have time for that right now :-(.
>   
:-(. I will figure out how to test this.
>   
>> @@:
>>      Sorry. It seems I was out of order when write the patch. There is
>> so many
>> logic problems. :-).
>> And my test had not return errors other than NT_STATUS_OBJECT_NAME_
>> NOT_FOUND or NT_STATUS_NO_SUCH_USER. ........ Sorry.  
>> Thank you very much for your patient guidance! thank you!
>> I can resubmit the patch at 7th, July. To avoid duplicate work, who
>> will  rewrite
>> the patch, you or I?
>>     
>
> Apart from the "cache_ret" should be "result" error you pointed
> out above, does my patch functionally fix the issues I pointed
> out before ? If so, then I'll fix that one error and commit it.
>   
Yes. It fixed the issue you pointed out before.
> If not, let me know what other problems you see (let's not
> go into fixing the duplication right now, too much change to
> take on for a simple fix :-).
>
> I'm happy to finish this up and commit, but I'd appreciate
> you being able to test it for me.
>   
Sure. I'll test it.
> Thanks,
>
> Jeremy.
>
>   

-------------- next part --------------
A non-text attachment was scrubbed...
Name: boyang.vcf
Type: text/x-vcard
Size: 209 bytes
Desc: not available
Url : http://lists.samba.org/archive/samba-technical/attachments/20080704/d023b41e/boyang.vcf