Refactor: remove dependency on Benefits models, rename modules, improve Client parameters

[angela-tran]

Part of the changes to support multiple verifiers in cal-itp/benefits#317 was to update the parameters for TransitAgency.types_to_verify. The API was using this method and doesn't work now because it needs to pass a verifier into the types_to_verify method. This was missed in PR #25.

We can use this as an opportunity to refactor out the dependency on Benefits Django models.

[thekaveman]

Nice catch!

But I think the better approach here is to pass the list of types to verify into this function call. We shouldn't assume that agency is a model like what is defined in the benefits app. The function should just ask for exactly the data it needs.

[angela-tran]

Nice catch!

But I think the better approach here is to pass the list of types to verify into this function call. We shouldn't assume that agency is a model like what is defined in the benefits app. The function should just ask for exactly the data it needs.

I started thinking this too. Do you think we should just add the types list as a parameter to __init__, or should I go ahead and add parameters for all the other exact pieces of data that the method needs?

The method currently looks like this:

class Client:
    """Eligibility Verification API HTTP client."""
    def __init__(self, agency, verifier, issuer):
        logger.debug(f"Initialize client for agency: {agency.short_name}")
        self.issuer = issuer

        # get the eligibility type names
        self.types = list(map(lambda t: t.name, agency.types_to_verify(verifier)))
        self.agency_id = agency.agency_id
        self.jws_signing_alg = agency.jws_signing_alg
        self.client_private_jwk = agency.private_jwk
        self.jwe_encryption_alg = verifier.jwe_encryption_alg
        self.jwe_cek_enc = verifier.jwe_cek_enc
        self.server_public_jwk = verifier.public_jwk
        self.api_url = verifier.api_url
        self.api_auth_header = verifier.api_auth_header
        self.api_auth_key = verifier.api_auth_key

My question is essentially, do we want
def __init__(self, agency, verifier, types, issuer)
OR
def __init__(self, issuer, types, agency_id, jws_signing_alg, client_private_jwk, jwe_encryption_alg, jwe_cek_enc, server_public_jwk, api_url, api_auth_header, api_auth_key)?

[thekaveman]

This is the Client, right?

I would think we __init__ the client with the static configuration data, so I guess that is:

def __init__(
    self,
    api_url,
    api_auth_header,
    api_auth_key,
    jws_signing_alg,
    client_private_jwk,
    jwe_encryption_alg,
    jwe_cek_enc,
    server_public_jwk,
)

And then the verify function call passes the data to verify:

client.verify(sub, name, types)

Essentially we don't want this library to know anything or be built around the Benefits data model (pretend it doesn't exist, all you have is the spec: https://docs.calitp.org/eligibility-api/specification).

Just expose the configuration params and functions needed to do the thing.

[angela-tran]

@thekaveman Gotcha, thank you!

[angela-tran]

Just pushed one more commit to address this comment: cal-itp/benefits#606 (comment) - I renamed api.py and verify.py to client.py and server.py, respectively.

This is ready for re-review!

[thekaveman]

🚀