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.

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

feat(server/auth): validate flipt_client_token cookie in middleware #1139

Merged
merged 13 commits into from Dec 12, 2022

Conversation

GeorgeMac
Copy link
Contributor

@GeorgeMac GeorgeMac commented Nov 16, 2022

Supports #779

Fixes FLI-50

This adds support for passing a Flipt client token via the flipt_client_token key in the Cookie header.
This is necessary to support browser/cookie-based sessions.

The middleware now checks for the presence of either header key Authorization or Cookie.
Depending on which key is present, it parses them appropriately.

One additional change is optional support for skipping auth.
This is done by passing the pointer of the server instance to WithServerSkipsAuthentication(server).
The OIDC server implementation itself requires open access. Since it provides delegated authentication via an Authentication Server.
The authorize URL action simply returns a string which points to the delegated authenticator.
The callback URL must be provided with a valid and verifiable code for auth to be granted.

@GeorgeMac GeorgeMac requested a review from a team as a code owner November 23, 2022 12:14
@codecov-commenter
Copy link

codecov-commenter commented Nov 23, 2022

Codecov Report

Merging #1139 (3d80114) into main (edc61fb) will increase coverage by 0.27%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1139      +/-   ##
==========================================
+ Coverage   79.87%   80.15%   +0.27%     
==========================================
  Files          38       38              
  Lines        2758     2796      +38     
==========================================
+ Hits         2203     2241      +38     
  Misses        451      451              
  Partials      104      104              
Impacted Files Coverage Δ
internal/server/auth/middleware.go 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

// which skips authentication when the provided server instance matches the intercepted
// calls parent server instance.
// This allows the caller to registers servers which explicitly skip authentication (e.g. OIDC).
func WithServerSkipsAuthentication(server any) containers.Option[InterceptorOptions] {
Copy link
Collaborator

Choose a reason for hiding this comment

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

is there anyway to identify a server by name instead of doing pointer comparison (when checking for if it's in the o.skippedServers[])?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think there might be. We could do a comparison on the string of the full method: https://pkg.go.dev/google.golang.org/grpc#UnaryServerInfo

However, I felt this was relatively more concrete. Since it is direct pointer comparison with the instance we want to skip auth on. We compare with the method name and then someone renames a package or type, and it moves under us.
While this does use any you still have to pass it something and if that something gets renamed it won't compile until you correct it.

@GeorgeMac GeorgeMac merged commit 6fe76d0 into main Dec 12, 2022
@GeorgeMac GeorgeMac deleted the gm/oidc-middleware branch December 12, 2022 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants