Add basic OIDC client #15267
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| { | ||
| // Use IntelliSense to learn about possible attributes. | ||
| // Hover to view descriptions of existing attributes. | ||
| // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 | ||
| "version": "0.2.0", | ||
| "configurations": [ | ||
| { | ||
| "name": "Debug client", | ||
| "type": "go", | ||
| "request": "launch", | ||
| "mode": "debug", | ||
| "program": "/workspace/gitpod/components/iam/main.go", | ||
| "args": ["run"] | ||
| } | ||
| ] | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| // Copyright (c) 2022 Gitpod GmbH. All rights reserved. | ||
| // Licensed under the GNU Affero General Public License (AGPL). | ||
| // See License-AGPL.txt in the project root for license information. | ||
|
|
||
| package oidc | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "fmt" | ||
| "os" | ||
| ) | ||
|
|
||
| // The demo config is used to setup a OIDC client with Google. | ||
| // | ||
| // This is a temporary way to boot the OIDC client service with a single | ||
| // configuration, e.g. mounted as secret into a preview environment. | ||
| // | ||
| // ‼️ This demo config will be removed once the configuration is read from DB. | ||
| type DemoConfig struct { | ||
| Issuer string `json:"issuer"` | ||
| ClientID string `json:"clientID"` | ||
| ClientSecret string `json:"clientSecret"` | ||
| RedirectURL string `json:"redirectURL"` | ||
| } | ||
|
|
||
| func ReadDemoConfigFromFile(path string) (*DemoConfig, error) { | ||
| bytes, err := os.ReadFile(path) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to read test config: %w", err) | ||
| } | ||
|
|
||
| var config DemoConfig | ||
| err = json.Unmarshal(bytes, &config) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to unmarshal config: %w", err) | ||
| } | ||
|
|
||
| return &config, nil | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| // Copyright (c) 2022 Gitpod GmbH. All rights reserved. | ||
| // Licensed under the GNU Affero General Public License (AGPL). | ||
| // See License-AGPL.txt in the project root for license information. | ||
|
|
||
| package oidc | ||
|
|
||
| import ( | ||
| "context" | ||
| "net/http" | ||
|
|
||
| "github.com/gitpod-io/gitpod/common-go/log" | ||
| "golang.org/x/oauth2" | ||
| ) | ||
|
|
||
| type OAuth2Result struct { | ||
| OAuth2Token *oauth2.Token | ||
| Redirect string | ||
| } | ||
|
|
||
| type keyOAuth2Result struct{} | ||
|
|
||
| func OAuth2Middleware(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { | ||
| log.Trace("at oauth2 middleware") | ||
| ctx := r.Context() | ||
| config, ok := ctx.Value(keyOIDCClientConfig{}).(OIDCClientConfig) | ||
| if !ok { | ||
| http.Error(rw, "config not found", http.StatusInternalServerError) | ||
| return | ||
| } | ||
|
|
||
| // http-only cookie written during flow start request | ||
| stateCookie, err := r.Cookie(stateCookieName) | ||
| if err != nil { | ||
| http.Error(rw, "state cookie not found", http.StatusBadRequest) | ||
| return | ||
| } | ||
| // the starte param passed back from IdP | ||
| stateParam := r.URL.Query().Get("state") | ||
| if stateParam == "" { | ||
| http.Error(rw, "state param not found", http.StatusBadRequest) | ||
| return | ||
| } | ||
| // on mismatch, obviously there is a client side error | ||
| if stateParam != stateCookie.Value { | ||
| http.Error(rw, "state did not match", http.StatusBadRequest) | ||
| return | ||
| } | ||
|
|
||
| code := r.URL.Query().Get("code") | ||
| if code == "" { | ||
| http.Error(rw, "code param not found", http.StatusBadRequest) | ||
| return | ||
| } | ||
|
|
||
| oauth2Token, err := config.OAuth2Config.Exchange(ctx, code) | ||
| if err != nil { | ||
| http.Error(rw, "failed to exchange token: "+err.Error(), http.StatusInternalServerError) | ||
| return | ||
| } | ||
|
|
||
| ctx = context.WithValue(ctx, keyOAuth2Result{}, OAuth2Result{ | ||
| OAuth2Token: oauth2Token, | ||
| }) | ||
| next.ServeHTTP(rw, r.WithContext(ctx)) | ||
| }) | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,16 +5,149 @@ | |
| package oidc | ||
|
|
||
| import ( | ||
| "context" | ||
| "encoding/json" | ||
| "net/http" | ||
| "time" | ||
|
|
||
| "github.com/gitpod-io/gitpod/common-go/log" | ||
| "golang.org/x/oauth2" | ||
|
|
||
| "github.com/go-chi/chi/v5" | ||
| ) | ||
|
|
||
| func Router(oidcService *OIDCService) *chi.Mux { | ||
| router := chi.NewRouter() | ||
|
|
||
| router.Route("/start", func(r chi.Router) { | ||
| r.Use(oidcService.clientConfigMiddleware()) | ||
| r.Get("/", oidcService.getStartHandler()) | ||
| }) | ||
| router.Route("/callback", func(r chi.Router) { | ||
| r.Use(oidcService.clientConfigMiddleware()) | ||
| r.Use(OAuth2Middleware) | ||
| r.Get("/", oidcService.getCallbackHandler()) | ||
| }) | ||
|
|
||
| return router | ||
| } | ||
|
|
||
| type keyOIDCClientConfig struct{} | ||
|
|
||
| const ( | ||
| stateCookieName = "state" | ||
| nonceCookieName = "nonce" | ||
| ) | ||
|
|
||
| func (oidcService *OIDCService) getStartHandler() http.HandlerFunc { | ||
| return func(rw http.ResponseWriter, r *http.Request) { | ||
| log.Trace("at start handler") | ||
|
|
||
| ctx := r.Context() | ||
| config, ok := ctx.Value(keyOIDCClientConfig{}).(OIDCClientConfig) | ||
| if !ok { | ||
| http.Error(rw, "config not found", http.StatusInternalServerError) | ||
| return | ||
| } | ||
|
|
||
| startParams, err := oidcService.GetStartParams(&config) | ||
| if err != nil { | ||
| http.Error(rw, "failed to start auth flow", http.StatusInternalServerError) | ||
| return | ||
| } | ||
|
|
||
| http.SetCookie(rw, newCallbackCookie(r, nonceCookieName, startParams.Nonce)) | ||
| http.SetCookie(rw, newCallbackCookie(r, stateCookieName, startParams.State)) | ||
|
|
||
| http.Redirect(rw, r, startParams.AuthCodeURL, http.StatusTemporaryRedirect) | ||
| } | ||
| } | ||
|
|
||
| func newCallbackCookie(r *http.Request, name string, value string) *http.Cookie { | ||
| return &http.Cookie{ | ||
| Name: name, | ||
| Value: value, | ||
| MaxAge: int(10 * time.Minute.Seconds()), | ||
| Secure: r.TLS != nil, | ||
| SameSite: http.SameSiteLaxMode, | ||
| HttpOnly: true, | ||
| } | ||
| } | ||
|
|
||
| // The config middleware is responsible to retrieve the client config suitable for request | ||
| func (oidcService *OIDCService) clientConfigMiddleware() func(http.Handler) http.Handler { | ||
| return func(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { | ||
| log.Trace("at config middleware") | ||
AlexTugarev marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| config, err := oidcService.GetClientConfigFromRequest(r) | ||
| if err != nil { | ||
| log.Warn("client config not found: " + err.Error()) | ||
| http.Error(rw, "config not found", http.StatusNotFound) | ||
| return | ||
| } | ||
|
|
||
| ctx := context.WithValue(r.Context(), keyOIDCClientConfig{}, config) | ||
| next.ServeHTTP(rw, r.WithContext(ctx)) | ||
AlexTugarev marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }) | ||
| } | ||
| } | ||
|
|
||
| // The OIDC callback handler depends on the state produced in the OAuth2 middleware | ||
| func (oidcService *OIDCService) getCallbackHandler() http.HandlerFunc { | ||
| return func(rw http.ResponseWriter, r *http.Request) { | ||
| log.Trace("at callback handler") | ||
|
|
||
| ctx := r.Context() | ||
| config, ok := ctx.Value(keyOIDCClientConfig{}).(OIDCClientConfig) | ||
| if !ok { | ||
| http.Error(rw, "config not found", http.StatusInternalServerError) | ||
| return | ||
| } | ||
| oauth2Result, ok := ctx.Value(keyOAuth2Result{}).(OAuth2Result) | ||
| if !ok { | ||
| http.Error(rw, "OIDC precondition failure", http.StatusInternalServerError) | ||
| return | ||
| } | ||
|
|
||
| // nonce = number used once | ||
| nonceCookie, err := r.Cookie(nonceCookieName) | ||
| if err != nil { | ||
| http.Error(rw, "nonce not found", http.StatusBadRequest) | ||
| return | ||
| } | ||
|
|
||
| result, err := oidcService.Authenticate(ctx, &oauth2Result, | ||
| config.Issuer, nonceCookie.Value) | ||
| if err != nil { | ||
| http.Error(rw, "OIDC authentication failed", http.StatusInternalServerError) | ||
| return | ||
| } | ||
|
|
||
| // TODO(at) given the result of OIDC authN, let's proceed with the redirect | ||
|
|
||
| // For testing purposes, let's print out redacted results | ||
| oauth2Result.OAuth2Token.AccessToken = "*** REDACTED ***" | ||
|
|
||
| var claims map[string]interface{} | ||
| err = result.IDToken.Claims(&claims) | ||
| if err != nil { | ||
| http.Error(rw, err.Error(), http.StatusInternalServerError) | ||
|
There was a problem hiding this comment. This should be a user error, no? If the user supplies the wrong claim, we shouldn't return a Server error. There was a problem hiding this comment. Claims is what Gitpod (as the OIDC client) can obtain from the end user. Not being able to parse the
Also, while the error modes are still relevant, following PRs will remove this section. |
||
| return | ||
| } | ||
| resp := struct { | ||
| OAuth2Token *oauth2.Token | ||
| Claims map[string]interface{} | ||
| }{oauth2Result.OAuth2Token, claims} | ||
|
|
||
| data, err := json.MarshalIndent(resp, "", " ") | ||
| if err != nil { | ||
| http.Error(rw, err.Error(), http.StatusInternalServerError) | ||
| return | ||
| } | ||
| _, err = rw.Write(data) | ||
| if err != nil { | ||
| http.Error(rw, err.Error(), http.StatusInternalServerError) | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd strongly recommend not to over-use middlewares which extract concrete data. Here's why:
express.Requestcontext. The context in go is used for downstream calls irrespective of whether it's an http handler or not. Using the context to pass request specific arguments results in carry extra baggage.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point in using middleware here is to be made clear: this is to separate protocol implementations. First there is the OAuth2 layer, then the OIDC AuthN validation. It seems to be an acceptable cost if you consider that the OAuth2 layer can be reused for Git Auth later with a different handler.
The point on configuration and lax contract, indeed that's key-value onion might be fragile and requires complete coverage and precondition checks. I'll make improvements to get the coverage needed.
Do you have examples, or docs for that? I read different and saw different examples, see https://github.com/go-chi/chi and comparable, they advertising it as such. Could you name concrete problems with that?
I see the usage of middleware is justified here, especially for the separation of two protocols, where one is an extension to the other. The outcome of the first part can/must be checked and verified. To test the first part separately, one would need to slice it anyways. Same applies for the testability of the second part.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That said, checking for a solid connection should be a must here, thus it's worth revisiting tests. 👍🏻