search

weaveworks / weave-gitops

Weave GitOps provides insights into your application deployments, and makes continuous delivery with GitOps easier to adopt and scale across your teams.
https://docs.gitops.weave.works/
Apache License 2.0
927 stars 153 forks source link

Fix username/password flow #1722

Closed Callisto13 closed 2 years ago

Callisto13 commented 2 years ago

Describe the bug Right now if I configure a username and password to log in with, the same underlying User+Role is used.

I should be able to create a brand new user with all the trimmings and have that user restricted as its particular RBAC dictates.

Acceptance:

This may affect how the login works, as now it seems we look up a very specific secret and user. We may need to do something with labels.

luizbafilho commented 2 years ago

this will be fixed by https://github.com/weaveworks/weave-gitops/pull/1755

bigkevmcd commented 2 years ago

FWIW I think it'd be better to delegate to an OIDC provider rather than add support for further users.

To solve this problem, we should document how to configure the Role that the local user gets, rather than adding support for more users.

Dex provides local users, we don't need to.

ozamosi commented 2 years ago

~Dex does not provide local users.~

Nevermind, I misunderstood the manual.

bigkevmcd commented 2 years ago

You can configure users in your config as staticPasswords https://github.com/dexidp/dex/blob/master/examples/config-dev.yaml#L141

You can have multiple users in that YAML array

Callisto13 commented 2 years ago

Not saying that I disagree, but I understood the whole point of the Static/Admin user was for users who explicitly did not want OIDC? Wasn't this like a deliberate feature? (Granted it came from Pesto, so I may be missing context)

bigkevmcd commented 2 years ago

The goal was to allow for the initial user and for emergency use (when your OIDC provider is down).

Callisto13 commented 2 years ago

Ah gotcha. So do we want to unwind some of the User work which has recently come in? And only have the Admin flow

Callisto13 commented 2 years ago

bringing up in planning

bigkevmcd commented 2 years ago

To reiterate what has been said elsewhere, I think it's fine to have a name changeable admin user, that has very little impact, and arguably improves security slightly.

Further users, this is veering into "we're building you an identity server" and at the moment, I'm not convinced we want to do this.