Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Initial implementation of IDP interface and Keycloak implementation #3155

Merged
merged 18 commits into from
Apr 26, 2024
Merged

Initial implementation of IDP interface and Keycloak implementation #3155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
a29852f
Add skeleton of IDP interface for resolving friendly names
evankanderson Feb 21, 2024
dbb5831
Address feedback on interface
evankanderson Feb 29, 2024
c177b79
Initial implementation of keycloak IDP lookups. Does not yet create n…
evankanderson Apr 24, 2024
55b456a
Add flag controls for IDP resolution
evankanderson Apr 24, 2024
f67c638
Merge remote-tracking branch 'upstream/main' into 3151-merge-idp
evankanderson Apr 24, 2024
39f333b
Add tests for internal/auth
evankanderson Apr 25, 2024
c327340
Re-add lookup by numeric ID used by install -> user mapping
evankanderson Apr 25, 2024
ccaf484
Fix exclusion regex. Oops!
evankanderson Apr 25, 2024
cb265de
Add tests for handlers_authz.go
evankanderson Apr 25, 2024
b1d1faf
Merge remote-tracking branch 'upstream/main' into 3151-merge-idp
evankanderson Apr 25, 2024
542733c
Fix lints
evankanderson Apr 25, 2024
39a2f4c
Mark generated files as generated
evankanderson Apr 25, 2024
b08806d
Rename ProviderFromFlags per Ozz feedback
evankanderson Apr 25, 2024
2014e7a
Introduce fake for openfeature.IClient
evankanderson Apr 25, 2024
d4467af
No longer need openfeature.Shutdown
evankanderson Apr 25, 2024
09f9e39
Stop defensive loop copying not needed in Go 1.22
evankanderson Apr 25, 2024
0969fb1
Fix token refresh and clean up flags interface
evankanderson Apr 25, 2024
69f94dd
Merge remote-tracking branch 'upstream/main' into 3151-merge-idp
evankanderson Apr 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitattributes
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,5 @@
/internal/db/querier.go linguist-generated=true
/internal/db/*.sql.go linguist-generated=true
/internal/authz/model/minder.generated.json linguist-generated=true
/internal/auth/keycloak/client/client.gen.go linguist-generated=true
/internal/auth/keycloak/client/keycloak-api.yaml linguist-generated=true
2 changes: 1 addition & 1 deletion .github/workflows/license-check.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ jobs:
- name: Check license headers
run: |
set -e
addlicense -l apache -c 'Stacklok, Inc' -v -ignore "pkg/generated/*" -ignore "**/database/query/**" -ignore "internal/db/*" -ignore "docs/docs/**" -ignore "docs/src/**" -ignore "docs/static/**" -ignore "pkg/controlplane/policy_types/**" -ignore "docs/build/**" -ignore "examples/**" *
addlicense -l apache -c 'Stacklok, Inc' -v -ignore "pkg/generated/*" -ignore "**/database/query/**" -ignore "internal/db/*" -ignore "docs/docs/**" -ignore "docs/src/**" -ignore "docs/static/**" -ignore "pkg/controlplane/policy_types/**" -ignore "docs/build/**" -ignore "examples/**" -ignore "internal/auth/keycloak/client/keycloak-api.yaml" *
git diff --exit-code
6 changes: 5 additions & 1 deletion .mk/gen.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ clean-gen: ## clean generated files
rm -rf $(shell find pkg/api -iname "*.go") & rm -rf $(shell find pkg/api -iname "*.swagger.json") & rm -rf pkg/api/protodocs

.PHONY: gen
gen: buf sqlc mock ## run code generation targets (buf, sqlc, mock)
gen: buf sqlc mock oapi ## run code generation targets (buf, sqlc, mock)
$(MAKE) authz-model

.PHONY: buf
Expand All @@ -29,6 +29,10 @@ buf: ## generate protobuf files
sqlc: ## generate sqlc files
sqlc generate

.PHONY: oapi
oapi: ## generate openapi files
go generate ./internal/auth/keycloak/client

## Note: Do not add `mockgen` commands here unless you are mocking an interface
## from a third party library, the `pkg` directory, or autogenerated code
## For all other uses, use the following go generate in the .go file:
Expand Down
2 changes: 1 addition & 1 deletion .mk/test.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
# exclude auto-generated DB code as well as mocks
# in future, we may want to parse these from a file instead of hardcoding them
# in the Makefile
COVERAGE_EXCLUSIONS="internal/db\|/mock/"
COVERAGE_EXCLUSIONS="internal/db\|/mock/\|internal/auth/keycloak/client"
COVERAGE_PACKAGES=./internal/...

.PHONY: clean
Expand Down
11 changes: 11 additions & 0 deletions cmd/server/app/serve.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/spf13/viper"

"github.com/stacklok/minder/internal/auth"
"github.com/stacklok/minder/internal/auth/keycloak"
"github.com/stacklok/minder/internal/authz"
"github.com/stacklok/minder/internal/config"
serverconfig "github.com/stacklok/minder/internal/config/server"
Expand Down Expand Up @@ -106,6 +107,15 @@ var serveCmd = &cobra.Command{
return fmt.Errorf("unable to prepare authz client for run: %w", err)
}

kc, err := keycloak.NewKeyCloak("", cfg.Identity.Server)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the significance of the name argument to NewKeyCloak, and why is it left blank here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the definition of IdentityProvider:

	// String returns the name of the identity provider.  This should be a short
	// one-word string suitable for presentation.  As a special case, a _single_
	// provider may use the empty string as its name to act as a default / fallback
	// provider.

We're using KeyCloak as the default provider. At some point in the future, I'd like to be able to add various machine identities as providers which can be granted access to the system without needing to have accounts in our primary IDP. (For example, an AWS role, a GCP service account, or a GitHub Action -- these might be stored as user:gha/repo:octo-org/octo-repo:environment:prod as the user key in OpenFGA, for example.)

I'm not ready to commit to implementing one of these machine identity providers yet, but I wanted to put the foundations in place while I was doing this refactoring.

Currently, we only have the one default identity provider, which is Keycloak. This PR doesn't switch over all our Keycloak identity usage yet, only puts a backwards-compatible shim over the Keycloak + OpenFGA combination.

if err != nil {
return fmt.Errorf("unable to create keycloak identity provider: %w", err)
}
idClient, err := auth.NewIdentityClient(kc)
if err != nil {
return fmt.Errorf("unable to create identity client: %w", err)
}

providerMetrics := provtelemetry.NewProviderMetrics()
restClientCache := ratecache.NewRestClientCache(ctx)
defer restClientCache.Close()
Expand All @@ -114,6 +124,7 @@ var serveCmd = &cobra.Command{
controlplane.WithServerMetrics(cpmetrics.NewMetrics()),
controlplane.WithProviderMetrics(providerMetrics),
controlplane.WithAuthzClient(authzc),
controlplane.WithIdentityClient(idClient),
controlplane.WithRestClientCache(restClientCache),
}

Expand Down
2 changes: 2 additions & 0 deletions docker-compose.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ services:
- "9090:9090"
volumes:
- ./server-config.yaml:/app/server-config.yaml:z
- ./flags-config.yaml:/app/flags-config.yaml:z
# If you don't want to store your GitHub client ID and secret in the main
# config file, point to them here:
# - ./.github_client_id:/secrets/github_client_id:z
Expand All @@ -62,6 +63,7 @@ services:
- MINDER_AUTH_TOKEN_KEY=/app/.ssh/token_key_passphrase
- MINDER_UNSTABLE_TRUSTY_ENDPOINT=https://api.trustypkg.dev
- MINDER_PROVIDER_GITHUB_APP_PRIVATE_KEY=/app/.secrets/github-app.pem
- MINDER_FLAGS_GO_FEATURE_FILE_PATH=/app/flags-config.yaml
- MINDER_LOG_GITHUB_REQUESTS=1
networks:
- app_net
Expand Down
12 changes: 10 additions & 2 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ require (
github.com/lib/pq v1.10.9
github.com/mitchellh/mapstructure v1.5.0
github.com/motemen/go-loghttp v0.0.0-20231107055348-29ae44b293f4
github.com/oapi-codegen/runtime v1.1.1
github.com/olekukonko/tablewriter v0.0.5
github.com/open-feature/go-sdk v1.11.0
github.com/open-feature/go-sdk-contrib/providers/go-feature-flag v0.1.35
github.com/open-policy-agent/opa v0.63.0
github.com/openfga/go-sdk v0.3.5
github.com/openfga/openfga v1.5.3
Expand All @@ -52,6 +55,7 @@ require (
github.com/stacklok/frizbee v0.0.15
github.com/stretchr/testify v1.9.0
github.com/styrainc/regal v0.21.0
github.com/thomaspoignant/go-feature-flag v1.26.0
github.com/xeipuuv/gojsonschema v1.2.0
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.51.0
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.51.0
Expand Down Expand Up @@ -80,12 +84,15 @@ require (
require (
filippo.io/edwards25519 v1.1.0 // indirect
github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161 // indirect
github.com/BurntSushi/toml v1.3.2 // indirect
github.com/Masterminds/squirrel v1.5.4 // indirect
github.com/MicahParks/keyfunc v1.9.0 // indirect
github.com/Microsoft/hcsshim v0.11.4 // indirect
github.com/antlr4-go/antlr/v4 v4.13.0 // indirect
github.com/apapsch/go-jsonmerge/v2 v2.0.0 // indirect
github.com/atotto/clipboard v0.1.4 // indirect
github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect
github.com/bluele/gcache v0.0.2 // indirect
github.com/charmbracelet/bubbles v0.17.1 // indirect
github.com/charmbracelet/bubbletea v0.25.0 // indirect
github.com/containerd/console v1.0.4-0.20230313162750-1ae8d489ac81 // indirect
Expand Down Expand Up @@ -134,6 +141,7 @@ require (
github.com/muesli/cancelreader v0.2.2 // indirect
github.com/muhlemmer/gu v0.3.1 // indirect
github.com/natefinch/wrap v0.2.0 // indirect
github.com/nikunjy/rules v1.5.0 // indirect
github.com/oklog/ulid/v2 v2.1.0 // indirect
github.com/openfga/api/proto v0.0.0-20240318145204-66b9e5cb403c // indirect
github.com/openfga/language/pkg/go v0.0.0-20240409225820-a53ea2892d6d // indirect
Expand All @@ -160,7 +168,7 @@ require (
github.com/tklauser/numcpus v0.6.1 // indirect
github.com/xi2/xz v0.0.0-20171230120015-48954b6210f8 // indirect
github.com/yusufpapurcu/wmi v1.2.3 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.24.0 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.25.0 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.24.0 // indirect
go.opentelemetry.io/proto/otlp v1.1.0 // indirect
golang.org/x/time v0.5.0 // indirect
Expand Down Expand Up @@ -282,7 +290,7 @@ require (
github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect
github.com/yashtewari/glob-intersection v0.2.0 // indirect
github.com/zitadel/oidc/v2 v2.12.0
go.mongodb.org/mongo-driver v1.14.0 // indirect
go.mongodb.org/mongo-driver v1.15.0 // indirect
go.opentelemetry.io/otel/metric v1.26.0
go.uber.org/atomic v1.11.0 // indirect
go.uber.org/multierr v1.11.0 // indirect
Expand Down
Loading
Loading