Fixes to make the library work correctly, some fixes to the "simple" example and some internal documentation. #803
Conversation
|
I have a working project that uses this fixes and other fixes so I will try to make more PRs in the future. |
| @@ -298,7 +298,7 @@ def verify_id_token(instance, check_hash=False, **kwargs): | |||
|
|
|||
| if "keyjar" in kwargs: | |||
| try: | |||
| if _body["iss"] not in kwargs["keyjar"]: | |||
| if _body["iss"] not in kwargs["iss"]: | |||
There was a problem hiding this comment.
This is checking if we have the needed key to make the signature and/or encryption.
| @@ -255,6 +255,9 @@ def __init__( | |||
| logout_path="", | |||
| settings: PyoidcSettings = None, | |||
| ): | |||
| """ | |||
| :param name the iss, which is the URI of the OP. | |||
There was a problem hiding this comment.
I would rather use the google style conventions.
There was a problem hiding this comment.
I will take a look at it, thanks for your advice.
| @@ -425,6 +425,8 @@ def construct_AuthorizationRequest( | |||
| else: | |||
| request_args = {} | |||
|
|
|||
| request_args.update(kwargs) | |||
There was a problem hiding this comment.
All the content should be in the request_args, the kwargs can potentially contain bad arguments.
There was a problem hiding this comment.
My problem is that if I don't add the request args manually from kwargs these arguments like client_id, redirect_uri passed to the oic.oic. construct_AuthenticationRequest method are never added to the authentication request. So, if I'm not doing anything else wrong, there is a problem when picking the args passed to this method because if I run my app with the authenticationRequest method detailed below without my changes the redirect_uri arg in the authentication request made to the OP is equal to "h". When I use the python debug console to print session["client"].registration_response["redirect_uris"][0] its value is 'https://localhost:8080/authenticateDigitelTS' and therefore it's correct.
def authenticationRequest(self, session):
"""
Sends a request to the OP to authenticate the user that made this request.
:param session: a CherryPy Session with the session of the user that made the request.
:return a RegistrationResponse instance with the response.
"""
session["state"] = rndstr()
session["nonce"] = rndstr()
args = {
"client_id": session["client"].client_id,
"response_type": session["client"].response_type,
"scope": session["client"].scope,
"nonce": session["nonce"],
"redirect_uri": session["client"].registration_response["redirect_uris"][0],
"state": session["state"]
}
auth_request = session["client"].construct_AuthorizationRequest(**args)
# Send the request to the authorization endpoint.
loginPseudoURL = auth_request.request(session["client"].authorization_endpoint)
return loginPseudoURL
I don't know if the problem is because of these arguments never being added to request_args in oic.oauth2.construct_AuthorizationRequest or there is an error in any other place because I don't know all the implementation details of the library that well. Maybe you can point me in the right direction. Thanks for your help, have a good day.
PD: I will check google documentation conventions for future PRs
Documentation changes (b9e18b6 and e22e228)
Added some internal documentation to methods of the "simple" examples and the library.
Fixes to make the authentication request/response work
0cd2944
In the oic.oauth2 construct_Authorization method the request args in kwargs are never added to the request_args of the request and therefore you can't get the client_id, the state, the redirection_uris etc when trying to complete the auth in the VerifierMiddleware of the OP, atleast in the "simple op" example. There are some changes needed for the VerifierMiddleware to work too. In my proposal I use the HTTP_REFERER header to get the query args of the previous request which is the request to get the login static page and it has the arguments we added in the construct_Authorization method which makes it possible to get authenticated correctly.
Fixes to make the token request/response work and the userinfo request/response work
897a652
The token endpoint and userinfo endpoint are never added to the Client instance so I added them just after the Client creation. Someone with more knowledge about the library than me can try and add these changes inside the library functions almost certainly.
ce469a9
When trying to get the access token some method of the library needs to check the authorization header with the Bearer token of the HTTP Basic Auth when using the authorization code flow but this header is ignored when the request is processed using the pyoidcMiddleware so I add this header in each request to the token endpoint and it works now.
1e98232
When the issuer of the id token is verified in the library methods the iss is checked against the keyjar which makes no sense and makes the verification fail always. Now it checks it against the known iss.