opendev/storyboard
Code Issues Proposed changes

OAuth check for response_type 'code'

Browse Source 
This patch adds sanity checking for response_type 'code'. It does
this by switching errors within the OAuth controller to use
exception handling rather than explicit response creation, so that we
can more easily raise errors encountered during our validation process.

Change-Id: I4f3821f8f63f4102def95e407c59c81571c0bb55
This commit is contained in:
Michael Krotscheck 2015-02-04 14:07:49 -08:00
parent bd51492142
commit b3239ba248
7 changed files with 266 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
requirements.txt
View File

@ -24,3 +24,4 @@ eventlet>=0.13.0
stevedore>=1.0.0
python-crontab>=1.8.1
tzlocal>=1.1.2
rfc3987>=1.3.4

35
storyboard/api/auth/openid_client.py
View File

@ -4,7 +4,7 @@
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
@ -13,20 +13,34 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import requests
from oslo.config import cfg
from oslo_log import log
import requests
import six
import storyboard.common.exception as exc
from storyboard.api.auth import utils
LOG = log.getLogger(__name__)
CONF = cfg.CONF
class OpenIdClient(object):
def send_openid_redirect(self, request, response):
# Extract needed parameters
redirect_uri = request.params.get("redirect_uri")
response_type = request.params.get("response_type")
# Sanity Check: response_type
if response_type != 'code':
raise exc.UnsupportedResponseType(redirect_uri=redirect_uri,
message='response_type must '
'be \'code\'')
redirect_location = CONF.oauth.openid_url
response.status_code = 303
@ -34,16 +48,16 @@ class OpenIdClient(object):
"scope": six.text_type(request.params.get("scope")),
"state": six.text_type(request.params.get("state")),
"client_id": six.text_type(request.params.get("client_id")),
"response_type": six.text_type(request.params.get(
"response_type")),
"sb_redirect_uri": six.text_type(request.params.get(
"redirect_uri"))
"response_type": six.text_type(response_type),
"sb_redirect_uri": six.text_type(redirect_uri)
}
#TODO(krotscheck): URI base should be fully inferred from the request.
# TODO(krotscheck): URI base should be fully inferred from the request.
# assuming that the API is hosted at /api isn't good.
return_to_url = request.host_url + "/api/v1/openid/authorize_return?" \
+ utils.join_params(return_params, encode=True)
return_to_url = "%s/api/v1/openid/authorize_return?%s" % (
request.host_url,
utils.join_params(return_params, encode=True)
)
response.status_code = 303
@ -111,4 +125,5 @@ class OpenIdClient(object):
return data_dict["assoc_handle"]
client = OpenIdClient()

7
storyboard/api/v1/auth.py
View File

