From ef3e14f107059b664601c54b752453bb4e521f75 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 19 Mar 2024 09:39:34 +0000 Subject: [PATCH] CSS-7081 Add OAuth-specific methods to secrets store (#1175) * Add `Get/Put` OAuth key methods to Vault store Signed-off-by: Babak K. Shandiz * Add `Get/Put` OAuth key methods to Postgres store Signed-off-by: Babak K. Shandiz * Add `Get/Put` OAuth key methods to in-memory store Signed-off-by: Babak K. Shandiz * Add `Get/Put` OAuth key methods to mock store Signed-off-by: Babak K. Shandiz * Add `Get/Put` OAuth key methods to credential store interface Signed-off-by: Babak K. Shandiz * Expose underlying `CredentialStore` via a `JIMM` interface method Signed-off-by: Babak K. Shandiz * Use `New*` method to instantiate in-memory credential store Signed-off-by: Babak K. Shandiz * Use credential store to retrieve OAuth session JWT secret key Signed-off-by: Babak K. Shandiz * Update `CredentialStore` godoc Signed-off-by: Babak K. Shandiz * Add test to verify `GetOAuthKey` returns not found error Signed-off-by: Babak K. Shandiz * Add test to verify `GetOAuthKey` returns not found error Signed-off-by: Babak K. Shandiz * Add `CheckOrGenerateOAuthKey` method Signed-off-by: Babak K. Shandiz * Generate OAuth key on the leader unit Signed-off-by: Babak K. Shandiz * Update suite to generate OAuth key as well Signed-off-by: Babak K. Shandiz * Add package godoc Signed-off-by: Babak K. Shandiz * Reuse shared `JWTTestSecret` in `JimmCmdSuite` Signed-off-by: Babak K. Shandiz * Fix godoc Signed-off-by: Babak K. Shandiz * Add `CleanupOAuth` to Postgres store Signed-off-by: Babak K. Shandiz * Add `CleanupOAuth` to Vault store Signed-off-by: Babak K. Shandiz * Add `CleanupOAuth` to mock store Signed-off-by: Babak K. Shandiz * Add `CleanupOAuth` to in-memory store Signed-off-by: Babak K. Shandiz * Add `CleanupOAuth` to credential store interface Signed-off-by: Babak K. Shandiz * Use same const secret for in-memory store Signed-off-by: Babak K. Shandiz * fix tests with populating OAuth key secrets in store Signed-off-by: Babak K. Shandiz * Use `*WithContext` variants for read/write methods Signed-off-by: Babak K. Shandiz * Use `net.Listen` to find an available TCP port Signed-off-by: Babak K. Shandiz * Rename `CleanupOAuth` to `CleanupOAuthSecrets` Signed-off-by: Babak K. Shandiz * Rename credential store `*OAuthKey` methods to `*OAuthSecret` Signed-off-by: Babak K. Shandiz * Run `go mod tidy` Signed-off-by: Babak K. Shandiz --------- Signed-off-by: Babak K. Shandiz --- cmd/jimmsrv/main.go | 12 ++- go.mod | 7 -- go.sum | 21 ----- internal/cmdtest/jimmsuite.go | 3 + internal/db/export_test.go | 2 + internal/db/secrets.go | 44 +++++++++ internal/db/secrets_test.go | 28 ++++++ internal/jimm/cloudcredential_test.go | 12 +++ internal/jimm/credentials/credentials.go | 12 +++ internal/jimm/jimm.go | 5 + internal/jimm/jimm_test.go | 4 +- internal/jimmhttp/auth_handler_test.go | 17 ++-- internal/jimmtest/auth.go | 6 +- internal/jimmtest/jimm_mock.go | 8 ++ internal/jimmtest/store.go | 44 +++++++++ internal/jimmtest/suite.go | 4 +- internal/jujuapi/admin.go | 18 ++-- internal/jujuapi/controllerroot.go | 2 + internal/vault/vault.go | 114 +++++++++++++++++++---- internal/vault/vault_test.go | 41 ++++++++ service.go | 29 ++++++ service_test.go | 28 ++++-- 22 files changed, 382 insertions(+), 79 deletions(-) diff --git a/cmd/jimmsrv/main.go b/cmd/jimmsrv/main.go index 37079c200..e9918c682 100644 --- a/cmd/jimmsrv/main.go +++ b/cmd/jimmsrv/main.go @@ -179,13 +179,17 @@ func start(ctx context.Context, s *service.Service) error { s.Go(func() error { return jimmsvc.WatchModelSummaries(ctx) }) if os.Getenv("JIMM_ENABLE_JWKS_ROTATOR") != "" { - zapctx.Info(ctx, "attempting to start JWKS rotator") + zapctx.Info(ctx, "attempting to start JWKS rotator and generate OAuth secret key") s.Go(func() error { - err := jimmsvc.StartJWKSRotator(ctx, time.NewTicker(time.Hour).C, time.Now().UTC().AddDate(0, 3, 0)) - if err != nil { + if err := jimmsvc.StartJWKSRotator(ctx, time.NewTicker(time.Hour).C, time.Now().UTC().AddDate(0, 3, 0)); err != nil { zapctx.Error(ctx, "failed to start JWKS rotator", zap.Error(err)) + return err } - return err + if err := jimmsvc.CheckOrGenerateOAuthKey(ctx); err != nil { + zapctx.Error(ctx, "failed to check/generate OAuth secret key", zap.Error(err)) + return err + } + return nil }) } diff --git a/go.mod b/go.mod index d931672c5..a27924975 100644 --- a/go.mod +++ b/go.mod @@ -102,10 +102,7 @@ require ( github.com/aws/smithy-go v1.19.0 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/bmizerany/pat v0.0.0-20210406213842-e4b6760bdd6f // indirect - github.com/canonical/go-flags v0.0.0-20230403090104-105d09a091b8 // indirect github.com/canonical/lxd v0.0.0-20231214113525-e676fc63c50a // indirect - github.com/canonical/pebble v1.9.0 // indirect - github.com/canonical/x-go v0.0.0-20230522092633-7947a7587f5b // indirect github.com/cenkalti/backoff/v3 v3.0.0 // indirect github.com/cespare/xxhash/v2 v2.2.0 // indirect github.com/cjlapao/common-go v0.0.39 // indirect @@ -144,7 +141,6 @@ require ( github.com/google/s2a-go v0.1.7 // indirect github.com/googleapis/enterprise-certificate-proxy v0.3.2 // indirect github.com/googleapis/gax-go/v2 v2.12.0 // indirect - github.com/gorilla/mux v1.8.1 // indirect github.com/gorilla/schema v1.2.1 // indirect github.com/gorilla/securecookie v1.1.2 // indirect github.com/gorilla/sessions v1.2.1 // indirect @@ -262,7 +258,6 @@ require ( github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pkg/sftp v1.13.6 // indirect - github.com/pkg/term v1.1.0 // indirect github.com/pkg/xattr v0.4.9 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/prometheus/client_model v0.5.0 // indirect @@ -302,12 +297,10 @@ require ( go.uber.org/multierr v1.11.0 // indirect golang.org/x/crypto v0.19.0 // indirect golang.org/x/exp v0.0.0-20231127185646-65229373498e // indirect - golang.org/x/mod v0.14.0 // indirect golang.org/x/sys v0.17.0 // indirect golang.org/x/term v0.17.0 // indirect golang.org/x/text v0.14.0 // indirect golang.org/x/time v0.5.0 // indirect - golang.org/x/tools v0.16.1 // indirect google.golang.org/api v0.154.0 // indirect google.golang.org/appengine v1.6.8 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20231127180814-3a041ad873d4 // indirect diff --git a/go.sum b/go.sum index b53d035bd..d82c9d791 100644 --- a/go.sum +++ b/go.sum @@ -145,18 +145,12 @@ github.com/bmizerany/pat v0.0.0-20210406213842-e4b6760bdd6f h1:gOO/tNZMjjvTKZWpY github.com/bmizerany/pat v0.0.0-20210406213842-e4b6760bdd6f/go.mod h1:8rLXio+WjiTceGBHIoTvn60HIbs7Hm7bcHjyrSqYB9c= github.com/canonical/go-dqlite v1.21.0 h1:4gLDdV2GF+vg0yv9Ff+mfZZNQ1JGhnQ3GnS2GeZPHfA= github.com/canonical/go-dqlite v1.21.0/go.mod h1:Uvy943N8R4CFUAs59A1NVaziWY9nJ686lScY7ywurfg= -github.com/canonical/go-flags v0.0.0-20230403090104-105d09a091b8 h1:zGaJEJI9qPVyM+QKFJagiyrM91Ke5S9htoL1D470g6E= -github.com/canonical/go-flags v0.0.0-20230403090104-105d09a091b8/go.mod h1:ZZFeR9K9iGgpwOaLYF9PdT44/+lfSJ9sQz3B+SsGsYU= github.com/canonical/go-service v1.0.0 h1:TF6TsEp04xAoI5pPoWjTYmEwLjbPATSnHEyeJCvzElg= github.com/canonical/go-service v1.0.0/go.mod h1:GzNLXpkGdglL0kjREXoLXj2rB2Qx+EvAGncRDqCENYQ= github.com/canonical/lxd v0.0.0-20231214113525-e676fc63c50a h1:Tfo/MzXK5GeG7gzSHqxGeY/669Mhh5ea43dn1mRDnk8= github.com/canonical/lxd v0.0.0-20231214113525-e676fc63c50a/go.mod h1:UxfHGKFoRjgu1NUA9EFiR++dKvyAiT0h9HT0ffMlzjc= github.com/canonical/ofga v0.10.0 h1:DHXhG/DAXWWQT/I+2jzr4qm0uTIYrILmtMxd6ZqmEzE= github.com/canonical/ofga v0.10.0/go.mod h1:u4Ou8dbIhO7FmVlT7W3rX2roD9AOGz/CqmGh7AdF0Lo= -github.com/canonical/pebble v1.9.0 h1:FWVEh1fg3aaW2HNue2Z2eYMwkJEQT8mgMFW3R5Iocn4= -github.com/canonical/pebble v1.9.0/go.mod h1:9Qkjmq298g0+9SvM2E5eekkEN4pjHDWhgg9eB2I0tjk= -github.com/canonical/x-go v0.0.0-20230522092633-7947a7587f5b h1:Da2fardddn+JDlVEYtrzBLTtyzoyU3nIS0Cf0GvjmwU= -github.com/canonical/x-go v0.0.0-20230522092633-7947a7587f5b/go.mod h1:upTK9n6rlqITN9rCN69hdreI37dRDFUk2thlGGD5Cg8= github.com/cenkalti/backoff/v3 v3.0.0 h1:ske+9nBpD9qZsTBoF41nW5L+AIuFBKMeze18XQ3eG1c= github.com/cenkalti/backoff/v3 v3.0.0/go.mod h1:cIeZDE3IrqwwJl6VUwCN6trj1oXrTS4rc0ij+ULvLYs= github.com/cenkalti/backoff/v4 v4.2.1 h1:y4OZtCnogmCPw98Zjyt5a6+QwPLGkiQsYW5oUqylYbM= @@ -727,8 +721,6 @@ github.com/julienschmidt/httprouter v1.3.0 h1:U0609e9tgbseu3rBINet9P48AI/D3oJs4d github.com/julienschmidt/httprouter v1.3.0/go.mod h1:JR6WtHb+2LUe8TCKY3cZOxFyyO8IZAc4RVcycCCAKdM= github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs= github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8= -github.com/kian99/juju v0.0.0-20240301094235-2688d7cd925e h1:MnSWbm0Th+V7YI61C4ledtaxg564bzwBdh665fj/MeM= -github.com/kian99/juju v0.0.0-20240301094235-2688d7cd925e/go.mod h1:V5eSJgiG7Evs4ejKhI7na7olYzHR1rxZXwx1/27Sa18= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= @@ -922,8 +914,6 @@ github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE github.com/pkg/sftp v1.10.1/go.mod h1:lYOWFsE0bwd1+KfKJaKeuokY15vzFx25BLbzYYoAxZI= github.com/pkg/sftp v1.13.6 h1:JFZT4XbOU7l77xGSpOdW+pwIMqP044IyjXX6FGyEKFo= github.com/pkg/sftp v1.13.6/go.mod h1:tz1ryNURKu77RL+GuCzmoJYxQczL3wLNNpPWagdg4Qk= -github.com/pkg/term v1.1.0 h1:xIAAdCMh3QIAy+5FrE8Ad8XoDhEU4ufwbaSozViP9kk= -github.com/pkg/term v1.1.0/go.mod h1:E25nymQcrSllhX42Ok8MRm1+hyBdHY0dCeiKZ9jpNGw= github.com/pkg/xattr v0.4.9 h1:5883YPCtkSd8LFbs13nXplj9g9tlrwoJRjgpgMu1/fE= github.com/pkg/xattr v0.4.9/go.mod h1:di8WF84zAKk8jzR1UBTEWh9AUlIZZ7M/JNt8e9B6ktU= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= @@ -1128,8 +1118,6 @@ golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5y golang.org/x/crypto v0.0.0-20211215153901-e495a2d5b3d3/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.1.0/go.mod h1:RecgLatLF4+eUMCP1PoPZQb+cVrJcOPbHkTkbkB9sbw= -golang.org/x/crypto v0.17.0 h1:r8bRNjWL3GshPW3gkd+RpvzWrZAwPS49OmTGZ/uhM4k= -golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= golang.org/x/crypto v0.19.0 h1:ENy+Az/9Y1vSrlrvBSyna3PITt4tiZLf7sgCjZBX7Wo= golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= @@ -1170,8 +1158,6 @@ golang.org/x/mod v0.4.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.1/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= -golang.org/x/mod v0.14.0 h1:dGoOF9QVLYng8IHTm7BAyWqCqSheQ5pYWGhzW00YJr0= -golang.org/x/mod v0.14.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/net v0.0.0-20150829230318-ea47fc708ee3/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180406214816-61147c48b25b/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -1220,8 +1206,6 @@ golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qx golang.org/x/net v0.0.0-20211216030914-fe4d6282115f/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= golang.org/x/net v0.1.0/go.mod h1:Cx3nUiGt4eDBEyega/BKRp+/AlGL8hYe7U9odMt2Cco= -golang.org/x/net v0.19.0 h1:zTwKpTd2XuCqf8huc7Fo2iSy+4RHPd10s4KzeTnVr1c= -golang.org/x/net v0.19.0/go.mod h1:CfAk/cbD4CthTvqiEl8NpboMuiuOYsAr/7NOjZJtv1U= golang.org/x/net v0.21.0 h1:AQyQV4dYCvJ7vGmJyKki9+PBdyvhkSd8EIx/qb0AYv4= golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -1291,7 +1275,6 @@ golang.org/x/sys v0.0.0-20200523222454-059865788121/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200728102440-3e129f6d46b1/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200803210538-64077c9b5642/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200905004654-be1d3432aa8f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20200909081042-eff7692f9009/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201201145000-ef89a241ccb3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -1320,8 +1303,6 @@ golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.15.0 h1:h48lPFYpsTvQJZF4EKyI4aLHaev3CxivZmv7yZig9pc= -golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.17.0 h1:25cE3gD+tdBA7lp7QfhuV+rJiE9YXTcS3VG1SqssI/Y= golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= @@ -1330,8 +1311,6 @@ golang.org/x/term v0.0.0-20201210144234-2321bbc49cbf/go.mod h1:bj7SfCRtBDWHUb9sn golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.1.0/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= -golang.org/x/term v0.15.0 h1:y/Oo/a/q3IXu26lQgl04j/gjuBDOBlx7X6Om1j2CPW4= -golang.org/x/term v0.15.0/go.mod h1:BDl952bC7+uMoWR75FIrCDx79TPU9oHkTZ9yRbYOrX0= golang.org/x/term v0.17.0 h1:mkTF7LCd6WGJNL3K1Ad7kwxNfYAW6a8a8QqtMblp/4U= golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= diff --git a/internal/cmdtest/jimmsuite.go b/internal/cmdtest/jimmsuite.go index bc90bc5ec..50de3118f 100644 --- a/internal/cmdtest/jimmsuite.go +++ b/internal/cmdtest/jimmsuite.go @@ -100,6 +100,9 @@ func (s *JimmCmdSuite) SetUpTest(c *gc.C) { err = s.Service.StartJWKSRotator(ctx, time.NewTicker(time.Hour).C, time.Now().UTC().AddDate(0, 3, 0)) c.Assert(err, gc.Equals, nil) + err = s.JIMM.GetCredentialStore().PutOAuthSecret(ctx, []byte(jimmtest.JWTTestSecret)) + c.Assert(err, gc.Equals, nil) + s.HTTP.StartTLS() // NOW we can set up the juju conn suites diff --git a/internal/db/export_test.go b/internal/db/export_test.go index 2d5d6efba..be98a0c08 100644 --- a/internal/db/export_test.go +++ b/internal/db/export_test.go @@ -7,4 +7,6 @@ var ( JwksPublicKeyTag = jwksPublicKeyTag JwksPrivateKeyTag = jwksPrivateKeyTag JwksExpiryTag = jwksExpiryTag + OAuthKind = oauthKind + OAuthKeyTag = oauthKeyTag ) diff --git a/internal/db/secrets.go b/internal/db/secrets.go index 9cd9b129c..87434599f 100644 --- a/internal/db/secrets.go +++ b/internal/db/secrets.go @@ -26,6 +26,8 @@ const ( jwksPublicKeyTag = "jwksPublicKey" jwksPrivateKeyTag = "jwksPrivateKey" jwksExpiryTag = "jwksExpiry" + oauthKind = "oauth" + oauthKeyTag = "oauthKey" ) // UpsertSecret stores secret information. @@ -280,3 +282,45 @@ func (d *Database) PutJWKSExpiry(ctx context.Context, expiry time.Time) error { secret := dbmodel.NewSecret(jwksKind, jwksExpiryTag, expiryJson) return d.UpsertSecret(ctx, &secret) } + +// CleanupOAuthSecrets removes all secrets associated with OAuth. +func (d *Database) CleanupOAuthSecrets(ctx context.Context) error { + const op = errors.Op("database.CleanupOAuthSecrets") + secret := dbmodel.NewSecret(oauthKind, oauthKeyTag, nil) + err := d.DeleteSecret(ctx, &secret) + if err != nil { + zapctx.Error(ctx, "failed to cleanup OAUth key", zap.Error(err)) + return errors.E(op, err, "failed to cleanup OAUth key") + } + return nil +} + +// GetOAuthSecret returns the current HS256 (symmetric encryption) secret used to sign OAuth session tokens. +func (d *Database) GetOAuthSecret(ctx context.Context) ([]byte, error) { + const op = errors.Op("database.GetOAuthSecret") + secret := dbmodel.NewSecret(oauthKind, oauthKeyTag, nil) + err := d.GetSecret(ctx, &secret) + if err != nil { + zapctx.Error(ctx, "failed to get oauth key", zap.Error(err)) + return nil, errors.E(op, err) + } + var pem []byte + err = json.Unmarshal(secret.Data, &pem) + if err != nil { + zapctx.Error(ctx, "failed to unmarshal pem data", zap.Error(err)) + return nil, errors.E(op, err) + } + return pem, nil +} + +// PutOAuthSecret puts a HS256 (symmetric encryption) secret into the credentials store for signing OAuth session tokens. +func (d *Database) PutOAuthSecret(ctx context.Context, raw []byte) error { + const op = errors.Op("database.PutOAuthSecret") + oauthKey, err := json.Marshal(raw) + if err != nil { + zapctx.Error(ctx, "failed to marshal pem data", zap.Error(err)) + return errors.E(op, err, "failed to marshal oauth key") + } + secret := dbmodel.NewSecret(oauthKind, oauthKeyTag, oauthKey) + return d.UpsertSecret(ctx, &secret) +} diff --git a/internal/db/secrets_test.go b/internal/db/secrets_test.go index 71e2249f4..cc3f79af3 100644 --- a/internal/db/secrets_test.go +++ b/internal/db/secrets_test.go @@ -279,3 +279,31 @@ func (s *dbSuite) TestCleanupJWKS(c *qt.C) { c.Assert(s.Database.DB.Model(&dbmodel.Secret{}).Count(&count).Error, qt.IsNil) c.Assert(count, qt.Equals, int64(0)) } + +func (s *dbSuite) TestPutAndGetOAuthSecret(c *qt.C) { + err := s.Database.Migrate(context.Background(), true) + c.Assert(err, qt.Equals, nil) + ctx := context.Background() + key := []byte(uuid.NewString()) + c.Assert(s.Database.PutOAuthSecret(ctx, key), qt.IsNil) + + secret := dbmodel.Secret{} + tx := s.Database.DB.First(&secret) + c.Assert(tx.Error, qt.IsNil) + c.Assert(secret.Type, qt.Equals, db.OAuthKind) + c.Assert(secret.Tag, qt.Equals, db.OAuthKeyTag) + + retrievedKey, err := s.Database.GetOAuthSecret(ctx) + c.Assert(err, qt.IsNil) + c.Assert(retrievedKey, qt.DeepEquals, key) +} + +func (s *dbSuite) TestGetOAuthSecretFailsIfNotFound(c *qt.C) { + err := s.Database.Migrate(context.Background(), true) + c.Assert(err, qt.Equals, nil) + ctx := context.Background() + + retrieved, err := s.Database.GetOAuthSecret(ctx) + c.Assert(err, qt.ErrorMatches, "secret not found") + c.Assert(retrieved, qt.IsNil) +} diff --git a/internal/jimm/cloudcredential_test.go b/internal/jimm/cloudcredential_test.go index 717a0fc04..2cd671d0c 100644 --- a/internal/jimm/cloudcredential_test.go +++ b/internal/jimm/cloudcredential_test.go @@ -1770,3 +1770,15 @@ func (s testCloudCredentialAttributeStore) PutJWKSExpiry(ctx context.Context, ex func (s testCloudCredentialAttributeStore) CleanupJWKS(ctx context.Context) error { return errors.E(errors.CodeNotImplemented) } + +func (s testCloudCredentialAttributeStore) CleanupOAuthSecrets(ctx context.Context) error { + return errors.E(errors.CodeNotImplemented) +} + +func (s testCloudCredentialAttributeStore) GetOAuthSecret(ctx context.Context) ([]byte, error) { + return nil, errors.E(errors.CodeNotImplemented) +} + +func (s testCloudCredentialAttributeStore) PutOAuthSecret(ctx context.Context, raw []byte) error { + return errors.E(errors.CodeNotImplemented) +} diff --git a/internal/jimm/credentials/credentials.go b/internal/jimm/credentials/credentials.go index 239854042..4f28d70ee 100644 --- a/internal/jimm/credentials/credentials.go +++ b/internal/jimm/credentials/credentials.go @@ -1,5 +1,7 @@ // Copyright 2023 canonical. +// Package credentials provides abstractions/definitions for credential storage +// backends and caching mechanisms. package credentials import ( @@ -16,6 +18,7 @@ import ( // - JWK Set // - JWK expiry // - JWK private key +// - OAuth session signing secret type CredentialStore interface { // Get retrieves the stored attributes of a cloud credential. Get(context.Context, names.CloudCredentialTag) (map[string]string, error) @@ -51,4 +54,13 @@ type CredentialStore interface { // PutJWKSExpiry sets the expiry time for the current JWKS within the store. PutJWKSExpiry(ctx context.Context, expiry time.Time) error + + // CleanupOAuthSecrets removes all secrets associated with OAuth. + CleanupOAuthSecrets(ctx context.Context) error + + // GetOAuthSecret returns the current HS256 (symmetric encryption) secret used to sign OAuth session tokens. + GetOAuthSecret(ctx context.Context) ([]byte, error) + + // PutOAuthSecret puts a HS256 (symmetric encryption) secret into the credentials store for signing OAuth session tokens. + PutOAuthSecret(ctx context.Context, raw []byte) error } diff --git a/internal/jimm/jimm.go b/internal/jimm/jimm.go index af6ce42f3..828e30109 100644 --- a/internal/jimm/jimm.go +++ b/internal/jimm/jimm.go @@ -173,6 +173,11 @@ type OAuthAuthenticator interface { VerifyClientCredentials(ctx context.Context, clientID string, clientSecret string) error } +// GetCredentialStore returns the credential store used by JIMM. +func (j *JIMM) GetCredentialStore() credentials.CredentialStore { + return j.CredentialStore +} + type permission struct { resource string relation string diff --git a/internal/jimm/jimm_test.go b/internal/jimm/jimm_test.go index 166cfae15..690d5410a 100644 --- a/internal/jimm/jimm_test.go +++ b/internal/jimm/jimm_test.go @@ -696,7 +696,7 @@ func TestFillMigrationTarget(t *testing.T) { err := db.Migrate(ctx, false) c.Assert(err, qt.IsNil) - store := &jimmtest.InMemoryCredentialStore{} + store := jimmtest.NewInMemoryCredentialStore() err = store.PutControllerCredentials(context.Background(), test.controllerName, "admin", "test-secret") c.Assert(err, qt.IsNil) @@ -775,7 +775,7 @@ func TestInitiateInternalMigration(t *testing.T) { c.Patch(jimm.InitiateMigration, func(ctx context.Context, j *jimm.JIMM, user *openfga.User, spec jujuparams.MigrationSpec, targetID uint) (jujuparams.InitiateMigrationResult, error) { return jujuparams.InitiateMigrationResult{}, nil }) - store := &jimmtest.InMemoryCredentialStore{} + store := jimmtest.NewInMemoryCredentialStore() err := store.PutControllerCredentials(context.Background(), test.migrateInfo.TargetController, "admin", "test-secret") c.Assert(err, qt.IsNil) diff --git a/internal/jimmhttp/auth_handler_test.go b/internal/jimmhttp/auth_handler_test.go index 849623985..81cdc95ef 100644 --- a/internal/jimmhttp/auth_handler_test.go +++ b/internal/jimmhttp/auth_handler_test.go @@ -4,14 +4,12 @@ import ( "context" "fmt" "io" - "math/rand" "net" "net/http" "net/http/cookiejar" "net/http/httptest" "net/url" "regexp" - "strconv" "testing" "time" @@ -42,17 +40,14 @@ func setupDbAndSessionStore(c *qt.C) (*db.Database, *pgstore.PGStore) { } func setupTestServer(c *qt.C, dashboardURL string, db *db.Database, sessionStore *pgstore.PGStore) *httptest.Server { + // Find a random free TCP port. + listener, err := net.Listen("tcp", "127.0.0.1:0") + c.Assert(err, qt.IsNil) + port := fmt.Sprintf("%d", listener.Addr().(*net.TCPAddr).Port) + // Create unstarted server to enable auth service s := httptest.NewUnstartedServer(nil) - // Setup random port listener - minPort := 30000 - maxPort := 50000 - - port := strconv.Itoa(rand.Intn(maxPort-minPort+1) + minPort) - l, err := net.Listen("tcp", "localhost:"+port) - c.Assert(err, qt.IsNil) - // Set the listener with a random port - s.Listener = l + s.Listener = listener // Remember redirect url to check it matches after test server starts redirectURL := "http://127.0.0.1:" + port + "/callback" diff --git a/internal/jimmtest/auth.go b/internal/jimmtest/auth.go index fe7066463..19212b0f1 100644 --- a/internal/jimmtest/auth.go +++ b/internal/jimmtest/auth.go @@ -18,8 +18,8 @@ import ( "github.com/canonical/jimm/internal/openfga" ) -var ( - jwtTestSecret = "test-secret" +const ( + JWTTestSecret = "test-secret" ) // A SimpleTester is a simple version of the test interface @@ -70,7 +70,7 @@ func NewUserSessionLogin(c SimpleTester, username string) api.LoginProvider { c.Fatalf("failed to generate test session token") } - freshToken, err := jwt.Sign(token, jwt.WithKey(jwa.HS256, []byte(jwtTestSecret))) + freshToken, err := jwt.Sign(token, jwt.WithKey(jwa.HS256, []byte(JWTTestSecret))) if err != nil { c.Fatalf("failed to sign test session token") } diff --git a/internal/jimmtest/jimm_mock.go b/internal/jimmtest/jimm_mock.go index b22deb2e6..6b598f344 100644 --- a/internal/jimmtest/jimm_mock.go +++ b/internal/jimmtest/jimm_mock.go @@ -17,6 +17,7 @@ import ( "github.com/canonical/jimm/internal/dbmodel" "github.com/canonical/jimm/internal/errors" "github.com/canonical/jimm/internal/jimm" + jimmcreds "github.com/canonical/jimm/internal/jimm/credentials" "github.com/canonical/jimm/internal/openfga" ofganames "github.com/canonical/jimm/internal/openfga/names" "github.com/canonical/jimm/internal/pubsub" @@ -59,6 +60,7 @@ type JIMM struct { GetCloudCredential_ func(ctx context.Context, user *openfga.User, tag names.CloudCredentialTag) (*dbmodel.CloudCredential, error) GetCloudCredentialAttributes_ func(ctx context.Context, u *openfga.User, cred *dbmodel.CloudCredential, hidden bool) (attrs map[string]string, redacted []string, err error) GetControllerConfig_ func(ctx context.Context, u *dbmodel.Identity) (*dbmodel.ControllerConfig, error) + GetCredentialStore_ func() jimmcreds.CredentialStore GetJimmControllerAccess_ func(ctx context.Context, user *openfga.User, tag names.UserTag) (string, error) GetUser_ func(ctx context.Context, username string) (*openfga.User, error) GetOpenFGAUserAndAuthorise_ func(ctx context.Context, email string) (*openfga.User, error) @@ -300,6 +302,12 @@ func (j *JIMM) GetControllerConfig(ctx context.Context, u *dbmodel.Identity) (*d } return j.GetControllerConfig_(ctx, u) } +func (j *JIMM) GetCredentialStore() jimmcreds.CredentialStore { + if j.GetCredentialStore_ == nil { + return nil + } + return j.GetCredentialStore_() +} func (j *JIMM) GetJimmControllerAccess(ctx context.Context, user *openfga.User, tag names.UserTag) (string, error) { if j.GetJimmControllerAccess_ == nil { return "", errors.E(errors.CodeNotImplemented) diff --git a/internal/jimmtest/store.go b/internal/jimmtest/store.go index 9e542e6b6..f44fc0972 100644 --- a/internal/jimmtest/store.go +++ b/internal/jimmtest/store.go @@ -24,10 +24,19 @@ type InMemoryCredentialStore struct { jwks jwk.Set privateKey []byte expiry time.Time + oauthKey []byte controllerCredentials map[string]controllerCredentials cloudCredentialAttributes map[string]map[string]string } +// NewInMemoryCredentialStore returns a new instance of `InMemoryCredentialStore` +// with some secrets/keys being populated. +func NewInMemoryCredentialStore() *InMemoryCredentialStore { + return &InMemoryCredentialStore{ + oauthKey: []byte(JWTTestSecret), + } +} + // Get retrieves the stored attributes of a cloud credential. func (s *InMemoryCredentialStore) Get(ctx context.Context, credTag names.CloudCredentialTag) (map[string]string, error) { s.mu.Lock() @@ -177,3 +186,38 @@ func (s *InMemoryCredentialStore) PutJWKSExpiry(ctx context.Context, expiry time return nil } + +// CleanupOAuthSecrets removes all secrets associated with OAuth. +func (s *InMemoryCredentialStore) CleanupOAuthSecrets(ctx context.Context) error { + s.mu.Lock() + defer s.mu.Unlock() + + s.oauthKey = nil + return nil +} + +// GetOAuthSecret returns the current HS256 (symmetric encryption) secret used to sign OAuth session tokens. +func (s *InMemoryCredentialStore) GetOAuthSecret(ctx context.Context) ([]byte, error) { + s.mu.RLock() + defer s.mu.RUnlock() + + if s.oauthKey == nil || len(s.oauthKey) == 0 { + return nil, errors.E(errors.CodeNotFound) + } + + key := make([]byte, len(s.oauthKey)) + copy(key, s.oauthKey) + + return key, nil +} + +// PutOAuthSecret puts a HS256 (symmetric encryption) secret into the credentials store for signing OAuth session tokens. +func (s *InMemoryCredentialStore) PutOAuthSecret(ctx context.Context, raw []byte) error { + s.mu.Lock() + defer s.mu.Unlock() + + s.oauthKey = make([]byte, len(raw)) + copy(s.oauthKey, raw) + + return nil +} diff --git a/internal/jimmtest/suite.go b/internal/jimmtest/suite.go index 01fc0d9a8..ff8a465e1 100644 --- a/internal/jimmtest/suite.go +++ b/internal/jimmtest/suite.go @@ -74,7 +74,7 @@ func (s *JIMMSuite) SetUpTest(c *gc.C) { Database: db.Database{ DB: PostgresDB(GocheckTester{c}, nil), }, - CredentialStore: &InMemoryCredentialStore{}, + CredentialStore: NewInMemoryCredentialStore(), Pubsub: &pubsub.Hub{MaxConcurrency: 10}, UUID: ControllerUUID, OpenFGAClient: s.OFGAClient, @@ -84,7 +84,7 @@ func (s *JIMMSuite) SetUpTest(c *gc.C) { s.cancel = cancel // Note that the secret key here must match what is used in tests. - s.JIMM.OAuthAuthenticator = NewMockOAuthAuthenticator(jwtTestSecret) + s.JIMM.OAuthAuthenticator = NewMockOAuthAuthenticator(JWTTestSecret) err = s.JIMM.Database.Migrate(ctx, false) c.Assert(err, gc.Equals, nil) diff --git a/internal/jujuapi/admin.go b/internal/jujuapi/admin.go index 80868248b..e6e8644a5 100644 --- a/internal/jujuapi/admin.go +++ b/internal/jujuapi/admin.go @@ -88,9 +88,12 @@ func (r *controllerRoot) GetDeviceSessionToken(ctx context.Context) (params.GetD return response, errors.E(op, err) } - // TODO(ale8k): Add vault logic to get secret key and generate one - // on start up. - encToken, err := authSvc.MintSessionToken(email, "test-secret") + secretKey, err := r.jimm.GetCredentialStore().GetOAuthSecret(ctx) + if err != nil { + return response, errors.E(op, err, "failed to retrieve oauth secret key") + } + + encToken, err := authSvc.MintSessionToken(email, string(secretKey)) if err != nil { return response, errors.E(op, err) } @@ -110,9 +113,12 @@ func (r *controllerRoot) LoginWithSessionToken(ctx context.Context, req params.L authenticationSvc := r.jimm.OAuthAuthenticationService() // Verify the session token - // TODO(CSS-7081): Ensure for tests that the secret key can be configured. - // Or configure cmd tests to use the configured secret. - jwtToken, err := authenticationSvc.VerifySessionToken(req.SessionToken, "test-secret") + secretKey, err := r.jimm.GetCredentialStore().GetOAuthSecret(ctx) + if err != nil { + return jujuparams.LoginResult{}, errors.E(op, err, "failed to retrieve oauth secret key") + } + + jwtToken, err := authenticationSvc.VerifySessionToken(req.SessionToken, string(secretKey)) if err != nil { var aerr *auth.AuthenticationError if stderrors.As(err, &aerr) { diff --git a/internal/jujuapi/controllerroot.go b/internal/jujuapi/controllerroot.go index 883cd9881..8c3eb6778 100644 --- a/internal/jujuapi/controllerroot.go +++ b/internal/jujuapi/controllerroot.go @@ -19,6 +19,7 @@ import ( "github.com/canonical/jimm/internal/dbmodel" "github.com/canonical/jimm/internal/errors" "github.com/canonical/jimm/internal/jimm" + "github.com/canonical/jimm/internal/jimm/credentials" "github.com/canonical/jimm/internal/jujuapi/rpc" "github.com/canonical/jimm/internal/openfga" ofganames "github.com/canonical/jimm/internal/openfga/names" @@ -57,6 +58,7 @@ type JIMM interface { GetCloudCredential(ctx context.Context, user *openfga.User, tag names.CloudCredentialTag) (*dbmodel.CloudCredential, error) GetCloudCredentialAttributes(ctx context.Context, u *openfga.User, cred *dbmodel.CloudCredential, hidden bool) (attrs map[string]string, redacted []string, err error) GetControllerConfig(ctx context.Context, u *dbmodel.Identity) (*dbmodel.ControllerConfig, error) + GetCredentialStore() credentials.CredentialStore GetJimmControllerAccess(ctx context.Context, user *openfga.User, tag names.UserTag) (string, error) GetUser(ctx context.Context, username string) (*openfga.User, error) GetUserCloudAccess(ctx context.Context, user *openfga.User, cloud names.CloudTag) (string, error) diff --git a/internal/vault/vault.go b/internal/vault/vault.go index a9e7c4cb0..a25809963 100644 --- a/internal/vault/vault.go +++ b/internal/vault/vault.go @@ -59,7 +59,7 @@ func (s *VaultStore) Get(ctx context.Context, tag names.CloudCredentialTag) (map return nil, errors.E(op, err) } - secret, err := client.Logical().Read(s.path(tag)) + secret, err := client.Logical().ReadWithContext(ctx, s.path(tag)) if err != nil { return nil, errors.E(op, err) } @@ -96,7 +96,7 @@ func (s *VaultStore) Put(ctx context.Context, tag names.CloudCredentialTag, attr for k, v := range attr { data[k] = v } - _, err = client.Logical().Write(s.path(tag), data) + _, err = client.Logical().WriteWithContext(ctx, s.path(tag), data) if err != nil { return errors.E(op, err) } @@ -112,7 +112,7 @@ func (s *VaultStore) delete(ctx context.Context, tag names.CloudCredentialTag) e if err != nil { return errors.E(op, err) } - _, err = client.Logical().Delete(s.path(tag)) + _, err = client.Logical().DeleteWithContext(ctx, s.path(tag)) if rerr, ok := err.(*api.ResponseError); ok && rerr.StatusCode == http.StatusNotFound { // Ignore the error if attempting to delete something that isn't there. err = nil @@ -133,7 +133,7 @@ func (s *VaultStore) GetControllerCredentials(ctx context.Context, controllerNam return "", "", errors.E(op, err) } - secret, err := client.Logical().Read(s.controllerCredentialsPath(controllerName)) + secret, err := client.Logical().ReadWithContext(ctx, s.controllerCredentialsPath(controllerName)) if err != nil { return "", "", errors.E(op, err) } @@ -169,7 +169,7 @@ func (s *VaultStore) PutControllerCredentials(ctx context.Context, controllerNam usernameKey: username, passwordKey: password, } - _, err = client.Logical().Write(s.controllerCredentialsPath(controllerName), data) + _, err = client.Logical().WriteWithContext(ctx, s.controllerCredentialsPath(controllerName), data) if err != nil { return errors.E(op, err) } @@ -186,9 +186,9 @@ func (s *VaultStore) CleanupJWKS(ctx context.Context) error { } // Vault does not return errors on deletion requests where // the secret does not exist. As such we just return the last known error. - client.Logical().Delete(s.getJWKSExpiryPath()) - client.Logical().Delete(s.getJWKSPath()) - if _, err = client.Logical().Delete(s.getJWKSPrivateKeyPath()); err != nil { + client.Logical().DeleteWithContext(ctx, s.getJWKSExpiryPath()) + client.Logical().DeleteWithContext(ctx, s.getJWKSPath()) + if _, err = client.Logical().DeleteWithContext(ctx, s.getJWKSPrivateKeyPath()); err != nil { return errors.E(op, err) } return nil @@ -203,7 +203,7 @@ func (s *VaultStore) GetJWKS(ctx context.Context) (jwk.Set, error) { return nil, errors.E(op, err) } - secret, err := client.Logical().Read(s.getJWKSPath()) + secret, err := client.Logical().ReadWithContext(ctx, s.getJWKSPath()) if err != nil { return nil, errors.E(op, err) } @@ -239,7 +239,7 @@ func (s *VaultStore) GetJWKSPrivateKey(ctx context.Context) ([]byte, error) { return nil, errors.E(op, err) } - secret, err := client.Logical().Read(s.getJWKSPrivateKeyPath()) + secret, err := client.Logical().ReadWithContext(ctx, s.getJWKSPrivateKeyPath()) if err != nil { return nil, errors.E(op, err) } @@ -269,7 +269,7 @@ func (s *VaultStore) GetJWKSExpiry(ctx context.Context) (time.Time, error) { return now, errors.E(op, err) } - secret, err := client.Logical().Read(s.getJWKSExpiryPath()) + secret, err := client.Logical().ReadWithContext(ctx, s.getJWKSExpiryPath()) if err != nil { return now, errors.E(op, err) } @@ -310,7 +310,8 @@ func (s *VaultStore) PutJWKS(ctx context.Context, jwks jwk.Set) error { return errors.E(op, err) } - _, err = client.Logical().WriteBytes( + _, err = client.Logical().WriteBytesWithContext( + ctx, // We persist in a similar folder to the controller credentials, but sub-route // to .well-known for further extensions and mental clarity within our vault. s.getJWKSPath(), @@ -332,7 +333,8 @@ func (s *VaultStore) PutJWKSPrivateKey(ctx context.Context, pem []byte) error { return errors.E(op, err) } - if _, err := client.Logical().Write( + if _, err := client.Logical().WriteWithContext( + ctx, // We persist in a similar folder to the controller credentials, but sub-route // to .well-known for further extensions and mental clarity within our vault. s.getJWKSPrivateKeyPath(), @@ -352,7 +354,8 @@ func (s *VaultStore) PutJWKSExpiry(ctx context.Context, expiry time.Time) error return errors.E(op, err) } - if _, err := client.Logical().Write( + if _, err := client.Logical().WriteWithContext( + ctx, s.getJWKSExpiryPath(), map[string]interface{}{ "jwks-expiry": expiry, @@ -385,6 +388,85 @@ func (s *VaultStore) getJWKSExpiryPath() string { return path.Join(s.getWellKnownPath(), "jwks-expiry") } +// CleanupOAuthSecrets removes all secrets associated with OAuth. +func (s *VaultStore) CleanupOAuthSecrets(ctx context.Context) error { + const op = errors.Op("vault.CleanupOAuthSecrets") + + client, err := s.client(ctx) + if err != nil { + return errors.E(op, err) + } + + // Vault does not return errors on deletion requests where + // the secret does not exist. + if _, err := client.Logical().DeleteWithContext(ctx, s.GetOAuthSecretPath()); err != nil { + return errors.E(op, err) + } + return nil +} + +// GetOAuthSecret returns the current HS256 (symmetric encryption) secret used to sign OAuth session tokens. +func (s *VaultStore) GetOAuthSecret(ctx context.Context) ([]byte, error) { + const op = errors.Op("vault.GetOAuthSecret") + + client, err := s.client(ctx) + if err != nil { + return nil, errors.E(op, err) + } + + secret, err := client.Logical().ReadWithContext(ctx, s.GetOAuthSecretPath()) + if err != nil { + return nil, errors.E(op, err) + } + + if secret == nil { + msg := "no OAuth key exists" + zapctx.Debug(ctx, msg) + return nil, errors.E(op, errors.CodeNotFound, msg) + } + + raw := secret.Data["key"] + if secret.Data["key"] == nil { + msg := "nil OAuth key data" + zapctx.Debug(ctx, msg) + return nil, errors.E(op, errors.CodeNotFound, msg) + } + + keyPemB64 := raw.(string) + + keyPem, err := base64.StdEncoding.DecodeString(keyPemB64) + if err != nil { + return nil, errors.E(op, err) + } + + return keyPem, nil +} + +// PutOAuthSecret puts a HS256 (symmetric encryption) secret into the credentials store for signing OAuth session tokens. +func (s *VaultStore) PutOAuthSecret(ctx context.Context, raw []byte) error { + const op = errors.Op("vault.PutOAuthSecret") + + client, err := s.client(ctx) + if err != nil { + return errors.E(op, err) + } + + if _, err := client.Logical().WriteWithContext( + ctx, + s.GetOAuthSecretPath(), + map[string]interface{}{"key": raw}, + ); err != nil { + return errors.E(op, err) + } + return nil +} + +// GetOAuthSecretPath returns a hardcoded suffixed vault path (dependent on +// the initial KVPath) to the OAuth JWK location. +func (s *VaultStore) GetOAuthSecretPath() string { + return path.Join(s.KVPath, "creds", "oauth", "key") +} + // deleteControllerCredentials removes the credentials associated with the controller in // the vault service. func (s *VaultStore) deleteControllerCredentials(ctx context.Context, controllerName string) error { @@ -394,7 +476,7 @@ func (s *VaultStore) deleteControllerCredentials(ctx context.Context, controller if err != nil { return errors.E(op, err) } - _, err = client.Logical().Delete(s.controllerCredentialsPath(controllerName)) + _, err = client.Logical().DeleteWithContext(ctx, s.controllerCredentialsPath(controllerName)) if rerr, ok := err.(*api.ResponseError); ok && rerr.StatusCode == http.StatusNotFound { // Ignore the error if attempting to delete something that isn't there. err = nil @@ -417,7 +499,7 @@ func (s *VaultStore) client(ctx context.Context) (*api.Client, error) { return s.client_, nil } - secret, err := s.Client.Logical().Write(s.AuthPath, s.AuthSecret) + secret, err := s.Client.Logical().WriteWithContext(ctx, s.AuthPath, s.AuthSecret) if err != nil { return nil, errors.E(op, err) } diff --git a/internal/vault/vault_test.go b/internal/vault/vault_test.go index ab2f182e0..5a2eaa3bd 100644 --- a/internal/vault/vault_test.go +++ b/internal/vault/vault_test.go @@ -184,3 +184,44 @@ func TestGetAndPutJWKSPrivateKey(t *testing.T) { c.Assert(err, qt.IsNil) c.Assert(string(keyPem), qt.Contains, "-----BEGIN RSA PRIVATE KEY-----") } + +func TestGetAndPutOAuthSecret(t *testing.T) { + c := qt.New(t) + ctx := context.Background() + store := newStore(c) + + // We didn't use a pre-defined/constant key here because in that case we had + // to make sure there's nothing left from last test runs in Vault. + key := []byte(uuid.NewString()) // A random UUID as key + err := store.PutOAuthSecret(ctx, key) + c.Assert(err, qt.IsNil) + retrievedKey, err := store.GetOAuthSecret(ctx) + c.Assert(err, qt.IsNil) + c.Assert(retrievedKey, qt.DeepEquals, key) +} + +func TestGetOAuthSecretFailsIfDataIsNil(t *testing.T) { + c := qt.New(t) + ctx := context.Background() + store := newStore(c) + + err := store.PutOAuthSecret(ctx, nil) + c.Assert(err, qt.IsNil) + + retrieved, err := store.GetOAuthSecret(ctx) + c.Assert(err, qt.ErrorMatches, "nil OAuth key data") + c.Assert(retrieved, qt.IsNil) +} + +func TestGetOAuthSecretFailsIfNotFound(t *testing.T) { + c := qt.New(t) + ctx := context.Background() + store := newStore(c) + + err := store.CleanupOAuthSecrets(ctx) + c.Assert(err, qt.IsNil) + + retrieved, err := store.GetOAuthSecret(ctx) + c.Assert(err, qt.ErrorMatches, "no OAuth key exists") + c.Assert(retrieved, qt.IsNil) +} diff --git a/service.go b/service.go index a73b5505e..72936970c 100644 --- a/service.go +++ b/service.go @@ -4,6 +4,7 @@ package jimm import ( "context" + "crypto/rand" "database/sql" "net/http" "net/url" @@ -492,6 +493,34 @@ func newVaultStore(ctx context.Context, p Params) (jimmcreds.CredentialStore, er }, nil } +// CheckOrGenerateOAuthKey checks if the OAuth secret key already exists on the +// credential store, and if not, generates a random 4096-bit secret key and +func (s *Service) CheckOrGenerateOAuthKey(ctx context.Context) error { + const op = errors.Op("CheckOrGenerateOAuthKey") + store := s.jimm.CredentialStore + if store == nil { + zapctx.Info(ctx, "skipped generating initial OAuth secret key due to nil credential store") + return nil + } + + if secret, err := store.GetOAuthSecret(ctx); err == nil && secret != nil && len(secret) > 0 { + zapctx.Info(ctx, "detected existing OAuth secret key") + return nil + } + + secret := make([]byte, 4096) + if _, err := rand.Read(secret); err != nil { + zapctx.Error(ctx, "failed to generate OAuth secret key", zap.Error(err)) + return errors.E(op, err, "failed to generate OAuth secret key") + } + + if err := store.PutOAuthSecret(ctx, secret); err != nil { + zapctx.Error(ctx, "failed to store generated OAuth secret key", zap.Error(err)) + return errors.E(op, err, "failed to store generated OAuth secret key") + } + return nil +} + func newOpenFGAClient(ctx context.Context, p OpenFGAParams) (*openfga.OFGAClient, error) { const op = errors.Op("newOpenFGAClient") cofgaClient, err := cofga.NewClient(ctx, cofga.OpenFGAParams{ diff --git a/service_test.go b/service_test.go index 5d28b5801..17d61bef2 100644 --- a/service_test.go +++ b/service_test.go @@ -84,6 +84,7 @@ func TestServiceStartsWithoutSecretStore(t *testing.T) { func TestAuthenticator(t *testing.T) { c := qt.New(t) + ctx := context.Background() _, _, cofgaParams, err := jimmtest.SetupTestOFGAClient(c.Name()) c.Assert(err, qt.IsNil) @@ -102,7 +103,10 @@ func TestAuthenticator(t *testing.T) { }, DashboardFinalRedirectURL: "", } - svc, err := jimm.NewService(context.Background(), p) + svc, err := jimm.NewService(ctx, p) + c.Assert(err, qt.IsNil) + + err = svc.JIMM().GetCredentialStore().PutOAuthSecret(ctx, []byte(jimmtest.JWTTestSecret)) c.Assert(err, qt.IsNil) srv := httptest.NewTLSServer(svc) @@ -151,6 +155,7 @@ const testVaultEnv = `clouds: func TestVault(t *testing.T) { c := qt.New(t) + ctx := context.Background() ofgaClient, _, cofgaParams, err := jimmtest.SetupTestOFGAClient(c.Name()) c.Assert(err, qt.IsNil) @@ -173,7 +178,10 @@ func TestVault(t *testing.T) { } vaultClient, _, creds, _ := jimmtest.VaultClient(c, ".") - svc, err := jimm.NewService(context.Background(), p) + svc, err := jimm.NewService(ctx, p) + c.Assert(err, qt.IsNil) + + err = svc.JIMM().GetCredentialStore().PutOAuthSecret(ctx, []byte(jimmtest.JWTTestSecret)) c.Assert(err, qt.IsNil) env := jimmtest.ParseEnvironment(c, testVaultEnv) @@ -246,15 +254,17 @@ func TestPostgresSecretStore(t *testing.T) { func TestOpenFGA(t *testing.T) { c := qt.New(t) + ctx := context.Background() _, _, cofgaParams, err := jimmtest.SetupTestOFGAClient(c.Name()) c.Assert(err, qt.IsNil) p := jimm.Params{ - ControllerUUID: "6acf4fd8-32d6-49ea-b4eb-dcb9d1590c11", - DSN: jimmtest.CreateEmptyDatabase(c), - OpenFGAParams: cofgaParamsToJIMMOpenFGAParams(*cofgaParams), - ControllerAdmins: []string{"alice", "eve"}, + ControllerUUID: "6acf4fd8-32d6-49ea-b4eb-dcb9d1590c11", + DSN: jimmtest.CreateEmptyDatabase(c), + OpenFGAParams: cofgaParamsToJIMMOpenFGAParams(*cofgaParams), + ControllerAdmins: []string{"alice", "eve"}, + InsecureSecretStorage: true, OAuthAuthenticatorParams: jimm.OAuthAuthenticatorParams{ IssuerURL: "http://localhost:8082/realms/jimm", ClientID: "jimm-device", @@ -263,7 +273,11 @@ func TestOpenFGA(t *testing.T) { }, DashboardFinalRedirectURL: "", } - svc, err := jimm.NewService(context.Background(), p) + + svc, err := jimm.NewService(ctx, p) + c.Assert(err, qt.IsNil) + + err = svc.JIMM().GetCredentialStore().PutOAuthSecret(ctx, []byte(jimmtest.JWTTestSecret)) c.Assert(err, qt.IsNil) srv := httptest.NewTLSServer(svc)