feat(auth/oidc): OIDC authentication method support #1197

GeorgeMac · 2022-12-06T10:53:22Z

Fixes FLI-48
Fixes FLI-49

This implements the OIDC authorize URL and callback operations. Initially, just for the single provider, Google.
It uses HashiCorp cap as the wrapper around the Go OIDC library and Googles OAuth lib.
This library has the added benefit of a nice testing authorization service implementation.

The two operations implemented in this PR are:

AuthorizeURL, which returns an authorize URL for the configuration associated with the requested provider.
Callback, which handles exchanging an authentication code to verify authenticity and acquire an id_token. Once acquired, a Flipt client token is established with the user's metadata.

Additionally, there are a number of utility middlewares and gRPC gateway options.
These additions ensure that when interacting with the OIDC authentication method, that the resulting client token is automatically established as an HTTP cookie. This is to support browser sessions.

Update:

This is now flexible enough to support various generic OIDC providers.
This is after merging in #1223

codecov-commenter · 2022-12-08T14:41:47Z

Codecov Report

Merging #1197 (f3709d5) into main (b4dc11f) will decrease coverage by 0.13%.
The diff coverage is 81.08%.

@@            Coverage Diff             @@
##             main    #1197      +/-   ##
==========================================
- Coverage   80.19%   80.05%   -0.14%     
==========================================
  Files          39       41       +2     
  Lines        2842     3113     +271     
==========================================
+ Hits         2279     2492     +213     
- Misses        459      499      +40     
- Partials      104      122      +18

Impacted Files	Coverage Δ
internal/server/auth/method/oidc/server.go	`67.52% <67.52%> (ø)`
internal/config/authentication.go	`83.33% <77.77%> (-5.04%)`	⬇️
internal/server/auth/method/oidc/http.go	`85.55% <85.55%> (ø)`
internal/config/config.go	`88.35% <97.10%> (+3.17%)`	⬆️
internal/server/middleware/grpc/middleware.go	`57.57% <100.00%> (-1.73%)`	⬇️

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

markphelps · 2022-12-12T01:45:48Z

internal/config/authentication.go

+		if v.GetBool(fmt.Sprintf("authentication.methods.%s.enabled", k)) {
+			method["cleanup"] = map[string]any{
+				"interval":     time.Hour,
+				"grace_period": 30 * time.Minute,


outside of the scope of this PR, but we should also track modifying the cue and jsonschema schemas for these config additions, as well as docs changes

Yeah good shout. Is this something we could potentially add as a CI step?

markphelps · 2022-12-12T01:47:30Z

internal/server/auth/method/oidc/http.go

+// appropriate state parameter. It does so by wrapping any provided state parameter
+// in a JSON object with an additional cryptographically-random generated security
+// token. The payload is then encoded in base64 and added back to the state query param.
+// The payload is then also encoded as a http cookie which is bound to the callback path.


love these docs/comments 🤩

markphelps · 2022-12-12T01:52:04Z

internal/server/auth/method/oidc/server.go

+	})
+	if err != nil {
+		return nil, err
+


Suggested change

markphelps · 2022-12-12T01:54:02Z

internal/server/auth/method/oidc/server_test.go

+// with the login form.
+// It collecs the hidden values into a url.Values so that we can post the form
+// using our Go client.
+func parseLoginFormHiddenValues(r io.Reader) (url.Values, error) {


this function is wild

😭 It definitely either sits-on or leaps over the line of acceptability 😂

I wanted to avoid bringing in more dependencies if I could. I just need to pull a particular form's hidden inputs out of an HTML page. But perhaps using a library would make this a lot more digestible.
What I really wanted was that test library to offer a way to get the code parameter without parsing HTML. I just haven't found it yet.
I guess, at-least, this is a little closer to what a user in a browser would do. Slightly more representative.

haha totally understand. just saw html parsing as was like 🤯

…o main

feat(auth/oidc): implement authorize and callback refactor(oidc): switch to hashicorp/cap package test(oidc): add extra logging context for failure in CI chore: more test logging context chore: logging context around client cookie jar contents chore: print entire cookie jar fix(oidc): test use localhost over ip for cookie jar interactions chore: appears the linters refactor(internal/cmd): flatten bootstrapping package into one

markphelps

lgtm!! this is huuuge

just one minor question/comment about error helpers

internal/server/evaluator.go

…dc-google

* feat(oidc): support generic OIDC providers * refactor(config): support maps with keys containing underscore * chore: appease the linters * chore(config): update doc string Co-authored-by: Mark Phelps <[email protected]> * chore(config): update bind function doc * feat(oidc): add more standard OIDC claims * chore(oidc): remove redundent return statement Co-authored-by: Mark Phelps <[email protected]>

markphelps

🤩

pull-request-size bot added the size/XXL label Dec 6, 2022

refactor(main): move grpc and http composition into internal/cmd

e61206a

GeorgeMac mentioned this pull request Dec 9, 2022

refactor(main): move grpc and http composition into internal/cmd #1216

Merged

chore(internal/cmd): unexport grpc register types

ddb5ee4

markphelps reviewed Dec 12, 2022

View reviewed changes

GeorgeMac added 6 commits December 12, 2022 10:51

Merge branch 'main' into gm/main-refactor

367b01e

chore(cmd/grpc): rename pushShutdown onShutdown and move clientConn t…

052d192

…o main

chore(method/oidc): improve docs around http middleware

607b74b

refactor(errors): add generic errors.NewErrorf

fc7a329

test(oidc): increase coverage around invalid state parameter on callback

3c8cc8d

GeorgeMac force-pushed the gm/oidc-google branch from 238e53b to 3c8cc8d Compare December 12, 2022 16:03

GeorgeMac added 2 commits December 12, 2022 16:05

chore: remove excess newline

590cc81

Merge branch 'main' into gm/oidc-google

c8a48c9

GeorgeMac marked this pull request as ready for review December 12, 2022 19:20

GeorgeMac requested a review from a team as a code owner December 12, 2022 19:20

GeorgeMac changed the title ~~draft(auth/oidc): complete implementation of the OIDC authentication method~~ feat(auth/oidc): OIDC authentication method support Dec 12, 2022

Merge branch 'main' into gm/oidc-google

42f2ccd

markphelps approved these changes Dec 13, 2022

View reviewed changes

internal/server/evaluator.go Outdated Show resolved Hide resolved

GeorgeMac and others added 5 commits December 13, 2022 17:05

fix(oidc): install missing auth middleware

01f0270

Merge branch 'gm/oidc-google' of github.com:flipt-io/flipt into gm/oi…

ba7da0d

…dc-google

chore(errors): restore Errorf helper functions

5f7b178

Merge branch 'main' into gm/oidc-google

85a7061

GeorgeMac requested a review from markphelps December 21, 2022 10:21

chore(errors): switch back to Errorf utility functions

f3709d5

markphelps approved these changes Dec 21, 2022

View reviewed changes

GeorgeMac merged commit 2ca03a7 into main Dec 22, 2022

GeorgeMac deleted the gm/oidc-google branch December 22, 2022 10:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(auth/oidc): OIDC authentication method support #1197

feat(auth/oidc): OIDC authentication method support #1197

GeorgeMac commented Dec 6, 2022 •

edited

Loading

codecov-commenter commented Dec 8, 2022 •

edited

Loading

markphelps Dec 12, 2022

GeorgeMac Dec 12, 2022 •

edited

Loading

markphelps Dec 13, 2022

markphelps Dec 12, 2022

markphelps Dec 12, 2022

markphelps Dec 12, 2022

GeorgeMac Dec 12, 2022

markphelps Dec 13, 2022

markphelps left a comment

markphelps left a comment

feat(auth/oidc): OIDC authentication method support #1197

feat(auth/oidc): OIDC authentication method support #1197

Conversation

GeorgeMac commented Dec 6, 2022 • edited Loading

codecov-commenter commented Dec 8, 2022 • edited Loading

Codecov Report

markphelps Dec 12, 2022

Choose a reason for hiding this comment

GeorgeMac Dec 12, 2022 • edited Loading

Choose a reason for hiding this comment

markphelps Dec 13, 2022

Choose a reason for hiding this comment

markphelps Dec 12, 2022

Choose a reason for hiding this comment

markphelps Dec 12, 2022

Choose a reason for hiding this comment

markphelps Dec 12, 2022

Choose a reason for hiding this comment

GeorgeMac Dec 12, 2022

Choose a reason for hiding this comment

markphelps Dec 13, 2022

Choose a reason for hiding this comment

markphelps left a comment

Choose a reason for hiding this comment

markphelps left a comment

Choose a reason for hiding this comment

GeorgeMac commented Dec 6, 2022 •

edited

Loading

codecov-commenter commented Dec 8, 2022 •

edited

Loading

GeorgeMac Dec 12, 2022 •

edited

Loading