Skip to content

Support OAuth endpoints which don't return expires_in

Support OAuth endpoints which don't return expires_in field

Fixes #297 (closed)

Previously when using the OAuthAuthenticator with an OAuth endpoint which didn't include the expires_in field in the response you'd see a KeyError, e.g.

File "/.../site-packages/singer_sdk/authenticators.py", line 432, in update_access_token
    self.expires_in = token_json["expires_in"]
KeyError: 'expires_in'

The expires_in field is RECOMMENDED but not REQUIRED1 - so this is a use-case we want to handle. To solve the issue we add a default_expiration argument to OAuthAuthenticator, which will be used as a fallback if the OAuth endpoint does not include an expires_in with its response. This change also comes with a slightly more subtle side-effect, now if no default_expiration is set, and the OAuth response has no expires_in then the authenticator will assume the token never expires (see the second condition in is_token_valid()).

Questions

Firstly, I haven't written much Python in my life - I'm very open to feedback on code style, testing approach, naming convention, etc., - but here are the thing I think I'd like advice on most:

  • I'm not too familiar with the python testing ecosystem, does introducing requests-mock here feel useful or is there something already used in this project which we could use to test the OAauthAuthenticator behaviour I'm interested in here?
  • The test setup feels clunky, any advice appreciated on making improvements here
  • Does testing the authenticator behaviour at this level make sense? I notice we don't seem to have many direct tests for authenticator behaviour in general
  • Does treating no expires_in and no default_expiration as "token never expires" seem ok? Or would we prefer raise some kind of error?
Edited by Daniel Ferguson

Merge request reports