From 4919938ce5ff22a956ee523cfe76f8d0ebd210ca Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Thu, 18 Nov 2021 10:24:38 +0800 Subject: [PATCH 1/9] chore: add pull-request CI automation Allow PRs to generate images pushed to GHCR. --- .github/workflows/build.yaml | 39 +++++++++++++++++++++++++++++++++++ .github/workflows/lint.yaml | 22 ++++++++++++++++++++ .github/workflows/test.yaml | 14 +++++++++++++ .goreleaser.yml | 9 ++++++++ internal/serviceapi/server.go | 9 +++++++- 5 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/build.yaml create mode 100644 .github/workflows/lint.yaml create mode 100644 .github/workflows/test.yaml create mode 100644 .goreleaser.yml diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml new file mode 100644 index 00000000..196af334 --- /dev/null +++ b/.github/workflows/build.yaml @@ -0,0 +1,39 @@ +name: Build +on: pull_request +jobs: + build: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: Install Go + uses: actions/setup-go@v2 + with: + go-version: "^1.17" + - name: Set up environment + run: echo "GOVERSION=$(go version)" >> $GITHUB_ENV + - name: Run GoReleaser + uses: goreleaser/goreleaser-action@v2 + with: + version: latest + args: build --snapshot --rm-dist + - name: Login to GHCR + uses: docker/login-action@v1 + with: + registry: ghcr.io + username: ${{ github.repository_owner }} + password: ${{ secrets.GITHUB_TOKEN }} + - name: Docker metadata + id: service_api_meta + uses: docker/metadata-action@v3 + with: + images: uselagoon/lagoon-ssh-portal/service-api + - name: Build and push service-api container image + id: docker_build + uses: docker/build-push-action@v2 + with: + push: true + tags: ghcr.io/${{ steps.service_api_meta.outputs.tags }} + labels: ${{ steps.service_api_meta.outputs.labels }} + file: deploy/service-api/Dockerfile + context: dist/lagoon-ssh-portal_linux_amd64 diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml new file mode 100644 index 00000000..a792f98f --- /dev/null +++ b/.github/workflows/lint.yaml @@ -0,0 +1,22 @@ +name: Lint +on: pull_request +jobs: + golangci-lint: + name: lint + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: golangci-lint + uses: golangci/golangci-lint-action@v2.5.2 + with: + version: latest + commitlint: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v2 + with: + fetch-depth: 0 + - name: Lint commit messages + uses: wagoid/commitlint-github-action@v4.1.9 diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml new file mode 100644 index 00000000..f2fc8eb5 --- /dev/null +++ b/.github/workflows/test.yaml @@ -0,0 +1,14 @@ +name: Test +on: pull_request +jobs: + go-test: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: Install Go + uses: actions/setup-go@v2 + with: + go-version: "^1.17" + - name: Run Tests + run: go test -v ./... diff --git a/.goreleaser.yml b/.goreleaser.yml new file mode 100644 index 00000000..ce24adbc --- /dev/null +++ b/.goreleaser.yml @@ -0,0 +1,9 @@ +builds: +- dir: cmd/service-api + binary: service-api + ldflags: + - > + -s -w -X main.date={{.Date}} -X "main.goVersion={{.Env.GOVERSION}}" + -X main.shortCommit={{.ShortCommit}} -X main.version={{.Version}} + env: + - CGO_ENABLED=0 diff --git a/internal/serviceapi/server.go b/internal/serviceapi/server.go index c7585b8e..b04aade4 100644 --- a/internal/serviceapi/server.go +++ b/internal/serviceapi/server.go @@ -63,7 +63,14 @@ func sshportal(log *zap.Logger, c *nats.EncodedConn, l LagoonDBService, // calculate permission ok := permission.UserCanSSHToEnvironment(env, realmRoles, userGroups, groupProjectIDs) - c.Publish(reply, ok) + if err = c.Publish(reply, ok); err != nil { + log.Error("couldn't publish reply", + zap.Any("query", query), + zap.Bool("reply value", ok), + zap.String("user UUID", user.UUID.String()), + zap.Error(err)) + return + } } } From a20afc628992f9e1d8a63c0d1fc8233acf33bb27 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Thu, 18 Nov 2021 14:12:56 +0800 Subject: [PATCH 2/9] feat: add release automation --- .github/workflows/release.yaml | 77 +++++++++++++++++++++++++++ .github/workflows/tag-to-release.yaml | 44 +++++++++++++++ .gitignore | 1 + 3 files changed, 122 insertions(+) create mode 100644 .github/workflows/release.yaml create mode 100644 .github/workflows/tag-to-release.yaml create mode 100644 .gitignore diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml new file mode 100644 index 00000000..1c2af026 --- /dev/null +++ b/.github/workflows/release.yaml @@ -0,0 +1,77 @@ +name: Release +on: + push: + branches: + - main +jobs: + tag: + runs-on: ubuntu-latest + outputs: + new-tag: ${{ steps.bump-tag.outputs.new }} + steps: + - name: Checkout + uses: actions/checkout@v2 + with: + fetch-depth: 0 + - name: Configure Git + run: | + git config --global user.name "$GITHUB_ACTOR" + git config --global user.email "$GITHUB_ACTOR@users.noreply.github.com" + - name: Install Go + uses: actions/setup-go@v2 + with: + go-version: "^1.17" + - name: Install ccv + run: > + curl -sSL https://github.com/smlx/ccv/releases/download/v0.3.2/ccv_0.3.2_linux_amd64.tar.gz + | sudo tar -xz -C /usr/local/bin ccv + - name: Bump tag if necessary + id: bump-tag + run: | + if [ -z $(git tag -l $(ccv)) ]; then + git tag $(ccv) + git push --tags + echo "::set-output name=new::true" + fi + release: + needs: tag + if: needs.tag.outputs.new-tag == 'true' + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v2 + with: + fetch-depth: 0 + - name: Install Go + uses: actions/setup-go@v2 + with: + go-version: "^1.17" + - name: Set up environment + run: echo "GOVERSION=$(go version)" >> $GITHUB_ENV + - name: Run GoReleaser + uses: goreleaser/goreleaser-action@v2 + with: + version: latest + args: release --rm-dist + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Login to GHCR + uses: docker/login-action@v1 + with: + registry: ghcr.io + username: ${{ github.repository_owner }} + password: ${{ secrets.GITHUB_TOKEN }} + - name: Docker metadata + id: service_api_meta + uses: docker/metadata-action@v3 + with: + images: uselagoon/lagoon-ssh-portal/service-api + - name: Build and push service-api container image + id: docker_build + uses: docker/build-push-action@v2 + with: + push: true + tags: ghcr.io/${{ steps.service_api_meta.outputs.tags }} + labels: ${{ steps.service_api_meta.outputs.labels }} + file: deploy/service-api/Dockerfile + context: dist/lagoon-ssh-portal_linux_amd64 diff --git a/.github/workflows/tag-to-release.yaml b/.github/workflows/tag-to-release.yaml new file mode 100644 index 00000000..0427a785 --- /dev/null +++ b/.github/workflows/tag-to-release.yaml @@ -0,0 +1,44 @@ +name: Tag to Release +on: + push: + tags: + - v* +jobs: + release: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: Install Go + uses: actions/setup-go@v2 + with: + go-version: "^1.17" + - name: Set up environment + run: echo "GOVERSION=$(go version)" >> $GITHUB_ENV + - name: Run GoReleaser + uses: goreleaser/goreleaser-action@v2 + with: + version: latest + args: release --rm-dist + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Login to GHCR + uses: docker/login-action@v1 + with: + registry: ghcr.io + username: ${{ github.repository_owner }} + password: ${{ secrets.GITHUB_TOKEN }} + - name: Docker metadata + id: service_api_meta + uses: docker/metadata-action@v3 + with: + images: uselagoon/lagoon-ssh-portal/service-api + - name: Build and push service-api container image + id: docker_build + uses: docker/build-push-action@v2 + with: + push: true + tags: ghcr.io/${{ steps.service_api_meta.outputs.tags }} + labels: ${{ steps.service_api_meta.outputs.labels }} + file: deploy/service-api/Dockerfile + context: dist/lagoon-ssh-portal_linux_amd64 diff --git a/.gitignore b/.gitignore new file mode 100644 index 00000000..9b1c8b13 --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +/dist From 0c88495dfae71001098ca630238529b6a241d205 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Thu, 18 Nov 2021 20:47:45 +0800 Subject: [PATCH 3/9] feat: refactor database connection specification Split connection parameters instead of requiring a DSN. This makes for much easier chart integration and usage. Also fix the default DB user. --- cmd/service-api/main.go | 2 +- cmd/service-api/serve.go | 26 +++++++++++++++++--------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/cmd/service-api/main.go b/cmd/service-api/main.go index 9f3b222a..48a964e9 100644 --- a/cmd/service-api/main.go +++ b/cmd/service-api/main.go @@ -14,7 +14,7 @@ var ( // CLI represents the command-line interface. type CLI struct { - Debug bool `kong:"help='Enable debug logging'"` + Debug bool `kong:"env='DEBUG',help='Enable debug logging'"` Serve ServeCmd `kong:"cmd,default=1,help='(default) Serve service-api requests'"` Version VersionCmd `kong:"cmd,help='Print version information'"` } diff --git a/cmd/service-api/serve.go b/cmd/service-api/serve.go index 5e420658..4a600e7c 100644 --- a/cmd/service-api/serve.go +++ b/cmd/service-api/serve.go @@ -6,22 +6,24 @@ import ( "os" "os/signal" + "github.com/go-sql-driver/mysql" "github.com/uselagoon/ssh-portal/internal/keycloak" "github.com/uselagoon/ssh-portal/internal/lagoondb" "github.com/uselagoon/ssh-portal/internal/serviceapi" "go.uber.org/zap" - - _ "github.com/go-sql-driver/mysql" ) // ServeCmd represents the serve command. type ServeCmd struct { - NATSServer string `kong:"required,help='NATS server URL (nats://... or tls://...)'"` - APIDB string `kong:"required,help='Lagoon API Database DSN (https://github.com/go-sql-driver/mysql#dsn-data-source-name)'"` - JWTSecret string `kong:"required,help='JWT Symmetric Secret'"` - KeycloakBaseURL string `kong:"required,help='Keycloak Base URL'"` - KeycloakClientID string `kong:"default='service-api',help='Keycloak OAuth2 Client ID'"` - KeycloakClientSecret string `kong:"required,help='Keycloak OAuth2 Client Secret'"` + APIDBAddress string `kong:"required,env='API_DB_ADDRESS',help='Lagoon API DB Address (host[:port])'"` + APIDBDatabase string `kong:"default='infrastructure',env='API_DB_DATABASE',help='Lagoon API DB Database Name'"` + APIDBPassword string `kong:"required,env='API_DB_PASSWORD',help='Lagoon API DB Password'"` + APIDBUsername string `kong:"default='api',env='API_DB_USERNAME',help='Lagoon API DB Username'"` + JWTSecret string `kong:"required,env='JWTSECRET',help='JWT Symmetric Secret'"` + KeycloakBaseURL string `kong:"required,env='KEYCLOAK_BASE_URL',help='Keycloak Base URL'"` + KeycloakClientID string `kong:"default='service-api',env='KEYCLOAK_SERVICE_API_CLIENT_ID',help='Keycloak OAuth2 Client ID'"` + KeycloakClientSecret string `kong:"required,env='KEYCLOAK_SERVICE_API_CLIENT_SECRET',help='Keycloak OAuth2 Client Secret'"` + NATSServer string `kong:"required,env='NATS_URL',help='NATS server URL (nats://... or tls://...)'"` } // getContext starts a goroutine to handle ^C gracefully, and returns a context @@ -48,7 +50,13 @@ func (cmd *ServeCmd) Run(log *zap.Logger) error { ctx, cancel := getContext() defer cancel() // init lagoon DB client - l, err := lagoondb.NewClient(ctx, cmd.APIDB) + dbConf := mysql.NewConfig() + dbConf.Addr = cmd.APIDBAddress + dbConf.DBName = cmd.APIDBDatabase + dbConf.Net = "tcp" + dbConf.Passwd = cmd.APIDBPassword + dbConf.User = cmd.APIDBUsername + l, err := lagoondb.NewClient(ctx, dbConf.FormatDSN()) if err != nil { return fmt.Errorf("couldn't init lagoon DBClient: %v", err) } From 939e4e9bb7ebf5c43d1d6b07fdd54a31f134d501 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Thu, 18 Nov 2021 21:28:57 +0800 Subject: [PATCH 4/9] fix: query syntax and typo --- internal/lagoondb/client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/lagoondb/client.go b/internal/lagoondb/client.go index 5ecea5a3..03e60097 100644 --- a/internal/lagoondb/client.go +++ b/internal/lagoondb/client.go @@ -61,9 +61,9 @@ func (c *Client) EnvironmentByNamespaceName(name string) (*Environment, error) { environment.openshift_project_name AS namespace_name, project.id AS project_id, project.name AS project_name, - environment.environment_type AS type, + environment.environment_type AS type FROM environment JOIN project ON environment.project = project.id - WHERE project.openshift_project_name = ?`, name) + WHERE environment.openshift_project_name = ?`, name) } // UserBySSHFingerprint returns the User associated with the given From 6bc0afbe6070c46fea3d819f07e18f6f89d61976 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Thu, 18 Nov 2021 21:33:33 +0800 Subject: [PATCH 5/9] feat: better malformed query handling --- internal/serviceapi/server.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/serviceapi/server.go b/internal/serviceapi/server.go index b04aade4..63cf8760 100644 --- a/internal/serviceapi/server.go +++ b/internal/serviceapi/server.go @@ -31,6 +31,10 @@ type KeycloakService interface { func sshportal(log *zap.Logger, c *nats.EncodedConn, l LagoonDBService, k KeycloakService) nats.Handler { return func(subj, reply string, query *lagoondb.SSHAccessQuery) { + if query.SSHFingerprint == "" || query.NamespaceName == "" { + log.Warn("malformed sshportal query", zap.Any("query", query)) + return + } // get the environment env, err := l.EnvironmentByNamespaceName(query.NamespaceName) if err != nil { From a2c7c180a6556e879f1823b73cc3772c12158ba3 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Thu, 18 Nov 2021 21:56:07 +0800 Subject: [PATCH 6/9] feat: gracefully more error cases * handle unknown namespace name * missing user SSH key fingerprint indicates no access --- internal/lagoondb/client.go | 23 +++++++++++++++++++++-- internal/serviceapi/server.go | 21 ++++++++++++++++++--- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/internal/lagoondb/client.go b/internal/lagoondb/client.go index 03e60097..6623bb8b 100644 --- a/internal/lagoondb/client.go +++ b/internal/lagoondb/client.go @@ -2,6 +2,8 @@ package lagoondb import ( "context" + "database/sql" + "errors" "time" "github.com/google/uuid" @@ -35,6 +37,9 @@ type User struct { UUID *uuid.UUID `db:"uuid"` } +// ErrNoResult is returned by client methods if there is no result. +var ErrNoResult = errors.New("no rows in result set") + // NewClient returns a new Lagoon DB Client. func NewClient(ctx context.Context, dsn string) (*Client, error) { db, err := sqlx.ConnectContext(ctx, "mysql", dsn) @@ -55,7 +60,7 @@ func NewClient(ctx context.Context, dsn string) (*Client, error) { // Namespace name (on Openshift this is the project name). func (c *Client) EnvironmentByNamespaceName(name string) (*Environment, error) { env := Environment{} - return &env, c.db.GetContext(c.ctx, &env, ` + err := c.db.GetContext(c.ctx, &env, ` SELECT environment.name AS name, environment.openshift_project_name AS namespace_name, @@ -64,14 +69,28 @@ func (c *Client) EnvironmentByNamespaceName(name string) (*Environment, error) { environment.environment_type AS type FROM environment JOIN project ON environment.project = project.id WHERE environment.openshift_project_name = ?`, name) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return nil, ErrNoResult + } + return nil, err + } + return &env, nil } // UserBySSHFingerprint returns the User associated with the given // SSH fingerprint. func (c *Client) UserBySSHFingerprint(fingerprint string) (*User, error) { user := User{} - return &user, c.db.GetContext(c.ctx, &user, ` + err := c.db.GetContext(c.ctx, &user, ` SELECT user_ssh_key.usid AS uuid FROM user_ssh_key JOIN ssh_key ON user_ssh_key.skid = ssh_key.id WHERE ssh_key.key_fingerprint = ?`, fingerprint) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return nil, ErrNoResult + } + return nil, err + } + return &user, nil } diff --git a/internal/serviceapi/server.go b/internal/serviceapi/server.go index 63cf8760..5095dba6 100644 --- a/internal/serviceapi/server.go +++ b/internal/serviceapi/server.go @@ -2,6 +2,7 @@ package serviceapi import ( "context" + "errors" "fmt" "sync" @@ -31,6 +32,10 @@ type KeycloakService interface { func sshportal(log *zap.Logger, c *nats.EncodedConn, l LagoonDBService, k KeycloakService) nats.Handler { return func(subj, reply string, query *lagoondb.SSHAccessQuery) { + var ok bool + var realmRoles, userGroups []string + var groupProjectIDs map[string][]int + // sanity check the query if query.SSHFingerprint == "" || query.NamespaceName == "" { log.Warn("malformed sshportal query", zap.Any("query", query)) return @@ -38,6 +43,11 @@ func sshportal(log *zap.Logger, c *nats.EncodedConn, l LagoonDBService, // get the environment env, err := l.EnvironmentByNamespaceName(query.NamespaceName) if err != nil { + if errors.Is(err, lagoondb.ErrNoResult) { + log.Warn("unknown namespace name", + zap.Any("query", query), zap.Error(err)) + return + } log.Error("couldn't query environment", zap.Any("query", query), zap.Error(err)) return @@ -45,12 +55,17 @@ func sshportal(log *zap.Logger, c *nats.EncodedConn, l LagoonDBService, // get the user user, err := l.UserBySSHFingerprint(query.SSHFingerprint) if err != nil { + if errors.Is(err, lagoondb.ErrNoResult) { + log.Debug("unknown SSH Fingerprint", + zap.Any("query", query), zap.Error(err)) + goto reply + } log.Error("couldn't query user", zap.Any("query", query), zap.Error(err)) return } // get the user roles and groups - realmRoles, userGroups, groupProjectIDs, err := + realmRoles, userGroups, groupProjectIDs, err = k.UserRolesAndGroups(user.UUID) if err != nil { log.Error("couldn't query user roles and groups", @@ -65,15 +80,15 @@ func sshportal(log *zap.Logger, c *nats.EncodedConn, l LagoonDBService, zap.Any("group project IDs", groupProjectIDs), zap.String("user UUID", user.UUID.String())) // calculate permission - ok := permission.UserCanSSHToEnvironment(env, realmRoles, userGroups, + ok = permission.UserCanSSHToEnvironment(env, realmRoles, userGroups, groupProjectIDs) + reply: if err = c.Publish(reply, ok); err != nil { log.Error("couldn't publish reply", zap.Any("query", query), zap.Bool("reply value", ok), zap.String("user UUID", user.UUID.String()), zap.Error(err)) - return } } } From 781c7baea73b234c9367148155ad46a6788fb87e Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Thu, 18 Nov 2021 23:26:36 +0800 Subject: [PATCH 7/9] fix: extract the JWT signing public key from the keycloak This is different to the JWTSECRET used elsewhere, so we don't even need to know that value. --- Makefile | 4 +-- cmd/service-api/serve.go | 3 +- go.mod | 1 + internal/keycloak/client.go | 65 ++++++++++++++++++++++++++++++++++--- 4 files changed, 64 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index e5e71863..73368287 100644 --- a/Makefile +++ b/Makefile @@ -5,9 +5,9 @@ test: mod-tidy generate go test -v ./... .PHONY: mod-tidy -mod-tidy: generate +mod-tidy: go mod tidy .PHONY: generate -generate: +generate: mod-tidy go generate ./... diff --git a/cmd/service-api/serve.go b/cmd/service-api/serve.go index 4a600e7c..cb699643 100644 --- a/cmd/service-api/serve.go +++ b/cmd/service-api/serve.go @@ -19,7 +19,6 @@ type ServeCmd struct { APIDBDatabase string `kong:"default='infrastructure',env='API_DB_DATABASE',help='Lagoon API DB Database Name'"` APIDBPassword string `kong:"required,env='API_DB_PASSWORD',help='Lagoon API DB Password'"` APIDBUsername string `kong:"default='api',env='API_DB_USERNAME',help='Lagoon API DB Username'"` - JWTSecret string `kong:"required,env='JWTSECRET',help='JWT Symmetric Secret'"` KeycloakBaseURL string `kong:"required,env='KEYCLOAK_BASE_URL',help='Keycloak Base URL'"` KeycloakClientID string `kong:"default='service-api',env='KEYCLOAK_SERVICE_API_CLIENT_ID',help='Keycloak OAuth2 Client ID'"` KeycloakClientSecret string `kong:"required,env='KEYCLOAK_SERVICE_API_CLIENT_SECRET',help='Keycloak OAuth2 Client Secret'"` @@ -62,7 +61,7 @@ func (cmd *ServeCmd) Run(log *zap.Logger) error { } // init keycloak client k, err := keycloak.NewClient(ctx, log, cmd.KeycloakBaseURL, - cmd.KeycloakClientID, cmd.KeycloakClientSecret, cmd.JWTSecret) + cmd.KeycloakClientID, cmd.KeycloakClientSecret) if err != nil { return fmt.Errorf("couldn't init keycloak Client: %v", err) } diff --git a/go.mod b/go.mod index 71bc1c5f..4eea3869 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.17 require ( github.com/alecthomas/kong v0.2.18 + github.com/davecgh/go-spew v1.1.1 github.com/go-sql-driver/mysql v1.6.0 github.com/google/uuid v1.3.0 github.com/jmoiron/sqlx v1.3.4 diff --git a/internal/keycloak/client.go b/internal/keycloak/client.go index cae02562..07a845e1 100644 --- a/internal/keycloak/client.go +++ b/internal/keycloak/client.go @@ -2,12 +2,18 @@ package keycloak import ( "context" + "crypto/rsa" + "crypto/x509" + "encoding/base64" + "encoding/json" "fmt" + "io" "net/http" "net/url" "path" "time" + "github.com/davecgh/go-spew/spew" "github.com/google/uuid" "go.uber.org/zap" "golang.org/x/oauth2" @@ -20,7 +26,7 @@ type Client struct { baseURL *url.URL clientID string clientSecret string - jwtSecret string + jwtPubKey *rsa.PublicKey log *zap.Logger } @@ -38,21 +44,69 @@ type userAttributes struct { // NewClient creates a new keycloak client. func NewClient(ctx context.Context, log *zap.Logger, baseURL, clientID, - clientSecret, jwtSecret string) (*Client, error) { + clientSecret string) (*Client, error) { u, err := url.Parse(baseURL) if err != nil { - return nil, err + return nil, fmt.Errorf("couldn't parse base URL %s: %v", baseURL, err) + } + pubKey, err := publicKey(*u) + if err != nil { + return nil, fmt.Errorf("couldn't get realm public key: %v", err) } return &Client{ ctx: ctx, baseURL: u, clientID: clientID, clientSecret: clientSecret, - jwtSecret: jwtSecret, + jwtPubKey: pubKey, log: log, }, nil } +// publicKey queries the keycloak lagoon realm metadata endpoint and returns +// the RSA public key used to sign JWTs +func publicKey(u url.URL) (*rsa.PublicKey, error) { + // get the metadata JSON + client := &http.Client{Timeout: 10 * time.Second} + u.Path = path.Join(u.Path, `/auth/realms/lagoon`) + res, err := client.Get(u.String()) + if err != nil { + return nil, fmt.Errorf("couldn't get realm metadata: %v", err) + } + defer res.Body.Close() + if res.StatusCode > 299 { + body, _ := io.ReadAll(res.Body) + return nil, fmt.Errorf("bad realm metadata response: %d\n%s", + res.StatusCode, body) + } + // extract public key + jd := json.NewDecoder(res.Body) + metadata := struct { + PubKey string `json:"public_key"` + }{} + if err = jd.Decode(&metadata); err != nil { + return nil, fmt.Errorf("couldn't decode public key from metadata: %v", nil) + } + if len(metadata.PubKey) == 0 { + return nil, fmt.Errorf("couldn't extract public key from metadata") + } + spew.Dump(metadata) + // decode and parse RSA public key + pubKeyBytes, err := base64.StdEncoding.DecodeString(metadata.PubKey) + if err != nil { + return nil, fmt.Errorf("couldn't decode public key value: %v", err) + } + pubKey, err := x509.ParsePKIXPublicKey(pubKeyBytes) + if err != nil { + return nil, fmt.Errorf("couldn't parse PKIX pub key: %v", err) + } + rsaPubKey, ok := pubKey.(*rsa.PublicKey) + if !ok { + return nil, fmt.Errorf("unexpected public key type: %T", pubKey) + } + return rsaPubKey, nil +} + // UserRolesAndGroups queries Keycloak given the user UUID, and returns the // user's realm roles, group memberships, and the project IDs associated with // those groups. @@ -88,8 +142,9 @@ func (c *Client) UserRolesAndGroups(userUUID *uuid.UUID) ([]string, []string, if err != nil { return nil, nil, nil, fmt.Errorf("couldn't parse verified access token: %v", err) } + spew.Dump(tok) var attr userAttributes - if err = tok.Claims(c.jwtSecret, &attr); err != nil { + if err = tok.Claims(c.jwtPubKey, &attr); err != nil { return nil, nil, nil, fmt.Errorf("couldn't extract token claims: %v", err) } From 5ad98f09d8dca419f765f002180dffbb3b57dfe4 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Fri, 19 Nov 2021 12:19:11 +0800 Subject: [PATCH 8/9] fix: don't mutate the token baseURL Instead use a copy of the baseURL so that we can update the path each time. --- go.mod | 1 - internal/keycloak/client.go | 5 +---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/go.mod b/go.mod index 4eea3869..71bc1c5f 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,6 @@ go 1.17 require ( github.com/alecthomas/kong v0.2.18 - github.com/davecgh/go-spew v1.1.1 github.com/go-sql-driver/mysql v1.6.0 github.com/google/uuid v1.3.0 github.com/jmoiron/sqlx v1.3.4 diff --git a/internal/keycloak/client.go b/internal/keycloak/client.go index 07a845e1..e0042d27 100644 --- a/internal/keycloak/client.go +++ b/internal/keycloak/client.go @@ -13,7 +13,6 @@ import ( "path" "time" - "github.com/davecgh/go-spew/spew" "github.com/google/uuid" "go.uber.org/zap" "golang.org/x/oauth2" @@ -90,7 +89,6 @@ func publicKey(u url.URL) (*rsa.PublicKey, error) { if len(metadata.PubKey) == 0 { return nil, fmt.Errorf("couldn't extract public key from metadata") } - spew.Dump(metadata) // decode and parse RSA public key pubKeyBytes, err := base64.StdEncoding.DecodeString(metadata.PubKey) if err != nil { @@ -113,7 +111,7 @@ func publicKey(u url.URL) (*rsa.PublicKey, error) { func (c *Client) UserRolesAndGroups(userUUID *uuid.UUID) ([]string, []string, map[string][]int, error) { // get user token - tokenURL := c.baseURL + tokenURL := *c.baseURL tokenURL.Path = path.Join(tokenURL.Path, `/auth/realms/lagoon/protocol/openid-connect/token`) userConfig := oauth2.Config{ @@ -142,7 +140,6 @@ func (c *Client) UserRolesAndGroups(userUUID *uuid.UUID) ([]string, []string, if err != nil { return nil, nil, nil, fmt.Errorf("couldn't parse verified access token: %v", err) } - spew.Dump(tok) var attr userAttributes if err = tok.Claims(c.jwtPubKey, &attr); err != nil { return nil, nil, nil, From 1b1633ab6161dc654335b8b47b5c0161df4bd9d6 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Fri, 19 Nov 2021 14:20:41 +0800 Subject: [PATCH 9/9] fix: correctly handle extraction of encoded group attributes --- internal/keycloak/client.go | 14 +-- internal/keycloak/userattributes.go | 44 +++++++++ internal/keycloak/userattributes_test.go | 111 +++++++++++++++++++++++ 3 files changed, 156 insertions(+), 13 deletions(-) create mode 100644 internal/keycloak/userattributes.go create mode 100644 internal/keycloak/userattributes_test.go diff --git a/internal/keycloak/client.go b/internal/keycloak/client.go index e0042d27..ada4f24c 100644 --- a/internal/keycloak/client.go +++ b/internal/keycloak/client.go @@ -29,18 +29,6 @@ type Client struct { log *zap.Logger } -// realmAccess is a helper struct for json unmarshalling -type realmAccess struct { - Roles []string `json:"roles"` -} - -// attributes injected into the access token by keycloak -type userAttributes struct { - RealmAccess *realmAccess `json:"realm_access"` - UserGroups []string `json:"groups"` - GroupProjectIDs map[string][]int `json:"group_lagoon_project_ids"` -} - // NewClient creates a new keycloak client. func NewClient(ctx context.Context, log *zap.Logger, baseURL, clientID, clientSecret string) (*Client, error) { @@ -145,5 +133,5 @@ func (c *Client) UserRolesAndGroups(userUUID *uuid.UUID) ([]string, []string, return nil, nil, nil, fmt.Errorf("couldn't extract token claims: %v", err) } - return attr.RealmAccess.Roles, attr.UserGroups, attr.GroupProjectIDs, nil + return attr.RealmRoles, attr.UserGroups, attr.GroupProjectIDs, nil } diff --git a/internal/keycloak/userattributes.go b/internal/keycloak/userattributes.go new file mode 100644 index 00000000..3ccbe349 --- /dev/null +++ b/internal/keycloak/userattributes.go @@ -0,0 +1,44 @@ +package keycloak + +import "encoding/json" + +type regularAttributes struct { + RealmRoles []string `json:"realm_roles"` + UserGroups []string `json:"group_membership"` +} + +// attributes injected into the access token by keycloak +type userAttributes struct { + regularAttributes + GroupProjectIDs map[string][]int +} + +type stringAttributes struct { + GroupPIDs []string `json:"group_lagoon_project_ids"` +} + +func (u *userAttributes) UnmarshalJSON(data []byte) error { + if err := json.Unmarshal(data, &u.regularAttributes); err != nil { + return err + } + // unmarshal the double-encoded group-pid attributes + var s stringAttributes + if err := json.Unmarshal(data, &s); err != nil { + return err + } + var gpaMaps []map[string][]int + for _, gpa := range s.GroupPIDs { + var gpaMap map[string][]int + if err := json.Unmarshal([]byte(gpa), &gpaMap); err != nil { + return err + } + gpaMaps = append(gpaMaps, gpaMap) + } + u.GroupProjectIDs = map[string][]int{} + for _, gpaMap := range gpaMaps { + for k, v := range gpaMap { + u.GroupProjectIDs[k] = v + } + } + return nil +} diff --git a/internal/keycloak/userattributes_test.go b/internal/keycloak/userattributes_test.go new file mode 100644 index 00000000..047ab550 --- /dev/null +++ b/internal/keycloak/userattributes_test.go @@ -0,0 +1,111 @@ +package keycloak + +import ( + "encoding/json" + "reflect" + "testing" +) + +func TestUnmarshalUserAttributes(t *testing.T) { + var testCases = map[string]struct { + input []byte + expect *userAttributes + }{ + "two groups": { + input: []byte(`{ + "group_lagoon_project_ids": [ + "{\"credentialtest-group1\":[1]}", + "{\"ci-group\":[3,4,5,6,7,8,9,10,11,12,17,14,16,20,21,24,19,23,31]}"]}`), + expect: &userAttributes{ + regularAttributes: regularAttributes{ + RealmRoles: nil, + UserGroups: nil, + }, + GroupProjectIDs: map[string][]int{ + "credentialtest-group1": {1}, + "ci-group": {3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 17, 14, 16, 20, 21, 24, + 19, 23, 31}, + }, + }, + }, + "multiple attributes": { + input: []byte(`{ + "jti":"ba279e79-4f38-43ae-83e7-fe461aad59d1", + "exp":1637296288, + "nbf":0, + "iat":1637295988, + "iss":"http://lagoon-core-keycloak:8080/auth/realms/lagoon", + "aud":"account", + "sub":"91435afe-ba81-406b-9308-f80b79fae350", + "typ":"Bearer", + "azp":"service-api", + "auth_time":0, + "session_state":"14ffd91a-86e3-4ce3-93b7-2df3591fcdaf", + "acr":"1", + "realm_access": + {"roles": + ["owner", + "platform-owner", + "offline_access", + "guest", + "reporter", + "developer", + "uma_authorization", + "maintainer"]}, + "resource_access": + {"account": + {"roles":["manage-account", "manage-account-links", "view-profile"]}}, + "scope":"profile email", + "group_membership": + ["/ci-group/ci-group-owner", + "/credentialtest-group1/credentialtest-group1-owner"], + "realm_roles": + ["owner", + "platform-owner", + "offline_access", + "guest", + "reporter", + "developer", + "uma_authorization", + "maintainer"], + "email_verified":true, + "group_lagoon_project_ids": + ["{\"credentialtest-group1\":[1]}", + "{\"ci-group\":[3,4,5,6,7,8,9,10,11,12,17,14,16,20,21,24,19,23,31]}"] + }`), + expect: &userAttributes{ + regularAttributes: regularAttributes{ + RealmRoles: []string{ + "owner", + "platform-owner", + "offline_access", + "guest", + "reporter", + "developer", + "uma_authorization", + "maintainer"}, + UserGroups: []string{ + "/ci-group/ci-group-owner", + "/credentialtest-group1/credentialtest-group1-owner"}, + }, + GroupProjectIDs: map[string][]int{ + "credentialtest-group1": {1}, + "ci-group": {3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 17, 14, 16, 20, 21, 24, + 19, 23, 31}, + }, + }, + }, + } + for name, tc := range testCases { + t.Run(name, func(tt *testing.T) { + var ua *userAttributes + err := json.Unmarshal(tc.input, &ua) + if err != nil { + tt.Fatal(err) + } + if !reflect.DeepEqual(ua, tc.expect) { + tt.Fatalf("got: %v, expected %v", ua, tc.expect) + } + }) + } +}