Merge pull request #2520 from coreos-inc/oidc-config-fix

OIDC configuration and debugging fixes

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.
@@ -141,7 +143,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):
@@ -181,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(),
@@ -189,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

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>

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.
       },

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()