Flask Dance not automatically refreshing access token when using the Strava Blueprint

[Vlarabor]

Im using the Strava OAuth Blueprint in my project.

After granting access through the OAuth Dance I can issue requests to the Strava API as expected (except from having to use full URLs in the strava.get() call, as relative URLs send requests to https://www.strava.com/ and not https://www.strava.com/api/v3, see #336 ).

However, calling the Strava API after the access token has expired, raises a TokenExpiredError in oauthlib/oauth2/rfc6749/clients/base.py in line 198:

# from oauthlib/oauth2/rfc6749/clients/base.py

if self._expires_at and self._expires_at < time.time():
    raise TokenExpiredError()

Checking the strava Proxy, the strava._client has correctly loaded both the access and refresh token (matching the tokens saved in my database).

Can i check if Flask Dance tries to refresh the token after it has expired? Could it be that the wrong Strava API endpoint is used for refreshing the token even if the make_strava_blueprint function has the correct refresh_url set (similar to how the relative URLs do not work for querying the API even though the correct base_url is set)?

[Vlarabor]

So i had some time to try and get some more debugging info. As it seems, Flask Dance detects that the currently stored access token is expired and tries and request a new token. This is the logging output i get (i replaced my actual client id with "my_client_id")

[2021-08-15 12:49:17,419] DEBUG in oauth2_session: Auto refresh is set, attempting to refresh at https://www.strava.com/api/v3/oauth/token.
[2021-08-15 12:49:17,419] DEBUG in oauth2_session: Encoding client_id "my_client_id" with client_secret as Basic auth credentials.
[2021-08-15 12:49:17,419] DEBUG in oauth2_session: Adding auto refresh key word arguments {}.
[2021-08-15 12:49:17,419] DEBUG in oauth2_session: Prepared refresh token request body grant_type=refresh_token&scope=read%2Cactivity%3Aread&refresh_token="my_refresh_token"&allow_redirects=True
[2021-08-15 12:49:17,420] DEBUG in oauth2_session: Requesting url https://www.strava.com/api/v3/oauth/token using method POST.
[2021-08-15 12:49:17,420] DEBUG in oauth2_session: Supplying headers {'Accept': 'application/json', 'Content-Type': 'application/x-www-form-urlencoded;charset=UTF-8'} and data {'grant_type': 'refresh_token', 'scope': 'read,activity:read', 'refresh_token': 'my_refresh_token', 'allow_redirects': 'True'}
[2021-08-15 12:49:17,420] DEBUG in oauth2_session: Passing through key word arguments {'json': None, 'auth': <requests.auth.HTTPBasicAuth object at 0x113604310>, 'timeout': None, 'verify': True, 'proxies': None}.
[2021-08-15 12:49:17,445] DEBUG in connectionpool: Starting new HTTPS connection (1): www.strava.com:443
[2021-08-15 12:49:17,926] DEBUG in connectionpool: https://www.strava.com:443 "POST /api/v3/oauth/token HTTP/1.1" 400 None
[2021-08-15 12:49:17,927] DEBUG in oauth2_session: Request to refresh token completed with status 400.
[2021-08-15 12:49:17,927] DEBUG in oauth2_session: Response headers were {'Date': 'Sun, 15 Aug 2021 10:49:17 GMT', 'Content-Type': 'application/json; charset=utf-8', 'Transfer-Encoding': 'chunked', 'Connection': 'keep-alive', 'Via': '1.1 linkerd, 1.1 linkerd', 'Vary': 'Origin', 'Server': 'nginx/1.19.5', 'Status': '400 Bad Request', 'X-Request-Id': '04b4a888-8ea8-41f2-bf02-0abca528b610', 'Cache-Control': 'no-cache', 'Referrer-Policy': 'strict-origin-when-cross-origin', 'X-FRAME-OPTIONS': 'DENY', 'content-encoding': 'gzip', 'X-XSS-Protection': '1; mode=block', 'X-Download-Options': 'noopen', 'X-Content-Type-Options': 'nosniff', 'X-Permitted-Cross-Domain-Policies': 'none'} and content {"message":"Bad Request","errors":[{"resource":"Application","field":"client_id","code":"invalid"}]}.

It seems like the client_id and client_secret are not included as data on the refresh request which is however required according to the Strava documentation (https://developers.strava.com/docs/authentication/#refreshing-expired-access-tokens)

I tried setting additional auto_refresh_kwargs with the following code when i create my strava blueprint:

blueprint = make_strava_blueprint(scope=scope, storage=SQLAlchemyStorage(OAuth, db.session, user=current_user))

blueprint.auto_refresh_kwargs = {
    "client_id": blueprint.client_id,
    "client_secret": blueprint.client_secret
}

However, Flask Dance still seems to not provide both client_id and client_secret in the request as I still get the same logging output:

[2021-08-15 13:07:34,567] DEBUG in oauth2_session: Auto refresh is set, attempting to refresh at https://www.strava.com/api/v3/oauth/token.
[2021-08-15 13:07:34,567] DEBUG in oauth2_session: Encoding client_id "my_client_id" with client_secret as Basic auth credentials.
[2021-08-15 13:07:34,567] DEBUG in oauth2_session: Adding auto refresh key word arguments {}.
[2021-08-15 13:07:34,567] DEBUG in oauth2_session: Prepared refresh token request body grant_type=refresh_token&scope=read%2Cactivity%3Aread&refresh_token="my_refresh_token"&allow_redirects=True
[2021-08-15 13:07:34,568] DEBUG in oauth2_session: Requesting url https://www.strava.com/api/v3/oauth/token using method POST.
[2021-08-15 13:07:34,568] DEBUG in oauth2_session: Supplying headers {'Accept': 'application/json', 'Content-Type': 'application/x-www-form-urlencoded;charset=UTF-8'} and data {'grant_type': 'refresh_token', 'scope': 'read,activity:read', 'refresh_token': 'my_refresh_token', 'allow_redirects': 'True'}
[2021-08-15 13:07:34,568] DEBUG in oauth2_session: Passing through key word arguments {'json': None, 'auth': <requests.auth.HTTPBasicAuth object at 0x1133a60a0>, 'timeout': None, 'verify': True, 'proxies': None}.
[2021-08-15 13:07:34,592] DEBUG in connectionpool: Starting new HTTPS connection (1): www.strava.com:443
[2021-08-15 13:07:35,055] DEBUG in connectionpool: https://www.strava.com:443 "POST /api/v3/oauth/token HTTP/1.1" 400 None
[2021-08-15 13:07:35,056] DEBUG in oauth2_session: Request to refresh token completed with status 400.
[2021-08-15 13:07:35,056] DEBUG in oauth2_session: Response headers were {'Date': 'Sun, 15 Aug 2021 11:07:35 GMT', 'Content-Type': 'application/json; charset=utf-8', 'Transfer-Encoding': 'chunked', 'Connection': 'keep-alive', 'Via': '1.1 linkerd, 1.1 linkerd', 'Vary': 'Origin', 'Server': 'nginx/1.19.5', 'Status': '400 Bad Request', 'X-Request-Id': '6d93eccb-e4cc-455c-b65d-749d6041105e', 'Cache-Control': 'no-cache', 'Referrer-Policy': 'strict-origin-when-cross-origin', 'X-FRAME-OPTIONS': 'DENY', 'content-encoding': 'gzip', 'X-XSS-Protection': '1; mode=block', 'X-Download-Options': 'noopen', 'X-Content-Type-Options': 'nosniff', 'X-Permitted-Cross-Domain-Policies': 'none'} and content {"message":"Bad Request","errors":[{"resource":"Application","field":"client_id","code":"invalid"}]}.

Am I adding the auto_refresh_kwargs correctly? I hope my debugging information is helpful and I'm happy to share any additional information if it is required.

[daenney]

Mmm, I'm surprised this doesn't work. The other blueprint that uses this feature is Twitch, but there we set it up in the blueprint itself

flask-dance/flask_dance/contrib/twitch.py

Line 98 in d616de9

twitch_bp.auto_refresh_kwargs = {

.

It doesn't feel like this should make a difference, but I'm wondering if you could try patching strava.py locally to reflect what we do for Twitch and see what happens?

[Vlarabor]

I just added the changes to reflect the twitch_bp by adding the following lines to strava.py which unfortunately did not solve the problem.

strava_bp.auto_refresh_url = strava_bp.token_url
    strava_bp.auto_refresh_kwargs = {
        "client_id": strava_bp.client_id,
        "client_secret": strava_bp.client_secret
    }

I then tried setting the auto_refresh_kwargs in the Constructor of strava.py directly like so:

    strava_bp = OAuth2ConsumerBlueprint(
        "strava",
        __name__,
        client_id=client_id,
        client_secret=client_secret,
        scope=scope,
        base_url="https://www.strava.com/api/v3",
        authorization_url="https://www.strava.com/api/v3/oauth/authorize",
        token_url="https://www.strava.com/api/v3/oauth/token",
        auto_refresh_url="https://www.strava.com/api/v3/oauth/token",
        auto_refresh_kwargs={
            "client_id": client_id,
            "client_secret": client_secret
        },
        redirect_url=redirect_url,
        redirect_to=redirect_to,
        login_url=login_url,
        authorized_url=authorized_url,
        session_class=session_class or StravaOAuth2Session,
        storage=storage,
        rule_kwargs=rule_kwargs,
    )

and specifically passing the client_id and client_secret in my make_strava_blueprint call.

blueprint = make_strava_blueprint(client_id=os.environ.get("STRAVA_OAUTH_CLIENT_ID"),
                                  client_secret=os.environ.get("STRAVA_OAUTH_CLIENT_SECRET"),
                                  scope=scope,
                                  storage=SQLAlchemyStorage(OAuth,
                                                            db.session,
                                                            user=current_user))

With this I receive a new access token, however now I get an Integrity Error when flask_dance/consumer/base.py tries to store the new token in my SQLAlchemyStorage. My OAuth models follows the multiple provider example from the Quickstart section. The storage.set() method does not check for provider_user_id and provider_user_login so None is passed as a value for both in the Insert query violating the nullable=False condition for both columns. I would get both values from the existing entry before deleting it, and passing it to the new entry that is created. Does this sound reasonable?

[Vlarabor]

I'm not sure if I should open a second issue for this, but this is my current workaround solution for the IntegrityErrorcaused by the the self.storage.set(self, _token) call in line 156 of flask_dance/consumer/base.py. I modified the set() method in flask_dance.consumer.storage.sqla.py to check for the provider_user_id and provider_user_login attributes:

    def set(self, blueprint, token, user=None, user_id=None):
        uid = first([user_id, self.user_id, blueprint.config.get("user_id")])
        u = first(
            _get_real_user(ref, self.anon_user)
            for ref in (user, self.user, blueprint.config.get("user"))
        )

        if self.user_required and not u and not uid:
            raise ValueError("Cannot set OAuth token without an associated user")

        # if there was an existing model, delete it
        existing_query = self.session.query(self.model).filter_by(
            provider=blueprint.name
        )
        # check for user ID
        has_user_id = hasattr(self.model, "user_id")
        if has_user_id and uid:
            existing_query = existing_query.filter_by(user_id=uid)
        # check for user (relationship property)
        has_user = hasattr(self.model, "user")
        if has_user and u:
            existing_query = existing_query.filter_by(user=u)
        existing_entry = existing_query.first()
        # check for provider user id
        has_provider_user_id = hasattr(self.model, "provider_user_id")
        # check for provider user login
        has_provider_user_login = hasattr(self.model, "provider_user_login")
        # queue up delete query -- won't be run until commit()
        existing_query.delete()
        # create a new model for this token
        kwargs = {"provider": blueprint.name, "token": token}
        if has_user_id and uid:
            kwargs["user_id"] = uid
        if has_user and u:
            kwargs["user"] = u
        if existing_entry and has_provider_user_login:
            kwargs["provider_user_login"] = existing_entry.provider_user_login
        if existing_entry and has_provider_user_id:
            kwargs["provider_user_id"] = existing_entry.provider_user_id

        self.session.add(self.model(**kwargs))
        # commit to delete and add simultaneously
        self.session.commit()
        # invalidate cache
        self.cache.delete(
            self.make_cache_key(blueprint=blueprint, user=user, user_id=user_id)
        )

I was wondering, if updating an existing entry (if it exists) would be a better solution instead of deleting the old entry and adding a new one? Also adding any non-nullable fields in the OAuth model would cause the same problem as with provider_user_id and provider_user_login. This workaround also does not cover the case, when there is no existing entry (but should this even get called without an existing entry?).