matrix-org/synapse

Password provider does not work properly since Synapse v1.19.0 due to "user_get_threepids" is now async

Closed this issue ยท 7 comments

After upgrading to Synapse v1.19.0 the following exception is thrown within a password provider :

...for threepid in threepids:
TypeError: 'coroutine' object is not iterable

The affected piece of code of the password provider looks like this:

@defer.inlineCallbacks
def check_3pid_auth(self, medium, address, password):

   store = yield self.account_handler._hs.get_profile_handler().store

   threepids = yield store.user_get_threepids(user_id)
   for threepid in threepids:
      ...

This is obviously due to "user_get_threepids" is defined async since Synapse v1.19.0, see in "registration.py" # 526,

async def user_get_threepids(self, user_id):

So my question is:
How to request the user's threepids (asynchronously) within a password provider?

I don't have intimate knowledge of this area of the code but I can offer some suggestions.

You may need to do

yield ensureDeferred(store.user_get_threepids(user_id))

which converts the coroutine to a Deferred. (My understanding, which may be wrong, is that yield will only deal with Deferreds, whilst other values (including coroutines) will get passed straight through as though there was no yield).

Docs for ensureDeferred.

Or, ideally, if/when possible, switch from

@defer.inlineCallbacks
def check_3pid_auth(self, medium, address, password):

to

async def check_3pid_auth(self, medium, address, password):

(and use await instead of yield in this function)

You may need other code changes to make this work.

I also tried switching from

@defer.inlineCallbacks
def check_3pid_auth(self, medium, address, password):

to
async def check_3pid_auth(self, medium, address, password):
in use await store.user_get_threepids(user_id) . This, however, throws an exception.

In this case you would need to switch all yields to awaits and you might have to end up touching other functions in your code as well. I only suggested it as this is the modern, clean style going forward (inlineCallbacks might even be getting deprecated in recognition of it being a relic from days gone by). If what you have works, I don't think there is need to worry, though :)

I'll take a look, but I suspect this is because private properties are being used which aren't expected to be exposed as part of the password provider API.

I'm going to close this since password providers are really only expected to use the public methods on the config and ModuleApi passed into the constructor.

There's not really anything to stop you from doing that in Python, but converting the code to be async is the right solution here IMO! ๐Ÿ‘

If there is additional functionality that the ModuleApi class could offer, please file a separate issue with the rationale of why that should be public. It will help us not break your code in the future! ๐Ÿ˜„

I'd also be curious if your password provider is open source / what functionality it provides as I was recently looking for examples of these. ๐Ÿ˜„

It looks like #7734 might be related to what you'd like?

@menturion heads up there's another modified version of that password provider here: https://github.com/ma1uta/matrix-synapse-rest-password-provider

which may or may not be useful to you :)

@menturion Ah, no, that repo may not be aware of this yet either. I'll create an issue to warn them and point to your above comment with the required changes.