Merge pull request #3 from Peter-Slump/improved-code-climate

Fixed some code smells

docs/index.rst

@@ -58,6 +58,8 @@ The OpenID Connect entry point can be retrieved from the realm object.
                                         client_secret='very-secret-client-secret')
 
 
+.. automethod:: keycloak.openid_connect.KeycloakOpenidConnect.decode_token
+
 .. automethod:: keycloak.openid_connect.KeycloakOpenidConnect.authorization_url
 
 .. automethod:: keycloak.openid_connect.KeycloakOpenidConnect.authorization_code

src/keycloak/admin/roles.py

@@ -5,6 +5,17 @@
 from keycloak.admin import KeycloakAdminBase
 
 
+ROLE_KWARGS = ['description', 'id', 'client_role', 'composite', 'composites',
+               'container_id', 'scope_param_required']
+
+
+def to_camel_case(snake_cased_str):
+    components = snake_cased_str.split('_')
+    # We capitalize the first letter of each component except the first one
+    # with the 'title' method and join them together.
+    return components[0] + ''.join(map(str.capitalize, components[1:]))
+
+
 class Roles(KeycloakAdminBase):
 
     _client_id = None
@@ -22,31 +33,26 @@ def by_name(self, role_name):
         return Role(realm_name=self._realm_name, client_id=self._client_id,
                     role_name=role_name, client=self._client)
 
-    def create(self, name, description=None, id=None, client_role=None,
-               composite=None, composites=None, container_id=None,
-               scope_param_required=None):
+    def create(self, name, **kwargs):
+        """
+        Create new role
+
+        http://www.keycloak.org/docs-api/3.4/rest-api/index.html#_roles_resource
+
+        :param str name: Name for the role
+        :param str description: (optional)
+        :param str id: (optional)
+        :param bool client_role: (optional)
+        :param bool composite: (optional)
+        :param object composites: (optional)
+        :param str container_id: (optional)
+        :param bool scope_param_required: (optional)
+        """
         payload = OrderedDict(name=name)
 
