From 9bf448be7aa5e2468d5a6302d7c37ebad0f84176 Mon Sep 17 00:00:00 2001 From: 9p4 Date: Tue, 27 Feb 2024 10:07:29 -0500 Subject: [PATCH] [feature/oidc] Add support for very basic RBAC (#2642) * Add support for very basic RBAC * Add some small tests for allowedGroup and adminGroup * Switch to table-driven tests --- docs/configuration/oidc.md | 6 ++++ example/config.yaml | 6 ++++ internal/api/auth/callback.go | 48 +++++++++++++++++++++++++----- internal/api/auth/callback_test.go | 45 ++++++++++++++++++++++++++++ internal/config/config.go | 1 + internal/config/helpers.gen.go | 25 ++++++++++++++++ test/envparsing.sh | 4 +++ testrig/config.go | 2 ++ 8 files changed, 130 insertions(+), 7 deletions(-) create mode 100644 internal/api/auth/callback_test.go diff --git a/docs/configuration/oidc.md b/docs/configuration/oidc.md index b30cd8410..482c0fa3f 100644 --- a/docs/configuration/oidc.md +++ b/docs/configuration/oidc.md @@ -79,6 +79,12 @@ oidc-scopes: # Default: false oidc-link-existing: false +# Array of string. If the returned ID token contains a 'groups' claim that matches one of the +# groups in oidc-allowed-groups, then this user will be granted access on the GtS instance. If the array is empty, +# then all groups will be granted permission. +# Default: [] +oidc-allowed-groups: [] + # Array of string. If the returned ID token contains a 'groups' claim that matches one of the # groups in oidc-admin-groups, then this user will be granted admin rights on the GtS instance # Default: [] diff --git a/example/config.yaml b/example/config.yaml index 1073c656b..bdc09da79 100644 --- a/example/config.yaml +++ b/example/config.yaml @@ -729,6 +729,12 @@ oidc-scopes: # Default: false oidc-link-existing: false +# Array of string. If the returned ID token contains a 'groups' claim that matches one of the +# groups in oidc-allowed-groups, then this user will be granted access on the GtS instance. If the array is empty, +# then all groups will be granted permission. +# Default: [] +oidc-allowed-groups: [] + # Array of string. If the returned ID token contains a 'groups' claim that matches one of the # groups in oidc-admin-groups, then this user will be granted admin rights on the GtS instance # Default: [] diff --git a/internal/api/auth/callback.go b/internal/api/auth/callback.go index d0fa78322..37c257229 100644 --- a/internal/api/auth/callback.go +++ b/internal/api/auth/callback.go @@ -23,6 +23,7 @@ import ( "fmt" "net" "net/http" + "slices" "strings" "github.com/gin-contrib/sessions" @@ -156,6 +157,14 @@ func (m *Module) CallbackGETHandler(c *gin.Context) { apiutil.TemplateWebPage(c, page) return } + + // Check user permissions on login + if !allowedGroup(claims.Groups) { + err := fmt.Errorf("User groups %+v do not include an allowed group", claims.Groups) + apiutil.ErrorHandler(c, gtserror.NewErrorUnauthorized(err, err.Error()), m.processor.InstanceGetV1) + return + } + s.Set(sessionUserID, user.ID) if err := s.Save(); err != nil { m.clearSession(s) @@ -297,6 +306,11 @@ func (m *Module) createUserFromOIDC(ctx context.Context, claims *oidc.Claims, ex return nil, gtserror.NewErrorConflict(err, help) } + if !allowedGroup(claims.Groups) { + err := fmt.Errorf("User groups %+v do not include an allowed group", claims.Groups) + return nil, gtserror.NewErrorUnauthorized(err, err.Error()) + } + // We still need to set something as a password, even // if it's not a password the user will end up using. // @@ -356,13 +370,12 @@ func (m *Module) createUserFromOIDC(ctx context.Context, claims *oidc.Claims, ex // adminGroup returns true if one of the given OIDC // groups is equal to at least one admin OIDC group. func adminGroup(groups []string) bool { - for _, ag := range config.GetOIDCAdminGroups() { - for _, g := range groups { - if strings.EqualFold(ag, g) { - // This is an admin group, - // ∴ user is an admin. - return true - } + adminGroups := config.GetOIDCAdminGroups() + for _, claimedGroup := range groups { + if slices.ContainsFunc(adminGroups, func(allowedGroup string) bool { + return strings.EqualFold(claimedGroup, allowedGroup) + }) { + return true } } @@ -370,3 +383,24 @@ func adminGroup(groups []string) bool { // ∴ user is not an admin. return false } + +// allowedGroup returns true if one of the given OIDC +// groups is equal to at least one allowed OIDC group. +func allowedGroup(groups []string) bool { + allowedGroups := config.GetOIDCAllowedGroups() + if len(allowedGroups) == 0 { + // If no groups are configured, allow access (for backwards compatibility) + return true + } + for _, claimedGroup := range groups { + if slices.ContainsFunc(allowedGroups, func(allowedGroup string) bool { + return strings.EqualFold(claimedGroup, allowedGroup) + }) { + return true + } + } + + // User is in no allowed groups, + // ∴ user is not allowed to log in + return false +} diff --git a/internal/api/auth/callback_test.go b/internal/api/auth/callback_test.go new file mode 100644 index 000000000..2624f3f3f --- /dev/null +++ b/internal/api/auth/callback_test.go @@ -0,0 +1,45 @@ +package auth + +import ( + "testing" + + "github.com/superseriousbusiness/gotosocial/testrig" +) + +func TestAdminGroup(t *testing.T) { + testrig.InitTestConfig() + for _, test := range []struct { + name string + groups []string + expected bool + }{ + {name: "not in admin group", groups: []string{"group1", "group2", "allowedRole"}, expected: false}, + {name: "in admin group", groups: []string{"group1", "group2", "adminRole"}, expected: true}, + } { + test := test // loopvar capture + t.Run(test.name, func(t *testing.T) { + if got := adminGroup(test.groups); got != test.expected { + t.Fatalf("got: %t, wanted: %t", got, test.expected) + } + }) + } +} + +func TestAllowedGroup(t *testing.T) { + testrig.InitTestConfig() + for _, test := range []struct { + name string + groups []string + expected bool + }{ + {name: "not in allowed group", groups: []string{"group1", "group2", "adminRole"}, expected: false}, + {name: "in allowed group", groups: []string{"group1", "group2", "allowedRole"}, expected: true}, + } { + test := test // loopvar capture + t.Run(test.name, func(t *testing.T) { + if got := allowedGroup(test.groups); got != test.expected { + t.Fatalf("got: %t, wanted: %t", got, test.expected) + } + }) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index 292b6b7ee..c810222a1 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -133,6 +133,7 @@ type Configuration struct { OIDCClientSecret string `name:"oidc-client-secret" usage:"ClientSecret of GoToSocial, as registered with the OIDC provider."` OIDCScopes []string `name:"oidc-scopes" usage:"OIDC scopes."` OIDCLinkExisting bool `name:"oidc-link-existing" usage:"link existing user accounts to OIDC logins based on the stored email value"` + OIDCAllowedGroups []string `name:"oidc-allowed-groups" usage:"Membership of one of the listed groups allows access to GtS. If this is empty, all groups are allowed."` OIDCAdminGroups []string `name:"oidc-admin-groups" usage:"Membership of one of the listed groups makes someone a GtS admin"` TracingEnabled bool `name:"tracing-enabled" usage:"Enable OTLP Tracing"` diff --git a/internal/config/helpers.gen.go b/internal/config/helpers.gen.go index 9549d67c1..f458074b1 100644 --- a/internal/config/helpers.gen.go +++ b/internal/config/helpers.gen.go @@ -1975,6 +1975,31 @@ func GetOIDCLinkExisting() bool { return global.GetOIDCLinkExisting() } // SetOIDCLinkExisting safely sets the value for global configuration 'OIDCLinkExisting' field func SetOIDCLinkExisting(v bool) { global.SetOIDCLinkExisting(v) } +// GetOIDCAllowedGroups safely fetches the Configuration value for state's 'OIDCAllowedGroups' field +func (st *ConfigState) GetOIDCAllowedGroups() (v []string) { + st.mutex.RLock() + v = st.config.OIDCAllowedGroups + st.mutex.RUnlock() + return +} + +// SetOIDCAllowedGroups safely sets the Configuration value for state's 'OIDCAllowedGroups' field +func (st *ConfigState) SetOIDCAllowedGroups(v []string) { + st.mutex.Lock() + defer st.mutex.Unlock() + st.config.OIDCAllowedGroups = v + st.reloadToViper() +} + +// OIDCAllowedGroupsFlag returns the flag name for the 'OIDCAllowedGroups' field +func OIDCAllowedGroupsFlag() string { return "oidc-allowed-groups" } + +// GetOIDCAllowedGroups safely fetches the value for global configuration 'OIDCAllowedGroups' field +func GetOIDCAllowedGroups() []string { return global.GetOIDCAllowedGroups() } + +// SetOIDCAllowedGroups safely sets the value for global configuration 'OIDCAllowedGroups' field +func SetOIDCAllowedGroups(v []string) { global.SetOIDCAllowedGroups(v) } + // GetOIDCAdminGroups safely fetches the Configuration value for state's 'OIDCAdminGroups' field func (st *ConfigState) GetOIDCAdminGroups() (v []string) { st.mutex.RLock() diff --git a/test/envparsing.sh b/test/envparsing.sh index 272046214..90a5e62c9 100755 --- a/test/envparsing.sh +++ b/test/envparsing.sh @@ -119,6 +119,9 @@ EXPECT=$(cat << "EOF" "oidc-admin-groups": [ "steamy" ], + "oidc-allowed-groups": [ + "sloths" + ], "oidc-client-id": "1234", "oidc-client-secret": "shhhh its a secret", "oidc-enabled": true, @@ -252,6 +255,7 @@ GTS_OIDC_CLIENT_ID='1234' \ GTS_OIDC_CLIENT_SECRET='shhhh its a secret' \ GTS_OIDC_SCOPES='read,write' \ GTS_OIDC_LINK_EXISTING=true \ +GTS_OIDC_ALLOWED_GROUPS='sloths' \ GTS_OIDC_ADMIN_GROUPS='steamy' \ GTS_SMTP_HOST='example.com' \ GTS_SMTP_PORT=4269 \ diff --git a/testrig/config.go b/testrig/config.go index f8330ac14..5dbacc155 100644 --- a/testrig/config.go +++ b/testrig/config.go @@ -119,6 +119,8 @@ var testDefaults = config.Configuration{ OIDCClientSecret: "", OIDCScopes: []string{oidc.ScopeOpenID, "profile", "email", "groups"}, OIDCLinkExisting: false, + OIDCAdminGroups: []string{"adminRole"}, + OIDCAllowedGroups: []string{"allowedRole"}, SMTPHost: "", SMTPPort: 0,