Fix for missing client config

Author: AlexTugarev

components/iam/pkg/oidc/router.go

@@ -44,13 +44,13 @@ func (oidcService *OIDCService) getStartHandler() http.HandlerFunc {
 		log.Trace("at start handler")
 
 		ctx := r.Context()
-		config, ok := ctx.Value(keyOIDCClientConfig{}).(OIDCClientConfig)
+		config, ok := ctx.Value(keyOIDCClientConfig{}).(*OIDCClientConfig)
 		if !ok {
 			http.Error(rw, "config not found", http.StatusInternalServerError)
 			return
 		}
 
-		startParams, err := oidcService.GetStartParams(&config)
+		startParams, err := oidcService.GetStartParams(config)
 		if err != nil {
 			http.Error(rw, "failed to start auth flow", http.StatusInternalServerError)
 			return
@@ -99,7 +99,7 @@ func (oidcService *OIDCService) getCallbackHandler() http.HandlerFunc {
 		log.Trace("at callback handler")
 
 		ctx := r.Context()
-		config, ok := ctx.Value(keyOIDCClientConfig{}).(OIDCClientConfig)
+		config, ok := ctx.Value(keyOIDCClientConfig{}).(*OIDCClientConfig)
 		if !ok {
 			http.Error(rw, "config not found", http.StatusInternalServerError)
 			return

components/iam/pkg/oidc/router_test.go

@@ -0,0 +1,78 @@
+// 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 (
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"github.com/go-chi/chi/v5"
+	"github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/require"
+	"golang.org/x/oauth2"
+
+	goidc "github.com/coreos/go-oidc/v3/oidc"
+
+	"github.com/gitpod-io/gitpod/common-go/log"
+)
+
+func TestRoute_start(t *testing.T) {
+	log.Log.Logger.SetLevel(logrus.TraceLevel)
+
+	idpUrl := newFakeIdP(t)
+	baseUrl := newTestServer(t, idpUrl)
+
+	client := &http.Client{Timeout: 10 * time.Second}
+	resp, err := client.Get(baseUrl + "/oidc/start?issuer=" + idpUrl)
+	if err != nil {
+		t.Error(err)
+		t.FailNow()
+	}
+	defer resp.Body.Close()
+
+	body, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		t.Error(err)
+		t.FailNow()
+	}
+	t.Log(string(body))
+	require.Equal(t, 200, resp.StatusCode)
+	require.NotEqual(t, "config not found", string(body))
+}
+
+func newTestServer(t *testing.T, issuer string) string {
+	oidcService := NewOIDCService()
+	oidcConfig := &goidc.Config{
+		ClientID: "123",
+	}
+	oauth2Config := &oauth2.Config{
+		ClientID:     "123",
+		ClientSecret: "secret",
+		RedirectURL:  "http://localhost/callback",
+		Scopes:       []string{goidc.ScopeOpenID, "profile", "email"},
+	}
+	clientConfig := &OIDCClientConfig{
+		Issuer:       issuer,
+		ID:           "R4ND0M1D",
+		OAuth2Config: oauth2Config,
+		OIDCConfig:   oidcConfig,
+	}
+	err := oidcService.AddClientConfig(clientConfig)
+	if err != nil {
+		t.Error(err)
+		t.FailNow()
+	}
+
+	router := chi.NewRouter()
+	router.Mount("/oidc", Router(oidcService))
+
+	ts := httptest.NewServer(router)
+	url := ts.URL
+
+	return url
+}

components/iam/pkg/oidc/service_test.go

@@ -57,8 +57,7 @@ func TestGetStartParams(t *testing.T) {
 }
 
 func TestGetClientConfigFromRequest(t *testing.T) {
-	issuer, err := setupFakeIdP(t)
-	require.NoError(t, err)
+	issuer := newFakeIdP(t)
 
 	testCases := []struct {
 		Location      string
@@ -83,7 +82,7 @@ func TestGetClientConfigFromRequest(t *testing.T) {
 	}
 
 	service := NewOIDCService()
-	err = service.AddClientConfig(&OIDCClientConfig{
+	err := service.AddClientConfig(&OIDCClientConfig{
 		ID:           "google-1",
 		Issuer:       issuer,
 		OIDCConfig:   &oidc.Config{},
@@ -108,11 +107,10 @@ func TestGetClientConfigFromRequest(t *testing.T) {
 }
 
 func TestAuthenticate_nonce_check(t *testing.T) {
-	issuer, err := setupFakeIdP(t)
-	require.NoError(t, err)
+	issuer := newFakeIdP(t)
 
 	service := NewOIDCService()
-	err = service.AddClientConfig(&OIDCClientConfig{
+	err := service.AddClientConfig(&OIDCClientConfig{
 		ID:     "google-1",
 		Issuer: issuer,
 		OIDCConfig: &oidc.Config{
@@ -141,7 +139,7 @@ func TestAuthenticate_nonce_check(t *testing.T) {
 	require.NotNil(t, result)
 }
 
-func setupFakeIdP(t *testing.T) (string, error) {
+func newFakeIdP(t *testing.T) string {
 	router := chi.NewRouter()
 	ts := httptest.NewServer(router)
 	url := ts.URL
@@ -156,6 +154,12 @@ func setupFakeIdP(t *testing.T) (string, error) {
 			log.Fatal(err)
 		}
 	})
+	router.Get("/o/oauth2/v2/auth", func(w http.ResponseWriter, r *http.Request) {
+		_, err := w.Write([]byte(r.URL.RawQuery))
+		if err != nil {
+			log.Fatal(err)
+		}
+	})
 	router.Get("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) {
 		_, err := w.Write([]byte(fmt.Sprintf(`{
 			"issuer": "%[1]s",
@@ -216,10 +220,11 @@ func setupFakeIdP(t *testing.T) (string, error) {
 			]
 		   }`, url)))
 		if err != nil {
-			log.Fatal(err)
+			t.Error((err))
+			t.FailNow()
 		}
 	})
 
 	t.Cleanup(ts.Close)
-	return url, nil
+	return url
 }