Added token endpoint to oidc-op/oauth2. #64
Conversation
Added OAuth2ClaimsInterface Added ASConfiguration Changed name of tests to make clear if they where oauth2 or oidc based. Made all OAuth2/OIDC based tests use ASConfiguration/OPConfiguration.
|
it was 2018-2019 when I had talked you to divide OAuth2 profile from OIDC one. You did it finally, great! |
There was a problem hiding this comment.
In a single commit we have changes of a different nature. I would have definitely split the code formatting style commit versus adding the additional functionality. It is difficult to review changes to 74 files in a single commit.
Anyway the only change I feel like asking you is to avoid replicating the OPConfiguration code in ASConfiguration as a copy / paste. The parts they have in common must be inherited.
Starting from OAuth2 as a basis we could do to inherit ASConfiguration in OPConfiguration and cut the bull's-head!
| keys = _context.keyjar.get( | ||
| "sig", "oct", ca_jwt["iss"], ca_jwt.jws_header.get("kid") | ||
| ) | ||
| keys = _context.keyjar.get("sig", "oct", ca_jwt["iss"], ca_jwt.jws_header.get("kid")) |
There was a problem hiding this comment.
I'm wondering why you brought all the pep8 modifications back to their original state but if you like it more so for me it's ok and totally relative.
Right it occurs to me that this revision will be more challenging;)
There was a problem hiding this comment.
Yeah, I shouldn't have run black on it.
There was a problem hiding this comment.
Not without another commit in between 😜
| self.session_key = None | ||
| self.template_dir = None | ||
| self.token_handler_args = {} | ||
| self.userinfo = None |
There was a problem hiding this comment.
and these are OP's
self.add_on = None
self.authz = None
self.authentication = None
self.base_url = ""
self.capabilities = None
self.cookie_handler = None
self.endpoint = {}
self.httpc_params = {}
self.id_token = None
self.issuer = ""
self.keys = None
self.login_hint2acrs = {}
self.login_hint_lookup = None
self.session_key = None
self.sub_func = {}
self.template_dir = None
self.token_handler_args = {}
self.userinfo = None
self.password = None
self.salt = None
They are replicated from OPconfiguration, in a way that if we should add a argument we then need to put in OPConf and at the same time in ASConf. I think that it would be better to have a BaseClass to inherit and take all these attribute from it instead of repeat them
2 participants
As a follow up on the above: