diff --git a/config/config.go b/config/config.go index 368663cc..2d0cbac6 100644 --- a/config/config.go +++ b/config/config.go @@ -99,6 +99,7 @@ var ( RedirectBaseURL: "", UsernameField: "", RoleField: "", + ImplicitRoles: false, CustomFields: []string{}, }, Security: httpd.SecurityConf{ @@ -1332,6 +1333,12 @@ func getHTTPDOIDCFromEnv(idx int) (httpd.OIDC, bool) { isSet = true } + implicitRoles, ok := lookupBoolFromEnv(fmt.Sprintf("SFTPGO_HTTPD__BINDINGS__%v__OIDC__IMPLICIT_ROLES", idx)) + if ok { + result.ImplicitRoles = implicitRoles + isSet = true + } + customFields, ok := lookupStringListFromEnv(fmt.Sprintf("SFTPGO_HTTPD__BINDINGS__%v__OIDC__CUSTOM_FIELDS", idx)) if ok { result.CustomFields = customFields diff --git a/config/config_test.go b/config/config_test.go index 07924e6d..0ca24106 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -928,6 +928,7 @@ func TestHTTPDBindingsFromEnv(t *testing.T) { os.Setenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__REDIRECT_BASE_URL", "redirect base url") os.Setenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__USERNAME_FIELD", "preferred_username") os.Setenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__ROLE_FIELD", "sftpgo_role") + os.Setenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__IMPLICIT_ROLES", "1") os.Setenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__CUSTOM_FIELDS", "field1,field2") os.Setenv("SFTPGO_HTTPD__BINDINGS__2__SECURITY__ENABLED", "true") os.Setenv("SFTPGO_HTTPD__BINDINGS__2__SECURITY__ALLOWED_HOSTS", "*.example.com,*.example.net") @@ -989,6 +990,7 @@ func TestHTTPDBindingsFromEnv(t *testing.T) { os.Unsetenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__REDIRECT_BASE_URL") os.Unsetenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__USERNAME_FIELD") os.Unsetenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__ROLE_FIELD") + os.Unsetenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__IMPLICIT_ROLES") os.Unsetenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__CUSTOM_FIELDS") os.Unsetenv("SFTPGO_HTTPD__BINDINGS__2__SECURITY__ENABLED") os.Unsetenv("SFTPGO_HTTPD__BINDINGS__2__SECURITY__ALLOWED_HOSTS") @@ -1073,6 +1075,7 @@ func TestHTTPDBindingsFromEnv(t *testing.T) { require.Equal(t, "redirect base url", bindings[2].OIDC.RedirectBaseURL) require.Equal(t, "preferred_username", bindings[2].OIDC.UsernameField) require.Equal(t, "sftpgo_role", bindings[2].OIDC.RoleField) + require.True(t, bindings[2].OIDC.ImplicitRoles) require.Len(t, bindings[2].OIDC.CustomFields, 2) require.Equal(t, "field1", bindings[2].OIDC.CustomFields[0]) require.Equal(t, "field2", bindings[2].OIDC.CustomFields[1]) diff --git a/docs/full-configuration.md b/docs/full-configuration.md index 90fc4f39..96ee03f0 100644 --- a/docs/full-configuration.md +++ b/docs/full-configuration.md @@ -261,6 +261,7 @@ The configuration file contains the following sections: - `redirect_base_url`, string. Defines the base URL to redirect to after OpenID authentication. The suffix `/web/oidc/redirect` will be added to this base URL, adding also the `web_root` if configured. Default: blank. - `username_field`, string. Defines the ID token claims field to map to the SFTPGo username. Default: blank. - `role_field`, string. Defines the optional ID token claims field to map to a SFTPGo role. If the defined ID token claims field is set to `admin` the authenticated user is mapped to an SFTPGo admin. You don't need to specify this field if you want to use OpenID only for the Web Client UI. Default: blank. + - `implicit_roles`, boolean. If set, the `role_field` is ignored and the SFTPGo role is assumed based on the login link used. Default: `false`. - `custom_fields`, list of strings. Custom token claims fields to pass to the pre-login hook. Default: empty. - `security`, struct. Defines security headers to add to HTTP responses and allows to restrict allowed hosts. The following parameters are supported: - `enabled`, boolean. Set to `true` to enable security configurations. Default: `false`. diff --git a/docs/oidc.md b/docs/oidc.md index 1b3cb6d7..f58ef7f6 100644 --- a/docs/oidc.md +++ b/docs/oidc.md @@ -43,6 +43,7 @@ Add the following configuration parameters to the SFTPGo configuration file (or "redirect_base_url": "http://192.168.1.50:8080", "username_field": "preferred_username", "role_field": "sftpgo_role", + "implicit_roles": false, "custom_fields": [] } ... @@ -51,6 +52,7 @@ Add the following configuration parameters to the SFTPGo configuration file (or From SFTPGo login page click `Login with OpenID` button, you will be redirected to the Keycloak login page, after a successful authentication Keyclock will redirect back to SFTPGo Web Admin or SFTPGo Web Client. Please note that the ID token returned from Keycloak must contain the `username_field` specified in the SFTPGo configuration and optionally the `role_field`. The mapped usernames must exist in SFTPGo. +If you don't want to explicitly define SFTPGo roles in your identity provider, you can set `implicit_roles` to `true`. With this configuration, the SFTPGo role is assumed based on the login link used. Here is an example ID token which allows the SFTPGo admin `root` to access to the Web Admin UI. diff --git a/httpd/oidc.go b/httpd/oidc.go index eec7bd35..ad3d026d 100644 --- a/httpd/oidc.go +++ b/httpd/oidc.go @@ -22,6 +22,7 @@ import ( const ( oidcCookieKey = "oidc" + adminRoleFieldValue = "admin" authStateValidity = 1 * 60 * 1000 // 1 minute tokenUpdateInterval = 3 * 60 * 1000 // 3 minutes tokenDeleteInterval = 2 * 3600 * 1000 // 2 hours @@ -66,6 +67,9 @@ type OIDC struct { // You don't need to specify this field if you want to use OpenID only for the // Web Client UI RoleField string `json:"role_field" mapstructure:"role_field"` + // If set, the `RoleField` is ignored and the SFTPGo role is assumed based on + // the login link used + ImplicitRoles bool `json:"implicit_roles" mapstructure:"implicit_roles"` // Custom token claims fields to pass to the pre-login hook CustomFields []string `json:"custom_fields" mapstructure:"custom_fields"` provider *oidc.Provider @@ -79,7 +83,17 @@ func (o *OIDC) isEnabled() bool { } func (o *OIDC) hasRoles() bool { - return o.isEnabled() && o.RoleField != "" + return o.isEnabled() && (o.RoleField != "" || o.ImplicitRoles) +} + +func (o *OIDC) getForcedRole(audience string) string { + if !o.ImplicitRoles { + return "" + } + if audience == tokenAudienceWebAdmin { + return adminRoleFieldValue + } + return "" } func (o *OIDC) getRedirectURL() string { @@ -167,7 +181,9 @@ type oidcToken struct { UsedAt int64 `json:"used_at"` } -func (t *oidcToken) parseClaims(claims map[string]any, usernameField, roleField string, customFields []string) error { +func (t *oidcToken) parseClaims(claims map[string]any, usernameField, roleField string, customFields []string, + forcedRole string, +) error { getClaimsFields := func() []string { keys := make([]string, 0, len(claims)) for k := range claims { @@ -182,10 +198,14 @@ func (t *oidcToken) parseClaims(claims map[string]any, usernameField, roleField return errors.New("no username field") } t.Username = username - if roleField != "" { - role, ok := claims[roleField] - if ok { - t.Role = role + if forcedRole != "" { + t.Role = forcedRole + } else { + if roleField != "" { + role, ok := claims[roleField] + if ok { + t.Role = role + } } } t.CustomFields = nil @@ -213,10 +233,10 @@ func (t *oidcToken) parseClaims(claims map[string]any, usernameField, roleField func (t *oidcToken) isAdmin() bool { switch v := t.Role.(type) { case string: - return v == "admin" + return v == adminRoleFieldValue case []any: for _, s := range v { - if val, ok := s.(string); ok && val == "admin" { + if val, ok := s.(string); ok && val == adminRoleFieldValue { return true } } @@ -511,7 +531,9 @@ func (s *httpdServer) handleOIDCRedirect(w http.ResponseWriter, r *http.Request) if !oauth2Token.Expiry.IsZero() { token.ExpiresAt = util.GetTimeAsMsSinceEpoch(oauth2Token.Expiry) } - if err = token.parseClaims(claims, s.binding.OIDC.UsernameField, s.binding.OIDC.RoleField, s.binding.OIDC.CustomFields); err != nil { + err = token.parseClaims(claims, s.binding.OIDC.UsernameField, s.binding.OIDC.RoleField, + s.binding.OIDC.CustomFields, s.binding.OIDC.getForcedRole(authReq.Audience)) + if err != nil { logger.Debug(logSender, "", "unable to parse oidc token claims: %v", err) setFlashMessage(w, r, fmt.Sprintf("Unable to parse OpenID token claims: %v", err)) doRedirect() diff --git a/httpd/oidc_test.go b/httpd/oidc_test.go index 49832887..6873dfcb 100644 --- a/httpd/oidc_test.go +++ b/httpd/oidc_test.go @@ -793,6 +793,129 @@ func TestOIDCToken(t *testing.T) { assert.NoError(t, err) } +func TestOIDCImplicitRoles(t *testing.T) { + oidcMgr, ok := oidcMgr.(*memoryOIDCManager) + require.True(t, ok) + + server := getTestOIDCServer() + server.binding.OIDC.ImplicitRoles = true + err := server.binding.OIDC.initialize() + assert.NoError(t, err) + server.initializeRouter() + + authReq := newOIDCPendingAuth(tokenAudienceWebAdmin) + oidcMgr.addPendingAuth(authReq) + token := &oauth2.Token{ + AccessToken: "1234", + Expiry: time.Now().Add(5 * time.Minute), + } + token = token.WithExtra(map[string]any{ + "id_token": "id_token_val", + }) + server.binding.OIDC.oauth2Config = &mockOAuth2Config{ + tokenSource: &mockTokenSource{}, + authCodeURL: webOIDCRedirectPath, + token: token, + } + idToken := &oidc.IDToken{ + Nonce: authReq.Nonce, + Expiry: time.Now().Add(5 * time.Minute), + } + setIDTokenClaims(idToken, []byte(`{"preferred_username":"admin","sid":"sid456"}`)) + server.binding.OIDC.verifier = &mockOIDCVerifier{ + err: nil, + token: idToken, + } + rr := httptest.NewRecorder() + r, err := http.NewRequest(http.MethodGet, webOIDCRedirectPath+"?state="+authReq.State, nil) + assert.NoError(t, err) + server.router.ServeHTTP(rr, r) + assert.Equal(t, http.StatusFound, rr.Code) + assert.Equal(t, webUsersPath, rr.Header().Get("Location")) + require.Len(t, oidcMgr.pendingAuths, 0) + require.Len(t, oidcMgr.tokens, 1) + var tokenCookie string + for k := range oidcMgr.tokens { + tokenCookie = k + } + // Web Client is not available with an admin token + rr = httptest.NewRecorder() + r, err = http.NewRequest(http.MethodGet, webClientFilesPath, nil) + assert.NoError(t, err) + r.Header.Set("Cookie", fmt.Sprintf("%v=%v", oidcCookieKey, tokenCookie)) + server.router.ServeHTTP(rr, r) + assert.Equal(t, http.StatusFound, rr.Code) + assert.Equal(t, webClientLoginPath, rr.Header().Get("Location")) + // logout the admin user + rr = httptest.NewRecorder() + r, err = http.NewRequest(http.MethodGet, webLogoutPath, nil) + assert.NoError(t, err) + r.Header.Set("Cookie", fmt.Sprintf("%v=%v", oidcCookieKey, tokenCookie)) + server.router.ServeHTTP(rr, r) + assert.Equal(t, http.StatusFound, rr.Code) + assert.Equal(t, webAdminLoginPath, rr.Header().Get("Location")) + require.Len(t, oidcMgr.pendingAuths, 0) + require.Len(t, oidcMgr.tokens, 0) + // now login and logout a user + username := "test_oidc_implicit_user" + user := dataprovider.User{ + BaseUser: sdk.BaseUser{ + Username: username, + Password: "pwd", + HomeDir: filepath.Join(os.TempDir(), username), + Status: 1, + Permissions: map[string][]string{ + "/": {dataprovider.PermAny}, + }, + }, + Filters: dataprovider.UserFilters{ + BaseUserFilters: sdk.BaseUserFilters{ + WebClient: []string{sdk.WebClientSharesDisabled}, + }, + }, + } + err = dataprovider.AddUser(&user, "", "") + assert.NoError(t, err) + + authReq = newOIDCPendingAuth(tokenAudienceWebClient) + oidcMgr.addPendingAuth(authReq) + idToken = &oidc.IDToken{ + Nonce: authReq.Nonce, + Expiry: time.Now().Add(5 * time.Minute), + } + setIDTokenClaims(idToken, []byte(`{"preferred_username":"test_oidc_implicit_user"}`)) + server.binding.OIDC.verifier = &mockOIDCVerifier{ + err: nil, + token: idToken, + } + rr = httptest.NewRecorder() + r, err = http.NewRequest(http.MethodGet, webOIDCRedirectPath+"?state="+authReq.State, nil) + assert.NoError(t, err) + server.router.ServeHTTP(rr, r) + assert.Equal(t, http.StatusFound, rr.Code) + assert.Equal(t, webClientFilesPath, rr.Header().Get("Location")) + require.Len(t, oidcMgr.pendingAuths, 0) + require.Len(t, oidcMgr.tokens, 1) + for k := range oidcMgr.tokens { + tokenCookie = k + } + + rr = httptest.NewRecorder() + r, err = http.NewRequest(http.MethodGet, webClientLogoutPath, nil) + assert.NoError(t, err) + r.Header.Set("Cookie", fmt.Sprintf("%v=%v", oidcCookieKey, tokenCookie)) + server.router.ServeHTTP(rr, r) + assert.Equal(t, http.StatusFound, rr.Code) + assert.Equal(t, webClientLoginPath, rr.Header().Get("Location")) + require.Len(t, oidcMgr.pendingAuths, 0) + require.Len(t, oidcMgr.tokens, 0) + + err = os.RemoveAll(user.GetHomeDir()) + assert.NoError(t, err) + err = dataprovider.DeleteUser(username, "", "") + assert.NoError(t, err) +} + func TestMemoryOIDCManager(t *testing.T) { oidcMgr, ok := oidcMgr.(*memoryOIDCManager) require.True(t, ok) @@ -1139,6 +1262,7 @@ func getTestOIDCServer() *httpdServer { RedirectBaseURL: "http://127.0.0.1:8081/", UsernameField: "preferred_username", RoleField: "sftpgo_role", + ImplicitRoles: false, CustomFields: nil, }, }, diff --git a/sftpgo.json b/sftpgo.json index f4c2114e..19ac7b17 100644 --- a/sftpgo.json +++ b/sftpgo.json @@ -245,6 +245,7 @@ "redirect_base_url": "", "username_field": "", "role_field": "", + "implicit_roles": false, "custom_fields": [] }, "security": { diff --git a/templates/webadmin/groups.html b/templates/webadmin/groups.html index 761700c1..e8d061b9 100644 --- a/templates/webadmin/groups.html +++ b/templates/webadmin/groups.html @@ -30,7 +30,7 @@