Skip to content

Commit 8cb44d6

Browse files
bajnokkbajnokk
committed
amend! fixup! ModuleRouter: support paths in BASE
If Satosa is installed under a path which is not the root of the webserver (ie. "https://example.com/satosa"), then endpoint routing must take the base path into consideration. Some modules registered some of their endpoints with the base path included, but other times the base path was omitted, thus it made the routing fail. Now all endpoint registrations include the base path in their endpoint map. Provide a simple implementation for joining path components, since we don't want to add the separator for empty strings and when any of the path components already have it. Additionally, DEBUG logging was configured for the tests so that the debug logs are accessible during testing.
1 parent fc9374c commit 8cb44d6

File tree

11 files changed

+98
-32
lines changed

11 files changed

+98
-32
lines changed

‎doc/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ bind_password: !ENVFILE LDAP_BIND_PASSWORD_FILE
7878

7979
| Parameter name | Data type | Example value | Description |
8080
| -------------- | --------- | ------------- | ----------- |
81-
| `BASE` | string | `https://proxy.example.com` | The base url of the proxy. For the OIDC Frontend, this is used to set the issuer as well, and due to implementation constraints, avoid using trailing slashes in this case. |
81+
| `BASE` | string | `https://proxy.example.com` | base url of the proxy |
8282
| `COOKIE_STATE_NAME` | string | `satosa_state` | name of the cookie SATOSA uses for preserving state between requests |
8383
| `CONTEXT_STATE_DELETE` | bool | `True` | controls whether SATOSA will delete the state cookie after receiving the authentication response from the upstream IdP|
8484
| `STATE_ENCRYPTION_KEY` | string | `52fddd3528a44157` | key used for encrypting the state cookie, will be overridden by the environment variable `SATOSA_STATE_ENCRYPTION_KEY` if it is set |

‎src/satosa/frontends/base.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
Holds a base class for frontend modules used in the SATOSA proxy.
33
"""
44
from ..attribute_mapping import AttributeMapper
5+
from ..util import join_paths
56

6-
import os.path
77
from urllib.parse import urlparse
88

99

@@ -31,7 +31,7 @@ def __init__(self, auth_req_callback_func, internal_attributes, base_url, name):
3131
self.converter = AttributeMapper(internal_attributes)
3232
self.base_url = base_url or ""
3333
self.name = name
34-
self.endpoint_baseurl = os.path.join(self.base_url, self.name)
34+
self.endpoint_baseurl = join_paths(self.base_url, self.name)
3535
self.endpoint_basepath = urlparse(self.endpoint_baseurl).path.lstrip("/")
3636

3737
def handle_authn_response(self, context, internal_resp):

‎src/satosa/frontends/openid_connect.py‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
import json
66
import logging
7-
import os.path
87
from collections import defaultdict
98
from urllib.parse import urlencode, urlparse
109

@@ -38,7 +37,7 @@
3837
from ..response import BadRequest, Created
3938
from ..response import SeeOther, Response
4039
from ..response import Unauthorized
41-
from ..util import rndstr
40+
from ..util import join_paths, rndstr
4241

4342
import satosa.logging_util as lu
4443
from satosa.internal import InternalData
@@ -174,13 +173,14 @@ def register_endpoints(self, backend_names):
174173
:raise ValueError: if more than one backend is configured
175174
"""
176175
# See https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfig
177-
# Unfortunately since the issuer is always `base_url` for all OIDC frontend instances,
178-
# the discovery endpoint will be the same for every instance.
179-
# This means that only one frontend will be usable for autodiscovery.
176+
#
177+
# We skip the scheme + host + port of the issuer URL, because we can only map the
178+
# path for the provider config endpoint. We are safe to use urlparse().path here,
179+
# because for issuer OIDC allows only https URLs without query and fragment parts.
180+
issuer = self.provider.configuration_information["issuer"]
180181
autoconf_path = ".well-known/openid-configuration"
181-
base_path = urlparse(self.base_url).path.lstrip("/")
182182
provider_config = (
183-
"^{}$".format(os.path.join(base_path, autoconf_path)),
183+
"^{}$".format(join_paths(urlparse(issuer).path.lstrip("/"), autoconf_path)),
184184
self.provider_config,
185185
)
186186
jwks_uri = ("^{}/jwks$".format(self.endpoint_basepath), self.jwks)
@@ -203,7 +203,7 @@ def register_endpoints(self, backend_names):
203203

