Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ref: Tidy up how we process OIDC config #1740

Merged
merged 1 commit into from Mar 17, 2022
Merged

ref: Tidy up how we process OIDC config #1740

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
66 changes: 11 additions & 55 deletions cmd/gitops-server/cmd/cmd.go
View file
Open in desktop
Expand Up @@ -11,7 +11,6 @@ import (
"log"
"net"
"net/http"
"net/url"
"os"
"os/signal"
"path/filepath"
Expand Down Expand Up @@ -51,24 +50,14 @@ type Options struct {
WatcherPort int
Path string
LoggingEnabled bool
OIDC OIDCAuthenticationOptions
OIDC auth.OIDCConfig
NotificationControllerAddress string
TLSCertFile string
TLSKeyFile string
Insecure bool
MTLS bool
}

// OIDCAuthenticationOptions contains the OIDC authentication options for the
// gitops-server
type OIDCAuthenticationOptions struct {
IssuerURL string
ClientID string
ClientSecret string
RedirectURL string
TokenDuration time.Duration
}

var options Options

func NewCommand() *cobra.Command {
Expand Down Expand Up @@ -181,62 +170,29 @@ func runCmd(cmd *cobra.Command, args []string) error {

var authServer *auth.AuthServer

if server.AuthEnabled() {
var OIDCConfig auth.OIDCConfig
oidcConfig := options.OIDC

// If OIDC auth secret is not found use CLI parameters
if server.AuthEnabled() {
// If OIDC auth secret is found prefer that over CLI parameters
var secret corev1.Secret
if err := rawClient.Get(cmd.Context(), client.ObjectKey{
Namespace: v1alpha1.DefaultNamespace,
Name: auth.OIDCAuthSecretName,
}, &secret); err != nil {
appConfig.Logger.Error(err, "OIDC auth secret not found")

OIDCConfig.IssuerURL = options.OIDC.IssuerURL
OIDCConfig.ClientID = options.OIDC.ClientID
OIDCConfig.ClientSecret = options.OIDC.ClientSecret
OIDCConfig.RedirectURL = options.OIDC.RedirectURL
OIDCConfig.TokenDuration = options.OIDC.TokenDuration
} else {
OIDCConfig.IssuerURL = string(secret.Data["issuerURL"])
OIDCConfig.ClientID = string(secret.Data["clientID"])
OIDCConfig.ClientSecret = string(secret.Data["clientSecret"])
OIDCConfig.RedirectURL = string(secret.Data["redirectURL"])

tokenDuration, err := time.ParseDuration(string(secret.Data["tokenDuration"]))
if err != nil {
appConfig.Logger.Error(err, "Invalid token duration")
tokenDuration = time.Hour
}
OIDCConfig.TokenDuration = tokenDuration
}, &secret); err == nil {
oidcConfig = auth.NewOIDCConfigFromSecret(secret)
}

_, err := url.Parse(OIDCConfig.IssuerURL)
tsv, err := auth.NewHMACTokenSignerVerifier(oidcConfig.TokenDuration)
if err != nil {
return fmt.Errorf("invalid issuer URL: %w", err)
}

_, err = url.Parse(OIDCConfig.RedirectURL)
if err != nil {
return fmt.Errorf("invalid redirect URL: %w", err)
return fmt.Errorf("could not create HMAC token signer: %w", err)
}

tsv, err := auth.NewHMACTokenSignerVerifier(OIDCConfig.TokenDuration)
authCfg, err := auth.NewAuthServerConfig(appConfig.Logger, oidcConfig, rawClient, tsv)
if err != nil {
return fmt.Errorf("could not create HMAC token signer: %w", err)
return err
}

srv, err := auth.NewAuthServer(cmd.Context(), appConfig.Logger, http.DefaultClient,
auth.AuthConfig{
OIDCConfig: auth.OIDCConfig{
IssuerURL: OIDCConfig.IssuerURL,
ClientID: OIDCConfig.ClientID,
ClientSecret: OIDCConfig.ClientSecret,
RedirectURL: OIDCConfig.RedirectURL,
TokenDuration: OIDCConfig.TokenDuration,
},
}, rawClient, tsv,
)
srv, err := auth.NewAuthServer(cmd.Context(), authCfg)
if err != nil {
return fmt.Errorf("could not create auth server: %w", err)
}
Expand Down
165 changes: 74 additions & 91 deletions pkg/server/auth/auth_test.go
View file
Open in desktop
Expand Up @@ -14,8 +14,8 @@ import (
"github.com/coreos/go-oidc/v3/oidc"
"github.com/go-logr/logr"
"github.com/oauth2-proxy/mockoidc"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/onsi/gomega"
. "github.com/onsi/gomega"
"github.com/weaveworks/weave-gitops/pkg/server/auth"
"golang.org/x/crypto/bcrypt"
corev1 "k8s.io/api/core/v1"
Expand All @@ -24,44 +24,41 @@ import (
)

func TestWithAPIAuthReturns401ForUnauthenticatedRequests(t *testing.T) {
ctx := context.Background()
g := gomega.NewGomegaWithT(t)

m, err := mockoidc.Run()
if err != nil {
t.Error("failed to create fake OIDC server")
}
g.Expect(err).NotTo(HaveOccurred())

defer func() {
t.Cleanup(func() {
_ = m.Shutdown()
}()
})

fake := m.Config()
mux := http.NewServeMux()
fakeKubernetesClient := ctrlclient.NewClientBuilder().Build()

tokenSignerVerifier, err := auth.NewHMACTokenSignerVerifier(5 * time.Minute)
if err != nil {
t.Errorf("failed to create HMAC signer: %v", err)
}
g.Expect(err).NotTo(HaveOccurred())

srv, err := auth.NewAuthServer(ctx, logr.Discard(), http.DefaultClient,
auth.AuthConfig{
auth.OIDCConfig{
IssuerURL: fake.Issuer,
ClientID: fake.ClientID,
ClientSecret: fake.ClientSecret,
RedirectURL: "",
TokenDuration: 20 * time.Minute,
},
}, fakeKubernetesClient, tokenSignerVerifier)
if err != nil {
t.Error("failed to create auth config")
oidcCfg := auth.OIDCConfig{
ClientID: fake.ClientID,
ClientSecret: fake.ClientSecret,
IssuerURL: fake.Issuer,
}

_ = auth.RegisterAuthServer(mux, "/oauth2", srv, 1)
authCfg, err := auth.NewAuthServerConfig(logr.Discard(), oidcCfg, fakeKubernetesClient, tokenSignerVerifier)
g.Expect(err).NotTo(HaveOccurred())

srv, err := auth.NewAuthServer(context.Background(), authCfg)
g.Expect(err).NotTo(HaveOccurred())

g.Expect(auth.RegisterAuthServer(mux, "/oauth2", srv, 1)).To(Succeed())

s := httptest.NewServer(mux)
defer s.Close()

t.Cleanup(func() {
s.Close()
})

// Set the correct redirect URL now that we have a server running
srv.SetRedirectURL(s.URL + "/oauth2/callback")
Expand All @@ -70,59 +67,52 @@ func TestWithAPIAuthReturns401ForUnauthenticatedRequests(t *testing.T) {
req := httptest.NewRequest(http.MethodGet, s.URL, nil)
auth.WithAPIAuth(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {}), srv, nil).ServeHTTP(res, req)

if res.Result().StatusCode != http.StatusUnauthorized {
t.Errorf("expected status of %d but got %d", http.StatusUnauthorized, res.Result().StatusCode)
}
g.Expect(res).To(HaveHTTPStatus(http.StatusUnauthorized))

// Test out the publicRoutes
res = httptest.NewRecorder()
req = httptest.NewRequest(http.MethodGet, s.URL+"/v1/featureflags", nil)
auth.WithAPIAuth(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {}), srv, []string{"/v1/featureflags"}).ServeHTTP(res, req)

if res.Result().StatusCode != http.StatusOK {
t.Errorf("expected status of %d but got %d", http.StatusUnauthorized, res.Result().StatusCode)
}
g.Expect(res).To(HaveHTTPStatus(http.StatusOK))
}

func TestOauth2FlowRedirectsToOIDCIssuerForUnauthenticatedRequests(t *testing.T) {
ctx := context.Background()
g := gomega.NewGomegaWithT(t)

m, err := mockoidc.Run()
if err != nil {
t.Error("failed to create fake OIDC server")
}
g.Expect(err).NotTo(HaveOccurred())

defer func() {
t.Cleanup(func() {
_ = m.Shutdown()
}()
})

fake := m.Config()
mux := http.NewServeMux()
fakeKubernetesClient := ctrlclient.NewClientBuilder().Build()

tokenSignerVerifier, err := auth.NewHMACTokenSignerVerifier(5 * time.Minute)
if err != nil {
t.Errorf("failed to create HMAC signer: %v", err)
}
g.Expect(err).NotTo(HaveOccurred())

srv, err := auth.NewAuthServer(ctx, logr.Discard(), http.DefaultClient,
auth.AuthConfig{
auth.OIDCConfig{
IssuerURL: fake.Issuer,
ClientID: fake.ClientID,
ClientSecret: fake.ClientSecret,
RedirectURL: "",
TokenDuration: 20 * time.Minute,
},
}, fakeKubernetesClient, tokenSignerVerifier)
if err != nil {
t.Error("failed to create auth config")
oidcCfg := auth.OIDCConfig{
ClientID: fake.ClientID,
ClientSecret: fake.ClientSecret,
IssuerURL: fake.Issuer,
}

_ = auth.RegisterAuthServer(mux, "/oauth2", srv, 1)
authCfg, err := auth.NewAuthServerConfig(logr.Discard(), oidcCfg, fakeKubernetesClient, tokenSignerVerifier)
g.Expect(err).NotTo(HaveOccurred())

srv, err := auth.NewAuthServer(context.Background(), authCfg)
g.Expect(err).NotTo(HaveOccurred())

g.Expect(auth.RegisterAuthServer(mux, "/oauth2", srv, 1)).To(Succeed())

s := httptest.NewServer(mux)
defer s.Close()

t.Cleanup(func() {
s.Close()
})

// Set the correct redirect URL now that we have a server running
redirectURL := s.URL + "/oauth2/callback"
Expand All @@ -132,31 +122,30 @@ func TestOauth2FlowRedirectsToOIDCIssuerForUnauthenticatedRequests(t *testing.T)
req := httptest.NewRequest(http.MethodGet, s.URL, nil)
srv.OAuth2Flow().ServeHTTP(res, req)

if res.Result().StatusCode != http.StatusSeeOther {
t.Errorf("expected status of %d but got %d", http.StatusSeeOther, res.Result().StatusCode)
}
g.Expect(res).To(HaveHTTPStatus(http.StatusSeeOther))

authCodeURL := fmt.Sprintf("%s?client_id=%s&redirect_uri=%s&response_type=code&scope=%s", m.AuthorizationEndpoint(), fake.ClientID, url.QueryEscape(redirectURL), strings.Join([]string{"profile", oidc.ScopeOpenID, oidc.ScopeOfflineAccess, "email"}, "+"))
if !strings.HasPrefix(res.Result().Header.Get("Location"), authCodeURL) {
t.Errorf("expected Location header URL to include scopes %s but does not: %s", authCodeURL, res.Result().Header.Get("Location"))
}
g.Expect(res.Result().Header.Get("Location")).To(ContainSubstring(authCodeURL))
}

func TestIsPublicRoute(t *testing.T) {
assert.True(t, auth.IsPublicRoute(&url.URL{Path: "/foo"}, []string{"/foo"}))
assert.False(t, auth.IsPublicRoute(&url.URL{Path: "foo"}, []string{"/foo"}))
assert.False(t, auth.IsPublicRoute(&url.URL{Path: "/foob"}, []string{"/foo"}))
g := gomega.NewGomegaWithT(t)

g.Expect(auth.IsPublicRoute(&url.URL{Path: "/foo"}, []string{"/foo"})).To(BeTrue())
g.Expect(auth.IsPublicRoute(&url.URL{Path: "foo"}, []string{"/foo"})).To(BeFalse())
g.Expect(auth.IsPublicRoute(&url.URL{Path: "/foob"}, []string{"/foo"})).To(BeFalse())
}

func TestRateLimit(t *testing.T) {
ctx := context.Background()
g := gomega.NewGomegaWithT(t)

mux := http.NewServeMux()
tokenSignerVerifier, err := auth.NewHMACTokenSignerVerifier(5 * time.Minute)
require.NoError(t, err)
g.Expect(err).NotTo(HaveOccurred())

password := "my-secret-password"
hashed, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost)
require.NoError(t, err)
g.Expect(err).NotTo(HaveOccurred())

hashedSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -169,39 +158,33 @@ func TestRateLimit(t *testing.T) {
}
fakeKubernetesClient := ctrlclient.NewClientBuilder().WithObjects(hashedSecret).Build()

srv, err := auth.NewAuthServer(ctx, logr.Discard(), http.DefaultClient,
auth.AuthConfig{
auth.OIDCConfig{
TokenDuration: 20 * time.Minute,
},
}, fakeKubernetesClient, tokenSignerVerifier)
require.NoError(t, err)
err = auth.RegisterAuthServer(mux, "/oauth2", srv, 1)
require.NoError(t, err)
oidcCfg := auth.OIDCConfig{}

authCfg, err := auth.NewAuthServerConfig(logr.Discard(), oidcCfg, fakeKubernetesClient, tokenSignerVerifier)
g.Expect(err).NotTo(HaveOccurred())

srv, err := auth.NewAuthServer(context.Background(), authCfg)
g.Expect(err).NotTo(HaveOccurred())

g.Expect(auth.RegisterAuthServer(mux, "/oauth2", srv, 1)).To(Succeed())

s := httptest.NewServer(mux)
defer s.Close()

res1, err := http.Post(s.URL+"/oauth2/sign_in", "application/json", bytes.NewReader([]byte(`{"password":"my-secret-password"}`)))
require.NoError(t, err)
t.Cleanup(func() {
s.Close()
})

if res1.StatusCode != http.StatusOK {
t.Errorf("expected 200 but got %d instead", res1.StatusCode)
}
res1, err := http.Post(s.URL+"/oauth2/sign_in", "application/json", bytes.NewReader([]byte(`{"password":"my-secret-password"}`)))
g.Expect(err).NotTo(HaveOccurred())
g.Expect(res1).To(HaveHTTPStatus(http.StatusOK))

res2, err := http.Post(s.URL+"/oauth2/sign_in", "application/json", bytes.NewReader([]byte(`{"password":"my-secret-password"}`)))
require.NoError(t, err)

if res2.StatusCode != http.StatusTooManyRequests {
t.Errorf("expected 429 but got %d instead", res2.StatusCode)
}
g.Expect(err).NotTo(HaveOccurred())
g.Expect(res2).To(HaveHTTPStatus(http.StatusTooManyRequests))

time.Sleep(time.Second)

res3, err := http.Post(s.URL+"/oauth2/sign_in", "application/json", bytes.NewReader([]byte(`{"password":"my-secret-password"}`)))
require.NoError(t, err)

if res3.StatusCode != http.StatusOK {
t.Errorf("expected 200 but got %d instead", res3.StatusCode)
}
g.Expect(err).NotTo(HaveOccurred())
g.Expect(res3).To(HaveHTTPStatus(http.StatusOK))
}