[papi] OIDC service signs state with RSA256

Author: easyCZ

components/public-api-server/pkg/oidc/service.go

@@ -14,11 +14,13 @@ import (
 	"io"
 	"io/ioutil"
 	"net/http"
+	"time"
 
 	"github.com/coreos/go-oidc/v3/oidc"
 	goidc "github.com/coreos/go-oidc/v3/oidc"
 	"github.com/gitpod-io/gitpod/common-go/log"
 	db "github.com/gitpod-io/gitpod/components/gitpod-db/go"
+	"github.com/gitpod-io/gitpod/public-api-server/pkg/jws"
 	"github.com/google/uuid"
 	"golang.org/x/oauth2"
 	"google.golang.org/grpc/codes"
@@ -27,11 +29,13 @@ import (
 )
 
 type Service struct {
-	dbConn   *gorm.DB
-	cipher   db.Cipher
-	stateJWT *StateJWT
+	dbConn *gorm.DB
+	cipher db.Cipher
+
+	// jwts
+	stateExpiry    time.Duration
+	signerVerifier jws.SignerVerifier
 
-	// verifierByIssuer      map[string]*goidc.IDTokenVerifier
 	sessionServiceAddress string
 
 	// TODO(at) remove by enhancing test setups
@@ -57,14 +61,15 @@ type AuthFlowResult struct {
 	Claims  map[string]interface{} `json:"claims"`
 }
 
-func NewService(sessionServiceAddress string, dbConn *gorm.DB, cipher db.Cipher, stateJWT *StateJWT) *Service {
+func NewService(sessionServiceAddress string, dbConn *gorm.DB, cipher db.Cipher, signerVerifier jws.SignerVerifier, stateExpiry time.Duration) *Service {
 	return &Service{
 		sessionServiceAddress: sessionServiceAddress,
 
 		dbConn: dbConn,
 		cipher: cipher,
 
-		stateJWT: stateJWT,
+		signerVerifier: signerVerifier,
+		stateExpiry:    stateExpiry,
 	}
 }
 
@@ -98,18 +103,24 @@ func (s *Service) GetStartParams(config *ClientConfig, redirectURL string, retur
 }
 
 func (s *Service) encodeStateParam(state StateParam) (string, error) {
-	encodedState, err := s.stateJWT.Encode(StateClaims{
-		ClientConfigID: state.ClientConfigID,
-		ReturnToURL:    state.ReturnToURL,
-	})
-	return encodedState, err
+	now := time.Now().UTC()
+	expiry := now.Add(s.stateExpiry)
+	token := NewStateJWT(state.ClientConfigID, state.ReturnToURL, now, expiry)
+
+	signed, err := s.signerVerifier.Sign(token)
+	if err != nil {
+		return "", fmt.Errorf("failed to sign jwt: %w", err)
+	}
+	return signed, nil
 }
 
 func (s *Service) decodeStateParam(encodedToken string) (StateParam, error) {
-	claims, err := s.stateJWT.Decode(encodedToken)
+	claims := &StateClaims{}
+	_, err := s.signerVerifier.Verify(encodedToken, claims)
 	if err != nil {
-		return StateParam{}, err
+		return StateParam{}, fmt.Errorf("failed to verify state token: %w", err)
 	}
+
 	return StateParam{
 		ClientConfigID: claims.ClientConfigID,
 		ReturnToURL:    claims.ReturnToURL,

components/public-api-server/pkg/oidc/state_jwt.go

@@ -10,24 +10,6 @@ import (
 	"github.com/golang-jwt/jwt/v5"
 )
 
-type StateJWT struct {
-	key       []byte
-	expiresIn time.Duration
-}
-
-func NewStateJWT(key []byte) *StateJWT {
-	return &StateJWT{
-		key:       key,
-		expiresIn: 5 * time.Minute,
-	}
-}
-
-func newTestStateJWT(key []byte, expiresIn time.Duration) *StateJWT {
-	thing := NewStateJWT(key)
-	thing.expiresIn = expiresIn
-	return thing
-}
-
 type StateClaims struct {
 	// Internal client ID
 	ClientConfigID string `json:"clientId"`
@@ -36,26 +18,13 @@ type StateClaims struct {
 	jwt.RegisteredClaims
 }
 
-func (s *StateJWT) Encode(claims StateClaims) (string, error) {
-
-	expirationTime := time.Now().Add(s.expiresIn)
-	claims.ExpiresAt = jwt.NewNumericDate(expirationTime)
-
-	token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims)
-	encodedToken, err := token.SignedString(s.key)
-
-	return encodedToken, err
-}
-
-func (s *StateJWT) Decode(tokenString string) (*StateClaims, error) {
-	claims := &StateClaims{}
-	_, err := jwt.ParseWithClaims(
-		tokenString,
-		claims,
-		func(token *jwt.Token) (interface{}, error) {
-			return []byte(s.key), nil
+func NewStateJWT(clientConfigID string, returnURL string, issuedAt, expiry time.Time) *jwt.Token {
+	return jwt.NewWithClaims(jwt.SigningMethodRS256, &StateClaims{
+		ClientConfigID: clientConfigID,
+		ReturnToURL:    returnURL,
+		RegisteredClaims: jwt.RegisteredClaims{
+			ExpiresAt: jwt.NewNumericDate(expiry),
+			IssuedAt:  jwt.NewNumericDate(issuedAt),
 		},
-	)
-
-	return claims, err
+	})
 }

components/public-api-server/pkg/server/server.go

@@ -97,22 +97,11 @@ func Start(logger *logrus.Entry, version string, cfg *config.Configuration) erro
 	if err != nil {
 		return fmt.Errorf("failed to setup JWS Keyset: %w", err)
 	}
-	_, err = jws.NewRSA256(keyset)
+	rsa256, err := jws.NewRSA256(keyset)
 	if err != nil {
 		return fmt.Errorf("failed to setup jws.RSA256: %w", err)
 	}
 
-	var stateJWT *oidc.StateJWT
-	if cfg.OIDCClientJWTSigningSecretPath != "" {
-		oidcClientJWTSigningSecret, err := readSecretFromFile(cfg.OIDCClientJWTSigningSecretPath)
-		if err != nil {
-			return fmt.Errorf("failed to read JWT signing secret for OIDC flows: %w", err)
-		}
-		stateJWT = oidc.NewStateJWT([]byte(oidcClientJWTSigningSecret))
-	} else {
-		log.Info("No JWT signing secret for OIDC flows is configured.")
-	}
-
 	var stripeWebhookHandler http.Handler = webhooks.NewNoopWebhookHandler()
 	if cfg.StripeWebhookSigningSecretPath != "" {
 		stripeWebhookSecret, err := readSecretFromFile(cfg.StripeWebhookSigningSecretPath)
@@ -138,7 +127,7 @@ func Start(logger *logrus.Entry, version string, cfg *config.Configuration) erro
 
 	srv.HTTPMux().Handle("/stripe/invoices/webhook", handlers.ContentTypeHandler(stripeWebhookHandler, "application/json"))
 
-	oidcService := oidc.NewService(cfg.SessionServiceAddress, dbConn, cipherSet, stateJWT)
+	oidcService := oidc.NewService(cfg.SessionServiceAddress, dbConn, cipherSet, rsa256, 5*time.Minute)
 
 	if redisClient == nil {
 		return fmt.Errorf("no Redis configiured")