wip

Author: AlexTugarev

components/iam/pkg/oidc/router.go

@@ -82,7 +82,9 @@ func (oidcService *OIDCService) clientConfigMiddleware() func(http.Handler) http
 
 			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)

components/iam/pkg/oidc/service.go

@@ -14,6 +14,7 @@ import (
 	"net/url"
 
 	"github.com/coreos/go-oidc/v3/oidc"
+	"github.com/gitpod-io/gitpod/common-go/log"
 	"github.com/go-chi/chi/v5"
 	"golang.org/x/oauth2"
 )
@@ -106,6 +107,7 @@ func (service *OIDCService) GetClientConfigFromRequest(r *http.Request) (*OIDCCl
 	if err != nil {
 		return nil, errors.New("bad issuer param")
 	}
+	log.WithField("issuer", issuer).Trace("at GetClientConfigFromRequest")
 
 	for _, value := range service.configsById {
 		if value.Issuer == issuer {

components/iam/pkg/oidc/service_test.go

@@ -6,11 +6,16 @@ package oidc
 
 import (
 	"context"
+	"fmt"
+	"log"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"testing"
 
 	"github.com/coreos/go-oidc/v3/oidc"
+	"github.com/go-chi/chi/v5"
+	"github.com/go-chi/chi/v5/middleware"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/oauth2"
 )
@@ -53,9 +58,9 @@ func TestGetStartParams(t *testing.T) {
 }
 
 func TestGetClientConfigFromRequest(t *testing.T) {
-	const (
-		issuerG = "https://accounts.google.com"
-	)
+	issuer, err := setupFakeIdP(t)
+	require.NoError(t, err)
+
 	testCases := []struct {
 		Location      string
 		ExpectedError bool
@@ -67,7 +72,7 @@ func TestGetClientConfigFromRequest(t *testing.T) {
 			ExpectedId:    "",
 		},
 		{
-			Location:      "/start?issuer=https%3A%2F%2Faccounts.google.com",
+			Location:      "/start?issuer=" + url.QueryEscape(issuer),
 			ExpectedError: false,
 			ExpectedId:    "google-1",
 		},
@@ -79,9 +84,9 @@ func TestGetClientConfigFromRequest(t *testing.T) {
 	}
 
 	service := NewOIDCService()
-	err := service.AddClientConfig(&OIDCClientConfig{
+	err = service.AddClientConfig(&OIDCClientConfig{
 		ID:           "google-1",
-		Issuer:       issuerG,
+		Issuer:       issuer,
 		OIDCConfig:   &oidc.Config{},
 		OAuth2Config: &oauth2.Config{},
 	})
@@ -103,34 +108,119 @@ func TestGetClientConfigFromRequest(t *testing.T) {
 	}
 }
 
-func TestAuthenticate(t *testing.T) {
-	t.Skip() //
-	const (
-		issuerG = "https://accounts.google.com"
-	)
+func TestAuthenticate_nonce_check(t *testing.T) {
+	issuer, err := setupFakeIdP(t)
+	require.NoError(t, err)
+
 	service := NewOIDCService()
-	err := service.AddClientConfig(&OIDCClientConfig{
+	err = service.AddClientConfig(&OIDCClientConfig{
 		ID:     "google-1",
-		Issuer: issuerG,
+		Issuer: issuer,
 		OIDCConfig: &oidc.Config{
-			SkipClientIDCheck: true,
+			SkipClientIDCheck:          true,
+			SkipIssuerCheck:            true,
+			SkipExpiryCheck:            true,
+			InsecureSkipSignatureCheck: true,
 		},
 		OAuth2Config: &oauth2.Config{},
 	})
 	require.NoError(t, err, "failed to initialize test")
 
 	token := oauth2.Token{}
+	rawIDToken := `eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkN1bXF1YXQgRG9wdGlnIiwibm9uY2UiOiIxMTEiLCJpYXQiOjE1MTYyMzkwMjJ9.NfbRZns-Sefhw6MT4ULWMj_7bX0vScklaZA2ObCYkStYlo2SvNu5Be79-5Lwcy4GY95vY_dFvLIKrZjfqv_duURSKLUbtH8VxskhcrW4sPAK2R5lzz62a6d_OnVydjNJRZf754TQZILAzMm81tEDNAJSDQjaTFl7t8Bp0iYapNyyH9ZoBrGAPaZkXHYoq4lNH88gCZj5JMRIbrZbsvhOuR3CAzbAMplOmKIWxhFVnHdm6aq6HRjz0ra6Y7yh0R9jEF3vWl2w5D3aN4XESPNBbyB3CHKQ5TG0WkbgdUpv1wwzbPfz4aFHOt--qLy7ZK0TOrS-A7YLFFsJKtoPGRjAPA`
 	extra := map[string]interface{}{
-		"id_token": "foo123",
+		"id_token": rawIDToken,
 	}
 
-	ctx := context.Background()
-	nonceCookieValue := "foobar123"
-	result, err := service.Authenticate(ctx, &OAuth2Result{
+	nonceCookieValue := "111"
+	oauth2Result := &OAuth2Result{
 		OAuth2Token: token.WithExtra(extra),
-	}, issuerG, nonceCookieValue)
+	}
+	result, err := service.Authenticate(context.Background(), oauth2Result, issuer, nonceCookieValue)
 
 	require.NoError(t, err, "failed to authenticate")
 	require.NotNil(t, result)
+}
+
+func setupFakeIdP(t *testing.T) (string, error) {
+	router := chi.NewRouter()
+	ts := httptest.NewServer(router)
+	url := ts.URL
+
+	router.Use(middleware.Logger)
+	router.Get("/oauth2/v3/certs", func(w http.ResponseWriter, r *http.Request) {
+		_, err := w.Write([]byte(`{
+			"keys": [
+			]
+		  }`))
+		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",
+			"authorization_endpoint": "%[1]s/o/oauth2/v2/auth",
+			"device_authorization_endpoint": "%[1]s/device/code",
+			"token_endpoint": "%[1]s/token",
+			"userinfo_endpoint": "%[1]s/v1/userinfo",
+			"revocation_endpoint": "%[1]s/revoke",
+			"jwks_uri": "%[1]s/oauth2/v3/certs",
+			"response_types_supported": [
+			 "code",
+			 "token",
+			 "id_token",
+			 "code token",
+			 "code id_token",
+			 "token id_token",
+			 "code token id_token",
+			 "none"
+			],
+			"subject_types_supported": [
+			 "public"
+			],
+			"id_token_signing_alg_values_supported": [
+			 "RS256"
+			],
+			"scopes_supported": [
+			 "openid",
+			 "email",
+			 "profile"
+			],
+			"token_endpoint_auth_methods_supported": [
+			 "client_secret_post",
+			 "client_secret_basic"
+			],
+			"claims_supported": [
+			 "aud",
+			 "email",
+			 "email_verified",
+			 "exp",
+			 "family_name",
+			 "given_name",
+			 "iat",
+			 "iss",
+			 "locale",
+			 "name",
+			 "picture",
+			 "sub"
+			],
+			"code_challenge_methods_supported": [
+			 "plain",
+			 "S256"
+			],
+			"grant_types_supported": [
+			 "authorization_code",
+			 "refresh_token",
+			 "urn:ietf:params:oauth:grant-type:device_code",
+			 "urn:ietf:params:oauth:grant-type:jwt-bearer"
+			]
+		   }`, url)))
+		if err != nil {
+			log.Fatal(err)
+		}
+	})
 
+	t.Cleanup(ts.Close)
+	return url, nil
 }