oidc: deprecate grant_type argument

The OpenID Connect grant type is something implicit to our plugins,
so there is no need to pass it as a keyword argument, we can define
this as a class argument. Moreover, it makes no sense to pass a
grant_type that is different to the grant type that is being
implemented. This change deprecates the keyword argument (showing a
warning to the users if they're passing it) and checking if the
grant_type is different to the one defined by the plugin itself.

Change-Id: I4fffc2c651487d3240bbc8458ad728b06b0ffa64

keystoneauth1/exceptions/oidc.py

@@ -16,7 +16,7 @@ from keystoneauth1.exceptions import auth_plugins
 __all__ = (
     'InvalidDiscoveryEndpoint', 'InvalidOidcDiscoveryDocument',
     'OidcAccessTokenEndpointNotFound', 'OidcAuthorizationEndpointNotFound',
-    'OidcPluginNotSupported',
+    'OidcGrantTypeMissmatch', 'OidcPluginNotSupported',
 )
 
 
@@ -36,5 +36,9 @@ class OidcAuthorizationEndpointNotFound(auth_plugins.AuthPluginException):
     message = "OpenID Connect authorization endpoint not provided."
 
 
+class OidcGrantTypeMissmatch(auth_plugins.AuthPluginException):
+    message = "Missmatch between OpenID Connect plugin and grant_type argument"
+
+
 class OidcPluginNotSupported(auth_plugins.AuthPluginException):
     message = "OpenID Connect grant type not supported by provider."

keystoneauth1/identity/v3/oidc.py

@@ -10,6 +10,8 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import warnings
+
 from positional import positional
 
 from keystoneauth1 import _utils as utils
@@ -31,12 +33,16 @@ class _OidcBase(federation.FederationBaseAuth):
     ``http://openid.net/specs/openid-connect-core-1_0.html``
     """
 
+    grant_type = None
+
     def __init__(self, auth_url, identity_provider, protocol,
-                 client_id, client_secret, grant_type,
+                 client_id, client_secret,
                  access_token_type,
                  scope="openid profile",
                  access_token_endpoint=None,
-                 discovery_endpoint=None, **kwargs):
+                 discovery_endpoint=None,
+                 grant_type=None,
+                 **kwargs):
         """The OpenID Connect plugin expects the following.
 
         :param auth_url: URL of the Identity Service
@@ -55,14 +61,6 @@ class _OidcBase(federation.FederationBaseAuth):
         :param client_secret: OAuth 2.0 Client Secret
         :type client_secret: string
 
-        :param grant_type: OpenID Connect grant type, it represents the flow
-                           that is used to talk to the OP. Valid values are:
-                           "authorization_code", "refresh_token", or
-                           "password". If a discovery document is being used,
-                           this class will check if the provided value is
-                           supported by the provider.
-        :type grant_type: string
-
         :param access_token_type: OAuth 2.0 Authorization Server Introspection
                                   token type, it is used to decide which type
                                   of token will be used when processing token
@@ -101,7 +99,18 @@ class _OidcBase(federation.FederationBaseAuth):
         self.access_token_type = access_token_type
         self.scope = scope
 
-        self.grant_type = grant_type
+        if grant_type is not None:
+            if grant_type != self.grant_type:
+                raise exceptions.OidcGrantTypeMissmatch()
+            warnings.warn("Passing grant_type as an argument has been "
+                          "deprecated as it is now defined in the plugin "
+                          "itself. You should stop passing this argument "
+                          "to the plugin, as it will be ignored, since you "
+                          "cannot pass a free text string as a grant_type. "
+                          "This argument will be dropped from the plugin in "
+                          "July 2017 or with the next major release of "
+                          "keystoneauth (3.0.0)",
+                          DeprecationWarning)
 
     def _get_discovery_document(self, session):
         """Get the contents of the OpenID Connect Discovery Document.
@@ -231,13 +240,16 @@ class _OidcBase(federation.FederationBaseAuth):
 class OidcPassword(_OidcBase):
     """Implementation for OpenID Connect Resource Owner Password Credential."""
 
+    grant_type = "password"
+
     @positional(4)
     def __init__(self, auth_url, identity_provider, protocol,
                  client_id, client_secret,
                  access_token_endpoint=None,
                  discovery_endpoint=None,
-                 grant_type='password', access_token_type='access_token',
+                 access_token_type='access_token',
-                 username=None, password=None, **kwargs):
+                 username=None, password=None,
+                 **kwargs):
         """The OpenID Password plugin expects the following.
 
         :param username: Username used to authenticate
@@ -254,7 +266,6 @@ class OidcPassword(_OidcBase):
             client_secret=client_secret,
             access_token_endpoint=access_token_endpoint,
             discovery_endpoint=discovery_endpoint,
-            grant_type=grant_type,
             access_token_type=access_token_type,
             **kwargs)
         self.username = username
