add nonce for authz, and minor bugfix with trailing slash

dowjones · Dec 18, 2023 · 21f20f4 · 21f20f4
1 parent f39ce92
commit 21f20f4
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 14 deletions.
diff --git a/tests/unit/test_okta.py b/tests/unit/test_okta.py
@@ -543,7 +543,7 @@ def test_create_authz_cookies():
     """Test create_authz_cookies."""
     from tokendito import okta
 
-    pytest_oauth2_session_data = {"state": "pyteststate"}
+    pytest_oauth2_session_data = {"state": "pyteststate", "nonce": "pytestnonce"}
 
     pytest_oauth2_config = {
         "client_id": "123",
@@ -637,6 +637,9 @@ def test_get_authorize_code():
     response.url = "https://example.com?code=pytest"
     assert okta.get_authorize_code(response, "sessionToken") == "pytest"
 
+    response.url = "https//example.com?error=login_required"
+    assert okta.get_authorize_code(response, None) is None
+
 
 def test_authorization_code_enabled():
     """Test authorization_code_enabled."""
@@ -687,12 +690,33 @@ def test_authorize_request(mocker):
     assert okta.authorize_request(pytest_oauth2_config, pytest_oauth2_session_data) == "pytest"
 
 
-def test_generate_oauth2_session_data():
-    """Test generate_oauth2_session_data."""
+def test_get_nonce(mocker):
+    """Test get_nonce."""
+    from tokendito import okta
+
+    response = Mock()
+    response.text = """
+    <html>
+    <script nonce="PYTEST_NONCE" type="text/javascript">'
+    </html>
+    """
+    mocker.patch.object(HTTP_client, "get", return_value=response)
+
+    assert okta.get_nonce("https://acme.com") == "PYTEST_NONCE"
+
+    response.text = "nonce-non-present"
+    mocker.patch.object(HTTP_client, "get", return_value=response)
+    assert okta.get_nonce("https://acme.com") is None
+
+
+def test_get_oauth2_session_data(mocker):
+    """Test get_oauth2_session_data."""
     from tokendito import okta
 
+    mocker.patch("tokendito.okta.get_nonce", return_value="ABC")
     assert (
-        okta.generate_oauth2_session_data("acme.com")["redirect_uri"] == "acme.com/enduser/callback"
+        okta.get_oauth2_session_data("https://acme.com")["redirect_uri"]
+        == "https://acme.com/enduser/callback"
     )
 
 
@@ -883,7 +907,7 @@ def test_access_control(mocker):
 
     mocker.patch("tokendito.okta.oie_enabled", return_value=True)
     mocker.patch("tokendito.okta.get_oauth2_configuration", return_value=None)
-    mocker.patch("tokendito.okta.generate_oauth2_session_data", return_value=None)
+    mocker.patch("tokendito.okta.get_oauth2_session_data", return_value=None)
     mocker.patch("tokendito.okta.create_authz_cookies", return_value=None)
     mocker.patch("tokendito.okta.idp_authenticate", return_value=None)
     mocker.patch("tokendito.okta.idp_authorize", return_value=None)

diff --git a/tokendito/okta.py b/tokendito/okta.py
@@ -17,8 +17,7 @@
 import re
 import sys
 import time
-from urllib.parse import urlencode
-from urllib.parse import urlparse
+import urllib
 
 import bs4
 from bs4 import BeautifulSoup
@@ -208,16 +207,17 @@ def create_authz_cookies(oauth2_config, oauth2_session_data):
             "userInfoUrl": f"{oauth2_url}/userinfo",
             "revokeUrl": f"{oauth2_url}/revoke",
             "logoutUrl": f"{oauth2_url}/logout",
+            "nonce": oauth2_session_data["nonce"],
         }
     except KeyError as e:
         logger.error(f"Missing key in config:{e}")
         sys.exit(1)
 
     cookiejar = requests.cookies.RequestsCookieJar()
-    domain = urlparse(oauth2_config["org"]).netloc
+    domain = urllib.parse.urlparse(oauth2_config["org"]).netloc
     cookiejar.set(
         "okta-oauth-redirect-params",
-        f"{{{urlencode(oauth2_config_reformatted)}}}",
+        f"{{{urllib.parse.urlencode(oauth2_config_reformatted)}}}",
         domain=domain,
         path="/",
     )
@@ -559,9 +559,22 @@ def authorize_request(oauth2_config, oauth2_session_data):
     return authorize_code
 
 
-def generate_oauth2_session_data(url):
+def get_nonce(url):
+    """Get nonce from the org server."""
+    userhome_url = f"{url}/app/UserHome"
+    payload = {"session_hint": "AUTHENTICATED", "iss": urllib.parse.quote(userhome_url, safe="")}
+    response = HTTP_client.get(url, params=payload)
+    # '<script nonce="ABCXXXXXXXXXXXXXXXXYZ" type="text/javascript">'
+    nonce = re.search(r"<script nonce=\"(.*)\"\ ", response.text)
+    if nonce:
+        return nonce.group(1)
+    else:
+        return None
+
+
+def get_oauth2_session_data(url):
     """
-    Generate some oauth2 session data.
+    Get some oauth2 session data.
 
     We do this to have the same in oath2 cookies and /authorize call.
     """
@@ -572,6 +585,8 @@ def generate_oauth2_session_data(url):
         "redirect_uri": get_redirect_uri(url),
         "grant_type": "authorization_code",
     }
+    authz_session_data["nonce"] = get_nonce(url)
+
     if pkce_enabled():
         code_verifier = get_pkce_code_verifier()
         authz_session_data["code_verifier"] = code_verifier
@@ -656,8 +671,8 @@ def create_authn_cookies(authn_org_url, session_token):
     user.add_sensitive_value_to_be_masked(session_id)
 
     cookiejar = requests.cookies.RequestsCookieJar()
-    domain = urlparse(url).netloc
-    cookiejar.set("sid", session_id, domain=urlparse(url).netloc, path="/")
+    domain = urllib.parse.urlparse(url).netloc
+    cookiejar.set("sid", session_id, domain=domain, path="/")
     cookiejar.set("sessionToken", session_token, domain=domain, path="/")
     HTTP_client.add_cookies(cookiejar)  # add cookies
 
@@ -714,7 +729,7 @@ def access_control(config):
         logger.debug("OIE enabled")
         # save some oauth2 config data + create session data, and create authz cookies
         oauth2_config = get_oauth2_configuration(config)
-        oauth2_session_data = generate_oauth2_session_data(config.okta["org"])
+        oauth2_session_data = get_oauth2_session_data(config.okta["org"])
         create_authz_cookies(oauth2_config, oauth2_session_data)
         # The flow says to initially call /authorize here, but that doesnt do anything...
         # idp_authorize(oauth2_config, oauth2_session_data)

diff --git a/tokendito/user.py b/tokendito/user.py
@@ -64,6 +64,8 @@ def cmd_interface(args):
         )
         sys.exit(1)
 
+    config.okta["org"] = config.okta["org"].strip("/")  # rm trailing / if provided as such
+    # so the urls with this as base dont have //
     if config.user["use_device_token"]:
         device_token = config.okta["device_token"]
         if device_token: