[PATCH] Use systemd-login session information to check if user is logged in

[Simo Sorce]

This patch was meant to remove the trolling of /proc for uids but I
haven't got there yet.
I meant to use utmp as a fallback but I realized utmp does not deal with
uid numbers rather usernames, and I need to think if it makes sense to
go that way as it may be hairy when we use subdomains as now I need to
know what kind of user name was used (fully qualified or not etcc.).

So for now I left the old code as a fallback in case systemd is not
available or fails unexpectedly.

Ticket: #2084

Simo.

[Jakub Hrozek]

On Wed, Sep 11, 2013 at 10:26:41AM -0400, Simo Sorce wrote:
> This patch was meant to remove the trolling of /proc for uids but I
> haven't got there yet.
> I meant to use utmp as a fallback but I realized utmp does not deal with
> uid numbers rather usernames, and I need to think if it makes sense to
> go that way as it may be hairy when we use subdomains as now I need to
> know what kind of user name was used (fully qualified or not etcc.).
>
> So for now I left the old code as a fallback in case systemd is not
> available or fails unexpectedly.
>
> Ticket: #2084
>
> Simo.
> +#ifdef HAVE_SYSTEMD_LOGIN
> ret = sd_uid_get_sessions(uid, 0, NULL);
> if (ret > 0) {
>    *result = true;
> }
> if (ret == 0) {
>    *result = false;
> }
> if (ret >= 0) {
>    return EOK;
> }
> DEBUG(0, ("systemd-login gave error %d: %s\n",
>          -ret, strerror(-ret)));
> /* fall back to the old method */

Please use SSSDBG here. Otherwise the code looks OK. Can you also open a
ticket to to document this change in behaviour either in man page or at
the very least the release notes?

[Simo Sorce]

On Wed, 2013-09-11 at 19:19 +0200, Jakub Hrozek wrote:
> On Wed, Sep 11, 2013 at 10:26:41AM -0400, Simo Sorce wrote:
> > This patch was meant to remove the trolling of /proc for uids but I
> > haven't got there yet.
> > I meant to use utmp as a fallback but I realized utmp does not deal with
> > uid numbers rather usernames, and I need to think if it makes sense to
> > go that way as it may be hairy when we use subdomains as now I need to
> > know what kind of user name was used (fully qualified or not etcc.).
> >
> > So for now I left the old code as a fallback in case systemd is not
> > available or fails unexpectedly.
> >
> > Ticket: #2084
> >
> > Simo.
> > +#ifdef HAVE_SYSTEMD_LOGIN
> > ret = sd_uid_get_sessions(uid, 0, NULL);
> > if (ret > 0) {
> >    *result = true;
> > }
> > if (ret == 0) {
> >    *result = false;
> > }
> > if (ret >= 0) {
> >    return EOK;
> > }
> > DEBUG(0, ("systemd-login gave error %d: %s\n",
> >          -ret, strerror(-ret)));
> > /* fall back to the old method */
>
> Please use SSSDBG here.

attached.
> Otherwise the code looks OK. Can you also open a
> ticket to to document this change in behaviour either in man page or at
> the very least the release notes?

I already have a ticket for this change, so I added a release note in
that appropriate field there.

Simo.

[Jakub Hrozek]

