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

Add WithAuth middleware #1268

Merged
merged 2 commits into from Jan 11, 2022
Merged

Add WithAuth middleware #1268

merged 2 commits into from Jan 11, 2022

Conversation

yiannistri
Copy link
Contributor

@yiannistri yiannistri commented Jan 5, 2022 •
edited

Part of: #1092

What changed?

  • Added feature flag to enable authentication flow
  • Added middleware that inspects cookie or auth header for an ID token
    • Unauthenticated API requests will 401
    • Unauthenticated web/html requests will redirect to OIDC issuer

Why?
The end goal is to execute calls to the Kubernetes API on behalf of the authenticated user.

How did you test it?
Unit tests and manually.

Release notes
Not yet, still behind a feature flag.

Documentation Changes
Not yet, still behind a feature flag.

Note that more changes (follow-up PRs) will be needed to actually use the identity of the authenticated user in subsequent calls to the Kubernetes API.

@yiannistri yiannistri added the type/enhancement New feature or request label Jan 5, 2022
@yiannistri yiannistri force-pushed the add-auth-feature-flag branch 8 times, most recently from 0e97266 to 5dff28e Compare January 6, 2022 19:09
@yiannistri yiannistri marked this pull request as ready for review January 6, 2022 20:37
jpellizzari
jpellizzari approved these changes Jan 6, 2022
View reviewed changes
Copy link
Contributor

@jpellizzari jpellizzari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very elegant and well thought out 👍 .

I don't fully grasp all of the OIDC stuff, but this is implemented in a low-risk way. Nice work.

cmd/gitops/ui/run/cmd.go Outdated Show resolved Hide resolved
pkg/server/auth/jwt.go Show resolved Hide resolved
pkg/server/handler.go Show resolved Hide resolved
bigkevmcd
bigkevmcd requested changes Jan 7, 2022
View reviewed changes
Copy link
Contributor

@bigkevmcd bigkevmcd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing major in here.

cmd/gitops/root/cmd.go Outdated Show resolved Hide resolved
cmd/gitops/ui/run/cmd.go Outdated Show resolved Hide resolved
cmd/gitops/ui/run/cmd.go Outdated Show resolved Hide resolved
cmd/gitops/ui/run/cmd.go Outdated Show resolved Hide resolved
cmd/gitops/ui/run/cmd.go Outdated Show resolved Hide resolved
pkg/server/auth/auth.go Outdated Show resolved Hide resolved
pkg/server/auth/auth.go Outdated Show resolved Hide resolved
pkg/server/auth/jwt.go Show resolved Hide resolved
pkg/server/auth/auth.go Outdated Show resolved Hide resolved
@yiannistri yiannistri force-pushed the add-auth-feature-flag branch 3 times, most recently from 8c21cf3 to c6dd7a4 Compare January 8, 2022 14:33
@yiannistri yiannistri force-pushed the add-auth-feature-flag branch 3 times, most recently from 8ed18fc to 25343ad Compare January 9, 2022 10:37
@yiannistri yiannistri added exclude from release notes and removed type/enhancement New feature or request labels Jan 9, 2022
@yiannistri yiannistri force-pushed the add-auth-feature-flag branch 5 times, most recently from 7525f8d to 60bf9cb Compare January 11, 2022 11:21
bigkevmcd
bigkevmcd approved these changes Jan 11, 2022
View reviewed changes
Copy link
Contributor

@bigkevmcd bigkevmcd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the work to make this better!

@yiannistri yiannistri merged commit 0d2aa5e into main Jan 11, 2022
@yiannistri yiannistri deleted the add-auth-feature-flag branch January 11, 2022 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bigkevmcd bigkevmcd bigkevmcd approved these changes

@jpellizzari jpellizzari jpellizzari approved these changes

Assignees
No one assigned
Labels
exclude from release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@yiannistri @bigkevmcd @jpellizzari