Page MenuHomePhabricator

Unable to apply for Toolforge access
Closed, ResolvedPublic

Description

From -cloud, per request:

[11:06:39] hi taavi - there was an issue yesterday with a person trying to apply for toolforge membership, repeatedly receiving Error 500. Logstash suggests something to do with LdapUser.
[11:07:08] https://logstash.wikimedia.org/goto/744406da3b68c65df3edf23320324782
[11:07:45] I assume the flood of keystone ERR from cloud.*-dev machines are not related
[11:10:44] <taavi>	 hauskater: hi, can you file a task with all of the details please? I'm a bit busy at the moment, but will look later today

@Bsadowski1 is trying to apply for Toolforge access at https://toolsadmin.wikimedia.org/tools/membership/apply but he's repeatedly getting Error 500 since yesterday. His LDAP account exist and looks OK to me via ldapsearch and ldap.toolforge.

Event Timeline

taavi triaged this task as High priority.Sep 28 2023, 9:06 PM
taavi added a project: Bitu.

This seems to affect all new developer accounts created via the IDM.

Traceback (most recent call last):
  File "/opt/lib/python/site-packages/django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File "/opt/lib/python/site-packages/django/core/handlers/base.py", line 115, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/opt/lib/python/site-packages/django/core/handlers/base.py", line 113, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/opt/lib/python/site-packages/django/views/decorators/debug.py", line 76, in sensitive_post_parameters_wrapper
    return view(request, *args, **kwargs)
  File "/opt/lib/python/site-packages/django/views/decorators/cache.py", line 44, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "/srv/app/striker/labsauth/views.py", line 52, in login
    authentication_form=forms.LabsAuthenticationForm)
  File "/opt/lib/python/site-packages/django/views/decorators/debug.py", line 76, in sensitive_post_parameters_wrapper
    return view(request, *args, **kwargs)
  File "/opt/lib/python/site-packages/django/utils/decorators.py", line 142, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/opt/lib/python/site-packages/django/views/decorators/cache.py", line 44, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "/opt/lib/python/site-packages/ratelimitbackend/views.py", line 42, in login
    auth_login(request, form.get_user())
  File "/opt/lib/python/site-packages/django/contrib/auth/__init__.py", line 132, in login
    user_logged_in.send(sender=user.__class__, request=request, user=user)
  File "/opt/lib/python/site-packages/django/dispatch/dispatcher.py", line 175, in send
    for receiver in self._live_receivers(sender)
  File "/opt/lib/python/site-packages/django/dispatch/dispatcher.py", line 175, in <listcomp>
    for receiver in self._live_receivers(sender)
  File "/srv/app/striker/goals/signals/handlers.py", line 76, in on_user_login
    check_goal_ssh(user.ldapuser)
  File "/srv/app/striker/labsauth/models.py", line 168, in ldapuser
    return LdapUser.objects.get(dn=self.ldap_dn)
  File "/opt/lib/python/site-packages/django/db/models/manager.py", line 82, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/opt/lib/python/site-packages/django/db/models/query.py", line 408, in get
    self.model._meta.object_name
striker.labsauth.models.LdapUser.DoesNotExist: LdapUser matching query does not exist.

If it helps, I spotted as well https://logstash.wikimedia.org/goto/3133502b2a636d527d47b5ed691cf467 (Not Acceptable: /register/api/username/Bsadowski1) although I can't find an error like that for today.

Combined search with the reqIds provided by the user for the last few days: https://logstash.wikimedia.org/goto/55be3e1ff666773ef030fac60149ded5

I suspect this has to do with the object classes that users are in. My main account is in:

objectClass: inetOrgPerson
objectClass: person
objectClass: ldapPublicKey
objectClass: posixAccount
objectClass: shadowAccount
objectClass: wikimediaPerson

and a freshly created account has:

objectClass: inetOrgPerson
objectClass: posixAccount
objectClass: ldapPublicKey

And Striker has this in the code:

# striker/labsauth/models.py
class LdapUser(ldapdb.models.Model):
    """Equivalent of OpenStackNovaUser"""
    base_dn = settings.LABSAUTH_USER_BASE
    object_classes = [
        'person',
        'inetOrgPerson',
        'organizationalPerson',
        'ldapPublicKey',
        'posixAccount',
        'shadowAccount',
    ]

