Administrators group

[dav1312]

Create a new group called administrators that has the same permissions as approvers but that can also change the group of other users

Users in this group can't be blocked and their group can't be changed

When the group of a user is changed, an action will be added to events

[ppigazzini]

There are a couple of conflicts to be solved.

[vondele]

btw, one feature for these admins could be to see the list of approvers. Right now, I think that's not easily found.

[dav1312]

btw, one feature for these admins could be to see the list of approvers. Right now, I think that's not easily found.

It would be nice to have an advanced search to filter users and runs but I don't think I have the required knowledge to do that

[ppigazzini]

DEV updated, added user00 in administrators group.

[ppigazzini]

@dav1312 If I'm not wrong it's possible to set the group only for a pending/idle user.

[dav1312]

I don't really know what an "idle" user is. A user that is not logged in? Or that it is?

[ppigazzini]

It's a user still without a contribution to a finished test (either creating a new test or donating a worker CPU). After some days the idle users are purged, see: https://github.com/glinscott/fishtest/blob/208952ca548c95fd0d48075aba5d86f5e29cbe83/server/utils/delta_update_users.py#L205-L231

[dav1312]

I don't know... when testing locally, my user01 is not in the idle list and I can block it and change its group

[ppigazzini]

Be sure to have the latest delta_update_users.py script (the previous one crashed with an empty DB) and run:

${VENV}/bin/python3 ${HOME}/fishtest/server/utils/delta_update_users.py

[ppigazzini]

I don't know... when testing locally, my user01 is not in the idle list and I can block it and change its group

What is the URI where you are checking the users?

[dav1312]

What is the URI where you are checking the users?

I used /pending

I don't know why but in dev user01 works but Vizvezdenec crashes and neither appear in the idle list

[ppigazzini]

Because Viz is not an idle user. The active users disappear from /pending and appear on /users

[dav1312]

Ok, I think I found the problem The block and group change only work for users that have a known registered date? https://dfts-0.pigazzini.it/user/dv8silencer

Does this happen in prod too? (the blocking part)

[ppigazzini]

On PROD on /pending I view only blocked and idle users (email cropped for privacy).

[ppigazzini]

@dav1312 blocked on PROD

[ppigazzini]

