oidc: fix OpenID scope management
The OpenID scope is something common to all the OpenID grant types, therefore we move the OIDC scope parameter 'scope' from the OidcPassword class into the base _OidcBase class, moving the option as well into the corresponding loader. Moreover, OpenID scopes are not handled properly, as the loaders have the option defined as "openid-scope" whereas the class constructor argument is named "openid". Lastly, OpenID states that the OpenID scope MUST contain "openid" at least, so we should include this in our defaults argument. Closes-Bug: #1594272 Closes-Bug: #1597334 Change-Id: I9a242ae93a61737d032c19830c5d89ef6237f875
This commit is contained in:
parent
bd18bc3d0d
commit
68a7962488
|
@ -29,7 +29,8 @@ class _OidcBase(federation.FederationBaseAuth):
|
|||
|
||||
def __init__(self, auth_url, identity_provider, protocol,
|
||||
client_id, client_secret, access_token_endpoint,
|
||||
grant_type, access_token_type, **kwargs):
|
||||
grant_type, access_token_type,
|
||||
scope="openid profile", **kwargs):
|
||||
"""The OpenID Connect plugin expects the following.
|
||||
|
||||
:param auth_url: URL of the Identity Service
|
||||
|
@ -66,6 +67,11 @@ class _OidcBase(federation.FederationBaseAuth):
|
|||
"access_token" or "id_token"
|
||||
:type access_token_type: string
|
||||
|
||||
:param scope: OpenID Connect scope that is requested from OP,
|
||||
for example: "openid profile email", defaults to
|
||||
"openid profile". Note that OpenID Connect specification
|
||||
states that "openid" must be always specified.
|
||||
:type scope: string
|
||||
"""
|
||||
super(_OidcBase, self).__init__(auth_url, identity_provider, protocol,
|
||||
**kwargs)
|
||||
|
@ -74,6 +80,7 @@ class _OidcBase(federation.FederationBaseAuth):
|
|||
self.access_token_endpoint = access_token_endpoint
|
||||
self.grant_type = grant_type
|
||||
self.access_token_type = access_token_type
|
||||
self.scope = scope
|
||||
|
||||
def _get_access_token(self, session, payload):
|
||||
"""Exchange a variety of user supplied values for an access token.
|
||||
|
@ -127,7 +134,7 @@ class OidcPassword(_OidcBase):
|
|||
def __init__(self, auth_url, identity_provider, protocol,
|
||||
client_id, client_secret, access_token_endpoint,
|
||||
grant_type='password', access_token_type='access_token',
|
||||
username=None, password=None, scope='profile', **kwargs):
|
||||
username=None, password=None, **kwargs):
|
||||
"""The OpenID Password plugin expects the following.
|
||||
|
||||
:param username: Username used to authenticate
|
||||
|
@ -135,11 +142,6 @@ class OidcPassword(_OidcBase):
|
|||
|
||||
:param password: Password used to authenticate
|
||||
:type password: string
|
||||
|
||||
:param scope: OpenID Connect scope that is requested from OP,
|
||||
defaults to "profile", for example: "profile email"
|
||||
:type scope: string
|
||||
|
||||
"""
|
||||
super(OidcPassword, self).__init__(
|
||||
auth_url=auth_url,
|
||||
|
@ -153,7 +155,6 @@ class OidcPassword(_OidcBase):
|
|||
**kwargs)
|
||||
self.username = username
|
||||
self.password = password
|
||||
self.scope = scope
|
||||
|
||||
def get_unscoped_auth_ref(self, session):
|
||||
"""Authenticate with OpenID Connect and get back claims.
|
||||
|
|
|
@ -85,6 +85,12 @@ class _OpenIDConnectBase(loading.BaseFederationLoader):
|
|||
loading.Opt('client-id', help='OAuth 2.0 Client ID'),
|
||||
loading.Opt('client-secret', secret=True,
|
||||
help='OAuth 2.0 Client Secret'),
|
||||
loading.Opt('openid-scope', default="openid profile",
|
||||
dest="scope",
|
||||
help='OpenID Connect scope that is requested from '
|
||||
'authorization server. Note that the OpenID '
|
||||
'Connect specification states that "openid" '
|
||||
'must be always specified.'),
|
||||
loading.Opt('access-token-endpoint',
|
||||
help='OpenID Connect Provider Token Endpoint'),
|
||||
loading.Opt('access-token-type',
|
||||
|
@ -110,9 +116,6 @@ class OpenIDConnectPassword(_OpenIDConnectBase):
|
|||
options.extend([
|
||||
loading.Opt('username', help='Username'),
|
||||
loading.Opt('password', secret=True, help='Password'),
|
||||
loading.Opt('openid-scope', default="profile",
|
||||
dest='scope',
|
||||
help='OpenID Connect scope that is requested from OP')
|
||||
])
|
||||
|
||||
return options
|
||||
|
|
|
@ -135,9 +135,11 @@ class OpenIDConnectBaseTests(object):
|
|||
options = loading.get_plugin_loader(self.plugin_name).get_options()
|
||||
self.assertTrue(
|
||||
set(['client-id', 'client-secret', 'access-token-endpoint',
|
||||
'access-token-type']).issubset(
|
||||
'access-token-type', 'openid-scope']).issubset(
|
||||
set([o.name for o in options]))
|
||||
)
|
||||
# openid-scope gets renamed into "scope"
|
||||
self.assertIn('scope', [o.dest for o in options])
|
||||
|
||||
|
||||
class OpenIDConnectPasswordTests(OpenIDConnectBaseTests, utils.TestCase):
|
||||
|
@ -150,8 +152,6 @@ class OpenIDConnectPasswordTests(OpenIDConnectBaseTests, utils.TestCase):
|
|||
set(['username', 'password', 'openid-scope']).issubset(
|
||||
set([o.name for o in options]))
|
||||
)
|
||||
# openid-scope gets renamed into "scope"
|
||||
self.assertIn('scope', [o.dest for o in options])
|
||||
|
||||
def test_basic(self):
|
||||
access_token_endpoint = uuid.uuid4().hex
|
||||
|
@ -160,6 +160,7 @@ class OpenIDConnectPasswordTests(OpenIDConnectBaseTests, utils.TestCase):
|
|||
scope = uuid.uuid4().hex
|
||||
identity_provider = uuid.uuid4().hex
|
||||
protocol = uuid.uuid4().hex
|
||||
scope = uuid.uuid4().hex
|
||||
client_id = uuid.uuid4().hex
|
||||
client_secret = uuid.uuid4().hex
|
||||
|
||||
|
@ -174,6 +175,7 @@ class OpenIDConnectPasswordTests(OpenIDConnectBaseTests, utils.TestCase):
|
|||
|
||||
self.assertEqual(username, oidc.username)
|
||||
self.assertEqual(password, oidc.password)
|
||||
self.assertEqual(scope, oidc.scope)
|
||||
self.assertEqual(identity_provider, oidc.identity_provider)
|
||||
self.assertEqual(protocol, oidc.protocol)
|
||||
self.assertEqual(access_token_endpoint, oidc.access_token_endpoint)
|
||||
|
@ -196,6 +198,7 @@ class OpenIDConnectAuthCodeTests(OpenIDConnectBaseTests, utils.TestCase):
|
|||
access_token_endpoint = uuid.uuid4().hex
|
||||
redirect_uri = uuid.uuid4().hex
|
||||
authorization_code = uuid.uuid4().hex
|
||||
scope = uuid.uuid4().hex
|
||||
identity_provider = uuid.uuid4().hex
|
||||
protocol = uuid.uuid4().hex
|
||||
client_id = uuid.uuid4().hex
|
||||
|
@ -207,10 +210,12 @@ class OpenIDConnectAuthCodeTests(OpenIDConnectBaseTests, utils.TestCase):
|
|||
protocol=protocol,
|
||||
access_token_endpoint=access_token_endpoint,
|
||||
client_id=client_id,
|
||||
client_secret=client_secret)
|
||||
client_secret=client_secret,
|
||||
scope=scope)
|
||||
|
||||
self.assertEqual(redirect_uri, oidc.redirect_uri)
|
||||
self.assertEqual(authorization_code, oidc.code)
|
||||
self.assertEqual(scope, oidc.scope)
|
||||
self.assertEqual(identity_provider, oidc.identity_provider)
|
||||
self.assertEqual(protocol, oidc.protocol)
|
||||
self.assertEqual(access_token_endpoint, oidc.access_token_endpoint)
|
||||
|
|
|
@ -38,7 +38,7 @@ ACCESS_TOKEN_VIA_PASSWORD_RESP = {
|
|||
"access_token": "z5H1ITZLlJVDHQXqJun",
|
||||
"token_type": "bearer",
|
||||
"expires_in": 3599,
|
||||
"scope": "profile",
|
||||
"scope": "openid profile",
|
||||
"refresh_token": "DCERsh83IAhu9bhavrp"
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue