From 8fb4d65a57a87402bcef4cc401a4a52181300542 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Fri, 6 Oct 2023 15:43:02 -0500 Subject: [PATCH] Fix location for userinfo call; deprecate old name The `oauth2_userinfo` method was accidentally placed on the `AuthLoginClient` class in #849 rather than the `AuthClient` class. The mistake was made because it has a name which suggests that it is an OAuth2 login-related method, when that is not the case. In order to more clearly delineate that this is an API-client method, the `oauth2_` prefix is here stripped and deprecated. A compatibility shim method is included for now, but it emits a deprecation warning. The new name is `AuthClient.userinfo`. Data in `_testing` has also been shimmed temporarily. This is an arguably unnecessary step, but it is nearly trivial and saves us potential migration and rollout pain at the time of our next release. Because `_testing` has a weaker compatibility contract, the shim there can be removed after a single release. --- .../20231006_154216_sirosen_fix_userinfo.rst | 5 +++ .../_testing/data/auth/oauth2_userinfo.py | 29 ++----------- src/globus_sdk/_testing/data/auth/userinfo.py | 26 +++++++++++ .../services/auth/client/base_login_client.py | 34 --------------- .../services/auth/client/service_client.py | 43 +++++++++++++++++++ .../auth/base/test_oauth2_userinfo.py | 23 ---------- .../auth/service_client/test_userinfo.py | 36 ++++++++++++++++ 7 files changed, 114 insertions(+), 82 deletions(-) create mode 100644 changelog.d/20231006_154216_sirosen_fix_userinfo.rst create mode 100644 src/globus_sdk/_testing/data/auth/userinfo.py delete mode 100644 tests/functional/services/auth/base/test_oauth2_userinfo.py create mode 100644 tests/functional/services/auth/service_client/test_userinfo.py diff --git a/changelog.d/20231006_154216_sirosen_fix_userinfo.rst b/changelog.d/20231006_154216_sirosen_fix_userinfo.rst new file mode 100644 index 000000000..808ac0a27 --- /dev/null +++ b/changelog.d/20231006_154216_sirosen_fix_userinfo.rst @@ -0,0 +1,5 @@ +Deprecated +~~~~~~~~~~ + +- ``AuthClient.oauth2_userinfo`` method has been deprecated in favor of + ``AuthClient.userinfo``. Callers should prefer the new method name. (:pr:`NUMBER`) diff --git a/src/globus_sdk/_testing/data/auth/oauth2_userinfo.py b/src/globus_sdk/_testing/data/auth/oauth2_userinfo.py index 2a72fb918..5f7b2190b 100644 --- a/src/globus_sdk/_testing/data/auth/oauth2_userinfo.py +++ b/src/globus_sdk/_testing/data/auth/oauth2_userinfo.py @@ -1,26 +1,5 @@ -from globus_sdk._testing.models import RegisteredResponse, ResponseSet +# this is a clone of the userinfo.py data for compatibility across testing +# it should be removed in a future release +from .userinfo import RESPONSES -from ._common import FORBIDDEN_AUTH_RESPONSE, UNAUTHORIZED_AUTH_RESPONSE - -RESPONSES = ResponseSet( - unauthorized=RegisteredResponse( - service="auth", - path="/v2/oauth2/userinfo", - status=401, - json=UNAUTHORIZED_AUTH_RESPONSE.json, - metadata={ - "error_id": UNAUTHORIZED_AUTH_RESPONSE.error_id, - **UNAUTHORIZED_AUTH_RESPONSE.metadata_include, - }, - ), - forbidden=RegisteredResponse( - service="auth", - path="/v2/oauth2/userinfo", - status=403, - json=FORBIDDEN_AUTH_RESPONSE.json, - metadata={ - "error_id": FORBIDDEN_AUTH_RESPONSE.error_id, - **FORBIDDEN_AUTH_RESPONSE.metadata_include, - }, - ), -) +__all__ = ("RESPONSES",) diff --git a/src/globus_sdk/_testing/data/auth/userinfo.py b/src/globus_sdk/_testing/data/auth/userinfo.py new file mode 100644 index 000000000..2a72fb918 --- /dev/null +++ b/src/globus_sdk/_testing/data/auth/userinfo.py @@ -0,0 +1,26 @@ +from globus_sdk._testing.models import RegisteredResponse, ResponseSet + +from ._common import FORBIDDEN_AUTH_RESPONSE, UNAUTHORIZED_AUTH_RESPONSE + +RESPONSES = ResponseSet( + unauthorized=RegisteredResponse( + service="auth", + path="/v2/oauth2/userinfo", + status=401, + json=UNAUTHORIZED_AUTH_RESPONSE.json, + metadata={ + "error_id": UNAUTHORIZED_AUTH_RESPONSE.error_id, + **UNAUTHORIZED_AUTH_RESPONSE.metadata_include, + }, + ), + forbidden=RegisteredResponse( + service="auth", + path="/v2/oauth2/userinfo", + status=403, + json=FORBIDDEN_AUTH_RESPONSE.json, + metadata={ + "error_id": FORBIDDEN_AUTH_RESPONSE.error_id, + **FORBIDDEN_AUTH_RESPONSE.metadata_include, + }, + ), +) diff --git a/src/globus_sdk/services/auth/client/base_login_client.py b/src/globus_sdk/services/auth/client/base_login_client.py index aacb2d464..b1e04245d 100644 --- a/src/globus_sdk/services/auth/client/base_login_client.py +++ b/src/globus_sdk/services/auth/client/base_login_client.py @@ -449,37 +449,3 @@ def oauth2_token( encoding="form", ) ) - - def oauth2_userinfo(self) -> GlobusHTTPResponse: - """ - Call the Userinfo endpoint of Globus Auth. - Userinfo is specified as part of the OpenID Connect (OIDC) standard, - and Globus Auth's Userinfo is OIDC-compliant. - - The exact data returned will depend upon the set of OIDC-related scopes - which were used to acquire the token being used for this call. For - details, see the **API Info** below. - - .. tab-set:: - - .. tab-item:: Example Usage - - .. code-block:: python - - ac = AuthClient(...) - info = ac.oauth2_userinfo() - print( - 'Effective Identity "{info["sub"]}" has ' - f'Full Name "{info["name"]}" and ' - f'Email "{info["email"]}"' - ) - - .. tab-item:: API Info - - ``GET /v2/oauth2/userinfo`` - - .. extdoclink:: Get Userinfo - :ref: auth/reference/#get_or_post_v2_oauth2_userinfo_resource - """ - log.info("Looking up OIDC-style Userinfo from Globus Auth") - return self.get("/v2/oauth2/userinfo") diff --git a/src/globus_sdk/services/auth/client/service_client.py b/src/globus_sdk/services/auth/client/service_client.py index 5d7ff6358..62dafff0c 100644 --- a/src/globus_sdk/services/auth/client/service_client.py +++ b/src/globus_sdk/services/auth/client/service_client.py @@ -161,6 +161,49 @@ def get_jwk( ) return pem_decode_jwk_data(jwk_data=jwk_data) if as_pem else jwk_data + def userinfo(self) -> GlobusHTTPResponse: + """ + Call the Userinfo endpoint of Globus Auth. + Userinfo is specified as part of the OpenID Connect (OIDC) standard, + and Globus Auth's Userinfo is OIDC-compliant. + + The exact data returned will depend upon the set of OIDC-related scopes + which were used to acquire the token being used for this call. For + details, see the **API Info** below. + + .. tab-set:: + + .. tab-item:: Example Usage + + .. code-block:: python + + ac = AuthClient(...) + info = ac.oauth2_userinfo() + print( + 'Effective Identity "{info["sub"]}" has ' + f'Full Name "{info["name"]}" and ' + f'Email "{info["email"]}"' + ) + + .. tab-item:: API Info + + ``GET /v2/oauth2/userinfo`` + + .. extdoclink:: Get Userinfo + :ref: auth/reference/#get_or_post_v2_oauth2_userinfo_resource + """ + log.info("Looking up OIDC-style Userinfo from Globus Auth") + return self.get("/v2/oauth2/userinfo") + + def oauth2_userinfo(self) -> GlobusHTTPResponse: + """ + A deprecated alias for ``userinfo``. + """ + exc.warn_deprecated( + "The method `oauth2_userinfo` is deprecated. Use `userinfo` instead." + ) + return self.userinfo() + def get_identities( self, *, diff --git a/tests/functional/services/auth/base/test_oauth2_userinfo.py b/tests/functional/services/auth/base/test_oauth2_userinfo.py deleted file mode 100644 index 22f36eaf5..000000000 --- a/tests/functional/services/auth/base/test_oauth2_userinfo.py +++ /dev/null @@ -1,23 +0,0 @@ -import pytest - -import globus_sdk -from globus_sdk._testing import load_response - - -# TODO: add data for the success case and test it -@pytest.mark.xfail -def test_oauth2_userinfo(): - raise NotImplementedError - - -@pytest.mark.parametrize("casename", ("unauthorized", "forbidden")) -def test_oauth2_error_handling(login_client, casename): - meta = load_response(login_client.oauth2_userinfo, case=casename).metadata - - with pytest.raises(globus_sdk.AuthAPIError) as excinfo: - login_client.oauth2_userinfo() - - err = excinfo.value - assert err.http_status == meta["http_status"] - assert err.code == meta["code"] - assert err.request_id == meta["error_id"] diff --git a/tests/functional/services/auth/service_client/test_userinfo.py b/tests/functional/services/auth/service_client/test_userinfo.py new file mode 100644 index 000000000..3ecd1dee7 --- /dev/null +++ b/tests/functional/services/auth/service_client/test_userinfo.py @@ -0,0 +1,36 @@ +import pytest + +import globus_sdk +from globus_sdk._testing import load_response + + +# TODO: add data for the success case and test it +@pytest.mark.xfail +def test_userinfo(): + raise NotImplementedError + + +@pytest.mark.parametrize("casename", ("unauthorized", "forbidden")) +def test_userinfo_error_handling(service_client, casename): + meta = load_response(service_client.oauth2_userinfo, case=casename).metadata + + with pytest.raises(globus_sdk.AuthAPIError) as excinfo: + service_client.userinfo() + + err = excinfo.value + assert err.http_status == meta["http_status"] + assert err.code == meta["code"] + assert err.request_id == meta["error_id"] + + +def test_oauth2_userinfo_warns(service_client): + # TODO: + # if the above success case is added, this test can be changed to use it + # that would let us get rid of the try-except guard below + load_response(service_client.oauth2_userinfo, case="unauthorized") + + with pytest.warns(globus_sdk.RemovedInV4Warning, match="Use `userinfo` instead."): + try: + service_client.oauth2_userinfo() + except globus_sdk.AuthAPIError: + pass