Merge pull request #5889 from TaykYoku/8.0_fixOAuth

fstagni · web-flow · commit dce42201936a · 2022-02-23T12:18:28.000+01:00
[8.0][OAuth 2] fix bugs in TokenManager, AuthServer
diff --git a/src/DIRAC/FrameworkSystem/API/AuthHandler.py b/src/DIRAC/FrameworkSystem/API/AuthHandler.py
@@ -382,7 +382,7 @@ def get_redirect(self, state, error=None, error_description="", chooseScope=[]):
 
         # RESPONSE to basic DIRAC client request
         resp = self.server.create_authorization_response(response, grant_user)
-        if not resp.payload.startswith("<!DOCTYPE html>"):
+        if isinstance(resp.payload, str) and not resp.payload.startswith("<!DOCTYPE html>"):
             resp.payload = getHTML("authorization response", state=resp.status_code, body=resp.payload)
         return resp
 
diff --git a/src/DIRAC/FrameworkSystem/Client/TokenManagerClient.py b/src/DIRAC/FrameworkSystem/Client/TokenManagerClient.py
@@ -6,6 +6,7 @@
 from DIRAC.Core.Utilities.DictCache import DictCache
 from DIRAC.Core.Base.Client import Client, createClient
 from DIRAC.ConfigurationSystem.Client.Helpers import Registry
+from DIRAC.Resources.IdProvider.IdProviderFactory import IdProviderFactory
 
 gTokensSync = ThreadSafe.Synchronizer()
 
@@ -20,6 +21,7 @@ def __init__(self, **kwargs):
         super(TokenManagerClient, self).__init__(**kwargs)
         self.setServer("Framework/TokenManager")
         self.__tokensCache = DictCache()
