Apache Allura™

• size: --> 1

• Milestone: forge-apr-19 --> forge-may-03

• Milestone: forge-may-03 --> forge-may-17

• Milestone: forge-may-17 --> forge-may-31

• Milestone: forge-may-31 --> forge-jun-06

• labels: --> 42cc
• Size: 1 --> 0
• Milestone: forge-jun-06 --> forge-backlog

• status: open --> in-progress

Created #363: [#6053] Check for disabled users in user lookups (2cp)

• status: in-progress --> code-review

Closed #363. je/42cc_6053

We've changed the claimed_by_user() to check disabled field.

There are other places we think this check should be done also. But I don't sure about them all, can you review them, please? And then we can make the changes.

1. User lookup for autocompletion (Allura/allura/controllers/project.py:406, ProjectController.user_search())
2. Finding users with named roles for given project (Allura/allura/model/project.py:708, Project.users())
3. Various lookups for notifications. Disabled users don't need to receive any, I guess.
   • Notification.send_direct (Allura/allura/model/notification.py:237)
   • Notification.send_digest (Allura/allura/model/notification.py:261)
   • sendmail() task (Allura/allura/tasks/mail_tasks.py:87, Allura/allura/tasks/mail_tasks.py:99)
   • sendsimplemail() task (Allura/allura/tasks/mail_tasks.py:149)
4. Auth-related lookups
   • AuthenticationProvider.authenticate_request (Allura/allura/lib/plugin.py:86)
   • LocalAuthenticationProvider.by_username (Allura/allura/lib/plugin.py:236)
   • LocalAuthenticationProvider.user_by_project_shortname (Allura/allura/lib/plugin.py:254)
   • LdapAuthenticationProvider.by_username (Allura/allura/lib/plugin.py:318)
   • LdapAuthenticationProvider._login (Allura/allura/lib/plugin.py:332)

• QA: Dave Brondsema

I've merged your current commits. Go ahead and make the other changes as follows:

1. yes
2. yes
3. yes. Make sure to add proper handling if no user is found. Just one example is send_direct() which assumes it'll always find a user.
   
4. authenticate_request already checks for disabled. Yes for the rest.
   

Don't be overly concerned about testing all these spots. Some will be easy & good to test, others (e.g. ldap) probably don't have any existing tests, so I don't think its necessary to spend time now building out tests in that area.

• status: code-review --> in-progress

Created #376: [#6053] Check for disabled users in user lookups (followup) (2cp)

• status: in-progress --> code-review

Closed #376. je/42cc_6053_followup

• status: code-review --> closed
• Milestone: forge-backlog --> forge-jun-28

This change seems unnecessary and tests pass without it, so I'm going to merge without it, unless there's some reason we need it:

--- Allura/allura/controllers/basetest_project_root.py
+++ Allura/allura/controllers/basetest_project_root.py
@@ -123,6 +123,8 @@ def __call__(self, environ, start_response):
         c.project = M.Project.query.get(shortname='test', neighborhood_id=self.p_nbhd._id)
         auth = plugin.AuthenticationProvider.get(request)
         user = auth.by_username(environ.get('username', 'test-admin'))
+        if not user:
+            user = M.User.by_username('test-admin')
         environ['beaker.session']['userid'] = user._id
         c.user = auth.authenticate_request()
         return WsgiDispatchController.__call__(self, environ, start_response)

Ah, I guess I didn't run the tests properly earlier. I see the failures in allura.tests.functional.test_home:TestProjectHome.test_user_search_for_disabled_user and allura.tests.functional.test_auth:TestAuth.test_disabled_user now.

I've added similar code back, but set the user to *anonymous instead of test-admin if the auth lookup can't find a valid user. I also added matching code to basetest_neighborhood_root.py