sudo journalctl -u fishtest@6544 --since "30 minutes ago"
...
...
Sep 12 13:38:35 dfts-0 pserve[30799]: 2022-09-12 13:38:35,346 ERROR [waitress][waitress-1] Exception while serving /user/dv8silencer
Sep 12 13:38:35 dfts-0 pserve[30799]: Traceback (most recent call last):
Sep 12 13:38:35 dfts-0 pserve[30799]:   File "/home/usr00/fishtest/server/env/lib/python3.10/site-packages/pyramid/tweens.py", line 13, in _error_handler
Sep 12 13:38:35 dfts-0 pserve[30799]:     response = request.invoke_exception_view(exc_info)
Sep 12 13:38:35 dfts-0 pserve[30799]:   File "/home/usr00/fishtest/server/env/lib/python3.10/site-packages/pyramid/view.py", line 786, in invoke_exception_view
Sep 12 13:38:35 dfts-0 pserve[30799]:     raise HTTPNotFound
Sep 12 13:38:35 dfts-0 pserve[30799]: pyramid.httpexceptions.HTTPNotFound: The resource could not be found.
Sep 12 13:38:35 dfts-0 pserve[30799]: During handling of the above exception, another exception occurred:
Sep 12 13:38:35 dfts-0 pserve[30799]: Traceback (most recent call last):
Sep 12 13:38:35 dfts-0 pserve[30799]:   File "/home/usr00/fishtest/server/env/lib/python3.10/site-packages/waitress/channel.py", line 428, in service
Sep 12 13:38:35 dfts-0 pserve[30799]:     task.service()
Sep 12 13:38:35 dfts-0 pserve[30799]:   File "/home/usr00/fishtest/server/env/lib/python3.10/site-packages/waitress/task.py", line 168, in service
Sep 12 13:38:35 dfts-0 pserve[30799]:     self.execute()
Sep 12 13:38:35 dfts-0 pserve[30799]:   File "/home/usr00/fishtest/server/env/lib/python3.10/site-packages/waitress/task.py", line 434, in execute
Sep 12 13:38:35 dfts-0 pserve[30799]:     app_iter = self.channel.server.application(environ, start_response)
Sep 12 13:38:35 dfts-0 pserve[30799]:   File "/home/usr00/fishtest/server/env/lib/python3.10/site-packages/waitress/proxy_headers.py", line 64, in translate_proxy_headers
Sep 12 13:38:35 dfts-0 pserve[30799]:     return app(environ, start_response)
Sep 12 13:38:35 dfts-0 pserve[30799]:   File "/home/usr00/fishtest/server/env/lib/python3.10/site-packages/pyramid/router.py", line 270, in __call__
Sep 12 13:38:35 dfts-0 pserve[30799]:     response = self.execution_policy(environ, self)
Sep 12 13:38:35 dfts-0 pserve[30799]:   File "/home/usr00/fishtest/server/env/lib/python3.10/site-packages/pyramid/router.py", line 276, in default_execution_policy
Sep 12 13:38:35 dfts-0 pserve[30799]:     return router.invoke_request(request)
Sep 12 13:38:35 dfts-0 pserve[30799]:   File "/home/usr00/fishtest/server/env/lib/python3.10/site-packages/pyramid/router.py", line 245, in invoke_request
Sep 12 13:38:35 dfts-0 pserve[30799]:     response = handle_request(request)
Sep 12 13:38:35 dfts-0 pserve[30799]:   File "/home/usr00/fishtest/server/env/lib/python3.10/site-packages/pyramid/tweens.py", line 43, in excview_tween
Sep 12 13:38:35 dfts-0 pserve[30799]:     response = _error_handler(request, exc)
Sep 12 13:38:35 dfts-0 pserve[30799]:   File "/home/usr00/fishtest/server/env/lib/python3.10/site-packages/pyramid/tweens.py", line 17, in _error_handler
Sep 12 13:38:35 dfts-0 pserve[30799]:     reraise(*exc_info)
Sep 12 13:38:35 dfts-0 pserve[30799]:   File "/home/usr00/fishtest/server/env/lib/python3.10/site-packages/pyramid/util.py", line 733, in reraise
Sep 12 13:38:35 dfts-0 pserve[30799]:     raise value
Sep 12 13:38:35 dfts-0 pserve[30799]:   File "/home/usr00/fishtest/server/env/lib/python3.10/site-packages/pyramid/tweens.py", line 41, in excview_tween
Sep 12 13:38:35 dfts-0 pserve[30799]:     response = handler(request)
Sep 12 13:38:35 dfts-0 pserve[30799]:   File "/home/usr00/fishtest/server/env/lib/python3.10/site-packages/pyramid/router.py", line 143, in handle_request
Sep 12 13:38:35 dfts-0 pserve[30799]:     response = _call_view(
Sep 12 13:38:35 dfts-0 pserve[30799]:   File "/home/usr00/fishtest/server/env/lib/python3.10/site-packages/pyramid/view.py", line 674, in _call_view
Sep 12 13:38:35 dfts-0 pserve[30799]:     response = view_callable(context, request)
Sep 12 13:38:35 dfts-0 pserve[30799]:   File "/home/usr00/fishtest/server/env/lib/python3.10/site-packages/pyramid/viewderivers.py", line 427, in rendered_view
Sep 12 13:38:35 dfts-0 pserve[30799]:     result = view(context, request)
Sep 12 13:38:35 dfts-0 pserve[30799]:   File "/home/usr00/fishtest/server/env/lib/python3.10/site-packages/pyramid/viewderivers.py", line 141, in _requestonly_view
Sep 12 13:38:35 dfts-0 pserve[30799]:     response = view(request)
Sep 12 13:38:35 dfts-0 pserve[30799]:   File "/home/usr00/fishtest/server/fishtest/views.py", line 455, in user
Sep 12 13:38:35 dfts-0 pserve[30799]:     if user_data["blocked"] == unblock:
Sep 12 13:38:35 dfts-0 pserve[30799]: KeyError: 'blocked'

[ppigazzini]

Adding the group "approvers" (verified the db entry):

Deleting the group "approvers" (verified the db entry):

Possible improvements:

• dropping the info about the unblocking
• have an easier way to view the active users (eg a link on the name in the /users page if logged as administrators)

[dav1312]

dropping the info about the unblocking

I already tried that and as u could see, it crashed, so I removed it 😅 It happens on prod right now too. If u just press the submit button without changing anything else, the unblocking message will appear

[ppigazzini]

DEV update ("user00" administrator, "user01" approver, "user02-19" normal user) the link works fine for administrators and approvers.

[dav1312]

This is what I used to use to prevent the unblocking warning

unblock = request.POST.get("blocked") == None
if user_data["blocked"] == unblock:
    user_data["blocked"] = "blocked" in request.POST
    ...

I don't know, maybe u can figure out a way to make it work with those special users

[peregrineshahin]

Is the typo fixed https://github.com/glinscott/fishtest/commit/2ab066cb5d409c198bfb4a6f5f1c9642dcd349c9 The cause of the crash here?

[ppigazzini]

No. That bugfixed code is not used anymore, all "group:admins" are longitme dropped from the users collection.

The dave's snippet of code crashes simply because it checks a not existent key in the second line (we have 1946 users and only 801 with the "blocked" key) . The 3 lines of code should be functionally equivalent to only the last line. I haven't checked the whole code, though.

user_data["blocked"] = "blocked" in request.POST

[ppigazzini]

Added the "blocked" key to all documents in the users collection. Dropped the not used "group:stats" too.

#!/usr/bin/env python3

from fishtest.rundb import RunDb

def update_users():
    rundb = RunDb()
    u_block= 0
    u_stats = 0
    for u in rundb.userdb.get_users():
        if "blocked" not in u:
            u["blocked"] = False
            rundb.userdb.save_user(u)
            u_block += 1
        update = False
        while "group:stats" in u["groups"]:
            u["groups"].remove("group:stats")
            update = True
        if update:
            rundb.userdb.save_user(u)
            u_stats += 1

    print("added blocked users:", u_block)
    print("dropped stats:", u_stats)

if __name__ == "__main__":
    update_users()

[ppigazzini]

DEV updated, it seems to work. Late here, I will test/review tomorrow morning.

[ppigazzini]

The PR worked in my tests, but there is an architectural problem, though. For a document in the users collection the "groups" key is a list with zero to many groups, and a single group enable one "power". In the past were used the groups "admin", "stats" and "approvers", now master is using only the group "approvers". In this PR code the groups seems to be intended as hierarchical (or zero or one group) and not additive.

[dav1312]

In this PR code the groups seems to be intended as hierarchical (or zero or one group) and not additive.

It was easier for me to code that way. Just remove all groups instead of searching and removing a specific one when changing groups etc. I can try to make it additive but help would be appreciated.

[ppigazzini]

I will help with the multi group approach. BTW this code was mostly new for me, and there are some things that I need to study :)

