Merge pull request #1966 from GSA/USN-COMPLY-49_Verify_Nonce_on_Login

Verify NONCE on login.

.github/workflows/deploy-demo.yml

@@ -63,6 +63,7 @@ jobs:
           LOGIN_DOT_GOV_BASE_LOGOUT_URL: "https://secure.login.gov/openid_connect/logout?"
           LOGIN_DOT_GOV_SIGNOUT_REDIRECT: "https://notify-demo.app.cloud.gov/sign-out"
           LOGIN_DOT_GOV_INITIAL_SIGNIN_URL: "https://secure.login.gov/openid_connect/authorize?acr_values=http%3A%2F%2Fidmanagement.gov%2Fns%2Fassurance%2Fial%2F1&client_id=urn:gov:gsa:openidconnect.profiles:sp:sso:gsa:notify-gov&nonce=NONCE&prompt=select_account&redirect_uri=https://notify-demo.app.cloud.gov/sign-in&response_type=code&scope=openid+email&state=STATE"
+          LOGIN_DOT_GOV_CERTS_URL: "https://secure.login.gov/api/openid_connect/certs"
         with:
           cf_username: ${{ secrets.CLOUDGOV_USERNAME }}
           cf_password: ${{ secrets.CLOUDGOV_PASSWORD }}
@@ -85,6 +86,7 @@ jobs:
             --var LOGIN_DOT_GOV_BASE_LOGOUT_URL="$LOGIN_DOT_GOV_BASE_LOGOUT_URL"
             --var LOGIN_DOT_GOV_SIGNOUT_REDIRECT="$LOGIN_DOT_GOV_SIGNOUT_REDIRECT"
             --var LOGIN_DOT_GOV_INITIAL_SIGNIN_URL="$LOGIN_DOT_GOV_INITIAL_SIGNIN_URL"
+            --var LOGIN_DOT_GOV_CERTS_URL="$LOGIN_DOT_GOV_CERTS_URL"
 
       - name: Check for changes to egress config
         id: changed-egress-config

.github/workflows/deploy-prod.yml

@@ -63,6 +63,7 @@ jobs:
           LOGIN_DOT_GOV_BASE_LOGOUT_URL: "https://secure.login.gov/openid_connect/logout?"
           LOGIN_DOT_GOV_SIGNOUT_REDIRECT: "https://beta.notify.gov/sign-out"
           LOGIN_DOT_GOV_INITIAL_SIGNIN_URL: "https://secure.login.gov/openid_connect/authorize?acr_values=http%3A%2F%2Fidmanagement.gov%2Fns%2Fassurance%2Fial%2F1&client_id=urn:gov:gsa:openidconnect.profiles:sp:sso:gsa:notify-gov&nonce=NONCE&prompt=select_account&redirect_uri=https://beta.notify.gov/sign-in&response_type=code&scope=openid+email&state=STATE"
+          LOGIN_DOT_GOV_CERTS_URL: "https://secure.login.gov/api/openid_connect/certs"
         with:
           cf_username: ${{ secrets.CLOUDGOV_USERNAME }}
           cf_password: ${{ secrets.CLOUDGOV_PASSWORD }}
@@ -85,6 +86,7 @@ jobs:
             --var LOGIN_DOT_GOV_BASE_LOGOUT_URL="$LOGIN_DOT_GOV_BASE_LOGOUT_URL"
             --var LOGIN_DOT_GOV_SIGNOUT_REDIRECT="$LOGIN_DOT_GOV_SIGNOUT_REDIRECT"
             --var LOGIN_DOT_GOV_INITIAL_SIGNIN_URL="$LOGIN_DOT_GOV_INITIAL_SIGNIN_URL"
+            --var LOGIN_DOT_GOV_CERTS_URL="$LOGIN_DOT_GOV_CERTS_URL"
 
       - name: Check for changes to egress config
         id: changed-egress-config

.github/workflows/deploy.yml

@@ -69,6 +69,7 @@ jobs:
         LOGIN_DOT_GOV_BASE_LOGOUT_URL: "https://secure.login.gov/openid_connect/logout?"
         LOGIN_DOT_GOV_SIGNOUT_REDIRECT: "https://notify-staging.app.cloud.gov/sign-out"
         LOGIN_DOT_GOV_INITIAL_SIGNIN_URL: "https://secure.login.gov/openid_connect/authorize?acr_values=http%3A%2F%2Fidmanagement.gov%2Fns%2Fassurance%2Fial%2F1&client_id=urn:gov:gsa:openidconnect.profiles:sp:sso:gsa:notify-gov&nonce=NONCE&prompt=select_account&redirect_uri=https://notify-staging.app.cloud.gov/sign-in&response_type=code&scope=openid+email&state=STATEE"
