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

[Jeremy Allison]

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 :-(.

> @@:
>      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.

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.

Thanks,

Jeremy.