Merge pull request #4674 from communitybridge/unicron-github-oauth-app-redirect

Rewrite /v2/user-from-session to use already existing callback to avoid need of updating GitHub OAuth app

Author: lukaszgryglicki

cla-backend/cla/controllers/repository_service.py

@@ -6,7 +6,7 @@
 """
 
 import cla
-from falcon import HTTP_202, HTTP_404
+from falcon import HTTP_404
 
 def received_activity(provider, data):
     """
@@ -34,19 +34,20 @@ def sign_request(provider, installation_id, github_repository_id, change_request
     service = cla.utils.get_repository_service(provider)
     return service.sign_request(installation_id, github_repository_id, change_request_id, request)
 
-def user_from_session(redirect, redirect_url, state, code, request, response=None):
+def user_from_session(request, response=None):
     """
     Return user from OAuth2 session
     """
-    # LG: to test using MockGitHub class
+    # LG: to test with other GitHub APP and BASE API URL (for OAuth redirects)
     # import os
+    # os.environ["GH_OAUTH_CLIENT_ID"] = os.getenv("GH_OAUTH_CLIENT_ID_CLI", os.environ["GH_OAUTH_CLIENT_ID"])
+    # os.environ["GH_OAUTH_SECRET"] = os.getenv("GH_OAUTH_SECRET_CLI", os.environ["GH_OAUTH_SECRET"])
+    # os.environ["CLA_API_BASE"] = os.getenv("CLA_API_BASE_CLI", os.environ["CLA_API_BASE"])
+    # LG: to test using MockGitHub class
     # from cla.models.github_models import MockGitHub
-    # user = MockGitHub(os.environ["GITHUB_OAUTH_TOKEN"]).user_from_session(request, redirect, redirect_url, state, code)
-    user = cla.utils.get_repository_service('github').user_from_session(request, redirect, redirect_url, state, code)
+    # user = MockGitHub(os.environ["GITHUB_OAUTH_TOKEN"]).user_from_session(request)
+    user = cla.utils.get_repository_service('github').user_from_session(request)
     if user is None:
         response.status = HTTP_404
         return {"errors": "Cannot find user from session"}
-    if isinstance(user, dict):
-        response.status = HTTP_202
-        return user
     return user.to_dict()

cla-backend/cla/models/github_models.py

@@ -7,6 +7,8 @@
 import concurrent.futures
 import json
 import os
+import base64
+import binascii
 import threading
 import time
 import uuid
@@ -96,53 +98,29 @@ def received_activity(self, data):
         else:
             cla.log.debug("github_models.received_activity - Ignoring unsupported action: {}".format(data["action"]))
 
-    def user_from_session(self, request, redirect, redirect_url, state, code):
+    def user_from_session(self, request):
         fn = "github_models.user_from_session"
-        cla.log.debug(f"{fn} - Loading session from request: {request}...")
+        cla.log.debug(f"{fn} - loading session from request: {request}...")
         session = self._get_request_session(request)
-        cla.log.debug(f"{fn} - redirect: {redirect}, redirect_url: {redirect_url}, state: {state}, code: {code}, session: {session}")
+        cla.log.debug(f"{fn} - session: {session}")
 
-        # we can already have token in the session
+        # We can already have token in the session
         if "github_oauth2_token" in session:
-            cla.log.debug(f"{fn} - Using existing session GitHub OAuth2 token")
+            cla.log.debug(f"{fn} - using existing session GitHub OAuth2 token")
             user = self.get_or_create_user(request)
-            cla.log.debug(f"{fn} - loaded user {user.to_dict()}")
-            return user
-
-        # if not then we can either request a new OAuth2 GitHub authentication or user code & state from GitHub to create a session
-        if code and state:
-            session_state = None
-            if "github_oauth2_state" in session:
-                session_state = session["github_oauth2_state"]
-                cla.log.warning(f"{fn} - github_oauth2_state in current session: {session_state}")
+            if user is None:
+                cla.log.debug(f"{fn} - cannot find user, returning HTTP 404 status")
             else:
-                cla.log.warning(f"{fn} - github_oauth2_state not set in current session")
-            if session_state and state != session_state:
-                cla.log.warning(f"{fn} - invalid GitHub OAuth2 state {session_state} expecting {state}")
-                raise falcon.HTTPBadRequest(f"Invalid OAuth2 state: '{session_state}' != '{state}'")
-            token_url = cla.conf["GITHUB_OAUTH_TOKEN_URL"]
-            client_id = os.environ["GH_OAUTH_CLIENT_ID"]
-            client_secret = os.environ["GH_OAUTH_SECRET"]
-            try:
-                token = self._fetch_token(client_id, state, token_url, client_secret, code)
-            except Exception as err:
-                cla.log.warning(f"{fn} - GitHub OAuth2 error: {err}. Likely bad or expired code.")
-                raise falcon.HTTPBadRequest("OAuth2 code is invalid or expired")
-            cla.log.debug(f"{fn} - oauth2 token received for state {state}: {token} - storing token in session")
-            session["github_oauth2_token"] = token
-            user = self.get_or_create_user(request)
-            cla.log.debug(f"{fn} - loaded user {user.to_dict()}")
+                cla.log.debug(f"{fn} - loaded user {user.to_dict()} returning HTTP 200 status")
             return user
-        else:
-            cla.log.debug(f"{fn} - No existing GitHub OAuth2 token - building authorization url and state")
-            authorization_url, new_state = self.get_github_oauth2_redirect_url_and_state(redirect_url)
-            cla.log.debug(f"{fn} - Obtained GitHub OAuth2 state from authorization - storing state in the session...")
-            session["github_oauth2_state"] = new_state
-            cla.log.debug(f"{fn} - GitHub OAuth2 request with state {new_state} - sending user to {authorization_url}")
-            if redirect:
-                raise falcon.HTTPFound(authorization_url)
-            else:
-                return { "redirect_url": authorization_url }
+
+        authorization_url, csrf_token = self.get_authorization_url_and_state(None, None, None, ["user:email"], state='user-from-session')
+        cla.log.debug(f"{fn} - obtained GitHub OAuth2 state from authorization - storing CSRF token in the session...")
+        session["github_oauth2_state"] = csrf_token
+        cla.log.debug(f"{fn} - GitHub OAuth2 request with CSRF token {csrf_token} - sending user to {authorization_url}")
+        cla.log.debug(f"{fn} - redirecting by returning 302 and redirect URL")
+        # We must redirect to GitHub OAuth app for authentication, it will return you to /v2/github/installation which will handle returning user data
+        raise falcon.HTTPFound(authorization_url)
 
     def sign_request(self, installation_id, github_repository_id, change_request_id, request):
         """
@@ -204,26 +182,7 @@ def _get_request_session(self, request) -> dict:  # pylint: disable=no-self-use
 
         return session
 
-    def get_github_oauth2_redirect_url_and_state(self, redirect_uri):
-        fn = "github_models.get_github_oauth2_redirect_url_and_state"
-        github_oauth_url = cla.conf["GITHUB_OAUTH_AUTHORIZE_URL"]
-        github_oauth_client_id = os.environ["GH_OAUTH_CLIENT_ID"]
-        if not redirect_uri:
-            redirect_uri = os.environ.get("CLA_API_BASE", "").strip() + "/v2/user-from-session"
-
-        scope = ["user:email"]
-        cla.log.debug(
-            f"{fn} - Directing user to the github authorization url: {github_oauth_url} via "
-            f"our github installation flow: {redirect_uri} "
-            f"using the github oauth client id: {github_oauth_client_id[0:5]} "
-            f"with scope: {scope}"
-        )
-
-        return self._get_authorization_url_and_state(
-            client_id=github_oauth_client_id, redirect_uri=redirect_uri, scope=scope, authorize_url=github_oauth_url
-        )
-
-    def get_authorization_url_and_state(self, installation_id, github_repository_id, pull_request_number, scope):
+    def get_authorization_url_and_state(self, installation_id, github_repository_id, pull_request_number, scope, state=None):
         """
         Helper method to get the GitHub OAuth2 authorization URL and state.
 
@@ -254,14 +213,14 @@ def get_authorization_url_and_state(self, installation_id, github_repository_id,
         )
 
         return self._get_authorization_url_and_state(
-            client_id=github_oauth_client_id, redirect_uri=redirect_uri, scope=scope, authorize_url=github_oauth_url
+            client_id=github_oauth_client_id, redirect_uri=redirect_uri, scope=scope, authorize_url=github_oauth_url, state=state,
         )
 
-    def _get_authorization_url_and_state(self, client_id, redirect_uri, scope, authorize_url):
+    def _get_authorization_url_and_state(self, client_id, redirect_uri, scope, authorize_url, state=None):
         """
         Mockable helper method to do the fetching of the authorization URL and state from GitHub.
         """
-        return cla.utils.get_authorization_url_and_state(client_id, redirect_uri, scope, authorize_url)
+        return cla.utils.get_authorization_url_and_state(client_id, redirect_uri, scope, authorize_url, state)
 
     def oauth2_redirect(self, state, code, request):  # pylint: disable=too-many-arguments
         """
@@ -283,8 +242,38 @@ def oauth2_redirect(self, state, code, request):  # pylint: disable=too-many-arg
             cla.log.warning(f"{fn} - github_oauth2_state not set in current session")
 
         if state != session_state:
-            cla.log.warning(f"{fn} - invalid GitHub OAuth2 state {session_state} expecting {state}")
-            raise falcon.HTTPBadRequest("Invalid OAuth2 state", state)
+            # Eventually handle user-from-session API callback
+            try:
+                state_data = json.loads(base64.urlsafe_b64decode(state.encode()).decode())
+            except (ValueError, json.JSONDecodeError, binascii.Error):
+                cla.log.warning(f"{fn} - failed to decode state: {state}, error: {err}")
+                raise falcon.HTTPBadRequest("Invalid OAuth2 state", state)
+            state_token = state_data["csrf"]
+            value = state_data["state"]
+            if value != "user-from-session":
+                cla.log.warning(f"{fn} - invalid GitHub OAuth2 state {session_state} expecting {state}, value: {value}")
+                raise falcon.HTTPBadRequest("Invalid OAuth2 state", state)
+            if state_token != session_state:
+                cla.log.warning(f"{fn} - invalid GitHub OAuth2 state {session_state} expecting {state_token} while handling user-from-session callback")
+                raise falcon.HTTPBadRequest(f"Invalid OAuth2 state")
+            cla.log.debug(f"handling user-from-session callback")
+            token_url = cla.conf["GITHUB_OAUTH_TOKEN_URL"]
+            client_id = os.environ["GH_OAUTH_CLIENT_ID"]
+            cla.log.debug(f"{fn} - using client ID {client_id}")
+            client_secret = os.environ["GH_OAUTH_SECRET"]
+            try:
+                token = self._fetch_token(client_id, state, token_url, client_secret, code)
+            except Exception as err:
+                cla.log.warning(f"{fn} - GitHub OAuth2 error: {err}. Likely bad or expired code, returning HTTP 404 state.")
+                raise falcon.HTTPBadRequest("OAuth2 code is invalid or expired")
+            cla.log.debug(f"{fn} - oauth2 token received for state {state}: {token} - storing token in session")
+            session["github_oauth2_token"] = token
+            user = self.get_or_create_user(request)
+            if user is None:
+                cla.log.debug(f"{fn} - cannot find user, returning HTTP 404 status")
+            else:
+                cla.log.debug(f"{fn} - loaded user {user.to_dict()} returning HTTP 200 status")
+            return user.to_dict()
 
         # Get session information for this request.
         cla.log.debug(f"{fn} - attempting to fetch OAuth2 token for state {state}")
@@ -1884,7 +1873,7 @@ def __init__(self, oauth2_token=False):
     def _get_github_client(self, username, token):
         return MockGitHubClient(username, token)
 
-    def _get_authorization_url_and_state(self, client_id, redirect_uri, scope, authorize_url):
+    def _get_authorization_url_and_state(self, client_id, redirect_uri, scope, authorize_url, state=None):
         authorization_url = "http://authorization.url"
         state = "random-state-here"
         return authorization_url, state
@@ -1895,7 +1884,7 @@ def _fetch_token(self, client_id, state, token_url, client_secret, code):  # pyl
     def _get_request_session(self, request) -> dict:
         if self.oauth2_token:
             return {
-                "github_oauth2_token": "random-token", # LG: comment this out to see how Mock class woudl attempt to fetch GitHub token using state & code
+                "github_oauth2_token": "random-token", # LG: comment this out to see how Mock class would attempt to fetch GitHub token using state & code
                 "github_oauth2_state": "random-state",
                 "github_origin_url": "http://github/origin/url",
                 "github_installation_id": 1,

cla-backend/cla/routes.py

@@ -1808,8 +1808,7 @@ def get_event(event_id: hug.types.text, response):
 def user_from_session(request, response):
     """
     GET: /user-from-session
-    Example: https://api.dev.lfcla.com/v2/user-from-session?redirect=0&redirect_url=localhost%3A4200
-    Example: https://api.dev.lfcla.com/v2/user-from-session?state=xyz&code=xyz
+    Example: https://api.dev.lfcla.com/v2/user-from-session
     Returns user object from OAuth2 session
     Example user returned:
     {
@@ -1833,20 +1832,11 @@ def user_from_session(request, response):
       "user_name": "Test User",
       "version": "v1"
     }
-    Can also return 302 redirect (if redirect mode is set redirect=1|yes|true)
-    Can also return 202 redirect_url for GitHub OAuth2 if redirect mode is not set (redirect=0|no|false) with payload:
-    {
-        "redirect_url": "https://github.com/login/oauth/authorize?response_type=code&client_id=38f6d46ff92b7ed04071&redirect_uri=abc&scope=user%3Aemail&state=VCshZQtMs0hPMw6XuMBBZODVaWAxXX"
-    }
-    Can also return 404 on OAuth2 errors or missing redirect_url when no session present
-    return_url should ideally be "CLA contributor console" URL + /v2/user-from-session, Github will add "?state=xyz&code=xyz"
-    """
-    raw_redirect = request.params.get('redirect', 'false').lower()
-    redirect = raw_redirect in ('1', 'true', 'yes')
-    redirect_url = request.params.get('redirect_url', '')
-    state = request.params.get('state', '')
-    code = request.params.get('code', '')
-    return cla.controllers.repository_service.user_from_session(redirect, redirect_url, state, code, request, response)
+    Will 302 redirect to /v2/github/installation if there is no session and that callback will return user data then
+    Will return 200 and user data if there is an active GitHub session
+    Can return 404 on OAuth2 errors
+    """
+    return cla.controllers.repository_service.user_from_session(request, response)
 
 
 @hug.post("/events", versions=1)

cla-backend/cla/utils.py

@@ -9,6 +9,8 @@
 import json
 import os
 import re
+import secrets
+import base64
 import urllib.parse
 import urllib.parse as urlparse
 from datetime import datetime
@@ -1160,7 +1162,7 @@ def get_comment_body(repository_type, sign_url, signed: List[UserCommitSummary],
     return text + committers_comment
 
 
-def get_authorization_url_and_state(client_id, redirect_uri, scope, authorize_url):
+def get_authorization_url_and_state(client_id, redirect_uri, scope, authorize_url, state=None):
     """
     Helper function to get an OAuth2 session authorization URL and state.
 
@@ -1174,17 +1176,38 @@ def get_authorization_url_and_state(client_id, redirect_uri, scope, authorize_ur
     :type authorize_url: string
     """
     fn = "utils.get_authorization_url_and_state"
-    oauth = OAuth2Session(client_id, redirect_uri=redirect_uri, scope=scope)
-    authorization_url, state = oauth.authorization_url(authorize_url)
-    cla.log.debug(
-        f"{fn} - initialized a new oauth session "
-        f"using the github oauth client id: {client_id[0:5]}... "
-        f"with the redirect_uri: {redirect_uri} "
-        f"using scope of: {scope}. Obtained the "
-        f"state: {state} and the "
-        f"generated authorization_url: {authorize_url}"
-    )
-    return authorization_url, state
+    if state is None:
+        oauth = OAuth2Session(client_id, redirect_uri=redirect_uri, scope=scope)
+        authorization_url, state = oauth.authorization_url(authorize_url)
+        cla.log.debug(
+            f"{fn} - initialized a new oauth session "
+            f"using the github oauth client id: {client_id[0:5]}... "
+            f"with the redirect_uri: {redirect_uri} "
+            f"using scope of: {scope}. Obtained the "
+            f"state: {state} and the "
+            f"generated authorization_url: {authorize_url}"
+        )
+        return authorization_url, state
+    else:
+        csrf_token = secrets.token_urlsafe(16)
+        state_payload = {"csrf": csrf_token, "state": state }
+        state_json = json.dumps(state_payload)
+        encoded_state = base64.urlsafe_b64encode(state_json.encode()).decode()
+        oauth = OAuth2Session(client_id, redirect_uri=redirect_uri, scope=scope)
+        authorization_url, _ = oauth.authorization_url(authorize_url, state=encoded_state)
+
+        # Logging
+        cla.log.debug(
+            f"{fn} - initialized a new oauth session "
+            f"using the github oauth client id: {client_id[0:5]}... "
+            f"with the redirect_uri: {redirect_uri}. "
+            f"using scope of: {scope}. "
+            f"CSRF token: {csrf_token}. "
+            f"custom value: {state}. "
+            f"encoded state: {encoded_state}."
+            f"Generated authorization_url: {authorization_url}"
+        )
+        return authorization_url, csrf_token
 
 
 def fetch_token(client_id, state, token_url, client_secret, code, redirect_uri=None):  # pylint: disable=too-many-arguments

utils/api_urls.md

@@ -0,0 +1,7 @@
+# V1/V2
+API_URL="https://api.easycla.lfx.linuxfoundation.org"
+API_URL="https://api.lfcla.dev.platform.linuxfoundation.org"
+
+# V3/V4
+API_URL="https://api-gw.platform.linuxfoundation.org/cla-service"
+API_URL="https://api-gw.dev.platform.linuxfoundation.org/cla-service"

utils/get_user_from_session_py.sh

@@ -1,33 +1,17 @@
 #!/bin/bash
 # API_URL=https://[xyz].ngrok-free.app (defaults to localhost:5000)
 # API_URL=https://api.lfcla.dev.platform.linuxfoundation.org
-# REDIRECT=0|1 DEBUG='' ./utils/get_user_from_session_py.sh
-# API_URL=https://api.lfcla.dev.platform.linuxfoundation.org DEBUG=1 REDIRECT=0 ./utils/get_user_from_session_py.sh 'https://contributor.easycla.lfx.linuxfoundation.org'
-# CODE=xyz STAE=xyz
-# ./utils/ngrok.sh then DEBUG='' REDIRECT=0 ./utils/get_user_from_session_py.sh 'https://edc5-147-75-85-27.ngrok-free.app/v2/user-from-session'
-# yarn serve:ext:
-# DEBUG='' REDIRECT=0 ./utils/get_user_from_session_py.sh 'http://147.75.85.27:5000/v2/user-from-session'
-# yarn serve:ext && API_URL='http://147.75.85.27:5000' DEBUG='' REDIRECT=0 ./utils/get_user_from_session_py.sh 'http://147.75.85.27:5000/v2/user-from-session'
-
-export redirect_url="${1}"
-export encoded_redirect_url=$(jq -rn --arg x "$redirect_url" '$x|@uri')
+# DEBUG='' ./utils/get_user_from_session_py.sh
+# Flow with custom GitHub app: see 'LG:' in cla/controllers/repository_service.py, then:
+# Start server via: CLA_API_BASE_CLI='http://147.75.85.27:5000' GH_OAUTH_CLIENT_ID_CLI="$(cat ../lg-github-oauth-app.client-id.secret)" GH_OAUTH_SECRET_CLI="$(cat ../lg-github-oauth-app.client-secret.secret)" yarn serve:ext
+# In the browser: open page: http://147.75.85.27:5000/v2/user-from-session
 
 if [ -z "$API_URL" ]
 then
   export API_URL="http://localhost:5000"
 fi
 
-if [ -z "${REDIRECT}" ]
-then
-  export REDIRECT="0"
-fi
-
-if ( [ -z "${CODE}" ] && [ -z "${STATE}" ] )
-then
-  export API="${API_URL}/v2/user-from-session?redirect=${REDIRECT}&redirect_url=${encoded_redirect_url}"
-else
-  export API="${API_URL}/v2/user-from-session?code=${CODE}&state=${STATE}"
-fi
+export API="${API_URL}/v2/user-from-session"
 
 if [ ! -z "$DEBUG" ]
 then