Skip to content

Conversation

@arithmetic1728
Copy link
Contributor

@arithmetic1728 arithmetic1728 commented May 31, 2023

BEGIN_COMMIT_OVERRIDE
fix: fix "AttributeError: 'str' object has no attribute 'get'"
docs: replacing abc.com with example.com
END_COMMIT_OVERRIDE

Fix reauth logic so it can handle token response where error response is a string instead of dict. Similar issue see #857

@arithmetic1728 arithmetic1728 merged commit dac7cc3 into main Jun 1, 2023
@arithmetic1728 arithmetic1728 deleted the fixes branch June 1, 2023 18:43
"client_secret",
scopes=["foo", "bar"],
rapt_token="rapt_token",
enable_reauth_refresh=True,
Copy link

@glabdulhadi glabdulhadi Jun 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in continuation from above comment.
In other words, i would like to understand what should be the expected behaviour of the method for the below test case?

with mock.patch("google.oauth2._client._token_endpoint_request_no_throw") as mock_token_request:
mock_token_request.return_value = (False, _REAUTH_NEEDED_ERROR, False)
with pytest.raises(exceptions.RefreshError) as excinfo:
reauth.refresh_grant(
MOCK_REQUEST,
"token_uri",
"refresh_token",
"client_id",
"client_secret",
scopes=["foo", "bar"],
rapt_token="rapt_token",
enable_reauth_refresh=True,
)

whether RefreshError exception is expected or whether reauth refresh flow (i.e., 354-364) is expected since enable_reauth_refresh=True and reponse_data=_REAUTH_NEEDED_ERROR?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clundin25 , please let me know your thoughts on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe in case of reauth needed error, the oauth2 token endpoint always returns a json, in other words, something like mock_token_request.return_value = (False, {"error":_REAUTH_NEEDED_ERROR, ...}, False). The server won't return mock_token_request.return_value = (False, _REAUTH_NEEDED_ERROR, False) as described in the test case you mentioned.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. then agree.

@clundin25
Copy link
Contributor

@arithmetic1728 perhaps enable_reauth_refresh=True should just be an early return in the function?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants