configurable-passwordDB

[alhama7a]

This change addresses #76, allowing enablePasswordDB to be disabled. Changes are:

• The enablePasswordDB flag is now configurable
• Updated the unit test to cover the new configuration

[ca-scribner]

Hey @alhama7a, thanks for the contribution!

I tried to look up this feature but didn't see a lot of documentation in Dex's docs. Can you explain how it works? I understand the way to enable it and that seems good, but I'm not sure how a user interacts with this feature once it is enabled.

ty!

[alhama7a]

Hey @alhama7a, thanks for the contribution!

I tried to look up this feature but didn't see a lot of documentation in Dex's docs. Can you explain how it works? I understand the way to enable it and that seems good, but I'm not sure how a user interacts with this feature once it is enabled.

ty!

Hi @ca-scribner AFAIK, the enablePasswordDB option allows dex to store the static-passwords in it's configured data store (e.g. etcd or MySQL).

If the enablePasswordDB is disabled and static-passwords are provided in the config, then dex will complain and raise an error.

if not enablePasswordDB: then users that are defined under static-passowrds won't be able to authenticate. In this case, a centralized IAM such as LDAP must be configured under the connectors section of dex configuration to authenticate and authorize LDAP users via passwords and group assignment respectively. Many thanks for reviewing this PR. Really appreciated.

[ca-scribner]

Ok makes more sense now. Adding this lets an admin deploying this charm disable the storing of static passwords.

Whoever reviews this more, pay attention to https://github.com/canonical/dex-auth-operator/blob/cdf0f5e7946ce7e02a09c3a1df7bdf973480b4c8/src/charm.py#L126-L135

I wonder if we need to refactor the charm a little to not send a static user at all if this is disabled?

[beliaev-maksim]

@ca-scribner @i-chvets Just checking here. What if we change defaults instead of config options.

What is more secure and what is the best UX?

[alhama7a]

Ok makes more sense now. Adding this lets an admin deploying this charm disable the storing of static passwords.

Whoever reviews this more, pay attention to

https://github.com/canonical/dex-auth-operator/blob/cdf0f5e7946ce7e02a09c3a1df7bdf973480b4c8/src/charm.py#L126-L135

I wonder if we need to refactor the charm a little to not send a static user at all if this is disabled?

I agree. It'd be a good idea to ignore static user config if the enablePasswordDB flag is disabled. Shall we do that @ca-scribner?

[alhama7a]

@ca-scribner @i-chvets Just checking here. What if we change defaults instead of config options.

What is more secure and what is the best UX?

Sorry for answering a question not addressed to me, but I can't help it. It's just an opinion :) Setting the enablePasswordDB option to true by default is necessary for both, casual users and enterprise admins, because this is how they will initially authenticate. Casual users will most likely continue to use the static user config. Admins on the other hand, may at some point choose to disable this option for security reasons. That's just my opinion. Looking forward to hearing from @ca-scribner and @i-chvets

[beliaev-maksim]

hi @alhama7a, thanks for putting the PR

we will need to perform some manual testing and currently we are under the time pressure due to the end of the cycle. We will come back to you right after the sprint

[alhama7a]

hi @alhama7a, thanks for putting the PR

we will need to perform some manual testing and currently we are under the time pressure due to the end of the cycle. We will come back to you right after the sprint

Thanks @beliaev-maksim. I do appreciate how busy your team is; no pressure and thanks for letting me know. Best of luck everyone :+1:

[DnPlas]

Hi @alhama7a, thanks for your contribution and providing insights on how this configuration option could be used by different types of users.

The team has analyzed having a charm configuration for enabling or disabling the passwordDB, but since this can be done through the connector configuration (see Dex connectors), having it as a charm configuration may not be the best approach.

Talking about #76, the proposed fix there will actually work: disabling the passwordDB will automatically disable the static username and password login as Dex won't have a store to save them and act as a "virtual" IdP. Yet again, disabling it can be done from the connector configuration.

Because of this, I propose closing this PR and continue the conversation about configuring Dex in #130. Feel free to re-open this PR if any of the statements above do not suffice.

[DnPlas]

In my previous comment I mentioned the connectors configuration (a Dex concept) could be passed to the charm as a full yaml file that could include configuration options such as enablePasswordDB. After dicussing with @kimwnasptd about this, we have agreed that the aforementioned approach goes against the core values of charms and have agreed to enable this configuration option to the charm, thus I'm re-opening this PR again and approving in case we can merge this as is.

[alhama7a]

Thanks @DnPlas. I have updated the files of this PR based in the latest main branch and re-uploaded them. Please check the tests status in my repo. and also let me know if there are any additional changes are required.

[DnPlas]

Closing in favour of #149, thanks @alhama7a