I believe that's the filter Striker uses via the ldapdb package (and the list of object classes for users it creates). It matches my main account but not a fresh account:

taavi@cloudweb1004 ~ $ ldapsearch -x '(&(uid=taavi)(objectClass=person)(objectClass=inetOrgPerson)(objectClass=organizationalPerson)(objectClass=ldapPublicKey)(objectClass=posixAccount)(objectClass=shadowAccount))'
# numEntries: 1
taavi@cloudweb1004 ~ $ ldapsearch -x '(&(uid=taavi-test-20230928-01)(objectClass=person)(objectClass=inetOrgPerson)(objectClass=organizationalPerson)(objectClass=ldapPublicKey)(objectClass=posixAccount)(objectClass=shadowAccount))'
:# (nothing)

@MoritzMuehlenhoff @SLyngshede-WMF Should Bitu create users with more object classes, or should we remove some unnecessary ones from the Striker list?

If it helps, I spotted as well https://logstash.wikimedia.org/goto/3133502b2a636d527d47b5ed691cf467 (Not Acceptable: /register/api/username/Bsadowski1) although I can't find an error like that for today.

If you check out the relevant source code you'll find that this is expected for already taken usernames.

@MoritzMuehlenhoff @SLyngshede-WMF Should Bitu create users with more object classes, or should we remove some unnecessary ones from the Striker list?

Bitu currently only applies the object classes which are also set/managed and the generic editing of user attributes isn't implemented yet. Looking at the list of OCs:

  • organizationalPerson is a superset of "person" and the additional attributes in there are not needed or totally obsolete (telexNumber, internationaliSDNNumber, facsimileTelephoneNumber), I think Striker should drop it
  • person seems useful for the "description", I think Bitu should add it
  • shadowAccount is what likely what Toolforge bails on/expects? We could add it to an test account and see if that helps? It wouldn't hurt to add it in the general case if needed/used by Toolforge
  • shadowAccount is what likely what Toolforge bails on/expects? We could add it to an test account and see if that helps? It wouldn't hurt to add it in the general case if needed/used by Toolforge

This is indeed it, adding it to my testing account made it able to log in to Toolforge. Although I don't immediately see a need for any of its attributes so we might want to drop it from the Striker filter too.

@MoritzMuehlenhoff @SLyngshede-WMF Should Bitu create users with more object classes, or should we remove some unnecessary ones from the Striker list?

  • person seems useful for the "description", I think Bitu should add it

@SLyngshede-WMF What do you think, let's add it in Bitu?

This is indeed it, adding it to my testing account made it able to log in to Toolforge. Although I don't immediately see a need for any of its attributes so we might want to drop it from the Striker filter too.

Agreed, I think Striker can just drop it.

Change 963671 had a related patch set uploaded (by Majavah; author: Majavah):

[labs/striker@master] Relax LDAP objectClass requirements

https://gerrit.wikimedia.org/r/963671

We can just add it as an auxiliary class. I just tested and Bitu has a bug where it gets confused if new schemas are added during an edit. I'll just create a patch for that.

It's the "shadowAccount" class you want to add, and not a "person" class correct? Because I can't seem to find an AUX class called "person".

Okay... So I was missing the core.schema in my test setup, that contains "person". The description field is also available for shadowAccount, inetOrgPerson and a bunch of other objects classes. We don't need to load the person objectClass to get the "description" field, it's already available via the "inetOrgPerson" and "posixAccount".

Okay... So I was missing the core.schema in my test setup, that contains "person". The description field is also available for shadowAccount, inetOrgPerson and a bunch of other objects classes. We don't need to load the person objectClass to get the "description" field, it's already available via the "inetOrgPerson" and "posixAccount".

Ack, ok. That makes sense.

Change 963671 merged by jenkins-bot:

[labs/striker@master] Relax LDAP objectClass requirements

https://gerrit.wikimedia.org/r/963671

Change 963782 had a related patch set uploaded (by Majavah; author: Majavah):

[operations/puppet@production] hieradata: bump striker docker image

https://gerrit.wikimedia.org/r/963782

Change 963782 merged by Majavah:

[operations/puppet@production] hieradata: bump striker docker image

https://gerrit.wikimedia.org/r/963782