On Thu, 16 Feb 2023 at 19:16, Calvin Wan <calvinwan@xxxxxxxxxx> wrote:
>
> > static int run_credential_helper(struct credential *c,
> > @@ -342,6 +352,10 @@ void credential_fill(struct credential *c)
> >
> > for (i = 0; i < c->helpers.nr; i++) {
> > credential_do(c, c->helpers.items[i].string, "get");
> > + if (c->password_expiry_utc < time(NULL)) {
> > + FREE_AND_NULL(c->password);
> > + c->password_expiry_utc = TIME_MAX;
> > + }
> > if (c->username && c->password)
> > return;
> > if (c->quit)
>
> I see you null out c->password in the expiry if block so that the
> following c->password check in the following if statement fails.
> While I think it's neat little trick, I wonder if others on list
> think it's better to be more explicit with how the logic should
> work (eg. adding the c->passowrd_expiry_utc check as an inner
> block inside of the c->username && c->password block).
It's important to reset the expiry date as well as discard the expired
password so that fill accepts a later password without expiry (see
test cases). I'll add a comment in patch v4.
