From bc82edb2d163b26ab4f7fef8a736e8de2752d6c3 Mon Sep 17 00:00:00 2001
From: Joseph Schorr <josephschorr@users.noreply.github.com>
Date: Fri, 9 Jun 2017 17:12:05 -0400
Subject: [PATCH] Add ability to configure OIDC internal auth engine via
 superuser panel

---
 data/users/oidc.py                            |  6 ++-
 oauth/oidc.py                                 |  2 +-
 .../directives/config/config-setup-tool.html  | 40 ++++++++++++++-----
 static/js/core-config-setup.js                | 12 +++++-
 util/config/validator.py                      |  2 +
 .../validators/test/test_validate_oidcauth.py | 32 +++++++++++++++
 util/config/validators/validate_oidcauth.py   | 23 +++++++++++
 7 files changed, 103 insertions(+), 14 deletions(-)
 create mode 100644 util/config/validators/test/test_validate_oidcauth.py
 create mode 100644 util/config/validators/validate_oidcauth.py

diff --git a/data/users/oidc.py b/data/users/oidc.py
index 1014b513a..8c7831bf2 100644
--- a/data/users/oidc.py
+++ b/data/users/oidc.py
@@ -9,6 +9,10 @@ from util.security.jwtutil import InvalidTokenError
 logger = logging.getLogger(__name__)
 
 
