Pull out github trigger and login validation into validator class

util/config/validator.py

@@ -1,13 +1,7 @@
 import logging
 
-import peewee
-
-from app import app, get_app_url
 from auth.auth_context import get_authenticated_user
-from data.database import validate_database_url
 from data.users import LDAP_CERT_FILENAME
-from oauth.services.github import GithubOAuthService
-from oauth.services.gitlab import GitLabOAuthService
 
 from util.config.validators.validate_database import DatabaseValidator
 from util.config.validators.validate_redis import RedisValidator
@@ -23,6 +17,7 @@ from util.config.validators.validate_ssl import SSLValidator, SSL_FILENAMES
 from util.config.validators.validate_google_login import GoogleLoginValidator
 from util.config.validators.validate_bitbucket_trigger import BitbucketTriggerValidator
 from util.config.validators.validate_gitlab_trigger import GitLabTriggerValidator
+from util.config.validators.validate_github import GitHubLoginValidator, GitHubTriggerValidator
 
 logger = logging.getLogger(__name__)
 
@@ -30,7 +25,6 @@ class ConfigValidationException(Exception):
   """ Exception raised when the configuration fails to validate for a known reason. """
   pass
 
-
 # Note: Only add files required for HTTPS to the SSL_FILESNAMES list.
 DB_SSL_FILENAMES = ['database.pem']
 JWT_FILENAMES = ['jwt-authn.cert']
@@ -40,6 +34,24 @@ CONFIG_FILENAMES = (SSL_FILENAMES + DB_SSL_FILENAMES + JWT_FILENAMES + ACI_CERT_
                     LDAP_FILENAMES)
 EXTRA_CA_DIRECTORY = 'extra_ca_certs'
 
+VALIDATORS = {
+  DatabaseValidator.name: DatabaseValidator.validate,
+  RedisValidator.name: RedisValidator.validate,
+  StorageValidator.name: StorageValidator.validate,
+  EmailValidator.name: EmailValidator.validate,
+  GitHubLoginValidator.name: GitHubLoginValidator.validate,
+  GitHubTriggerValidator.name: GitHubTriggerValidator.validate,
+  GitLabTriggerValidator.name: GitLabTriggerValidator.validate,
+  BitbucketTriggerValidator.name: BittorrentValidator.validate,
+  GoogleLoginValidator.name: GoogleLoginValidator.validate,
+  SSLValidator.name: SSLValidator.validate,
+  LDAPValidator.name: LDAPValidator.validate,
+  JWTAuthValidator.name: JWTAuthValidator.validate,
+  KeystoneValidator.name: KeystoneValidator.validate,
+  SignerValidator.name: SignerValidator.validate,
+  SecurityScannerValidator.name: SecurityScannerValidator.validate,
+  BittorrentValidator.name: BittorrentValidator.validate,
+}
 
 def validate_service_for_config(service, config, password=None):
   """ Attempts to validate the configuration for the given service. """
@@ -59,73 +71,3 @@ def validate_service_for_config(service, config, password=None):
       'status': False,
       'reason': str(ex)
     }
-
-
-def _validate_database(config, user_obj, _):
-  """ Validates connecting to the database. """
-  try:
-    validate_database_url(config['DB_URI'], config.get('DB_CONNECTION_ARGS', {}))
-  except peewee.OperationalError as ex:
-    if ex.args and len(ex.args) > 1:
-      raise ConfigValidationException(ex.args[1])
-    else:
-      raise ex
-
-
-def _validate_github(config_key):
-  return lambda config, user_obj, _: _validate_github_with_key(config_key, config)
-
-
-def _validate_github_with_key(config_key, config):
-  """ Validates the OAuth credentials and API endpoint for a Github service. """
-  github_config = config.get(config_key)
-  if not github_config:
-    raise ConfigValidationException('Missing GitHub client id and client secret')
-
-  endpoint = github_config.get('GITHUB_ENDPOINT')
-  if not endpoint:
-    raise ConfigValidationException('Missing GitHub Endpoint')
-
-  if endpoint.find('http://') != 0 and endpoint.find('https://') != 0:
-    raise ConfigValidationException('Github Endpoint must start with http:// or https://')
-
-  if not github_config.get('CLIENT_ID'):
-    raise ConfigValidationException('Missing Client ID')
-
-  if not github_config.get('CLIENT_SECRET'):
-    raise ConfigValidationException('Missing Client Secret')
-
-  if github_config.get('ORG_RESTRICT') and not github_config.get('ALLOWED_ORGANIZATIONS'):
-    raise ConfigValidationException('Organization restriction must have at least one allowed ' +
-                                    'organization')
-
-  client = app.config['HTTPCLIENT']
-  oauth = GithubOAuthService(config, config_key)
-  result = oauth.validate_client_id_and_secret(client, app.config)
-  if not result:
-    raise ConfigValidationException('Invalid client id or client secret')
-
-  if github_config.get('ALLOWED_ORGANIZATIONS'):
-    for org_id in github_config.get('ALLOWED_ORGANIZATIONS'):
-      if not oauth.validate_organization(org_id, client):
-        raise ConfigValidationException('Invalid organization: %s' % org_id)
-
-
-VALIDATORS = {
-  DatabaseValidator.name: DatabaseValidator.validate,
-  RedisValidator.name: RedisValidator.validate,
-  StorageValidator.name: StorageValidator.validate,
-  EmailValidator.name: EmailValidator.validate,
-  'github-login': _validate_github('GITHUB_LOGIN_CONFIG'),
-  'github-trigger': _validate_github('GITHUB_TRIGGER_CONFIG'),
-  GitLabTriggerValidator.name: GitLabTriggerValidator.validate,
-  BitbucketTriggerValidator.name: BittorrentValidator.validate,
-  GoogleLoginValidator.name: GoogleLoginValidator.validate,
-  SSLValidator.name: SSLValidator.validate,
-  LDAPValidator.name: LDAPValidator.validate,
-  JWTAuthValidator.name: JWTAuthValidator.validate,
-  KeystoneValidator.name: KeystoneValidator.validate,
-  SignerValidator.name: SignerValidator.validate,
-  SecurityScannerValidator.name: SecurityScannerValidator.validate,
-  BittorrentValidator.name: BittorrentValidator.validate,
-}

