Fix types on SAMLConfig to allow privateKey

[nicksrandall]

Description

The docs say to use privateKey instead of privateCert but when I do that, I get a Typescript error. This should fix that issue.

Checklist:

• Issue Addressed: [X]
• Link to SAML spec: [ ]
• Tests included? [ ]
• Documentation updated? [ ]

[cjbarth]

If this is merged, then other things will break as the subsequent refactoring of privateCert to privateKey hasn't been done.

Having said that, there are two interfaces at play: SamlConfig and SAMLOptions. The SAMLOptions is the one that was refactored, not the SamlConfig interface version of privateCert. Maybe we missed something, but maybe not, as they are different uses.

[nicksrandall]

IMO, if users are not supposed to be using privateKey yet, then I think the error message in the console should be removed. If they should be using privateKey, then that property needs to be added to the public types.

[gugu]

I don't think anything will break, @deprecated instruction will only mark a field as deprecated in IDE, no additional effects

[cjbarth]

As I look this cover over closer, I see there are places were the deprecation of privateCert is incomplete. I'll see if I can address that.

[midgleyc]

@cjbarth This affects SamlConfig even when SAMLOptions is the one changed.

• SAML has a constructor wanting options: Partial<SAMLOptions>
• Strategy has a constructor taking options: SamlConfig
• Strategy's constructor calls this._saml = new saml.SAML(options);

This works type-wise because SamlConfig is a subset of SAMLOptions, but shows a warning because SamlConfig can't have a privateKey field while SAMLOptions prefers it to privateCert.

A nicer solution might be to have SAML's constructor take a SAMLOptions where the optional fields have ?s, but this PR as-is would fix the typing issue on TypeScript projects using this library.

[cjbarth]

@midgleyc I agree with your assessment. I really think these two interfaces should be consolidated. I believe this is just an artifact of the quick conversion to TypeScript that was done. I'll approve this to get things moving, but would you be willing to submit a PR to consolidate these interfaces?

[midgleyc]

@cjbarth Thanks, see #515

[cjbarth]

@nicksrandall, does the recent PR that landed address your concerns here? If so, please close this, otherwise, let's keep the dialog going.

[cjbarth]

Closing in favor of #515