Implement proper Factory-Pattern for IJwsTool

[glatzert]

I think the JwsTool could need some refactoring.

There should be a proper JwsTool::Create, to create the matching Signer.
I'll submit a proposal, if you agree. That'll also make it easily usable for PS-Core scenarios.

[ebekker]

Possibly, that was the approach I took in the original ACMESharp project, but I decided to simplify it, this time around by using plain interface and implementations approach (i.e. just new up one of the included implementations (RSA or ECDSA) that come in the box).

However, if it makes it easier with your approach, let's explore it.

[glatzert]

So i dug a Little bit around and explored some ideas about the JwsTool and thus the keys.
My initial problem was, that i was not able to understand how the ACMEClient obtains the first key for creating an account and storing that.
Your examples made that clear and I think what you are doing with the AccountKey in the CLI-Sample should be a first-class Citizen of the Implementation.
Also I think the jwsTool itself should be more internal and a wrapper, which takes just the jwsAlgorithmName OR the AccountKey as Parameters should be created and enabled to Export the AlgorithmName and Parameters as somhow serializable object.

Also I did not see a good reason for using init() (without making sure, it has been called - all sorts of malice can happen, if you do not call it) over a .ctor with proper Parameters.

Last but not least, I think creating a default IJwsTool (and thus a key) in the ACMEProtocolClient is not a good thing to do. It "hides" away the JwsTool, which and it's key, despite beeing rather important.
The Client should enforce getting a proper initialized JwsTool from the caller, just to make the caller perfectly aware of this tool and the key it uses.

[glatzert]

I forked and added a proposal, but it's an early state. Also I did not fixup the Tests, yet

[ebekker]

Good points, once you make the PR available, let's discuss.

I agree with your thinking about hiding away the IJwsTool, should force the user to set that explicitly, no defaults.

[glatzert]

I am nearly happy with the implementation now.
Unfortunately it imposes an Dependency on Newtonsoft.Json on the PS-Module. The dependency could be removed, if the PublicJWK would be an object and not a string.
I'd probably be able to port over the JwsSigner to PS. This does not look too hard …

Independent of the PSModule, I think this PR would be worth joining, since it removes Default signers and makes the Keys a little bit more visible. I also made sure, that a constructed object is well initialized.
And Renamed some functions to be more clear About the Purpose.

[ebekker]

I think the dependency on NS.Json is OK, so much of other Microsoft sanctioned base code (i.e. ASP.NET Core) is already taking a dependency on it. But if we want to isolate it that's possible too, we would just need to create the abstractions of JSON use (serialization/deserialization and controlling the behavior through alternate attributes, etc.).

That could be a worthwhile effort in the future, perhaps as an improvement but I wouldn't worry about it for now.

Oh and the reason I export a string is to remove any ambiguity about how to serialize the object, for example if it's just an object, what if the fields of the object need to be serialized in a special way, such as order-dependent (which is true for the canonical format used to generate signatures) or if you need to serialize members under a different name in JSON format than what they are called in the class representation?

[glatzert]

Well since I was able to create a new account with the current Version of the module, I'll leave it that way for now :)

[glatzert]

I think PR #7 is worth a shot..
The factory is extensible, if it needs to be, and can create the algorithms, which are provided via the implementation.