Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to track last sign-in time of users #9942

Merged
merged 5 commits into from
Sep 17, 2024

Conversation

rhafer
Copy link
Contributor

@rhafer rhafer commented Aug 29, 2024

Related issue: https://github.com/owncloud/enterprise/issues/6726

To allow tracking new user signins we're are now emitting an UserSignedIn event from the proxy's account resolve middleware.

A goroutine, started as part of the graph service (the only service that might have write access to the idm database currently) consumes that event and updates a new LDAP attribute ocLastSignInTimestamp on the user object.

The PR does not yet expose the attribute on the users endpoint. Support for that will be added in a separate PR.

@rhafer rhafer force-pushed the enterprise/6726 branch 2 times, most recently from 336e81e to a5aadca Compare August 29, 2024 12:13
@rhafer rhafer marked this pull request as ready for review August 29, 2024 12:14
@rhafer rhafer force-pushed the enterprise/6726 branch 2 times, most recently from 1e01cc4 to 3c87192 Compare August 29, 2024 13:09
@2403905
Copy link
Contributor

2403905 commented Aug 29, 2024

LGFM

@mmattel
Copy link
Contributor

mmattel commented Aug 29, 2024

Missing a description in the graph's README about this functionality...

Every time the OIDC middleware sees a new access token (i.e when it needs
to update the userinfo cache) we consider that as a new login. In this case
the middleware add a new flag to the context, which is then used by the
accountresolver middleware to publish a UserSignedIn event.
The event needs to be sent by the accountresolver middleware, because only
at that point we know the user id of the user that just logged in.

(It would probably makes sense to merge the auth and account middleware into a
single component to avoid passing flags around via context)
Pass them to the identity backend to update the last sign-in date of the user.
@rhafer
Copy link
Contributor Author

rhafer commented Sep 17, 2024

Missing a description in the graph's README about this functionality...

The README is currently lacking ANY information about the user management related feature. IMO adding a single paragraph about the sign-in time tracking there without any additional context brings no additional value.

I am fine adding adding a more complete section about the user management features and related options. Including the features added with this PR. But I will work on that in a separate PR.

Copy link

sonarcloud bot commented Sep 17, 2024

@rhafer rhafer merged commit 0bd4b90 into owncloud:master Sep 17, 2024
4 checks passed
@rhafer rhafer deleted the enterprise/6726 branch September 17, 2024 14:02
@d7oc
Copy link
Contributor

d7oc commented Sep 20, 2024

@rhafer Which release will this end up in?

@rhafer
Copy link
Contributor Author

rhafer commented Sep 23, 2024

It's not part of any release yet. It's only in master AFAICS

@rhafer
Copy link
Contributor Author

rhafer commented Sep 23, 2024

Should become part of 6.4.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants