Adds command to list user sessions #24675

MorrisJobke · 2016-05-17T15:07:58Z

extends public interface IToken to expose name and lastActivity
~~extends public interface Provider to expose tokens by user~~ see add method to query all user auth tokens #24686
add command to list sessions per user

cc @LukasReschke @PVince81 @DeepDiver1975 @ChristophWurst @nickvergessen

mention-bot · 2016-05-17T15:07:59Z

By analyzing the blame information on this pull request, we identified @ChristophWurst, @nickvergessen and @schiesbn to be potential reviewers

MorrisJobke · 2016-05-17T15:08:21Z

lib/private/Authentication/Token/DefaultTokenMapper.php

+	 * @param string $uid
+	 * @return DefaultToken[]
+	 */
+	public function getTokenByUser($uid) {


I'm open for naming suggestions :)

ChristophWurst · 2016-05-17T15:15:29Z

Note that currently multiple IProviders could be used, but I'm working on a PR to remove that unnecessary complexity.

ChristophWurst · 2016-05-17T15:16:36Z

lib/private/Authentication/Token/IProvider.php

+	 * @return IToken[]
+	 * @since 9.1
+	 */
+	public function getTokenByUser($uid);


How about passing an IUser object for greater flexibility?

ChristophWurst · 2016-05-17T15:32:57Z

Since on master we're querying multiple providers, while this PR assumes there is only one, #24677 is required for this PR to work correctly.

ChristophWurst · 2016-05-17T15:44:48Z

How about also showing the token type (browser/client)?

ChristophWurst · 2016-05-18T08:40:17Z

core/register_command.php

@@ -127,6 +127,7 @@
 	$application->add(new OC\Core\Command\User\LastSeen(\OC::$server->getUserManager()));
 	$application->add(new OC\Core\Command\User\Report(\OC::$server->getUserManager()));
 	$application->add(new OC\Core\Command\User\ResetPassword(\OC::$server->getUserManager()));
+	$application->add(new OC\Core\Command\User\Sessions(\OC::$server->query('OC\Authentication\Token\DefaultTokenProvider')));


I'd prefer to query the interface here, so we can easily swap the implementation.

Add a get method to the server container, querying an interface is a good base idea, but not the final solution.
Basically query() should be used internally only.

Add a get method to the server container, querying an interface is a good base idea, but not the final solution.
Basically query() should be used internally only.

It is the interim solution until the auto injection also works in core ;)

I added the server method in my PR: https://github.com/owncloud/core/pull/24703/files#diff-0a830b0354b26e84f08f705b5356a05bR842

Well the problem is, with the class name being used it can not be replaced. It should use the interface and the interface should be a service using a factory, to allow "swapping" with a different provider.
Basically something like https://github.com/owncloud/core/blob/master/lib/private/Server.php#L160-L172 but with the public interface as service name for the auto-injection

davitol · 2016-05-18T14:02:53Z

sudo -u www-data ./occ user:sessions admin
+----+---------------------------------------------------------------------------------------------------------------------------------+----------------------------------+
| id | name                                                                                                                            | last activity                    |
+----+---------------------------------------------------------------------------------------------------------------------------------+----------------------------------+
| 1  | curl/7.35.0                                                                                                                     | 2016-05-18 13:21:41 UTC (+00:00) |
| 3  | Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/50.0.2661.102 Safari/537.36       | 2016-05-18 13:59:31 UTC (+00:00) |
| 4  | Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_5) AppleWebKit/600.8.9 (KHTML, like Gecko) Version/8.0.8 Safari/600.8.9            | 2016-05-18 13:59:42 UTC (+00:00) |
| 5  | Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2486.0 Safari/537.36 Edge/13.10586 | 2016-05-18 14:00:04 UTC (+00:00) |
| 7  | Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:46.0) Gecko/20100101 Firefox/46.0                                              | 2016-05-18 13:59:34 UTC (+00:00) |
| 8  | Mozilla/5.0 (iOS) ownCloud-iOS/3.4.9                                                                                            | 2016-05-18 14:00:07 UTC (+00:00) |
| 10 | Mozilla/5.0 (iOS) ownCloud-iOS/3.4.9                                                                                            | 2016-05-18 14:00:10 UTC (+00:00) |
+----+---------------------------------------------------------------------------------------------------------------------------------+----------------------------------+

Why if I start a session from a iOS oC app, the command shows two active sessions of a user?

@ChristophWurst @MorrisJobke

ChristophWurst · 2016-05-18T14:18:01Z

@davitol that might be a bug. We had to add a little hack that creates session tokens for DAV clients, which can cause multiple session tokens being created if the client does not use cookies.
@LukasReschke you said you'll add some magic to fix that, didn't you? :-)

MorrisJobke · 2016-05-18T14:45:01Z

lib/private/Server.php

@@ -223,6 +223,7 @@ public function __construct($webRoot, \OC\Config $config) {
 			$timeFactory = new TimeFactory();
 			return new \OC\Authentication\Token\DefaultTokenProvider($mapper, $crypto, $config, $logger, $timeFactory);
 		});
+		$this->registerAlias('OC\Authentication\Token\IProvider', 'OC\Authentication\Token\DefaultTokenProvider');


@nickvergessen This is the interface thingy you requested - cc @ChristophWurst

MorrisJobke · 2016-05-18T14:57:12Z

I rebased this on top of #24686 - this now uses the proposed methods from @ChristophWurst PR :)

@nickvergessen @ChristophWurst Please re-review

MorrisJobke · 2016-05-18T14:58:04Z

This is the stuff, that is added here: 904d50c

ChristophWurst · 2016-05-18T15:02:12Z

👍 very nice

nickvergessen · 2016-05-18T15:04:49Z

core/Command/User/Sessions.php

