From fc64cbd7e29dd252ffab66cef5a541c5316358b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tommy=20Tr=C3=B8en?= Date: Wed, 23 Aug 2023 14:10:22 +0200 Subject: [PATCH 1/8] refactor: remove unused config options/flags --- pkg/config/config.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 82d8875..d43ea64 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -22,17 +22,13 @@ type Config struct { type Tokendings struct { BaseURL string - ClientID string Metadata *oauth.MetadataOAuth WellKnownURL string } type AuthProvider struct { - ClientID string - ClientJwkFile string - ClientJwk *jose.JSONWebKey - Metadata *oauth.MetadataOpenID - WellKnownURL string + ClientID string + ClientJwk *jose.JSONWebKey } func New() (*Config, error) { @@ -41,11 +37,9 @@ func New() (*Config, error) { var clientJwkJson string flag.StringVar(&clientJwkJson, "client-jwk-json", os.Getenv("JWKER_PRIVATE_JWK"), "json with private JWK credential") flag.StringVar(&cfg.AuthProvider.ClientID, "client-id", os.Getenv("JWKER_CLIENT_ID"), "Client ID of Jwker at Auth Provider.") - flag.StringVar(&cfg.AuthProvider.WellKnownURL, "auth-provider-well-known-url", os.Getenv("AUTH_PROVIDER_WELL_KNOWN_URL"), "Well-known URL to Auth Provider.") flag.StringVar(&cfg.ClusterName, "cluster-name", os.Getenv("CLUSTER_NAME"), "nais cluster") flag.StringVar(&cfg.MetricsAddr, "metrics-addr", ":8181", "The address the metric endpoint binds to.") flag.StringVar(&cfg.Tokendings.BaseURL, "tokendings-base-url", os.Getenv("TOKENDINGS_URL"), "Base URL to Tokendings.") - flag.StringVar(&cfg.Tokendings.ClientID, "tokendings-client-id", os.Getenv("TOKENDINGS_CLIENT_ID"), "Client ID of Tokendings at Auth Provider") flag.StringVar(&cfg.LogLevel, "log-level", "info", "Log level for jwker") flag.Parse() From 26eebf6cab3530d221238d8e0b3f75fa189521dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tommy=20Tr=C3=B8en?= Date: Wed, 23 Aug 2023 14:12:32 +0200 Subject: [PATCH 2/8] refactor: rename files with camelcase --- pkg/tokendings/{clientRegistration.go => registration.go} | 0 .../{clientRegistration_test.go => registration_test.go} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename pkg/tokendings/{clientRegistration.go => registration.go} (100%) rename pkg/tokendings/{clientRegistration_test.go => registration_test.go} (100%) diff --git a/pkg/tokendings/clientRegistration.go b/pkg/tokendings/registration.go similarity index 100% rename from pkg/tokendings/clientRegistration.go rename to pkg/tokendings/registration.go diff --git a/pkg/tokendings/clientRegistration_test.go b/pkg/tokendings/registration_test.go similarity index 100% rename from pkg/tokendings/clientRegistration_test.go rename to pkg/tokendings/registration_test.go From 04c2c5ab95577fbabe9b701abea1bf03d9a90d57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tommy=20Tr=C3=B8en?= Date: Wed, 23 Aug 2023 14:14:08 +0200 Subject: [PATCH 3/8] refactor: remove deprecated ioutil.ReadAll in favour of io.ReadAll --- pkg/tokendings/registration.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/tokendings/registration.go b/pkg/tokendings/registration.go index 332585d..e4a157d 100644 --- a/pkg/tokendings/registration.go +++ b/pkg/tokendings/registration.go @@ -5,7 +5,7 @@ import ( "context" "encoding/json" "fmt" - "io/ioutil" + "io" "net/http" "net/url" @@ -64,7 +64,7 @@ func DeleteClient(ctx context.Context, accessToken string, tokenDingsUrl string, return nil } - msg, _ := ioutil.ReadAll(resp.Body) + msg, _ := io.ReadAll(resp.Body) return fmt.Errorf("delete client from tokendings: %s: %s", resp.Status, msg) } @@ -119,7 +119,7 @@ func RegisterClient(cr ClientRegistration, accessToken string, tokenDingsUrl str defer resp.Body.Close() if resp.StatusCode != http.StatusCreated { - response, _ := ioutil.ReadAll(resp.Body) + response, _ := io.ReadAll(resp.Body) return fmt.Errorf("unable to register application with tokendings: %s: %s", resp.Status, response) } From 3168750ac06a654eafc1856341509f7b4a345eba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tommy=20Tr=C3=B8en?= Date: Thu, 24 Aug 2023 09:26:48 +0200 Subject: [PATCH 4/8] tests: add tests for registration.go before refactoring --- pkg/tokendings/registration.go | 1 - pkg/tokendings/registration_test.go | 78 +++++++++++++++++++++++++++-- 2 files changed, 73 insertions(+), 6 deletions(-) diff --git a/pkg/tokendings/registration.go b/pkg/tokendings/registration.go index e4a157d..c84b0a6 100644 --- a/pkg/tokendings/registration.go +++ b/pkg/tokendings/registration.go @@ -40,7 +40,6 @@ type ClientRegistrationResponse struct { TokenEndpointAuthMethod string `json:"token_endpoint_auth_method"` } -// TODO: sign type SoftwareStatement struct { AppId string `json:"appId"` AccessPolicyInbound []string `json:"accessPolicyInbound"` diff --git a/pkg/tokendings/registration_test.go b/pkg/tokendings/registration_test.go index a4201d0..92a7012 100644 --- a/pkg/tokendings/registration_test.go +++ b/pkg/tokendings/registration_test.go @@ -1,7 +1,12 @@ -package tokendings_test +package tokendings import ( + "context" "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" "testing" "github.com/golang-jwt/jwt/v4" @@ -10,8 +15,6 @@ import ( "github.com/stretchr/testify/assert" "gopkg.in/square/go-jose.v2" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/nais/jwker/pkg/tokendings" ) type clientRegistrationTest struct { @@ -63,6 +66,71 @@ var ( } ) +func TestDeleteClient(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/registration/client/cluster1:team1:app1", r.URL.Path) + assert.Equal(t, "DELETE", r.Method) + assert.Equal(t, "Bearer token", r.Header.Get("Authorization")) + w.WriteHeader(http.StatusNoContent) + })) + defer server.Close() + + err := DeleteClient(context.Background(), "token", server.URL, ClientId{ + Name: "app1", + Namespace: "team1", + Cluster: "cluster1", + }) + fmt.Printf("Error: %v\n", err) + assert.NoError(t, err) +} + +func TestRegisterClient(t *testing.T) { + app := ClientId{ + Name: "app1", + Namespace: "team1", + Cluster: "cluster1", + } + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + + assert.Equal(t, "/registration/client", r.URL.Path) + assert.Equal(t, "POST", r.Method) + assert.Equal(t, "Bearer token", r.Header.Get("Authorization")) + + body, err := io.ReadAll(r.Body) + if err != nil { + w.WriteHeader(http.StatusBadRequest) + } + + var clientRegistration ClientRegistration + err = json.Unmarshal(body, &clientRegistration) + if err != nil { + w.WriteHeader(http.StatusBadRequest) + } + assert.Equal(t, app.String(), clientRegistration.ClientName) + assert.Equal(t, 1, len(clientRegistration.Jwks.Keys)) + assert.Equal(t, "signedstatement", clientRegistration.SoftwareStatement) + + w.WriteHeader(http.StatusCreated) + })) + defer server.Close() + + jwk, err := jwkutils.GenerateJWK() + assert.NoError(t, err) + + err = RegisterClient(ClientRegistration{ + ClientName: app.String(), + Jwks: jose.JSONWebKeySet{ + Keys: []jose.JSONWebKey{ + jwk, + }, + }, + SoftwareStatement: "signedstatement", + }, "token", server.URL) + + assert.NoError(t, err) +} + func TestMakeClientRegistration(t *testing.T) { signkey, err := jwkutils.GenerateJWK() if err != nil { @@ -75,13 +143,13 @@ func TestMakeClientRegistration(t *testing.T) { } appkeys := jwkutils.KeySetWithExisting(appkey, []jose.JSONWebKey{}) - clientid := tokendings.ClientId{ + clientid := ClientId{ Name: "myapplication", Namespace: "mynamespace", Cluster: "mycluster", } - output, err := tokendings.MakeClientRegistration(&signkey, &appkeys.Public, clientid, test.input) + output, err := MakeClientRegistration(&signkey, &appkeys.Public, clientid, test.input) assert.NoError(t, err) assert.Equal(t, test.clientName, output.ClientName) From 64c66b179f08c5e5759786358d75a6187e9955e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tommy=20Tr=C3=B8en?= Date: Thu, 24 Aug 2023 10:07:59 +0200 Subject: [PATCH 5/8] tests: add tests for clientassertion --- pkg/tokendings/gettoken_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 pkg/tokendings/gettoken_test.go diff --git a/pkg/tokendings/gettoken_test.go b/pkg/tokendings/gettoken_test.go new file mode 100644 index 0000000..b71ae10 --- /dev/null +++ b/pkg/tokendings/gettoken_test.go @@ -0,0 +1,28 @@ +package tokendings + +import ( + "encoding/json" + "github.com/nais/jwker/jwkutils" + "github.com/stretchr/testify/assert" + "gopkg.in/square/go-jose.v2" + "testing" +) + +func TestClientAssertion(t *testing.T) { + jwk, err := jwkutils.GenerateJWK() + assert.NoError(t, err) + + raw, err := ClientAssertion(&jwk, "client1", "http://endpoint/registration/client") + assert.NoError(t, err) + + sign, err := jose.ParseSigned(raw) + payload, err := sign.Verify(jwk.Public()) + assert.NoError(t, err) + + claims := CustomClaims{} + err = json.Unmarshal(payload, &claims) + assert.NoError(t, err) + assert.Equal(t, "client1", claims.Issuer) + assert.Equal(t, "client1", claims.Subject) + assert.Equal(t, "http://endpoint/registration/client", claims.Audience) +} From 0e13bfd95ce4478628278e60fc568331e3cb28c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tommy=20Tr=C3=B8en?= Date: Thu, 24 Aug 2023 14:42:21 +0200 Subject: [PATCH 6/8] feat/refactor: add possibility to configure multiple tokendings instances * refactor and simplify where necessary * move token handling into registration.go * add more tests --- controllers/jwker_controller.go | 76 +++++++++++++---------------- controllers/suite_test.go | 43 ++++++++-------- pkg/config/config.go | 49 ++++++++++--------- pkg/tokendings/registration.go | 55 ++++++++++++++++++--- pkg/tokendings/registration_test.go | 57 ++++++++++++++++++---- 5 files changed, 174 insertions(+), 106 deletions(-) diff --git a/controllers/jwker_controller.go b/controllers/jwker_controller.go index f21d3ab..bfc38de 100644 --- a/controllers/jwker_controller.go +++ b/controllers/jwker_controller.go @@ -28,20 +28,18 @@ import ( ) const ( - refreshTokenRetryInterval = 10 * time.Second - requeueInterval = 10 * time.Second + requeueInterval = 10 * time.Second ) // JwkerReconciler reconciles a Jwker object type JwkerReconciler struct { client.Client - Log logr.Logger - Scheme *runtime.Scheme - Reader client.Reader - Recorder record.EventRecorder - logger logr.Logger - TokendingsToken *tokendings.TokenResponse - Config *config.Config + Log logr.Logger + Scheme *runtime.Scheme + Reader client.Reader + Recorder record.EventRecorder + logger logr.Logger + Config *config.Config } func (r *JwkerReconciler) appClientID(req ctrl.Request) tokendings.ClientId { @@ -52,25 +50,6 @@ func (r *JwkerReconciler) appClientID(req ctrl.Request) tokendings.ClientId { } } -func (r *JwkerReconciler) CreateToken() { - jwk := r.Config.AuthProvider.ClientJwk - clientID := r.Config.AuthProvider.ClientID - endpoint := fmt.Sprintf("%s/registration/client", r.Config.Tokendings.BaseURL) - - var err error - - jwt, err := tokendings.ClientAssertion(jwk, clientID, endpoint) - if err != nil { - r.Log.Error(err, "failed to generate client assertion") - } - r.TokendingsToken = &tokendings.TokenResponse{ - AccessToken: jwt, - TokenType: "Bearer", - ExpiresIn: 0, - Scope: "", - } -} - // delete all associated objects // TODO: needs finalizer func (r *JwkerReconciler) purge(ctx context.Context, req ctrl.Request) error { @@ -78,9 +57,12 @@ func (r *JwkerReconciler) purge(ctx context.Context, req ctrl.Request) error { r.logger.Info(fmt.Sprintf("Jwker resource %s in namespace: %s has been deleted. Cleaning up resources", req.Name, req.Namespace)) - r.logger.Info(fmt.Sprintf("Deleting resource %s in namespace %s from tokendings", req.Name, req.Namespace)) - if err := tokendings.DeleteClient(ctx, r.TokendingsToken.AccessToken, r.Config.Tokendings.BaseURL, aid); err != nil { - return fmt.Errorf("deleting resource from Tokendings: %s", err) + r.logger.Info(fmt.Sprintf("Deleting resource %s in namespace %s from %d tokendings instances", req.Name, req.Namespace, len(r.Config.TokendingsInstances))) + for _, instance := range r.Config.TokendingsInstances { + r.logger.Info(fmt.Sprintf("Deleting client from tokendings instance %s", instance.BaseURL)) + if err := instance.DeleteClient(ctx, aid); err != nil { + return fmt.Errorf("deleting resource from Tokendings instance '%s': %w", instance.BaseURL, err) + } } r.logger.Info(fmt.Sprintf("Deleting application %s jwker secrets in namespace %s from cluster", req.Name, req.Namespace)) @@ -158,7 +140,7 @@ func (r *JwkerReconciler) create(tx transaction) error { app := r.appClientID(tx.req) cr, err := tokendings.MakeClientRegistration( - r.Config.AuthProvider.ClientJwk, + r.Config.ClientJwk, &tx.keyset.Public, app, tx.jwker, @@ -168,12 +150,18 @@ func (r *JwkerReconciler) create(tx transaction) error { return fmt.Errorf("create client registration payload: %s", err) } - r.logger.Info(fmt.Sprintf("Registering app %s with tokendings", app.String())) - err = tokendings.RegisterClient( - *cr, - r.TokendingsToken.AccessToken, - r.Config.Tokendings.BaseURL, - ) + instances := r.Config.TokendingsInstances + if len(instances) == 0 { + return fmt.Errorf("no tokendings instances configured, cannot create resources") + } + r.logger.Info(fmt.Sprintf("Registering app %s with %d tokendings instances", app.String(), len(instances))) + + for _, instance := range instances { + r.logger.Info(fmt.Sprintf("Registering client with tokendings instance %s", instance.BaseURL)) + if err := instance.RegisterClient(*cr); err != nil { + return fmt.Errorf("registering client with Tokendings instance '%s': %w", instance.BaseURL, err) + } + } if err != nil { return fmt.Errorf("failed registering client: %s", err) @@ -187,9 +175,13 @@ func (r *JwkerReconciler) create(tx transaction) error { } secretData := secret.PodSecretData{ - ClientId: app, - Jwk: *jwk, - TokendingsConfig: r.Config.Tokendings, + ClientId: app, + Jwk: *jwk, + TokendingsConfig: config.Tokendings{ + BaseURL: instances[0].BaseURL, + Metadata: instances[0].Metadata, + WellKnownURL: instances[0].WellKnownURL, + }, } if err := secret.CreateAppSecret(r.Client, tx.ctx, tx.jwker.Spec.SecretName, secretData); err != nil { @@ -209,8 +201,6 @@ func (r *JwkerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl jwkermetrics.JwkersProcessedCount.Inc() - r.CreateToken() - r.logger = r.Log.WithValues( "jwker", req.NamespacedName, "jwker_name", req.Name, diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 713d366..46d4583 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "log" "net/http" "net/http/httptest" "testing" @@ -13,7 +14,6 @@ import ( nais_io_v1 "github.com/nais/liberator/pkg/apis/nais.io/v1" "github.com/nais/liberator/pkg/crd" "github.com/nais/liberator/pkg/events" - "github.com/nais/liberator/pkg/oauth" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -217,15 +217,16 @@ func TestReconciler(t *testing.T) { tokendingsServer := httptest.NewServer(&tokendingsHandler{}) cfg, err := makeConfig(tokendingsServer.URL) - assert.NoError(t, err) + if err != nil { + log.Fatalf("unable to create tokendings instances: %+v", err) + } jwker := &controllers.JwkerReconciler{ - Client: cli, - Log: ctrl.Log.WithName("controllers").WithName("Jwker"), - Recorder: mgr.GetEventRecorderFor("jwker"), - Scheme: mgr.GetScheme(), - TokendingsToken: &tokendings.TokenResponse{}, - Config: cfg, + Client: cli, + Log: ctrl.Log.WithName("controllers").WithName("Jwker"), + Recorder: mgr.GetEventRecorderFor("jwker"), + Scheme: mgr.GetScheme(), + Config: cfg, } err = jwker.SetupWithManager(mgr) @@ -256,6 +257,15 @@ func TestReconciler(t *testing.T) { // secret must have data assert.NotEmpty(t, sec.Data[secret.TokenXPrivateJwkKey]) + t.Run("should contain secret data", func(t *testing.T) { + assert.NoError(t, err) + assert.Equal(t, "local:default:app1", string(sec.Data[secret.TokenXClientIdKey])) + assert.Equal(t, fmt.Sprintf("%s/.well-known/oauth-authorization-server", tokendingsServer.URL), string(sec.Data[secret.TokenXWellKnownUrlKey])) + assert.Equal(t, fmt.Sprintf("%s", tokendingsServer.URL), string(sec.Data[secret.TokenXIssuerKey])) + assert.Equal(t, fmt.Sprintf("%s/jwks", tokendingsServer.URL), string(sec.Data[secret.TokenXJwksUriKey])) + assert.Equal(t, fmt.Sprintf("%s/token", tokendingsServer.URL), string(sec.Data[secret.TokenXTokenEndpointKey])) + }) + // existing, in-use secret should be preserved sec, err = getSecret(ctx, cli, namespace, alreadyInUseSecret) assert.NoError(t, err) @@ -367,20 +377,11 @@ func makeConfig(tokendingsURL string) (*config.Config, error) { } return &config.Config{ - AuthProvider: config.AuthProvider{ - ClientJwk: &jwk, - }, + ClientID: "jwker", + ClientJwk: &jwk, ClusterName: "local", - Tokendings: config.Tokendings{ - BaseURL: tokendingsURL, - Metadata: &oauth.MetadataOAuth{ - MetadataCommon: oauth.MetadataCommon{ - Issuer: tokendingsURL, - JwksURI: tokendingsURL + "/jwks", - TokenEndpoint: tokendingsURL + "/token", - }, - }, - WellKnownURL: tokendingsURL + "/.well-known/oauth/authorization-server", + TokendingsInstances: []*tokendings.Instance{ + tokendings.NewInstance(tokendingsURL, "jwker", &jwk), }, }, nil } diff --git a/pkg/config/config.go b/pkg/config/config.go index d43ea64..d316fa8 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -3,9 +3,10 @@ package config import ( "flag" "fmt" + "github.com/nais/jwker/pkg/tokendings" "net/url" "os" - "path" + "strings" "github.com/nais/jwker/jwkutils" "github.com/nais/liberator/pkg/oauth" @@ -13,11 +14,12 @@ import ( ) type Config struct { - AuthProvider AuthProvider - ClusterName string - LogLevel string - MetricsAddr string - Tokendings Tokendings + ClientID string + ClientJwk *jose.JSONWebKey + ClusterName string + LogLevel string + MetricsAddr string + TokendingsInstances []*tokendings.Instance } type Tokendings struct { @@ -26,20 +28,17 @@ type Tokendings struct { WellKnownURL string } -type AuthProvider struct { - ClientID string - ClientJwk *jose.JSONWebKey -} - func New() (*Config, error) { cfg := &Config{} - cfg.Tokendings.Metadata = &oauth.MetadataOAuth{} var clientJwkJson string + var instanceString string + var tokendingsURL string flag.StringVar(&clientJwkJson, "client-jwk-json", os.Getenv("JWKER_PRIVATE_JWK"), "json with private JWK credential") - flag.StringVar(&cfg.AuthProvider.ClientID, "client-id", os.Getenv("JWKER_CLIENT_ID"), "Client ID of Jwker at Auth Provider.") + flag.StringVar(&cfg.ClientID, "client-id", os.Getenv("JWKER_CLIENT_ID"), "Client ID of Jwker at Auth Provider.") flag.StringVar(&cfg.ClusterName, "cluster-name", os.Getenv("CLUSTER_NAME"), "nais cluster") flag.StringVar(&cfg.MetricsAddr, "metrics-addr", ":8181", "The address the metric endpoint binds to.") - flag.StringVar(&cfg.Tokendings.BaseURL, "tokendings-base-url", os.Getenv("TOKENDINGS_URL"), "Base URL to Tokendings.") + flag.StringVar(&tokendingsURL, "tokendings-base-url", os.Getenv("TOKENDINGS_URL"), "The base URL to Tokendings.") + flag.StringVar(&instanceString, "tokendings-instances", os.Getenv("TOKENDINGS_INSTANCES"), "Comma separated list of baseUrls to Tokendings instances.") flag.StringVar(&cfg.LogLevel, "log-level", "info", "Log level for jwker") flag.Parse() @@ -47,18 +46,20 @@ func New() (*Config, error) { if err != nil { return nil, err } - cfg.AuthProvider.ClientJwk = j - - cfg.Tokendings.Metadata.Issuer = cfg.Tokendings.BaseURL - cfg.Tokendings.Metadata.JwksURI = fmt.Sprintf("%s/jwks", cfg.Tokendings.BaseURL) - cfg.Tokendings.Metadata.TokenEndpoint = fmt.Sprintf("%s/token", cfg.Tokendings.BaseURL) + cfg.ClientJwk = j - tokendingsWellKnownURL, err := url.Parse(cfg.Tokendings.BaseURL) - if err != nil { - return nil, fmt.Errorf("invalid base url for tokendings: %w", err) + instances := make([]*tokendings.Instance, 0) + raw := strings.TrimSpace(instanceString) + if raw == "" { + raw = tokendingsURL + } + for _, u := range strings.Split(raw, ",") { + _, err := url.Parse(strings.TrimSpace(u)) + if err != nil { + return nil, fmt.Errorf("invalid base url for tokendings instance: %w", err) + } + instances = append(instances, tokendings.NewInstance(u, cfg.ClientID, cfg.ClientJwk)) } - tokendingsWellKnownURL.Path = path.Join(tokendingsWellKnownURL.Path, oauth.WellKnownOAuthPath) - cfg.Tokendings.WellKnownURL = tokendingsWellKnownURL.String() return cfg, nil } diff --git a/pkg/tokendings/registration.go b/pkg/tokendings/registration.go index c84b0a6..25eb533 100644 --- a/pkg/tokendings/registration.go +++ b/pkg/tokendings/registration.go @@ -5,13 +5,14 @@ import ( "context" "encoding/json" "fmt" - "io" - "net/http" - "net/url" - v1 "github.com/nais/liberator/pkg/apis/nais.io/v1" + "github.com/nais/liberator/pkg/oauth" "gopkg.in/square/go-jose.v2" "gopkg.in/square/go-jose.v2/jwt" + "io" + "net/http" + "net/url" + "strings" ) type ClientId struct { @@ -46,11 +47,42 @@ type SoftwareStatement struct { AccessPolicyOutbound []string `json:"accessPolicyOutbound"` } -func DeleteClient(ctx context.Context, accessToken string, tokenDingsUrl string, appClientId ClientId) error { - request, err := http.NewRequestWithContext(ctx, "DELETE", fmt.Sprintf("%s/registration/client/%s", tokenDingsUrl, url.QueryEscape(appClientId.String())), nil) +type Instance struct { + BaseURL string + ClientID string + ClientJwk *jose.JSONWebKey + Metadata *oauth.MetadataOAuth + WellKnownURL string +} + +func NewInstance(baseURL, clientID string, clientJwk *jose.JSONWebKey) *Instance { + i := &Instance{ + BaseURL: baseURL, + ClientID: clientID, + ClientJwk: clientJwk, + } + + i.Metadata = &oauth.MetadataOAuth{} + i.Metadata.Issuer = i.BaseURL + i.Metadata.JwksURI = fmt.Sprintf("%s/jwks", i.BaseURL) + i.Metadata.TokenEndpoint = fmt.Sprintf("%s/token", i.BaseURL) + i.WellKnownURL = fmt.Sprintf("%s%s", strings.TrimSuffix(i.BaseURL, "/"), oauth.WellKnownOAuthPath) + return i +} + +func (t *Instance) DeleteClient(ctx context.Context, appClientId ClientId) error { + endpoint := fmt.Sprintf("%s/registration/client", t.BaseURL) + + request, err := http.NewRequestWithContext(ctx, "DELETE", fmt.Sprintf("%s/%s", endpoint, url.QueryEscape(appClientId.String())), nil) if err != nil { return err } + + accessToken, err := ClientAssertion(t.ClientJwk, t.ClientID, endpoint) + if err != nil { + return fmt.Errorf("unable to create token for invoking tokendings: %w", err) + } + request.Header.Add("Authorization", fmt.Sprintf("Bearer %s", accessToken)) client := http.Client{} resp, err := client.Do(request) @@ -98,16 +130,23 @@ func MakeClientRegistration(jwkerPrivateJwk *jose.JSONWebKey, clientPublicJwks * }, nil } -func RegisterClient(cr ClientRegistration, accessToken string, tokenDingsUrl string) error { +func (t *Instance) RegisterClient(cr ClientRegistration) error { + endpoint := fmt.Sprintf("%s/registration/client", t.BaseURL) + data, err := json.Marshal(cr) if err != nil { return err } - request, err := http.NewRequest("POST", fmt.Sprintf("%s/registration/client", tokenDingsUrl), bytes.NewReader(data)) + request, err := http.NewRequest("POST", endpoint, bytes.NewReader(data)) if err != nil { return err } request.Header.Add("Content-Type", "application/json") + + accessToken, err := ClientAssertion(t.ClientJwk, t.ClientID, endpoint) + if err != nil { + return fmt.Errorf("unable to create token for invoking tokendings: %w", err) + } request.Header.Add("Authorization", fmt.Sprintf("Bearer %s", accessToken)) client := http.Client{} diff --git a/pkg/tokendings/registration_test.go b/pkg/tokendings/registration_test.go index 92a7012..8d83b39 100644 --- a/pkg/tokendings/registration_test.go +++ b/pkg/tokendings/registration_test.go @@ -3,10 +3,11 @@ package tokendings import ( "context" "encoding/json" - "fmt" "io" "net/http" "net/http/httptest" + "net/url" + "strings" "testing" "github.com/golang-jwt/jwt/v4" @@ -66,21 +67,35 @@ var ( } ) +func TestNewInstance(t *testing.T) { + jwk, err := jwkutils.GenerateJWK() + assert.NoError(t, err) + inst1 := NewInstance("http://localhost:8080", "jwker", &jwk) + inst2 := NewInstance("http://localhost:8080/", "jwker", &jwk) + assert.Equal(t, "http://localhost:8080/.well-known/oauth-authorization-server", inst1.WellKnownURL) + assert.Equal(t, "http://localhost:8080/.well-known/oauth-authorization-server", inst2.WellKnownURL) +} + func TestDeleteClient(t *testing.T) { + jwk, err := jwkutils.GenerateJWK() + assert.NoError(t, err) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, "/registration/client/cluster1:team1:app1", r.URL.Path) assert.Equal(t, "DELETE", r.Method) - assert.Equal(t, "Bearer token", r.Header.Get("Authorization")) + verifyToken(t, r, jwk) + w.WriteHeader(http.StatusNoContent) })) defer server.Close() - err := DeleteClient(context.Background(), "token", server.URL, ClientId{ + td := NewInstance(server.URL, "jwker", &jwk) + + err = td.DeleteClient(context.Background(), ClientId{ Name: "app1", Namespace: "team1", Cluster: "cluster1", }) - fmt.Printf("Error: %v\n", err) assert.NoError(t, err) } @@ -91,21 +106,27 @@ func TestRegisterClient(t *testing.T) { Cluster: "cluster1", } + jwk, err := jwkutils.GenerateJWK() + assert.NoError(t, err) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, "/registration/client", r.URL.Path) assert.Equal(t, "POST", r.Method) - assert.Equal(t, "Bearer token", r.Header.Get("Authorization")) + + verifyToken(t, r, jwk) body, err := io.ReadAll(r.Body) if err != nil { w.WriteHeader(http.StatusBadRequest) + return } var clientRegistration ClientRegistration err = json.Unmarshal(body, &clientRegistration) if err != nil { w.WriteHeader(http.StatusBadRequest) + return } assert.Equal(t, app.String(), clientRegistration.ClientName) assert.Equal(t, 1, len(clientRegistration.Jwks.Keys)) @@ -115,10 +136,8 @@ func TestRegisterClient(t *testing.T) { })) defer server.Close() - jwk, err := jwkutils.GenerateJWK() - assert.NoError(t, err) - - err = RegisterClient(ClientRegistration{ + td := NewInstance(server.URL, "jwker", &jwk) + err = td.RegisterClient(ClientRegistration{ ClientName: app.String(), Jwks: jose.JSONWebKeySet{ Keys: []jose.JSONWebKey{ @@ -126,7 +145,7 @@ func TestRegisterClient(t *testing.T) { }, }, SoftwareStatement: "signedstatement", - }, "token", server.URL) + }) assert.NoError(t, err) } @@ -166,3 +185,21 @@ func TestMakeClientRegistration(t *testing.T) { assert.Equal(t, test.softwareStatement, string(js)) } + +func verifyToken(t *testing.T, r *http.Request, jwk jose.JSONWebKey) { + raw := strings.Replace(r.Header.Get("Authorization"), "Bearer ", "", 1) + sign, err := jose.ParseSigned(raw) + payload, err := sign.Verify(jwk.Public()) + assert.NoError(t, err) + + claims := CustomClaims{} + err = json.Unmarshal(payload, &claims) + assert.NoError(t, err) + + assert.Equal(t, "jwker", claims.Issuer) + assert.Equal(t, "jwker", claims.Subject) + + aud, err := url.Parse(claims.Audience) + assert.NoError(t, err) + assert.Equal(t, "/registration/client", aud.Path) +} From d862c3f55facde00851726d89128b665ff834b01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tommy=20Tr=C3=B8en?= Date: Thu, 24 Aug 2023 19:23:00 +0200 Subject: [PATCH 7/8] refactor: move error handling for instances to Config.New() * actually set the instances to config --- controllers/jwker_controller.go | 3 --- pkg/config/config.go | 5 +++++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/controllers/jwker_controller.go b/controllers/jwker_controller.go index bfc38de..7db4244 100644 --- a/controllers/jwker_controller.go +++ b/controllers/jwker_controller.go @@ -151,9 +151,6 @@ func (r *JwkerReconciler) create(tx transaction) error { } instances := r.Config.TokendingsInstances - if len(instances) == 0 { - return fmt.Errorf("no tokendings instances configured, cannot create resources") - } r.logger.Info(fmt.Sprintf("Registering app %s with %d tokendings instances", app.String(), len(instances))) for _, instance := range instances { diff --git a/pkg/config/config.go b/pkg/config/config.go index d316fa8..fa4bd77 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -61,5 +61,10 @@ func New() (*Config, error) { instances = append(instances, tokendings.NewInstance(u, cfg.ClientID, cfg.ClientJwk)) } + if len(instances) == 0 { + return nil, fmt.Errorf("no tokendings instances configured") + } + cfg.TokendingsInstances = instances + return cfg, nil } From 57fd86fd44ff1e71ea06e064eb3fbf5102588199 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tommy=20Tr=C3=B8en?= Date: Thu, 24 Aug 2023 19:24:32 +0200 Subject: [PATCH 8/8] refactor: error handling covered in for loop --- controllers/jwker_controller.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/controllers/jwker_controller.go b/controllers/jwker_controller.go index 7db4244..7a476a2 100644 --- a/controllers/jwker_controller.go +++ b/controllers/jwker_controller.go @@ -160,10 +160,6 @@ func (r *JwkerReconciler) create(tx transaction) error { } } - if err != nil { - return fmt.Errorf("failed registering client: %s", err) - } - r.logger.Info(fmt.Sprintf("Reconciling secrets for app %s in namespace %s", app.Name, app.Namespace)) jwk, err := secret.FirstJWK(tx.keyset.Private)