@@ -297,12 +308,13 @@ class OidcPassword(_OidcBase):
 class OidcAuthorizationCode(_OidcBase):
     """Implementation for OpenID Connect Authorization Code."""
 
+    grant_type = 'authorization_code'
+
     @positional(4)
     def __init__(self, auth_url, identity_provider, protocol,
                  client_id, client_secret,
                  access_token_endpoint=None,
                  discovery_endpoint=None,
-                 grant_type='authorization_code',
                  access_token_type='access_token',
                  redirect_uri=None, code=None, **kwargs):
         """The OpenID Authorization Code plugin expects the following.
@@ -322,7 +334,6 @@ class OidcAuthorizationCode(_OidcBase):
             client_secret=client_secret,
             access_token_endpoint=access_token_endpoint,
             discovery_endpoint=discovery_endpoint,
-            grant_type=grant_type,
             access_token_type=access_token_type,
             **kwargs)
         self.redirect_uri = redirect_uri
@@ -390,7 +401,6 @@ class OidcAccessToken(_OidcBase):
                                               client_id=None,
                                               client_secret=None,
                                               access_token_endpoint=None,
-                                              grant_type=None,
                                               access_token_type=None,
                                               **kwargs)
         self.access_token = access_token

keystoneauth1/tests/unit/identity/test_identity_v3_oidc.py

@@ -11,6 +11,7 @@
 #    under the License.
 
 import uuid
+import warnings
 
 from six.moves import urllib
 
@@ -48,6 +49,33 @@ class BaseOIDCTests(object):
 
         self.DISCOVERY_URL = ('https://localhost:8020/oidc/.well-known/'
                               'openid-configuration')
+        self.GRANT_TYPE = None
+
+    def test_grant_type_and_plugin_missmatch(self):
+        self.assertRaises(
+            exceptions.OidcGrantTypeMissmatch,
+            self.plugin.__class__,
+            self.AUTH_URL,
+            self.IDENTITY_PROVIDER,
+            self.PROTOCOL,
+            client_id=self.CLIENT_ID,
+            client_secret=self.CLIENT_SECRET,
+            grant_type=uuid.uuid4().hex
+        )
+
+    def test_can_pass_grant_type_but_warning_is_issued(self):
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter("always")
+            self.plugin.__class__(
+                self.AUTH_URL,
+                self.IDENTITY_PROVIDER,
+                self.PROTOCOL,
+                client_id=self.CLIENT_ID,
+                client_secret=self.CLIENT_SECRET,
+                grant_type=self.GRANT_TYPE)
+            assert len(w) == 1
+            assert issubclass(w[-1].category, DeprecationWarning)
+            assert "grant_type" in str(w[-1].message)
 
     def test_discovery_not_found(self):
         self.requests_mock.get("http://not.found",
@@ -156,6 +184,8 @@ class OIDCPasswordTests(BaseOIDCTests, utils.TestCase):
     def setUp(self):
         super(OIDCPasswordTests, self).setUp()
 
+        self.GRANT_TYPE = 'password'
+
         self.plugin = oidc.OidcPassword(
             self.AUTH_URL,
             self.IDENTITY_PROVIDER,
@@ -228,6 +258,8 @@ class OIDCAuthorizationGrantTests(BaseOIDCTests, utils.TestCase):
     def setUp(self):
         super(OIDCAuthorizationGrantTests, self).setUp()
 
+        self.GRANT_TYPE = 'authorization_code'
+
         self.plugin = oidc.OidcAuthorizationCode(
             self.AUTH_URL,
             self.IDENTITY_PROVIDER,