Dex: configuration data management

[sym3tri]

One source of friction around bootstrapping/migrating has been with the way dex handles its config data (especially the client_identity table).

It was always the intention to allow config data to be edited via the API. However we can't do that now b/c the workers only load data on startup, and it is never refreshed. This is a separate issue we should address separately.

Today updating clients is actually not possible at all since their IDs/secrets are auto-generated upon insert, and cannot be updated. Meaning it is impossible to recreate an exact copy of a dex deployment, or configure a deterministic deployment.

We should probably _not_ diverge from the OIDC spec and allow some sort of special static ID/secret client registration. However I propose some new dexctl sub-commands and bootstrap API endpoints do raw import/export/deletion of various config data. Perhaps the following would solve most of these issues:

Connectors

• dexctl connectors export
• dexctl connectors import -f ./my-connector-config.json (this could replace set-connector-configs)
• dexctl connectors delete-all

Clients

• dexctl clients export
• dexctl clients import -f ./my-clients-config.json
• dexctl clients delete-all

NOTE:

• These are only accessible to the system operator.
• Imports should fail and rollback if _ANY_ duplicate ID already exists. (if dupes exist user is expected to export, delete-all, import).
• Imports must have strict validation checks (considering we don't handle client/server version skew in any form today).
• These are distinct from get-connector-configs and set-connector-configs b/c they are only intended to be machine readable json files.
• Should be properly authenticated.
• Encryption/Decryption of secrets must be considered.

[bobbyrullo]

These are distinct from get-connector-configs and set-connector-configs b/c they are only intended to be machine readable json files

Wouldn't it just replace both of those?

Should be properly authenticated.

If we have it on the overlord API I guess we should use the Admin API Key probably.

[sym3tri]

Wouldn't it just replace both of those?

"set" for sure.

"get" might be good for displaing in a nice format on the CLI. But assuming the above proposal, it would be better named as a subcommand: dexctl connectors list

[ericchiang]

Since this has been brought up twice https://github.com/coreos/dex/issues/388#issuecomment-204023483, https://github.com/coreos/dex/issues/382#issuecomment-204140402

Currently dex can be configured in a few ways. Flags, config files, and reading info form the db.

In addition, connectors currently contain sensitive information (client secrets and passwords) and store this unencrypted in the db. Also #14 (changing connectors in the DB doesn't update the workers) has been a pain for a while.

It's also been brought up that email configuration is split between flags and a config file: https://github.com/coreos/dex/issues/389

@chancez had proposed that we store everything in a configuration file, and skip the db. I like this for two reasons.

1. A single point of configuration. If you want to update the config, update it and bounce the worker.
2. We already have to configure things through files anyway.
3. No sensitive info in the DB.
4. Eliminates the need to address #14.
5. We don't have to add a bunch of stuff to the bootstrapping API.

@sym3tri also asked brought this up in the other thread:

Also agree that app config in the DB is not good. However we need a separate discussion on what is actually app config and what is app data.

Since I think this is a better form to discuss this I'd like to propose the following:

App config:

• Email config
• Connector config
• Email and HTML templates
• Encryption secrets
• Issuer

App data:

• Everything else

TL;DR - I propose having a dex worker pull all app config (see above for what that entails) from a single config file.

[sym3tri]

There are 2 issues to consider with the file-based approach.

1. Do we ever envision a feature in which a dex administrator could log into a UI and add CRUD connectors? The AWS console has such a feature and it's quite nice to be able to do this without having to manually restart the processes. Or there may be situations where the dex admin (person pushing buttons and admining users/connectors) is not necessarily the cluster admin (person deploying dex and setting up the config file).
2. The connector configs are tricky because they're used across workers in the context of the same user-session, but user-sessions span multiple requests contexts possibly on different workers (b/c of OAuth2 redirects). The deployer must make absolutely sure that all dex workers are using the same config file. Having a mismatch of connector configs would lead to scenarios like: end user clicks "login with Google", after the OAuth2 dance, user lands on a different worker with a different connector config (with no Google connector). Then the auth attempt fails.

Say you want to modify a connector config in a running cluster of 3 dex workers on a site that is processing 10 logins per second. How do you reliably edit all 3 connector configs at the same time without resulting in end-user login errors? Maybe doing a cutover behind a load-balancer is good enough, but even then you will get some login errors. The tradeoffs are:

• A: we're ok with dropping login attempts on the floor every time a connector is added/removed/edited, in return for reduced complexity.
• B: we care a lot about these login sessions and are willing to add more complexity to avoid dropping them.

[bobbyrullo]

Do we ever envision a feature in which a dex administrator could log into a UI and add CRUD connectors?

I always thought no, but @bcwaldon had brought up a good use case for this; imagine a situation where you host some protected resource, authenticated with Dex. You want to give customers, who are companies, the ability to have their employees access that data, but through their own company's login. In that case, you'd want to dynamically add a connector for this new customer

The connector configs are tricky because they're used across workers in the context of the same user-session, but user-sessions span multiple requests contexts possibly on different workers (b/c of OAuth2 redirects). The deployer must make absolutely sure that all dex workers are using the same config file.

I don't see that as much of a problem, unless we're changing connector configs often.

[sym3tri]

I don't see that as much of a problem, unless we're changing connector configs often.

_often_ or _ever_?

Seems weird to be like:
"I'm going to allow users to login with github now... while I roll this out we need to schedule downtime or be aware that many login attempts will fail over the next 5 minutes or so."

I'm just playing devil's advocate. It might totally be fine.

[ericchiang]

"I'm going to allow users to login with github now... while I roll this out we need to schedule downtime or be aware that many login attempts will fail over the next 5 minutes or so."

Where does the downtime or failed login attempts come from adding an option? Wouldn't the existing login methods work just fine it would just take some time from every worker to get the github login option?

Edit: work -> worker

[sym3tri]

"add" was a bad example. Pretend I said "remove".

[bobbyrullo]

Where does the downtime or failed login attempts come from adding an option?

Because you could be bounced from one worker to another during different phases of the flow.

To be fair, this is a problem with files or with state in the DB, unless you are checking the DB each time you start a flow.

[sym3tri]

To be fair, this is a problem with files or with state in the DB, unless you are checking the DB each time you start a flow.

True, and checking the DB before each flow is not a bad solution IMO.

[chancez]

Because you could be bounced from one worker to another during different phases of the flow.

@bobbyrullo That problem is also fixable with sticky sessions, which is something we should probably consider as a improved (but not required) deployment option anyways. Kubernetes ingress controllers allow this today, and so does the kubernetes/contrib/service-loadbalancer. Might be worth at least exploring what implications this would have on dex, as it would make the per-worker state problem easier to deal with (by moving that complexity up to the load balancer).

In traditional high-throughput web-apps, you don't really want sessions stored in the DB for performance reasons, and often times the solution is some kind of node-local cache for cookies/sessions, which means sticky sessions too, so if we ever wanted to support storing sessions outside of postgres this would be useful anyways. Not that we're looking to solve this problem now, but always good to be thinking about it.

To re-iterate. I don't think we need to handle sticky sessions directly in dex, or worry about them today, but they would prevent the problem with bouncing between workers which have slightly different configs when rolling out config changes.

[bobbyrullo]

I think we're getting way out of the scope of the issue at hand FWIW

[ericchiang]

I think we're getting way out of the scope of the issue at hand FWIW

Yes. Sticky sessions should be discusses elsewhere.

True, and checking the DB before each flow is not a bad solution IMO.

I'm okay with this solution if it's the one you guys prefer.

And again, can we set the email config in the same way as we set the connectors? Just for consistency. It's really weird to have two very similar configs be set in different ways.

[sym3tri]

can we set the email config in the same way as we set the connectors?

I am +1 for this.

EDIT:
However I strongly believe that connectors are stateful data, not app config. And that we really should support changing them at runtime.

[ericchiang]

From conversations yesterday:

• Connectors will stay in the DB. Consider encrypting them later.
• Try to move as much as possible into a config file. This includes email config and a bunch of options currently being passed as flags.

[chancez]

Any reason connectors couldn't also be optionally provided via config?

[bobbyrullo]

The connectors would get out of sync with what's in the file. I think it would be weird to have stuff in the config that is only supposed to "load once" the first time dex starts, and then just sits there.

[chancez]

@bobbyrullo This issue is true for most systems which have configuration per instance.

I don't think because there _could_ be out of sync configurations means we shouldn't support the feature. It's possible to setup your load balancer to send requests to the same worker while rotating configuration if necessary.

An example use case is, I have 2 different groups of dex-workers. One group which only has local-connectors configured, and one group which does only google. I can setup my load balancer to send someone to the right worker depending on for example, their location. Users from china cannot use Google, thus I could hide my google connector by sending users with chinese IPs to my "local only" worker, and then I could send everyone else to my "google only" worker. Again, this use-case is completely static configuration, without dynamic client registration.

I don't think the problem of syncing configs across multiple web instances is something we should concern ourselves with, particularly since it's something which can be handled via external systems.

[bobbyrullo]

Ah, so you are actually talking about per-instance config. I thought you were saying that the file loads it into the DB.

I don't think we want to support both having it in a file and in the database

[chancez]

@bobbyrullo Yeah, I meant a static config, not one which gets loaded into the DB. I think if this was to be added, it would be _either_ db based connectors, _or_ static config file based ones. It might be hard to enforce that, but we could explicitly document that we do not support mixing.

This is basically how we do things today on account.tectonic.com, because we don't really need the dynamic behavior currently, and this would help improve the deployment process for Dex significantly.

[sym3tri]

Chance, we already decided on a plan. We don't want to spend extra time
removing a feature now (the ability to configure connectors at runtime).
It's less work to leave it as-is, and saves us from doing something we may
have to undo later. We can reevaluate as more specific use-cases come to
light, but I don't see this as high priority right now.

On Fri, Apr 8, 2016 at 12:03 PM Chance Zibolski [email protected]
wrote:

@bobbyrullo https://github.com/bobbyrullo Yeah, I meant a static
config, not one which gets loaded into the DB. I think if this was to be
added, it would be _either_ db based connectors, _or_ static config file
based ones. It might be hard to enforce that, but we could explicitly
document that we do not support mixing.

This is basically how we do things today on account.tectonic.com, because
we don't really need the dynamic behavior currently, and this would help
improve the deployment process for Dex significantly.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/coreos/dex/issues/377#issuecomment-207560063

[bcwaldon]

Hey guys, sorry I didn't chime in earlier when @bobbyrullo pinged me. I added my use case at https://github.com/coreos/dex/issues/410. Let me know if I can add any more info

[chancez]

On my wishlist in addition to more static config, I would really like to see a way to create connectors idempotently. I currently handle this with external systems, but it would be nice if I could accomplish this using just dexctl/the dex api.

[ecordell]

I came across this issue when reviewing #436. See this line: https://github.com/coreos/dex/pull/436/files#diff-f1e8fb42f56cebe8f44f7667427adca6R37

A dexctl command sleeps for 10s to give dex time to start up, so that it can issue the command to create the connectors. If dex could read from a config file this could be skipped, and the config loaded into the container on startup.

[bobbyrullo]

@ecordell : please see https://github.com/coreos/dex/issues/377#issuecomment-207590632

[ericchiang]

Everything this issue has brought up can now be configured via a static config files. Some of these objects, such as OAuth2 clients can be managed through the gRPC API. Users are recommended to use config management or an orchestration framework to manage the static config values, such as connectors.