From c55ad59f1f70cdf76f3bbc14d4bce2382740adc5 Mon Sep 17 00:00:00 2001
From: Joseph Schorr <joseph.schorr@coreos.com>
Date: Tue, 5 Dec 2017 10:59:04 -0500
Subject: [PATCH] Allow admins to configure the login scopes for OIDC login

Some OIDC implementations return a larger set of scopes than is necessary, so we allow admins to override.
---
 oauth/oidc.py                                 |  6 +++--
 oauth/test/test_oidc.py                       | 22 +++++++++++++++++--
 .../directives/config/config-setup-tool.html  | 11 ++++++++++
 3 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/oauth/oidc.py b/oauth/oidc.py
index 2b7272652..e19e5c466 100644
--- a/oauth/oidc.py
+++ b/oauth/oidc.py
@@ -60,7 +60,9 @@ class OIDCLoginService(OAuthService):
     if self._mailing:
       default_scopes.append('email')
 
-    return self._oidc_config().get('scopes_supported', default_scopes)
+    supported_scopes = self._oidc_config().get('scopes_supported', default_scopes)
+    login_scopes = self.config.get('LOGIN_SCOPES') or supported_scopes
+    return list(set(login_scopes) & set(supported_scopes))
 
   def authorize_endpoint(self):
     return self._oidc_config().get('authorization_endpoint', '') + '?response_type=code&'
@@ -72,7 +74,7 @@ class OIDCLoginService(OAuthService):
     return self._oidc_config().get('userinfo_endpoint')
 
   def validate(self):
-    return bool(self.token_endpoint())
+    return bool(self.get_login_scopes())
 
   def validate_client_id_and_secret(self, http_client, app_config):
     # TODO: find a way to verify client secret too.
diff --git a/oauth/test/test_oidc.py b/oauth/test/test_oidc.py
index b1ea38a83..73208cbe4 100644
--- a/oauth/test/test_oidc.py
+++ b/oauth/test/test_oidc.py
@@ -69,6 +69,16 @@ def app_config(http_client, mailing_feature):
       'DEBUGGING': True,
     },
 
+    'ANOTHEROIDC_LOGIN_CONFIG': {
+      'CLIENT_ID': 'foo',
+      'CLIENT_SECRET': 'bar',
+      'SERVICE_NAME': 'Some Other Service',
+      'SERVICE_ICON': 'http://some/icon',
+      'OIDC_SERVER': 'http://fakeoidc',
+      'LOGIN_SCOPES': ['openid'],
+      'DEBUGGING': True,
+    },
+
     'HTTPCLIENT': http_client,
   }
 
@@ -76,10 +86,14 @@ def app_config(http_client, mailing_feature):
 def oidc_service(app_config):
   return OIDCLoginService(app_config, 'SOMEOIDC_LOGIN_CONFIG')
 
+@pytest.fixture()
+def another_oidc_service(app_config):
+  return OIDCLoginService(app_config, 'ANOTHEROIDC_LOGIN_CONFIG')
+
 @pytest.fixture()
 def discovery_content(userinfo_supported):
   return {
-    'scopes_supported': ['profile'],
+    'scopes_supported': ['openid', 'profile', 'somescope'],
     'authorization_endpoint': 'http://fakeoidc/authorize',
     'token_endpoint': 'http://fakeoidc/token',
     'userinfo_endpoint': 'http://fakeoidc/userinfo' if userinfo_supported else None,
@@ -209,7 +223,11 @@ def test_discovery(oidc_service, http_client, discovery_content, discovery_handl
 
     assert oidc_service.token_endpoint() == discovery_content['token_endpoint']
     assert oidc_service.user_endpoint() == discovery_content['userinfo_endpoint']
-    assert oidc_service.get_login_scopes() == discovery_content['scopes_supported']
+    assert set(oidc_service.get_login_scopes()) == set(discovery_content['scopes_supported'])
+
+def test_filtered_discovery(another_oidc_service, http_client, discovery_content, discovery_handler):
+  with HTTMock(discovery_handler):
+    assert another_oidc_service.get_login_scopes() == ['openid']
 
 def test_public_config(oidc_service, discovery_handler):
   with HTTMock(discovery_handler):
diff --git a/static/directives/config/config-setup-tool.html b/static/directives/config/config-setup-tool.html
index 822737bf5..45a3f2ca0 100644
--- a/static/directives/config/config-setup-tool.html
+++ b/static/directives/config/config-setup-tool.html
@@ -1172,6 +1172,17 @@
                   </div>
                 </td>
               </tr>
+              <tr>
+                <td>Login Scopes:</td>
+                <td>
+                  <span class="config-list-field" item-title="Login Scope" binding="config[provider].LOGIN_SCOPES"></span>
+                  <div class="help-text">
+                    If specified, the scopes to send to the OIDC provider when performing the login flow. Note that, <strong>if specified</strong>, these scopes will
+                    <strong>override</strong> those set by default, so this list <strong>must</strong> include a scope for OpenID Connect
+                    (typically the <code>openid</code> scope) or this provider will fail.
+                  </div>
+                </td>
+              </tr>
             </table>
             <div>
               <h4>Callback URLs for this service:</h4>