-
Notifications
You must be signed in to change notification settings - Fork 167
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
EdDSA Support for OAuth2 #17519
Comments
Completed the implementation, now testing (this could take a while, I don't have a very good computer :P) |
It appears to fail the following tests:
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? |
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. |
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
The text was updated successfully, but these errors were encountered: