From a853a80f8f7db6df8495c7bf7b92551ddef5cd2c Mon Sep 17 00:00:00 2001 From: Reynier Ortiz Vega Date: Mon, 30 Mar 2026 17:34:18 -0400 Subject: [PATCH 1/7] feat(config/server): add LoadIDPConfig and unit tests Introduces a new config/server package with LoadIDPConfig to read OIDC IDP settings (issuer, audience, required scope) from environment variables. Implements absent-vs-empty semantics for CONFIG_SERVER_REQUIRED_SCOPE via an optional lookupEnvReader extension of env.Reader, as needed by the config server. Co-Authored-By: Claude Sonnet 4.6 --- config/server/idp.go | 80 +++++++++++++++++++ config/server/idp_test.go | 162 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 242 insertions(+) create mode 100644 config/server/idp.go create mode 100644 config/server/idp_test.go diff --git a/config/server/idp.go b/config/server/idp.go new file mode 100644 index 0000000..78d27cb --- /dev/null +++ b/config/server/idp.go @@ -0,0 +1,80 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +// Package server provides configuration loading utilities for the ToolHive config server. +package server + +import ( + "errors" + + "github.com/stacklok/toolhive-core/env" +) + +// Environment variable names for OIDC / IDP configuration. +const ( + EnvIssuer = "CONFIG_SERVER_ISSUER" + EnvAudience = "CONFIG_SERVER_AUDIENCE" + EnvRequiredScope = "CONFIG_SERVER_REQUIRED_SCOPE" +) + +// DefaultRequiredScope is the OIDC scope required when CONFIG_SERVER_REQUIRED_SCOPE is absent. +const DefaultRequiredScope = "openid" + +// IDPConfig holds the OIDC identity-provider settings read from the environment. +type IDPConfig struct { + Issuer string + Audience string + RequiredScope string +} + +// lookupEnvReader is an optional extension of env.Reader that distinguishes a +// variable that is absent (returns "", false) from one that is set to "" +// (returns "", true). Map-backed test doubles satisfy this automatically via +// the two-value map lookup; production code should use OSLookupReader. +type lookupEnvReader interface { + env.Reader + Lookupenv(key string) (string, bool) +} + +// LoadIDPConfig reads OIDC settings from environment variables via r. +// +// CONFIG_SERVER_REQUIRED_SCOPE is handled with absent-vs-empty semantics when r +// implements lookupEnvReader: absent → DefaultRequiredScope, present-but-empty → +// scope checking disabled. Without lookupEnvReader an empty value falls back to +// DefaultRequiredScope. +// Returns an error if CONFIG_SERVER_ISSUER or CONFIG_SERVER_AUDIENCE are empty. +func LoadIDPConfig(r env.Reader) (IDPConfig, error) { + issuer := r.Getenv(EnvIssuer) + if issuer == "" { + return IDPConfig{}, errors.New("CONFIG_SERVER_ISSUER is required but not set") + } + + audience := r.Getenv(EnvAudience) + if audience == "" { + return IDPConfig{}, errors.New("CONFIG_SERVER_AUDIENCE is required but not set") + } + + requiredScope := resolveRequiredScope(r) + + return IDPConfig{ + Issuer: issuer, + Audience: audience, + RequiredScope: requiredScope, + }, nil +} + +// resolveRequiredScope applies absent-vs-empty semantics for CONFIG_SERVER_REQUIRED_SCOPE. +func resolveRequiredScope(r env.Reader) string { + if lr, ok := r.(lookupEnvReader); ok { + val, present := lr.Lookupenv(EnvRequiredScope) + if !present { + return DefaultRequiredScope + } + return val // present-but-empty disables scope checking + } + // Fallback: cannot distinguish absent from empty; treat empty as default. + if val := r.Getenv(EnvRequiredScope); val != "" { + return val + } + return DefaultRequiredScope +} diff --git a/config/server/idp_test.go b/config/server/idp_test.go new file mode 100644 index 0000000..862acdd --- /dev/null +++ b/config/server/idp_test.go @@ -0,0 +1,162 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package server + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// mapReader is a map-backed env.Reader used in tests. +// Because map lookups are two-valued, it also satisfies lookupEnvReader. +type mapReader map[string]string + +func (m mapReader) Getenv(key string) string { + return m[key] +} + +func (m mapReader) Lookupenv(key string) (string, bool) { + v, ok := m[key] + return v, ok +} + +// plainReader wraps a map but only exposes Getenv, so it does NOT satisfy +// lookupEnvReader. This exercises the fallback path in resolveRequiredScope. +type plainReader map[string]string + +func (p plainReader) Getenv(key string) string { + return p[key] +} + +func TestLoadIDPConfig(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + env map[string]string + useLookup bool // true → mapReader (lookupEnvReader), false → plainReader + wantConfig IDPConfig + wantErrSubstr string + }{ + { + name: "all variables set", + useLookup: true, + env: map[string]string{ + EnvIssuer: "https://issuer.example.com", + EnvAudience: "my-audience", + EnvRequiredScope: "openid profile", + }, + wantConfig: IDPConfig{ + Issuer: "https://issuer.example.com", + Audience: "my-audience", + RequiredScope: "openid profile", + }, + }, + { + name: "scope absent uses default (lookupEnvReader)", + useLookup: true, + env: map[string]string{ + EnvIssuer: "https://issuer.example.com", + EnvAudience: "my-audience", + // EnvRequiredScope intentionally absent + }, + wantConfig: IDPConfig{ + Issuer: "https://issuer.example.com", + Audience: "my-audience", + RequiredScope: DefaultRequiredScope, + }, + }, + { + name: "scope present-but-empty disables scope checking (lookupEnvReader)", + useLookup: true, + env: map[string]string{ + EnvIssuer: "https://issuer.example.com", + EnvAudience: "my-audience", + EnvRequiredScope: "", + }, + wantConfig: IDPConfig{ + Issuer: "https://issuer.example.com", + Audience: "my-audience", + RequiredScope: "", // empty → scope checking disabled + }, + }, + { + name: "scope empty without lookupEnvReader falls back to default", + useLookup: false, + env: map[string]string{ + EnvIssuer: "https://issuer.example.com", + EnvAudience: "my-audience", + // EnvRequiredScope absent (indistinguishable from empty without Lookupenv) + }, + wantConfig: IDPConfig{ + Issuer: "https://issuer.example.com", + Audience: "my-audience", + RequiredScope: DefaultRequiredScope, + }, + }, + { + name: "scope set without lookupEnvReader uses the value", + useLookup: false, + env: map[string]string{ + EnvIssuer: "https://issuer.example.com", + EnvAudience: "my-audience", + EnvRequiredScope: "custom-scope", + }, + wantConfig: IDPConfig{ + Issuer: "https://issuer.example.com", + Audience: "my-audience", + RequiredScope: "custom-scope", + }, + }, + { + name: "missing issuer returns error", + useLookup: true, + env: map[string]string{EnvAudience: "my-audience"}, + wantErrSubstr: "CONFIG_SERVER_ISSUER", + }, + { + name: "empty issuer returns error", + useLookup: true, + env: map[string]string{EnvIssuer: "", EnvAudience: "my-audience"}, + wantErrSubstr: "CONFIG_SERVER_ISSUER", + }, + { + name: "missing audience returns error", + useLookup: true, + env: map[string]string{EnvIssuer: "https://issuer.example.com"}, + wantErrSubstr: "CONFIG_SERVER_AUDIENCE", + }, + { + name: "empty audience returns error", + useLookup: true, + env: map[string]string{EnvIssuer: "https://issuer.example.com", EnvAudience: ""}, + wantErrSubstr: "CONFIG_SERVER_AUDIENCE", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var got IDPConfig + var err error + if tt.useLookup { + got, err = LoadIDPConfig(mapReader(tt.env)) + } else { + got, err = LoadIDPConfig(plainReader(tt.env)) + } + + if tt.wantErrSubstr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErrSubstr) + return + } + + require.NoError(t, err) + assert.Equal(t, tt.wantConfig, got) + }) + } +} From 4924bee7d25e0a9418c65e2bdc1c766cf61a86d6 Mon Sep 17 00:00:00 2001 From: Reynier Ortiz Vega Date: Mon, 30 Mar 2026 17:38:45 -0400 Subject: [PATCH 2/7] fix(config/server): align LookupEnv casing with OSReader method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit lookupEnvReader interface and mapReader test double were using Lookupenv (lowercase e), not matching OSReader.LookupEnv — so production readers never satisfied the interface. Fixes the method name to LookupEnv. Co-Authored-By: Claude Sonnet 4.6 --- config/server/idp.go | 4 ++-- config/server/idp_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/config/server/idp.go b/config/server/idp.go index 78d27cb..962def8 100644 --- a/config/server/idp.go +++ b/config/server/idp.go @@ -33,7 +33,7 @@ type IDPConfig struct { // the two-value map lookup; production code should use OSLookupReader. type lookupEnvReader interface { env.Reader - Lookupenv(key string) (string, bool) + LookupEnv(key string) (string, bool) } // LoadIDPConfig reads OIDC settings from environment variables via r. @@ -66,7 +66,7 @@ func LoadIDPConfig(r env.Reader) (IDPConfig, error) { // resolveRequiredScope applies absent-vs-empty semantics for CONFIG_SERVER_REQUIRED_SCOPE. func resolveRequiredScope(r env.Reader) string { if lr, ok := r.(lookupEnvReader); ok { - val, present := lr.Lookupenv(EnvRequiredScope) + val, present := lr.LookupEnv(EnvRequiredScope) if !present { return DefaultRequiredScope } diff --git a/config/server/idp_test.go b/config/server/idp_test.go index 862acdd..d2b9780 100644 --- a/config/server/idp_test.go +++ b/config/server/idp_test.go @@ -18,7 +18,7 @@ func (m mapReader) Getenv(key string) string { return m[key] } -func (m mapReader) Lookupenv(key string) (string, bool) { +func (m mapReader) LookupEnv(key string) (string, bool) { v, ok := m[key] return v, ok } From adb8b0095e7b22ade40625145b7554e91333aea4 Mon Sep 17 00:00:00 2001 From: Reynier Ortiz Vega Date: Mon, 30 Mar 2026 17:48:26 -0400 Subject: [PATCH 3/7] feat(env): add LookupEnv to OSReader Allows *OSReader to satisfy the optional lookupEnvReader interface used by packages that need absent-vs-empty semantics for environment variables (e.g. config/server.LoadIDPConfig). Co-Authored-By: Claude Sonnet 4.6 --- env/env.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/env/env.go b/env/env.go index 6d6481d..aa4403b 100644 --- a/env/env.go +++ b/env/env.go @@ -19,3 +19,8 @@ type OSReader struct{} func (*OSReader) Getenv(key string) string { return os.Getenv(key) } + +// LookupEnv returns the value of the environment variable named by the key +func (r *OSReader) LookupEnv(key string) (string, bool) { + return os.LookupEnv(key) +} From 3b2edb8cae2d78b087d718003cadfeb52319e90f Mon Sep 17 00:00:00 2001 From: Reynier Ortiz Vega Date: Mon, 30 Mar 2026 17:50:04 -0400 Subject: [PATCH 4/7] feat(env): add LookupEnv to Reader interface; simplify config/server Adding LookupEnv to env.Reader removes the need for a private lookupEnvReader type-assertion in callers. config/server.LoadIDPConfig and its tests are simplified accordingly, and the mock is regenerated. Co-Authored-By: Claude Sonnet 4.6 --- config/server/idp.go | 36 ++++------------------- config/server/idp_test.go | 60 ++++----------------------------------- env/env.go | 1 + env/mocks/mock_reader.go | 15 ++++++++++ 4 files changed, 27 insertions(+), 85 deletions(-) diff --git a/config/server/idp.go b/config/server/idp.go index 962def8..54fee78 100644 --- a/config/server/idp.go +++ b/config/server/idp.go @@ -27,21 +27,10 @@ type IDPConfig struct { RequiredScope string } -// lookupEnvReader is an optional extension of env.Reader that distinguishes a -// variable that is absent (returns "", false) from one that is set to "" -// (returns "", true). Map-backed test doubles satisfy this automatically via -// the two-value map lookup; production code should use OSLookupReader. -type lookupEnvReader interface { - env.Reader - LookupEnv(key string) (string, bool) -} - // LoadIDPConfig reads OIDC settings from environment variables via r. // -// CONFIG_SERVER_REQUIRED_SCOPE is handled with absent-vs-empty semantics when r -// implements lookupEnvReader: absent → DefaultRequiredScope, present-but-empty → -// scope checking disabled. Without lookupEnvReader an empty value falls back to -// DefaultRequiredScope. +// CONFIG_SERVER_REQUIRED_SCOPE uses absent-vs-empty semantics via env.Reader.LookupEnv: +// absent → DefaultRequiredScope, present-but-empty → scope checking disabled. // Returns an error if CONFIG_SERVER_ISSUER or CONFIG_SERVER_AUDIENCE are empty. func LoadIDPConfig(r env.Reader) (IDPConfig, error) { issuer := r.Getenv(EnvIssuer) @@ -54,7 +43,10 @@ func LoadIDPConfig(r env.Reader) (IDPConfig, error) { return IDPConfig{}, errors.New("CONFIG_SERVER_AUDIENCE is required but not set") } - requiredScope := resolveRequiredScope(r) + requiredScope, present := r.LookupEnv(EnvRequiredScope) + if !present { + requiredScope = DefaultRequiredScope + } return IDPConfig{ Issuer: issuer, @@ -62,19 +54,3 @@ func LoadIDPConfig(r env.Reader) (IDPConfig, error) { RequiredScope: requiredScope, }, nil } - -// resolveRequiredScope applies absent-vs-empty semantics for CONFIG_SERVER_REQUIRED_SCOPE. -func resolveRequiredScope(r env.Reader) string { - if lr, ok := r.(lookupEnvReader); ok { - val, present := lr.LookupEnv(EnvRequiredScope) - if !present { - return DefaultRequiredScope - } - return val // present-but-empty disables scope checking - } - // Fallback: cannot distinguish absent from empty; treat empty as default. - if val := r.Getenv(EnvRequiredScope); val != "" { - return val - } - return DefaultRequiredScope -} diff --git a/config/server/idp_test.go b/config/server/idp_test.go index d2b9780..29933f2 100644 --- a/config/server/idp_test.go +++ b/config/server/idp_test.go @@ -11,7 +11,7 @@ import ( ) // mapReader is a map-backed env.Reader used in tests. -// Because map lookups are two-valued, it also satisfies lookupEnvReader. +// Two-value map lookups satisfy the LookupEnv contract automatically. type mapReader map[string]string func (m mapReader) Getenv(key string) string { @@ -23,27 +23,17 @@ func (m mapReader) LookupEnv(key string) (string, bool) { return v, ok } -// plainReader wraps a map but only exposes Getenv, so it does NOT satisfy -// lookupEnvReader. This exercises the fallback path in resolveRequiredScope. -type plainReader map[string]string - -func (p plainReader) Getenv(key string) string { - return p[key] -} - func TestLoadIDPConfig(t *testing.T) { t.Parallel() tests := []struct { name string env map[string]string - useLookup bool // true → mapReader (lookupEnvReader), false → plainReader wantConfig IDPConfig wantErrSubstr string }{ { - name: "all variables set", - useLookup: true, + name: "all variables set", env: map[string]string{ EnvIssuer: "https://issuer.example.com", EnvAudience: "my-audience", @@ -56,8 +46,7 @@ func TestLoadIDPConfig(t *testing.T) { }, }, { - name: "scope absent uses default (lookupEnvReader)", - useLookup: true, + name: "scope absent uses default", env: map[string]string{ EnvIssuer: "https://issuer.example.com", EnvAudience: "my-audience", @@ -70,8 +59,7 @@ func TestLoadIDPConfig(t *testing.T) { }, }, { - name: "scope present-but-empty disables scope checking (lookupEnvReader)", - useLookup: true, + name: "scope present-but-empty disables scope checking", env: map[string]string{ EnvIssuer: "https://issuer.example.com", EnvAudience: "my-audience", @@ -83,55 +71,23 @@ func TestLoadIDPConfig(t *testing.T) { RequiredScope: "", // empty → scope checking disabled }, }, - { - name: "scope empty without lookupEnvReader falls back to default", - useLookup: false, - env: map[string]string{ - EnvIssuer: "https://issuer.example.com", - EnvAudience: "my-audience", - // EnvRequiredScope absent (indistinguishable from empty without Lookupenv) - }, - wantConfig: IDPConfig{ - Issuer: "https://issuer.example.com", - Audience: "my-audience", - RequiredScope: DefaultRequiredScope, - }, - }, - { - name: "scope set without lookupEnvReader uses the value", - useLookup: false, - env: map[string]string{ - EnvIssuer: "https://issuer.example.com", - EnvAudience: "my-audience", - EnvRequiredScope: "custom-scope", - }, - wantConfig: IDPConfig{ - Issuer: "https://issuer.example.com", - Audience: "my-audience", - RequiredScope: "custom-scope", - }, - }, { name: "missing issuer returns error", - useLookup: true, env: map[string]string{EnvAudience: "my-audience"}, wantErrSubstr: "CONFIG_SERVER_ISSUER", }, { name: "empty issuer returns error", - useLookup: true, env: map[string]string{EnvIssuer: "", EnvAudience: "my-audience"}, wantErrSubstr: "CONFIG_SERVER_ISSUER", }, { name: "missing audience returns error", - useLookup: true, env: map[string]string{EnvIssuer: "https://issuer.example.com"}, wantErrSubstr: "CONFIG_SERVER_AUDIENCE", }, { name: "empty audience returns error", - useLookup: true, env: map[string]string{EnvIssuer: "https://issuer.example.com", EnvAudience: ""}, wantErrSubstr: "CONFIG_SERVER_AUDIENCE", }, @@ -141,13 +97,7 @@ func TestLoadIDPConfig(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - var got IDPConfig - var err error - if tt.useLookup { - got, err = LoadIDPConfig(mapReader(tt.env)) - } else { - got, err = LoadIDPConfig(plainReader(tt.env)) - } + got, err := LoadIDPConfig(mapReader(tt.env)) if tt.wantErrSubstr != "" { require.Error(t, err) diff --git a/env/env.go b/env/env.go index aa4403b..be7b6b9 100644 --- a/env/env.go +++ b/env/env.go @@ -10,6 +10,7 @@ import "os" // Reader defines an interface for environment variable access type Reader interface { Getenv(key string) string + LookupEnv(key string) (string, bool) } // OSReader implements Reader using the standard os package diff --git a/env/mocks/mock_reader.go b/env/mocks/mock_reader.go index 17358bb..3bc8c50 100644 --- a/env/mocks/mock_reader.go +++ b/env/mocks/mock_reader.go @@ -56,3 +56,18 @@ func (mr *MockReaderMockRecorder) Getenv(key any) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Getenv", reflect.TypeOf((*MockReader)(nil).Getenv), key) } + +// LookupEnv mocks base method. +func (m *MockReader) LookupEnv(key string) (string, bool) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "LookupEnv", key) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(bool) + return ret0, ret1 +} + +// LookupEnv indicates an expected call of LookupEnv. +func (mr *MockReaderMockRecorder) LookupEnv(key any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LookupEnv", reflect.TypeOf((*MockReader)(nil).LookupEnv), key) +} From 45aa87e706f36db556183eecfd04fef5f0fbf6ff Mon Sep 17 00:00:00 2001 From: Reynier Ortiz Vega Date: Mon, 30 Mar 2026 17:51:23 -0400 Subject: [PATCH 5/7] revert: remove config/server package Will live in the enterprise/private repo instead. Co-Authored-By: Claude Sonnet 4.6 --- config/server/idp.go | 56 ------------------- config/server/idp_test.go | 112 -------------------------------------- 2 files changed, 168 deletions(-) delete mode 100644 config/server/idp.go delete mode 100644 config/server/idp_test.go diff --git a/config/server/idp.go b/config/server/idp.go deleted file mode 100644 index 54fee78..0000000 --- a/config/server/idp.go +++ /dev/null @@ -1,56 +0,0 @@ -// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. -// SPDX-License-Identifier: Apache-2.0 - -// Package server provides configuration loading utilities for the ToolHive config server. -package server - -import ( - "errors" - - "github.com/stacklok/toolhive-core/env" -) - -// Environment variable names for OIDC / IDP configuration. -const ( - EnvIssuer = "CONFIG_SERVER_ISSUER" - EnvAudience = "CONFIG_SERVER_AUDIENCE" - EnvRequiredScope = "CONFIG_SERVER_REQUIRED_SCOPE" -) - -// DefaultRequiredScope is the OIDC scope required when CONFIG_SERVER_REQUIRED_SCOPE is absent. -const DefaultRequiredScope = "openid" - -// IDPConfig holds the OIDC identity-provider settings read from the environment. -type IDPConfig struct { - Issuer string - Audience string - RequiredScope string -} - -// LoadIDPConfig reads OIDC settings from environment variables via r. -// -// CONFIG_SERVER_REQUIRED_SCOPE uses absent-vs-empty semantics via env.Reader.LookupEnv: -// absent → DefaultRequiredScope, present-but-empty → scope checking disabled. -// Returns an error if CONFIG_SERVER_ISSUER or CONFIG_SERVER_AUDIENCE are empty. -func LoadIDPConfig(r env.Reader) (IDPConfig, error) { - issuer := r.Getenv(EnvIssuer) - if issuer == "" { - return IDPConfig{}, errors.New("CONFIG_SERVER_ISSUER is required but not set") - } - - audience := r.Getenv(EnvAudience) - if audience == "" { - return IDPConfig{}, errors.New("CONFIG_SERVER_AUDIENCE is required but not set") - } - - requiredScope, present := r.LookupEnv(EnvRequiredScope) - if !present { - requiredScope = DefaultRequiredScope - } - - return IDPConfig{ - Issuer: issuer, - Audience: audience, - RequiredScope: requiredScope, - }, nil -} diff --git a/config/server/idp_test.go b/config/server/idp_test.go deleted file mode 100644 index 29933f2..0000000 --- a/config/server/idp_test.go +++ /dev/null @@ -1,112 +0,0 @@ -// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. -// SPDX-License-Identifier: Apache-2.0 - -package server - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// mapReader is a map-backed env.Reader used in tests. -// Two-value map lookups satisfy the LookupEnv contract automatically. -type mapReader map[string]string - -func (m mapReader) Getenv(key string) string { - return m[key] -} - -func (m mapReader) LookupEnv(key string) (string, bool) { - v, ok := m[key] - return v, ok -} - -func TestLoadIDPConfig(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - env map[string]string - wantConfig IDPConfig - wantErrSubstr string - }{ - { - name: "all variables set", - env: map[string]string{ - EnvIssuer: "https://issuer.example.com", - EnvAudience: "my-audience", - EnvRequiredScope: "openid profile", - }, - wantConfig: IDPConfig{ - Issuer: "https://issuer.example.com", - Audience: "my-audience", - RequiredScope: "openid profile", - }, - }, - { - name: "scope absent uses default", - env: map[string]string{ - EnvIssuer: "https://issuer.example.com", - EnvAudience: "my-audience", - // EnvRequiredScope intentionally absent - }, - wantConfig: IDPConfig{ - Issuer: "https://issuer.example.com", - Audience: "my-audience", - RequiredScope: DefaultRequiredScope, - }, - }, - { - name: "scope present-but-empty disables scope checking", - env: map[string]string{ - EnvIssuer: "https://issuer.example.com", - EnvAudience: "my-audience", - EnvRequiredScope: "", - }, - wantConfig: IDPConfig{ - Issuer: "https://issuer.example.com", - Audience: "my-audience", - RequiredScope: "", // empty → scope checking disabled - }, - }, - { - name: "missing issuer returns error", - env: map[string]string{EnvAudience: "my-audience"}, - wantErrSubstr: "CONFIG_SERVER_ISSUER", - }, - { - name: "empty issuer returns error", - env: map[string]string{EnvIssuer: "", EnvAudience: "my-audience"}, - wantErrSubstr: "CONFIG_SERVER_ISSUER", - }, - { - name: "missing audience returns error", - env: map[string]string{EnvIssuer: "https://issuer.example.com"}, - wantErrSubstr: "CONFIG_SERVER_AUDIENCE", - }, - { - name: "empty audience returns error", - env: map[string]string{EnvIssuer: "https://issuer.example.com", EnvAudience: ""}, - wantErrSubstr: "CONFIG_SERVER_AUDIENCE", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - got, err := LoadIDPConfig(mapReader(tt.env)) - - if tt.wantErrSubstr != "" { - require.Error(t, err) - assert.Contains(t, err.Error(), tt.wantErrSubstr) - return - } - - require.NoError(t, err) - assert.Equal(t, tt.wantConfig, got) - }) - } -} From e9f3c76f4921557bfaceec4a1631f1f02fabc318 Mon Sep 17 00:00:00 2001 From: Reynier Ortiz Vega Date: Mon, 30 Mar 2026 17:53:29 -0400 Subject: [PATCH 6/7] fix(env): drop unused receiver variable in LookupEnv Co-Authored-By: Claude Sonnet 4.6 --- env/env.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/env/env.go b/env/env.go index be7b6b9..70eba50 100644 --- a/env/env.go +++ b/env/env.go @@ -22,6 +22,6 @@ func (*OSReader) Getenv(key string) string { } // LookupEnv returns the value of the environment variable named by the key -func (r *OSReader) LookupEnv(key string) (string, bool) { +func (*OSReader) LookupEnv(key string) (string, bool) { return os.LookupEnv(key) } From e92186b130681c8a2db46c76db1f9342ee595d87 Mon Sep 17 00:00:00 2001 From: Reynier Ortiz Vega Date: Mon, 30 Mar 2026 17:54:18 -0400 Subject: [PATCH 7/7] test(env): add unit tests for OSReader.LookupEnv Covers present variable, present-but-empty, and absent variable to verify absent-vs-empty semantics of the new Reader.LookupEnv method. Co-Authored-By: Claude Sonnet 4.6 --- env/env_test.go | 59 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/env/env_test.go b/env/env_test.go index ce21916..c2cf0bb 100644 --- a/env/env_test.go +++ b/env/env_test.go @@ -59,6 +59,65 @@ func TestOSReader_Getenv(t *testing.T) { //nolint:paralleltest // Modifies envir } } +func TestOSReader_LookupEnv(t *testing.T) { //nolint:paralleltest // Modifies environment variables + testKey := "TEST_LOOKUP_ENV_VARIABLE_FOR_TESTING" + testValue := "lookup_test_value_123" + + originalValue, wasSet := os.LookupEnv(testKey) + t.Cleanup(func() { + if wasSet { + os.Setenv(testKey, originalValue) + } else { + os.Unsetenv(testKey) + } + }) + + reader := &OSReader{} + + tests := []struct { + name string + setup func() + key string + wantVal string + wantFound bool + }{ + { + name: "existing variable returns value and true", + setup: func() { os.Setenv(testKey, testValue) }, + key: testKey, + wantVal: testValue, + wantFound: true, + }, + { + name: "variable set to empty string returns empty and true", + setup: func() { os.Setenv(testKey, "") }, + key: testKey, + wantVal: "", + wantFound: true, + }, + { + name: "absent variable returns empty and false", + setup: func() { os.Unsetenv(testKey) }, + key: testKey, + wantVal: "", + wantFound: false, + }, + } + + for _, tt := range tests { //nolint:paralleltest // Test modifies environment variables + t.Run(tt.name, func(t *testing.T) { + tt.setup() + gotVal, gotFound := reader.LookupEnv(tt.key) + if gotVal != tt.wantVal { + t.Errorf("OSReader.LookupEnv() val = %q, want %q", gotVal, tt.wantVal) + } + if gotFound != tt.wantFound { + t.Errorf("OSReader.LookupEnv() found = %v, want %v", gotFound, tt.wantFound) + } + }) + } +} + // TestReader_InterfaceCompliance ensures OSReader implements the Reader interface func TestReader_InterfaceCompliance(t *testing.T) { t.Parallel()