Various OAuth fixes

- fixes the proxy session token verification - changes the format of the client credentials client id bringing it in line with juju user tags - various local testing fixes
canonical · Apr 4, 2024 · 8d5e945 · 8d5e945
1 parent eeac45c
commit 8d5e945
Show file tree

Hide file tree

Showing 16 changed files with 107 additions and 84 deletions.
diff --git a/cmd/jaas/cmd/addserviceaccount_test.go b/cmd/jaas/cmd/addserviceaccount_test.go
@@ -24,7 +24,7 @@ type addServiceAccountSuite struct {
 var _ = gc.Suite(&addServiceAccountSuite{})
 
 func (s *addServiceAccountSuite) TestAddServiceAccount(c *gc.C) {
-	clientID := "abda51b2-d735-4794-a8bd-49c506baa4af"
+	clientID := "abda51b2-d735-4794-a8bd-49c506baa4af@canonical.com"
 	// alice is superuser
 	bClient := jimmtest.NewUserSessionLogin(c, "alice")
 	_, err := cmdtesting.RunCommand(c, cmd.NewAddServiceAccountCommandForTesting(s.ClientStore(), bClient), clientID)

diff --git a/cmd/jaas/cmd/grant_test.go b/cmd/jaas/cmd/grant_test.go
@@ -27,7 +27,7 @@ var _ = gc.Suite(&grantSuite{})
 func (s *grantSuite) TestGrant(c *gc.C) {
 	ctx := context.Background()
 
-	clientID := "abda51b2-d735-4794-a8bd-49c506baa4af"
+	clientID := "abda51b2-d735-4794-a8bd-49c506baa4af@canonical.com"
 
 	// alice is superuser
 	bClient := jimmtest.NewUserSessionLogin(c, "alice")

diff --git a/cmd/jaas/cmd/listserviceaccountcredentials_test.go b/cmd/jaas/cmd/listserviceaccountcredentials_test.go
@@ -34,7 +34,7 @@ func (s *listServiceAccountCredentialsSuite) TestListServiceAccountCredentials(c
 	})
 	c.Assert(err, gc.IsNil)
 	// Create Alice Identity and Service Account Identity.
-	clientID := "abda51b2-d735-4794-a8bd-49c506baa4af"
+	clientID := "abda51b2-d735-4794-a8bd-49c506baa4af@canonical.com"
 	// alice is superuser
 	ctx := context.Background()
 	user := dbmodel.Identity{Name: "alice@canonical.com"}

diff --git a/cmd/jaas/cmd/updatecredentials_test.go b/cmd/jaas/cmd/updatecredentials_test.go
@@ -28,7 +28,7 @@ var _ = gc.Suite(&updateCredentialsSuite{})
 func (s *updateCredentialsSuite) TestUpdateCredentialsWithNewCredentials(c *gc.C) {
 	ctx := context.Background()
 
-	clientID := "abda51b2-d735-4794-a8bd-49c506baa4af"
+	clientID := "abda51b2-d735-4794-a8bd-49c506baa4af@canonical.com"
 
 	// alice is superuser
 	bClient := jimmtest.NewUserSessionLogin(c, "alice")
@@ -69,7 +69,7 @@ func (s *updateCredentialsSuite) TestUpdateCredentialsWithNewCredentials(c *gc.C
 	cmdContext, err := cmdtesting.RunCommand(c, cmd.NewUpdateCredentialsCommandForTesting(clientStore, bClient), clientID, "test-cloud", "test-credentials")
 	c.Assert(err, gc.IsNil)
 	c.Assert(cmdtesting.Stdout(cmdContext), gc.Equals, `results:
-- credentialtag: cloudcred-test-cloud_abda51b2-d735-4794-a8bd-49c506baa4af_test-credentials
+- credentialtag: cloudcred-test-cloud_abda51b2-d735-4794-a8bd-49c506baa4af@canonical.com_test-credentials
   error: null
   models: []
 `)
@@ -89,7 +89,7 @@ func (s *updateCredentialsSuite) TestUpdateCredentialsWithNewCredentials(c *gc.C
 func (s *updateCredentialsSuite) TestUpdateCredentialsWithExistingCredentials(c *gc.C) {
 	ctx := context.Background()
 
-	clientID := "abda51b2-d735-4794-a8bd-49c506baa4af"
+	clientID := "abda51b2-d735-4794-a8bd-49c506baa4af@canonical.com"
 
 	// alice is superuser
 	bClient := jimmtest.NewUserSessionLogin(c, "alice")
@@ -139,7 +139,7 @@ func (s *updateCredentialsSuite) TestUpdateCredentialsWithExistingCredentials(c
 	cmdContext, err := cmdtesting.RunCommand(c, cmd.NewUpdateCredentialsCommandForTesting(clientStore, bClient), clientID, "test-cloud", "test-credentials")
 	c.Assert(err, gc.IsNil)
 	c.Assert(cmdtesting.Stdout(cmdContext), gc.Equals, `results:
-- credentialtag: cloudcred-test-cloud_abda51b2-d735-4794-a8bd-49c506baa4af_test-credentials
+- credentialtag: cloudcred-test-cloud_abda51b2-d735-4794-a8bd-49c506baa4af@canonical.com_test-credentials
   error: null
   models: []
 `)
@@ -159,7 +159,7 @@ func (s *updateCredentialsSuite) TestUpdateCredentialsWithExistingCredentials(c
 func (s *updateCredentialsSuite) TestCloudNotInLocalStore(c *gc.C) {
 	bClient := jimmtest.NewUserSessionLogin(c, "alice")
 	_, err := cmdtesting.RunCommand(c, cmd.NewUpdateCredentialsCommandForTesting(s.ClientStore(), bClient),
-		"00000000-0000-0000-0000-000000000000",
+		"00000000-0000-0000-0000-000000000000@canonical.com",
 		"non-existing-cloud",
 		"foo",
 	)
@@ -178,7 +178,7 @@ func (s *updateCredentialsSuite) TestCredentialNotInLocalStore(c *gc.C) {
 	c.Assert(err, gc.IsNil)
 
 	_, err = cmdtesting.RunCommand(c, cmd.NewUpdateCredentialsCommandForTesting(clientStore, bClient),
-		"00000000-0000-0000-0000-000000000000",
+		"00000000-0000-0000-0000-000000000000@canonical.com",
 		"some-cloud",
 		"non-existing-credential-name",
 	)
@@ -196,15 +196,15 @@ func (s *updateCredentialsSuite) TestMissingArgs(c *gc.C) {
 		expectedError: "client ID not specified",
 	}, {
 		name:          "missing cloud",
-		args:          []string{"some-client-id"},
+		args:          []string{"some-client-id@canonical.com"},
 		expectedError: "cloud not specified",
 	}, {
 		name:          "missing credential name",
-		args:          []string{"some-client-id", "some-cloud"},
+		args:          []string{"some-client-id@canonical.com", "some-cloud"},
 		expectedError: "credential name not specified",
 	}, {
 		name:          "too many args",
-		args:          []string{"some-client-id", "some-cloud", "some-credential-name", "extra-arg"},
+		args:          []string{"some-client-id@canonical.com", "some-cloud", "some-credential-name", "extra-arg"},
 		expectedError: "too many args",
 	}}
 

diff --git a/docker-compose.yaml b/docker-compose.yaml
@@ -21,7 +21,12 @@ services:
       interval: 10s
       timeout: 5s
       retries: 3
-
+    labels:
+      traefik.enable: true
+      traefik.http.routers.traefik.rule: Host(`127.0.0.1`)
+      traefik.http.routers.traefik.entrypoints: websecure
+      traefik.http.routers.traefik.tls: true
+
   jimm:
     image: cosmtrek/air:latest
     profiles: ["dev"]
@@ -46,7 +51,7 @@ services:
       JIMM_DSN: "postgresql://jimm:jimm@db/jimm"
       # Not needed for local test (yet).
       # BAKERY_AGENT_FILE: ""
-      JIMM_ADMINS: "jimm@candid.localhost"
+      JIMM_ADMINS: "jimm-test@canonical.com"
       # Note: You can comment out the Vault ENV vars below and instead use INSECURE_SECRET_STORAGE to place secrets in Postgres.
       VAULT_ADDR: "http://vault:8200"
       VAULT_PATH: "/jimm-kv/"

diff --git a/go.mod b/go.mod
@@ -50,14 +50,15 @@ require (
 	github.com/dustinkirkland/golang-petname v0.0.0-20231002161417-6a283f1aaaf2
 	github.com/go-chi/chi/v5 v5.0.8
 	github.com/go-chi/render v1.0.2
+	github.com/gorilla/sessions v1.2.1
 	github.com/hashicorp/golang-lru/v2 v2.0.7
 	github.com/itchyny/gojq v0.12.12
 	github.com/juju/charm/v12 v12.0.0
 	github.com/juju/names/v5 v5.0.0
 	github.com/lestrrat-go/iter v1.0.2
-	github.com/lestrrat-go/jwx/v2 v2.0.19
+	github.com/lestrrat-go/jwx/v2 v2.0.21
 	github.com/oklog/ulid/v2 v2.1.0
-	github.com/stretchr/testify v1.8.4
+	github.com/stretchr/testify v1.9.0
 	golang.org/x/oauth2 v0.15.0
 	gopkg.in/errgo.v1 v1.0.1
 	gopkg.in/httprequest.v1 v1.2.1
@@ -121,7 +122,7 @@ require (
 	github.com/gdamore/encoding v1.0.0 // indirect
 	github.com/gdamore/tcell/v2 v2.5.1 // indirect
 	github.com/go-goose/goose/v5 v5.0.0-20230421180421-abaee9096e3a // indirect
-	github.com/go-jose/go-jose/v3 v3.0.1 // indirect
+	github.com/go-jose/go-jose/v3 v3.0.3 // indirect
 	github.com/go-logr/logr v1.4.1 // indirect
 	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/go-macaroon-bakery/macaroonpb v1.0.0 // indirect
@@ -143,7 +144,6 @@ require (
 	github.com/googleapis/gax-go/v2 v2.12.0 // 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
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
 	github.com/hashicorp/go-multierror v1.1.1 // indirect
@@ -212,7 +212,7 @@ require (
 	github.com/kylelemons/godebug v1.1.0 // indirect
 	github.com/lestrrat-go/blackmagic v1.0.2 // indirect
 	github.com/lestrrat-go/httpcc v1.0.1 // indirect
-	github.com/lestrrat-go/httprc v1.0.4 // indirect
+	github.com/lestrrat-go/httprc v1.0.5 // indirect
 	github.com/lestrrat-go/option v1.0.1 // indirect
 	github.com/lestrrat/go-jspointer v0.0.0-20160229021354-f4881e611bdb // indirect
 	github.com/lestrrat/go-jsref v0.0.0-20160601013240-e452c7b5801d // indirect
@@ -251,7 +251,6 @@ require (
 	github.com/muhlemmer/gu v0.3.1 // indirect
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
 	github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f // indirect
-	github.com/oauth2-proxy/mockoidc v0.0.0-20240214162133-caebfff84d25 // indirect
 	github.com/opencontainers/go-digest v1.0.0 // indirect
 	github.com/oracle/oci-go-sdk/v65 v65.55.0 // indirect
 	github.com/packethost/packngo v0.28.1 // indirect
@@ -296,10 +295,10 @@ require (
 	go.uber.org/atomic v1.11.0 // indirect
 	go.uber.org/mock v0.4.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
-	golang.org/x/crypto v0.19.0 // indirect
+	golang.org/x/crypto v0.21.0 // indirect
 	golang.org/x/exp v0.0.0-20231127185646-65229373498e // indirect
-	golang.org/x/sys v0.17.0 // indirect
-	golang.org/x/term v0.17.0 // indirect
+	golang.org/x/sys v0.18.0 // indirect
+	golang.org/x/term v0.18.0 // indirect
 	golang.org/x/text v0.14.0 // indirect
 	golang.org/x/time v0.5.0 // indirect
 	google.golang.org/api v0.154.0 // indirect