204204
if backend_name:
205205
# if there is only one backend, include its name in the path so the default routing can work
206-
auth_endpoint = os.path.join(
206+
auth_endpoint = join_paths(
207207
self.base_url,
208208
backend_name,
209209
self.name,
@@ -212,28 +212,28 @@ def register_endpoints(self, backend_names):
212212
self.provider.configuration_information["authorization_endpoint"] = auth_endpoint
213213
auth_path = urlparse(auth_endpoint).path.lstrip("/")
214214
else:
215-
auth_path = os.path.join(self.endpoint_basepath, AuthorizationRequest.url)
215+
auth_path = join_paths(self.endpoint_basepath, AuthorizationRequest.url)
216216

217217
authentication = ("^{}$".format(auth_path), self.handle_authn_request)
218218
url_map = [provider_config, jwks_uri, authentication]
219219

220220
if any("code" in v for v in self.provider.configuration_information["response_types_supported"]):
221-
self.provider.configuration_information["token_endpoint"] = os.path.join(
221+
self.provider.configuration_information["token_endpoint"] = join_paths(
222222
self.endpoint_baseurl,
223223
TokenEndpoint.url,
224224
)
225225
token_endpoint = (
226-
"^{}".format(os.path.join(self.endpoint_basepath, TokenEndpoint.url)),
226+
"^{}".format(join_paths(self.endpoint_basepath, TokenEndpoint.url)),
227227
self.token_endpoint,
228228
)
229229
url_map.append(token_endpoint)
230230

231231
self.provider.configuration_information["userinfo_endpoint"] = (
232-
os.path.join(self.endpoint_baseurl, UserinfoEndpoint.url)
232+
join_paths(self.endpoint_baseurl, UserinfoEndpoint.url)
233233
)
234234
userinfo_endpoint = (
235235
"^{}".format(
236-
os.path.join(self.endpoint_basepath, UserinfoEndpoint.url)
236+
join_paths(self.endpoint_basepath, UserinfoEndpoint.url)
237237
),
238238
self.userinfo_endpoint,
239239
)
@@ -242,7 +242,7 @@ def register_endpoints(self, backend_names):
242242
if "registration_endpoint" in self.provider.configuration_information:
243243
client_registration = (
244244
"^{}".format(
245-
os.path.join(self.endpoint_basepath, RegistrationEndpoint.url)
245+
join_paths(self.endpoint_basepath, RegistrationEndpoint.url)
246246
),
247247
self.client_registration,
248248
)

‎src/satosa/frontends/ping.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import logging
2-
import os.path
32

43
import satosa.logging_util as lu
54
from satosa.frontends.base import FrontendModule
65
from satosa.response import Response
6+
from satosa.util import join_paths
77

88

99
logger = logging.getLogger(__name__)
@@ -44,7 +44,7 @@ def register_endpoints(self, backend_names):
4444
:rtype: list[(str, ((satosa.context.Context, Any) -> satosa.response.Response, Any))]
4545
:raise ValueError: if more than one backend is configured
4646
"""
47-
url_map = [("^{}".format(os.path.join(self.endpoint_basepath, self.name)), self.ping_endpoint)]
47+
url_map = [("^{}".format(join_paths(self.endpoint_basepath, self.name)), self.ping_endpoint)]
4848

4949
return url_map
5050

‎src/satosa/micro_services/account_linking.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
"""
44
import json
55
import logging
6-
import os.path
76

87
import requests
98
from jwkest.jwk import rsa_load, RSAKey
@@ -13,6 +12,7 @@
1312
from ..exception import SATOSAAuthenticationError
1413
from ..micro_services.base import ResponseMicroService
1514
from ..response import Redirect
15+
from ..util import join_paths
1616

1717
import satosa.logging_util as lu
1818
logger = logging.getLogger(__name__)
@@ -165,8 +165,8 @@ def register_endpoints(self):
165165
return [
166166
(
167167
"^{}$".format(
168-
os.path.join(
169-
self.base_path, "account_linking", self.endpoint.lstrip("/")
168+
join_paths(
169+
self.base_path, "account_linking", self.endpoint
170170
)
171171
),
172172
self._handle_al_response,

‎src/satosa/micro_services/consent.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
import hashlib
55
import json
66
import logging
7-
import os.path
87
from base64 import urlsafe_b64encode
98

109
import requests
@@ -17,6 +16,7 @@
1716
from satosa.internal import InternalData
1817
from satosa.micro_services.base import ResponseMicroService
1918
from satosa.response import Redirect
19+
from satosa.util import join_paths
2020

2121

2222
logger = logging.getLogger(__name__)
@@ -242,8 +242,8 @@ def register_endpoints(self):
242242
return [
243243
(
244244
"^{}$".format(
245-
os.path.join(
246-
self.base_path, "consent", self.endpoint.lstrip("/")
245+
join_paths(
246+
self.base_path, "consent", self.endpoint
247247
)
248248
),
249249
self._handle_consent_response,

‎src/satosa/util.py‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,29 @@ def rndstr(size=16, alphabet=""):
8989
if not alphabet:
9090
alphabet = string.ascii_letters[0:52] + string.digits
9191
return type(alphabet)().join(rng.choice(alphabet) for _ in range(size))
92+
93+
94+
def join_paths(base, *paths):
95+
"""
96+
Joins strings with a "/" separator, like they were path components, but
97+
tries to avoid adding an unnecessary separator. Note that the contents of
98+
the strings are not sanitized in any way. If any of the components begins or
99+
ends with a "/", the separator is not inserted, and any number of empty
100+
strings at the beginning would not add a leading slash. Any number of empty
101+
strings at the end only add a single trailing slash.
102+
103+
Raises TypeError if any of the components are not strings.
104+
"""
105+
sep = "/"
106+
107+
path = base
108+
try:
109+
for p in paths:
110+
if not path or path.endswith(sep) or p.startswith(sep):
111+
path += p
112+
else:
113+
path += sep + p
114+
except (AttributeError, TypeError) as err:
115+
raise TypeError("Arguments must be strings") from err
116+
117+
return path

‎tests/flows/test_account_linking.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
import os.path
21
import responses
32
from urllib.parse import urlparse
43
from werkzeug.test import Client
54
from werkzeug.wrappers import Response
65

76
from satosa.proxy_server import make_app
87
from satosa.satosa_config import SATOSAConfig
8+
from satosa.util import join_paths
99

1010

1111
class TestAccountLinking:
@@ -15,7 +15,7 @@ def test_full_flow(self, satosa_config_dict, account_linking_module_config):
1515
account_linking_module_config["config"]["api_url"] = api_url
1616
account_linking_module_config["config"]["redirect_url"] = redirect_url
1717
satosa_config_dict["MICRO_SERVICES"].insert(0, account_linking_module_config)
18-
base_path = urlparse(satosa_config_dict["BASE"]).path.lstrip("\n")
18+
base_path = urlparse(satosa_config_dict["BASE"]).path.lstrip("\n/")
1919

2020
# application
2121
test_client = Client(make_app(SATOSAConfig(satosa_config_dict)), Response)
@@ -39,5 +39,5 @@ def test_full_flow(self, satosa_config_dict, account_linking_module_config):
3939
rsps.add(responses.GET, "{}/get_id".format(api_url), "test_userid", status=200)
4040

4141
# incoming account linking response
42-
http_resp = test_client.get(os.path.join("/", base_path, "account_linking/handle_account_linking"))
42+
http_resp = test_client.get(join_paths("/", base_path, "account_linking/handle_account_linking"))
4343
assert http_resp.status_code == 200

‎tests/flows/test_consent.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import json
2-
import os.path
32
import re
43

54
import responses
@@ -9,6 +8,7 @@
98

109
from satosa.proxy_server import make_app
1110
from satosa.satosa_config import SATOSAConfig
11+
from satosa.util import join_paths
1212

1313

1414
class TestConsent:
@@ -18,7 +18,7 @@ def test_full_flow(self, satosa_config_dict, consent_module_config):
1818
consent_module_config["config"]["api_url"] = api_url
1919
consent_module_config["config"]["redirect_url"] = redirect_url
2020
satosa_config_dict["MICRO_SERVICES"].append(consent_module_config)
21-
base_path = urlparse(satosa_config_dict["BASE"]).path.lstrip("\n")
21+
base_path = urlparse(satosa_config_dict["BASE"]).path.lstrip("\n/")
2222

2323
# application
2424
test_client = Client(make_app(SATOSAConfig(satosa_config_dict)), Response)
@@ -46,5 +46,5 @@ def test_full_flow(self, satosa_config_dict, consent_module_config):
4646
rsps.add(responses.GET, verify_url_re, json.dumps({"foo": "bar"}), status=200)
4747

4848
# incoming consent response
49-
http_resp = test_client.get(os.path.join("/", base_path, "consent/handle_consent"))
49+
http_resp = test_client.get(join_paths("/", base_path, "consent/handle_consent"))
5050
assert http_resp.status_code == 200

‎tests/flows/test_oidc-saml.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from satosa.metadata_creation.saml_metadata import create_entity_descriptors
1818
from satosa.proxy_server import make_app
1919
from satosa.satosa_config import SATOSAConfig
20+
from satosa.util import join_paths
2021
from tests.users import USERS
2122
from tests.users import OIDC_USERS
2223
from tests.util import FakeIdP
@@ -94,7 +95,7 @@ def _client_setup(self):
9495

9596
def _discover_provider(self, client, provider):
9697
discovery_path = (
97-
os.path.join(urlparse(provider).path, ".well-known/openid-configuration")
98+
join_paths(urlparse(provider).path, ".well-known/openid-configuration")
9899
)
99100
return json.loads(client.get(discovery_path).data.decode("utf-8"))
100101

0 commit comments

Comments
 (0)