+        LOGIN_DOT_GOV_CERTS_URL: "https://secure.login.gov/api/openid_connect/certs"
       with:
         cf_username: ${{ secrets.CLOUDGOV_USERNAME }}
         cf_password: ${{ secrets.CLOUDGOV_PASSWORD }}
@@ -91,6 +92,7 @@ jobs:
           --var LOGIN_DOT_GOV_BASE_LOGOUT_URL="$LOGIN_DOT_GOV_BASE_LOGOUT_URL"
           --var LOGIN_DOT_GOV_SIGNOUT_REDIRECT="$LOGIN_DOT_GOV_SIGNOUT_REDIRECT"
           --var LOGIN_DOT_GOV_INITIAL_SIGNIN_URL="$LOGIN_DOT_GOV_INITIAL_SIGNIN_URL"
+          --var LOGIN_DOT_GOV_CERTS_URL="$LOGIN_DOT_GOV_CERTS_URL"
 
 
     - name: Check for changes to egress config

app/main/views/index.py

@@ -1,6 +1,15 @@
 import os
+import secrets
 
-from flask import abort, current_app, redirect, render_template, request, url_for
+from flask import (
+    abort,
+    current_app,
+    redirect,
+    render_template,
+    request,
+    session,
+    url_for,
+)
 from flask_login import current_user
 
 from app import status_api_client
@@ -23,8 +32,12 @@ def index():
     )
     url = os.getenv("LOGIN_DOT_GOV_INITIAL_SIGNIN_URL")
     # handle unit tests
+
+    nonce = secrets.token_urlsafe()
+    session["nonce"] = nonce
+
     if url is not None:
