Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Refactor OIDC tests to better mimic an actual OIDC provider. (#13910)
Browse files Browse the repository at this point in the history
This implements a fake OIDC server, which intercepts calls to the HTTP client.
Improves accuracy of tests by covering more internal methods.

One particular example was the ID token validation, which previously mocked.

This uncovered an incorrect dependency: Synapse actually requires at least
authlib 0.15.1, not 0.14.0.
  • Loading branch information
sandhose authored Oct 25, 2022
1 parent 2d0ba3f commit 9192d74
Show file tree
Hide file tree
Showing 10 changed files with 747 additions and 460 deletions.
1 change: 1 addition & 0 deletions changelog.d/13910.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Refactor OIDC tests to better mimic an actual OIDC provider.
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ psycopg2 = { version = ">=2.8", markers = "platform_python_implementation != 'Py
psycopg2cffi = { version = ">=2.8", markers = "platform_python_implementation == 'PyPy'", optional = true }
psycopg2cffi-compat = { version = "==1.1", markers = "platform_python_implementation == 'PyPy'", optional = true }
pysaml2 = { version = ">=4.5.0", optional = true }
authlib = { version = ">=0.14.0", optional = true }
authlib = { version = ">=0.15.1", optional = true }
# systemd-python is necessary for logging to the systemd journal via
# `systemd.journal.JournalHandler`, as is documented in
# `contrib/systemd/log_config.yaml`.
Expand Down
15 changes: 11 additions & 4 deletions synapse/handlers/oidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ def __init__(
provider: OidcProviderConfig,
):
self._store = hs.get_datastores().main
self._clock = hs.get_clock()

self._macaroon_generaton = macaroon_generator

Expand Down Expand Up @@ -673,6 +674,13 @@ async def _parse_id_token(self, token: Token, nonce: str) -> CodeIDToken:
Returns:
The decoded claims in the ID token.
"""
id_token = token.get("id_token")
logger.debug("Attempting to decode JWT id_token %r", id_token)

# That has been theoritically been checked by the caller, so even though
# assertion are not enabled in production, it is mainly here to appease mypy
assert id_token is not None

metadata = await self.load_metadata()
claims_params = {
"nonce": nonce,
Expand All @@ -688,9 +696,6 @@ async def _parse_id_token(self, token: Token, nonce: str) -> CodeIDToken:

claim_options = {"iss": {"values": [metadata["issuer"]]}}

id_token = token["id_token"]
logger.debug("Attempting to decode JWT id_token %r", id_token)

# Try to decode the keys in cache first, then retry by forcing the keys
# to be reloaded
jwk_set = await self.load_jwks()
Expand All @@ -715,7 +720,9 @@ async def _parse_id_token(self, token: Token, nonce: str) -> CodeIDToken:

logger.debug("Decoded id_token JWT %r; validating", claims)

claims.validate(leeway=120) # allows 2 min of clock skew
claims.validate(
now=self._clock.time(), leeway=120
) # allows 2 min of clock skew

return claims

Expand Down
36 changes: 7 additions & 29 deletions tests/federation/test_federation_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,20 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import json
from unittest import mock

import twisted.web.client
from twisted.internet import defer
from twisted.internet.protocol import Protocol
from twisted.python.failure import Failure
from twisted.test.proto_helpers import MemoryReactor

from synapse.api.room_versions import RoomVersions
from synapse.events import EventBase
from synapse.rest import admin
from synapse.rest.client import login, room
from synapse.server import HomeServer
from synapse.types import JsonDict
from synapse.util import Clock

from tests.test_utils import event_injection
from tests.test_utils import FakeResponse, event_injection
from tests.unittest import FederatingHomeserverTestCase


Expand Down Expand Up @@ -98,8 +94,8 @@ def test_get_room_state(self):

# mock up the response, and have the agent return it
self._mock_agent.request.side_effect = lambda *args, **kwargs: defer.succeed(
_mock_response(
{
FakeResponse.json(
payload={
"pdus": [
create_event_dict,
member_event_dict,
Expand Down Expand Up @@ -208,8 +204,8 @@ def _get_pdu_once(self) -> EventBase:

# mock up the response, and have the agent return it
self._mock_agent.request.side_effect = lambda *args, **kwargs: defer.succeed(
_mock_response(
{
FakeResponse.json(
payload={
"origin": "yet.another.server",
"origin_server_ts": 900,
"pdus": [
Expand Down Expand Up @@ -269,8 +265,8 @@ def test_backfill_invalid_signature_records_failed_pull_attempts(

# We expect an outbound request to /backfill, so stub that out
self._mock_agent.request.side_effect = lambda *args, **kwargs: defer.succeed(
_mock_response(
{
FakeResponse.json(
payload={
"origin": "yet.another.server",
"origin_server_ts": 900,
# Mimic the other server returning our new `pulled_event`
Expand Down Expand Up @@ -305,21 +301,3 @@ def test_backfill_invalid_signature_records_failed_pull_attempts(
# This is 2 because it failed once from `self.OTHER_SERVER_NAME` and the
# other from "yet.another.server"
self.assertEqual(backfill_num_attempts, 2)


def _mock_response(resp: JsonDict):
body = json.dumps(resp).encode("utf-8")

def deliver_body(p: Protocol):
p.dataReceived(body)
p.connectionLost(Failure(twisted.web.client.ResponseDone()))

response = mock.Mock(
code=200,
phrase=b"OK",
headers=twisted.web.client.Headers({"content-Type": ["application/json"]}),
length=len(body),
deliverBody=deliver_body,
)
mock.seal(response)
return response
Loading

0 comments on commit 9192d74

Please sign in to comment.