remove old authenticated user sessions

[dnauck]

when you're using the restrict_owner setting there is no possibility to delete old users from this dropdown box.

[sylvain@…]

In fact, the drop down list in restrict_owner mode, should be better build from the permission list of something which list the user of the site.

My be a new permission priviledge (TICKET_ASSIGNABLE) could decide if a user listed here is selectable to be assigned as a ticket owner.

[Christopher Lenz]

Postponing…

[Christopher Lenz]

See also #2351.

[Christopher Lenz]

Later.

[markus]

The attached patch adds a User Sessions tab in WebAdmin from where you can delete the project's user sessions.

[markus]

With the second patch you can also change a user session's name or email. I'm looking forward to your comments. (I'm not sure if the readonly text input is consistent with the rest of Trac's UI)

[markus]

Is there any demand for an updated patch or should we close this ticket and wait for milestone:0.12 (announcing a Better user/session system)?

[Lutz Frommberger <webkontakt@…>]

It will be quite a while until 0.12 will show up. I'd prefer to have the patch again, even if I actually don't need it.

[sid]

A workaround for this problem is removing the TICKET_MODIFY permission for that user. If a user doesn't have that permission, they will not show up in the "Assign to" list. (according to TracTickets)

[Noah Kantrowitz (coderanger) <coderanger@…>]

Why does this have to be a patch? Just make a plugin to handle it.

[Emmanuel Blot]

Replying to Noah Kantrowitz (coderanger):

Why does this have to be a patch? Just make a plugin to handle it.

IMHO it would better be part of the WebAdmin plugin subsystem (vs. a plugin): this is an administrative task that is useful to any administrator.

[Noah Kantrowitz (coderanger) <coderanger@…>]

Replying to eblot:

Replying to Noah Kantrowitz (coderanger):

Why does this have to be a patch? Just make a plugin to handle it.

IMHO it would better be part of the WebAdmin plugin subsystem (vs. a plugin): this is an administrative task that is useful to any administrator.

At the point that Trac has a better user directory system, yes. Until then I am just saying it may be easier for them to make a small plugin than to try to keep a patch in sync with WebAdmin.

[anonymous]

Hi, Im a brazilian user of trac and I´ve made de modifications of the three patches and nothing has chance. There is some other information to apply this changes ?? Tks.

[Christian Boos]

Please don't change arbitrarily the milestone. not applicable is a special milestone (see TracTicketTriage#Milestone if you want more details).

[Archon810]

Replying to sid:

A workaround for this problem is removing the TICKET_MODIFY permission for that user. If a user doesn't have that permission, they will not show up in the "Assign to" list. (according to TracTickets)

That is a faulty solution as there may be a wider permission group giving this permission (i.e. 'authenticated' group). There needs to be a utility or a plugin that does the sync. I'm quite shocked that Trac would have this bug.

[John Hampton]

Patch for session management in trac-admin

[John Hampton]

OK, so, the attached patch​ adds session management to the trac-admin command. It comes with lots of tests, which pass, though I would like someone to review it for me.

Additionally, the ​account manager plugin removes sessions when removing users. ​AccountManagerPlugin should be merged for 0.13. The big question remains: Do we add the session management to the web admin, or just leave it in trac-admin until ​AccountManagerPlugin is merged. I think that session management in trac-admin is sufficient for 0.12. What say ye all?

[Remy Blank]

Replying to jhampton:

though I would like someone to review it for me.

A few comments from reading the patch:

• I would remove the periods at the end of the short command descriptions for consistency with the other commands.
• There are a few lines >79 characters, in test_session_delete_multiple_sids(), _set_attr(), test_session_admin_purge().
• There are a number of unnecessary pass statements.
• Using the separate variables CHECK_AUTH_SQL, CHECK_SID_SQL and ALL_SQL is a bit unusual, all the rest of Trac places the statements in the execute() call.
• CHECK_SID_SQL can be written more simply with a ",".join().
• Instead of raise StopIteration, you can simply return from the generator function.
• Some of your translation markers won't work. You must place the string literal within _() for it to be extracted correctly, and let _() make the string substitutions.
• When logging exceptions, use exception_to_unicode() as the exception can contain non-ascii characters that are not handled correctly by log. OTOH, other commands don't catch exceptions during their execution, so I would remove the try: except: blocks in _add_session(), _set_attr(), _delete_session() and _purge_sessions() altogether.

I have collected all this into a patch, I'll add it later as an attachment, so you can pick what you like from it (if anything at all).

I think that session management in trac-admin is sufficient for 0.12. What say ye all?

I think so, too.

[Remy Blank]

Cleanup + minor fixes, applies on top of t1347_session_management.patch​.

[Remy Blank]

I may have gone a bit too far with the cleanup, so… pick what you want and drop the rest :)

[John Hampton]

I committed the updated patch in [9262]. Thanks for the thorough review. I took all of your changes except for using != in the SQL query. I believe that <> is the SQL operator. I could only find a draft of ​SQL92 but I don't think != is standard.

I am closing the ticket as fixed due to trac-admin support being sufficient for 0.12