+		$user = $this->userManager->get($uid);
+		if(is_null($user)) {
+			$output->writeln('<error>User "' . $uid . '" does not exist</error>');
+			return;


Error means return != 0 ? 😉

georgehrke · 2016-05-24T14:33:50Z

please rebase

MorrisJobke · 2016-05-25T09:34:02Z

@ChristophWurst Feel free to take over

PVince81 · 2016-05-30T12:42:28Z

We're past feature freeze, moving to 9.2.

CC @cmonteroluque @MTRichards

ghost · 2016-05-31T16:15:50Z

nod

mmattel · 2016-08-17T06:56:10Z

@ChristophWurst

We had to add a little hack that creates session tokens for DAV clients, which can cause multiple session tokens being created if the client does not use cookies.

Is this hack still necessary respectively can that be fixed that a DAV client produces only one session entry? Reason: querying table authtoken for statistical or access purposes via occ user command (see also referenced issue #25708)

ChristophWurst · 2016-08-17T07:02:58Z

Is this hack still necessary respectively can that be fixed that a DAV client produces only one session entry?

AFAIK that is more or less impossible. Clients can be stateless, hence there's nothing that identifies/groups a set of related requests.

* extends public interface IToken to expose name and lastActivity * extends public interface Provider to expose tokens by user * add command to list sessions per user

PVince81 · 2016-12-08T09:13:15Z

@PhilippSchaffrath FYI

PVince81 · 2017-04-06T15:03:24Z

@pmaier1 not sure if you have a concept in regards to session / token management completeness.

I think the purpose was for an admin to be able to list all user sessions (or search by a user) and then be able to terminate such sessions with another OCC command.

Moving to backlog until we can find time to finalize this

lock · 2019-08-03T02:01:47Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

MorrisJobke added the 3 - To Review label May 17, 2016

MorrisJobke added this to the 9.1-current milestone May 17, 2016

MorrisJobke reviewed May 17, 2016
View reviewed changes

ChristophWurst reviewed May 17, 2016
View reviewed changes

MorrisJobke added 2 - Developing and removed 3 - To Review labels May 17, 2016

MorrisJobke self-assigned this May 17, 2016

ChristophWurst reviewed May 18, 2016
View reviewed changes

ChristophWurst mentioned this pull request May 18, 2016

add method to query all user auth tokens #24686

Merged

MorrisJobke force-pushed the command-to-list-user-sessions branch from 1c1e1f6 to 3dfc75d Compare May 18, 2016 14:43

MorrisJobke reviewed May 18, 2016
View reviewed changes

MorrisJobke force-pushed the command-to-list-user-sessions branch from 3dfc75d to 904d50c Compare May 18, 2016 14:56

MorrisJobke added 3 - To Review and removed 2 - Developing labels May 18, 2016

nickvergessen reviewed May 18, 2016
View reviewed changes

MorrisJobke added 2 - Developing and removed 3 - To Review labels May 19, 2016

MorrisJobke assigned ChristophWurst and unassigned MorrisJobke May 25, 2016

PVince81 modified the milestones: 9.2-next, 9.1-current May 30, 2016

DeepDiver1975 mentioned this pull request Aug 5, 2016

[Feature Request] ./occ user:logonlist #25708

Closed

MorrisJobke and others added 3 commits September 22, 2016 08:53

Adds command to list user sessions

3cadaf3

* extends public interface IToken to expose name and lastActivity * extends public interface Provider to expose tokens by user * add command to list sessions per user

return proper status code for occ user:sessions

399eeb1

Fix warnings

5d5d017

DeepDiver1975 force-pushed the command-to-list-user-sessions branch from 5a0f1a7 to 5d5d017 Compare September 22, 2016 07:18

ChristophWurst removed their assignment Dec 5, 2016

PVince81 modified the milestones: backlog, 10.0 Apr 6, 2017

PVince81 closed this Jul 19, 2017

PVince81 deleted the command-to-list-user-sessions branch July 19, 2017 10:24

PVince81 mentioned this pull request Nov 13, 2017

Ability for admins to terminate other user's sessions #29548

Closed

lock bot locked as resolved and limited conversation to collaborators Aug 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds command to list user sessions #24675

Adds command to list user sessions #24675

MorrisJobke commented May 17, 2016 •

edited

Loading

mention-bot commented May 17, 2016

MorrisJobke May 17, 2016

ChristophWurst commented May 17, 2016

ChristophWurst May 17, 2016

ChristophWurst commented May 17, 2016

ChristophWurst commented May 17, 2016

ChristophWurst May 18, 2016

nickvergessen May 18, 2016

MorrisJobke May 18, 2016

ChristophWurst May 18, 2016

nickvergessen May 18, 2016

davitol commented May 18, 2016 •

edited by nickvergessen

Loading

ChristophWurst commented May 18, 2016

MorrisJobke May 18, 2016

MorrisJobke commented May 18, 2016

MorrisJobke commented May 18, 2016

ChristophWurst commented May 18, 2016

nickvergessen May 18, 2016

MorrisJobke May 18, 2016

georgehrke commented May 24, 2016

MorrisJobke commented May 25, 2016

PVince81 commented May 30, 2016

ghost commented May 31, 2016

mmattel commented Aug 17, 2016

ChristophWurst commented Aug 17, 2016

PVince81 commented Dec 8, 2016

PVince81 commented Apr 6, 2017

lock bot commented Aug 3, 2019

Adds command to list user sessions #24675

Adds command to list user sessions #24675

Conversation

MorrisJobke commented May 17, 2016 • edited Loading

mention-bot commented May 17, 2016

Choose a reason for hiding this comment

ChristophWurst commented May 17, 2016

Choose a reason for hiding this comment

ChristophWurst commented May 17, 2016

ChristophWurst commented May 17, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davitol commented May 18, 2016 • edited by nickvergessen Loading

ChristophWurst commented May 18, 2016

Choose a reason for hiding this comment

MorrisJobke commented May 18, 2016

MorrisJobke commented May 18, 2016

ChristophWurst commented May 18, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

georgehrke commented May 24, 2016

MorrisJobke commented May 25, 2016

PVince81 commented May 30, 2016

ghost commented May 31, 2016

mmattel commented Aug 17, 2016

ChristophWurst commented Aug 17, 2016

PVince81 commented Dec 8, 2016

PVince81 commented Apr 6, 2017

lock bot commented Aug 3, 2019

MorrisJobke commented May 17, 2016 •

edited

Loading

davitol commented May 18, 2016 •

edited by nickvergessen

Loading