From: Stefan Sperling <stsp@stsp.name>
Subject: Re: resolve account name of users connecting to gotd
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Wed, 27 Dec 2023 14:21:42 +0100

On Wed, Dec 27, 2023 at 02:11:27PM +0100, Omar Polo wrote:
> On 2023/12/26 17:54:30 +0100, Stefan Sperling <stsp@stsp.name> wrote:
> > Pass the account name of successfully authenticated users from the
> > gotd auth process to the gotd parent process.
> > 
> > This can be used to make log messages more readable. With this patch
> > we display the username instead of a UID when reporting successful
> > authentication in the logs. We could expand upon this later and tweak
> > other messages which currently display the UID. But I actually wrote
> > this patch because account name information will be required to
> > provide meaningful commit notifications.
> 
> It is already useful on its own, I'm happy not to have to grep
> /etc/passwd to remember which user is which uid :)
> 
> > One quirk which deviates from our usual style is that auth_check() will
> > not free the *username pointer on failure. I chose to free it in the
> > caller instead to keep the existing auth_check() implementation mostly
> > as it is, since the early returns are an important part of the code flow.
> > Alternatively, we could do another getpwuid() call in the caller but a
> > call to strdup() on the existing passwd object is likely cheaper.
> 
> I think it's fine.  Alternatively, we could just use a stack-allocated
> buffer and avoid all the headaches with managing another allocation.

The maximum length of the username is not easy to determine in
a portable way.

> > There is a small drive-by fix in here which ensures that the message
> > GOTD_IMSG_ACCESS_GRANTED will only be accepted once.
> > 
> > ok?
> 
> ok op@
> 
> two nits below.

Thanks! I've committed a slightly tweaked version which includes your nits.