From 5dbd6a94b5e2487b5b35f713c6fa6a8d8295ad3c Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Thu, 29 Aug 2024 14:11:50 +0800 Subject: [PATCH 1/7] chore: minor formatting fixes --- internal/keycloak/useraccesstoken.go | 18 ++++++++++++------ internal/keycloak/userrolesandgroups.go | 8 +++++--- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/internal/keycloak/useraccesstoken.go b/internal/keycloak/useraccesstoken.go index 5e7bd1d9..1005684b 100644 --- a/internal/keycloak/useraccesstoken.go +++ b/internal/keycloak/useraccesstoken.go @@ -12,8 +12,10 @@ import ( "golang.org/x/oauth2" ) -func (c *Client) getUserToken(ctx context.Context, - userUUID *uuid.UUID) (*oauth2.Token, error) { +func (c *Client) getUserToken( + ctx context.Context, + userUUID uuid.UUID, +) (*oauth2.Token, error) { // set up tracing ctx, span := otel.Tracer(pkgName).Start(ctx, "getUserToken") defer span.End() @@ -49,8 +51,10 @@ func (c *Client) getUserToken(ctx context.Context, // access token response containing both access_token and refresh_token. // Authorized party for these tokens is auth-server. Authorization is done by // the Lagoon API. -func (c *Client) UserAccessTokenResponse(ctx context.Context, - userUUID *uuid.UUID) (string, error) { +func (c *Client) UserAccessTokenResponse( + ctx context.Context, + userUUID uuid.UUID, +) (string, error) { // set up tracing ctx, span := otel.Tracer(pkgName).Start(ctx, "UserAccessToken") defer span.End() @@ -73,8 +77,10 @@ func (c *Client) UserAccessTokenResponse(ctx context.Context, // UserAccessToken queries Keycloak given the user UUID, and returns an access // token. Authorized party for this token is auth-server. Authorization is done // by the Lagoon API. -func (c *Client) UserAccessToken(ctx context.Context, - userUUID *uuid.UUID) (string, error) { +func (c *Client) UserAccessToken( + ctx context.Context, + userUUID uuid.UUID, +) (string, error) { // set up tracing ctx, span := otel.Tracer(pkgName).Start(ctx, "UserAccessToken") defer span.End() diff --git a/internal/keycloak/userrolesandgroups.go b/internal/keycloak/userrolesandgroups.go index d310f97f..903fdb1b 100644 --- a/internal/keycloak/userrolesandgroups.go +++ b/internal/keycloak/userrolesandgroups.go @@ -12,9 +12,11 @@ import ( ) // UserRolesAndGroups queries Keycloak given the user UUID, and returns the -// user's realm roles, and group memberships (by name, including subgroups). -func (c *Client) UserRolesAndGroups(ctx context.Context, - userUUID *uuid.UUID) ([]string, []string, error) { +// user's realm roles, and group memberships (by path). +func (c *Client) UserRolesAndGroups( + ctx context.Context, + userUUID uuid.UUID, +) ([]string, []string, error) { // set up tracing ctx, span := otel.Tracer(pkgName).Start(ctx, "UserRolesAndGroups") defer span.End() From d2331b803f92545377d0f9db3e16a3bc4c269ca1 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Fri, 30 Aug 2024 21:17:47 +0800 Subject: [PATCH 2/7] chore: add invalid zero value to user role enum Slightly improve safety by making the zero value invalid. --- internal/lagoon/userrole.go | 4 ++- internal/lagoon/userrole_enumer.go | 52 ++++++++++++++++-------------- 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/internal/lagoon/userrole.go b/internal/lagoon/userrole.go index a9eeaeff..6973aeaf 100644 --- a/internal/lagoon/userrole.go +++ b/internal/lagoon/userrole.go @@ -6,8 +6,10 @@ package lagoon type UserRole int const ( + // InvalidUserRole is an invalid zero value + InvalidUserRole UserRole = iota // Guest user role. - Guest UserRole = iota + Guest // Reporter user role. Reporter // Developer user role. diff --git a/internal/lagoon/userrole_enumer.go b/internal/lagoon/userrole_enumer.go index 272b4072..fcdf097d 100644 --- a/internal/lagoon/userrole_enumer.go +++ b/internal/lagoon/userrole_enumer.go @@ -8,11 +8,11 @@ import ( "strings" ) -const _UserRoleName = "guestreporterdevelopermaintainerowner" +const _UserRoleName = "invaliduserroleguestreporterdevelopermaintainerowner" -var _UserRoleIndex = [...]uint8{0, 5, 13, 22, 32, 37} +var _UserRoleIndex = [...]uint8{0, 15, 20, 28, 37, 47, 52} -const _UserRoleLowerName = "guestreporterdevelopermaintainerowner" +const _UserRoleLowerName = "invaliduserroleguestreporterdevelopermaintainerowner" func (i UserRole) String() string { if i < 0 || i >= UserRole(len(_UserRoleIndex)-1) { @@ -25,34 +25,38 @@ func (i UserRole) String() string { // Re-run the stringer command to generate them again. func _UserRoleNoOp() { var x [1]struct{} - _ = x[Guest-(0)] - _ = x[Reporter-(1)] - _ = x[Developer-(2)] - _ = x[Maintainer-(3)] - _ = x[Owner-(4)] + _ = x[InvalidUserRole-(0)] + _ = x[Guest-(1)] + _ = x[Reporter-(2)] + _ = x[Developer-(3)] + _ = x[Maintainer-(4)] + _ = x[Owner-(5)] } -var _UserRoleValues = []UserRole{Guest, Reporter, Developer, Maintainer, Owner} +var _UserRoleValues = []UserRole{InvalidUserRole, Guest, Reporter, Developer, Maintainer, Owner} var _UserRoleNameToValueMap = map[string]UserRole{ - _UserRoleName[0:5]: Guest, - _UserRoleLowerName[0:5]: Guest, - _UserRoleName[5:13]: Reporter, - _UserRoleLowerName[5:13]: Reporter, - _UserRoleName[13:22]: Developer, - _UserRoleLowerName[13:22]: Developer, - _UserRoleName[22:32]: Maintainer, - _UserRoleLowerName[22:32]: Maintainer, - _UserRoleName[32:37]: Owner, - _UserRoleLowerName[32:37]: Owner, + _UserRoleName[0:15]: InvalidUserRole, + _UserRoleLowerName[0:15]: InvalidUserRole, + _UserRoleName[15:20]: Guest, + _UserRoleLowerName[15:20]: Guest, + _UserRoleName[20:28]: Reporter, + _UserRoleLowerName[20:28]: Reporter, + _UserRoleName[28:37]: Developer, + _UserRoleLowerName[28:37]: Developer, + _UserRoleName[37:47]: Maintainer, + _UserRoleLowerName[37:47]: Maintainer, + _UserRoleName[47:52]: Owner, + _UserRoleLowerName[47:52]: Owner, } var _UserRoleNames = []string{ - _UserRoleName[0:5], - _UserRoleName[5:13], - _UserRoleName[13:22], - _UserRoleName[22:32], - _UserRoleName[32:37], + _UserRoleName[0:15], + _UserRoleName[15:20], + _UserRoleName[20:28], + _UserRoleName[28:37], + _UserRoleName[37:47], + _UserRoleName[47:52], } // UserRoleString retrieves an enum value from the enum constants string name. From a35ce9392914950e0b1524c81e70cb4eff726771 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Fri, 30 Aug 2024 21:15:28 +0800 Subject: [PATCH 3/7] feat: implement subgroup support in keycloak client Add new methods to the Keycloak client to: * look up user group permissions with support for subgroups. * look up ancestor groups of a given group. In addition, refactor the keycloak client to support paging through results from the Keycloak API. In support of the headline change, also add a "cache map" type which implements an in-memory cache map with individual value lifetimes. --- cmd/keycloak-debug/dumpgroups.go | 2 +- cmd/keycloak-debug/main.go | 2 +- go.mod | 2 +- go.sum | 4 +- internal/cache/{cache.go => any.go} | 24 +-- internal/cache/cache_test.go | 26 +-- internal/cache/map.go | 69 +++++++ internal/keycloak/ancestorgroups.go | 119 +++++++++++ internal/keycloak/client.go | 56 +++++- internal/keycloak/groups.go | 77 ++++--- internal/keycloak/usergroups.go | 300 ++++++++++++++++++++++++++++ 11 files changed, 613 insertions(+), 68 deletions(-) rename internal/cache/{cache.go => any.go} (57%) create mode 100644 internal/cache/map.go create mode 100644 internal/keycloak/ancestorgroups.go create mode 100644 internal/keycloak/usergroups.go diff --git a/cmd/keycloak-debug/dumpgroups.go b/cmd/keycloak-debug/dumpgroups.go index 2308da4e..b62ed3a4 100644 --- a/cmd/keycloak-debug/dumpgroups.go +++ b/cmd/keycloak-debug/dumpgroups.go @@ -33,7 +33,7 @@ func (cmd *DumpGroupsCmd) Run(log *slog.Logger) error { if err != nil { return fmt.Errorf("couldn't init keycloak client: %v", err) } - groupMap, err := k.GroupNameGroupIDMap(ctx) + groupMap, err := k.TopLevelGroupNameGroupIDMap(ctx) if err != nil { return fmt.Errorf("couldn't get keycloak group map: %v", err) } diff --git a/cmd/keycloak-debug/main.go b/cmd/keycloak-debug/main.go index 5c8327f2..fdc7267d 100644 --- a/cmd/keycloak-debug/main.go +++ b/cmd/keycloak-debug/main.go @@ -11,7 +11,7 @@ import ( // CLI represents the command-line interface. type CLI struct { Debug bool `kong:"env='DEBUG',help='Enable debug logging'"` - DumpGroups DumpGroupsCmd `kong:"cmd,default=1,help='(default) Serve ssh-portal-api requests'"` + DumpGroups DumpGroupsCmd `kong:"cmd,default=1,help='(default) Dump top-level Keycloak groups to stdout'"` } func main() { diff --git a/go.mod b/go.mod index 7cda3199..2dc781bd 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/gliderlabs/ssh v0.3.7 github.com/go-sql-driver/mysql v1.8.1 github.com/golang-jwt/jwt/v5 v5.2.1 - github.com/google/uuid v1.6.0 + github.com/google/uuid v1.6.1-0.20240806143717-0e97ed3b5379 github.com/jmoiron/sqlx v1.4.0 github.com/moby/spdystream v0.5.0 github.com/nats-io/nats.go v1.37.0 diff --git a/go.sum b/go.sum index 33fd6229..cf413611 100644 --- a/go.sum +++ b/go.sum @@ -67,8 +67,8 @@ github.com/google/gofuzz v1.2.0 h1:xRy4A+RhZaiKjJ1bPfwQ8sedCA+YS2YcCHW6ec7JMi0= github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/pprof v0.0.0-20240525223248-4bfdf5a9a2af h1:kmjWCqn2qkEml422C2Rrd27c3VGxi6a/6HNq8QmHRKM= github.com/google/pprof v0.0.0-20240525223248-4bfdf5a9a2af/go.mod h1:K1liHPHnj73Fdn/EKuT8nrFqBihUSKXoLYU0BuatOYo= -github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= -github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/google/uuid v1.6.1-0.20240806143717-0e97ed3b5379 h1:9pvPp/2VCtCB2xdSUCaKe1VKCzVHMR+GGgIAVLfQxIs= +github.com/google/uuid v1.6.1-0.20240806143717-0e97ed3b5379/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/gorilla/securecookie v1.1.2 h1:YCIWL56dvtr73r6715mJs5ZvhtnY73hBvEF8kXD8ePA= github.com/gorilla/securecookie v1.1.2/go.mod h1:NfCASbcHqRSY+3a8tlWJwsQap2VX5pwzwo4h3eOamfo= github.com/gorilla/websocket v1.5.0 h1:PPwGk2jz7EePpoHN/+ClbZu8SPxiqlu12wZP/3sWmnc= diff --git a/internal/cache/cache.go b/internal/cache/any.go similarity index 57% rename from internal/cache/cache.go rename to internal/cache/any.go index 13db1887..7aa5bcbe 100644 --- a/internal/cache/cache.go +++ b/internal/cache/any.go @@ -10,28 +10,28 @@ const ( defaultTTL = time.Minute ) -// Cache is a generic, thread-safe, in-memory cache that stores a value with a +// Any is a generic, thread-safe, in-memory cache that stores a value with a // TTL, after which the cache expires. -type Cache[T any] struct { +type Any[T any] struct { data T expiry time.Time ttl time.Duration mu sync.Mutex } -// Option is a functional option argument to NewCache(). -type Option[T any] func(*Cache[T]) +// AnyOption is a functional option argument to NewCache(). +type AnyOption[T any] func(*Any[T]) -// WithTTL sets the the Cache time-to-live to ttl. -func WithTTL[T any](ttl time.Duration) Option[T] { - return func(c *Cache[T]) { +// AnyWithTTL sets the the Cache time-to-live to ttl. +func AnyWithTTL[T any](ttl time.Duration) AnyOption[T] { + return func(c *Any[T]) { c.ttl = ttl } } -// NewCache instantiates a Cache for type T with a default TTL of 1 minute. -func NewCache[T any](options ...Option[T]) *Cache[T] { - c := Cache[T]{ +// NewAny instantiates an Any cache for type T with a default TTL of 1 minute. +func NewAny[T any](options ...AnyOption[T]) *Any[T] { + c := Any[T]{ ttl: defaultTTL, } for _, option := range options { @@ -41,7 +41,7 @@ func NewCache[T any](options ...Option[T]) *Cache[T] { } // Set updates the value in the cache and sets the expiry to now+TTL. -func (c *Cache[T]) Set(value T) { +func (c *Any[T]) Set(value T) { c.mu.Lock() defer c.mu.Unlock() c.data = value @@ -50,7 +50,7 @@ func (c *Cache[T]) Set(value T) { // Get retrieves the value from the cache. If cache has expired, the second // return value will be false. -func (c *Cache[T]) Get() (T, bool) { +func (c *Any[T]) Get() (T, bool) { c.mu.Lock() defer c.mu.Unlock() if time.Now().After(c.expiry) { diff --git a/internal/cache/cache_test.go b/internal/cache/cache_test.go index d614b0d7..7675e2ea 100644 --- a/internal/cache/cache_test.go +++ b/internal/cache/cache_test.go @@ -19,7 +19,7 @@ func TestIntCache(t *testing.T) { } for name, tc := range testCases { t.Run(name, func(tt *testing.T) { - c := cache.NewCache[int](cache.WithTTL[int](time.Second)) + c := cache.NewAny[int](cache.AnyWithTTL[int](time.Second)) c.Set(tc.input) if tc.expired { time.Sleep(2 * time.Second) @@ -36,33 +36,35 @@ func TestIntCache(t *testing.T) { func TestMapCache(t *testing.T) { var testCases = map[string]struct { - input map[string]string - expect map[string]string + key string + value string expired bool }{ "expired": { - input: map[string]string{"foo": "bar"}, + key: "foo", + value: "bar", expired: true, }, "not expired": { - input: map[string]string{"foo": "bar"}, - expect: map[string]string{"foo": "bar"}, + key: "foo", + value: "bar", }, } for name, tc := range testCases { t.Run(name, func(tt *testing.T) { - c := cache.NewCache[map[string]string]( - cache.WithTTL[map[string]string](time.Second), + c := cache.NewMap[string, string]( + cache.MapWithTTL[string, string](time.Second), ) - c.Set(tc.input) + c.Set(tc.key, tc.value) if tc.expired { time.Sleep(2 * time.Second) - _, ok := c.Get() + value, ok := c.Get(tc.key) assert.False(tt, ok, name) + assert.Equal(tt, "", value, name) } else { - value, ok := c.Get() + value, ok := c.Get(tc.key) assert.True(tt, ok, name) - assert.Equal(tt, tc.expect, value, name) + assert.Equal(tt, tc.value, value, name) } }) } diff --git a/internal/cache/map.go b/internal/cache/map.go new file mode 100644 index 00000000..d95abfff --- /dev/null +++ b/internal/cache/map.go @@ -0,0 +1,69 @@ +package cache + +import ( + "sync" + "time" +) + +type mapValue[T any] struct { + data T + expiry time.Time +} + +// Map is a generic, thread-safe, in-memory cache map that stores a key-value +// pairs with a TTL, after which the cache expires. +type Map[K comparable, V any] struct { + data map[K]mapValue[V] + ttl time.Duration + mu sync.Mutex +} + +// MapOption is a functional option argument to NewCache(). +type MapOption[K comparable, V any] func(*Map[K, V]) + +// MapWithTTL sets the the Cache time-to-live to ttl. +func MapWithTTL[K comparable, V any](ttl time.Duration) MapOption[K, V] { + return func(c *Map[K, V]) { + c.ttl = ttl + } +} + +// NewMap instantiates a Map for key type K and value type V with a default TTL +// of 1 minute. +func NewMap[K comparable, V any](options ...MapOption[K, V]) *Map[K, V] { + c := Map[K, V]{ + data: map[K]mapValue[V]{}, + ttl: defaultTTL, + } + for _, option := range options { + option(&c) + } + return &c +} + +// Set updates the value in the cache and sets the expiry to now+TTL. +func (c *Map[K, V]) Set(key K, data V) { + c.mu.Lock() + defer c.mu.Unlock() + c.data[key] = mapValue[V]{ + data: data, + expiry: time.Now().Add(c.ttl), + } +} + +// Get retrieves the value from the cache. If the value doesn't exist in the +// cache, or if the cache has expired, the second return value will be false. +func (c *Map[K, V]) Get(key K) (V, bool) { + c.mu.Lock() + defer c.mu.Unlock() + var zero mapValue[V] + value, ok := c.data[key] + if !ok { + return zero.data, false + } + if time.Now().After(value.expiry) { + delete(c.data, key) + return zero.data, false + } + return value.data, true +} diff --git a/internal/keycloak/ancestorgroups.go b/internal/keycloak/ancestorgroups.go new file mode 100644 index 00000000..ba96dbbd --- /dev/null +++ b/internal/keycloak/ancestorgroups.go @@ -0,0 +1,119 @@ +package keycloak + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "path" + "slices" + + "github.com/google/uuid" +) + +// rawGroup returns the raw JSON group representation of a single keycloak +// group. +func (c *Client) rawGroup( + ctx context.Context, + groupID uuid.UUID, +) ([]byte, error) { + groupURL := *c.baseURL + groupURL.Path = path.Join( + c.baseURL.Path, + "/auth/admin/realms/lagoon/groups", + groupID.String()) + req, err := http.NewRequestWithContext(ctx, "GET", groupURL.String(), nil) + if err != nil { + return nil, fmt.Errorf("couldn't construct group request: %v", err) + } + q := req.URL.Query() + q.Add("briefRepresentation", "false") + req.URL.RawQuery = q.Encode() + res, err := c.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf(`couldn't get groupID "%s": %v`, groupID.String(), err) + } + defer res.Body.Close() + if res.StatusCode > 299 { + body, _ := io.ReadAll(res.Body) + return nil, fmt.Errorf("bad group response: %d\n%s", res.StatusCode, body) + } + return io.ReadAll(res.Body) +} + +// groupByID takes a group (UU)ID and returns the group object it identifies. +func (c *Client) groupByID( + ctx context.Context, + groupID uuid.UUID, +) (*Group, error) { + // prefer to use cached value + group, ok := c.groupIDGroupCache.Get(groupID) + if ok { + return &group, nil + } + // otherwise get data from keycloak + if err := c.limiter.Wait(ctx); err != nil { + return nil, fmt.Errorf("couldn't wait for limiter: %v", err) + } + data, err := c.rawGroup(ctx, groupID) + if err != nil { + return nil, fmt.Errorf("couldn't get group from Keycloak API: %v", err) + } + if err := json.Unmarshal(data, &group); err != nil { + return nil, fmt.Errorf("couldn't unmarshal group: %v", err) + } + if group.ID == nil { + return nil, fmt.Errorf("group with nil ID: %v", group) + } + // update cache + c.groupIDGroupCache.Set(*group.ID, group) + return &group, nil +} + +// ancestorGroupIDs takes a group (UU)ID and returns a slice of all ancestor +// group IDs. +func (c *Client) ancestorGroupIDs( + ctx context.Context, + groupID uuid.UUID, +) ([]uuid.UUID, error) { + var ancestorGIDs []uuid.UUID + group, err := c.groupByID(ctx, groupID) + if err != nil { + return nil, + fmt.Errorf("couldn't get group %s by ID: %v", groupID.String(), err) + } + if group.ParentID != nil { + // this is not a top level group + // get the ancestors of the parent + grandParentGIDs, err := c.ancestorGroupIDs(ctx, *group.ParentID) + if err != nil { + return nil, + fmt.Errorf("couldn't get ancestors of %s: %v", group.ParentID.String(), err) + } + ancestorGIDs = append(ancestorGIDs, *group.ParentID) + ancestorGIDs = append(ancestorGIDs, grandParentGIDs...) + } + return ancestorGIDs, nil +} + +// AncestorGroups takes a slice of group IDs, and returns the same slice +// with any ancestor group IDs appended. +func (c *Client) AncestorGroups( + ctx context.Context, + groupIDs []uuid.UUID, +) ([]uuid.UUID, error) { + var allGIDs []uuid.UUID + allGIDs = append(allGIDs, groupIDs...) + for _, gid := range groupIDs { + ancestorGIDs, err := c.ancestorGroupIDs(ctx, gid) + if err != nil { + return nil, + fmt.Errorf(`couldn't get ancestor group IDs for "%v": %v`, gid, err) + } + allGIDs = append(allGIDs, ancestorGIDs...) + } + // remove duplicates from allGIDs + slices.SortFunc(allGIDs, uuid.Compare) + return slices.Compact(allGIDs), nil +} diff --git a/internal/keycloak/client.go b/internal/keycloak/client.go index 197f4370..10abf769 100644 --- a/internal/keycloak/client.go +++ b/internal/keycloak/client.go @@ -12,13 +12,38 @@ import ( "time" "github.com/MicahParks/keyfunc/v2" + "github.com/google/uuid" "github.com/uselagoon/ssh-portal/internal/cache" oidcClient "github.com/zitadel/oidc/v3/pkg/client" "github.com/zitadel/oidc/v3/pkg/oidc" + "golang.org/x/oauth2/clientcredentials" "golang.org/x/time/rate" ) -const pkgName = "github.com/uselagoon/ssh-portal/internal/keycloak" +const ( + pkgName = "github.com/uselagoon/ssh-portal/internal/keycloak" + + httpTimeout = 8 * time.Second +) + +// newHTTPClient constructs an HTTP client with a reasonable timeout using +// oauth2 client credentials. This client will automatically and transparently +// refresh its OAuth2 token as requried. +func newHTTPClient( + ctx context.Context, + clientID, + clientSecret, + tokenURL string, +) *http.Client { + cc := clientcredentials.Config{ + ClientID: clientID, + ClientSecret: clientSecret, + TokenURL: tokenURL, + } + client := cc.Client(ctx) + client.Timeout = httpTimeout + return client +} // Client is a keycloak client. type Client struct { @@ -29,14 +54,26 @@ type Client struct { log *slog.Logger oidcConfig *oidc.DiscoveryConfiguration limiter *rate.Limiter + httpClient *http.Client + pageSize int - // groupNameGroupIDMap cache - groupCache *cache.Cache[map[string]string] + // top level groupName to groupID map cache + topLevelGroupNameIDCache *cache.Any[map[string]uuid.UUID] + // group ID to Group cache + groupIDGroupCache *cache.Map[uuid.UUID, Group] + // parent group IDs to child groups cache + parentIDChildGroupCache *cache.Map[uuid.UUID, []Group] } // NewClient creates a new keycloak client for the lagoon realm. -func NewClient(ctx context.Context, log *slog.Logger, keycloakURL, clientID, - clientSecret string, rateLimit int) (*Client, error) { +func NewClient( + ctx context.Context, + log *slog.Logger, + keycloakURL, + clientID, + clientSecret string, + rateLimit int, +) (*Client, error) { // discover OIDC config baseURL, err := url.Parse(keycloakURL) if err != nil { @@ -46,7 +83,7 @@ func NewClient(ctx context.Context, log *slog.Logger, keycloakURL, clientID, issuerURL := *baseURL issuerURL.Path = path.Join(issuerURL.Path, "auth/realms/lagoon") oidcConfig, err := oidcClient.Discover(ctx, issuerURL.String(), - &http.Client{Timeout: 8 * time.Second}) + &http.Client{Timeout: httpTimeout}) if err != nil { return nil, fmt.Errorf("couldn't discover OIDC config: %v", err) } @@ -63,6 +100,11 @@ func NewClient(ctx context.Context, log *slog.Logger, keycloakURL, clientID, log: log, oidcConfig: oidcConfig, limiter: rate.NewLimiter(rate.Limit(rateLimit), rateLimit), - groupCache: cache.NewCache[map[string]string](), + httpClient: newHTTPClient(ctx, clientID, clientSecret, oidcConfig.TokenEndpoint), + pageSize: defaultPageSize, + + topLevelGroupNameIDCache: cache.NewAny[map[string]uuid.UUID](), + groupIDGroupCache: cache.NewMap[uuid.UUID, Group](), + parentIDChildGroupCache: cache.NewMap[uuid.UUID, []Group](), }, nil } diff --git a/internal/keycloak/groups.go b/internal/keycloak/groups.go index 90c1b44e..6b063615 100644 --- a/internal/keycloak/groups.go +++ b/internal/keycloak/groups.go @@ -7,28 +7,27 @@ import ( "io" "net/http" "path" + "strconv" - "golang.org/x/oauth2/clientcredentials" + "github.com/google/uuid" ) +// defaultPageSize is the default size of the page requested when scrolling +// through group results from Keycloak. +const defaultPageSize = 1000 + // Group represents a Keycloak Group. It holds the fields required when getting // a list of groups from keycloak. type Group struct { - ID string `json:"id"` - Name string `json:"name"` -} - -func (c *Client) httpClient(ctx context.Context) *http.Client { - cc := clientcredentials.Config{ - ClientID: c.clientID, - ClientSecret: c.clientSecret, - TokenURL: c.oidcConfig.TokenEndpoint, - } - return cc.Client(ctx) + ID *uuid.UUID `json:"id"` + ParentID *uuid.UUID `json:"parentId"` + Name string `json:"name"` + Attributes map[string][]string `json:"attributes"` + RealmRoles []string `json:"realmRoles"` } -// rawGroups returns the raw JSON group representation from the Keycloak API. -func (c *Client) rawGroups(ctx context.Context) ([]byte, error) { +// rawGroups returns the raw JSON group representation of all top-level groups. +func (c *Client) rawGroups(ctx context.Context, first int) ([]byte, error) { groupsURL := *c.baseURL groupsURL.Path = path.Join(c.baseURL.Path, "/auth/admin/realms/lagoon/groups") @@ -38,8 +37,10 @@ func (c *Client) rawGroups(ctx context.Context) ([]byte, error) { } q := req.URL.Query() q.Add("briefRepresentation", "true") + q.Add("first", strconv.Itoa(first)) + q.Add("max", strconv.Itoa(c.pageSize)) req.URL.RawQuery = q.Encode() - res, err := c.httpClient(ctx).Do(req) + res, err := c.httpClient.Do(req) if err != nil { return nil, fmt.Errorf("couldn't get groups: %v", err) } @@ -51,31 +52,43 @@ func (c *Client) rawGroups(ctx context.Context) ([]byte, error) { return io.ReadAll(res.Body) } -// GroupNameGroupIDMap returns a map of Keycloak Group names to Group IDs. -func (c *Client) GroupNameGroupIDMap( +// TopLevelGroupNameGroupIDMap returns a map of top-level Keycloak Group names +// to Group IDs. +func (c *Client) TopLevelGroupNameGroupIDMap( ctx context.Context, -) (map[string]string, error) { +) (map[string]uuid.UUID, error) { // prefer to use cached value - if groupNameGroupIDMap, ok := c.groupCache.Get(); ok { + if groupNameGroupIDMap, ok := c.topLevelGroupNameIDCache.Get(); ok { return groupNameGroupIDMap, nil } // otherwise get data from keycloak - if err := c.limiter.Wait(ctx); err != nil { - return nil, fmt.Errorf("couldn't wait for limiter: %v", err) - } - data, err := c.rawGroups(ctx) - if err != nil { - return nil, fmt.Errorf("couldn't get groups from Keycloak API: %v", err) - } var groups []Group - if err := json.Unmarshal(data, &groups); err != nil { - return nil, fmt.Errorf("couldn't unmarshal Keycloak groups: %v", err) + var first int + for { + var page []Group + if err := c.limiter.Wait(ctx); err != nil { + return nil, fmt.Errorf("couldn't wait for limiter: %v", err) + } + data, err := c.rawGroups(ctx, first) + if err != nil { + return nil, fmt.Errorf("couldn't get groups from Keycloak API: %v", err) + } + if err := json.Unmarshal(data, &page); err != nil { + return nil, fmt.Errorf("couldn't unmarshal Keycloak groups: %v", err) + } + groups = append(groups, page...) + if len(page) < c.pageSize { + break // reached last page + } + first += c.pageSize // scroll to next page } - groupNameGroupIDMap := map[string]string{} + groupNameGroupIDMap := map[string]uuid.UUID{} for _, group := range groups { - groupNameGroupIDMap[group.Name] = group.ID + groupNameGroupIDMap[group.Name] = *group.ID + // update group ID cache + c.groupIDGroupCache.Set(*group.ID, group) } - // update cache - c.groupCache.Set(groupNameGroupIDMap) + // update top level group name cache + c.topLevelGroupNameIDCache.Set(groupNameGroupIDMap) return groupNameGroupIDMap, nil } diff --git a/internal/keycloak/usergroups.go b/internal/keycloak/usergroups.go new file mode 100644 index 00000000..e7be36f8 --- /dev/null +++ b/internal/keycloak/usergroups.go @@ -0,0 +1,300 @@ +package keycloak + +import ( + "context" + "encoding/json" + "fmt" + "io" + "log/slog" + "net/http" + "path" + "slices" + "strconv" + "strings" + + "github.com/google/uuid" + "github.com/uselagoon/ssh-portal/internal/lagoon" +) + +// rawChildGroups returns the raw JSON group representation of child groups of +// the given group ID. +func (c *Client) rawChildGroups( + ctx context.Context, + parentID uuid.UUID, + first int, +) ([]byte, error) { + groupsURL := *c.baseURL + groupsURL.Path = path.Join( + c.baseURL.Path, + "/auth/admin/realms/lagoon/groups", + parentID.String(), + "children") + req, err := http.NewRequestWithContext(ctx, "GET", groupsURL.String(), nil) + if err != nil { + return nil, fmt.Errorf("couldn't construct groups request: %v", err) + } + q := req.URL.Query() + q.Add("briefRepresentation", "false") + q.Add("first", strconv.Itoa(first)) + q.Add("max", strconv.Itoa(c.pageSize)) + req.URL.RawQuery = q.Encode() + res, err := c.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("couldn't get groups: %v", err) + } + defer res.Body.Close() + if res.StatusCode > 299 { + body, _ := io.ReadAll(res.Body) + return nil, fmt.Errorf("bad child groups response for group ID %s: %d\n%s", + parentID.String(), res.StatusCode, body) + } + return io.ReadAll(res.Body) +} + +// topLevelGroupNameFromPath takes a slice of top level group path segments, +// such as ["", "example-company"], and performs some sanity checks to confirm +// it has the correct structure before returning the name of the top level +// group, such as "example-company". +func topLevelGroupNameFromPath(path []string) (string, error) { + switch { + case len(path) != 2: + return "", fmt.Errorf(`wrong number of path segments: %v`, path) + case len(path[0]) != 0: + return "", fmt.Errorf(`first path segment is not empty: %v`, path) + case len(path[1]) == 0: + return "", fmt.Errorf(`second path segment (group name) is empty: %v`, path) + default: + return path[1], nil + } +} + +// topLevelGroupPathID returns the group ID for the given slice of path +// segments of a top level group path. +func (c *Client) topLevelGroupPathID( + ctx context.Context, + path []string, +) (*uuid.UUID, error) { + name, err := topLevelGroupNameFromPath(path) + if err != nil { + return nil, fmt.Errorf("couldn't get top level group name from path: %v", err) + } + groupNameIDMap, err := c.TopLevelGroupNameGroupIDMap(ctx) + if err != nil { + return nil, fmt.Errorf("couldn't get group name group ID map: %v", err) + } + gid, ok := groupNameIDMap[name] + if !ok { + return nil, + fmt.Errorf("couldn't find group name in top level groups: %v", err) + } + return &gid, nil +} + +// groupIDFromParentAndNameCache takes a parent group ID and a child group +// name and returns the child group ID, if it exists in cache, and a boolean +// indicating if the parent group ID was found in the cache. +// +// If the parent ID exists in the cache but no cached child groups match the +// given name, the returned child group ID will be nil. +func (c *Client) groupIDFromParentAndNameCache( + parentID uuid.UUID, + name string, +) (*uuid.UUID, bool) { + childGroups, ok := c.parentIDChildGroupCache.Get(parentID) + if !ok { + return nil, false + } + for _, group := range childGroups { + if group.Name == name { + return group.ID, true + } + } + return nil, true +} + +// groupIDFromParentAndName takes a parent group ID and a group name, and +// returns the group ID of the child group matching the given name. +func (c *Client) groupIDFromParentAndName( + ctx context.Context, + parentID uuid.UUID, + name string, +) (*uuid.UUID, error) { + // prefer to use cached value + gid, ok := c.groupIDFromParentAndNameCache(parentID, name) + if ok { + if gid == nil { + return nil, fmt.Errorf(`couldn't find child group "%v" in cache`, name) + } + return gid, nil + } + // otherwise get data from keycloak + var groups []Group + var first int + for { + var page []Group + if err := c.limiter.Wait(ctx); err != nil { + return nil, fmt.Errorf("couldn't wait for limiter: %v", err) + } + data, err := c.rawChildGroups(ctx, parentID, first) + if err != nil { + return nil, fmt.Errorf("couldn't get child groups from Keycloak: %v", err) + } + if err := json.Unmarshal(data, &page); err != nil { + return nil, fmt.Errorf("couldn't unmarshal child groups: %v", err) + } + groups = append(groups, page...) + if len(page) < c.pageSize { + break // reached last page + } + first += c.pageSize // scroll to next page + } + // update caches + c.parentIDChildGroupCache.Set(parentID, groups) + for _, group := range groups { + c.groupIDGroupCache.Set(*group.ID, group) + } + // return group ID + for _, group := range groups { + if group.Name == name { + return group.ID, nil + } + } + return nil, fmt.Errorf(`couldn't find child group "%v" in keycloak`, name) +} + +// groupPathID returns the ID of the group identified by path. +// path is a slice of path segments (i.e. full path split on /). +func (c *Client) groupPathID( + ctx context.Context, + path []string, +) (*uuid.UUID, error) { + switch { + case len(path) == 2: + gid, err := c.topLevelGroupPathID(ctx, path) + if err != nil { + return nil, + fmt.Errorf(`couldn't get ID for top level group path "%v": %v`, + path[1], err) + } + return gid, nil + case len(path) > 2: + // not a top level group. find the parent ID by slicing off the last + // segment and calling groupPathID recursively. + parentID, err := c.groupPathID(ctx, path[:len(path)-1]) + if err != nil { + return nil, + fmt.Errorf(`couldn't get ID for group path "%v": %v`, path[:1], err) + } + groupName := path[len(path)-1] + gid, err := c.groupIDFromParentAndName(ctx, *parentID, groupName) + if err != nil { + return nil, + fmt.Errorf(`couldn't get ID for group "%s" with parent ID "%v": %v`, + groupName, parentID, err) + } + return gid, nil + default: + return nil, fmt.Errorf(`invalid case for path "%v"`, path) + } +} + +// userGroup2Role takes a user group path, runs some validity checks to confirm +// it is a valid role subgroup, and uses it to construct a lagoon.UserRole. +func (c *Client) userGroup2Role( + ctx context.Context, + path []string, +) (lagoon.UserRole, error) { + parentGroupName, userGroupName := path[len(path)-2], path[len(path)-1] + parentNameSegments := strings.Split(parentGroupName, "-") + nameSegments := strings.Split(userGroupName, "-") + // validate group hierarchy + if !slices.Equal(parentNameSegments, nameSegments[:len(nameSegments)-1]) { + return lagoon.InvalidUserRole, + fmt.Errorf(`invalid parent "%s" and user "%s" group structure`, + parentGroupName, userGroupName) + } + // get group ID from path + gid, err := c.groupPathID(ctx, path) + if err != nil { + return lagoon.InvalidUserRole, + fmt.Errorf("couldn't get group ID from path: %v", err) + } + // get group from ID + group, err := c.groupByID(ctx, *gid) + if err != nil { + return lagoon.InvalidUserRole, + fmt.Errorf("couldn't get group %s by ID: %v", gid.String(), err) + } + // validate type attribute + if group.Attributes == nil || + len(group.Attributes["type"]) != 1 || + group.Attributes["type"][0] != "role-subgroup" { + return lagoon.InvalidUserRole, + fmt.Errorf("group %s invalid type for role subgroup: %v", + gid.String(), group.Attributes) + } + // validate name suffix and realmRole + if len(group.RealmRoles) != 1 { + return lagoon.InvalidUserRole, + fmt.Errorf(`invalid group %s: missing realm role`, gid.String()) + } + roleString := nameSegments[len(nameSegments)-1] + if group.RealmRoles[0] != roleString { + return lagoon.InvalidUserRole, + fmt.Errorf(`invalid group %s: realmRole "%s" doesn't match name suffix "%s"`, + gid.String(), group.RealmRoles[0], roleString) + } + // parse role + role, err := lagoon.UserRoleString(roleString) + if err != nil { + return lagoon.InvalidUserRole, + fmt.Errorf(`couldn't parse "%s" as user role: %v`, roleString, err) + } + return role, nil +} + +// UserGroupIDRole takes a slice of user group paths and converts them to a +// groupID-to-role map. +func (c *Client) UserGroupIDRole( + ctx context.Context, + userGroupPaths []string, +) map[uuid.UUID]lagoon.UserRole { + gidRole := map[uuid.UUID]lagoon.UserRole{} + for _, ugp := range userGroupPaths { + path := strings.Split(ugp, `/`) + if len(path) < 3 { + // Minimum segments in a valid path is three. For example, + // "/project-foo/project-foo-maintainer" splits into + // ["", "project-foo", "project-foo-maintainer"]. + c.log.Warn("invalid user group path", + slog.String("userGroupPath", ugp)) + continue + } + role, err := c.userGroup2Role(ctx, path) + if err != nil { + c.log.Warn("couldn't convert user group path to role", + slog.Any("error", err), + slog.String("userGroup", path[len(path)-1]), + ) + continue + } + // Get the group ID of the parent group. + // Note that this parent group is what Lagoon considers to be the user's + // group, because the lowest level containing group of the user only + // indicates the _role_. Due to this structure, user group paths always end + // in: $(groupName)/$(groupName)-$(role). + gid, err := c.groupPathID(ctx, path[:len(path)-1]) + if err != nil { + c.log.Warn("couldn't get ID of group by path", + slog.Any("error", err), + slog.Any("path", path[:len(path)-1]), + ) + continue + } + // Handle multiple roles in the same group. + if role > gidRole[*gid] { + gidRole[*gid] = role + } + } + return gidRole +} From 3b6b802733abaa490d614513658b2e87e8e41489 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Mon, 7 Oct 2024 17:12:31 +0800 Subject: [PATCH 4/7] chore: add keycloak AncestorGroups tests --- internal/keycloak/ancestorgroups_test.go | 199 ++++++++++++++++++ internal/keycloak/helper_test.go | 8 + .../testdata/ancestorgroup_child0.json | 18 ++ .../testdata/ancestorgroup_child1.json | 18 ++ .../testdata/ancestorgroup_child2.json | 18 ++ .../testdata/ancestorgroup_grandchild0.json | 18 ++ .../testdata/ancestorgroup_grandchild1.json | 18 ++ .../testdata/ancestorgroup_grandchild2.json | 18 ++ .../testdata/ancestorgroup_grandchild3.json | 18 ++ .../testdata/ancestorgroup_parent0.json | 17 ++ .../testdata/ancestorgroup_parent1.json | 17 ++ 11 files changed, 367 insertions(+) create mode 100644 internal/keycloak/ancestorgroups_test.go create mode 100644 internal/keycloak/testdata/ancestorgroup_child0.json create mode 100644 internal/keycloak/testdata/ancestorgroup_child1.json create mode 100644 internal/keycloak/testdata/ancestorgroup_child2.json create mode 100644 internal/keycloak/testdata/ancestorgroup_grandchild0.json create mode 100644 internal/keycloak/testdata/ancestorgroup_grandchild1.json create mode 100644 internal/keycloak/testdata/ancestorgroup_grandchild2.json create mode 100644 internal/keycloak/testdata/ancestorgroup_grandchild3.json create mode 100644 internal/keycloak/testdata/ancestorgroup_parent0.json create mode 100644 internal/keycloak/testdata/ancestorgroup_parent1.json diff --git a/internal/keycloak/ancestorgroups_test.go b/internal/keycloak/ancestorgroups_test.go new file mode 100644 index 00000000..42d0d313 --- /dev/null +++ b/internal/keycloak/ancestorgroups_test.go @@ -0,0 +1,199 @@ +package keycloak_test + +import ( + "bytes" + "context" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "os" + "testing" + + "github.com/alecthomas/assert/v2" + "github.com/google/uuid" + "github.com/uselagoon/ssh-portal/internal/keycloak" +) + +// newTestAncestorGroupsServer sets up a mock keycloak which responds with +// appropriate group JSON data to exercise AncestorGroups. +func newTestAncestorGroupsServer(tt *testing.T) *httptest.Server { + // set up the map of group IDs to responses + var reqRespMap map[string]string = map[string]string{ + // tree 0 + "078faf64-aa58-45cf-afb1-b585583feacf": "testdata/ancestorgroup_grandchild0.json", + "d2d90824-c807-4162-99cf-200e38affbe2": "testdata/ancestorgroup_child0.json", + "3c7dea60-6dec-4f2d-b8ac-f28aa9e206d9": "testdata/ancestorgroup_parent0.json", + // tree 1 + "879d1d38-97d8-449a-affd-8529b8e31feb": "testdata/ancestorgroup_grandchild1.json", + "2e833d9b-39b7-4f25-b37f-cfb8765015ab": "testdata/ancestorgroup_child1.json", + "ee6d02d1-b14b-41dd-95b6-cb8c26b1a321": "testdata/ancestorgroup_parent1.json", + // tree 1 branch + "7f22ce84-c0af-4ff4-afcd-288f0473deb5": "testdata/ancestorgroup_child2.json", + "c7d3b738-91f2-4cf1-aeec-2ab444eb3215": "testdata/ancestorgroup_grandchild2.json", + "139ad442-1d20-4c58-b009-c0afe21bf85b": "testdata/ancestorgroup_grandchild3.json", + } + // load the discovery JSON first, because the mux closure needs to + // reference its buffer + discoveryBuf, err := os.ReadFile("testdata/realm.oidc.discovery.json") + if err != nil { + tt.Fatal(err) + return nil + } + // configure router with the URLs that OIDC discovery and JWKS require + mux := http.NewServeMux() + mux.HandleFunc("/auth/realms/lagoon/.well-known/openid-configuration", + func(w http.ResponseWriter, r *http.Request) { + d := bytes.NewBuffer(discoveryBuf) + _, err = io.Copy(w, d) + if err != nil { + tt.Fatal(err) + } + }) + mux.HandleFunc("/auth/realms/lagoon/protocol/openid-connect/certs", + func(w http.ResponseWriter, r *http.Request) { + f, err := os.Open("testdata/realm.oidc.certs.json") + if err != nil { + tt.Fatal(err) + return + } + _, err = io.Copy(w, f) + if err != nil { + tt.Fatal(err) + } + }) + // configure the group paths + for groupID, file := range reqRespMap { + mux.HandleFunc("/auth/admin/realms/lagoon/groups/"+groupID, + func(w http.ResponseWriter, r *http.Request) { + responseData, err := os.Open(file) + if err != nil { + tt.Fatal(err) + return + } + _, err = io.Copy(w, responseData) + if err != nil { + tt.Fatal(err) + } + }) + } + ts := httptest.NewServer(mux) + // now replace the example URL in the discovery JSON with the actual + // httptest server URL + discoveryBuf = bytes.ReplaceAll(discoveryBuf, + []byte("https://keycloak.example.com"), []byte(ts.URL)) + return ts +} + +func TestAncestorGroups(t *testing.T) { + var testCases = map[string]struct { + groupIDs []uuid.UUID + ancestorGroupIDs []uuid.UUID + }{ + "single grandchild of ancestor group": { + groupIDs: []uuid.UUID{ + uuid.MustParse("078faf64-aa58-45cf-afb1-b585583feacf"), + }, + ancestorGroupIDs: []uuid.UUID{ + uuid.MustParse("078faf64-aa58-45cf-afb1-b585583feacf"), + uuid.MustParse("3c7dea60-6dec-4f2d-b8ac-f28aa9e206d9"), + uuid.MustParse("d2d90824-c807-4162-99cf-200e38affbe2"), + }, + }, + "single child of ancestor group": { + groupIDs: []uuid.UUID{ + uuid.MustParse("d2d90824-c807-4162-99cf-200e38affbe2"), + }, + ancestorGroupIDs: []uuid.UUID{ + uuid.MustParse("3c7dea60-6dec-4f2d-b8ac-f28aa9e206d9"), + uuid.MustParse("d2d90824-c807-4162-99cf-200e38affbe2"), + }, + }, + "two children of separate trees": { + groupIDs: []uuid.UUID{ + uuid.MustParse("d2d90824-c807-4162-99cf-200e38affbe2"), + uuid.MustParse("2e833d9b-39b7-4f25-b37f-cfb8765015ab"), + }, + ancestorGroupIDs: []uuid.UUID{ + uuid.MustParse("2e833d9b-39b7-4f25-b37f-cfb8765015ab"), + uuid.MustParse("3c7dea60-6dec-4f2d-b8ac-f28aa9e206d9"), + uuid.MustParse("d2d90824-c807-4162-99cf-200e38affbe2"), + uuid.MustParse("ee6d02d1-b14b-41dd-95b6-cb8c26b1a321"), + }, + }, + "one grandchild, one child of separate trees": { + groupIDs: []uuid.UUID{ + uuid.MustParse("078faf64-aa58-45cf-afb1-b585583feacf"), + uuid.MustParse("2e833d9b-39b7-4f25-b37f-cfb8765015ab"), + }, + ancestorGroupIDs: []uuid.UUID{ + uuid.MustParse("078faf64-aa58-45cf-afb1-b585583feacf"), + uuid.MustParse("2e833d9b-39b7-4f25-b37f-cfb8765015ab"), + uuid.MustParse("3c7dea60-6dec-4f2d-b8ac-f28aa9e206d9"), + uuid.MustParse("d2d90824-c807-4162-99cf-200e38affbe2"), + uuid.MustParse("ee6d02d1-b14b-41dd-95b6-cb8c26b1a321"), + }, + }, + "one grandchild, one child of the same tree": { + groupIDs: []uuid.UUID{ + uuid.MustParse("078faf64-aa58-45cf-afb1-b585583feacf"), + uuid.MustParse("d2d90824-c807-4162-99cf-200e38affbe2"), + }, + ancestorGroupIDs: []uuid.UUID{ + uuid.MustParse("078faf64-aa58-45cf-afb1-b585583feacf"), + uuid.MustParse("3c7dea60-6dec-4f2d-b8ac-f28aa9e206d9"), + uuid.MustParse("d2d90824-c807-4162-99cf-200e38affbe2"), + }, + }, + "two grandchildren of the same tree": { + groupIDs: []uuid.UUID{ + uuid.MustParse("879d1d38-97d8-449a-affd-8529b8e31feb"), + uuid.MustParse("c7d3b738-91f2-4cf1-aeec-2ab444eb3215"), + }, + ancestorGroupIDs: []uuid.UUID{ + uuid.MustParse("2e833d9b-39b7-4f25-b37f-cfb8765015ab"), + uuid.MustParse("879d1d38-97d8-449a-affd-8529b8e31feb"), + uuid.MustParse("c7d3b738-91f2-4cf1-aeec-2ab444eb3215"), + uuid.MustParse("ee6d02d1-b14b-41dd-95b6-cb8c26b1a321"), + }, + }, + "three grandchildren of the same tree": { + groupIDs: []uuid.UUID{ + uuid.MustParse("879d1d38-97d8-449a-affd-8529b8e31feb"), + uuid.MustParse("c7d3b738-91f2-4cf1-aeec-2ab444eb3215"), + uuid.MustParse("139ad442-1d20-4c58-b009-c0afe21bf85b"), + }, + ancestorGroupIDs: []uuid.UUID{ + uuid.MustParse("139ad442-1d20-4c58-b009-c0afe21bf85b"), + uuid.MustParse("2e833d9b-39b7-4f25-b37f-cfb8765015ab"), + uuid.MustParse("7f22ce84-c0af-4ff4-afcd-288f0473deb5"), + uuid.MustParse("879d1d38-97d8-449a-affd-8529b8e31feb"), + uuid.MustParse("c7d3b738-91f2-4cf1-aeec-2ab444eb3215"), + uuid.MustParse("ee6d02d1-b14b-41dd-95b6-cb8c26b1a321"), + }, + }, + } + for name, tc := range testCases { + t.Run(name, func(tt *testing.T) { + ts := newTestAncestorGroupsServer(tt) + defer ts.Close() + // init keycloak client + k, err := keycloak.NewClient( + context.Background(), + slog.New(slog.NewJSONHandler(os.Stderr, nil)), + ts.URL, + "auth-server", + "", + 10) + if err != nil { + tt.Fatal(err) + } + // override internal HTTP client for testing + k.UseDefaultHTTPClient() + // perform testing + ancestorGroupIDs, err := k.AncestorGroups(context.Background(), tc.groupIDs) + assert.NoError(tt, err, name) + assert.Equal(tt, tc.ancestorGroupIDs, ancestorGroupIDs, name) + }) + } +} diff --git a/internal/keycloak/helper_test.go b/internal/keycloak/helper_test.go index eaa72b53..273a45d6 100644 --- a/internal/keycloak/helper_test.go +++ b/internal/keycloak/helper_test.go @@ -1,6 +1,8 @@ package keycloak import ( + "net/http" + "github.com/golang-jwt/jwt/v5" "golang.org/x/oauth2" ) @@ -16,3 +18,9 @@ func (c *Client) ValidateToken(t *oauth2.Token, sub string, func (l *LagoonClaims) SetClientID(clientID string) { l.clientID = clientID } + +// UseDefaultHTTPClient uses the default http client to avoid token refresh in +// tests. +func (c *Client) UseDefaultHTTPClient() { + c.httpClient = http.DefaultClient +} diff --git a/internal/keycloak/testdata/ancestorgroup_child0.json b/internal/keycloak/testdata/ancestorgroup_child0.json new file mode 100644 index 00000000..8c04eb68 --- /dev/null +++ b/internal/keycloak/testdata/ancestorgroup_child0.json @@ -0,0 +1,18 @@ +{ + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": {}, + "clientRoles": {}, + "id": "d2d90824-c807-4162-99cf-200e38affbe2", + "name": "scott-test-child-group", + "parentId": "3c7dea60-6dec-4f2d-b8ac-f28aa9e206d9", + "path": "/scott-test-ancestor-group/scott-test-child-group", + "realmRoles": [], + "subGroupCount": 1, + "subGroups": [] +} diff --git a/internal/keycloak/testdata/ancestorgroup_child1.json b/internal/keycloak/testdata/ancestorgroup_child1.json new file mode 100644 index 00000000..1c082b64 --- /dev/null +++ b/internal/keycloak/testdata/ancestorgroup_child1.json @@ -0,0 +1,18 @@ +{ + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": {}, + "clientRoles": {}, + "id": "2e833d9b-39b7-4f25-b37f-cfb8765015ab", + "name": "scott-test-child-group2", + "parentId": "ee6d02d1-b14b-41dd-95b6-cb8c26b1a321", + "path": "/scott-test-ancestor-group2/scott-test-child-group2", + "realmRoles": [], + "subGroupCount": 2, + "subGroups": [] +} diff --git a/internal/keycloak/testdata/ancestorgroup_child2.json b/internal/keycloak/testdata/ancestorgroup_child2.json new file mode 100644 index 00000000..d97ef331 --- /dev/null +++ b/internal/keycloak/testdata/ancestorgroup_child2.json @@ -0,0 +1,18 @@ +{ + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": {}, + "clientRoles": {}, + "id": "7f22ce84-c0af-4ff4-afcd-288f0473deb5", + "name": "scott-test-child-group3", + "parentId": "ee6d02d1-b14b-41dd-95b6-cb8c26b1a321", + "path": "/scott-test-ancestor-group2/scott-test-child-group3", + "realmRoles": [], + "subGroupCount": 1, + "subGroups": [] +} diff --git a/internal/keycloak/testdata/ancestorgroup_grandchild0.json b/internal/keycloak/testdata/ancestorgroup_grandchild0.json new file mode 100644 index 00000000..bbcf06ec --- /dev/null +++ b/internal/keycloak/testdata/ancestorgroup_grandchild0.json @@ -0,0 +1,18 @@ +{ + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": {}, + "clientRoles": {}, + "id": "078faf64-aa58-45cf-afb1-b585583feacf", + "name": "scott-test-grandchild-group", + "parentId": "d2d90824-c807-4162-99cf-200e38affbe2", + "path": "/scott-test-ancestor-group/scott-test-child-group/scott-test-grandchild-group", + "realmRoles": [], + "subGroupCount": 0, + "subGroups": [] +} diff --git a/internal/keycloak/testdata/ancestorgroup_grandchild1.json b/internal/keycloak/testdata/ancestorgroup_grandchild1.json new file mode 100644 index 00000000..7d57654e --- /dev/null +++ b/internal/keycloak/testdata/ancestorgroup_grandchild1.json @@ -0,0 +1,18 @@ +{ + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": {}, + "clientRoles": {}, + "id": "879d1d38-97d8-449a-affd-8529b8e31feb", + "name": "scott-test-grandchild-group2", + "parentId": "2e833d9b-39b7-4f25-b37f-cfb8765015ab", + "path": "/scott-test-ancestor-group2/scott-test-child-group2/scott-test-grandchild-group2", + "realmRoles": [], + "subGroupCount": 0, + "subGroups": [] +} diff --git a/internal/keycloak/testdata/ancestorgroup_grandchild2.json b/internal/keycloak/testdata/ancestorgroup_grandchild2.json new file mode 100644 index 00000000..654e0c33 --- /dev/null +++ b/internal/keycloak/testdata/ancestorgroup_grandchild2.json @@ -0,0 +1,18 @@ +{ + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": {}, + "clientRoles": {}, + "id": "c7d3b738-91f2-4cf1-aeec-2ab444eb3215", + "name": "scott-test-grandchild-group2b", + "parentId": "2e833d9b-39b7-4f25-b37f-cfb8765015ab", + "path": "/scott-test-ancestor-group2/scott-test-child-group2/scott-test-grandchild-group2b", + "realmRoles": [], + "subGroupCount": 0, + "subGroups": [] +} diff --git a/internal/keycloak/testdata/ancestorgroup_grandchild3.json b/internal/keycloak/testdata/ancestorgroup_grandchild3.json new file mode 100644 index 00000000..31accccb --- /dev/null +++ b/internal/keycloak/testdata/ancestorgroup_grandchild3.json @@ -0,0 +1,18 @@ +{ + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": {}, + "clientRoles": {}, + "id": "139ad442-1d20-4c58-b009-c0afe21bf85b", + "name": "scott-test-grandchild-group3", + "parentId": "7f22ce84-c0af-4ff4-afcd-288f0473deb5", + "path": "/scott-test-ancestor-group2/scott-test-child-group3/scott-test-grandchild-group3", + "realmRoles": [], + "subGroupCount": 0, + "subGroups": [] +} diff --git a/internal/keycloak/testdata/ancestorgroup_parent0.json b/internal/keycloak/testdata/ancestorgroup_parent0.json new file mode 100644 index 00000000..8877bd74 --- /dev/null +++ b/internal/keycloak/testdata/ancestorgroup_parent0.json @@ -0,0 +1,17 @@ +{ + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": {}, + "clientRoles": {}, + "id": "3c7dea60-6dec-4f2d-b8ac-f28aa9e206d9", + "name": "scott-test-ancestor-group", + "path": "/scott-test-ancestor-group", + "realmRoles": [], + "subGroupCount": 2, + "subGroups": [] +} diff --git a/internal/keycloak/testdata/ancestorgroup_parent1.json b/internal/keycloak/testdata/ancestorgroup_parent1.json new file mode 100644 index 00000000..88a544f0 --- /dev/null +++ b/internal/keycloak/testdata/ancestorgroup_parent1.json @@ -0,0 +1,17 @@ +{ + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": {}, + "clientRoles": {}, + "id": "ee6d02d1-b14b-41dd-95b6-cb8c26b1a321", + "name": "scott-test-ancestor-group2", + "path": "/scott-test-ancestor-group2", + "realmRoles": [], + "subGroupCount": 1, + "subGroups": [] +} From 8e0ffd0c031979f94100e8554379bfc8b41d5147 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Thu, 29 Aug 2024 14:08:51 +0800 Subject: [PATCH 5/7] feat: add functional support for subgroups Refactor permission engine to use the new keycloak client functionality which adds support for subgroups, and refactor session logic to use the new permission engine. --- cmd/ssh-portal-api/serve.go | 18 +- cmd/ssh-token/serve.go | 17 +- internal/lagoon/groupnameprojectids.go | 75 --- internal/lagoon/groupnameprojectids_test.go | 89 --- internal/lagoondb/client.go | 58 +- internal/mock/lagoon_groupnameprojectids.go | 95 ---- internal/rbac/mock/permission.go | 127 +++++ internal/rbac/permission.go | 64 ++- internal/rbac/usercansshtoenvironment.go | 126 +++-- internal/rbac/usercansshtoenvironment_test.go | 516 ++++++++++-------- internal/sshportalapi/server.go | 14 +- internal/sshportalapi/sshportal.go | 42 +- internal/sshtoken/authhandler.go | 4 +- internal/sshtoken/serve.go | 8 +- internal/sshtoken/sessionhandler.go | 80 +-- 15 files changed, 608 insertions(+), 725 deletions(-) delete mode 100644 internal/lagoon/groupnameprojectids.go delete mode 100644 internal/lagoon/groupnameprojectids_test.go delete mode 100644 internal/mock/lagoon_groupnameprojectids.go create mode 100644 internal/rbac/mock/permission.go diff --git a/cmd/ssh-portal-api/serve.go b/cmd/ssh-portal-api/serve.go index aa38f49c..222c9e5c 100644 --- a/cmd/ssh-portal-api/serve.go +++ b/cmd/ssh-portal-api/serve.go @@ -39,13 +39,6 @@ func (cmd *ServeCmd) Run(log *slog.Logger) error { // get main process context, which cancels on SIGTERM ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGTERM) defer stop() - // init RBAC permission engine - var p *rbac.Permission - if cmd.BlockDeveloperSSH { - p = rbac.NewPermission(rbac.BlockDeveloperSSH()) - } else { - p = rbac.NewPermission() - } // init lagoon DB client dbConf := mysql.NewConfig() dbConf.Addr = cmd.APIDBAddress @@ -53,7 +46,7 @@ func (cmd *ServeCmd) Run(log *slog.Logger) error { dbConf.Net = "tcp" dbConf.Passwd = cmd.APIDBPassword dbConf.User = cmd.APIDBUsername - l, err := lagoondb.NewClient(ctx, dbConf.FormatDSN()) + ldb, err := lagoondb.NewClient(ctx, dbConf.FormatDSN()) if err != nil { return fmt.Errorf("couldn't init lagoondb client: %v", err) } @@ -66,6 +59,13 @@ func (cmd *ServeCmd) Run(log *slog.Logger) error { if err != nil { return fmt.Errorf("couldn't init keycloak client: %v", err) } + // init RBAC permission engine + var p *rbac.Permission + if cmd.BlockDeveloperSSH { + p = rbac.NewPermission(k, ldb, rbac.BlockDeveloperSSH()) + } else { + p = rbac.NewPermission(k, ldb) + } // set up goroutine handler eg, ctx := errgroup.WithContext(ctx) // start the metrics server @@ -73,7 +73,7 @@ func (cmd *ServeCmd) Run(log *slog.Logger) error { // start serving SSH token requests eg.Go(func() error { // start serving NATS requests - return sshportalapi.ServeNATS(ctx, stop, log, p, l, k, cmd.NATSURL) + return sshportalapi.ServeNATS(ctx, stop, log, p, ldb, cmd.NATSURL) }) return eg.Wait() } diff --git a/cmd/ssh-token/serve.go b/cmd/ssh-token/serve.go index 2e88225f..a35002ea 100644 --- a/cmd/ssh-token/serve.go +++ b/cmd/ssh-token/serve.go @@ -45,13 +45,6 @@ func (cmd *ServeCmd) Run(log *slog.Logger) error { // get main process context, which cancels on SIGTERM ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGTERM) defer stop() - // init RBAC permission engine - var p *rbac.Permission - if cmd.BlockDeveloperSSH { - p = rbac.NewPermission(rbac.BlockDeveloperSSH()) - } else { - p = rbac.NewPermission() - } // init lagoon DB client dbConf := mysql.NewConfig() dbConf.Addr = cmd.APIDBAddress @@ -81,6 +74,13 @@ func (cmd *ServeCmd) Run(log *slog.Logger) error { if err != nil { return fmt.Errorf("couldn't init keycloak permission client: %v", err) } + // init RBAC permission engine + var p *rbac.Permission + if cmd.BlockDeveloperSSH { + p = rbac.NewPermission(keycloakPermission, ldb, rbac.BlockDeveloperSSH()) + } else { + p = rbac.NewPermission(keycloakPermission, ldb) + } // start listening on TCP port l, err := net.Listen("tcp", fmt.Sprintf(":%d", cmd.SSHServerPort)) if err != nil { @@ -101,8 +101,7 @@ func (cmd *ServeCmd) Run(log *slog.Logger) error { metrics.Serve(ctx, eg, metricsPort) // start serving SSH token requests eg.Go(func() error { - return sshtoken.Serve(ctx, log, l, p, ldb, keycloakToken, keycloakPermission, - hostkeys) + return sshtoken.Serve(ctx, log, l, p, ldb, keycloakToken, hostkeys) }) return eg.Wait() } diff --git a/internal/lagoon/groupnameprojectids.go b/internal/lagoon/groupnameprojectids.go deleted file mode 100644 index 84de620a..00000000 --- a/internal/lagoon/groupnameprojectids.go +++ /dev/null @@ -1,75 +0,0 @@ -// Package lagoon provides Lagoon-specific functionality which doesn't fit -// cleanly into the other service packages such as Keycloak or Lagoon DB. -package lagoon - -import ( - "context" - "fmt" - "strings" -) - -// DBService provides methods for querying the Lagoon API DB. -type DBService interface { - GroupIDProjectIDsMap(context.Context) (map[string][]int, error) -} - -// KeycloakService provides methods for querying the Keycloak API. -type KeycloakService interface { - GroupNameGroupIDMap(context.Context) (map[string]string, error) -} - -// given a nested user group name like "/foo-bar/foo-bar-owner", sanity check -// the format and return the top-level group name (between the separators). -func groupNameFromUserGroup(userGroup string) (string, error) { - parts := strings.Split(userGroup, `/`) - switch { - case len(parts) != 3: - return "", fmt.Errorf(`unknown user group format: %v`, userGroup) - case len(parts[0]) != 0: - return "", fmt.Errorf(`missing leading "/": %v`, userGroup) - case len(parts[1]) == 0: - return "", fmt.Errorf(`missing group name: %v`, userGroup) - case len(parts[2]) == 0: - return "", fmt.Errorf(`missing subgroup name: %v`, userGroup) - default: - return parts[1], nil - } -} - -// GroupNameProjectIDsMap generates a map of group names to project IDs for the -// groups the user is a member of. userGroups should be a slice of groups -// including subgroups in the format returned from -func GroupNameProjectIDsMap( - ctx context.Context, - ldb DBService, - k KeycloakService, - userGroups []string, -) (map[string][]int, error) { - // get the map of group names to group IDs - groupNameGroupIDMap, err := k.GroupNameGroupIDMap(ctx) - if err != nil { - return nil, fmt.Errorf("couldn't query keycloak groups: %v", err) - } - // get the group -> project memberships - groupIDProjectIDsMap, err := ldb.GroupIDProjectIDsMap(ctx) - if err != nil { - return nil, fmt.Errorf("couldn't query Lagoon DB group projects: %v", err) - } - groupNameProjectIDsMap := map[string][]int{} - for _, userGroup := range userGroups { - // carve out group name from the user group - groupName, err := groupNameFromUserGroup(userGroup) - if err != nil { - return nil, fmt.Errorf("couldn't parse user group: %v", err) - } - // for each user group, get the group ID - groupID, ok := groupNameGroupIDMap[groupName] - if !ok { - return nil, fmt.Errorf("couldn't get group ID for group: %v", groupName) - } - // use the group ID to find the group projects and map groupName to project - // IDs in the groupProjectIDs map - groupNameProjectIDsMap[groupName] = groupIDProjectIDsMap[groupID] - } - return groupNameProjectIDsMap, nil -} diff --git a/internal/lagoon/groupnameprojectids_test.go b/internal/lagoon/groupnameprojectids_test.go deleted file mode 100644 index 27a34b66..00000000 --- a/internal/lagoon/groupnameprojectids_test.go +++ /dev/null @@ -1,89 +0,0 @@ -package lagoon_test - -import ( - "context" - "testing" - - "github.com/alecthomas/assert/v2" - "github.com/uselagoon/ssh-portal/internal/lagoon" - "github.com/uselagoon/ssh-portal/internal/mock" - "go.uber.org/mock/gomock" -) - -var ( - groupNameGroupIDMap = map[string]string{ - "project-bs-demo": "89c2894b-5345-453d-839d-2c210fe9b18d", - "project-drupal-example": "948adf3d-f075-4659-925d-7d1d4a85f0ba", - "project-skip-test-project": "ea6bd1a8-a1e7-46cc-a62e-cca8dc27f5ed", - "project-test-drupal-example-simple": "0ce10b5d-72ca-40a5-a33f-056b8565521f", - "another-random-group": "7fd49076-5fc9-4b2f-9998-3a3eff731ec0", - } - groupIDProjectIDsMap = map[string][]int{ - "89c2894b-5345-453d-839d-2c210fe9b18d": {1, 23}, - "948adf3d-f075-4659-925d-7d1d4a85f0ba": {45}, - "ea6bd1a8-a1e7-46cc-a62e-cca8dc27f5ed": {6, 7, 8}, - "0ce10b5d-72ca-40a5-a33f-056b8565521f": {90}, - "7fd49076-5fc9-4b2f-9998-3a3eff731ec0": {2, 3}, - } -) - -func TestGroupNameProjectIDsMap(t *testing.T) { - var testCases = map[string]struct { - input []string - expect map[string][]int - expectError bool - }{ - "happy path": { - input: []string{ - "/project-bs-demo/project-as-demo-developer", - "/project-drupal-example/project-drupal-example-maintainer", - "/project-skip-test-project/project-skip-test-project-owner", - "/project-test-drupal-example-simple/project-test-drupal-example-simple-maintainer", - }, - expect: map[string][]int{ - "project-bs-demo": {1, 23}, - "project-drupal-example": {45}, - "project-skip-test-project": {6, 7, 8}, - "project-test-drupal-example-simple": {90}, - }, - }, - "invalid group name": { - input: []string{ - "/project-bs-demo/project-as-demo-developer", - "/project-drupal-example/project-drupal-example-maintainer", - "/project-skip-test-project/project-skip-test-project-owner", - "invalid-group/foo", - }, - expectError: true, - }, - "unknown group": { - input: []string{ - "/project-vandelay/project-as-demo-developer", - "/project-drupal-example/project-drupal-example-maintainer", - "/project-skip-test-project/project-skip-test-project-owner", - "/project-test-drupal-example-simple/project-test-drupal-example-simple-maintainer", - }, - expectError: true, - }, - } - for name, tc := range testCases { - t.Run(name, func(tt *testing.T) { - ctx := context.Background() - // set up mocks - ctrl := gomock.NewController(tt) - kcService := mock.NewMockKeycloakService(ctrl) - dbService := mock.NewMockDBService(ctrl) - // configure mocks - kcService.EXPECT().GroupNameGroupIDMap(ctx).Return(groupNameGroupIDMap, nil) - dbService.EXPECT().GroupIDProjectIDsMap(ctx).Return(groupIDProjectIDsMap, nil) - // test function - gnpids, err := lagoon.GroupNameProjectIDsMap(ctx, dbService, kcService, tc.input) - if tc.expectError { - assert.Error(tt, err, name) - } else { - assert.NoError(tt, err, name) - assert.Equal(tt, tc.expect, gnpids, name) - } - }) - } -} diff --git a/internal/lagoondb/client.go b/internal/lagoondb/client.go index fa8255cc..a13829be 100644 --- a/internal/lagoondb/client.go +++ b/internal/lagoondb/client.go @@ -36,13 +36,6 @@ type User struct { UUID *uuid.UUID `db:"uuid"` } -// groupProjectMapping maps Lagoon group ID to project ID. -// This type is only used for database unmarshalling. -type groupProjectMapping struct { - GroupID string `db:"group_id"` - ProjectID int `db:"project_id"` -} - // ErrNoResult is returned by client methods if there is no result. var ErrNoResult = errors.New("no rows in result set") @@ -144,31 +137,6 @@ func (c *Client) SSHEndpointByEnvironmentID(ctx context.Context, return ssh.Host, ssh.Port, nil } -// GroupIDProjectIDsMap returns a map of Group (UU)IDs to Project IDs. -// This denotes Project Group membership in Lagoon. -func (c *Client) GroupIDProjectIDsMap( - ctx context.Context, -) (map[string][]int, error) { - var gpms []groupProjectMapping - err := c.db.SelectContext(ctx, &gpms, - `SELECT group_id, project_id `+ - `FROM kc_group_projects`) - if err != nil { - if errors.Is(err, sql.ErrNoRows) { - return nil, ErrNoResult - } - return nil, err - } - groupIDProjectIDsMap := map[string][]int{} - // no need to check for duplicates here since the table has: - // UNIQUE KEY `group_project` (`group_id`,`project_id`) - for _, gpm := range gpms { - groupIDProjectIDsMap[gpm.GroupID] = - append(groupIDProjectIDsMap[gpm.GroupID], gpm.ProjectID) - } - return groupIDProjectIDsMap, nil -} - // SSHKeyUsed sets the last_used attribute of the ssh key identified by the // given fingerprint to used. // @@ -182,6 +150,7 @@ func (c *Client) SSHKeyUsed( // set up tracing ctx, span := otel.Tracer(pkgName).Start(ctx, "SSHKeyUsed") defer span.End() + // run query _, err := c.db.ExecContext(ctx, `UPDATE ssh_key `+ `SET last_used = ? `+ @@ -194,3 +163,28 @@ func (c *Client) SSHKeyUsed( } return nil } + +// ProjectGroupIDs returns a slice of Group (UU)IDs of which the project +// identified by the given projectID is a member. +func (c *Client) ProjectGroupIDs( + ctx context.Context, + projectID int, +) ([]uuid.UUID, error) { + // set up tracing + ctx, span := otel.Tracer(pkgName).Start(ctx, "ProjectGroupIDs") + defer span.End() + // run query + var gids []uuid.UUID + err := c.db.SelectContext(ctx, &gids, + `SELECT group_id `+ + `FROM kc_group_projects `+ + `WHERE project_id = ?`, + projectID) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return nil, ErrNoResult + } + return nil, err + } + return gids, nil +} diff --git a/internal/mock/lagoon_groupnameprojectids.go b/internal/mock/lagoon_groupnameprojectids.go deleted file mode 100644 index b39e0660..00000000 --- a/internal/mock/lagoon_groupnameprojectids.go +++ /dev/null @@ -1,95 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: ../lagoon/groupnameprojectids.go -// -// Generated by this command: -// -// mockgen -source=../lagoon/groupnameprojectids.go -package=mock -destination=lagoon_groupnameprojectids.go -write_generate_directive -// - -// Package mock is a generated GoMock package. -package mock - -import ( - context "context" - reflect "reflect" - - gomock "go.uber.org/mock/gomock" -) - -//go:generate mockgen -source=../lagoon/groupnameprojectids.go -package=mock -destination=lagoon_groupnameprojectids.go -write_generate_directive - -// MockDBService is a mock of DBService interface. -type MockDBService struct { - ctrl *gomock.Controller - recorder *MockDBServiceMockRecorder -} - -// MockDBServiceMockRecorder is the mock recorder for MockDBService. -type MockDBServiceMockRecorder struct { - mock *MockDBService -} - -// NewMockDBService creates a new mock instance. -func NewMockDBService(ctrl *gomock.Controller) *MockDBService { - mock := &MockDBService{ctrl: ctrl} - mock.recorder = &MockDBServiceMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockDBService) EXPECT() *MockDBServiceMockRecorder { - return m.recorder -} - -// GroupIDProjectIDsMap mocks base method. -func (m *MockDBService) GroupIDProjectIDsMap(arg0 context.Context) (map[string][]int, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GroupIDProjectIDsMap", arg0) - ret0, _ := ret[0].(map[string][]int) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GroupIDProjectIDsMap indicates an expected call of GroupIDProjectIDsMap. -func (mr *MockDBServiceMockRecorder) GroupIDProjectIDsMap(arg0 any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GroupIDProjectIDsMap", reflect.TypeOf((*MockDBService)(nil).GroupIDProjectIDsMap), arg0) -} - -// MockKeycloakService is a mock of KeycloakService interface. -type MockKeycloakService struct { - ctrl *gomock.Controller - recorder *MockKeycloakServiceMockRecorder -} - -// MockKeycloakServiceMockRecorder is the mock recorder for MockKeycloakService. -type MockKeycloakServiceMockRecorder struct { - mock *MockKeycloakService -} - -// NewMockKeycloakService creates a new mock instance. -func NewMockKeycloakService(ctrl *gomock.Controller) *MockKeycloakService { - mock := &MockKeycloakService{ctrl: ctrl} - mock.recorder = &MockKeycloakServiceMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockKeycloakService) EXPECT() *MockKeycloakServiceMockRecorder { - return m.recorder -} - -// GroupNameGroupIDMap mocks base method. -func (m *MockKeycloakService) GroupNameGroupIDMap(arg0 context.Context) (map[string]string, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GroupNameGroupIDMap", arg0) - ret0, _ := ret[0].(map[string]string) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GroupNameGroupIDMap indicates an expected call of GroupNameGroupIDMap. -func (mr *MockKeycloakServiceMockRecorder) GroupNameGroupIDMap(arg0 any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GroupNameGroupIDMap", reflect.TypeOf((*MockKeycloakService)(nil).GroupNameGroupIDMap), arg0) -} diff --git a/internal/rbac/mock/permission.go b/internal/rbac/mock/permission.go new file mode 100644 index 00000000..4447dfe9 --- /dev/null +++ b/internal/rbac/mock/permission.go @@ -0,0 +1,127 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: ../permission.go +// +// Generated by this command: +// +// mockgen -source=../permission.go -package=mock -destination=permission.go -write_generate_directive +// + +// Package mock is a generated GoMock package. +package mock + +import ( + context "context" + reflect "reflect" + + uuid "github.com/google/uuid" + lagoon "github.com/uselagoon/ssh-portal/internal/lagoon" + gomock "go.uber.org/mock/gomock" +) + +//go:generate mockgen -source=../permission.go -package=mock -destination=permission.go -write_generate_directive + +// MockKeycloakService is a mock of KeycloakService interface. +type MockKeycloakService struct { + ctrl *gomock.Controller + recorder *MockKeycloakServiceMockRecorder +} + +// MockKeycloakServiceMockRecorder is the mock recorder for MockKeycloakService. +type MockKeycloakServiceMockRecorder struct { + mock *MockKeycloakService +} + +// NewMockKeycloakService creates a new mock instance. +func NewMockKeycloakService(ctrl *gomock.Controller) *MockKeycloakService { + mock := &MockKeycloakService{ctrl: ctrl} + mock.recorder = &MockKeycloakServiceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockKeycloakService) EXPECT() *MockKeycloakServiceMockRecorder { + return m.recorder +} + +// AncestorGroups mocks base method. +func (m *MockKeycloakService) AncestorGroups(arg0 context.Context, arg1 []uuid.UUID) ([]uuid.UUID, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AncestorGroups", arg0, arg1) + ret0, _ := ret[0].([]uuid.UUID) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// AncestorGroups indicates an expected call of AncestorGroups. +func (mr *MockKeycloakServiceMockRecorder) AncestorGroups(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AncestorGroups", reflect.TypeOf((*MockKeycloakService)(nil).AncestorGroups), arg0, arg1) +} + +// UserGroupIDRole mocks base method. +func (m *MockKeycloakService) UserGroupIDRole(arg0 context.Context, arg1 []string) map[uuid.UUID]lagoon.UserRole { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UserGroupIDRole", arg0, arg1) + ret0, _ := ret[0].(map[uuid.UUID]lagoon.UserRole) + return ret0 +} + +// UserGroupIDRole indicates an expected call of UserGroupIDRole. +func (mr *MockKeycloakServiceMockRecorder) UserGroupIDRole(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UserGroupIDRole", reflect.TypeOf((*MockKeycloakService)(nil).UserGroupIDRole), arg0, arg1) +} + +// UserRolesAndGroups mocks base method. +func (m *MockKeycloakService) UserRolesAndGroups(arg0 context.Context, arg1 uuid.UUID) ([]string, []string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UserRolesAndGroups", arg0, arg1) + ret0, _ := ret[0].([]string) + ret1, _ := ret[1].([]string) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// UserRolesAndGroups indicates an expected call of UserRolesAndGroups. +func (mr *MockKeycloakServiceMockRecorder) UserRolesAndGroups(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UserRolesAndGroups", reflect.TypeOf((*MockKeycloakService)(nil).UserRolesAndGroups), arg0, arg1) +} + +// MockLagoonDBService is a mock of LagoonDBService interface. +type MockLagoonDBService struct { + ctrl *gomock.Controller + recorder *MockLagoonDBServiceMockRecorder +} + +// MockLagoonDBServiceMockRecorder is the mock recorder for MockLagoonDBService. +type MockLagoonDBServiceMockRecorder struct { + mock *MockLagoonDBService +} + +// NewMockLagoonDBService creates a new mock instance. +func NewMockLagoonDBService(ctrl *gomock.Controller) *MockLagoonDBService { + mock := &MockLagoonDBService{ctrl: ctrl} + mock.recorder = &MockLagoonDBServiceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockLagoonDBService) EXPECT() *MockLagoonDBServiceMockRecorder { + return m.recorder +} + +// ProjectGroupIDs mocks base method. +func (m *MockLagoonDBService) ProjectGroupIDs(arg0 context.Context, arg1 int) ([]uuid.UUID, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ProjectGroupIDs", arg0, arg1) + ret0, _ := ret[0].([]uuid.UUID) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ProjectGroupIDs indicates an expected call of ProjectGroupIDs. +func (mr *MockLagoonDBServiceMockRecorder) ProjectGroupIDs(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ProjectGroupIDs", reflect.TypeOf((*MockLagoonDBService)(nil).ProjectGroupIDs), arg0, arg1) +} diff --git a/internal/rbac/permission.go b/internal/rbac/permission.go index a5d0ec09..7ca12381 100644 --- a/internal/rbac/permission.go +++ b/internal/rbac/permission.go @@ -1,12 +1,54 @@ // Package rbac contains permission logic for Lagoon. package rbac -import "github.com/uselagoon/ssh-portal/internal/lagoon" +import ( + "context" + + "github.com/google/uuid" + "github.com/uselagoon/ssh-portal/internal/lagoon" +) + +// Default permission map of environment type to roles which can SSH. +// +// By default: +// - Developer and higher can SSH to development environments. +// - Maintainer and higher can SSH to production environments. +// +// See https://docs.lagoon.sh/administering-lagoon/rbac/#group-roles for more +// information. +// +// Note that this does not affect the platform-owner role, which can always SSH +// to any environment. +var defaultEnvTypeRoleCanSSH = map[lagoon.EnvironmentType]map[lagoon.UserRole]bool{ + lagoon.Development: { + lagoon.Developer: true, + lagoon.Maintainer: true, + lagoon.Owner: true, + }, + lagoon.Production: { + lagoon.Maintainer: true, + lagoon.Owner: true, + }, +} + +// KeycloakService provides methods for querying the Keycloak API. +type KeycloakService interface { + AncestorGroups(context.Context, []uuid.UUID) ([]uuid.UUID, error) + UserGroupIDRole(context.Context, []string) map[uuid.UUID]lagoon.UserRole + UserRolesAndGroups(context.Context, uuid.UUID) ([]string, []string, error) +} + +// LagoonDBService provides methods for querying the Lagoon API DB. +type LagoonDBService interface { + ProjectGroupIDs(context.Context, int) ([]uuid.UUID, error) +} // Permission encapsulates the permission logic for Lagoon. // This object should not be constructed by itself, only via NewPermission(). type Permission struct { - envTypeRoleCanSSH map[lagoon.EnvironmentType][]lagoon.UserRole + keycloak KeycloakService + lagoonDB LagoonDBService + envTypeRoleCanSSH map[lagoon.EnvironmentType]map[lagoon.UserRole]bool } // Option performs optional configuration on Permission objects during @@ -19,22 +61,28 @@ type Option func(*Permission) // Production environments. func BlockDeveloperSSH() Option { return func(p *Permission) { - p.envTypeRoleCanSSH = map[lagoon.EnvironmentType][]lagoon.UserRole{ + p.envTypeRoleCanSSH = map[lagoon.EnvironmentType]map[lagoon.UserRole]bool{ lagoon.Development: { - lagoon.Maintainer, - lagoon.Owner, + lagoon.Maintainer: true, + lagoon.Owner: true, }, lagoon.Production: { - lagoon.Maintainer, - lagoon.Owner, + lagoon.Maintainer: true, + lagoon.Owner: true, }, } } } // NewPermission applies the given Options and returns a new Permission object. -func NewPermission(opts ...Option) *Permission { +func NewPermission( + k KeycloakService, + l LagoonDBService, + opts ...Option, +) *Permission { p := Permission{ + keycloak: k, + lagoonDB: l, envTypeRoleCanSSH: defaultEnvTypeRoleCanSSH, } for _, opt := range opts { diff --git a/internal/rbac/usercansshtoenvironment.go b/internal/rbac/usercansshtoenvironment.go index 3d087dd2..4c765f8c 100644 --- a/internal/rbac/usercansshtoenvironment.go +++ b/internal/rbac/usercansshtoenvironment.go @@ -3,35 +3,36 @@ package rbac import ( "context" "fmt" + "log/slog" + "github.com/google/uuid" "github.com/uselagoon/ssh-portal/internal/lagoon" - "github.com/uselagoon/ssh-portal/internal/lagoondb" "go.opentelemetry.io/otel" ) const pkgName = "github.com/uselagoon/ssh-portal/internal/rbac" -// Default permission map of environment type to roles which can SSH. -// -// By default: -// - Developer and higher can SSH to development environments. -// - Maintainer and higher can SSH to production environments. -// -// See https://docs.lagoon.sh/administering-lagoon/rbac/#group-roles for more -// information. -// -// Note that this does not affect the platform-owner role, which can always SSH -// to any environment. -var defaultEnvTypeRoleCanSSH = map[lagoon.EnvironmentType][]lagoon.UserRole{ - lagoon.Development: { - lagoon.Developer, - lagoon.Maintainer, - lagoon.Owner, - }, - lagoon.Production: { - lagoon.Maintainer, - lagoon.Owner, - }, +// calculateUserCanSSHToEnvironment takes a slice of project Group IDs +// (the direct project group as well as any ancestor groups), a map of user +// group IDs to Lagoon user roles, and a map of user roles to access +// permissions. +// This function returns true if the user is a member of any of the given +// project groups, with a role that permits SSH access, and false otherwise. +func calculateUserCanSSHToEnvironment( + projectGroupIDs []uuid.UUID, + userGroupIDRole map[uuid.UUID]lagoon.UserRole, + sshRoles map[lagoon.UserRole]bool, +) bool { + for _, pgid := range projectGroupIDs { + userRole, ok := userGroupIDRole[pgid] + if !ok { + continue + } + if sshRoles[userRole] { + return true + } + } + return false } // UserCanSSHToEnvironment returns true if the given environment can be @@ -39,57 +40,52 @@ var defaultEnvTypeRoleCanSSH = map[lagoon.EnvironmentType][]lagoon.UserRole{ // and false otherwise. func (p *Permission) UserCanSSHToEnvironment( ctx context.Context, - env *lagoondb.Environment, - realmRoles, - userGroups []string, - groupNameProjectIDsMap map[string][]int, -) bool { + log *slog.Logger, + userUUID uuid.UUID, + projectID int, + envType lagoon.EnvironmentType, +) (bool, error) { // set up tracing _, span := otel.Tracer(pkgName).Start(ctx, "UserCanSSHToEnvironment") defer span.End() + // get the user roles and group paths + realmRoles, userGroupPaths, err := p.keycloak.UserRolesAndGroups(ctx, userUUID) + if err != nil { + return false, + fmt.Errorf("couldn't query roles and groups for user %v: %v", userUUID, err) + } // check for platform owner for _, r := range realmRoles { if r == "platform-owner" { - return true + log.Debug("granting permission due to platform-owner realm role", + slog.Any("realmRoles", realmRoles)) + return true, nil } } - validRoles := p.envTypeRoleCanSSH[env.Type] - // check if the user is directly a member of the project group and has the - // required role - var validProjectGroups []string - for _, role := range validRoles { - validProjectGroups = append(validProjectGroups, - fmt.Sprintf("/project-%s/project-%s-%s", - env.ProjectName, env.ProjectName, role)) - } - for _, userGroup := range userGroups { - for _, validProjectGroup := range validProjectGroups { - if userGroup == validProjectGroup { - return true - } - } + // convert the group paths to group ID -> role map + userGroupIDRole := p.keycloak.UserGroupIDRole(ctx, userGroupPaths) + // get the IDs of all groups the project is in + projectGroupIDs, err := p.lagoonDB.ProjectGroupIDs(ctx, projectID) + if err != nil { + return false, + fmt.Errorf("couldn't get group IDs for project %v: %v", projectID, err) } - // check if the user is a member of a group containing the project and has - // the required role - for groupName, pids := range groupNameProjectIDsMap { - for _, pid := range pids { - if pid == env.ProjectID { - // user is in the same group as project, check if they have the - // required role - var validGroups []string - for _, role := range validRoles { - validGroups = append(validGroups, - fmt.Sprintf("/%s/%s-%s", groupName, groupName, role)) - } - for _, userGroup := range userGroups { - for _, validGroup := range validGroups { - if userGroup == validGroup { - return true - } - } - } - } - } + // expand the group IDs for the project with any ancestor groups, since the + // user's membership of all ancestor groups should be considered when + // calculating permissions. + ancestorGroups, err := p.keycloak.AncestorGroups(ctx, projectGroupIDs) + if err != nil { + return false, + fmt.Errorf("couldn't expand project group IDs %v: %v", projectID, err) } - return false + sshRoles := p.envTypeRoleCanSSH[envType] + log.Debug("assessing permission", + slog.Any("realmRoles", realmRoles), + slog.Any("userGroupIDRole", userGroupIDRole), + slog.Any("projectGroupIDs", projectGroupIDs), + slog.Any("sshRoles", sshRoles), + slog.String("userID", userUUID.String()), + ) + return calculateUserCanSSHToEnvironment( + ancestorGroups, userGroupIDRole, sshRoles), nil } diff --git a/internal/rbac/usercansshtoenvironment_test.go b/internal/rbac/usercansshtoenvironment_test.go index 171ce2b0..d100ec6b 100644 --- a/internal/rbac/usercansshtoenvironment_test.go +++ b/internal/rbac/usercansshtoenvironment_test.go @@ -2,353 +2,393 @@ package rbac_test import ( "context" + "log/slog" + "os" "testing" + "github.com/google/uuid" "github.com/uselagoon/ssh-portal/internal/lagoon" - "github.com/uselagoon/ssh-portal/internal/lagoondb" "github.com/uselagoon/ssh-portal/internal/rbac" + "github.com/uselagoon/ssh-portal/internal/rbac/mock" + "go.uber.org/mock/gomock" ) -type args struct { - env *lagoondb.Environment - realmRoles []string - userGroups []string - groupProjectIDs map[string][]int -} - func TestUserCanSSHDefaultRBAC(t *testing.T) { + log := slog.New(slog.NewJSONHandler(os.Stderr, nil)) var testCases = map[string]struct { - input *args - expect bool + // input + userUUID uuid.UUID + projectID int + envType lagoon.EnvironmentType + // mock data + realmRoles []string + userGroupPaths []string + userGroupIDRole map[uuid.UUID]lagoon.UserRole + projectGroupIDs []uuid.UUID + // ancestorGroups must be a superset of projectGroupIDs + ancestorGroups []uuid.UUID + // this flag avoids setting up mock expectations when realm role attributes + // mean RBAC logic is short-circuited + realmRoleShortCircuit bool + // expectations + permissionDefault bool + permissionBlockDevelopers bool }{ - "wrong project": {input: &args{ - env: &lagoondb.Environment{ - Name: "production", - NamespaceName: "project-bar-production", - ProjectID: 4, - ProjectName: "project-bar", - Type: lagoon.Production, - }, + "maintainer wrong project dev": { + userUUID: uuid.UUID{}, + projectID: 4, + envType: lagoon.Development, realmRoles: []string{ "offline_access", "uma_authorization", }, - userGroups: []string{ + userGroupPaths: []string{ "/project-foo/project-foo-maintainer", }, - groupProjectIDs: map[string][]int{ - "project-foo": {3}, - }, - }, expect: false}, - "right project": {input: &args{ - env: &lagoondb.Environment{ - Name: "production", - NamespaceName: "project-bar-production", - ProjectID: 4, - ProjectName: "project-bar", - Type: lagoon.Production, - }, - realmRoles: []string{ - "offline_access", - "uma_authorization", - }, - userGroups: []string{ - "/project-bar/project-bar-maintainer", + userGroupIDRole: map[uuid.UUID]lagoon.UserRole{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"): lagoon.Maintainer, }, - groupProjectIDs: map[string][]int{ - "project-bar": {4}, + projectGroupIDs: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000002"), }, - }, expect: true}, - "not group member": {input: &args{ - env: &lagoondb.Environment{ - Name: "production", - NamespaceName: "project-bar-production", - ProjectID: 4, - ProjectName: "project-bar", - Type: lagoon.Production, + ancestorGroups: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000002"), }, + permissionDefault: false, + permissionBlockDevelopers: false, + }, + "owner wrong project prod": { + userUUID: uuid.UUID{}, + projectID: 4, + envType: lagoon.Production, realmRoles: []string{ "offline_access", "uma_authorization", }, - userGroups: []string{ + userGroupPaths: []string{ "/customer-a/customer-a-maintainer", }, - groupProjectIDs: map[string][]int{ - "customer-b": {4}, + userGroupIDRole: map[uuid.UUID]lagoon.UserRole{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"): lagoon.Owner, + uuid.MustParse("00000000-0000-0000-0000-000000000002"): lagoon.Maintainer, + }, + projectGroupIDs: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000003"), }, - }, expect: false}, - "group member": {input: &args{ - env: &lagoondb.Environment{ - Name: "production", - NamespaceName: "project-bar-production", - ProjectID: 4, - ProjectName: "project-bar", - Type: lagoon.Production, + ancestorGroups: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000003"), }, + permissionDefault: false, + permissionBlockDevelopers: false, + }, + "maintainer ssh to prod": { + userUUID: uuid.UUID{}, + projectID: 4, + envType: lagoon.Production, realmRoles: []string{ "offline_access", "uma_authorization", }, - userGroups: []string{ - "/customer-b/customer-b-maintainer", - }, - groupProjectIDs: map[string][]int{ - "customer-b": {4}, + userGroupPaths: []string{ + "/project-bar/project-bar-maintainer", }, - }, expect: true}, - "platform-owner": {input: &args{ - env: &lagoondb.Environment{ - Name: "production", - NamespaceName: "project-bar-production", - ProjectID: 4, - ProjectName: "project-bar", - Type: lagoon.Production, + userGroupIDRole: map[uuid.UUID]lagoon.UserRole{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"): lagoon.Maintainer, }, - realmRoles: []string{ - "offline_access", - "uma_authorization", - "platform-owner", + projectGroupIDs: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"), }, - userGroups: []string{ - "/lagoonadmin", - }, - }, expect: true}, - "developer can't ssh to prod": {input: &args{ - env: &lagoondb.Environment{ - Name: "production", - NamespaceName: "project-bar-production", - ProjectID: 4, - ProjectName: "project-bar", - Type: lagoon.Production, + ancestorGroups: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"), }, + permissionDefault: true, + permissionBlockDevelopers: true, + }, + "maintainer ssh to dev": { + userUUID: uuid.UUID{}, + projectID: 4, + envType: lagoon.Development, realmRoles: []string{ "offline_access", "uma_authorization", }, - userGroups: []string{ - "/customer-b/customer-b-developer", + userGroupPaths: []string{ + "/customer-b/customer-b-maintainer", + }, + userGroupIDRole: map[uuid.UUID]lagoon.UserRole{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"): lagoon.Maintainer, + uuid.MustParse("00000000-0000-0000-0000-000000000002"): lagoon.Maintainer, + uuid.MustParse("00000000-0000-0000-0000-000000000003"): lagoon.Maintainer, }, - groupProjectIDs: map[string][]int{ - "customer-b": {4}, + projectGroupIDs: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000003"), }, - }, expect: false}, - "developer can ssh to dev": {input: &args{ - env: &lagoondb.Environment{ - Name: "pr-123", - NamespaceName: "project-bar-pr-123", - ProjectID: 4, - ProjectName: "project-bar", - Type: lagoon.Development, + ancestorGroups: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000003"), }, + permissionDefault: true, + permissionBlockDevelopers: true, + }, + "parent group maintainer ssh to prod": { + userUUID: uuid.UUID{}, + projectID: 4, + envType: lagoon.Production, realmRoles: []string{ "offline_access", "uma_authorization", }, - userGroups: []string{ - "/customer-b/customer-b-developer", + userGroupIDRole: map[uuid.UUID]lagoon.UserRole{ + uuid.MustParse("00000000-0000-0000-0000-000000000002"): lagoon.Maintainer, }, - groupProjectIDs: map[string][]int{ - "customer-b": {4}, + projectGroupIDs: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"), }, - }, expect: true}, - "owner can ssh to prod": {input: &args{ - env: &lagoondb.Environment{ - Name: "production", - NamespaceName: "project-bar-production", - ProjectID: 4, - ProjectName: "project-bar", - Type: lagoon.Production, + ancestorGroups: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"), + uuid.MustParse("00000000-0000-0000-0000-000000000002"), }, + permissionDefault: true, + permissionBlockDevelopers: true, + }, + "grandparent group maintainer ssh to prod": { + userUUID: uuid.UUID{}, + projectID: 4, + envType: lagoon.Production, realmRoles: []string{ "offline_access", "uma_authorization", }, - userGroups: []string{ - "/customer-b/customer-b-owner", + userGroupIDRole: map[uuid.UUID]lagoon.UserRole{ + uuid.MustParse("00000000-0000-0000-0000-000000000003"): lagoon.Maintainer, }, - groupProjectIDs: map[string][]int{ - "customer-b": {4}, + projectGroupIDs: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"), }, - }, expect: true}, - } - p := rbac.NewPermission() - for name, tc := range testCases { - t.Run(name, func(tt *testing.T) { - response := p.UserCanSSHToEnvironment(context.Background(), - tc.input.env, tc.input.realmRoles, tc.input.userGroups, - tc.input.groupProjectIDs) - if response != tc.expect { - tt.Fatalf("expected %v, got %v", tc.expect, response) - } - }) - } -} - -func TestUserCanSSHCustomRBAC(t *testing.T) { - var testCases = map[string]struct { - input *args - expect bool - }{ - "wrong project": {input: &args{ - env: &lagoondb.Environment{ - Name: "production", - NamespaceName: "project-bar-production", - ProjectID: 4, - ProjectName: "project-bar", - Type: lagoon.Production, + ancestorGroups: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"), + uuid.MustParse("00000000-0000-0000-0000-000000000002"), + uuid.MustParse("00000000-0000-0000-0000-000000000003"), }, + permissionDefault: true, + permissionBlockDevelopers: true, + }, + "grandparent group developer ssh to prod": { + userUUID: uuid.UUID{}, + projectID: 4, + envType: lagoon.Production, realmRoles: []string{ "offline_access", "uma_authorization", }, - userGroups: []string{ - "/project-foo/project-foo-maintainer", + userGroupIDRole: map[uuid.UUID]lagoon.UserRole{ + uuid.MustParse("00000000-0000-0000-0000-000000000003"): lagoon.Developer, }, - groupProjectIDs: map[string][]int{ - "project-foo": {3}, + projectGroupIDs: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"), }, - }, expect: false}, - "right project": {input: &args{ - env: &lagoondb.Environment{ - Name: "production", - NamespaceName: "project-bar-production", - ProjectID: 4, - ProjectName: "project-bar", - Type: lagoon.Production, + ancestorGroups: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"), + uuid.MustParse("00000000-0000-0000-0000-000000000002"), + uuid.MustParse("00000000-0000-0000-0000-000000000003"), }, + permissionDefault: false, + permissionBlockDevelopers: false, + }, + "grandparent group developer ssh to dev": { + userUUID: uuid.UUID{}, + projectID: 4, + envType: lagoon.Development, realmRoles: []string{ "offline_access", "uma_authorization", }, - userGroups: []string{ - "/project-bar/project-bar-maintainer", + userGroupIDRole: map[uuid.UUID]lagoon.UserRole{ + uuid.MustParse("00000000-0000-0000-0000-000000000003"): lagoon.Developer, }, - groupProjectIDs: map[string][]int{ - "project-bar": {4}, + projectGroupIDs: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"), }, - }, expect: true}, - "not group member": {input: &args{ - env: &lagoondb.Environment{ - Name: "production", - NamespaceName: "project-bar-production", - ProjectID: 4, - ProjectName: "project-bar", - Type: lagoon.Production, + ancestorGroups: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"), + uuid.MustParse("00000000-0000-0000-0000-000000000002"), + uuid.MustParse("00000000-0000-0000-0000-000000000003"), }, + permissionDefault: true, + permissionBlockDevelopers: false, + }, + "platform-owner ssh to prod": { + userUUID: uuid.UUID{}, + projectID: 4, + envType: lagoon.Production, realmRoles: []string{ "offline_access", "uma_authorization", + "platform-owner", }, - userGroups: []string{ - "/customer-a/customer-a-maintainer", - }, - groupProjectIDs: map[string][]int{ - "customer-b": {4}, - }, - }, expect: false}, - "group member": {input: &args{ - env: &lagoondb.Environment{ - Name: "production", - NamespaceName: "project-bar-production", - ProjectID: 4, - ProjectName: "project-bar", - Type: lagoon.Production, - }, + realmRoleShortCircuit: true, + permissionDefault: true, + permissionBlockDevelopers: true, + }, + "developer ssh to prod": { + userUUID: uuid.UUID{}, + projectID: 4, + envType: lagoon.Production, realmRoles: []string{ "offline_access", "uma_authorization", }, - userGroups: []string{ - "/customer-b/customer-b-maintainer", + userGroupIDRole: map[uuid.UUID]lagoon.UserRole{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"): lagoon.Developer, }, - groupProjectIDs: map[string][]int{ - "customer-b": {4}, + projectGroupIDs: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"), }, - }, expect: true}, - "platform-owner": {input: &args{ - env: &lagoondb.Environment{ - Name: "production", - NamespaceName: "project-bar-production", - ProjectID: 4, - ProjectName: "project-bar", - Type: lagoon.Production, + ancestorGroups: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"), }, + permissionDefault: false, + permissionBlockDevelopers: false, + }, + "developer ssh to dev": { + userUUID: uuid.UUID{}, + projectID: 4, + envType: lagoon.Development, realmRoles: []string{ "offline_access", "uma_authorization", - "platform-owner", }, - userGroups: []string{ - "/lagoonadmin", + userGroupIDRole: map[uuid.UUID]lagoon.UserRole{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"): lagoon.Developer, + }, + projectGroupIDs: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"), }, - }, expect: true}, - "developer can't ssh to prod": {input: &args{ - env: &lagoondb.Environment{ - Name: "production", - NamespaceName: "project-bar-production", - ProjectID: 4, - ProjectName: "project-bar", - Type: lagoon.Production, + ancestorGroups: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"), }, + permissionDefault: true, + permissionBlockDevelopers: false, + }, + "owner ssh to prod": { + userUUID: uuid.UUID{}, + projectID: 4, + envType: lagoon.Production, realmRoles: []string{ "offline_access", "uma_authorization", }, - userGroups: []string{ - "/customer-b/customer-b-developer", + userGroupIDRole: map[uuid.UUID]lagoon.UserRole{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"): lagoon.Owner, }, - groupProjectIDs: map[string][]int{ - "customer-b": {4}, + projectGroupIDs: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"), }, - }, expect: false}, - "developer can NOT ssh to dev": {input: &args{ - env: &lagoondb.Environment{ - Name: "pr-123", - NamespaceName: "project-bar-pr-123", - ProjectID: 4, - ProjectName: "project-bar", - Type: lagoon.Development, + ancestorGroups: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"), }, + permissionDefault: true, + permissionBlockDevelopers: true, + }, + "owner ssh to dev": { + userUUID: uuid.UUID{}, + projectID: 4, + envType: lagoon.Development, realmRoles: []string{ "offline_access", "uma_authorization", }, - userGroups: []string{ - "/customer-b/customer-b-developer", + userGroupIDRole: map[uuid.UUID]lagoon.UserRole{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"): lagoon.Owner, }, - groupProjectIDs: map[string][]int{ - "customer-b": {4}, + projectGroupIDs: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"), }, - }, expect: false}, - "owner can ssh to prod": {input: &args{ - env: &lagoondb.Environment{ - Name: "production", - NamespaceName: "project-bar-production", - ProjectID: 4, - ProjectName: "project-bar", - Type: lagoon.Production, + ancestorGroups: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"), }, + permissionDefault: true, + permissionBlockDevelopers: true, + }, + "guest ssh to dev": { + userUUID: uuid.UUID{}, + projectID: 4, + envType: lagoon.Development, realmRoles: []string{ "offline_access", "uma_authorization", }, - userGroups: []string{ - "/customer-b/customer-b-owner", + userGroupIDRole: map[uuid.UUID]lagoon.UserRole{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"): lagoon.Guest, }, - groupProjectIDs: map[string][]int{ - "customer-b": {4}, + projectGroupIDs: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"), }, - }, expect: true}, + ancestorGroups: []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"), + }, + permissionDefault: false, + permissionBlockDevelopers: false, + }, } - p := rbac.NewPermission(rbac.BlockDeveloperSSH()) for name, tc := range testCases { t.Run(name, func(tt *testing.T) { - response := p.UserCanSSHToEnvironment(context.Background(), - tc.input.env, tc.input.realmRoles, tc.input.userGroups, - tc.input.groupProjectIDs) - if response != tc.expect { - tt.Fatalf("expected %v, got %v", tc.expect, response) + ctx := context.Background() + // set up mocks + ctrl := gomock.NewController(tt) + defer ctrl.Finish() + kcService := mock.NewMockKeycloakService(ctrl) + kcService.EXPECT(). + UserRolesAndGroups(ctx, tc.userUUID). + Return(tc.realmRoles, tc.userGroupPaths, nil). + Times(2) + ldbService := mock.NewMockLagoonDBService(ctrl) + if !tc.realmRoleShortCircuit { + kcService.EXPECT(). + UserGroupIDRole(ctx, tc.userGroupPaths). + Return(tc.userGroupIDRole). + Times(2) + ldbService.EXPECT(). + ProjectGroupIDs(ctx, tc.projectID). + Return(tc.projectGroupIDs, nil). + Times(2) + kcService.EXPECT(). + AncestorGroups(ctx, tc.projectGroupIDs). + Return(tc.ancestorGroups, nil). + Times(2) + } + // test default permission engine + permDefault := rbac.NewPermission(kcService, ldbService) + ok, err := permDefault.UserCanSSHToEnvironment( + ctx, + log, + tc.userUUID, + tc.projectID, + tc.envType, + ) + if err != nil { + tt.Fatalf("couldn't perform user SSH permisison check: %v", err) + } + if ok != tc.permissionDefault { + tt.Fatalf("expected %v, got %v", tc.permissionDefault, ok) + } + // test alternative permission engine which blocks developer SSH access + permBlockDev := rbac.NewPermission( + kcService, + ldbService, + rbac.BlockDeveloperSSH(), + ) + ok, err = permBlockDev.UserCanSSHToEnvironment( + ctx, + log, + tc.userUUID, + tc.projectID, + tc.envType, + ) + if err != nil { + tt.Fatalf("couldn't perform user SSH permisison check: %v", err) + } + if ok != tc.permissionBlockDevelopers { + tt.Fatalf("expected %v, got %v", tc.permissionDefault, ok) } }) } diff --git a/internal/sshportalapi/server.go b/internal/sshportalapi/server.go index d1dd6da2..f466adb3 100644 --- a/internal/sshportalapi/server.go +++ b/internal/sshportalapi/server.go @@ -9,10 +9,8 @@ import ( "sync" "time" - "github.com/google/uuid" "github.com/nats-io/nats.go" "github.com/uselagoon/ssh-portal/internal/bus" - "github.com/uselagoon/ssh-portal/internal/lagoon" "github.com/uselagoon/ssh-portal/internal/lagoondb" "github.com/uselagoon/ssh-portal/internal/rbac" ) @@ -24,26 +22,18 @@ const ( // LagoonDBService provides methods for querying the Lagoon API DB. type LagoonDBService interface { - lagoon.DBService EnvironmentByNamespaceName(context.Context, string) (*lagoondb.Environment, error) UserBySSHFingerprint(context.Context, string) (*lagoondb.User, error) SSHKeyUsed(context.Context, string, time.Time) error } -// KeycloakService provides methods for querying the Keycloak API. -type KeycloakService interface { - lagoon.KeycloakService - UserRolesAndGroups(context.Context, *uuid.UUID) ([]string, []string, error) -} - // ServeNATS sshportalapi NATS requests. func ServeNATS( ctx context.Context, stop context.CancelFunc, log *slog.Logger, p *rbac.Permission, - l LagoonDBService, - k KeycloakService, + ldb LagoonDBService, natsURL string, ) error { // setup synchronisation @@ -72,7 +62,7 @@ func ServeNATS( _, err = nc.QueueSubscribe( bus.SubjectSSHAccessQuery, queue, - sshportal(ctx, log, nc, p, l, k), + sshportal(ctx, log, nc, p, ldb), ) if err != nil { return fmt.Errorf("couldn't subscribe to queue: %v", err) diff --git a/internal/sshportalapi/sshportal.go b/internal/sshportalapi/sshportal.go index 583a3f41..b9e9d8a4 100644 --- a/internal/sshportalapi/sshportal.go +++ b/internal/sshportalapi/sshportal.go @@ -11,7 +11,6 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" "github.com/uselagoon/ssh-portal/internal/bus" - "github.com/uselagoon/ssh-portal/internal/lagoon" "github.com/uselagoon/ssh-portal/internal/lagoondb" "github.com/uselagoon/ssh-portal/internal/rbac" "go.opentelemetry.io/otel" @@ -34,11 +33,9 @@ func sshportal( log *slog.Logger, c *nats.Conn, p *rbac.Permission, - l LagoonDBService, - k KeycloakService, + ldb LagoonDBService, ) nats.MsgHandler { return func(msg *nats.Msg) { - var realmRoles, userGroups []string // set up tracing and update metrics ctx, span := otel.Tracer(pkgName).Start(ctx, bus.SubjectSSHAccessQuery) defer span.End() @@ -55,7 +52,7 @@ func sshportal( return } // get the environment - env, err := l.EnvironmentByNamespaceName(ctx, query.NamespaceName) + env, err := ldb.EnvironmentByNamespaceName(ctx, query.NamespaceName) if err != nil { if errors.Is(err, lagoondb.ErrNoResult) { log.Warn("unknown namespace name", slog.Any("error", err)) @@ -82,7 +79,7 @@ func sshportal( return } // get the user - user, err := l.UserBySSHFingerprint(ctx, query.SSHFingerprint) + user, err := ldb.UserBySSHFingerprint(ctx, query.SSHFingerprint) if err != nil { if errors.Is(err, lagoondb.ErrNoResult) { log.Debug("unknown SSH Fingerprint", slog.Any("error", err)) @@ -95,36 +92,18 @@ func sshportal( return } // update last_used - if err := l.SSHKeyUsed(ctx, query.SSHFingerprint, time.Now()); err != nil { - log.Error("couldn't update ssh key last used: %v", + if err := ldb.SSHKeyUsed(ctx, query.SSHFingerprint, time.Now()); err != nil { + log.Error("couldn't update ssh key last used", slog.Any("error", err)) return } - // get the user roles and groups - realmRoles, userGroups, err = k.UserRolesAndGroups(ctx, user.UUID) - if err != nil { - log.Error("couldn't query keycloak user roles and groups", - slog.String("userUUID", user.UUID.String()), - slog.Any("error", err)) - return - } - // generate the group name to project IDs map - groupNameProjectIDsMap, err := - lagoon.GroupNameProjectIDsMap(ctx, l, k, userGroups) + // check permission + ok, err := p.UserCanSSHToEnvironment( + ctx, log, *user.UUID, env.ProjectID, env.Type) if err != nil { - log.Error("couldn't generate group name to project IDs map", + log.Error("couldn't check if user can ssh to environment", slog.Any("error", err)) - return } - log.Debug("keycloak user attributes", - slog.Any("realmRoles", realmRoles), - slog.Any("userGroups", userGroups), - slog.Any("groupNameProjectIDsMap", groupNameProjectIDsMap), - slog.String("userUUID", user.UUID.String()), - ) - // check permission - ok := p.UserCanSSHToEnvironment( - ctx, env, realmRoles, userGroups, groupNameProjectIDsMap) var logMsg string var response []byte if ok { @@ -136,8 +115,9 @@ func sshportal( } log.Info(logMsg, slog.Int("environmentID", env.ID), - slog.Int("projectID", env.ProjectID), + slog.String("environmentType", env.Type.String()), slog.String("environmentName", env.Name), + slog.Int("projectID", env.ProjectID), slog.String("projectName", env.ProjectName), slog.String("userUUID", user.UUID.String()), ) diff --git a/internal/sshtoken/authhandler.go b/internal/sshtoken/authhandler.go index a5273bd9..fd8ba1d7 100644 --- a/internal/sshtoken/authhandler.go +++ b/internal/sshtoken/authhandler.go @@ -15,7 +15,7 @@ import ( type ctxKey int const ( - userUUID ctxKey = iota + userUUIDkey ctxKey = iota ) var ( @@ -64,7 +64,7 @@ func pubKeyAuth(log *slog.Logger, ldb LagoonDBService) ssh.PublicKeyHandler { // successful. Inject the user UUID into the context so it can be used in // the session handler. authnSuccessTotal.Inc() - ctx.SetValue(userUUID, user.UUID) + ctx.SetValue(userUUIDkey, user.UUID) log.Info("authentication successful", slog.String("userUUID", user.UUID.String())) return true diff --git a/internal/sshtoken/serve.go b/internal/sshtoken/serve.go index 482417f4..d01c5c4e 100644 --- a/internal/sshtoken/serve.go +++ b/internal/sshtoken/serve.go @@ -11,7 +11,6 @@ import ( "github.com/gliderlabs/ssh" "github.com/uselagoon/ssh-portal/internal/keycloak" - "github.com/uselagoon/ssh-portal/internal/lagoon" "github.com/uselagoon/ssh-portal/internal/lagoondb" "github.com/uselagoon/ssh-portal/internal/rbac" ) @@ -21,7 +20,6 @@ const shutdownTimeout = 8 * time.Second // LagoonDBService provides methods for querying the Lagoon API DB. type LagoonDBService interface { - lagoon.DBService EnvironmentByNamespaceName(context.Context, string) (*lagoondb.Environment, error) UserBySSHFingerprint(context.Context, string) (*lagoondb.User, error) SSHEndpointByEnvironmentID(context.Context, int) (string, string, error) @@ -35,13 +33,11 @@ func Serve( l net.Listener, p *rbac.Permission, ldb *lagoondb.Client, - keycloakToken, - keycloakPermission *keycloak.Client, + keycloakToken *keycloak.Client, hostKeys [][]byte, ) error { srv := ssh.Server{ - Handler: sessionHandler( - log, p, keycloakToken, keycloakPermission, ldb), + Handler: sessionHandler(log, p, keycloakToken, ldb), PublicKeyHandler: pubKeyAuth(log, ldb), } for _, hk := range hostKeys { diff --git a/internal/sshtoken/sessionhandler.go b/internal/sshtoken/sessionhandler.go index fc018b37..824315e7 100644 --- a/internal/sshtoken/sessionhandler.go +++ b/internal/sshtoken/sessionhandler.go @@ -10,7 +10,6 @@ import ( "github.com/google/uuid" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" - "github.com/uselagoon/ssh-portal/internal/lagoon" "github.com/uselagoon/ssh-portal/internal/lagoondb" "github.com/uselagoon/ssh-portal/internal/rbac" ) @@ -18,15 +17,8 @@ import ( // KeycloakTokenService provides methods for querying the Keycloak API for user // access tokens. type KeycloakTokenService interface { - UserAccessTokenResponse(context.Context, *uuid.UUID) (string, error) - UserAccessToken(context.Context, *uuid.UUID) (string, error) -} - -// KeycloakUserInfoService provides methods for querying the Keycloak API for -// permission information contained in service-api user tokens. -type KeycloakUserInfoService interface { - lagoon.KeycloakService - UserRolesAndGroups(context.Context, *uuid.UUID) ([]string, []string, error) + UserAccessTokenResponse(context.Context, uuid.UUID) (string, error) + UserAccessToken(context.Context, uuid.UUID) (string, error) } var ( @@ -46,8 +38,12 @@ var ( // tokenSession returns a bare access token or full access token response based // on the user ID -func tokenSession(s ssh.Session, log *slog.Logger, - keycloakToken KeycloakTokenService, uid *uuid.UUID) { +func tokenSession( + s ssh.Session, + log *slog.Logger, + keycloakToken KeycloakTokenService, + userUUID uuid.UUID, +) { // valid commands: // - grant: returns a full access token response as per // https://www.rfc-editor.org/rfc/rfc6749#section-4.1.4 @@ -72,7 +68,7 @@ func tokenSession(s ssh.Session, log *slog.Logger, var err error switch cmd[0] { case "grant": - response, err = keycloakToken.UserAccessTokenResponse(ctx, uid) + response, err = keycloakToken.UserAccessTokenResponse(ctx, userUUID) if err != nil { log.Warn("couldn't get user access token response", slog.Any("error", err)) @@ -85,7 +81,7 @@ func tokenSession(s ssh.Session, log *slog.Logger, return } case "token": - response, err = keycloakToken.UserAccessToken(ctx, uid) + response, err = keycloakToken.UserAccessToken(ctx, userUUID) if err != nil { log.Warn("couldn't get user access token", slog.Any("error", err)) @@ -129,26 +125,10 @@ func redirectSession( s ssh.Session, log *slog.Logger, p *rbac.Permission, - keycloakUserInfo KeycloakUserInfoService, ldb LagoonDBService, - uid *uuid.UUID, + userUUID uuid.UUID, ) { ctx := s.Context() - // get the user roles and groups - realmRoles, userGroups, err := - keycloakUserInfo.UserRolesAndGroups(s.Context(), uid) - if err != nil { - log.Error("couldn't query user roles and groups", - slog.Any("error", err)) - _, err = fmt.Fprintf(s.Stderr(), - "This SSH server does not provide shell access. SID: %s\r\n", - ctx.SessionID()) - if err != nil { - log.Debug("couldn't write error message to session stream", - slog.Any("error", err)) - } - return - } env, err := ldb.EnvironmentByNamespaceName(s.Context(), s.User()) if err != nil { if errors.Is(err, lagoondb.ErrNoResult) { @@ -173,25 +153,19 @@ func redirectSession( slog.Int("environmentID", env.ID), slog.Int("projectID", env.ProjectID), slog.String("environmentName", env.Name), + slog.String("environmentType", env.Type.String()), slog.String("namespaceName", s.User()), slog.String("projectName", env.ProjectName), + slog.String("userUUID", userUUID.String()), ) - groupNameProjectIDsMap, err := - lagoon.GroupNameProjectIDsMap(ctx, ldb, keycloakUserInfo, userGroups) + // check permission + ok, err := p.UserCanSSHToEnvironment( + s.Context(), log, userUUID, env.ProjectID, env.Type) if err != nil { - log.Error("couldn't generate group name to project IDs map", - slog.Any("error", err)) - return + log.Error("couldn't check if user can ssh to environment") } - // check permission - ok := p.UserCanSSHToEnvironment(s.Context(), env, realmRoles, - userGroups, groupNameProjectIDsMap) if !ok { log.Info("user cannot SSH to environment") - log.Debug("user permissions", - slog.Any("realmRoles", realmRoles), - slog.Any("userGroups", userGroups), - slog.Any("groupProjectIDs", groupNameProjectIDsMap)) _, err = fmt.Fprintf(s.Stderr(), "This SSH server does not provide shell access. SID: %s\r\n", ctx.SessionID()) @@ -202,10 +176,6 @@ func redirectSession( return } log.Info("user can SSH to environment") - log.Debug("user permissions", - slog.Any("realmRoles", realmRoles), - slog.Any("userGroups", userGroups), - slog.Any("groupProjectIDs", groupNameProjectIDsMap)) sshHost, sshPort, err := ldb.SSHEndpointByEnvironmentID(s.Context(), env.ID) if err != nil { if errors.Is(err, lagoondb.ErrNoResult) { @@ -250,16 +220,18 @@ func redirectSession( // sessionHandler returns a ssh.Handler which writes a Lagoon access token to // the session stream and then closes the connection. -func sessionHandler(log *slog.Logger, p *rbac.Permission, +func sessionHandler( + log *slog.Logger, + p *rbac.Permission, keycloakToken KeycloakTokenService, - keycloakPermission KeycloakUserInfoService, - ldb LagoonDBService) ssh.Handler { + ldb LagoonDBService, +) ssh.Handler { return func(s ssh.Session) { sessionTotal.Inc() // extract required info from the session context ctx := s.Context() log := log.With(slog.String("sessionID", ctx.SessionID())) - uid, ok := ctx.Value(userUUID).(*uuid.UUID) + userUUID, ok := ctx.Value(userUUIDkey).(uuid.UUID) if !ok { log.Warn("couldn't get user UUID from context") _, err := fmt.Fprintf(s.Stderr(), "internal error. SID: %s\r\n", @@ -270,11 +242,11 @@ func sessionHandler(log *slog.Logger, p *rbac.Permission, } return } - log = log.With(slog.String("userUUID", uid.String())) + log = log.With(slog.String("userUUID", userUUID.String())) if s.User() == "lagoon" { - tokenSession(s, log, keycloakToken, uid) + tokenSession(s, log, keycloakToken, userUUID) } else { - redirectSession(s, log, p, keycloakPermission, ldb, uid) + redirectSession(s, log, p, ldb, userUUID) } } } From 4be767d05912f31b2c11ded1aed239c771fbc415 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Mon, 7 Oct 2024 23:32:54 +0800 Subject: [PATCH 6/7] chore: add keycloak UserGroupIDRole tests --- internal/keycloak/helper_test.go | 6 + .../testdata/usergroups_children0.json | 32 +++ .../testdata/usergroups_children1.json | 20 ++ .../testdata/usergroups_children2.json | 84 +++++++ .../testdata/usergroups_children3.json | 26 +++ .../testdata/usergroups_children4.json | 50 ++++ .../testdata/usergroups_children5.json | 74 ++++++ .../testdata/usergroups_children6.json | 50 ++++ .../testdata/usergroups_children7.json | 26 +++ .../testdata/usergroups_children8.json | 26 +++ .../testdata/usergroups_groups_first0.json | 72 ++++++ .../testdata/usergroups_groups_first10.json | 72 ++++++ .../testdata/usergroups_groups_first15.json | 72 ++++++ .../testdata/usergroups_groups_first20.json | 44 ++++ .../testdata/usergroups_groups_first5.json | 72 ++++++ internal/keycloak/usergroups_test.go | 217 ++++++++++++++++++ 16 files changed, 943 insertions(+) create mode 100644 internal/keycloak/testdata/usergroups_children0.json create mode 100644 internal/keycloak/testdata/usergroups_children1.json create mode 100644 internal/keycloak/testdata/usergroups_children2.json create mode 100644 internal/keycloak/testdata/usergroups_children3.json create mode 100644 internal/keycloak/testdata/usergroups_children4.json create mode 100644 internal/keycloak/testdata/usergroups_children5.json create mode 100644 internal/keycloak/testdata/usergroups_children6.json create mode 100644 internal/keycloak/testdata/usergroups_children7.json create mode 100644 internal/keycloak/testdata/usergroups_children8.json create mode 100644 internal/keycloak/testdata/usergroups_groups_first0.json create mode 100644 internal/keycloak/testdata/usergroups_groups_first10.json create mode 100644 internal/keycloak/testdata/usergroups_groups_first15.json create mode 100644 internal/keycloak/testdata/usergroups_groups_first20.json create mode 100644 internal/keycloak/testdata/usergroups_groups_first5.json create mode 100644 internal/keycloak/usergroups_test.go diff --git a/internal/keycloak/helper_test.go b/internal/keycloak/helper_test.go index 273a45d6..844c23fa 100644 --- a/internal/keycloak/helper_test.go +++ b/internal/keycloak/helper_test.go @@ -24,3 +24,9 @@ func (l *LagoonClaims) SetClientID(clientID string) { func (c *Client) UseDefaultHTTPClient() { c.httpClient = http.DefaultClient } + +// UsePageSize sets the page size used by the client when retrieving groups +// from Keycloak. +func (c *Client) UsePageSize(pageSize int) { + c.pageSize = pageSize +} diff --git a/internal/keycloak/testdata/usergroups_children0.json b/internal/keycloak/testdata/usergroups_children0.json new file mode 100644 index 00000000..83616dcf --- /dev/null +++ b/internal/keycloak/testdata/usergroups_children0.json @@ -0,0 +1,32 @@ +[ + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "2e833d9b-39b7-4f25-b37f-cfb8765015ab", + "name": "scott-test-child-group2", + "parentId": "ee6d02d1-b14b-41dd-95b6-cb8c26b1a321", + "path": "/scott-test-ancestor-group2/scott-test-child-group2", + "subGroupCount": 3, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "7f22ce84-c0af-4ff4-afcd-288f0473deb5", + "name": "scott-test-child-group3", + "parentId": "ee6d02d1-b14b-41dd-95b6-cb8c26b1a321", + "path": "/scott-test-ancestor-group2/scott-test-child-group3", + "subGroupCount": 1, + "subGroups": [] + } +] diff --git a/internal/keycloak/testdata/usergroups_children1.json b/internal/keycloak/testdata/usergroups_children1.json new file mode 100644 index 00000000..ffa5be15 --- /dev/null +++ b/internal/keycloak/testdata/usergroups_children1.json @@ -0,0 +1,20 @@ +[ + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": {}, + "clientRoles": {}, + "id": "139ad442-1d20-4c58-b009-c0afe21bf85b", + "name": "scott-test-grandchild-group3", + "parentId": "7f22ce84-c0af-4ff4-afcd-288f0473deb5", + "path": "/scott-test-ancestor-group2/scott-test-child-group3/scott-test-grandchild-group3", + "realmRoles": [], + "subGroupCount": 1, + "subGroups": [] + } +] diff --git a/internal/keycloak/testdata/usergroups_children2.json b/internal/keycloak/testdata/usergroups_children2.json new file mode 100644 index 00000000..7559ece8 --- /dev/null +++ b/internal/keycloak/testdata/usergroups_children2.json @@ -0,0 +1,84 @@ +[ + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": { + "type": [ + "role-subgroup" + ] + }, + "clientRoles": {}, + "id": "2c0f9a62-3e50-4e90-913d-90db6cb47cce", + "name": "scott-test-child-group2-developer", + "parentId": "2e833d9b-39b7-4f25-b37f-cfb8765015ab", + "path": "/scott-test-ancestor-group2/scott-test-child-group2/scott-test-child-group2-developer", + "realmRoles": [ + "developer" + ], + "subGroupCount": 0, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": { + "type": [ + "role-subgroup" + ] + }, + "clientRoles": {}, + "id": "43db2a06-ca7c-482e-b068-23127ae5d50a", + "name": "scott-test-child-group2-maintainer", + "parentId": "2e833d9b-39b7-4f25-b37f-cfb8765015ab", + "path": "/scott-test-ancestor-group2/scott-test-child-group2/scott-test-child-group2-maintainer", + "realmRoles": [], + "subGroupCount": 0, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": {}, + "clientRoles": {}, + "id": "879d1d38-97d8-449a-affd-8529b8e31feb", + "name": "scott-test-grandchild-group2", + "parentId": "2e833d9b-39b7-4f25-b37f-cfb8765015ab", + "path": "/scott-test-ancestor-group2/scott-test-child-group2/scott-test-grandchild-group2", + "realmRoles": [], + "subGroupCount": 1, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": {}, + "clientRoles": {}, + "id": "c7d3b738-91f2-4cf1-aeec-2ab444eb3215", + "name": "scott-test-grandchild-group2b", + "parentId": "2e833d9b-39b7-4f25-b37f-cfb8765015ab", + "path": "/scott-test-ancestor-group2/scott-test-child-group2/scott-test-grandchild-group2b", + "realmRoles": [], + "subGroupCount": 1, + "subGroups": [] + } +] diff --git a/internal/keycloak/testdata/usergroups_children3.json b/internal/keycloak/testdata/usergroups_children3.json new file mode 100644 index 00000000..b8e7797d --- /dev/null +++ b/internal/keycloak/testdata/usergroups_children3.json @@ -0,0 +1,26 @@ +[ + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": { + "type": [ + "role-subgroup" + ] + }, + "clientRoles": {}, + "id": "26fc9bcf-1d04-426f-ac25-67146d2d3fd0", + "name": "scott-test-grandchild-group3-owner", + "parentId": "139ad442-1d20-4c58-b009-c0afe21bf85b", + "path": "/scott-test-ancestor-group2/scott-test-child-group3/scott-test-grandchild-group3/scott-test-grandchild-group3-owner", + "realmRoles": [ + "owner" + ], + "subGroupCount": 0, + "subGroups": [] + } +] diff --git a/internal/keycloak/testdata/usergroups_children4.json b/internal/keycloak/testdata/usergroups_children4.json new file mode 100644 index 00000000..f212d700 --- /dev/null +++ b/internal/keycloak/testdata/usergroups_children4.json @@ -0,0 +1,50 @@ +[ + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": { + "type": [ + "role-subgroup" + ] + }, + "clientRoles": {}, + "id": "d56c752d-e6d5-47c1-980d-2722b5a7c589", + "name": "project-a-fishy-website-maintainer", + "parentId": "54486df8-450d-4b62-8e10-223ac3419d05", + "path": "/project-a-fishy-website/project-a-fishy-website-maintainer", + "realmRoles": [ + "maintainer" + ], + "subGroupCount": 0, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": { + "type": [ + "role-subgroup" + ] + }, + "clientRoles": {}, + "id": "59eaaf1f-f88c-4447-aea1-2b06fec92cb0", + "name": "project-a-fishy-website-owner", + "parentId": "54486df8-450d-4b62-8e10-223ac3419d05", + "path": "/project-a-fishy-website/project-a-fishy-website-owner", + "realmRoles": [ + "owner" + ], + "subGroupCount": 0, + "subGroups": [] + } +] diff --git a/internal/keycloak/testdata/usergroups_children5.json b/internal/keycloak/testdata/usergroups_children5.json new file mode 100644 index 00000000..44475f46 --- /dev/null +++ b/internal/keycloak/testdata/usergroups_children5.json @@ -0,0 +1,74 @@ +[ + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": { + "type": [ + "role-subgroup" + ] + }, + "clientRoles": {}, + "id": "0c104b4a-7573-4bb8-993c-7b8c816e40f4", + "name": "corp6-senior-devs-developer", + "parentId": "eca344cd-2b81-4447-bcf9-ce07aa9d4a1b", + "path": "/corp6-senior-devs/corp6-senior-devs-developer", + "realmRoles": [ + "developer" + ], + "subGroupCount": 0, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": { + "type": [ + "role-subgroup" + ] + }, + "clientRoles": {}, + "id": "68cd6ee1-96a0-4f33-9bb9-7583b154134b", + "name": "corp6-senior-devs-maintainer", + "parentId": "eca344cd-2b81-4447-bcf9-ce07aa9d4a1b", + "path": "/corp6-senior-devs/corp6-senior-devs-maintainer", + "realmRoles": [ + "maintainer" + ], + "subGroupCount": 0, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": { + "type": [ + "role-subgroup" + ] + }, + "clientRoles": {}, + "id": "8bd42c93-3e25-4453-b4f8-c040486a009e", + "name": "corp6-senior-devs-owner", + "parentId": "eca344cd-2b81-4447-bcf9-ce07aa9d4a1b", + "path": "/corp6-senior-devs/corp6-senior-devs-owner", + "realmRoles": [ + "owner" + ], + "subGroupCount": 0, + "subGroups": [] + } +] diff --git a/internal/keycloak/testdata/usergroups_children6.json b/internal/keycloak/testdata/usergroups_children6.json new file mode 100644 index 00000000..4dc8efd3 --- /dev/null +++ b/internal/keycloak/testdata/usergroups_children6.json @@ -0,0 +1,50 @@ +[ + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": { + "type": [ + "role-subgroup" + ] + }, + "clientRoles": {}, + "id": "60df5791-3676-42a4-9564-9dd7b09276b2", + "name": "project-a-website-for-cats-maintainer", + "parentId": "52c2e558-d939-4d76-b241-910386d59aa7", + "path": "/project-a-website-for-cats/project-a-website-for-cats-maintainer", + "realmRoles": [ + "maintainer" + ], + "subGroupCount": 0, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": { + "type": [ + "role-subgroup" + ] + }, + "clientRoles": {}, + "id": "5b5b78e1-43dd-4062-969a-159b60a0f063", + "name": "project-a-website-for-cats-owner", + "parentId": "52c2e558-d939-4d76-b241-910386d59aa7", + "path": "/project-a-website-for-cats/project-a-website-for-cats-owner", + "realmRoles": [ + "owner" + ], + "subGroupCount": 0, + "subGroups": [] + } +] diff --git a/internal/keycloak/testdata/usergroups_children7.json b/internal/keycloak/testdata/usergroups_children7.json new file mode 100644 index 00000000..19a73074 --- /dev/null +++ b/internal/keycloak/testdata/usergroups_children7.json @@ -0,0 +1,26 @@ +[ + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": { + "type": [ + "role-subgroup" + ] + }, + "clientRoles": {}, + "id": "e8574654-592b-4c09-8537-bab007a21fea", + "name": "scott-test-grandchild-group2b-owner", + "parentId": "c7d3b738-91f2-4cf1-aeec-2ab444eb3215", + "path": "/scott-test-ancestor-group2/scott-test-child-group2/scott-test-grandchild-group2b/scott-test-grandchild-group2b-owner", + "realmRoles": [ + "owner" + ], + "subGroupCount": 0, + "subGroups": [] + } +] diff --git a/internal/keycloak/testdata/usergroups_children8.json b/internal/keycloak/testdata/usergroups_children8.json new file mode 100644 index 00000000..5e6205ea --- /dev/null +++ b/internal/keycloak/testdata/usergroups_children8.json @@ -0,0 +1,26 @@ +[ + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "attributes": { + "type": [ + "role-subgroup" + ] + }, + "clientRoles": {}, + "id": "18c64f97-99c5-4a40-a660-0d626a4ce034", + "name": "scott-test-grandchild-group2-maintainer", + "parentId": "879d1d38-97d8-449a-affd-8529b8e31feb", + "path": "/scott-test-ancestor-group2/scott-test-child-group2/scott-test-grandchild-group2/scott-test-grandchild-group2-maintainer", + "realmRoles": [ + "maintainer" + ], + "subGroupCount": 0, + "subGroups": [] + } +] diff --git a/internal/keycloak/testdata/usergroups_groups_first0.json b/internal/keycloak/testdata/usergroups_groups_first0.json new file mode 100644 index 00000000..a2f086df --- /dev/null +++ b/internal/keycloak/testdata/usergroups_groups_first0.json @@ -0,0 +1,72 @@ +[ + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "3c1f5f78-3dba-44b9-94d1-27a0ca504238", + "name": "a", + "path": "/a", + "subGroupCount": 1, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "945d6316-aa1c-4d2f-94b9-eccb6ef3aa26", + "name": "awebsite-for-cats-devteam", + "path": "/awebsite-for-cats-devteam", + "subGroupCount": 2, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "f4d765e6-4d8e-4834-b8fc-9f89a19abcbf", + "name": "bigbus-contractors", + "path": "/bigbus-contractors", + "subGroupCount": 2, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "c46856da-bd2b-49f6-a943-d4d7a61f8cfb", + "name": "corp2-bigbus-contractors", + "path": "/corp2-bigbus-contractors", + "subGroupCount": 2, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "9ea08d19-0565-403b-9cde-8a3ed590dda7", + "name": "corp3-bigbus-contractors", + "path": "/corp3-bigbus-contractors", + "subGroupCount": 2, + "subGroups": [] + } +] diff --git a/internal/keycloak/testdata/usergroups_groups_first10.json b/internal/keycloak/testdata/usergroups_groups_first10.json new file mode 100644 index 00000000..96cf24c6 --- /dev/null +++ b/internal/keycloak/testdata/usergroups_groups_first10.json @@ -0,0 +1,72 @@ +[ + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "eca344cd-2b81-4447-bcf9-ce07aa9d4a1b", + "name": "corp6-senior-devs", + "path": "/corp6-senior-devs", + "subGroupCount": 3, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "3c7dea60-6dec-4f2d-b8ac-f28aa9e206d9", + "name": "scott-test-ancestor-group", + "path": "/scott-test-ancestor-group", + "subGroupCount": 2, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "ee6d02d1-b14b-41dd-95b6-cb8c26b1a321", + "name": "scott-test-ancestor-group2", + "path": "/scott-test-ancestor-group2", + "subGroupCount": 2, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "bd9d6c6c-7f46-4cb3-93ab-1023cb379d21", + "name": "scott-test-group", + "path": "/scott-test-group", + "subGroupCount": 0, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "796640a1-185c-4348-90ef-f776c9e828ed", + "name": "scott-test-group-empty", + "path": "/scott-test-group-empty", + "subGroupCount": 0, + "subGroups": [] + } +] diff --git a/internal/keycloak/testdata/usergroups_groups_first15.json b/internal/keycloak/testdata/usergroups_groups_first15.json new file mode 100644 index 00000000..eae37780 --- /dev/null +++ b/internal/keycloak/testdata/usergroups_groups_first15.json @@ -0,0 +1,72 @@ +[ + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "c97d32cf-2459-4ce3-8529-df2a223f6682", + "name": "scott-test-group2", + "path": "/scott-test-group2", + "subGroupCount": 0, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "d3fa3b78-1fe9-45c4-b735-4c07a41905eb", + "name": "test-group442", + "path": "/test-group442", + "subGroupCount": 0, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "54486df8-450d-4b62-8e10-223ac3419d05", + "name": "project-a-fishy-website", + "path": "/project-a-fishy-website", + "subGroupCount": 2, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "4dabd962-a772-4757-9de7-7245cc238086", + "name": "project-a-website-for-birds", + "path": "/project-a-website-for-birds", + "subGroupCount": 2, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "52c2e558-d939-4d76-b241-910386d59aa7", + "name": "project-a-website-for-cats", + "path": "/project-a-website-for-cats", + "subGroupCount": 2, + "subGroups": [] + } +] diff --git a/internal/keycloak/testdata/usergroups_groups_first20.json b/internal/keycloak/testdata/usergroups_groups_first20.json new file mode 100644 index 00000000..9fd36006 --- /dev/null +++ b/internal/keycloak/testdata/usergroups_groups_first20.json @@ -0,0 +1,44 @@ +[ + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "5005c22e-48c3-46cd-bf4a-393f6e13e9a8", + "name": "project-a-website-for-dogs", + "path": "/project-a-website-for-dogs", + "subGroupCount": 2, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "0472e029-e829-41ea-ac01-94891fd2332b", + "name": "project-a-website-for-ducks", + "path": "/project-a-website-for-ducks", + "subGroupCount": 2, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "1ad2cd0c-5046-434f-9580-7b036a08222b", + "name": "project-a-website-for-fish", + "path": "/project-a-website-for-fish", + "subGroupCount": 2, + "subGroups": [] + } +] diff --git a/internal/keycloak/testdata/usergroups_groups_first5.json b/internal/keycloak/testdata/usergroups_groups_first5.json new file mode 100644 index 00000000..c018d229 --- /dev/null +++ b/internal/keycloak/testdata/usergroups_groups_first5.json @@ -0,0 +1,72 @@ +[ + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "fe0fd424-a0fe-4796-9d4b-0c4b5a0b0f08", + "name": "corp4-bigbus-contractors", + "path": "/corp4-bigbus-contractors", + "subGroupCount": 2, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "754963d5-5154-410d-a0f4-ed5c029620a1", + "name": "corp5-bigbus-contractors", + "path": "/corp5-bigbus-contractors", + "subGroupCount": 2, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "7ffd7b50-fb33-487e-8331-d60925213f3d", + "name": "corp6-bigbus-contracts", + "path": "/corp6-bigbus-contracts", + "subGroupCount": 1, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "2b876213-78d8-4aff-8a92-ef3dacc8fc32", + "name": "corp6-devs-birds", + "path": "/corp6-devs-birds", + "subGroupCount": 2, + "subGroups": [] + }, + { + "access": { + "manage": true, + "manageMembers": true, + "manageMembership": true, + "view": true, + "viewMembers": true + }, + "id": "042a0523-5175-4b38-9fc9-55950afe5787", + "name": "corp6-devs-fish", + "path": "/corp6-devs-fish", + "subGroupCount": 1, + "subGroups": [] + } +] diff --git a/internal/keycloak/usergroups_test.go b/internal/keycloak/usergroups_test.go new file mode 100644 index 00000000..42756175 --- /dev/null +++ b/internal/keycloak/usergroups_test.go @@ -0,0 +1,217 @@ +package keycloak_test + +import ( + "bytes" + "context" + "fmt" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "os" + "testing" + + "github.com/alecthomas/assert/v2" + "github.com/google/uuid" + "github.com/uselagoon/ssh-portal/internal/keycloak" + "github.com/uselagoon/ssh-portal/internal/lagoon" +) + +// newTestUGIDRoleServer sets up a mock keycloak which responds with +// appropriate group JSON data to exercise UserGroupIDRole. +func newTestUGIDRoleServer(tt *testing.T) *httptest.Server { + // set up the map of group requests to responses + var reqRespMap map[string]string = map[string]string{ + "ee6d02d1-b14b-41dd-95b6-cb8c26b1a321/children": "testdata/usergroups_children0.json", + "7f22ce84-c0af-4ff4-afcd-288f0473deb5/children": "testdata/usergroups_children1.json", + "2e833d9b-39b7-4f25-b37f-cfb8765015ab/children": "testdata/usergroups_children2.json", + "139ad442-1d20-4c58-b009-c0afe21bf85b/children": "testdata/usergroups_children3.json", + "54486df8-450d-4b62-8e10-223ac3419d05/children": "testdata/usergroups_children4.json", + "eca344cd-2b81-4447-bcf9-ce07aa9d4a1b/children": "testdata/usergroups_children5.json", + "52c2e558-d939-4d76-b241-910386d59aa7/children": "testdata/usergroups_children6.json", + "c7d3b738-91f2-4cf1-aeec-2ab444eb3215/children": "testdata/usergroups_children7.json", + "879d1d38-97d8-449a-affd-8529b8e31feb/children": "testdata/usergroups_children8.json", + } + // load the discovery JSON first, because the mux closure needs to + // reference its buffer + discoveryBuf, err := os.ReadFile("testdata/realm.oidc.discovery.json") + if err != nil { + tt.Fatal(err) + return nil + } + // configure router with the URLs that OIDC discovery and JWKS require + mux := http.NewServeMux() + mux.HandleFunc("/auth/realms/lagoon/.well-known/openid-configuration", + func(w http.ResponseWriter, r *http.Request) { + d := bytes.NewBuffer(discoveryBuf) + _, err = io.Copy(w, d) + if err != nil { + tt.Fatal(err) + } + }) + mux.HandleFunc("/auth/realms/lagoon/protocol/openid-connect/certs", + func(w http.ResponseWriter, r *http.Request) { + f, err := os.Open("testdata/realm.oidc.certs.json") + if err != nil { + tt.Fatal(err) + return + } + _, err = io.Copy(w, f) + if err != nil { + tt.Fatal(err) + } + }) + // configure the group paths + for groupID, file := range reqRespMap { + mux.HandleFunc("/auth/admin/realms/lagoon/groups/"+groupID, + func(w http.ResponseWriter, r *http.Request) { + responseData, err := os.Open(file) + if err != nil { + tt.Fatal(err) + return + } + _, err = io.Copy(w, responseData) + if err != nil { + tt.Fatal(err) + } + }) + } + // configure the "all groups" paths + mux.HandleFunc("/auth/admin/realms/lagoon/groups", + func(w http.ResponseWriter, r *http.Request) { + paramFirst := r.URL.Query().Get("first") + paramMax := r.URL.Query().Get("max") + assert.Equal(tt, "5", paramMax) + dataPath := + fmt.Sprintf("testdata/usergroups_groups_first%s.json", paramFirst) + f, err := os.Open(dataPath) + if err != nil { + tt.Fatal(err) + return + } + _, err = io.Copy(w, f) + if err != nil { + tt.Fatal(err) + } + }) + ts := httptest.NewServer(mux) + // now replace the example URL in the discovery JSON with the actual + // httptest server URL + discoveryBuf = bytes.ReplaceAll(discoveryBuf, + []byte("https://keycloak.example.com"), []byte(ts.URL)) + return ts +} + +func TestUserGroupIDRole(t *testing.T) { + var testCases = map[string]struct { + userGroupPaths []string + expect map[uuid.UUID]lagoon.UserRole + }{ + "single project owner": { + userGroupPaths: []string{ + "/project-a-fishy-website/project-a-fishy-website-owner", + }, + expect: map[uuid.UUID]lagoon.UserRole{ + uuid.MustParse("54486df8-450d-4b62-8e10-223ac3419d05"): lagoon.Owner, + }, + }, + "multi project member": { + userGroupPaths: []string{ + "/project-a-fishy-website/project-a-fishy-website-owner", + "/project-a-website-for-cats/project-a-website-for-cats-maintainer", + }, + expect: map[uuid.UUID]lagoon.UserRole{ + uuid.MustParse("54486df8-450d-4b62-8e10-223ac3419d05"): lagoon.Owner, + uuid.MustParse("52c2e558-d939-4d76-b241-910386d59aa7"): lagoon.Maintainer, + }, + }, + "regular group maintainer": { + userGroupPaths: []string{ + "/corp6-senior-devs/corp6-senior-devs-maintainer", + }, + expect: map[uuid.UUID]lagoon.UserRole{ + uuid.MustParse("eca344cd-2b81-4447-bcf9-ce07aa9d4a1b"): lagoon.Maintainer, + }, + }, + "child subgroup developer": { + userGroupPaths: []string{ + "/scott-test-ancestor-group2/scott-test-child-group2/scott-test-child-group2-developer", + }, + expect: map[uuid.UUID]lagoon.UserRole{ + uuid.MustParse("2e833d9b-39b7-4f25-b37f-cfb8765015ab"): lagoon.Developer, + }, + }, + "grandchild subgroup owner": { + userGroupPaths: []string{ + "/scott-test-ancestor-group2/scott-test-child-group3/scott-test-grandchild-group3/scott-test-grandchild-group3-owner", + }, + expect: map[uuid.UUID]lagoon.UserRole{ + uuid.MustParse("139ad442-1d20-4c58-b009-c0afe21bf85b"): lagoon.Owner, + }, + }, + "multiple grandchild subgroups exercise cache": { + userGroupPaths: []string{ + "/scott-test-ancestor-group2/scott-test-child-group2/scott-test-grandchild-group2/scott-test-grandchild-group2-maintainer", + "/scott-test-ancestor-group2/scott-test-child-group2/scott-test-grandchild-group2b/scott-test-grandchild-group2b-owner", + }, + expect: map[uuid.UUID]lagoon.UserRole{ + uuid.MustParse("879d1d38-97d8-449a-affd-8529b8e31feb"): lagoon.Maintainer, + uuid.MustParse("c7d3b738-91f2-4cf1-aeec-2ab444eb3215"): lagoon.Owner, + }, + }, + "project, regular, and subgroups": { + userGroupPaths: []string{ + "/project-a-fishy-website/project-a-fishy-website-owner", + "/corp6-senior-devs/corp6-senior-devs-maintainer", + "/scott-test-ancestor-group2/scott-test-child-group2/scott-test-child-group2-developer", + }, + expect: map[uuid.UUID]lagoon.UserRole{ + uuid.MustParse("54486df8-450d-4b62-8e10-223ac3419d05"): lagoon.Owner, + uuid.MustParse("eca344cd-2b81-4447-bcf9-ce07aa9d4a1b"): lagoon.Maintainer, + uuid.MustParse("2e833d9b-39b7-4f25-b37f-cfb8765015ab"): lagoon.Developer, + }, + }, + "multiple roles in the same group highest first": { + userGroupPaths: []string{ + "/corp6-senior-devs/corp6-senior-devs-maintainer", + "/corp6-senior-devs/corp6-senior-devs-developer", + }, + expect: map[uuid.UUID]lagoon.UserRole{ + uuid.MustParse("eca344cd-2b81-4447-bcf9-ce07aa9d4a1b"): lagoon.Maintainer, + }, + }, + "multiple roles in the same group lowest first": { + userGroupPaths: []string{ + "/corp6-senior-devs/corp6-senior-devs-developer", + "/corp6-senior-devs/corp6-senior-devs-maintainer", + }, + expect: map[uuid.UUID]lagoon.UserRole{ + uuid.MustParse("eca344cd-2b81-4447-bcf9-ce07aa9d4a1b"): lagoon.Maintainer, + }, + }, + } + for name, tc := range testCases { + t.Run(name, func(tt *testing.T) { + ts := newTestUGIDRoleServer(tt) + defer ts.Close() + // init keycloak client + k, err := keycloak.NewClient( + context.Background(), + slog.New(slog.NewJSONHandler(os.Stderr, nil)), + ts.URL, + "auth-server", + "", + 10) + if err != nil { + tt.Fatal(err) + } + // override internal HTTP client for testing + k.UseDefaultHTTPClient() + // override default huge pages + k.UsePageSize(5) + // perform testing + gidRoleMap := k.UserGroupIDRole(context.Background(), tc.userGroupPaths) + assert.Equal(tt, tc.expect, gidRoleMap, name) + }) + } +} From 1e7f618756d2967c5862569e8006384cc917f773 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Tue, 8 Oct 2024 14:23:19 +0800 Subject: [PATCH 7/7] chore: add lagoondb ProjectGroupIDs tests --- internal/lagoondb/client_test.go | 57 +++++++++++++++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/internal/lagoondb/client_test.go b/internal/lagoondb/client_test.go index 1b74cf93..1fe3c742 100644 --- a/internal/lagoondb/client_test.go +++ b/internal/lagoondb/client_test.go @@ -49,7 +49,8 @@ func TestLastUsed(t *testing.T) { } else { assert.NoError(tt, err, name) } - // check expectations + // expect an error here in the expectError case because WithArgs has not + // be called with the expected value. err = mock.ExpectationsWereMet() if tc.expectError { assert.Error(tt, err, name) @@ -59,3 +60,57 @@ func TestLastUsed(t *testing.T) { }) } } + +func TestProjectGroupIDs(t *testing.T) { + var testCases = map[string]struct { + projectID int + expectError bool + rows *sqlmock.Rows + error error + }{ + "single-group project": { + projectID: 12, + expectError: false, + rows: sqlmock.NewRows([]string{"group_id"}). + AddRow("d79a42a6-a5b0-4d37-a1dd-44c2b1f6fddc"), + }, + "multi-group project": { + projectID: 12, + expectError: false, + rows: sqlmock.NewRows([]string{"group_id"}). + AddRow("486765ce-14ec-4ad8-a454-e026b8cc52a4"). + AddRow("d79a42a6-a5b0-4d37-a1dd-44c2b1f6fddc"), + }, + "no results": { + projectID: 12, + expectError: true, + rows: sqlmock.NewRows([]string{"group_id"}), + error: lagoondb.ErrNoResult, + }, + } + for name, tc := range testCases { + t.Run(name, func(tt *testing.T) { + // set up mocks + mockDB, mock, err := sqlmock.New() + assert.NoError(tt, err, name) + mock.ExpectQuery( + `SELECT group_id ` + + `FROM kc_group_projects ` + + `WHERE project_id = (.+)`). + WithArgs(tc.projectID). + WillReturnRows(tc.rows). + WillReturnError(tc.error) + // execute expected database operations + db := lagoondb.NewClientFromDB(mockDB) + _, err = db.ProjectGroupIDs(context.Background(), tc.projectID) + if tc.expectError { + assert.Error(tt, err, name) + } else { + assert.NoError(tt, err, name) + } + // check expectations + err = mock.ExpectationsWereMet() + assert.NoError(tt, err, name) + }) + } +}