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

Created on 18 Aug 2020  路  11Comments  路  Source: matrix-org/synapse

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, https://github.com/matrix-org/synapse/blob/3234d5c30551f1ea5c6a51621cdb6b237765fe0c/synapse/storage/databases/main/registration.py#L526

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

bug

All 11 comments

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.

@reivilibre

Many thanks for your reply!

According to your suggestion I replaced ...
threepids = yield store.user_get_threepids(user_id)
by
threepids = yield defer.ensureDeferred(store.user_get_threepids(user_id))

... and it works. The iterable object (i.e. threepids) is returned.

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.

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.

@reivilibre

Tried the second solution again. Finally I got the "cleaner style" working.

In detail for all who are going to have the same issue with existing password providers:

-1-
Declared the password provider method async by replacing (in my case)

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

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

-2-
Replaced
store = yield self.account_handler._hs.get_profile_handler().store
by
store = self.account_handler._hs.get_profile_handler().store
.. i.e. removed yield

-3-
Switched all remaining yields to awaits.

-4-
Replaced
defer.returnValue(user_id)
by
return user_id

-5-
Optionally, removed from twisted.internet import defer due to not needed anymore.

Thanks @reivilibre

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?

@clokep

Many thanks for your reply and the additional information.

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. 馃槃

Yes, the password provider is a modified version of the Rest Auth Provider you can find here ...
https://github.com/kamax-matrix/matrix-synapse-rest-password-provider/blob/master/rest_auth_provider.py

@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 :)

@anoadragon453

Many thanks.

Did you already test whether this one works properly with Synapse v1.19.0?

In my opinon it will run into the same issue reported here due to yield store.user_get_threepids(user_id) in line # 122
https://github.com/ma1uta/matrix-synapse-rest-password-provider/blob/c782c84aeab1872e73b6c29aadb99d3852e26bbd/rest_auth_provider.py#L122
will not return the iterable object (i.e. the threepids).

@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.

Was this page helpful?
0 / 5 - 0 ratings