Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WG529 Auth styling and flags improvements #1624

Merged
merged 13 commits into from
Mar 11, 2022
Merged

WG529 Auth styling and flags improvements #1624

merged 13 commits into from
Mar 11, 2022

Conversation

AlinaGoaga
Copy link
Contributor

@AlinaGoaga AlinaGoaga commented Mar 7, 2022 •
edited by yiannistri
Loading

Closes:

What changed?

  • Styling: updated background / welcome screen and user settings section
  • Introduced username input (with validation)
  • sign_in route redirects to applications page if auth flag is not switched on
  • sign in error does not persist anymore when user navigates away
  • If the user hasn't set up a password and / or OIDC we don't show the option to log in using that
  • Add link to docs on Welcome Screen
  • Default to CLI params for OIDC if config not present in Cluster

Documentation Changes
Added in: #1599

Use to log in with username/password (admin/my-secret-password):

apiVersion: v1
kind: Secret
metadata:
  name: cluster-user-auth
  namespace: wego-system
type: Opaque
data:
  password: JDJhJDEwJExLeXBUSUpFYlpkT0ZYMmJyQ29HN2Vta1QydjBwSkJ5UzRoSnFZdlFKWmRzQzdyN2NFWE15
  username: YWRtaW4=

Use to log in with OIDC (if this is not added to the cluster you can pass the CLI params in e.g: --oidc-issuer-url https://dex-01.wge.dev.weave.works/ --oidc-client-id weave-gitops --oidc-client-secret ZXhhbXBsZS1hcHAtc2VjcmV0 --oidc-redirect-url https://localhost:4567/oauth2/callback):

apiVersion: v1
data:
  clientID: d2VhdmUtZ2l0b3Bz
  clientSecret: WlhoaGJYQnNaUzFoY0hBdGMyVmpjbVYw
  issuerURL: aHR0cHM6Ly9kZXgtMDEud2dlLmRldi53ZWF2ZS53b3Jrcw==
  redirectURL: aHR0cHM6Ly9sb2NhbGhvc3Q6NDU2Ny9vYXV0aDIvY2FsbGJhY2s=
kind: Secret
metadata:
  name: oidc-auth
  namespace: wego-system

@AlinaGoaga AlinaGoaga added the type/enhancement New feature or request label Mar 8, 2022
@AlinaGoaga AlinaGoaga changed the title WG529 Styling User Settings WG529 Auth styling and flags improvements Mar 8, 2022
@AlinaGoaga AlinaGoaga requested a review from foot March 9, 2022 20:35
@AlinaGoaga AlinaGoaga marked this pull request as ready for review March 9, 2022 20:38
@AlinaGoaga
Copy link
Contributor Author

@foot Hey Simon! Got one issue here which seems to have to do with the proxy but not sure how to fix. If you log in with OIDC, you start with http://localhost:4567 go through the flow but the callback/redirect doesn't work as expected as it's still using https rather than coming back to http.
Screenshot 2022-03-09 at 20 46 30

@foot
Copy link
Contributor

foot commented Mar 10, 2022

Can you change to --oidc-redirect-url=http://localhost:4567?

@AlinaGoaga
Copy link
Contributor Author

Yup sorry I left 9001 in the example, I am testing with WEAVE_GITOPS_AUTH_ENABLED=true go run ./cmd/gitops/main.go ui run --oidc-issuer-url https://dex.dev.wkp.weave.works --oidc-client-id weave-gitops --oidc-client-secret ZXhhbXBsZS1hcHAtc2VjcmV0 --oidc-redirect-url http://localhost:4567/oauth2/callback --log. In the above example aHR0cDovL2xvY2FsaG9zdDo0NTY3L29hdXRoMi9jYWxsYmFjaw== is also the encoding of http://localhost:4567/oauth2/callback. I will adjust the port now in the PR description too

@Callisto13 Callisto13 mentioned this pull request Mar 11, 2022
Closed
5 tasks
@yiannistri yiannistri merged commit f7098fb into main Mar 11, 2022
@yiannistri yiannistri deleted the auth-updates branch March 11, 2022 15:06
ozamosi pushed a commit that referenced this pull request Mar 14, 2022
Merge 'v2' branch into 'main'
febc1f1 
There were quite a few conflicts, but many of them were very easy to
resolve: anything related to acceptance tests has been deleted,
lots of import conflicts, conficts in generated files, and some things
that don't have a common ancestor and yet have almost the same content
which makes git twitchy.

There were a few more significant conflicts I've dealt with manually:

* There's 2 independent implementations of https listeners (#1537,
  #1483). This also means it's been implemented in two places, `gitops
  run ui` and `gitops-server`. Keep the rate limiting from the main
  one, but otherwise mostly stick to the v2 one - it's been written to
  only work in docker. This means no more certificate generation code:
  v2 expects you to mount your certificates into the pod.
  TODO this deletes all gitops-server tests because they're all
  incompatible
* The auth system in main has grown a username, which isn't present in
  v2. Add it to relevant files.
* gitops profile upgrade is new. I've not really investigated how it
  works or if it still works, I've just adapted the API to the kind of
  changes we've done and checked that the tests pass.
* There's been a lot of tilt rewrites in both branches. Since what you
  need to deploy the app and what it needs has changed in v2, I've
  mostly ignored main.
* v2 and main disagreed on how to embed many svg images - v2 imported
  the images via filesystem path where they reside, while main
  imported them through images.ts. I've followed the v2 pattern.
* Commit "Get auth working" (fdd15fb) in v2 rewrote how feature flags
  work, while main continued developing the old API. I've ported the
  new main code to the v2 API.
* UserSettings had conflicts introduced in "Fix proxy port and user
  settings styling" (84bdc98) vs "WG529 Auth styling and flags
  improvements (#1624)" (f7098fb), I've tried to merge those.
* Both main and v2 have turned a bunch of hard-coded auth secret name
  strings into constants. They've placed those constants in different
  places and given them different values. I gave up on trying to make
  this good and sensible and decided to just make everything compile
  and pass tests and start, and am looking forward to someone asking
  themselves "who would do this!?"
ozamosi pushed a commit that referenced this pull request Mar 14, 2022
Merge 'v2' branch into 'main'
6e9dd50 
There were quite a few conflicts, but many of them were very easy to
resolve: anything related to acceptance tests has been deleted,
lots of import conflicts, conficts in generated files, and some things
that don't have a common ancestor and yet have almost the same content
which makes git twitchy.

There were a few more significant conflicts I've dealt with manually:

* There's 2 independent implementations of https listeners (#1537,
  #1483). This also means it's been implemented in two places, `gitops
  run ui` and `gitops-server`. Keep the rate limiting from the main
  one, but otherwise mostly stick to the v2 one - it's been written to
  only work in docker. This means no more certificate generation code:
  v2 expects you to mount your certificates into the pod.
* The auth system in main has grown a username, which isn't present in
  v2. Add it to relevant files.
* gitops profile upgrade is new. I've not really investigated how it
  works or if it still works, I've just adapted the API to the kind of
  changes we've done and checked that the tests pass.
* There's been a lot of tilt rewrites in both branches. Since what you
  need to deploy the app and what it needs has changed in v2, I've
  mostly ignored main.
* v2 and main disagreed on how to embed many svg images - v2 imported
  the images via filesystem path where they reside, while main
  imported them through images.ts. I've followed the v2 pattern.
* Commit "Get auth working" (fdd15fb) in v2 rewrote how feature flags
  work, while main continued developing the old API. I've ported the
  new main code to the v2 API.
* UserSettings had conflicts introduced in "Fix proxy port and user
  settings styling" (84bdc98) vs "WG529 Auth styling and flags
  improvements (#1624)" (f7098fb), I've tried to merge those.
* Both main and v2 have turned a bunch of hard-coded auth secret name
  strings into constants. They've placed those constants in different
  places and given them different values. I gave up on trying to make
  this good and sensible and decided to just make everything compile
  and pass tests and start.
@ozamosi ozamosi mentioned this pull request Mar 14, 2022
Merged
ozamosi pushed a commit that referenced this pull request Mar 14, 2022
Merge 'v2' branch into 'main'
48d138d 
There were quite a few conflicts, but many of them were very easy to
resolve: anything related to acceptance tests has been deleted,
lots of import conflicts, conficts in generated files (which I've just
re-generated and ignored), and some things that don't have a common
ancestor and yet have almost the same content which makes git twitchy.

There were a few more significant conflicts I've dealt with manually:

* There's 2 independent implementations of https listeners (#1537,
  #1483). This also means it's been implemented in two places, `gitops
  run ui` and `gitops-server`. Keep the rate limiting from the main
  one, but otherwise mostly stick to the v2 one - it's been written to
  only work in docker. This means no more certificate generation code:
  v2 expects you to mount your certificates into the pod.
* The auth system in main has grown a username, which isn't present in
  v2. Add it to relevant files.
* gitops profile upgrade is new. I've not really investigated how it
  works or if it still works, I've just adapted the API to the kind of
  changes we've done and checked that the tests pass.
* There's been a lot of tilt rewrites in both branches. Since what you
  need to deploy the app and what it needs has changed in v2, I've
  mostly ignored main.
* v2 and main disagreed on how to embed many svg images - v2 imported
  the images via filesystem path where they reside, while main
  imported them through images.ts. I've followed the v2 pattern.
* Commit "Get auth working" (fdd15fb) in v2 rewrote how feature flags
  work, while main continued developing the old API. I've ported the
  new main code to the v2 API.
* UserSettings had conflicts introduced in "Fix proxy port and user
  settings styling" (84bdc98) vs "WG529 Auth styling and flags
  improvements (#1624)" (f7098fb), I've tried to merge those.
* Both main and v2 have turned a bunch of hard-coded auth secret name
  strings into constants. They've placed those constants in different
  places and given them different values. I gave up on trying to make
  this good and sensible and decided to just make everything compile
  and pass tests and start.
ozamosi pushed a commit that referenced this pull request Mar 14, 2022
Merge 'v2' branch into 'main'
e9abe1e 
There were quite a few conflicts, but many of them were very easy to
resolve: anything related to acceptance tests has been deleted,
lots of import conflicts, conficts in generated files (which I've just
re-generated and ignored), and some things that don't have a common
ancestor and yet have almost the same content which makes git twitchy.

There were a few more significant conflicts I've dealt with manually:

* There's 2 independent implementations of https listeners (#1537,
  #1483). This also means it's been implemented in two places, `gitops
  run ui` and `gitops-server`. Keep the rate limiting from the main
  one, but otherwise mostly stick to the v2 one - it's been written to
  only work in docker. This means no more certificate generation code:
  v2 expects you to mount your certificates into the pod.
* The auth system in main has grown a username, which isn't present in
  v2. Add it to relevant files.
* gitops profile upgrade is new. I've not really investigated how it
  works or if it still works, I've just adapted the API to the kind of
  changes we've done and checked that the tests pass.
* There's been a lot of tilt rewrites in both branches. Since what you
  need to deploy the app and what it needs has changed in v2, I've
  mostly ignored main.
* v2 and main disagreed on how to embed many svg images - v2 imported
  the images via filesystem path where they reside, while main
  imported them through images.ts. I've followed the v2 pattern.
* Commit "Get auth working" (fdd15fb) in v2 rewrote how feature flags
  work, while main continued developing the old API. I've ported the
  new main code to the v2 API.
* UserSettings had conflicts introduced in "Fix proxy port and user
  settings styling" (84bdc98) vs "WG529 Auth styling and flags
  improvements (#1624)" (f7098fb), I've tried to merge those.
* Both main and v2 have turned a bunch of hard-coded auth secret name
  strings into constants. They've placed those constants in different
  places and given them different values. I gave up on trying to make
  this good and sensible and decided to just make everything compile
  and pass tests and start.
@Callisto13 Callisto13 mentioned this pull request Mar 23, 2022
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yiannistri yiannistri yiannistri approved these changes

@foot foot Awaiting requested review from foot

Assignees
No one assigned
Labels
team/pesto type/enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@AlinaGoaga @foot @yiannistri