handle case where provider challenge types not in response

[AlecTroemel]

What

Handle case where identifier authorization response does not include challenge type in the providers chal_types

Why

The challenges list returned in the identifier auth response may not include the challenge type of the provider the client is set up with. Here's the relevant part of the RFC

RFC 8555 Section 7.1.4 19:

For pending authorizations, the challenges that the client can fulfill in order to prove possession of the identifier. For valid authorizations, the challenge that was validated. For invalid authorizations, the challenge that was attempted and failed.

For example, if I am trying to acquire a cert for a domain using DNS validation, but there is already a valid cert that used HTTP validation, lets-encrypt will not include the DNS block in the response. Currently that will result in this (possibly confusing) error.

raised unexpected: UnboundLocalError("local variable 'identifier_auth' referenced before assignment")

this PR attempts to catch this situation and throw a more relevant error

[codecov[bot]]

Codecov Report

Merging #209 into master will decrease coverage by 0.06%. The diff coverage is 50.00%.

@@            Coverage Diff             @@
##           master     #209      +/-   ##
==========================================
- Coverage   88.22%   88.16%   -0.07%     
==========================================
  Files          19       19              
  Lines        1189     1191       +2     
==========================================
+ Hits         1049     1050       +1     
- Misses        140      141       +1     

Impacted Files	Coverage Δ
sewer/client.py	91.71% <50.00%> (-0.27%)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1b12fa7...dfe4e5a. Read the comment docs.

[mmaney]

@AlecTroemel I don't think that the case you describe (and thanks for that, I would have been "how could it do that???" otherwise) should be an error! It will take a larger change than just within get_identifier_authorization, but this should be treated as already authorized and so not of further concern. And that brings us to the swamp of get_certificate I think...

On second thought, maybe this bandaid is just enough to stay off that slippery slope that leads to that cliff until I get a few other loose ends tied up and push out 0.8.4, which turns out will include the crypto refactor. Thought that was going to be too big a mess, but it went smoothly. Modulo a few undocumented (or docs I couldn't find) about just what LE will and won't accept. So it goes...

[AlecTroemel]

@mmaney I could:

1. create a specific error class for this situation (something like InvalidChallengeTypeError)
2. catch that error in get_certificate
3. just log the situation and return early.. if that's what you mean by "should be treated as already authorized and so not of further concern".

[mmaney]

@AlecTroemel This is, really, part of get_certificate's tech debt. Heavier than mountains, it is. :-(

I think we'll table this until 0.8.4 is released (soon. no, actually soon, nothing up my sleeve...), and then get_certificate and its co-dependents will have to take center stage for 0.8.5, which will fix this failure far more robustly.

[mmaney]

@AlecTroemel Perhaps you can check on this, but I believe the problem as described is a situation where the HTTP authentication is still valid, so the server neither expects nor requires the ACME client to post a challenge response. So the proper handling would be to observe that the authorization's status is "valid", and then just skip it, as it needs no further work. That should be the case if the still-valid authorization were done with dns-01, but that would just be redundantly done over and so has no visible effect.

It would be easy enough to add that check to skip if valid (though perhaps it should be a little more careful than that), though that might well expose issues in the existing code if there are no pending authorizations that need to be completed! If you want to work on this for 0.8.5, rfc8555 §7.1.4 will be your guide. :-) Or you can wait for 0.9, which is well in progress, and is looking like pretty much throwing client.py away, aside from some recyclable scraps of code.