util/config/validators/test/test_validate_github.py

@@ -0,0 +1,62 @@
+import pytest
+
+from httmock import urlmatch, HTTMock
+
+from util.config.validators import ConfigValidationException
+from util.config.validators.validate_github import GitHubLoginValidator, GitHubTriggerValidator
+
+@pytest.fixture(params=[GitHubLoginValidator, GitHubTriggerValidator])
+def github_validator(request):
+  return request.param
+
+
+@pytest.mark.parametrize('github_config', [
+  ({}),
+  ({'GITHUB_ENDPOINT': 'foo'}),
+  ({'GITHUB_ENDPOINT': 'http://github.com'}),
+  ({'GITHUB_ENDPOINT': 'http://github.com', 'CLIENT_ID': 'foo'}),
+  ({'GITHUB_ENDPOINT': 'http://github.com', 'CLIENT_SECRET': 'foo'}),
+  ({
+    'GITHUB_ENDPOINT': 'http://github.com',
+    'CLIENT_ID': 'foo',
+    'CLIENT_SECRET': 'foo',
+    'ORG_RESTRICT': True
+  }),
+  ({
+    'GITHUB_ENDPOINT': 'http://github.com',
+    'CLIENT_ID': 'foo',
+    'CLIENT_SECRET': 'foo',
+    'ORG_RESTRICT': True,
+    'ALLOWED_ORGANIZATIONS': [],
+  }),
+])
+def test_validate_invalid_github_config(github_config, github_validator):
+  with pytest.raises(ConfigValidationException):
+    unvalidated_config = {}
+    unvalidated_config[github_validator.config_key] = github_config
+    github_validator.validate(unvalidated_config, None, None)
+
+def test_validate_github(github_validator):
+  url_hit = [False, False]
+
+  @urlmatch(netloc=r'somehost')
+  def handler(url, request):
+    url_hit[0] = True
+    return {'status_code': 200, 'content': '', 'headers': {'X-GitHub-Request-Id': 'foo'}}
+
+  @urlmatch(netloc=r'somehost', path=r'/api/v3/applications/foo/tokens/foo')
+  def app_handler(url, request):
+    url_hit[1] = True
+    return {'status_code': 404, 'content': '', 'headers': {'X-GitHub-Request-Id': 'foo'}}
+
+  with HTTMock(app_handler, handler):
+    github_validator.validate({
+      github_validator.config_key: {
+        'GITHUB_ENDPOINT': 'http://somehost',
+        'CLIENT_ID': 'foo',
+        'CLIENT_SECRET': 'bar',
+      },
+    }, None, None)
+
+  assert url_hit[0]
+  assert url_hit[1]

util/config/validators/validate_github.py

@@ -0,0 +1,51 @@
+from app import app
+from oauth.services.github import GithubOAuthService
+from util.config.validators import BaseValidator, ConfigValidationException
+
+class BaseGitHubValidator(BaseValidator):
+  name = None
+  config_key = None
+
+  @classmethod
+  def validate(cls, config, user, user_password):
+    """ Validates the OAuth credentials and API endpoint for a Github service. """
+    github_config = config.get(cls.config_key)
+    if not github_config:
+      raise ConfigValidationException('Missing GitHub client id and client secret')
+
+    endpoint = github_config.get('GITHUB_ENDPOINT')
+    if not endpoint:
+      raise ConfigValidationException('Missing GitHub Endpoint')
+
+    if endpoint.find('http://') != 0 and endpoint.find('https://') != 0:
+      raise ConfigValidationException('Github Endpoint must start with http:// or https://')
+
+    if not github_config.get('CLIENT_ID'):
+      raise ConfigValidationException('Missing Client ID')
+
+    if not github_config.get('CLIENT_SECRET'):
+      raise ConfigValidationException('Missing Client Secret')
+
+    if github_config.get('ORG_RESTRICT') and not github_config.get('ALLOWED_ORGANIZATIONS'):
+      raise ConfigValidationException('Organization restriction must have at least one allowed ' +
+                                      'organization')
+
+    client = app.config['HTTPCLIENT']
+    oauth = GithubOAuthService(config, cls.config_key)
+    result = oauth.validate_client_id_and_secret(client, app.config)
+    if not result:
+      raise ConfigValidationException('Invalid client id or client secret')
+
+    if github_config.get('ALLOWED_ORGANIZATIONS'):
+      for org_id in github_config.get('ALLOWED_ORGANIZATIONS'):
+        if not oauth.validate_organization(org_id, client):
+          raise ConfigValidationException('Invalid organization: %s' % org_id)
+
+
+class GitHubLoginValidator(BaseGitHubValidator):
+  name = "github-login"
+  config_key = "GITHUB_LOGIN_CONFIG"
+
+class GitHubTriggerValidator(BaseGitHubValidator):
+  name = "github-trigger"
+  config_key = "GITHUB_TRIGGER_CONFIG"