oidcccl: fix SSO login fails if IdP is down when CRDB starts

[souravcrl]

oidcccl: fix SSO login fails if IdP is down when CRDB starts

We are encountering errors both when IdP is down and when it comes back up:

1. When IdP is down(network is unavailable):

• OIDC attempts to re-initialize at the start of the request handler, this can fail as it is unable to fetch openid-configuration required for initializing the provider for oidc manager
• OIDC re-initialization assumes that the operation will succeed and continues serving the request even though initialization might have failed. Hence we need to add check to verify if oidcAuthentication.initialized is set to true post successful call to reloadConfigLocked.

2. After IdP comes back up (network becomes available):

• OIDC is initialized successfully and there is a call to oidc callback request handler.
• OIDC callback request handler is failing when we are trying to ExchangeVerifyGetClaims. This is because oidcManager.Verify() is using the older context used when creating the verifier in oidc manager initialization.
• The previous context got cancelled and there is an error thrown in Verify() code for cancelled context. The fix is to pass a non-cancellable context to the oidcManager instance.

Release note: None

Epic: CRDB-35370

[blathers-crl[bot]]

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

_{:owl: Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[cockroach-teamcity]

This change is 

[BabuSrithar]

pkg/ccl/oidcccl/authentication_oidc.go line 713 at r4 (raw file):

		if !oidcAuthentication.enabled {
			http.Error(w, "OIDC: disabled #2", http.StatusBadRequest)

the #2 and #3 later in the file seems to be user visible. If yes, how are the users expected to consume the numbers ?

Code quote:

 #2

[BabuSrithar]

pkg/ccl/oidcccl/authentication_oidc.go line 725 at r4 (raw file):

		}

		if !oidcAuthentication.enabled {

Just for my clarification, can the value of oidcAuthentication.enabled between line 712 and here ? Where we are already checking for same condition ? If yes, can we add a comment to explain it. Just looking at the code it seems like unreachable block to me,

[BabuSrithar]

pkg/ccl/oidcccl/authentication_oidc.go line 725 at r4 (raw file):

Previously, dhartunian (David Hartunian) wrote…

@BabuSrithar the two checks are not the same and you can reach the second one if the first one is not true. you could argue that this one should come first for readability though and to exit early if OIDC isn't enabled at all.

The code is changed now, at the time of my comment, the line 712 had the exact same condition check and return statement. I agree that this condition should go first for readability.

[souravcrl]

TFTR!

bors r=bdarnell,dhartunian

[craig[bot]]

Build succeeded:

• Bazel Essential CI (Cockroach)

[souravcrl]

blathers backport 23.2