+class UnknownServiceException(Exception):
+  pass
+
+
 class OIDCInternalAuth(FederatedUsers):
   """ Handles authentication by delegating authentication to a signed OIDC JWT produced by the
       configured OIDC service.
@@ -18,7 +22,7 @@ class OIDCInternalAuth(FederatedUsers):
     login_manager = OAuthLoginManager(config)
     self.login_service = login_manager.get_service(login_service_id)
     if self.login_service is None:
-      raise Exception('Unknown OIDC login service %s' % login_service_id)
+      raise UnknownServiceException('Unknown OIDC login service %s' % login_service_id)
 
   @property
   def supports_encrypted_credentials(self):
diff --git a/oauth/oidc.py b/oauth/oidc.py
index f5c9249a2..2b7272652 100644
--- a/oauth/oidc.py
+++ b/oauth/oidc.py
@@ -39,7 +39,7 @@ class OIDCLoginService(OAuthService):
 
     self._public_key_cache = TTLCache(1, PUBLIC_KEY_CACHE_TTL, missing=self._load_public_key)
     self._id = key_name[0:key_name.find('_')].lower()
-    self._http_client = client or config['HTTPCLIENT']
+    self._http_client = client or config.get('HTTPCLIENT')
     self._mailing = config.get('FEATURE_MAILING', False)
 
   def service_id(self):
diff --git a/static/directives/config/config-setup-tool.html b/static/directives/config/config-setup-tool.html
index 09368ba1f..0f68a78cb 100644
--- a/static/directives/config/config-setup-tool.html
+++ b/static/directives/config/config-setup-tool.html
@@ -622,21 +622,23 @@
       <div class="co-panel-body">
         <div class="description">
           <p>
-            Authentication for the registry can be handled by either the registry itself, LDAP or external JWT endpoint.
+            Authentication for the registry can be handled by either the registry itself, LDAP, Keystone, OIDC or external JWT endpoint.
           </p>
           <p>
             Additional <strong>external</strong> authentication providers (such as GitHub) can be used in addition for <strong>login into the UI</strong>.
           </p>
         </div>
 
-        <div class="co-alert co-alert-warning" ng-if="config.AUTHENTICATION_TYPE != 'Database' && !config.FEATURE_REQUIRE_ENCRYPTED_BASIC_AUTH">
-          It is <strong>highly recommended</strong> to require encrypted client passwords. External passwords used in the Docker client will be stored in <strong>plaintext</strong>!
-          <a ng-click="config.FEATURE_REQUIRE_ENCRYPTED_BASIC_AUTH = true">Enable this requirement now</a>.
-        </div>
+        <div ng-if="config.AUTHENTICATION_TYPE != 'OIDC'">
+          <div class="co-alert co-alert-warning" ng-if="config.AUTHENTICATION_TYPE != 'Database' && !config.FEATURE_REQUIRE_ENCRYPTED_BASIC_AUTH">
+            It is <strong>highly recommended</strong> to require encrypted client passwords. External passwords used in the Docker client will be stored in <strong>plaintext</strong>!
+            <a ng-click="config.FEATURE_REQUIRE_ENCRYPTED_BASIC_AUTH = true">Enable this requirement now</a>.
+          </div>
 
-        <div class="co-alert co-alert-success" ng-if="config.AUTHENTICATION_TYPE != 'Database' && config.FEATURE_REQUIRE_ENCRYPTED_BASIC_AUTH">
-          Note: The "Require Encrypted Client Passwords" feature is currently enabled which will
-          prevent passwords from being saved as plaintext by the Docker client.
+          <div class="co-alert co-alert-success" ng-if="config.AUTHENTICATION_TYPE != 'Database' && config.FEATURE_REQUIRE_ENCRYPTED_BASIC_AUTH">
+            Note: The "Require Encrypted Client Passwords" feature is currently enabled which will
+            prevent passwords from being saved as plaintext by the Docker client.
+          </div>
         </div>
 
         <table class="config-table" style="margin-bottom: 20px;">
@@ -648,6 +650,7 @@
                 <option value="LDAP">LDAP</option>
                 <option value="Keystone">Keystone (OpenStack Identity)</option>
                 <option value="JWT">JWT Custom Authentication</option>
+                <option value="OIDC">OIDC Token Authentication</option>
               </select>
             </td>
           </tr>
@@ -687,6 +690,21 @@
           </tr>
         </table>
 
+        <!--  OIDC Token Authentication -->
+        <table class="config-table" ng-if="config.AUTHENTICATION_TYPE == 'OIDC'">
+          <tr>
+            <td>OIDC Provider:</td>
+            <td>
+              <select class="form-control" ng-model="config.INTERNAL_OIDC_SERVICE_ID" ng-if="getOIDCProviders(config).length">
+                <option value="{{ getOIDCProviderId(provider) }}" ng-repeat="provider in getOIDCProviders(config)">{{ config[provider]['SERVICE_NAME'] || getOIDCProviderId(provider) }}</option>
+              </select>
+              <div class="co-alert co-alert-danger" ng-if="!getOIDCProviders(config).length">
+                An OIDC provider must be configured to use this authentication system
+              </div>
+            </td>
+          </tr>
+        </table>
+
         <!--  Keystone Authentication -->
         <table class="config-table" ng-if="config.AUTHENTICATION_TYPE == 'Keystone'">
           <tr>
@@ -1073,7 +1091,7 @@
             <span style="display: inline-block; margin-left: 10px">(<a href="javascript:void(0)" ng-click="removeOIDCProvider(provider)">Delete</a>)</span>
           </div>
           <div class="co-panel-body">
-            <div class="co-alert co-alert-warning" ng-if="config.AUTHENTICATION_TYPE != 'Database' && !(config[provider].LOGIN_BINDING_FIELD)">
+            <div class="co-alert co-alert-warning" ng-if="config.AUTHENTICATION_TYPE && config.AUTHENTICATION_TYPE != 'Database' && config.AUTHENTICATION_TYPE != 'OIDC' && !(config[provider].LOGIN_BINDING_FIELD)">
               Warning: This OIDC provider is not bound to your <strong>{{ config.AUTHENTICATION_TYPE }}</strong> authentication. Logging in via this provider will create a <strong><span class="registry-name"></span>-only user</strong>, which is not the recommended approach. It is <strong>highly</strong> recommended to choose a "Binding Field" below.
             </div>
 
@@ -1134,7 +1152,7 @@
                   </div>
                 </td>
               </tr>
-              <tr ng-if="config.AUTHENTICATION_TYPE != 'Database'">
+              <tr ng-if="config.AUTHENTICATION_TYPE != 'Database' && config.AUTHENTICATION_TYPE != 'OIDC'">
                 <td>Binding Field:</td>
                 <td>
                   <select class="form-control" ng-model="config[provider].LOGIN_BINDING_FIELD">
@@ -1262,7 +1280,7 @@
       </div>
       <div class="co-panel-body">
         <div class="description">
-           If enabled, users can submit Dockerfiles to be built and pushed by the Enterprise Registry.
+           If enabled, users can submit Dockerfiles to be built and pushed by <span class="registry-name"></span>.
         </div>
 
         <div class="config-bool-field" binding="config.FEATURE_BUILD_SUPPORT">
diff --git a/static/js/core-config-setup.js b/static/js/core-config-setup.js
index bc208ef5a..c025870c2 100644
--- a/static/js/core-config-setup.js
+++ b/static/js/core-config-setup.js
@@ -41,6 +41,10 @@ angular.module("core-config-setup", ['angularFileUpload'])
             return config.AUTHENTICATION_TYPE == 'Keystone';
           }, 'password': true},
 
+          {'id': 'oidc-auth', 'title': 'OIDC Authentication', 'condition': function(config) {
+            return config.AUTHENTICATION_TYPE == 'OIDC';
+          }},
+
           {'id': 'signer', 'title': 'ACI Signing', 'condition': function(config) {
             return config.FEATURE_ACI_CONVERSION;
           }},
@@ -201,7 +205,7 @@ angular.module("core-config-setup", ['angularFileUpload'])
             return null;
           }
 
-          return key.substr(0, index);
+          return key.substr(0, index).toLowerCase();
         };
 
         $scope.getOIDCProviders = function(config) {
@@ -685,6 +689,12 @@ angular.module("core-config-setup", ['angularFileUpload'])
           $scope.configform.$setValidity('storageConfig', valid);
         };
 
+        $scope.$watch('config.INTERNAL_OIDC_SERVICE_ID', function(service_id) {
+          if (service_id) {
+            $scope.config['FEATURE_DIRECT_LOGIN'] = false;
+          }
+        });
+
         $scope.$watch('config.FEATURE_STORAGE_REPLICATION', function() {
           refreshStorageConfig();
         });
diff --git a/util/config/validator.py b/util/config/validator.py
index dda3bd666..90582ccd1 100644
--- a/util/config/validator.py
+++ b/util/config/validator.py
@@ -22,6 +22,7 @@ from util.config.validators.validate_oidc import OIDCLoginValidator
 from util.config.validators.validate_timemachine import TimeMachineValidator
 from util.config.validators.validate_access import AccessSettingsValidator
 from util.config.validators.validate_actionlog_archiving import ActionLogArchivingValidator
+from util.config.validators.validate_oidcauth import OIDCAuthValidator
 
 logger = logging.getLogger(__name__)
 
@@ -59,6 +60,7 @@ VALIDATORS = {
   TimeMachineValidator.name: TimeMachineValidator.validate,
   AccessSettingsValidator.name: AccessSettingsValidator.validate,
   ActionLogArchivingValidator.name: ActionLogArchivingValidator.validate,
+  OIDCAuthValidator.name: OIDCAuthValidator.validate,
 }
 
 def validate_service_for_config(service, config, password=None):
diff --git a/util/config/validators/test/test_validate_oidcauth.py b/util/config/validators/test/test_validate_oidcauth.py
new file mode 100644
index 000000000..7c5609ccb
--- /dev/null
+++ b/util/config/validators/test/test_validate_oidcauth.py
@@ -0,0 +1,32 @@
+import pytest
+
+from util.config.validators import ConfigValidationException
+from util.config.validators.validate_oidcauth import OIDCAuthValidator
+
+from test.fixtures import *
+
+@pytest.mark.parametrize('unvalidated_config', [
+  ({'AUTHENTICATION_TYPE': 'OIDC'}),
+  ({'AUTHENTICATION_TYPE': 'OIDC', 'INTERNAL_OIDC_SERVICE_ID': 'someservice'}),
+])
+def test_validate_invalid_oidc_auth_config(unvalidated_config, app):
+  validator = OIDCAuthValidator()
+
+  with pytest.raises(ConfigValidationException):
+    validator.validate(unvalidated_config, None, None)
+
+
+def test_validate_oidc_auth(app):
+  config = {
+    'AUTHENTICATION_TYPE': 'OIDC',
+    'INTERNAL_OIDC_SERVICE_ID': 'someservice',
+    'SOMESERVICE_LOGIN_CONFIG': {
+      'CLIENT_ID': 'foo',
+      'CLIENT_SECRET': 'bar',
+      'OIDC_SERVER': 'http://someserver',
+    },
+    'HTTPCLIENT': None,
+  }
+
+  validator = OIDCAuthValidator()
+  validator.validate(config, None, None)
diff --git a/util/config/validators/validate_oidcauth.py b/util/config/validators/validate_oidcauth.py
new file mode 100644
index 000000000..bfec66ed7
--- /dev/null
+++ b/util/config/validators/validate_oidcauth.py
@@ -0,0 +1,23 @@
+from app import app
+from data.users.oidc import OIDCInternalAuth, UnknownServiceException
+from util.config.validators import BaseValidator, ConfigValidationException
+
+class OIDCAuthValidator(BaseValidator):
+  name = "oidc-auth"
+
+  @classmethod
+  def validate(cls, config, user, user_password):
+    if config.get('AUTHENTICATION_TYPE', 'Database') != 'OIDC':
+      return
+
+    login_service_id = config.get('INTERNAL_OIDC_SERVICE_ID')
+    if not login_service_id:
+        raise ConfigValidationException('Missing OIDC provider')
+
+    # By instantiating the auth engine, it will check if the provider exists and works.
+    try:
+      OIDCInternalAuth(config, login_service_id, False)
+    except UnknownServiceException as use:
+      raise ConfigValidationException(use.message)
+
+