[ppigazzini]

This is a first improvement, the code should still work for a user with more than one group. A proper solution needs a page to show the user groups, to add one of the system available groups and to remove one of system available groups. It seems many lines of server code to replace few lines of a python script.

diff --git a/server/fishtest/templates/user.mak b/server/fishtest/templates/user.mak
index bb4add3..78c814d 100644
--- a/server/fishtest/templates/user.mak
+++ b/server/fishtest/templates/user.mak
@@ -6,7 +6,8 @@

 <%
   user_is_blocked = user['blocked'] if 'blocked' in user else False
-  user_group = user['groups'][0] if len(user['groups']) > 0 else ''
+  user_group = "".join(user['groups'])
+  user_not_admin = not any(g == 'group:administrators' for g in user['groups'])
   show_submit = False
 %>

@@ -81,7 +82,7 @@
         </div>
         <span class="input-group-text toggle-password-visibility" role="button"><i class="fa-solid fa-lg fa-eye pe-none" style="width: 30px"></i></span>
       </div>
-    % elif user_group != 'group:administrators':
+    % elif user_not_admin:
       <%
         show_submit = True
       %>
@@ -98,7 +99,7 @@
       </div>
     % endif

-    % if not profile and admin and user_group != 'group:administrators':
+    % if not profile and admin and user_not_admin:
       <%
         show_submit = True
       %>
diff --git a/server/fishtest/views.py b/server/fishtest/views.py
index 85e44e5..6094aa1 100644
--- a/server/fishtest/views.py
+++ b/server/fishtest/views.py
@@ -446,9 +446,8 @@ def user(request):
                     request.session.flash("Success! Email updated")

         if request.has_permission("approve_run"):
-            user_group = user_data["groups"][0] if len(user_data["groups"]) > 0 else ''
             ## cannot block/unblock an administrator
-            if user_group == "group:administrators":
+            if any(g == "group:administrators" for g in user_data["groups"]):
                 request.session.flash("Cannot block/unblock an administrator", "error")
                 return HTTPFound(location=request.route_url("tests"))
             unblock = request.POST.get("blocked") is None
@@ -468,7 +467,7 @@ def user(request):
         if request.has_permission("administrate"):
             new_group = request.params.get("group")
             ## cannot change the group of an administrator
-            if user_group == "group:administrators":
+            if any(g == "group:administrators" for g in user_data["groups"]):
                 request.session.flash("Cannot change group", "error")
                 return HTTPFound(location=request.route_url("tests"))
             ## check that the new group is valid
@@ -476,10 +475,10 @@ def user(request):
                 request.session.flash("Invalid group", "error")
                 return HTTPFound(location=request.route_url("tests"))
             ## check that the user is not already in the group
-            if new_group != user_group:
+            if not any(g == new_group for g in user_data["groups"]):
                 request.actiondb.change_group(
                     request.authenticated_userid,
-                    {"user": user_name, "before": user_group, "after": new_group},
+                    {"user": user_name, "before": "".join(user_data["groups"]), "after": new_group},
                 )
                 request.userdb.remove_user_groups(user_name)
                 if len(new_group) > 0:
 

[dav1312]

Do you also want me to change the ACL and remove the approve perm from admins?

A proper solution needs a page to show the user groups, to add one of the system available groups and to remove one of system available groups.

For that we would need to get the list from the ACL I guess and send the list to the template