D4806: [greeter] Send the auth result through the server instead return value

[Martin Gräßlin]

graesslin added a comment.


  In https://phabricator.kde.org/D4806#96684, @subdiff wrote:
  
  > Since there seem to be not many others I'll try to give you reviews for this and your following patches, but first I've to understand this stuff.
  >
  > Tell me if the following is right:
  >
  > Until now the greeter waited for the exit code of the forked process with the pid `m_pid`.
  
  
  yes, the exit code was the auth result. exit code 0 means auth success, everything else failure.
  
  > The child process executes the kcheckpass binary via execlp for pam/etcshadow password validation and exits immediately. The parent process now reads the requests and results from kcheckpass via socket `m_fd`. It gets notified when kcheckpass has written something to the socket thanks to `m_notifier`.
  
  yes
  
  > First kcheckpass's pam/etcshadows part asks for the provided password, i.e. transmits `ConvGetNormal` / `ConvGetHidden`, so `KCheckPass::handleVerify` writes back the provided password to the socket. kcheckpass can apparently without having to wait directly read it from the socket. Pam/etcshadow compares it with the set user password and (until now) exits with the respective success or fail code.
  
  yes
  
  > With your change instead of setting an exit code conv_server writes back one more time to the socket with the result (i.e. only the enum signaling it with empty prompt).
  
  yes
  
  > EDIT: Follow up question: In the current version at which point in the execution is the reapVerify method called at all? handleVerify is only connected to the activated method of QSocketNotifier and I assume that kcheckpass writes to the socket in the end and handleVerify is called one last time with `GRecvInt( &ret )` being false. But I don't see any more writes to the socket in kcheckpass.c and for example checkpass_shadow.c after the initial query for the provided password.
  
  reapVerify seems to me to be only if the communication horribly breaks. I did not fully understand the code and what it was doing. Part is to wait for the auth result, part is further error checking.
  In the long run I want to replace this by a nice QProcess, but one step at a time. This change is actually only a preparation step for the follow up which has the long running kcheckpass.

INLINE COMMENTS

> subdiff wrote in authenticator.cpp:221
> When you're at it: This seems to do the exact same thing as in `ConvGetNormal` case. Code duplication?

not unlikely. That code is old and got carried around.

REPOSITORY
  R133 KScreenLocker

REVISION DETAIL
  https://phabricator.kde.org/D4806

To: graesslin, #plasma
Cc: subdiff, plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170324/77f4b58b/attachment.html>