OAuth Client resource calls fail if the token is expired

siddweiker commented 5 years ago

Re-using a resource will cause the first call on an expired token to fail, instead of refreshing the token. Creating a new resource for every call will automatically refresh the token as expected so as a workaround, one can simply create the resource for every call.

Steps to reproduce:

Create OAuth client
Set token access lifetime to 10 seconds in ServiceNOW
Create a resource for a table
Do a get() call, wait for longer than 10 seconds then do another get call
The call will fail with oauthlib.oauth2.rfc6749.errors.MissingTokenError: (missing_token) Missing access token parameter.

Using:

pysnow==0.7.4
requests==2.19.1
requests-oauthlib==1.0.0

Code Example

Setup an oauth client per the example in the docs

incident_resource = s.resource(api_path='/table/incident')
record = incident_resource.get(query={}).first()
time.sleep(15)
# This will fail because the token is expired
record = incident_resource.get(query={}).first()
time.sleep(15)
record = s.resource(api_path='/table/incident').get(query={}).first()
time.sleep(15)
# This works fine and the token will be refreshed
record = s.resource(api_path='/table/incident').get(query={}).first()

Exception

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "\lib\site-packages\pysnow\resource.py", line 104, in get
    return self._request.get(query, limit, offset, fields, stream)
  File "\lib\site-packages\pysnow\request.py", line 75, in get
    return self._get_response('GET', stream=stream)
  File "\lib\site-packages\pysnow\request.py", line 44, in _get_response
    response = self._session.request(method, self._url, stream=use_stream, params=params, **kwargs)
  File "\lib\site-packages\requests_oauthlib\oauth2_session.py", line 343, in request
    self.auto_refresh_url, auth=auth, **kwargs
  File "\lib\site-packages\requests_oauthlib\oauth2_session.py", line 309, in refresh_token
    self.token = self._client.parse_request_body_response(r.text, scope=self.scope)
  File "\lib\site-packages\oauthlib\oauth2\rfc6749\clients\base.py", line 411, in parse_request_body_response
    self.token = parse_token_response(body, scope=scope)
  File "\lib\site-packages\oauthlib\oauth2\rfc6749\parameters.py", line 379, in parse_token_response
    validate_token_parameters(params)
  File "\lib\site-packages\oauthlib\oauth2\rfc6749\parameters.py", line 389, in validate_token_parameters
    raise MissingTokenError(description="Missing access token parameter.")
oauthlib.oauth2.rfc6749.errors.MissingTokenError: (missing_token) Missing access token parameter.

rbw commented 5 years ago

Hey,

Are you sure you're using pysnow 0.7.4? Not explicitly passing stream=True to get() when using first() should fail with:

pysnow.exceptions.InvalidUsage: first() is only available when stream=True

Ensure you're using the latest version using pysnow.__version__, because I'm unable to reproduce this in 0.7.4 with stream=True to Resource.get()

And yes, I realize this hasn't been updated in the docs - sorry about that. I'll have that fixed asap.

rbw commented 5 years ago

Also, I'd like to explain why this change was introduced (buffer interface by default). I think most users are used to / expecting a buffer interface - some might read the documentation and find the stream parameter and use it in situations where it makes sense, i.e. when working with potentially large data sets.

I'm not sure if the buffer interface is the user-expected default, it's more of my assumptions expressed in the interface design I suppose. What's your opinion?

rbw commented 5 years ago

Updated docs with clarifications in 0.7.5

rbw / pysnow