smart-on-fhir / client-py

Python SMART on FHIR client
http://docs.smarthealthit.org
Other
574 stars 209 forks source link

Fix reauthorization to work beyond the first time called #128

Closed timharsch closed 1 year ago

timharsch commented 2 years ago

See also PR #127 and issue #112

This PR fixes a situation where the refresh_token is not properly address on calls beyond the first.

timharsch commented 1 year ago

@ducu Can this PR be merged please?

ducu commented 1 year ago

Hey @timharsch, no it cannot be merged like this. Please align your changes with the upstream master first, here's the conflict I see:

<<<<<<< HEAD
        # The refresh token issued by the authorization server. If present, the
        # app should discard any previous refresh_token associated with this
        # launch, replacing it with this new value.
        refresh_token = ret_params.get('refresh_token')
        if refresh_token is not None:
            self.refresh_token = refresh_token
            del ret_params['refresh_token']

        logger.debug("SMART AUTH: Received access token: {0}, refresh token: {1}"
=======
        self.refresh_token = ret_params.get('refresh_token') or params.get('refresh_token')
        if self.refresh_token is not None and 'refresh_token' in ret_params:
            del ret_params['refresh_token']

        logging.debug("SMART AUTH: Received access token: {0}, refresh token: {1}"
>>>>>>> c4f02e92a255ed09e2711f80e3b1d0ddd525589a
timharsch commented 1 year ago

Thanks for your reply.

I don't understand where you are seeing a conflict. The PR would state if there were a conflict and not allow a merge. Currently it is just blocked waiting on your review, once you review the PR it should allow you to merge.

If you click on this link you can see a comparison of master and my branch. There are no conflicts.. it's just a few lines of change. https://github.com/smart-on-fhir/client-py/compare/master...Unite-Genomics:client-py:unite-112

If you examine the git history of my branch you will see it is just the two commits on top of the head of the current master of this repository. https://github.com/Unite-Genomics/client-py/commits/unite-112

timharsch commented 1 year ago

Hi @ducu I've now done some fairly extensive testing with the latest commit. I think it is safe to merge now. Can you merge it please?