CZ-NIC / pyoidc

A complete OpenID Connect implementation in Python
Other
718 stars 259 forks source link

Access token validation: jwkest.jws.NoSuitableSigningKeys: No key with kid: #776

Closed Robin-Bonnin closed 3 years ago

Robin-Bonnin commented 3 years ago

Hi,

I am currently trying to implement pyoidc with OneLogin. Everything was going fine until I reach the do_access_token_request() function. An error is reached, and I can not figure out the reason, whether it is the config or the code.

I have a very basic example to illustrate my meaning. I am using Django alongside DRF.

This is the function which handles the redirection from my IdP (OneLogin).

Endpoint which handles the Authorization request

class SSORequest(APIView):
    """
     A viewset for viewing and job requistions.
     """

## Authorization flow

permission_classes = [AllowAny]

def get(self, request, format=None):
    """
    Return a list of all blacklisted domains.
    """

    ## Identity provider and client registration
    client = Client(client_authn_method=CLIENT_AUTHN_METHOD)

    info = {
        "client_id": "XXXXXXXX",
        "client_secret": "XXXXXXXXXX",
        "redirect_uris": ["https://XXXXXXX.ngrok.io/api/v1.0/redirect/"],
    }
    client_reg = RegistrationResponse(**info)
    client.store_registration_info(client_reg)

    op_info = ProviderConfigurationResponse(
        version="1.0",
        issuer="https://XXXXXX.onelogin.com/oidc/2",
        authorization_endpoint="https://XXXX.onelogin.com/oidc/2/auth",
        token_endpoint="https://XXXXXXXX.onelogin.com/oidc/2/token",
        jwks_uri="https://manatal-dev.onelogin.com/oidc/2/certs",
    )

    client.handle_provider_config(op_info, op_info["issuer"])

    request.session["state"] = rndstr()
    request.session["nonce"] = rndstr()
    args = {
        "client_id": client.client_id,
        "response_type": "code",
        "scope": "openid",
        "nonce": request.session["nonce"],
        "redirect_uri": client.registration_response["redirect_uris"][0],
        "state": "some-state-which-will-be-returned-unmodified",
    }

    auth_req = client.construct_AuthorizationRequest(request_args=args)
    login_url = auth_req.request(client.authorization_endpoint)

    return Response(login_url, status=HTTP_200_OK)

Redirection endpoint handling access token retrieval and get user information

   class SSORedirect(APIView):

    ## Access token flow

permission_classes = [AllowAny]
  def get(self, request, format=None):

      """
      Return a list of all blacklisted domains.
      """
      ## Identity provider and client registration
      client = Client(
          client_authn_method=CLIENT_AUTHN_METHOD,
      )

    info = {
        "client_id": "XXXXXX",
        "client_secret": "XXXXXXXXXXXXXX",
        "redirect_uris": ["https://19a89c23d8db.ngrok.io/api/v1.0/redirect/"],
    }
    client_reg = RegistrationResponse(**info)
    client.store_registration_info(client_reg)

    op_info = ProviderConfigurationResponse(
        version="1.0",
        issuer="https://XXXXX-dev.onelogin.com/oidc/2",
        authorization_endpoint="https://XXXXXX-dev.onelogin.com/oidc/2/auth",
        userinfo_endpoint="https://XXXXX-dev.onelogin.com/oidc/2/me",
        token_endpoint="https://XXXXX-dev.onelogin.com/oidc/2/token",
    )
    client.handle_provider_config(op_info, op_info["issuer"])

    response = request.GET.copy()

    aresp = client.parse_response(
        AuthorizationResponse, info=response, sformat="dict"
    )
    client.redirect_uris = [
        "https://XXXXXXXXXX.ngrok.io/api/v1.0/redirect/",
    ]

    if aresp.get("error"):
        return Response(
            {"error": aresp["error"], "reason": aresp.get("error_description", "")}
        )

    args = {
        "code": aresp["code"],
        "client_id": "XXXXXXXXXXX",
        "client_secret": "XXXXXXXXXXX",
        "redirect_uri": "https://XXXXXXXXX.ngrok.io/api/v1.0/redirect/",
    }

    resp = client.do_access_token_request(
        scope="openid",
        state=aresp["state"],
        request_args=args,
        authn_method="client_secret_basic",
    )
    print(resp)

    if resp.get("error"):
        return Response(
            {"error": resp["error"], "reason": resp.get("error_description", "")}
        )
    userinfo = client.do_user_info_request(state=aresp["state"])

    return Response(userinfo)

I just tried this snippet to handle the access token request, and when calling the function, it fails at veryfing the id token which is correctly received with the intended values.

I understand that my config is probably wrong but I cannot figure out the reason.

Side question: As we are using REST apis. Is it a good practice to instanciate the client at every API call. I did not find another way to do so, but all the examples I find seem to be using sessions.

Thank you for your help! Regards Robin Bonnin

tpazderka commented 3 years ago

Hi, the issue is that you do not have any keys for your IdP. I am not sure if OneLogin publishes the keys or not. But if yes, then you need to add jwks_uri to your instance of ProviderConfigurationResponse and the code should be able to get the keys itself. Otherwise, you might need to manually create a KeyStore and pass it to the Client.

I am not sure what is actually the question in your side question. Could you please elaborate?

Robin-Bonnin commented 3 years ago

Thank you very much for your answer, it does work now.

I have updated my first post to reflect the change and add the full code. I could have searched for a while without finding the hint. :+1:

Regarding the side question, the more I think about it the more I feel it is a dumb question, but here it is.

You can see that I added in the first post my 2 endpoints. In each endpoint I am instantiating the Client and ProviderConfigurationResponse class. What I saw in all the code snippets I saw was that the session was used to retrieve the client, hence it was not re instantiated.

So my question is, is it a best practice to re instantiate the client or not? But I think I answered myself by working with REST, you do not want to keep info between calls, so you have to.

Thank you anyway for your help!

tpazderka commented 3 years ago

Glad to hear it works now :)

It probably depends on the workflow, but in your case a new instance probably makes more sense.