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
919 stars 153 forks source link

`--custom-oidc-scopes` flag has no effect #3300

Open makkes opened 1 year ago

makkes commented 1 year ago

Describe the bug

When using OIDC then providing custom scopes via the --custom-oidc-scopes flag to the gitops-server binary doesn't result in the server actually requesting the scopes defined in the flag value.

Environment

To Reproduce

  1. Create the oidc-auth Secret with valid OIDC parameters
  2. gitops create dashboard wego --password=foo --export | yq e 'select(.kind == "HelmRelease") as $hr | select(.kind != "HelmRelease") as $o |$hr.spec.values.additionalArgs=["--custom-oidc-scopes=foo,bar"] | ($hr, $o)' | k apply -f-
  3. k -n flux-system port-forward svc/wego-weave-gitops 9001
  4. In the browser navigate to http://localhost:9001
  5. Open the developer tools of the browser to see network requests
  6. Click on "LOGIN WITH OIDC PROVIDER"
  7. Look at the list of requests and see that the Oauth2 request's scope URL query parameter doesn't have any of the values provided through the --custom-oidc-scopes flag

Expected behavior

The server should requests the scopes provided in the flag value.

Actual Behavior

The scopes provided in the flag value aren't used.

Additional Context (screenshots, logs, etc)

The reason for this happening is this line where the value is always overridden.

foot commented 1 year ago

Yes, right now there is an implicit and probably undoc'd behaviour where all config should specified in either but not both of:

If secret is found, everything specified on the cli will be ignored.

Solutions:

makkes commented 1 year ago

If you asked me I would ditch all the OIDC-related CLI flags, especially the client secret one as we likely don't want to encourage our users to have secrets lurking in Deployment resources (that usually aren't as well protected as Secrets in both, the cluster and also the source of truth, i.e. Git repository or the like).

Alternatively use viper and let it handle precendence for us.