From 6c7b6101cca930266eff86b1d6bfbea01c8f8e93 Mon Sep 17 00:00:00 2001
From: Joseph Schorr <josephschorr@users.noreply.github.com>
Date: Fri, 7 Apr 2017 11:30:31 -0400
Subject: [PATCH 1/4] Add missing client ID and client secret from OIDC config
in setup tool
Stupidly forget to add these
---
static/directives/config/config-setup-tool.html | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/static/directives/config/config-setup-tool.html b/static/directives/config/config-setup-tool.html
index 226a7f8da..715eaa62a 100644
--- a/static/directives/config/config-setup-tool.html
+++ b/static/directives/config/config-setup-tool.html
@@ -999,6 +999,18 @@
</div>
</td>
</tr>
+ <tr>
+ <td>Client ID:</td>
+ <td>
+ <span class="config-string-field" binding="config[provider].CLIENT_ID"></span>
+ </td>
+ </tr>
+ <tr>
+ <td>Client Secret:</td>
+ <td>
+ <span class="config-string-field" binding="config[provider].CLIENT_SECRET"></span>
+ </td>
+ </tr>
<tr>
<td>Service Name:</td>
<td>
From ed3da4697fff19d2cb14505e68c650e5b73ff264 Mon Sep 17 00:00:00 2001
From: Joseph Schorr <josephschorr@users.noreply.github.com>
Date: Fri, 7 Apr 2017 11:33:02 -0400
Subject: [PATCH 2/4] Add client ID and client secret to OIDC config validator
---
util/config/validators/test/test_validate_oidc.py | 5 +++++
util/config/validators/validate_oidc.py | 8 ++++++++
2 files changed, 13 insertions(+)
diff --git a/util/config/validators/test/test_validate_oidc.py b/util/config/validators/test/test_validate_oidc.py
index 0bc3c89a6..274f5e103 100644
--- a/util/config/validators/test/test_validate_oidc.py
+++ b/util/config/validators/test/test_validate_oidc.py
@@ -9,6 +9,9 @@ from util.config.validators.validate_oidc import OIDCLoginValidator
@pytest.mark.parametrize('unvalidated_config', [
({'SOMETHING_LOGIN_CONFIG': {}}),
+ ({'SOMETHING_LOGIN_CONFIG': {'OIDC_SERVER': 'foo'}}),
+ ({'SOMETHING_LOGIN_CONFIG': {'OIDC_SERVER': 'foo', 'CLIENT_ID': 'foobar'}}),
+ ({'SOMETHING_LOGIN_CONFIG': {'OIDC_SERVER': 'foo', 'CLIENT_SECRET': 'foobar'}}),
])
def test_validate_invalid_oidc_login_config(unvalidated_config):
validator = OIDCLoginValidator()
@@ -30,6 +33,8 @@ def test_validate_oidc_login():
validator = OIDCLoginValidator()
validator.validate({
'SOMETHING_LOGIN_CONFIG': {
+ 'CLIENT_ID': 'foo',
+ 'CLIENT_SECRET': 'bar',
'OIDC_SERVER': 'http://someserver',
'DEBUGGING': True, # Allows for HTTP.
},
diff --git a/util/config/validators/validate_oidc.py b/util/config/validators/validate_oidc.py
index 84e626b46..ba94b0362 100644
--- a/util/config/validators/validate_oidc.py
+++ b/util/config/validators/validate_oidc.py
@@ -18,6 +18,14 @@ class OIDCLoginValidator(BaseValidator):
msg = 'Missing OIDC_SERVER on OIDC service %s' % service.service_id()
raise ConfigValidationException(msg)
+ if service.config.get('CLIENT_ID') is None:
+ msg = 'Missing CLIENT_ID on OIDC service %s' % service.service_id()
+ raise ConfigValidationException(msg)
+
+ if service.config.get('CLIENT_SECRET') is None:
+ msg = 'Missing CLIENT_SECRET on OIDC service %s' % service.service_id()
+ raise ConfigValidationException(msg)
+
try:
if not service.validate():
msg = 'Could not validate OIDC service %s' % service.service_id()
From 002972fc2f7f10a27d6a473e80e0dd62a3e48de2 Mon Sep 17 00:00:00 2001
From: Joseph Schorr <josephschorr@users.noreply.github.com>
Date: Fri, 7 Apr 2017 11:39:34 -0400
Subject: [PATCH 3/4] Read OIDC issuer from the OIDC discovery document, if
present
---
oauth/oidc.py | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/oauth/oidc.py b/oauth/oidc.py
index f3dab2032..8d509317f 100644
--- a/oauth/oidc.py
+++ b/oauth/oidc.py
@@ -141,7 +141,11 @@ class OIDCLoginService(OAuthService):
@property
def _issuer(self):
- return self.config.get('OIDC_ISSUER', self.config['OIDC_SERVER'])
+ # Read the issuer from the OIDC config, falling back to the configured OIDC server.
+ issuer = self._oidc_config().get('issuer', self.config['OIDC_SERVER'])
+
+ # If specified, use the overridden OIDC issuer.
+ return self.config.get('OIDC_ISSUER', issuer)
@lru_cache(maxsize=1)
def _oidc_config(self):
From 0c7bac26b7d3720d30bcefdaacbc45aedbee45c1 Mon Sep 17 00:00:00 2001
From: Joseph Schorr <josephschorr@users.noreply.github.com>
Date: Fri, 7 Apr 2017 11:48:53 -0400
Subject: [PATCH 4/4] Add additional debug logs to OIDC auth to make debugging
easier
---
oauth/oidc.py | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/oauth/oidc.py b/oauth/oidc.py
index 8d509317f..2e714a555 100644
--- a/oauth/oidc.py
+++ b/oauth/oidc.py
@@ -127,6 +127,8 @@ class OIDCLoginService(OAuthService):
# Verify subs.
if user_info['sub'] != decoded_id_token['sub']:
+ logger.debug('Mismatch in `sub` returned by OIDC user info endpoint: %s vs %s',
+ user_info['sub'], decoded_id_token['sub'])
raise OAuthLoginException('Mismatch in `sub` returned by OIDC user info endpoint')
# Check if we have a verified email address.
@@ -185,6 +187,8 @@ class OIDCLoginService(OAuthService):
if kid is None:
raise InvalidTokenError('Missing `kid` header')
+ logger.debug('Using key `%s`, attempting to decode token `%s` with aud `%s` and iss `%s`',
+ kid, token, self.client_id(), self._issuer)
try:
return decode(token, self._get_public_key(kid), algorithms=ALLOWED_ALGORITHMS,
audience=self.client_id(),
@@ -193,12 +197,20 @@ class OIDCLoginService(OAuthService):
options=dict(require_nbf=False))
except InvalidTokenError:
# Public key may have expired. Try to retrieve an updated public key and use it to decode.
- return decode(token, self._get_public_key(kid, force_refresh=True),
- algorithms=ALLOWED_ALGORITHMS,
- audience=self.client_id(),
- issuer=self._issuer,
- leeway=JWT_CLOCK_SKEW_SECONDS,
- options=dict(require_nbf=False))
+ try:
+ return decode(token, self._get_public_key(kid, force_refresh=True),
+ algorithms=ALLOWED_ALGORITHMS,
+ audience=self.client_id(),
+ issuer=self._issuer,
+ leeway=JWT_CLOCK_SKEW_SECONDS,
+ options=dict(require_nbf=False))
+ except InvalidTokenError as ite:
+ # Decode again with verify=False, and log the decoded token to allow for easier debugging.
+ nonverified = decode(token, self._get_public_key(kid, force_refresh=True),
+ algorithms=ALLOWED_ALGORITHMS,
+ options=dict(require_nbf=False, verify=False))
+ logger.debug('Got an error when trying to verify OIDC JWT: %s', nonverified)
+ raise ite
def _get_public_key(self, kid, force_refresh=False):
""" Retrieves the public key for this handler with the given kid. Raises a