Don't use $USER for Local Login check

Don't use $USER for Local Login check

Update the PAM Local Login check to use the username from getuid()/getpwuid(). The result of this is much harder for unprivileged users to change than the $USER environment variable.


BUG=224958
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=191548

[Jamie, 2013-03-29 17:25:24]

Can you add something to the CL description explaining why this approach is
better?

[Wez, 2013-03-29 17:27:57]

https://codereview.chromium.org/13312005/diff/1/remoting/base/util.h
File remoting/base/util.h (right):

https://codereview.chromium.org/13312005/diff/1/remoting/base/util.h#newcode96
remoting/base/util.h:96: bool CanGetUsername();
Semantically there's no difference between this not being implemented, and us
not being able to fetch the name due to an error, so please fold this case into
GetUsername and adjust the logic in the Me2Me host.

https://codereview.chromium.org/13312005/diff/1/remoting/host/pam_authorizati...
File remoting/host/pam_authorization_factory_posix.cc (right):

https://codereview.chromium.org/13312005/diff/1/remoting/host/pam_authorizati...
remoting/host/pam_authorization_factory_posix.cc:106: if (username.empty())
This if() differs in style from other single-line ifs in this file.  Please
either match the existing style or, preferably, update the existing if()s to
match this one.

[Sergey Ulanov, 2013-03-29 17:50:01]

https://codereview.chromium.org/13312005/diff/1/remoting/base/util.cc
File remoting/base/util.cc (right):

https://codereview.chromium.org/13312005/diff/1/remoting/base/util.cc#newcode332
remoting/base/util.cc:332: return "";
please use std::string() instead of ""

[Jamie, 2013-03-29 18:30:00]

https://codereview.chromium.org/13312005/diff/1/remoting/host/pam_authorizati...
File remoting/host/pam_authorization_factory_posix.cc (right):

https://codereview.chromium.org/13312005/diff/1/remoting/host/pam_authorizati...
remoting/host/pam_authorization_factory_posix.cc:106: if (username.empty())
On 2013/03/29 17:27:57, Wez wrote:
> This if() differs in style from other single-line ifs in this file.  Please
> either match the existing style or, preferably, update the existing if()s to
> match this one.

I disagree with your use of the word "preferably" :)

[Wez, 2013-03-29 18:43:22]

On 2013/03/29 18:30:00, Jamie wrote:
>
https://codereview.chromium.org/13312005/diff/1/remoting/host/pam_authorizati...
> File remoting/host/pam_authorization_factory_posix.cc (right):
> 
>
https://codereview.chromium.org/13312005/diff/1/remoting/host/pam_authorizati...
> remoting/host/pam_authorization_factory_posix.cc:106: if (username.empty())
> On 2013/03/29 17:27:57, Wez wrote:
> > This if() differs in style from other single-line ifs in this file.  Please
> > either match the existing style or, preferably, update the existing if()s to
> > match this one.
> 
> I disagree with your use of the word "preferably" :)

Disagreement duly noted.

However, let's not re-paint that particular bike shed.

[Lambros, 2013-03-29 19:22:08]

On 2013/03/29 17:25:24, Jamie wrote:
> Can you add something to the CL description explaining why this approach is
> better?

Done.

[Lambros, 2013-03-29 19:30:22]

https://codereview.chromium.org/13312005/diff/1/remoting/base/util.cc
File remoting/base/util.cc (right):

https://codereview.chromium.org/13312005/diff/1/remoting/base/util.cc#newcode332
remoting/base/util.cc:332: return "";
On 2013/03/29 17:50:01, sergeyu wrote:
> please use std::string() instead of ""

Done.

https://codereview.chromium.org/13312005/diff/1/remoting/base/util.h
File remoting/base/util.h (right):

https://codereview.chromium.org/13312005/diff/1/remoting/base/util.h#newcode96
remoting/base/util.h:96: bool CanGetUsername();
On 2013/03/29 17:27:57, Wez wrote:
> Semantically there's no difference between this not being implemented, and us
> not being able to fetch the name due to an error, so please fold this case
into
> GetUsername and adjust the logic in the Me2Me host.

Done.

https://codereview.chromium.org/13312005/diff/1/remoting/host/pam_authorizati...
File remoting/host/pam_authorization_factory_posix.cc (right):

https://codereview.chromium.org/13312005/diff/1/remoting/host/pam_authorizati...
remoting/host/pam_authorization_factory_posix.cc:106: if (username.empty())
On 2013/03/29 17:27:57, Wez wrote:
> This if() differs in style from other single-line ifs in this file.  Please
> either match the existing style or, preferably, update the existing if()s to
> match this one.

Done.

[Wez, 2013-03-29 19:58:56]

lgtm

https://codereview.chromium.org/13312005/diff/12001/remoting/base/util.h
File remoting/base/util.h (right):

https://codereview.chromium.org/13312005/diff/12001/remoting/base/util.h#newc...
remoting/base/util.h:96: // error.
nit: ... on error, or if not implemented.

[Lambros, 2013-03-29 21:25:20]

https://codereview.chromium.org/13312005/diff/12001/remoting/base/util.h
File remoting/base/util.h (right):

https://codereview.chromium.org/13312005/diff/12001/remoting/base/util.h#newc...
remoting/base/util.h:96: // error.
On 2013/03/29 19:58:56, Wez wrote:
> nit: ... on error, or if not implemented.

Done.

[commit-bot: I haz the power, 2013-03-29 21:25:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/lambroslambrou@chromium.org/13312005/1...

[commit-bot: I haz the power, 2013-03-30 17:05:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/lambroslambrou@chromium.org/13312005/1...

[commit-bot: I haz the power, 2013-03-30 20:12:43]

Change committed as 191548