-        if description is not None:
-            payload['description'] = description
-
-        if id is not None:
-            payload['id'] = id
-
-        if client_role is not None:
-            payload['clientRole'] = client_role
-
-        if composite is not None:
-            payload['composite'] = composite
-
-        if composites is not None:
-            payload['composites'] = composites
-
-        if container_id is not None:
-            payload['containerId'] = container_id
-
-        if scope_param_required is not None:
-            payload['scopeParamRequired'] = scope_param_required
+        for key in ROLE_KWARGS:
+            if key in kwargs:
+                payload[to_camel_case(key)] = kwargs[key]
 
         return self._client.post(
             url=self._client.get_full_url(
@@ -71,31 +77,26 @@ def __init__(self, realm_name, client_id, role_name, *args, **kwargs):
 
         super(Role, self).__init__(*args, **kwargs)
 
-    def update(self, name, description=None, id=None, client_role=None,
-               composite=None, composites=None, container_id=None,
-               scope_param_required=None):
+    def update(self, name, **kwargs):
+        """
+        Update existing role.
+
+        http://www.keycloak.org/docs-api/3.4/rest-api/index.html#_roles_resource
+
+        :param str name: Name for the role
+        :param str description: (optional)
+        :param str id: (optional)
+        :param bool client_role: (optional)
+        :param bool composite: (optional)
+        :param object composites: (optional)
+        :param str container_id: (optional)
+        :param bool scope_param_required: (optional)
+        """
         payload = OrderedDict(name=name)
 
-        if description is not None:
-            payload['description'] = description
-
-        if id is not None:
-            payload['id'] = id
-
-        if client_role is not None:
-            payload['clientRole'] = client_role
-
-        if composite is not None:
-            payload['composite'] = composite
-
-        if composites is not None:
-            payload['composites'] = composites
-
-        if container_id is not None:
-            payload['containerId'] = container_id
-
-        if scope_param_required is not None:
-            payload['scopeParamRequired'] = scope_param_required
+        for key in ROLE_KWARGS:
+            if key in kwargs:
+                payload[to_camel_case(key)] = kwargs[key]
 
         return self._client.put(
             url=self._client.get_full_url(

src/keycloak/admin/users.py

@@ -17,36 +17,35 @@ def __init__(self, realm_name, *args, **kwargs):
         self._realm_name = realm_name
         super(Users, self).__init__(*args, **kwargs)
 
-    def create(self, username, credentials=None, first_name=None,
-               last_name=None, email=None, enabled=None):
+    def create(self, username, **kwargs):
         """
         Create a user in Keycloak
 
         http://www.keycloak.org/docs-api/3.4/rest-api/index.html#_users_resource
 
         :param str username:
-        :param object credentials:
-        :param str first_name:
-        :param str last_name:
-        :param str email:
-        :param boolean enabled:
+        :param object credentials: (optional)
+        :param str first_name: (optional)
+        :param str last_name: (optional)
+        :param str email: (optional)
+        :param boolean enabled: (optional)
         """
         payload = OrderedDict(username=username)
 
-        if credentials is not None:
-            payload['credentials'] = [credentials]
+        if 'credentials' in kwargs:
+            payload['credentials'] = [kwargs['credentials']]
 
-        if first_name is not None:
-            payload['firstName'] = first_name
+        if 'first_name' in kwargs:
+            payload['firstName'] = kwargs['first_name']
 
-        if last_name is not None:
-            payload['lastName'] = last_name
+        if 'last_name' in kwargs:
+            payload['lastName'] = kwargs['last_name']
 
-        if email is not None:
-            payload['email'] = email
+        if 'email' in kwargs:
+            payload['email'] = kwargs['email']
 
-        if enabled is not None:
-            payload['enabled'] = enabled
+        if 'enabled' in kwargs:
+            payload['enabled'] = kwargs['enabled']
 
         self._client.post(
             url=self._client.get_full_url(

src/keycloak/openid_connect.py

@@ -46,15 +46,55 @@ def decode_token(self, token, key, algorithms=None, **kwargs):
 
         https://tools.ietf.org/html/rfc7517
 
-        :param str token:
-        :param str key:
-        :param list | None algorithms: RS256 will be used by default.
-        :return:
+        :param str token: A signed JWS to be verified.
+        :param str key: A key to attempt to verify the payload with.
+        :param str,list algorithms: (optional) Valid algorithms that should be
+            used to verify the JWS. Defaults to `['RS256']`
+        :param str audience: (optional) The intended audience of the token. If
+            the "aud" claim is included in the claim set, then the audience
+            must be included and must equal the provided claim.
+        :param str,iterable issuer: (optional) Acceptable value(s) for the
+            issuer of the token. If the "iss" claim is included in the claim
+            set, then the issuer must be given and the claim in the token must
+            be among the acceptable values.
+        :param str subject: (optional) The subject of the token. If the "sub"
+            claim is included in the claim set, then the subject must be
+            included and must equal the provided claim.
+        :param str access_token: (optional) An access token returned alongside
+            the id_token during the authorization grant flow. If the "at_hash"
+            claim is included in the claim set, then the access_token must be
+            included, and it must match the "at_hash" claim.
+        :param dict options: (optional) A dictionary of options for skipping
+            validation steps.
+            defaults:
+
+             .. code-block:: python
+
+                 {
+                    'verify_signature': True,
+                    'verify_aud': True,
+                    'verify_iat': True,
+                    'verify_exp': True,
+                    'verify_nbf': True,
+                    'verify_iss': True,
+                    'verify_sub': True,
+                    'verify_jti': True,
+                    'leeway': 0,
+                }
+
+        :return: The dict representation of the claims set, assuming the
+            signature is valid and all requested data validation passes.
+        :rtype: dict
+        :raises jose.exceptions.JWTError: If the signature is invalid in any
+            way.
+        :raises jose.exceptions.ExpiredSignatureError: If the signature has
+            expired.
+        :raises jose.exceptions.JWTClaimsError: If any claim is invalid in any
+            way.
         """
-        algorithms = algorithms or ['RS256']
-
-        return jwt.decode(token, key, algorithms=algorithms,
-                          audience=self._client_id, **kwargs)
+        return jwt.decode(token, key,
+                          audience=kwargs.get('audience') or self._client_id,
+                          algorithms=algorithms or ['RS256'], **kwargs)
 
     def logout(self, refresh_token):
         """
@@ -103,82 +143,74 @@ def userinfo(self, token):
                                           )
                                       })
 
-    def authorization_url(self, response_type='code', redirect_uri=None,
-                          scope=None, state=None):
+    def authorization_url(self, **kwargs):
         """
         Get authorization URL to redirect the resource owner to.
 
         https://tools.ietf.org/html/rfc6749#section-4.1.1
 
-        :param str response_type:
-        :param str redirect_uri:
-        :param str scope:
-        :param str state:
+        :param str redirect_uri: (optional) Absolute URL of the client where
+            the user-agent will be redirected to.
+        :param str scope: (optional) Space delimited list of strings.
+        :param str state: (optional) An opaque value used by the client to
+            maintain state between the request and callback
         :return: URL to redirect the resource owner to
         :rtype: str
         """
         payload = OrderedDict()
-        payload['response_type'] = response_type
+        payload['response_type'] = 'code'
         payload['client_id'] = self._client_id
 
-        if redirect_uri:
-            payload['redirect_uri'] = redirect_uri
-
-        if scope:
-            payload['scope'] = scope
-
-        if state:
-            payload['state'] = state
+        for key in sorted(kwargs.keys()):
+            # Add items in a sorted way for unittest purposes.
+            payload[key] = kwargs[key]
 
         params = urlencode(payload)
         url = self.get_url('authorization_endpoint')
 
         return '{}?{}'.format(url, params)
 
-    def authorization_code(self, code, redirect_uri,
-                           grant_type='authorization_code'):
+    def authorization_code(self, code, redirect_uri):
         """
         Retrieve access token by `authorization_code` grant.
 
         https://tools.ietf.org/html/rfc6749#section-4.1.3
 
-        :param str code:
-        :param str redirect_uri:
-        :param str grant_type:
+        :param str code: The authorization code received from the authorization
+            server.
+        :param str redirect_uri: the identical value of the "redirect_uri"
+            parameter in the authorization request.
         :rtype: dict
+        :return: Access token response
         """
-        return self._token_request(grant_type=grant_type, code=code,
+        return self._token_request(grant_type='authorization_code', code=code,
                                    redirect_uri=redirect_uri)
 
-    def client_credentials(self, scope=None, grant_type='client_credentials'):
+    def client_credentials(self, **kwargs):
         """
         Retrieve access token by `client_credentials` grant.
 
-        :param str | None scope:
-        :param str grant_type:
+        https://tools.ietf.org/html/rfc6749#section-4.4
+
+        :param str scope: (optional) Space delimited list of strings.
         :rtype: dict
+        :return: Access token response
         """
-        return self._token_request(grant_type=grant_type, scope=scope)
+        return self._token_request(grant_type='client_credentials', **kwargs)
 
-    def refresh_token(self, refresh_token, grant_type='refresh_token',
-                      scope=None):
+    def refresh_token(self, refresh_token, **kwargs):
         """
         Refresh an access token
 
         https://tools.ietf.org/html/rfc6749#section-6
 
         :param str refresh_token:
-        :param str grant_type:
-        :param str \ None scope:
+        :param str scope: (optional) Space delimited list of strings.
         :rtype: dict
+        :return: Access token response
         """
-        if scope:
-            return self._token_request(grant_type=grant_type,
-                                       refresh_token=refresh_token,
-                                       scope=scope)
-        else:
-            return self._token_request(grant_type=grant_type,
-                                       refresh_token=refresh_token)
+        return self._token_request(grant_type='refresh_token',
+                                   refresh_token=refresh_token, **kwargs)
 
     def _token_request(self, grant_type, **kwargs):
         """