Prevent duplicated path prefix elements in generated URLs take two (#1350)

* Prevent duplicated path prefix elements in generated URLs

* add more debug info

* pure refactor

* let flask determine how to access site

* build this branch

* build image

* use more url_for, which includes a slash, and things are working locally

* fix hopefully the last missing url_for

* add some code that works for any openid provider

* one more url_for and remove more backend url config references

---------

Co-authored-by: Bret Mogilefsky <bmogilefsky@gmail.com>
Co-authored-by: burnettk <burnettk@users.noreply.github.com>

.github/workflows/docker_image_for_main_builds.yml

@@ -31,7 +31,7 @@ on:
     branches:
       - main
       - spiffdemo
-      - bugfix/data-object-management
+      - GSA-TTS-fix-path-routing-in-generated-openid-urls
 
 jobs:
   create_frontend_docker_image:

spiffworkflow-backend/bin/local_development_environment_setup

@@ -32,9 +32,13 @@ if [[ "$process_model_dir" == "acceptance" ]]; then
   export SPIFFWORKFLOW_BACKEND_LOAD_FIXTURE_DATA=true
   export SPIFFWORKFLOW_BACKEND_PERMISSIONS_FILE_NAME=acceptance_tests.yml
 elif [[ "$process_model_dir" == "localopenid" ]]; then
+  backend_base_url="$SPIFFWORKFLOW_BACKEND_URL"
+  if [[ -z "$backend_base_url" ]]; then
+    backend_base_url="http://localhost:$port"
+  fi
   export SPIFFWORKFLOW_BACKEND_AUTH_CONFIGS__0__identifier="default"
   export SPIFFWORKFLOW_BACKEND_AUTH_CONFIGS__0__label="internal openid"
-  export SPIFFWORKFLOW_BACKEND_AUTH_CONFIGS__0__uri="http://localhost:$port/openid"
+  export SPIFFWORKFLOW_BACKEND_AUTH_CONFIGS__0__uri="${backend_base_url}/openid"
   export SPIFFWORKFLOW_BACKEND_AUTH_CONFIGS__0__client_id="spiffworkflow-backend"
   export SPIFFWORKFLOW_BACKEND_AUTH_CONFIGS__0__client_secret="JXeQExm0JhQPLumgHtIIqf52bDalHz0q"
   export SPIFFWORKFLOW_BACKEND_PERMISSIONS_FILE_NAME="example.yml"

spiffworkflow-backend/src/spiffworkflow_backend/config/unit_testing.py

@@ -8,8 +8,8 @@ SPIFFWORKFLOW_BACKEND_LOG_TO_FILE = environ.get("SPIFFWORKFLOW_BACKEND_LOG_TO_FI
 
 SPIFFWORKFLOW_BACKEND_PERMISSIONS_FILE_NAME = "unit_testing.yml"
 
-SPIFFWORKFLOW_BACKEND_URL = "http://localhost:7000"
+SPIFFWORKFLOW_BACKEND_URL = "http://localhost"
-SPIFFWORKFLOW_BACKEND_OPEN_ID_SERVER_URL = "http://localhost:7000/openid"
+SPIFFWORKFLOW_BACKEND_OPEN_ID_SERVER_URL = "http://localhost/openid"
 SPIFFWORKFLOW_BACKEND_OPEN_ID_CLIENT_ID = "spiffworkflow-backend"
 SPIFFWORKFLOW_BACKEND_OPEN_ID_CLIENT_SECRET_KEY = "JXeQExm0JhQPLumgHtIIqf52bDalHz0q"  # noqa: S105
 SPIFFWORKFLOW_BACKEND_AUTH_CONFIGS = None

spiffworkflow-backend/src/spiffworkflow_backend/routes/authentication_controller.py

@@ -9,6 +9,7 @@ from flask import jsonify
 from flask import make_response
 from flask import redirect
 from flask import request
+from flask import url_for
 from werkzeug.wrappers import Response
 
 from spiffworkflow_backend.exceptions.api_error import ApiError
@@ -193,7 +194,9 @@ def login_with_access_token(access_token: str, authentication_identifier: str) -
 
 
 def login_api(authentication_identifier: str) -> Response:
-    redirect_url = "/v1.0/login_api_return"
+    host_url = request.host_url.strip("/")
+    login_return_path = url_for("/v1_0.spiffworkflow_backend_routes_authentication_controller_login_return")
+    redirect_url = f"{host_url}{login_return_path}"
     state = AuthenticationService.generate_state(redirect_url, authentication_identifier)
     login_redirect_url = AuthenticationService().get_login_redirect_url(state.decode("UTF-8"), redirect_url)
     return redirect(login_redirect_url)

spiffworkflow-backend/src/spiffworkflow_backend/routes/debug_controller.py

@@ -24,7 +24,15 @@ def version_info() -> Response:
 # that might be bad, and might require some server configuration to make sure flask knows it is running on https.
 # if using path based routing, the path will probably not be returned from this endpoint.
 def url_info() -> Response:
-    return make_response({"url": request.url, "cache": AuthenticationService.ENDPOINT_CACHE}, 200)
+    return make_response(
+        {
+            "request.root_path": request.root_path,  # type: ignore
+            "request.host_url": request.host_url,
+            "request.url": request.url,
+            "cache": AuthenticationService.ENDPOINT_CACHE,
+        },
+        200,
+    )
 
 
 def celery_backend_results(

spiffworkflow-backend/src/spiffworkflow_backend/routes/openid_blueprint/openid_blueprint.py

@@ -34,6 +34,12 @@ SPIFF_OPEN_ID_KEY_ID = "spiffworkflow_backend_open_id"
 SPIFF_OPEN_ID_ALGORITHM = "RS256"
 
 
+# just so /openid responds so we can route to it with url_for for populating issuer
+@openid_blueprint.route("", methods=["GET"])
+def index() -> Response:
+    return make_response({"ok": True}, 200)
+
+
 @openid_blueprint.route("/.well-known/openid-configuration", methods=["GET"])
 def well_known() -> dict:
     """Open ID Discovery endpoint.
@@ -42,9 +48,9 @@ def well_known() -> dict:
     """
 
     # using or instead of setting a default so we can set the env var to None in tests and this will still work
-    host_url = current_app.config.get("SPIFFWORKFLOW_BACKEND_URL") or request.host_url.strip("/")
+    host_url = _host_url_without_root_path()
     return {
-        "issuer": f"{host_url}/openid",
+        "issuer": f"{host_url}{url_for('openid.index')}",
         "authorization_endpoint": f"{host_url}{url_for('openid.auth')}",
         "token_endpoint": f"{host_url}{url_for('openid.token')}",
         "end_session_endpoint": f"{host_url}{url_for('openid.end_session')}",
@@ -55,7 +61,7 @@ def well_known() -> dict:
 @openid_blueprint.route("/auth", methods=["GET"])
 def auth() -> str:
     """Accepts a series of parameters."""
-    host_url = current_app.config.get("SPIFFWORKFLOW_BACKEND_URL") or request.host_url.strip("/")
+    host_url = _host_url_without_root_path()
     return render_template(
         "login.html",
         state=request.args.get("state"),
@@ -83,7 +89,7 @@ def form_submit() -> Any:
         url = request.values.get("redirect_uri") + "?" + urlencode(data)
         return redirect(url)
     else:
-        host_url = current_app.config.get("SPIFFWORKFLOW_BACKEND_URL") or request.host_url.strip("/")
+        host_url = _host_url_without_root_path()
         return render_template(
             "login.html",
             state=request.values.get("state"),
@@ -114,13 +120,12 @@ def token() -> Response | dict:
     authorization = base64.b64decode(authorization).decode("utf-8")
     client_id = authorization.split(":")
 
-    host_url = current_app.config.get("SPIFFWORKFLOW_BACKEND_URL", request.host_url.strip("/"))
+    host_url = _host_url_without_root_path()
-    base_url = f"{host_url}/openid"
     private_key = OpenIdConfigsForDevOnly.private_key
 
     id_token = jwt.encode(
         {
-            "iss": base_url,
+            "iss": f"{host_url}{url_for('openid.index')}",
             "aud": client_id,
             "iat": math.floor(time.time()),
             "exp": round(time.time()) + 3600,
@@ -180,3 +185,9 @@ def get_users() -> Any:
         return permission_cache["users"]
     else:
         return {}
+
+
+# if backend is being hosted at http://localhost:7000/api because SPIFFWORKFLOW_BACKEND_WSGI_PATH_PREFIX=/api,
+# this will return http://localhost:7000, because url_for will add the /api for us.
+def _host_url_without_root_path() -> str:
+    return request.host_url.strip("/")

spiffworkflow-backend/src/spiffworkflow_backend/routes/openid_blueprint/templates/login.html

@@ -5,14 +5,14 @@
     <link
       rel="stylesheet"
       type="text/css"
-      href="{{ host_url }}/{{ url_for('openid.static', filename='login.css') }}"
+      href="{{ host_url }}{{ url_for('openid.static', filename='login.css') }}"
     />
   </head>
   <body>
     <header>
       <img
         class="logo_small"
-        src="{{ host_url }}/{{ url_for('openid.static', filename='logo_small.png') }}"
+        src="{{ host_url }}{{ url_for('openid.static', filename='logo_small.png') }}"
         alt="Small SpiffWorkflow logo"
       />
     </header>
@@ -23,7 +23,7 @@
       <form
         id="login"
         method="post"
-        action="{{ host_url }}/{{ url_for('openid.form_submit') }}"
+        action="{{ host_url }}{{ url_for('openid.form_submit') }}"
       >
         <p>
           <b>Important:</b> This login form is for demonstration purposes only.

spiffworkflow-backend/src/spiffworkflow_backend/services/authentication_service.py

@@ -12,6 +12,7 @@ from typing import cast
 from cryptography.hazmat.backends import default_backend
 from cryptography.hazmat.primitives.asymmetric import rsa
 from cryptography.x509 import load_der_x509_certificate
+from flask import url_for
 
 from spiffworkflow_backend.models.user import SPIFF_GENERATED_JWT_ALGORITHM
 from spiffworkflow_backend.models.user import SPIFF_GENERATED_JWT_AUDIENCE
@@ -118,7 +119,9 @@ class AuthenticationService:
     @classmethod
     def open_id_endpoint_for_name(cls, name: str, authentication_identifier: str) -> str:
         """All openid systems provide a mapping of static names to the full path of that endpoint."""
-        openid_config_url = f"{cls.server_url(authentication_identifier)}/.well-known/openid-configuration"
+        appropriate_server_url = cls.server_url(authentication_identifier)
+        openid_config_url = f"{appropriate_server_url}/.well-known/openid-configuration"
+
         if authentication_identifier not in cls.ENDPOINT_CACHE:
             cls.ENDPOINT_CACHE[authentication_identifier] = {}
         if authentication_identifier not in cls.JSON_WEB_KEYSET_CACHE:
@@ -215,6 +218,7 @@ class AuthenticationService:
             )
         return cast(dict, parsed_token)
 
+    # returns either https://spiffworkflow.example.com or https://spiffworkflow.example.com/api
     @staticmethod
     def get_backend_url() -> str:
         return str(current_app.config["SPIFFWORKFLOW_BACKEND_URL"])
@@ -237,15 +241,19 @@ class AuthenticationService:
         )
         return state
 
-    def get_login_redirect_url(self, state: str, authentication_identifier: str, redirect_url: str = "/v1.0/login_return") -> str:
+    def get_login_redirect_url(self, state: str, authentication_identifier: str, redirect_url: str | None = None) -> str:
-        return_redirect_url = f"{self.get_backend_url()}{redirect_url}"
+        redirect_url_to_use = redirect_url
+        if redirect_url_to_use is None:
+            host_url = request.host_url.strip("/")
+            login_return_path = url_for("/v1_0.spiffworkflow_backend_routes_authentication_controller_login_return")
+            redirect_url_to_use = f"{host_url}{login_return_path}"
         login_redirect_url = (
             self.open_id_endpoint_for_name("authorization_endpoint", authentication_identifier=authentication_identifier)
             + f"?state={state}&"
             + "response_type=code&"
             + f"client_id={self.client_id(authentication_identifier)}&"
             + "scope=openid profile email&"
-            + f"redirect_uri={return_redirect_url}"
+            + f"redirect_uri={redirect_url_to_use}"
         )
         return login_redirect_url
 

spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_openid_blueprint.py

@@ -21,11 +21,13 @@ class TestOpenidBlueprint(BaseTest):
         with_db_and_bpmn_file_cleanup: None,
     ) -> None:
         """Test discovery endpoints."""
+
+        # SPIFFWORKFLOW_BACKEND_URL is set to http://localhost in unit_testing.py, but we ignore it anyway. See mock below.
         response = client.get("/openid/.well-known/openid-configuration")
         discovered_urls = response.json
-        assert "http://localhost:7000/openid" == discovered_urls["issuer"]
+        assert "http://localhost/openid" == discovered_urls["issuer"]
-        assert "http://localhost:7000/openid/auth" == discovered_urls["authorization_endpoint"]
+        assert "http://localhost/openid/auth" == discovered_urls["authorization_endpoint"]
-        assert "http://localhost:7000/openid/token" == discovered_urls["token_endpoint"]
+        assert "http://localhost/openid/token" == discovered_urls["token_endpoint"]
 
         with self.app_config_mock(app, "SPIFFWORKFLOW_BACKEND_URL", None):
             response = client.get("/openid/.well-known/openid-configuration")