Skip to content

Commit

Permalink
fix: panics in oauth providers (#374)
Browse files Browse the repository at this point in the history
We were just checking each item in the slice of interfaces if they were
nil. The interface points to the data of the encapsulated type and an
itable. Checking if the item is nil, does not actually check if the
data encapsulated by the interface is nil, so the previous check wasn't
doing what we actually wanted. Instead we can modify the existing interface
to add a `Provided() bool` to check if the data is actually nil or not. This
should prevent the previous panic and issues with OAuth not working.
  • Loading branch information
jonstacks authored May 14, 2024
1 parent e820323 commit bb257be
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 1 deletion.
32 changes: 32 additions & 0 deletions api/ingress/v1alpha1/ngrok_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,10 @@ type EndpointOAuthGitHub struct {
Organizations []string `json:"organizations,omitempty"`
}

func (github *EndpointOAuthGitHub) Provided() bool {
return github != nil
}

func (github *EndpointOAuthGitHub) ToNgrok(clientSecret *string) *ngrok.EndpointOAuth {
if github == nil {
return nil
Expand All @@ -295,6 +299,10 @@ type EndpointOAuthFacebook struct {
OAuthProviderCommon `json:",inline"`
}

func (facebook *EndpointOAuthFacebook) Provided() bool {
return facebook != nil
}

func (facebook *EndpointOAuthFacebook) ToNgrok(clientSecret *string) *ngrok.EndpointOAuth {
if facebook == nil {
return nil
Expand All @@ -315,6 +323,10 @@ type EndpointOAuthMicrosoft struct {
OAuthProviderCommon `json:",inline"`
}

func (microsoft *EndpointOAuthMicrosoft) Provided() bool {
return microsoft != nil
}

func (microsoft *EndpointOAuthMicrosoft) ToNgrok(clientSecret *string) *ngrok.EndpointOAuth {
if microsoft == nil {
return nil
Expand All @@ -335,6 +347,10 @@ type EndpointOAuthGoogle struct {
OAuthProviderCommon `json:",inline"`
}

func (google *EndpointOAuthGoogle) Provided() bool {
return google != nil
}

func (google *EndpointOAuthGoogle) ToNgrok(clientSecret *string) *ngrok.EndpointOAuth {
if google == nil {
return nil
Expand All @@ -355,6 +371,10 @@ type EndpointOAuthLinkedIn struct {
OAuthProviderCommon `json:",inline"`
}

func (linkedin *EndpointOAuthLinkedIn) Provided() bool {
return linkedin != nil
}

func (linkedin *EndpointOAuthLinkedIn) ToNgrok(clientSecret *string) *ngrok.EndpointOAuth {
if linkedin == nil {
return nil
Expand All @@ -375,6 +395,10 @@ type EndpointOAuthGitLab struct {
OAuthProviderCommon `json:",inline"`
}

func (gitlab *EndpointOAuthGitLab) Provided() bool {
return gitlab != nil
}

func (gitlab *EndpointOAuthGitLab) ToNgrok(clientSecret *string) *ngrok.EndpointOAuth {
if gitlab == nil {
return nil
Expand All @@ -395,6 +419,10 @@ type EndpointOAuthTwitch struct {
OAuthProviderCommon `json:",inline"`
}

func (twitch *EndpointOAuthTwitch) Provided() bool {
return twitch != nil
}

func (twitch *EndpointOAuthTwitch) ToNgrok(clientSecret *string) *ngrok.EndpointOAuth {
if twitch == nil {
return nil
Expand All @@ -415,6 +443,10 @@ type EndpointOAuthAmazon struct {
OAuthProviderCommon `json:",inline"`
}

func (amazon *EndpointOAuthAmazon) Provided() bool {
return amazon != nil
}

func (amazon *EndpointOAuthAmazon) ToNgrok(clientSecret *string) *ngrok.EndpointOAuth {
if amazon == nil {
return nil
Expand Down
45 changes: 45 additions & 0 deletions api/ingress/v1alpha1/ngrok_common_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package v1alpha1

import (
"testing"

"github.com/stretchr/testify/assert"
"k8s.io/utils/ptr"
)

type oauthProvider interface {
Provided() bool
}

func TestOAuthCommonProvided(t *testing.T) {
oauth := EndpointOAuth{}

providers := []oauthProvider{
oauth.Amazon,
oauth.Facebook,
oauth.Github,
oauth.Gitlab,
oauth.Google,
oauth.Linkedin,
oauth.Microsoft,
oauth.Twitch,
}

// When no provider config is present, all should return false for Provided()
for _, p := range providers {
assert.False(t, p.Provided())
}

microsoft := &EndpointOAuthMicrosoft{}
microsoft.ClientID = ptr.To("a")

oauth = EndpointOAuth{
Microsoft: microsoft,
}
// When a provider is present, it should return true for Provided() and ones
// that are not provided should return false.
assert.True(t, oauth.Microsoft.Provided())
assert.False(t, oauth.Google.Provided())
assert.False(t, oauth.Twitch.Provided())
assert.False(t, oauth.Github.Provided())
}
4 changes: 3 additions & 1 deletion internal/controller/ingress/httpsedge_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,7 @@ func (u *edgeRouteModuleUpdater) setEdgeRouteOAuth(ctx context.Context, route *n
}

for _, p := range providers {
if p == nil {
if !p.Provided() {
continue
}

Expand Down Expand Up @@ -954,6 +954,8 @@ func (u *edgeRouteModuleUpdater) getSecret(ctx context.Context, secretRef ingres

type OAuthProvider interface {
ClientSecretKeyRef() *ingressv1alpha1.SecretKeyRef
// Provided returns true if configuration was supplied for the provider
Provided() bool
ToNgrok(*string) *ngrok.EndpointOAuth
}

Expand Down

0 comments on commit bb257be

Please sign in to comment.