Support providing root client ID via env. variables when bootstrapping

[dimas-b]

Allow the root client ID and secrets to be provided via environment variables when bootstrapping.

For "in memory" use cases, this mean the main Polaris server will read user-provided root secrets from the env., if propvided.

For "persistent" use cases, the bootstrap command will read user-provided root secrets from the env., if propvided.

The env. variables are:

• POLARIS_BOOTSTRAP_<REALM>_ROOT_CLIENT_ID
• POLARIS_BOOTSTRAP_<REALM>_ROOT_CLIENT_SECRET

If these variables are not provided, random values are generated as before.

How Has This Been Tested?

Manual smoke tests.

[eric-maynard]

I recall that @collado-mike and I discussed this as one option when bootstrapping, but I can't remember why we went with the current approach.

For my part, I would ideally like to see as little a difference in the UX of the different metastores as possible.

[dimas-b]

@eric-maynard :

I would ideally like to see as little a difference in the UX of the different metastores as possible.

Do you mean supporting the same overrides under the bootstrap command? I can certainly add that. WDYT?

[adutra]

Taking a step back, and thinking about real-life scenarios: how is an operator supposed to get hold of the root credentials, after installing and bootstrapping Polaris? I read the Polaris documentation about production setups, it does cover bootstrapping, but it doesn't cover this detail in particular, so I guess the exact procedure depends on the metastore being used? Wouldn't it be possible to standardize that?

I'm also raising this point because while working on the Quarkus port, I realized that it is not possible to create a true integration test currently, that is, a test that spawns Polaris as a black box, and only interacts with it through its external APIs. This is because once started and bootstrapped, and regardless of the metastore used, the test is unable to infer which credentials to use to communicate with Polaris.

All in all, I wonder if we aren't missing a simple and standard way to pass the root credentials to the boostrapping process, even in production setups – or alternatively, a way to retrieve those, that would also be standardized across metastores.

[dimas-b]

[...] I wonder if we aren't missing a simple and standard way to pass the root credentials to the boostrapping process.

My personal take on that is that the bootstrap command should either take root credentials as a parameters, or output the uniquely generated root credentials. I think we can support both options (e.g. as CLI arguments) and I'm willing to add that to this PR if other developers agree.

[eric-maynard]

A few thoughts here.

1. For testing, I've found it generally sufficient just to use e.g. --access-token 'principal:root;realm:default-realm', so you shouldn't actually need root credentials in many cases unless you are testing a context resolver.
2. In general, I feel nervous about allowing the bootstrap process to use non-random credentials (even when supplied by the user). I am not a security expert but this seems insecure.
3. With respect to the bootstrap command printing the credentials to stdout, this was thought to be insecure. I remember now that this explicitly came up in my previous conversations with @collado-mike

[dimas-b]

@eric-maynard : thanks for sharing your views on this matter. Re: security of bootstrapping, I did not mean printing credentials to STDOUT. The "output" can be a file at a user-specified location. I think this is quite similar to downloading ssh certificates for a VM from a cloud vendor.

I wonder what options end users currently have for bootstrapping, though :thinking: How will a user be able to discover a generated root credential? (Apologies if I missed this in somewhere in docs).

[adutra]

I agree that we need to proceed carefully since it's a security-related issue.

I'd note, fwiw, that Keycloak allows to bootstrap the root credentials via environment variables.

I don't think we can suspect Keycloak of taking security lightly :-)

In another place of Keycloak docs, we can learn more about how the environment variables are processed:

If the initial administrator already exists and the environment variables are still present at startup, an error message stating the failed creation of the initial administrator is shown in the logs. Keycloak ignores the values and starts up correctly.

Couldn't we do something similar to that?

[adutra]

The "output" can be a file at a user-specified location.

If we go down this route, maybe check that the file permissions are as restrictive as possible.

[eric-maynard]

@dimas-b

I wonder what options end users currently have for bootstrapping, though 🤔 How will a user be able to discover a generated root credential? (Apologies if I missed this in somewhere in docs).