@ -4,7 +4,7 @@
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
@ -24,24 +24,28 @@ import six
from storyboard.api.auth.oauth_validator import SERVER
from storyboard.api.auth.openid_client import client as openid_client
from storyboard.common import decorators
from storyboard.db.api import auth as auth_api
LOG = log.getLogger(__name__)
class AuthController(rest.RestController):
_custom_actions = {
"authorize": ["GET"],
"authorize_return": ["GET"],
"token": ["POST"],
}
@decorators.oauth_exceptions
@pecan.expose()
def authorize(self):
"""Authorization code request."""
return openid_client.send_openid_redirect(request, response)
@decorators.oauth_exceptions
@pecan.expose()
def authorize_return(self):
"""Authorization code redirect endpoint.
@ -117,6 +121,7 @@ class AuthController(rest.RestController):
return response
@decorators.oauth_exceptions
@pecan.expose()
def token(self):
"""Token endpoint."""

49
storyboard/common/decorators.py
View File

@ -4,7 +4,7 @@
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
@ -13,11 +13,18 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import functools
import json
from urllib import urlencode
from urlparse import parse_qsl
import functools
from pecan import abort
from pecan import redirect
from pecan import response
import rfc3987
from storyboard.common import exception as exc
from storyboard.openstack.common.gettextutils import _ # noqa
def db_exceptions(func):
@ -27,4 +34,42 @@ def db_exceptions(func):
return func(self, *args, **kwargs)
except exc.DBException as db_exc:
abort(db_exc.code, db_exc.message)
return decorate
def oauth_exceptions(func):
@functools.wraps(func)
def decorate(self, *args, **kwargs):
try:
return func(self, *args, **kwargs)
except exc.OAuthException as o_exc:
# Extract the parameters
error = o_exc.error
error_description = o_exc.message or _("No details available.")
# If we have a redirect URL, build the error redirect.
if o_exc.redirect_uri:
# Split the redirect_url apart
parts = rfc3987.parse(o_exc.redirect_uri, 'URI')
# Add the error and error_description
params = parse_qsl(parts['query']) if parts['query'] else []
params.append(('error', error))
params.append(('error_description', error_description))
# Overwrite the old query params and reconstruct the URL
parts['query'] = urlencode(params)
location = rfc3987.compose(**parts)
redirect(location)
else:
error_body = {
'error': error,
'error_description': error_description
}
response.body = json.dumps(error_body)
abort(o_exc.code, error_description)
return decorate

79
storyboard/common/exception.py
View File

@ -13,10 +13,16 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import httplib
from oslo_log import log
import rfc3987
from wsme.exc import ClientSideError
from storyboard.openstack.common.gettextutils import _ # noqa
LOG = log.getLogger(__name__)
class StoryboardException(ClientSideError):
"""Base Exception for the project
@ -233,3 +239,76 @@ class DBInvalidSortKey(DBException):
"""
message = _("Invalid sort field")
class OAuthException(ClientSideError):
"""Base Exception for our OAuth response handling. All of the error codes
used for this exception type are defined in the OAuth 2.0 Specification.
https://tools.ietf.org/html/rfc6749#section-5.2
To use, extend and override the 'error' field. Specific error description
messages need to be added when the exception is raised.
"""
error = "server_error"
redirect_uri = None
def __init__(self, message=None, redirect_uri=None):
if not redirect_uri:
code = httplib.BAD_REQUEST
else:
try:
parts = rfc3987.parse(redirect_uri, 'URI')
if parts['scheme'] not in ['http', 'https']:
raise ValueError('Invalid scheme')
self.redirect_uri = redirect_uri
code = httplib.SEE_OTHER
except ValueError:
LOG.warning('Provided redirect_uri is invalid: %s'
% (redirect_uri,))
code = httplib.BAD_REQUEST
super(OAuthException, self) \
.__init__(msg=message, status_code=code)
class InvalidRequest(OAuthException):
error = "invalid_request"
class AccessDenied(OAuthException):
error = "access_denied"
class UnsupportedResponseType(OAuthException):
error = "unsupported_response_type"
class InvalidScope(OAuthException):
error = "invalid_scope"
class InvalidClient(OAuthException):
error = "invalid_client"
class UnauthorizedClient(OAuthException):
error = "unauthorized_client"
class InvalidGrant(OAuthException):
error = "invalid_grant"
class UnsupportedGrantType(OAuthException):
error = "unsupported_grant_type"
class ServerError(OAuthException):
error = "server_error"
class TemporarilyUnavailable(OAuthException):
error = "temporarily_unavailable"

26
storyboard/tests/api/auth/test_oauth.py
View File

@ -65,7 +65,7 @@ class BaseOAuthTest(base.FunctionalTest):
for key, value in six.iteritems(kwargs):
self.assertIn(key, parameters)
self.assertIsNotNone(parameters[key])
self.assertEqual(value, parameters[key])
self.assertEqual(value, parameters[key][0])
class TestOAuthAuthorize(BaseOAuthTest):
@ -83,7 +83,7 @@ class TestOAuthAuthorize(BaseOAuthTest):
valid_params = {
'response_type': 'code',
'client_id': 'storyboard.openstack.org',
'redirect_uri': 'https://storyboard.openstack.org/!#/auth/token',
'redirect_uri': 'https://storyboard.openstack.org/#!/auth/token',
'scope': 'user'
}
@ -128,6 +128,28 @@ class TestOAuthAuthorize(BaseOAuthTest):
self.assertEqual(self.valid_params['redirect_uri'],
redirect_params['sb_redirect_uri'][0])
def test_authorize_invalid_response_type(self):
"""Assert that an invalid response_type redirects back to the
redirect_uri and provides the expected error response.
"""
invalid_params = self.valid_params.copy()
invalid_params['response_type'] = 'invalid_code'
# Simple GET with invalid code parameters
random_state = six.text_type(uuid.uuid4())
response = self.get_json(path='/openid/authorize',
expect_errors=True,
state=random_state,
**invalid_params)
# Validate the error response
self.assertValidRedirect(response=response,
expected_status_code=302,
redirect_uri=invalid_params['redirect_uri'],
error='unsupported_response_type',
error_description='response_type must be '
'\'code\'')
class TestOAuthAccessToken(BaseOAuthTest):
"""Functional test for the /oauth/token endpoint for the generation of

84
storyboard/tests/common/test_exception.py Normal file
View File

@ -0,0 +1,84 @@
# Copyright (c) 2015 Hewlett-Packard Development Company, L.P.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
import httplib
from oslo.config import cfg
import storyboard.common.exception as exc
from storyboard.tests import base
CONF = cfg.CONF
class OAuthExceptionTest(base.FunctionalTest):
"""Test for our OAuth Exceptions."""
def test_error_names(self):
"""Assert that our various exceptions have the correct error code."""
self.assertEqual('invalid_request',
exc.InvalidRequest.error)
self.assertEqual('access_denied',
exc.AccessDenied.error)
self.assertEqual('unsupported_response_type',
exc.UnsupportedResponseType.error)
self.assertEqual('invalid_scope',
exc.InvalidScope.error)
self.assertEqual('invalid_client',
exc.InvalidClient.error)
self.assertEqual('unauthorized_client',
exc.UnauthorizedClient.error)
self.assertEqual('invalid_grant',
exc.InvalidGrant.error)
self.assertEqual('unsupported_grant_type',
exc.UnsupportedGrantType.error)
self.assertEqual('server_error',
exc.ServerError.error)
self.assertEqual('temporarily_unavailable',
exc.TemporarilyUnavailable.error)
def test_redirect_uri_parsing(self):
"""Assert that the exception can automatically detect whether it can
redirect.
"""
invalid_uris = [
None,
'not_a_uri',
'example.com/without/scheme',
'/relative/uri',
'gopher://example.com/not/http/scheme'
]
for uri in invalid_uris:
e = exc.OAuthException(redirect_uri=uri)
self.assertIsNone(e.redirect_uri)
self.assertEqual(httplib.BAD_REQUEST, e.code)
valid_uris = [
'http://example.com',
'https://example.com',
'https://example.com/',
'https://example.com/foo/bar',
'https://example.com/foo?uri=param',
'https://example.com/foo?uri=param#fragment',
'https://example.com/foo#fragment?fragment=param',
'https://example.com/foo?uri=param#fragment?fragment=param',
'https://example.com:1214/foo/bar'
]
for uri in valid_uris:
e = exc.OAuthException(redirect_uri=uri)
self.assertEqual(uri, e.redirect_uri)
self.assertEqual(httplib.SEE_OTHER, e.code)