diff --git a/config/config.go b/config/config.go index b16d8903..01a2287c 100644 --- a/config/config.go +++ b/config/config.go @@ -105,6 +105,7 @@ var ( UsernameField: "", RoleField: "", ImplicitRoles: false, + Scopes: []string{"openid", "profile", "email"}, CustomFields: []string{}, }, Security: httpd.SecurityConf{ @@ -1337,7 +1338,10 @@ func getHTTPDSecurityConfFromEnv(idx int) (httpd.SecurityConf, bool) { //nolint: } func getHTTPDOIDCFromEnv(idx int) (httpd.OIDC, bool) { - var result httpd.OIDC + result := defaultHTTPDBinding.OIDC + if len(globalConf.HTTPDConfig.Bindings) > idx { + result = globalConf.HTTPDConfig.Bindings[idx].OIDC + } isSet := false clientID, ok := os.LookupEnv(fmt.Sprintf("SFTPGO_HTTPD__BINDINGS__%v__OIDC__CLIENT_ID", idx)) @@ -1503,12 +1507,7 @@ func getHTTPDWebClientIntegrationsFromEnv(idx int) []httpd.WebClientIntegration } func getDefaultHTTPBinding(idx int) httpd.Binding { - binding := httpd.Binding{ - EnableWebAdmin: true, - EnableWebClient: true, - RenderOpenAPI: true, - MinTLSVersion: 12, - } + binding := defaultHTTPDBinding if len(globalConf.HTTPDConfig.Bindings) > idx { binding = globalConf.HTTPDConfig.Bindings[idx] } diff --git a/config/config_test.go b/config/config_test.go index 3322b9f4..59c295e1 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -915,6 +915,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__SCOPES", "openid") 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") @@ -979,6 +980,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__SCOPES") 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") @@ -1028,6 +1030,7 @@ func TestHTTPDBindingsFromEnv(t *testing.T) { require.Equal(t, 0, bindings[0].HideLoginURL) require.False(t, bindings[0].Security.Enabled) require.Equal(t, 0, bindings[0].ClientIPHeaderDepth) + require.Len(t, bindings[0].OIDC.Scopes, 3) require.Equal(t, 8000, bindings[1].Port) require.Equal(t, "127.0.0.1", bindings[1].Address) require.False(t, bindings[1].EnableHTTPS) @@ -1038,6 +1041,7 @@ func TestHTTPDBindingsFromEnv(t *testing.T) { require.Nil(t, bindings[1].TLSCipherSuites) require.Equal(t, 1, bindings[1].HideLoginURL) require.Empty(t, bindings[1].OIDC.ClientID) + require.Len(t, bindings[1].OIDC.Scopes, 3) require.False(t, bindings[1].Security.Enabled) require.Equal(t, "Web Admin", bindings[1].Branding.WebAdmin.Name) require.Equal(t, "WebClient", bindings[1].Branding.WebClient.ShortName) @@ -1068,6 +1072,8 @@ 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.Len(t, bindings[2].OIDC.Scopes, 1) + require.Equal(t, "openid", bindings[2].OIDC.Scopes[0]) require.True(t, bindings[2].OIDC.ImplicitRoles) require.Len(t, bindings[2].OIDC.CustomFields, 2) require.Equal(t, "field1", bindings[2].OIDC.CustomFields[0]) diff --git a/docs/full-configuration.md b/docs/full-configuration.md index e20bcd69..cf3aba68 100644 --- a/docs/full-configuration.md +++ b/docs/full-configuration.md @@ -279,7 +279,7 @@ The configuration file contains the following sections: - `client_secret`, string. Defines the application's secret. Default: blank. - `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. - - `scopes`, list of strings. Request the OAuth provider to provide the scope information from an authenticated users. Default: `"openid", "profile", "email"`. + - `scopes`, list of strings. Request the OAuth provider to provide the scope information from an authenticated users. The `openid` scope is mandatory. Default: `"openid", "profile", "email"`. - `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. diff --git a/go.mod b/go.mod index e7920d9d..0a397b1c 100644 --- a/go.mod +++ b/go.mod @@ -68,7 +68,7 @@ require ( golang.org/x/crypto v0.0.0-20220622213112-05595931fe9d golang.org/x/net v0.0.0-20220622184535-263ec571b305 golang.org/x/oauth2 v0.0.0-20220622183110-fd043fe589d2 - golang.org/x/sys v0.0.0-20220622161953-175b2fd9d664 + golang.org/x/sys v0.0.0-20220624220833-87e55d714810 golang.org/x/time v0.0.0-20220609170525-579cf78fd858 google.golang.org/api v0.85.0 gopkg.in/natefinch/lumberjack.v2 v2.0.0 @@ -155,7 +155,7 @@ require ( golang.org/x/tools v0.1.11 // indirect golang.org/x/xerrors v0.0.0-20220609144429-65e65417b02f // indirect google.golang.org/appengine v1.6.7 // indirect - google.golang.org/genproto v0.0.0-20220623142657-077d458a5694 // indirect + google.golang.org/genproto v0.0.0-20220624142145-8cd45d7dbd1f // indirect google.golang.org/grpc v1.47.0 // indirect google.golang.org/protobuf v1.28.0 // indirect gopkg.in/ini.v1 v1.66.6 // indirect diff --git a/go.sum b/go.sum index e5d2cd20..7e5d8780 100644 --- a/go.sum +++ b/go.sum @@ -966,8 +966,8 @@ golang.org/x/sys v0.0.0-20220513210249-45d2b4557a2a/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220610221304-9f5ed59c137d/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220615213510-4f61da869c0c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220622161953-175b2fd9d664 h1:wEZYwx+kK+KlZ0hpvP2Ls1Xr4+RWnlzGFwPP0aiDjIU= -golang.org/x/sys v0.0.0-20220622161953-175b2fd9d664/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220624220833-87e55d714810 h1:rHZQSjJdAI4Xf5Qzeh2bBc5YJIkPFVM6oDtMFYmgws0= +golang.org/x/sys v0.0.0-20220624220833-87e55d714810/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 h1:JGgROgKl9N8DuW20oFS5gxc+lE67/N3FcwmBPMe7ArY= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= @@ -1214,8 +1214,8 @@ google.golang.org/genproto v0.0.0-20220523171625-347a074981d8/go.mod h1:RAyBrSAP google.golang.org/genproto v0.0.0-20220608133413-ed9918b62aac/go.mod h1:KEWEmljWE5zPzLBa/oHl6DaEt9LmfH6WtH1OHIvleBA= google.golang.org/genproto v0.0.0-20220616135557-88e70c0c3a90/go.mod h1:KEWEmljWE5zPzLBa/oHl6DaEt9LmfH6WtH1OHIvleBA= google.golang.org/genproto v0.0.0-20220617124728-180714bec0ad/go.mod h1:KEWEmljWE5zPzLBa/oHl6DaEt9LmfH6WtH1OHIvleBA= -google.golang.org/genproto v0.0.0-20220623142657-077d458a5694 h1:itnFmgk4Ls5nT+mYO2ZK6F0DpKsGZLhB5BB9y5ZL2HA= -google.golang.org/genproto v0.0.0-20220623142657-077d458a5694/go.mod h1:KEWEmljWE5zPzLBa/oHl6DaEt9LmfH6WtH1OHIvleBA= +google.golang.org/genproto v0.0.0-20220624142145-8cd45d7dbd1f h1:hJ/Y5SqPXbarffmAsApliUlcvMU+wScNGfyop4bZm8o= +google.golang.org/genproto v0.0.0-20220624142145-8cd45d7dbd1f/go.mod h1:KEWEmljWE5zPzLBa/oHl6DaEt9LmfH6WtH1OHIvleBA= google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiqXj38= google.golang.org/grpc v1.21.1/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ijfRaM= diff --git a/httpd/oidc.go b/httpd/oidc.go index 9ccab490..6bf506b7 100644 --- a/httpd/oidc.go +++ b/httpd/oidc.go @@ -70,7 +70,9 @@ type OIDC struct { // 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"` - // Scopes required by the OAuth provider to retrieve information about the authenticated user. Refer to your OAuth provider documentation for more information about this + // Scopes required by the OAuth provider to retrieve information about the authenticated user. + // The "openid" scope is required. + // Refer to your OAuth provider documentation for more information about this Scopes []string `json:"scopes" mapstructure:"scopes"` // Custom token claims fields to pass to the pre-login hook CustomFields []string `json:"custom_fields" mapstructure:"custom_fields"` @@ -118,6 +120,9 @@ func (o *OIDC) initialize() error { if o.RedirectBaseURL == "" { return errors.New("oidc: redirect base URL cannot be empty") } + if !util.Contains(o.Scopes, oidc.ScopeOpenID) { + return fmt.Errorf("oidc: required scope %q is not set", oidc.ScopeOpenID) + } ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() diff --git a/httpd/oidc_test.go b/httpd/oidc_test.go index 6873dfcb..b1787fd0 100644 --- a/httpd/oidc_test.go +++ b/httpd/oidc_test.go @@ -93,6 +93,11 @@ func TestOIDCInitialization(t *testing.T) { RoleField: "sftpgo_role", } err = config.initialize() + if assert.Error(t, err) { + assert.Contains(t, err.Error(), "oidc: required scope \"openid\" is not set") + } + config.Scopes = []string{oidc.ScopeOpenID} + err = config.initialize() if assert.Error(t, err) { assert.Contains(t, err.Error(), "oidc: unable to initialize provider") } @@ -1263,6 +1268,7 @@ func getTestOIDCServer() *httpdServer { UsernameField: "preferred_username", RoleField: "sftpgo_role", ImplicitRoles: false, + Scopes: []string{oidc.ScopeOpenID, "profile", "email"}, CustomFields: nil, }, }, diff --git a/sftpgo.json b/sftpgo.json index 29937ae6..e821ebb9 100644 --- a/sftpgo.json +++ b/sftpgo.json @@ -262,7 +262,11 @@ "client_secret": "", "config_url": "", "redirect_base_url": "", - "scopes": [ "openid", "profile", "email" ], + "scopes": [ + "openid", + "profile", + "email" + ], "username_field": "", "role_field": "", "implicit_roles": false,