djpbessems/ContainerImage.Pinniped
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Require https scheme for OIDCProviderConfig Issuer field

Browse Source 
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
This commit is contained in:
Ryan Richard 2020-10-28 12:49:41 -07:00 committed by Andrew Keesler
parent 2542a8e175
commit 8ff64d4c1a
3 changed files with 17 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
internal/oidc/provider/oidcprovider.go
View File

@ -37,7 +37,7 @@ func (p *OIDCProvider) validate() error {
return fmt.Errorf("could not parse issuer as URL: %w", err) return fmt.Errorf("could not parse issuer as URL: %w", err)
} }
if issuerURL.Scheme != "https" && p.removeMeAfterWeNoLongerNeedHTTPIssuerSupport(issuerURL.Scheme) { if issuerURL.Scheme != "https" {
return constable.Error(`issuer must have "https" scheme`) return constable.Error(`issuer must have "https" scheme`)
} }
@ -74,7 +74,3 @@ func (p *OIDCProvider) IssuerHost() string {
func (p *OIDCProvider) IssuerPath() string { func (p *OIDCProvider) IssuerPath() string {
return p.issuerPath return p.issuerPath
} }
func (p *OIDCProvider) removeMeAfterWeNoLongerNeedHTTPIssuerSupport(scheme string) bool {
return scheme != "http"
}

5
internal/oidc/provider/oidcprovider_test.go
View File

@ -58,6 +58,11 @@ func TestOIDCProviderValidations(t *testing.T) {
name: "with path", name: "with path",
issuer: "https://tuna.com/fish/marlin", issuer: "https://tuna.com/fish/marlin",
}, },
{
name: "with http scheme",
issuer: "http://tuna.com",
wantError: `issuer must have "https" scheme`,
},
{ {
name: "trailing slash in path", name: "trailing slash in path",
issuer: "https://tuna.com/", issuer: "https://tuna.com/",

23
test/integration/supervisor_discovery_test.go
View File

@ -18,11 +18,10 @@ import (
"testing" "testing"
"time" "time"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1" corev1 "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes"
@ -34,7 +33,7 @@ import (
) )
const ( const (
specialNameForDefaultTLSCertSecret = "pinniped-supervisor-default-tls-certificate" specialNameForDefaultTLSCertSecret = "pinniped-supervisor-default-tls-certificate" //nolint:gosec // this is not a hardcoded credential
) )
func TestSupervisorTLSTerminationWithSNI(t *testing.T) { func TestSupervisorTLSTerminationWithSNI(t *testing.T) {
@ -193,14 +192,14 @@ func TestSupervisorOIDCDiscovery(t *testing.T) {
// Test that there is no default discovery endpoint available when there are no OIDCProviderConfigs. // Test that there is no default discovery endpoint available when there are no OIDCProviderConfigs.
requireDiscoveryEndpointsAreNotFound(t, scheme, addr, caBundle, fmt.Sprintf("%s://%s", scheme, addr)) requireDiscoveryEndpointsAreNotFound(t, scheme, addr, caBundle, fmt.Sprintf("%s://%s", scheme, addr))
// Define several unique issuer strings. // Define several unique issuer strings. Always use https in the issuer name even when we are accessing the http port.
issuer1 := fmt.Sprintf("%s://%s/nested/issuer1", scheme, addr) issuer1 := fmt.Sprintf("https://%s/nested/issuer1", addr)
issuer2 := fmt.Sprintf("%s://%s/nested/issuer2", scheme, addr) issuer2 := fmt.Sprintf("https://%s/nested/issuer2", addr)
issuer3 := fmt.Sprintf("%s://%s/issuer3", scheme, addr) issuer3 := fmt.Sprintf("https://%s/issuer3", addr)
issuer4 := fmt.Sprintf("%s://%s/issuer4", scheme, addr) issuer4 := fmt.Sprintf("https://%s/issuer4", addr)
issuer5 := fmt.Sprintf("%s://%s/issuer5", scheme, addr) issuer5 := fmt.Sprintf("https://%s/issuer5", addr)
issuer6 := fmt.Sprintf("%s://%s/issuer6", scheme, addr) issuer6 := fmt.Sprintf("https://%s/issuer6", addr)
badIssuer := fmt.Sprintf("%s://%s/badIssuer?cannot-use=queries", scheme, addr) badIssuer := fmt.Sprintf("https://%s/badIssuer?cannot-use=queries", addr)
// When OIDCProviderConfig are created in sequence they each cause a discovery endpoint to appear only for as long as the OIDCProviderConfig exists. // When OIDCProviderConfig are created in sequence they each cause a discovery endpoint to appear only for as long as the OIDCProviderConfig exists.
config1, jwks1 := requireCreatingOIDCProviderConfigCausesDiscoveryEndpointsToAppear(ctx, t, scheme, addr, caBundle, issuer1, client) config1, jwks1 := requireCreatingOIDCProviderConfigCausesDiscoveryEndpointsToAppear(ctx, t, scheme, addr, caBundle, issuer1, client)
@ -246,7 +245,7 @@ func TestSupervisorOIDCDiscovery(t *testing.T) {
requireDeletingOIDCProviderConfigCausesDiscoveryEndpointsToDisappear(t, config6Duplicate2, client, ns, scheme, addr, caBundle, issuer6) requireDeletingOIDCProviderConfigCausesDiscoveryEndpointsToDisappear(t, config6Duplicate2, client, ns, scheme, addr, caBundle, issuer6)
// "Host" headers can be used to send requests to discovery endpoints when the public address is different from the issuer name. // "Host" headers can be used to send requests to discovery endpoints when the public address is different from the issuer name.
issuer7 := fmt.Sprintf("%s://some-issuer-host-and-port-that-doesnt-match-public-supervisor-address.com:2684/issuer7", scheme) issuer7 := "https://some-issuer-host-and-port-that-doesnt-match-public-supervisor-address.com:2684/issuer7"
config7, _ := requireCreatingOIDCProviderConfigCausesDiscoveryEndpointsToAppear(ctx, t, scheme, addr, caBundle, issuer7, client) config7, _ := requireCreatingOIDCProviderConfigCausesDiscoveryEndpointsToAppear(ctx, t, scheme, addr, caBundle, issuer7, client)
requireDeletingOIDCProviderConfigCausesDiscoveryEndpointsToDisappear(t, config7, client, ns, scheme, addr, caBundle, issuer7) requireDeletingOIDCProviderConfigCausesDiscoveryEndpointsToDisappear(t, config7, client, ns, scheme, addr, caBundle, issuer7)