So if you're just doing testing like this PR would address, you can just use passwordless auth. You normally don't need the root credentials.

However if you're asking more generally how to retrieve the root credentials it is metastore-dependent. Ideally your metastore is set up with your auth provider and allows you to do something nice like set/retrieve the credentials using SSO. Or perhaps they are exposed through some API which is already secured. But in the simplest case where you're just using a postgres metastore, you can retrieve them through something like:

SELECT principalclientid, mainsecret FROM principal_secrets;

[dimas-b]

@eric-maynard :

So if you're just doing testing like this PR would address, you can just use passwordless auth. You normally don't need the root credentials.

I could use principal:root;realm:default-realm as an access token, but I happen to need to use the client credentials auth flow, which currently uses random values for the root user. Discovering the random credentials currently involves scanning Polaris STDOUT, which is inconvenient. I'd like to have control over the inputs I provide to Polaris.

So this PR proposes to make this an option for the user to define the root credential if the user so chooses. I think it could be convenient for other people too.

As for the general bootstrapping case, the above discussion is interesting, but maybe we can continue that on the dev list or separate PR. As for this PR, do you think the idea of allowing user overrides for root credentials is reasonable given that it only applies to the "test" authentication implementation, which is already not "secret" given the fixed root token?

[eric-maynard]

I see. If you want to test the auth flow and you can't call bootstrapRealms then I can see how the current bootstrap process is not great.

Could you call rotatePrincipalSecrets using the passwordless auth to receive new credentials?

Barring that, I am open to allowing the user to set the credentials using an env variable for the in-memory metastore.

[collado-mike]

It is a fairly common and accepted practice to generate random secrets during bootstrapping - e.g., terraform has built-in support for this - https://registry.terraform.io/providers/hashicorp/random/latest/docs/resources/password . Allowing Terraform to randomly generate a password, bootstrap Polaris, and stick the secrets in Vault or k8s secrets is a very secure pattern and something I think we ought to support.

[adutra]

It is a fairly common and accepted practice to generate random secrets during bootstrapping [..]

And I'd note that that is the usual pattern that most Helm charts adopt when bootstrapping things like databases.

[dimas-b]

SGTM, based on the above discussion, I think I'll extend the env. variables support to the normal bootstrap command too.

[dimas-b]

I believe that after #438 but without this PR it is not really possible to bootstrap Polaris with EclipseLink... I do not see a way to discover the auto-generated root secret :thinking: ;)

[eric-maynard]

+1 @dimas-b, I think this is a partial fix for that problem. However we probably shouldn't allow bootstrapping without specifying credentials if the intent is that users can retrieve secrets from the metastore but we no longer put them in the metastore.

I still think we can merge this as-is and follow up with that potential restriction.

Other possibilities for resolving this issue:

1. Make EclipseLink boostrap print secrets like in-memory bootstrap does
2. Make it so that we persist plaintext secrets for primary secrets but not secondary secrets
3. Hack #438 so that (2) applies only to root's first-time secret, and enforce rotation on root

Any preferences @dimas-b / @collado-mike ?

[dimas-b]

My preference would be to keep the env. variables for root password bootstrapping, plus add CLI options to the bootstrap command to print the auto-generated password to STDOUT, but only at user's request.

By the way, I'm rebasing this PR to resolve conflicts... will probably squash too.

[eric-maynard]

plus add CLI options to the bootstrap command to print the auto-generated password to STDOUT, but only at user's request.

I like this idea, and I can do this -- but do you think we should fail the bootstrap if

1. The env variables are not set
2. credential printing is disabled?

In this case the metastore is pretty much bricked, so I think we should not proceed with the bootstrap.

Edit: See my sketch of this idea here, which would be rebased on this PR.

[dimas-b]

but do you think we should fail the bootstrap if

The env variables are not set credential printing is disabled? [...]

SGTM :+1:

[dimas-b]

@collado-mike : are you ok to merge?