+        self.idps = IdProviderFactory()
 
     @gTokensSync
     def getToken(
@@ -53,9 +55,9 @@ def getToken(
             return result
         idpObj = result["Value"]
 
-        if userGroup and (groupScope := idpObj.getGroupScopes(userGroup)):
+        if userGroup and (result := idpObj.getGroupScopes(userGroup))["OK"]:
             # What scope correspond to the requested group?
-            scope = list(set((scope or []) + groupScope))
+            scope = list(set((scope or []) + result["Value"]))
 
         # Set the scope
         idpObj.scope = " ".join(scope)
diff --git a/src/DIRAC/FrameworkSystem/Service/TokenManagerHandler.py b/src/DIRAC/FrameworkSystem/Service/TokenManagerHandler.py
@@ -46,7 +46,7 @@
 
 class TokenManagerHandler(TornadoService):
 
-    DEFAULT_AUTHENTICATION = ["authenticated"]
+    DEFAULT_AUTHORIZATION = ["authenticated"]
     DEFAULT_RT_EXPIRATION_TIME = 24 * 3600
 
     @classmethod
@@ -211,9 +211,9 @@ def export_getToken(
             return result
         idpObj = result["Value"]
 
-        if userGroup and (groupScope := idpObj.getGroupScopes(userGroup)):
+        if userGroup and (result := idpObj.getGroupScopes(userGroup))["OK"]:
             # What scope correspond to the requested group?
-            scope = list(set((scope or []) + groupScope))
+            scope = list(set((scope or []) + result["Value"]))
 
         # Set the scope
         idpObj.scope = " ".join(scope)
diff --git a/src/DIRAC/FrameworkSystem/private/authorization/AuthServer.py b/src/DIRAC/FrameworkSystem/private/authorization/AuthServer.py
@@ -1,6 +1,5 @@
 """ This class provides authorization server activity. """
 import re
-import six
 import json
 import pprint
 from dominate import tags as dom
@@ -343,11 +342,15 @@ def handle_response(self, status_code=None, payload=None, headers=None, newSessi
 
         :return: TornadoResponse()
         """
-        sLog.debug(
-            f"Handle authorization response with {status_code} status code:",
-            "HTML page" if isinstance(payload, str) and payload.startswith("<!DOCTYPE html>") else payload,
-        )
         resp = TornadoResponse(payload, status_code)
+        if not isinstance(payload, dict):
+            sLog.debug(
+                f"Handle authorization response with {status_code} status code:",
+                "HTML page" if payload.startswith("<!DOCTYPE html>") else payload,
+            )
+        elif "error" in payload:
+            resp.clear_cookie("auth_session")  # pylint: disable=no-member
+            sLog.error(f"{payload['error']}: {payload.get('error_description', 'unknown')}")
         if headers:
             sLog.debug("Headers:", headers)
             for key, value in headers:
@@ -356,7 +359,7 @@ def handle_response(self, status_code=None, payload=None, headers=None, newSessi
             sLog.debug("Initialize new session:", newSession)
             # pylint: disable=no-member
             resp.set_secure_cookie("auth_session", json.dumps(newSession), secure=True, httponly=True)
-        if delSession or isinstance(payload, dict) and "error" in payload:
+        if delSession:
             resp.clear_cookie("auth_session")  # pylint: disable=no-member
         return resp
 
@@ -386,18 +389,13 @@ def validate_consent_request(self, request, provider=None):
 
         :return: response generated by `handle_response` or S_ERROR or html
         """
-        if request.method != "GET":
-            return self.handle_response(
-                payload=getHTML("use GET method", theme="error", info="Use GET method to access this endpoint."),
-                delSession=True,
-            )
         try:
             request = self.create_oauth2_request(request)
             # Check Identity Provider
             req = self.validateIdentityProvider(request, provider)
 
-            # If return IdP selector
-            if isinstance(req, six.string_types):
+            # If return HTML page with IdP selector
+            if isinstance(req, str):
                 return req
 
             sLog.info("Validate consent request for ", req.state)
diff --git a/src/DIRAC/Resources/IdProvider/OAuth2IdProvider.py b/src/DIRAC/Resources/IdProvider/OAuth2IdProvider.py
@@ -14,7 +14,6 @@
 from authlib.oidc.discovery.well_known import get_well_known_url
 from authlib.oauth2.rfc7636 import create_s256_code_challenge
 
-from DIRAC.FrameworkSystem.private.authorization.utils.Requests import createOAuth2Request
 
 from DIRAC import S_OK, S_ERROR
 from DIRAC.Core.Utilities import ThreadSafe
@@ -27,6 +26,7 @@
     getVOs,
 )
 from DIRAC.FrameworkSystem.private.authorization.utils.Tokens import OAuth2Token
+from DIRAC.FrameworkSystem.private.authorization.utils.Requests import createOAuth2Request
 
 DEFAULT_HEADERS = {"Accept": "application/json", "Content-Type": "application/x-www-form-urlencoded;charset=UTF-8"}
 
@@ -407,9 +407,9 @@ def deviceAuthorization(self, group=None):
 
         # Notify user to go to authorization endpoint
         if response.get("verification_uri_complete"):
-            showURL = "Use next link to continue\n%s" % response["verification_uri_complete"]
+            showURL = "Use the following link to continue\n%s" % response["verification_uri_complete"]
         else:
-            showURL = 'Use next link to continue, your user code is "%s"\n%s' % (
+            showURL = 'Use the following link to continue, your user code is "%s"\n%s' % (
                 response["user_code"],
                 response["verification_uri"],
             )
@@ -543,13 +543,10 @@ def getGroupScopes(self, group: str) -> list:
         return scope_to_list(idPScope) if idPScope else []
 
     def getScopeGroups(self, scope: str) -> list:
-        """Get DIRAC groups related to scope
-
-        :param scope: scope
-        """
+        """Get DIRAC groups related to scope"""
         groups = []
         for group in getAllGroups():
-            if g_scope := self.getGroupScopes(group) and set(g_scope).issubset(scope_to_list(scope)):
+            if (g_scope := self.getGroupScopes(group)) and set(g_scope).issubset(scope_to_list(scope)):
                 groups.append(group)
         return groups
 
