Add pinniped_supported_identity_provider_types to the IDP discovery endpoint

[joshuatcasey]

Add pinniped_supported_identity_provider_types to the IDP discovery endpoint

[codecov]

Codecov Report

Attention: Patch coverage is 88.29114% with 37 lines in your changes are missing coverage. Please review.

Project coverage is 29.62%. Comparing base (6b3f175) to head (7787885).

Files	Patch %	Lines
cmd/pinniped/cmd/oidc_client_options.go	0.00%	24 Missing ⚠️
pkg/oidcclient/login.go	93.50%	7 Missing and 3 partials ⚠️
.../controller/kubecertagent/mocks/mockdynamiccert.go	25.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1928      +/-   ##
==========================================
+ Coverage   29.40%   29.62%   +0.22%     
==========================================
  Files         348      350       +2     
  Lines       58499    58730     +231     
==========================================
+ Hits        17200    17400     +200     
- Misses      40785    40813      +28     
- Partials      514      517       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[cfryanr]

lgtm. Should we do the CLI changes in the same PR, or a new PR?

[joshuatcasey]

lgtm. Should we do the CLI changes in the same PR, or a new PR?

I'm not really sure exactly how we should translate this into CLI changes at the moment. Right now the best place to use it seems to be in pinniped get kubeconfig help text, but calling this endpoint in order to generate help text seems a little awkward. That command at the moment only conditionally calls the endpoint at all.

[joshuatcasey]

I'm actually not super clear how the CLI could best make use of this information for help text purposes.

The pinniped get kubeconfig command has a flag --oidc-issuer, which can be used to look up discovery information. If that flag is not set, it will inspect the JWTAuthenticator for the issuer. Even then it will only perform discovery if it needs additional information such as which IDPs are available.

Perhaps another way we could make use of the supported IDP types is by checking the --upstream-identity-provider-type against pinniped_supported_identity_provider_types? If a user calls pinniped get kubeconfig --upstream-identity-provider-name=<> --upstream-identity-provider-type=<> --upstream-identity-provider-flow=<> all specified, current logic will not perform upstream discovery at all. We'd likely want to rethink how this works.

We changed the scope of this PR to now also include the CLI code

[cfryanr]

Cases that we should make sure are covered:

• The issuer is not a Supervisor
• The issuer is a Supervisor but the user did not specify any IDP name/type
• The issuer is a Supervisor and the user specified an IDP name/type
• The issuer is an old Supervisor which does not have the new discovery data
• The issuer is a new Supervisor which has the new discovery data
• The issuer is a Supervisor using the backwards compatible FederationDomain mode where there are no IDPs listed on the FederationDomain CR
• The issuer is a Supervisor using the modern FederationDomain mode where the user lists IDPs on the FederationDomain CR

What else should we consider?

[cfryanr]

Rather than check that the endpoint is on the same server, wouldn't it be sufficient to just check that it is an https URL? If the user is trusting this issuer, then the user should also be trusting any URLs that the issuer is pointing them to for IDP discovery.

It's still important that the URL is https though, because plain http would break that chain of trust (which starts with the original issuer https URL and CA bundle), so the client should not follow plain http URLs.

[joshuatcasey]

I think the idea is that this client should make sure that Pinniped's IDP discovery is hosted on the same FederationDomain. Since this will always be true for a Pinniped Supervisor, it seems the natural thing to check here.