fix: fetchAndSetEntityDescriptor: return non-nil error if http status is not 200

[flyinghermit]

Fixes a bug where fetchAndSetEntityDescriptor returned non-nil error when it failed to set entity descriptor.

Background: fetchAndSetEntityDescriptor tries to fetch and set SAML service provider entity descriptor from given endpoint. It was implemented in a way that it should return error if it fails to fetch and set entity descriptor.

If it fails to set entity descriptor, the generateAndSetEntityDescriptor func will generate the entity descriptor with given entity_id and acs_url values.

Bug: In situation where remote endpoint would return HTTP status code above 200 and below 400, the fetchAndSetEntityDescriptor was returning early with a nil error, even when it still failed to set the entity descriptor. And since the generateAndSetEntityDescriptor is only called if fetchAndSetEntityDescriptor returns error, we would never set the entity descriptor in such case.

Root cause: incorrect use of trace.ReadError().
The trace.ReadError() returns non-nil error if statusCode >= http.StatusOK && statusCode < http.StatusBadRequest. So in scenario where, say a redirect response was received, fetchAndSetEntityDescriptor would return with non-nil error, without setting the entity descriptor, defeating the logic which follows.

The existing test only included 404, empty message and a valid message so it failed to catch the 302 redirect case.

Fix:

• return trace.BadParameter error regardless of HTTP status code except for 200. We aren't interested in parsing error type as any error in this func means entity descriptor is not set.
• updated test to also include 302 redirect.

changelog: Fixed an issue in SAML IdP entity descriptor generator process, which would fail to generate entity descriptor if the configured Entity ID endpoint would return HTTP status code above 200 and below 400 .

[r0mant]

In situation where remote endpoint would return HTTP status code above 200 and below 400, the fetchAndSetEntityDescriptor was returning early with a non-nil error, even it still failed to set the entity descriptor.

@flyinghermit I think in your PR description you wanted to say that it was returning early with a nil error, not with a non-nil error? It threw me off initially trying to understand the logic but I think it's just a typo in your description.

[flyinghermit]

I think in your PR description you wanted to say that it was returning early with a nil error, not with a non-nil error? It threw me off initially trying to understand the logic but I think it's just a typo in your description.

Haa Thanks for spotting that. Yeah I meant to write nil there.

[flyinghermit]

Ping @fspmarshall, @atburke

[public-teleport-github-review-bot]

@flyinghermit See the table below for backport results.

Branch	Result
branch/v14	Create PR
branch/v15	Create PR