Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion charts/session-manager/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type: application
# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
# Versions are expected to follow Semantic Versioning (https://semver.org/)
version: 0.9.4
version: 0.9.5

# This is the version number of the application being deployed. This version number should be
# incremented each time you make changes to the application. Versions are not expected to
Expand Down
5 changes: 4 additions & 1 deletion charts/session-manager/values-dev.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -313,11 +313,14 @@ config:
idleSessionTimeout: 90m
callbackURL: http://localhost:8080/sm/callback
clientAuth:
clientID: "client-id"
type: client_secret
clientSecret:
source: embedded
value: secret
# Deprecated: ClientID is no longer used in the application code, but is still required in the config
# to backfill existing database entries during migration.
# It will be removed in a future release once the migration has been applied in all environments.
clientID: "client-id"
sessionCookieTemplate:
name: "__Host-Http-SESSION"
path: "/"
Expand Down
5 changes: 4 additions & 1 deletion charts/session-manager/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -322,11 +322,14 @@ config:
idleSessionTimeout: 90m
callbackURL: http://localhost:8080/sm/callback
clientAuth:
clientID: "client-id"
type: client_secret
clientSecret:
source: embedded
value: secret
# Deprecated: ClientID is no longer used in the application code, but is still required in the config
# to backfill existing database entries during migration.
# It will be removed in a future release once the migration has been applied in all environments.
clientID: "client-id"
sessionCookieTemplate:
name: "__Host-Http-SESSION"
path: "/"
Expand Down
3 changes: 3 additions & 0 deletions config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,9 @@ sessionManager:
idleSessionTimeout: 90m
callbackURL: http://localhost:8080/sm/callback
clientAuth:
# Deprecated: ClientID is no longer used in the application code, but is still required in the config
# to backfill existing database entries during migration.
# It will be removed in a future release once the migration has been applied in all environments.
clientID: "client-id"
type: client_secret
clientSecret:
Expand Down
1 change: 0 additions & 1 deletion helm-tests/integration/helm-install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ func TestHelmInstall(t *testing.T) {
"config.valkey.host.value": "valkey.default.svc.cluster.local:6379",
"config.valkey.password.value": "",
"config.sessionManager.callbackURL": "http://localhost:8080/sm/callback",
"config.sessionManager.clientAuth.clientID": "test-client",
"config.sessionManager.clientAuth.clientSecret.value": "test-secret",
"config.sessionManager.csrfSecret.value": "test-csrf-secret-at-least-thirty-two-bits",
},
Expand Down
2 changes: 1 addition & 1 deletion integration/grpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ func startServer(t *testing.T, port int) (*stdgrpc.Server, *trust.Service, func(

srv := stdgrpc.NewServer()
oidcmappingv1.RegisterServiceServer(srv, grpc.NewOIDCMappingServer(service))
sessionv1.RegisterServiceServer(srv, grpc.NewSessionServer(ctx, nil, trustRepo, time.Hour, ""))
sessionv1.RegisterServiceServer(srv, grpc.NewSessionServer(ctx, nil, trustRepo, time.Hour))

// start
go func() {
Expand Down
2 changes: 1 addition & 1 deletion integration/session_grpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ func startSessionServer(t *testing.T, port int) (*stdgrpc.Server, session.Reposi
}

srv := stdgrpc.NewServer()
sessionv1.RegisterServiceServer(srv, grpc.NewSessionServer(ctx, sessionRepo, trustRepo, 90*time.Minute, ""))
sessionv1.RegisterServiceServer(srv, grpc.NewSessionServer(ctx, sessionRepo, trustRepo, 90*time.Minute))

// start
go func() {
Expand Down
9 changes: 6 additions & 3 deletions internal/business/business.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ func internalMain(ctx context.Context, cfg *config.Config) error {
sessionRepo,
trustRepo,
cfg.SessionManager.IdleSessionTimeout,
cfg.SessionManager.ClientAuth.ClientID,
grpc.WithQueryParametersIntrospect(cfg.SessionManager.AdditionalQueryParametersIntrospect),
grpc.WithTransportCredentials(credsBuilder),
)
Expand Down Expand Up @@ -225,7 +224,9 @@ func newCredsBuilder(cfg *config.Config) (credentials.Builder, error) {
return nil, fmt.Errorf("failed to load mTLS config: %w", err)
}

return func(clientID string) credentials.TransportCredentials { return credentials.NewTLS(clientID, tlsConfig) }, nil
return func(clientID string) credentials.TransportCredentials {
return credentials.NewTLS(clientID, tlsConfig)
}, nil
case "client_secret", "client_secret_post":
secret, err := commoncfg.LoadValueFromSourceRef(cfg.SessionManager.ClientAuth.ClientSecret)
if err != nil {
Expand All @@ -237,7 +238,9 @@ func newCredsBuilder(cfg *config.Config) (credentials.Builder, error) {
}, nil
case "insecure":
slog.Warn("insecure credentials are used. Do not use this in production")
return func(clientID string) credentials.TransportCredentials { return credentials.NewInsecure(clientID) }, nil
return func(clientID string) credentials.TransportCredentials {
return credentials.NewInsecure(clientID)
}, nil
default:
return nil, errors.New("unknown Client Auth type")
}
Expand Down
12 changes: 4 additions & 8 deletions internal/business/business_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ func TestLoadHTTPClient_MTLS(t *testing.T) {
cfg := &config.Config{
SessionManager: config.SessionManager{
ClientAuth: config.ClientAuth{
Type: "mtls",
ClientID: "test-client",
Type: "mtls",
MTLS: &commoncfg.MTLS{
Cert: commoncfg.SourceRef{File: commoncfg.CredentialFile{Path: "/nonexistent/cert.pem"}},
CertKey: commoncfg.SourceRef{File: commoncfg.CredentialFile{Path: "/nonexistent/key.pem"}},
Expand All @@ -42,7 +41,6 @@ func TestLoadHTTPClient_ClientSecret(t *testing.T) {
SessionManager: config.SessionManager{
ClientAuth: config.ClientAuth{
Type: "client_secret",
ClientID: "test-client",
ClientSecret: commoncfg.SourceRef{Source: "embedded", Value: "test-secret"},
},
},
Expand All @@ -53,7 +51,7 @@ func TestLoadHTTPClient_ClientSecret(t *testing.T) {
require.NotNil(t, builder)

// Verify it's using our custom transport
creds := builder(cfg.SessionManager.ClientAuth.ClientID)
creds := builder("test-client")
clientSecretCreds, ok := creds.(*credentials.ClientSecretPost)
require.True(t, ok)

Expand All @@ -65,8 +63,7 @@ func TestLoadHTTPClient_Insecure(t *testing.T) {
cfg := &config.Config{
SessionManager: config.SessionManager{
ClientAuth: config.ClientAuth{
Type: "insecure",
ClientID: "test-client",
Type: "insecure",
},
},
}
Expand All @@ -80,8 +77,7 @@ func TestLoadHTTPClient_UnknownType(t *testing.T) {
cfg := &config.Config{
SessionManager: config.SessionManager{
ClientAuth: config.ClientAuth{
Type: "unknown",
ClientID: "test-client",
Type: "unknown",
},
},
}
Expand Down
2 changes: 1 addition & 1 deletion internal/business/server/grpc_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func TestStartGRPCServer_ContextCancellation(t *testing.T) {

// Create minimal server instances
oidcmappingsrv := grpc.NewOIDCMappingServer(nil)
sessionsrv := grpc.NewSessionServer(ctx, nil, nil, 0, "")
sessionsrv := grpc.NewSessionServer(ctx, nil, nil, 0)

// Start the server in a goroutine
errChan := make(chan error, 1)
Expand Down
6 changes: 5 additions & 1 deletion internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ type CookieTemplate struct {
}

type ClientAuth struct {
ClientID string `yaml:"clientID"`
// Type defines how to authenticate the client.
// Supported types are:
// - mtls: Mutual TLS authentication
Expand All @@ -123,6 +122,11 @@ type ClientAuth struct {
MTLS *commoncfg.MTLS `yaml:"mTLS"`
// ClientSecret contains the client secret source reference when Type is set to "clientSecret".
ClientSecret commoncfg.SourceRef `yaml:"clientSecret"`

// Deprecated: ClientID is no longer used in the application code, but is still required in the config
// to backfill existing database entries during migration.
// It will be removed in a future release once the migration has been applied in all environments.
ClientID string `yaml:"clientID"`
}

type Migrate struct {
Expand Down
47 changes: 47 additions & 0 deletions internal/dbtest/postgrestest/postgres.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"database/sql"
"fmt"
"log/slog"
"os"
"path/filepath"
"time"

"github.com/jackc/pgx/v5"
Expand Down Expand Up @@ -89,6 +91,10 @@ func makeDBConn(ctx context.Context, port network.Port) *pgxpool.Pool {
}

func migrateDB(ctx context.Context, port network.Port) {
// Create a test config file for the migration
configCleanup := createTestConfig()
defer configCleanup()

db, err := sql.Open("pgx", connStr(port))
if err != nil {
panic(err)
Expand Down Expand Up @@ -122,3 +128,44 @@ func prepareDB(ctx context.Context, dbPool *pgxpool.Pool, port network.Port) {
panic(err)
}
}

// createTestConfig creates a temporary config file for the migration tests.
// It returns a cleanup function that should be called to remove the config file.
func createTestConfig() func() {
// Create a temporary directory for the config
tmpDir, err := os.MkdirTemp("", "session-manager-test-*")
if err != nil {
panic(fmt.Sprintf("Failed to create temp dir: %v", err))
}

// Create config.yaml with test client_id
configContent := `sessionManager:
clientAuth:
clientID: "test-client-id"
`
configPath := filepath.Join(tmpDir, "config.yaml")
err = os.WriteFile(configPath, []byte(configContent), 0644)
if err != nil {
os.RemoveAll(tmpDir)
panic(fmt.Sprintf("Failed to write config file: %v", err))
}

// Change to the temp directory so the config loader can find it
originalDir, err := os.Getwd()
if err != nil {
os.RemoveAll(tmpDir)
panic(fmt.Sprintf("Failed to get current directory: %v", err))
}

err = os.Chdir(tmpDir)
if err != nil {
os.RemoveAll(tmpDir)
panic(fmt.Sprintf("Failed to change directory: %v", err))
}

// Return cleanup function
return func() {
_ = os.Chdir(originalDir)
os.RemoveAll(tmpDir)
}
}
5 changes: 1 addition & 4 deletions internal/grpc/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ type SessionServer struct {
queryParametersIntrospect []string
idleSessionTimeout time.Duration
allowHttpScheme bool
clientID string

// cache introspection results
introspectionCache *ttlcache.Cache[string, oidc.Introspection]
Expand All @@ -52,15 +51,13 @@ func NewSessionServer(
sessionRepo session.Repository,
trustRepo trust.OIDCMappingRepository,
idleSessionTimeout time.Duration,
clientID string,
opts ...SessionServerOption,
) *SessionServer {
s := &SessionServer{
sessionRepo: sessionRepo,
trustRepo: trustRepo,
idleSessionTimeout: idleSessionTimeout,
newCreds: func(clientID string) credentials.TransportCredentials { return credentials.NewInsecure(clientID) },
clientID: clientID,
}
for _, opt := range opts {
if opt != nil {
Expand Down Expand Up @@ -220,7 +217,7 @@ func (s *SessionServer) getClientID(mapping *trust.OIDCMapping) string {
return mapping.ClientID
}

return s.clientID
return ""
}

func (s *SessionServer) httpClient(mapping *trust.OIDCMapping) *http.Client {
Expand Down
Loading
Loading