search

element-hq / synapse

Synapse: Matrix homeserver written in Python/Twisted.
https://element-hq.github.io/synapse
GNU Affero General Public License v3.0
1.63k stars 204 forks source link

EdDSA Support for OAuth2 #17519

Open Arzumify opened 3 months ago

Arzumify commented 3 months ago

Description: OAuth2 currently uses the authlib library, which does not support the EdDSA (Ed25519) encryption scheme. We should supplement the use of authlib in these areas with the use of the newer https://github.com/authlib/joserfc, part of the authlib project, which does support EdDSA. I will make a pull request soon adding support, but i'm sleepy right now :P

### Tasks
- [x] Add support in synapse/handlers/oidc.py
- [ ] Pass test suite
Arzumify commented 3 months ago

Completed the implementation, now testing (this could take a while, I don't have a very good computer :P)

Arzumify commented 3 months ago

It appears to fail the following tests:

I shall fix this by gracefully handling expired tokens

Arzumify commented 3 months ago

It appears to fail the following tests:

* tests.handlers.test_oidc.OidcHandlerTestCase.test_attribute_requirements
  joserfc.errors.ExpiredTokenError: expired_token: The token is expired

* tests.handlers.test_oidc.OidcHandlerTestCase.test_attribute_requirements_mismatch
  joserfc.errors.ExpiredTokenError: expired_token: The token is expired

* synapse.handlers.oidc
  joserfc.errors.ExpiredTokenError: expired_token: The token is expired

* tests.handlers.test_oidc.OidcHandlerTestCase.test_callback
  joserfc.errors.ExpiredTokenError: expired_token: The token is expired

* tests.handlers.test_oidc.OidcHandlerTestCase.test_attribute_requirements_contains
  joserfc.errors.ExpiredTokenError: expired_token: The token is expired

* tests.handlers.test_oidc.OidcHandlerTestCase.test_callback_session
  joserfc.errors.ExpiredTokenError: expired_token: The token is expired

* tests.handlers.test_oidc.OidcHandlerTestCase.test_map_userinfo_to_invalid_localpart
  joserfc.errors.ExpiredTokenError: expired_token: The token is expired

* tests.handlers.test_oidc.OidcHandlerTestCase.test_map_userinfo_to_existing_user
  joserfc.errors.ExpiredTokenError: expired_token: The token is expired

* tests.handlers.test_oidc.OidcHandlerTestCase.test_map_userinfo_to_user_does_not_register_new_user
  joserfc.errors.ExpiredTokenError: expired_token: The token is expired

* tests.handlers.test_oidc.OidcHandlerTestCase.test_map_userinfo_to_user_retries
  joserfc.errors.ExpiredTokenError: expired_token: The token is expired

* tests.handlers.test_oidc.OidcHandlerTestCase.test_null_localpart
  joserfc.errors.ExpiredTokenError: expired_token: The token is expired

I shall fix this by gracefully handling expired tokens

This doesn't seem to be a deliberate part of the test suite though. Is this a bug?

Arzumify commented 3 months ago 
    def test_attribute_requirements(self) -> None:
        """The required attributes must be met from the OIDC userinfo response."""
        # userinfo lacking "test": "foobar" attribute should fail.
        userinfo = {
            "sub": "tester",
            "username": "tester",
        }
    request, _ = self.start_authorization(userinfo)
        self.get_success(self.handler.handle_oidc_callback(request))
        self.complete_sso_login.assert_not_called()

        # userinfo with "test": "foobar" attribute should succeed.
        userinfo = {
            "sub": "tester",
            "username": "tester",
            "test": "foobar",
        }
    request, _ = self.start_authorization(userinfo)
        self.get_success(self.handler.handle_oidc_callback(request))

        # check that the auth handler got called as expected
        self.complete_sso_login.assert_called_once_with(
            "@tester:test",
            self.provider.idp_id,
            request,
            ANY,
            None,
            new_user=True,
            auth_provider_session_id=None,
        )

Looking at the test suite, it seems that it doesn't bother to add an expiration date for the tokens, causing the stricter JoseRFC to throw an expired error.