-        url = url.replace("NONCE", token)
+        url = url.replace("NONCE", nonce)
         url = url.replace("STATE", token)
     return render_template(
         "views/signedout.html",

app/main/views/sign_in.py

@@ -1,4 +1,6 @@
+import json
 import os
+import secrets
 import time
 import uuid
 
@@ -12,6 +14,7 @@ from flask import (
     redirect,
     render_template,
     request,
+    session,
     url_for,
 )
 from flask_login import current_user
@@ -28,7 +31,7 @@ from app.utils.user import is_gov_user
 from notifications_utils.url_safe_token import generate_token
 
 
-def _reformat_keystring(orig):
+def _reformat_keystring(orig):  # pragma: no cover
     arr = orig.split("-----")
     begin = arr[1]
     end = arr[3]
@@ -37,9 +40,10 @@ def _reformat_keystring(orig):
     return new_keystring
 
 
-def _get_access_token(code, state):
+def _get_access_token(code, state):  # pragma: no cover
     client_id = os.getenv("LOGIN_DOT_GOV_CLIENT_ID")
     access_token_url = os.getenv("LOGIN_DOT_GOV_ACCESS_TOKEN_URL")
+    certs_url = os.getenv("LOGIN_DOT_GOV_CERTS_URL")
     keystring = os.getenv("LOGIN_PEM")
     if " " in keystring:
         keystring = _reformat_keystring(keystring)
@@ -60,17 +64,48 @@ def _get_access_token(code, state):
     url = f"{base_url}{cli_assert}&{cli_assert_type}&{code_param}&grant_type=authorization_code"
     headers = {"Authorization": "Bearer %s" % token}
     response = requests.post(url, headers=headers)
-    if response.json().get("access_token") is None:
+    response_json = response.json()
+    try:
+        encoded_id_token = response_json["id_token"]
+    except KeyError as e:
         # Capture the response json here so it hopefully shows up in error reports
-        current_app.logger.error(
+        current_app.logger.exception(f"Error when getting id token {response_json}")
+        raise KeyError(f"'access_token' {response.json()}") from e
+
+    # Getting Login.gov signing keys for unpacking the id_token correctly.
+    jwks = requests.get(certs_url).json()
+    public_keys = {
+        jwk["kid"]: {
+            "key": jwt.algorithms.RSAAlgorithm.from_jwk(json.dumps(jwk)),
+            "algo": jwk["alg"],
+        }
+        for jwk in jwks["keys"]
+    }
+    kid = jwt.get_unverified_header(encoded_id_token)["kid"]
+    pub_key = public_keys[kid]["key"]
+    algo = public_keys[kid]["algo"]
+    id_token = jwt.decode(
+        encoded_id_token, pub_key, audience=client_id, algorithms=[algo]
+    )
+
+    nonce = id_token["nonce"]
+    saved_nonce = session.pop("nonce")
+    if nonce != saved_nonce:
+        current_app.logger.error(f"Nonce Error: {nonce} != {saved_nonce}")
+        abort(403)
+
+    try:
+        access_token = response_json["access_token"]
+    except KeyError as e:
+        # Capture the response json here so it hopefully shows up in error reports
+        current_app.logger.exception(
             f"Error when getting access token {response.json()} #notify-admin-1505"
         )
-        raise KeyError(f"'access_token' {response.json()}")
+        raise KeyError(f"'access_token' {response.json()}") from e
-    access_token = response.json()["access_token"]
     return access_token
 
 
-def _get_user_email_and_uuid(access_token):
+def _get_user_email_and_uuid(access_token):  # pragma: no cover
     headers = {"Authorization": "Bearer %s" % access_token}
     user_info_url = os.getenv("LOGIN_DOT_GOV_USER_INFO_URL")
     user_attributes = requests.get(
@@ -82,7 +117,7 @@ def _get_user_email_and_uuid(access_token):
     return user_email, user_uuid
 
 
-def _do_login_dot_gov():
+def _do_login_dot_gov():  # $ pragma: no cover
     # start login.gov
     code = request.args.get("code")
     state = request.args.get("state")
@@ -122,14 +157,14 @@ def _do_login_dot_gov():
             current_app.logger.info(f"activating user {usr.id} #notify-admin-1505")
             activate_user(usr.id)
         except BaseException as be:  # noqa B036
-            current_app.logger.error(f"Error signing in: {be} #notify-admin-1505 ")
+            current_app.logger.exception(f"Error signing in: {be} #notify-admin-1505 ")
             error(401)
         return redirect(url_for("main.show_accounts_or_dashboard", next=redirect_url))
 
     # end login.gov
 
 
-def verify_email(user, redirect_url):
+def verify_email(user, redirect_url):  # pragma: no cover
     user_api_client.send_verify_code(user["id"], "email", None, redirect_url)
     title = "Email resent" if request.args.get("email_resent") else "Check your email"
     redirect_url = request.args.get("next")
@@ -138,7 +173,7 @@ def verify_email(user, redirect_url):
     )
 
 
-def _handle_e2e_tests(redirect_url):
+def _handle_e2e_tests(redirect_url):  # pragma: no cover
     try:
         current_app.logger.warning("E2E TESTS ARE ENABLED.")
         current_app.logger.warning(
@@ -161,7 +196,7 @@ def _handle_e2e_tests(redirect_url):
 
 @main.route("/sign-in", methods=(["GET", "POST"]))
 @hide_from_search_engines
-def sign_in():
+def sign_in():  # pragma: no cover
     redirect_url = request.args.get("next")
 
     if os.getenv("NOTIFY_E2E_TEST_EMAIL"):
@@ -189,10 +224,15 @@ def sign_in():
         current_app.config["DANGEROUS_SALT"],
     )
     url = os.getenv("LOGIN_DOT_GOV_INITIAL_SIGNIN_URL")
+
+    nonce = secrets.token_urlsafe()
+    session["nonce"] = nonce
+
     # handle unit tests
     if url is not None:
-        url = url.replace("NONCE", token)
+        url = url.replace("NONCE", nonce)
         url = url.replace("STATE", token)
+
     return render_template(
         "views/signin.html",
         again=bool(redirect_url),
@@ -201,5 +241,5 @@ def sign_in():
 
 
 @login_manager.unauthorized_handler
-def sign_in_again():
+def sign_in_again():  # pragma: no cover
     return redirect(url_for("main.sign_in", next=request.path))

manifest.yml

@@ -59,3 +59,4 @@ applications:
       LOGIN_DOT_GOV_BASE_LOGOUT_URL: ((LOGIN_DOT_GOV_BASE_LOGOUT_URL))
       LOGIN_DOT_GOV_SIGNOUT_REDIRECT: ((LOGIN_DOT_GOV_SIGNOUT_REDIRECT))
       LOGIN_DOT_GOV_INITIAL_SIGNIN_URL: ((LOGIN_DOT_GOV_INITIAL_SIGNIN_URL))
+      LOGIN_DOT_GOV_CERTS_URL: ((LOGIN_DOT_GOV_CERTS_URL))

sample.env

@@ -43,3 +43,4 @@ LOGIN_DOT_GOV_LOGOUT_URL="https://idp.int.identitysandbox.gov/openid_connect/log
 LOGIN_DOT_GOV_BASE_LOGOUT_URL="https://idp.int.identitysandbox.gov/openid_connect/logout?"
 LOGIN_DOT_GOV_SIGNOUT_REDIRECT="http://localhost:6012/sign-out"
 LOGIN_DOT_GOV_INITIAL_SIGNIN_URL="https://idp.int.identitysandbox.gov/openid_connect/authorize?acr_values=http%3A%2F%2Fidmanagement.gov%2Fns%2Fassurance%2Fial%2F1&client_id=urn:gov:gsa:openidconnect.profiles:sp:sso:gsa:test_notify_gov&nonce=NONCE&prompt=select_account&redirect_uri=http://localhost:6012/sign-in&response_type=code&scope=openid+email&state=STATE"
+LOGIN_DOT_GOV_CERTS_URL = "https://idp.int.identitysandbox.gov/api/openid_connect/certs"