On Wed, Sep 11, 2013 at 05:47:31PM -0400, Simo Sorce wrote:
> On Wed, 2013-09-11 at 19:19 +0200, Jakub Hrozek wrote:
> > On Wed, Sep 11, 2013 at 10:26:41AM -0400, Simo Sorce wrote:
> > > This patch was meant to remove the trolling of /proc for uids but I
> > > haven't got there yet.
> > > I meant to use utmp as a fallback but I realized utmp does not deal with
> > > uid numbers rather usernames, and I need to think if it makes sense to
> > > go that way as it may be hairy when we use subdomains as now I need to
> > > know what kind of user name was used (fully qualified or not etcc.).
> > >
> > > So for now I left the old code as a fallback in case systemd is not
> > > available or fails unexpectedly.
> > >
> > > Ticket: #2084
> > >
> > > Simo.
> > > +#ifdef HAVE_SYSTEMD_LOGIN
> > > ret = sd_uid_get_sessions(uid, 0, NULL);
> > > if (ret > 0) {
> > >    *result = true;
> > > }
> > > if (ret == 0) {
> > >    *result = false;
> > > }
> > > if (ret >= 0) {
> > >    return EOK;
> > > }
> > > DEBUG(0, ("systemd-login gave error %d: %s\n",
> > >          -ret, strerror(-ret)));
> > > /* fall back to the old method */
> >
> > Please use SSSDBG here.
>
> attached.
> > Otherwise the code looks OK. Can you also open a
> > ticket to to document this change in behaviour either in man page or at
> > the very least the release notes?
>
> I already have a ticket for this change, so I added a release note in
> that appropriate field there.
>
> Simo.

That's fine, thank you.

ACK

[Jakub Hrozek]

On Thu, Sep 12, 2013 at 07:10:38PM +0200, Jakub Hrozek wrote:
> On Wed, Sep 11, 2013 at 05:47:31PM -0400, Simo Sorce wrote:
> > On Wed, 2013-09-11 at 19:19 +0200, Jakub Hrozek wrote:
> > > On Wed, Sep 11, 2013 at 10:26:41AM -0400, Simo Sorce wrote:
> > > > This patch was meant to remove the trolling of /proc for uids but I
> > > > haven't got there yet.
> > > > I meant to use utmp as a fallback but I realized utmp does not deal with
> > > > uid numbers rather usernames, and I need to think if it makes sense to
> > > > go that way as it may be hairy when we use subdomains as now I need to
> > > > know what kind of user name was used (fully qualified or not etcc.).
> > > >
> > > > So for now I left the old code as a fallback in case systemd is not
> > > > available or fails unexpectedly.
> > > >
> > > > Ticket: #2084
> > > >
> > > > Simo.
> > > > +#ifdef HAVE_SYSTEMD_LOGIN
> > > > ret = sd_uid_get_sessions(uid, 0, NULL);
> > > > if (ret > 0) {
> > > >    *result = true;
> > > > }
> > > > if (ret == 0) {
> > > >    *result = false;
> > > > }
> > > > if (ret >= 0) {
> > > >    return EOK;
> > > > }
> > > > DEBUG(0, ("systemd-login gave error %d: %s\n",
> > > >          -ret, strerror(-ret)));
> > > > /* fall back to the old method */
> > >
> > > Please use SSSDBG here.
> >
> > attached.
> > > Otherwise the code looks OK. Can you also open a
> > > ticket to to document this change in behaviour either in man page or at
> > > the very least the release notes?
> >
> > I already have a ticket for this change, so I added a release note in
> > that appropriate field there.
> >
> > Simo.
>
> That's fine, thank you.
>
> ACK

Actually, I think we also need to pull in systemd-devel. See the
attached patch, can I squash it in?

[Simo Sorce]

On Fri, 2013-09-13 at 17:14 +0200, Jakub Hrozek wrote:
> On Thu, Sep 12, 2013 at 07:10:38PM +0200, Jakub Hrozek wrote:
> > On Wed, Sep 11, 2013 at 05:47:31PM -0400, Simo Sorce wrote:
> > > On Wed, 2013-09-11 at 19:19 +0200, Jakub Hrozek wrote:
> > > > On Wed, Sep 11, 2013 at 10:26:41AM -0400, Simo Sorce wrote:
> > > > > This patch was meant to remove the trolling of /proc for uids but I
> > > > > haven't got there yet.
> > > > > I meant to use utmp as a fallback but I realized utmp does not deal with
> > > > > uid numbers rather usernames, and I need to think if it makes sense to
> > > > > go that way as it may be hairy when we use subdomains as now I need to
> > > > > know what kind of user name was used (fully qualified or not etcc.).
> > > > >
> > > > > So for now I left the old code as a fallback in case systemd is not
> > > > > available or fails unexpectedly.
> > > > >
> > > > > Ticket: #2084
> > > > >
> > > > > Simo.
> > > > > +#ifdef HAVE_SYSTEMD_LOGIN
> > > > > ret = sd_uid_get_sessions(uid, 0, NULL);
> > > > > if (ret > 0) {
> > > > >    *result = true;
> > > > > }
> > > > > if (ret == 0) {
> > > > >    *result = false;
> > > > > }
> > > > > if (ret >= 0) {
> > > > >    return EOK;
> > > > > }
> > > > > DEBUG(0, ("systemd-login gave error %d: %s\n",
> > > > >          -ret, strerror(-ret)));
> > > > > /* fall back to the old method */
> > > >
> > > > Please use SSSDBG here.
> > >
> > > attached.
> > > > Otherwise the code looks OK. Can you also open a
> > > > ticket to to document this change in behaviour either in man page or at
> > > > the very least the release notes?
> > >
> > > I already have a ticket for this change, so I added a release note in
> > > that appropriate field there.
> > >
> > > Simo.
> >
> > That's fine, thank you.
> >
> > ACK
>
> Actually, I think we also need to pull in systemd-devel. See the
> attached patch, can I squash it in?

ACK, I meant to check that detail and then forgot, thank you for
noticing.

Simo.

[Jakub Hrozek]

On Fri, Sep 13, 2013 at 12:42:46PM -0400, Simo Sorce wrote:
> On Fri, 2013-09-13 at 17:14 +0200, Jakub Hrozek wrote:
> > On Thu, Sep 12, 2013 at 07:10:38PM +0200, Jakub Hrozek wrote:
> > > On Wed, Sep 11, 2013 at 05:47:31PM -0400, Simo Sorce wrote:
> > > > On Wed, 2013-09-11 at 19:19 +0200, Jakub Hrozek wrote:
> > > > > On Wed, Sep 11, 2013 at 10:26:41AM -0400, Simo Sorce wrote:
> > > > > > This patch was meant to remove the trolling of /proc for uids but I
> > > > > > haven't got there yet.
> > > > > > I meant to use utmp as a fallback but I realized utmp does not deal with
> > > > > > uid numbers rather usernames, and I need to think if it makes sense to
> > > > > > go that way as it may be hairy when we use subdomains as now I need to
> > > > > > know what kind of user name was used (fully qualified or not etcc.).
> > > > > >
> > > > > > So for now I left the old code as a fallback in case systemd is not
> > > > > > available or fails unexpectedly.
> > > > > >
> > > > > > Ticket: #2084
> > > > > >
> > > > > > Simo.
> > > > > > +#ifdef HAVE_SYSTEMD_LOGIN
> > > > > > ret = sd_uid_get_sessions(uid, 0, NULL);
> > > > > > if (ret > 0) {
> > > > > >    *result = true;
> > > > > > }
> > > > > > if (ret == 0) {
> > > > > >    *result = false;
> > > > > > }
> > > > > > if (ret >= 0) {
> > > > > >    return EOK;
> > > > > > }
> > > > > > DEBUG(0, ("systemd-login gave error %d: %s\n",
> > > > > >          -ret, strerror(-ret)));
> > > > > > /* fall back to the old method */
> > > > >
> > > > > Please use SSSDBG here.
> > > >
> > > > attached.
> > > > > Otherwise the code looks OK. Can you also open a
> > > > > ticket to to document this change in behaviour either in man page or at
> > > > > the very least the release notes?
> > > >
> > > > I already have a ticket for this change, so I added a release note in
> > > > that appropriate field there.
> > > >
> > > > Simo.
> > >
> > > That's fine, thank you.
> > >
> > > ACK
> >
> > Actually, I think we also need to pull in systemd-devel. See the
> > attached patch, can I squash it in?
>
> ACK, I meant to check that detail and then forgot, thank you for
> noticing.
>
> Simo.

OK, squashed in and pushed both to master.