Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions api/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,14 @@ func (a *API) CreateInstance(w http.ResponseWriter, r *http.Request) error {
if err := params.BaseConfig.SMTP.Validate(a.config.SMTP.ReservedDomains); err != nil {
return badRequestError("Invalid SMTP configuration: %v", err)
}
// Secure-by-default only applies to instances created with a config.
// A config-less instance has no SiteURL to fall back to, so leaving it
// permissive avoids locking out the strict behaviors that derive their
// allowlists from it. An explicit Security.Strict=false does not opt
// out here: when the platform enables secure-by-default, secure wins.
if a.config.NewInstancesSecureByDefault && !params.BaseConfig.Security.Strict {
params.BaseConfig.Security.Strict = true
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

i := models.Instance{
Expand Down
142 changes: 142 additions & 0 deletions api/instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,148 @@ func (ts *InstanceTestSuite) TestCreate() {
assert.NotNil(ts.T(), i.BaseConfig)
}

func (ts *InstanceTestSuite) TestCreate_SecureByDefaultFlipsEnabled() {
prev := ts.API.config.NewInstancesSecureByDefault
ts.API.config.NewInstancesSecureByDefault = true
defer func() { ts.API.config.NewInstancesSecureByDefault = prev }()

freshUUID := uuid.Must(uuid.NewV4())
var buffer bytes.Buffer
require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
"uuid": freshUUID,
"config": map[string]interface{}{
"jwt": map[string]interface{}{"secret": "testsecret"},
},
}))

req := httptest.NewRequest(http.MethodPost, "/instances", &buffer)
req.Header.Set("Content-Type", "application/json")
req.Header.Set("Authorization", "Bearer "+operatorToken)
w := httptest.NewRecorder()
ts.API.handler.ServeHTTP(w, req)
require.Equal(ts.T(), http.StatusCreated, w.Code)

i, err := models.GetInstanceByUUID(ts.API.db, freshUUID)
require.NoError(ts.T(), err)
require.NotNil(ts.T(), i.BaseConfig)
assert.True(ts.T(), i.BaseConfig.Security.Strict, "secure-by-default should flip Security.Strict")
}

func (ts *InstanceTestSuite) TestCreate_SecureByDefaultPreservesExplicitConfig() {
// The secure-by-default flip only fires when Security.Strict is false, so a
// caller that explicitly enables it keeps that value untouched.
prev := ts.API.config.NewInstancesSecureByDefault
ts.API.config.NewInstancesSecureByDefault = true
defer func() { ts.API.config.NewInstancesSecureByDefault = prev }()

freshUUID := uuid.Must(uuid.NewV4())
var buffer bytes.Buffer
require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
"uuid": freshUUID,
"config": map[string]interface{}{
"jwt": map[string]interface{}{"secret": "testsecret"},
"security": map[string]interface{}{"strict": true},
},
}))

req := httptest.NewRequest(http.MethodPost, "/instances", &buffer)
req.Header.Set("Content-Type", "application/json")
req.Header.Set("Authorization", "Bearer "+operatorToken)
w := httptest.NewRecorder()
ts.API.handler.ServeHTTP(w, req)
require.Equal(ts.T(), http.StatusCreated, w.Code)

i, err := models.GetInstanceByUUID(ts.API.db, freshUUID)
require.NoError(ts.T(), err)
assert.True(ts.T(), i.BaseConfig.Security.Strict)
}

func (ts *InstanceTestSuite) TestCreate_SecureByDefaultOverridesExplicitFalse() {
// Secure-by-default is a security posture: when the platform enables it, an
// explicit "strict": false from the caller does not opt the new instance
// out. Lock that precedence in.
prev := ts.API.config.NewInstancesSecureByDefault
ts.API.config.NewInstancesSecureByDefault = true
defer func() { ts.API.config.NewInstancesSecureByDefault = prev }()

freshUUID := uuid.Must(uuid.NewV4())
var buffer bytes.Buffer
require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
"uuid": freshUUID,
"config": map[string]interface{}{
"jwt": map[string]interface{}{"secret": "testsecret"},
"security": map[string]interface{}{"strict": false},
},
}))

req := httptest.NewRequest(http.MethodPost, "/instances", &buffer)
req.Header.Set("Content-Type", "application/json")
req.Header.Set("Authorization", "Bearer "+operatorToken)
w := httptest.NewRecorder()
ts.API.handler.ServeHTTP(w, req)
require.Equal(ts.T(), http.StatusCreated, w.Code)

i, err := models.GetInstanceByUUID(ts.API.db, freshUUID)
require.NoError(ts.T(), err)
require.NotNil(ts.T(), i.BaseConfig)
assert.True(ts.T(), i.BaseConfig.Security.Strict, "secure-by-default must override explicit strict=false")
}

func (ts *InstanceTestSuite) TestCreate_SecureByDefaultKeepsConfigLess() {
// The handler intentionally skips the strict flip for config-less
// instances — a config-less instance has no SiteURL to seed the strict
// behaviors' allowlists from, so flipping Strict would lock them out
// immediately. Lock that anti-lockout behavior in.
prev := ts.API.config.NewInstancesSecureByDefault
ts.API.config.NewInstancesSecureByDefault = true
defer func() { ts.API.config.NewInstancesSecureByDefault = prev }()

freshUUID := uuid.Must(uuid.NewV4())
var buffer bytes.Buffer
require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
"uuid": freshUUID,
}))

req := httptest.NewRequest(http.MethodPost, "/instances", &buffer)
req.Header.Set("Content-Type", "application/json")
req.Header.Set("Authorization", "Bearer "+operatorToken)
w := httptest.NewRecorder()
ts.API.handler.ServeHTTP(w, req)
require.Equal(ts.T(), http.StatusCreated, w.Code)

i, err := models.GetInstanceByUUID(ts.API.db, freshUUID)
require.NoError(ts.T(), err)
if i.BaseConfig != nil {
assert.False(ts.T(), i.BaseConfig.Security.Strict, "config-less create must not auto-flip Security.Strict")
}
}

func (ts *InstanceTestSuite) TestCreate_LegacyLeavesSecurityDisabled() {
prev := ts.API.config.NewInstancesSecureByDefault
ts.API.config.NewInstancesSecureByDefault = false
defer func() { ts.API.config.NewInstancesSecureByDefault = prev }()

freshUUID := uuid.Must(uuid.NewV4())
var buffer bytes.Buffer
require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
"uuid": freshUUID,
"config": map[string]interface{}{
"jwt": map[string]interface{}{"secret": "testsecret"},
},
}))

req := httptest.NewRequest(http.MethodPost, "/instances", &buffer)
req.Header.Set("Content-Type", "application/json")
req.Header.Set("Authorization", "Bearer "+operatorToken)
w := httptest.NewRecorder()
ts.API.handler.ServeHTTP(w, req)
require.Equal(ts.T(), http.StatusCreated, w.Code)

i, err := models.GetInstanceByUUID(ts.API.db, freshUUID)
require.NoError(ts.T(), err)
assert.False(ts.T(), i.BaseConfig.Security.Strict, "secure-by-default OFF should leave Security disabled")
}

func (ts *InstanceTestSuite) TestGet() {
instanceID := uuid.Must(uuid.NewV4())
err := ts.API.db.Create(&models.Instance{
Expand Down
6 changes: 4 additions & 2 deletions api/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ type Settings struct {
ExternalLabels ProviderLabels `json:"external_labels"`
DisableSignup bool `json:"disable_signup"`
Autoconfirm bool `json:"autoconfirm"`
SecurityStrict bool `json:"security_strict"`
}

func (a *API) Settings(w http.ResponseWriter, r *http.Request) error {
Expand All @@ -39,7 +40,8 @@ func (a *API) Settings(w http.ResponseWriter, r *http.Request) error {
ExternalLabels: ProviderLabels{
SAML: config.External.Saml.Name,
},
DisableSignup: config.DisableSignup,
Autoconfirm: config.Mailer.Autoconfirm,
DisableSignup: config.DisableSignup,
Autoconfirm: config.Mailer.Autoconfirm,
SecurityStrict: config.Security.Strict,
})
}
48 changes: 48 additions & 0 deletions api/settings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package api
import (
"context"
"encoding/json"
"io"
"net/http"
"net/http/httptest"
"testing"
Expand Down Expand Up @@ -58,6 +59,53 @@ func TestSettings_EmailDisabled(t *testing.T) {
require.False(t, p.Email)
}

func TestSettings_SecurityStrictExposed(t *testing.T) {
api, _, _, err := setupAPIForTestForInstance()
require.NoError(t, err)

req := httptest.NewRequest(http.MethodGet, "http://localhost/settings", nil)
req.Header.Set("Content-Type", "application/json")
w := httptest.NewRecorder()
api.handler.ServeHTTP(w, req)
require.Equal(t, w.Code, http.StatusOK)

strict := decodeSecurityStrict(t, w.Body)
require.False(t, strict, "default config should report security_strict=false")
}

func TestSettings_SecurityStrictReflectsConfig(t *testing.T) {
api, config, instanceID, err := setupAPIForTestForInstance()
require.NoError(t, err)

config.Security.Strict = true

req := httptest.NewRequest(http.MethodGet, "http://localhost/settings", nil)
req.Header.Set("Content-Type", "application/json")
ctx, err := WithInstanceConfig(context.Background(), config, instanceID)
require.NoError(t, err)
req = req.WithContext(ctx)

w := httptest.NewRecorder()
api.handler.ServeHTTP(w, req)
require.Equal(t, w.Code, http.StatusOK)

require.True(t, decodeSecurityStrict(t, w.Body))
}

// decodeSecurityStrict reads the /settings response body as a raw map so the
// test fails loudly if the security_strict key disappears from the JSON
// contract — a Settings{} decode would silently treat a missing bool as false.
func decodeSecurityStrict(t *testing.T, body io.Reader) bool {
t.Helper()
raw := map[string]json.RawMessage{}
require.NoError(t, json.NewDecoder(body).Decode(&raw))
rawStrict, ok := raw["security_strict"]
require.True(t, ok, "settings response must include security_strict")
var strict bool
require.NoError(t, json.Unmarshal(rawStrict, &strict))
return strict
}

func TestSettings_ExternalName(t *testing.T) {
api, _, _, err := setupAPIForTestForInstance()
require.NoError(t, err)
Expand Down
17 changes: 17 additions & 0 deletions conf/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ type GlobalConfiguration struct {
Tracing TracingConfig
SMTP SMTPConfiguration
RateLimitHeader string `split_words:"true"`
// NewInstancesSecureByDefault flips Security.Strict to true for instances
// created via POST /instances. Secure-by-default takes precedence, so it
// overrides both an omitted and an explicit Security.Strict=false from the
// caller. Defaults to false until callers (e.g. the Netlify control plane)
// are ready to pre-populate the per-instance Security configuration.
NewInstancesSecureByDefault bool `split_words:"true"`
}

// EmailContentConfiguration holds the configuration for emails, both subjects and template URLs.
Expand Down Expand Up @@ -111,6 +117,16 @@ type MailerConfiguration struct {
InviteMaxAge time.Duration `json:"invite_max_age" split_words:"true"`
}

// SecurityConfiguration groups stricter security behaviors behind one switch.
// When Strict is false (default for existing instances), gotrue retains legacy
// behavior for backwards compatibility. New instances flip Strict to true via
// the POST /instances handler when the global NewInstancesSecureByDefault flag
// is set. The individual strict behaviors and their settings are added by
// follow-up changes.
type SecurityConfiguration struct {
Strict bool `json:"strict"`
}

// Configuration holds all the per-instance configuration.
type Configuration struct {
SiteURL string `json:"site_url" split_words:"true" required:"true"`
Expand All @@ -124,6 +140,7 @@ type Configuration struct {
Key string `json:"key"`
Duration int `json:"duration"`
} `json:"cookies"`
Security SecurityConfiguration `json:"security"`
}

func loadEnvironment(filename string) error {
Expand Down
53 changes: 53 additions & 0 deletions conf/configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,59 @@ func TestSMTPConfigurationValidate(t *testing.T) {
}
}

func TestSecurityConfigurationDefaults(t *testing.T) {
defer os.Clearenv()

baseEnv := func() {
os.Clearenv()
os.Setenv("GOTRUE_DB_DRIVER", "mysql")
os.Setenv("GOTRUE_DB_DATABASE_URL", "fake")
os.Setenv("GOTRUE_OPERATOR_TOKEN", "token")
os.Setenv("GOTRUE_SITE_URL", "https://example.com")
os.Setenv("GOTRUE_JWT_SECRET", "secret")
}

t.Run("disabled by default", func(t *testing.T) {
baseEnv()
c, err := LoadConfig("")
require.NoError(t, err)
assert.False(t, c.Security.Strict)
})

t.Run("loads strict from env", func(t *testing.T) {
baseEnv()
os.Setenv("GOTRUE_SECURITY_STRICT", "true")
c, err := LoadConfig("")
require.NoError(t, err)
assert.True(t, c.Security.Strict)
})
}

func TestNewInstancesSecureByDefault(t *testing.T) {
defer os.Clearenv()

t.Run("defaults to false", func(t *testing.T) {
os.Clearenv()
os.Setenv("GOTRUE_DB_DRIVER", "mysql")
os.Setenv("GOTRUE_DB_DATABASE_URL", "fake")
os.Setenv("GOTRUE_OPERATOR_TOKEN", "token")
gc, err := LoadGlobal("")
require.NoError(t, err)
assert.False(t, gc.NewInstancesSecureByDefault)
})

t.Run("loads from env", func(t *testing.T) {
os.Clearenv()
os.Setenv("GOTRUE_DB_DRIVER", "mysql")
os.Setenv("GOTRUE_DB_DATABASE_URL", "fake")
os.Setenv("GOTRUE_OPERATOR_TOKEN", "token")
os.Setenv("GOTRUE_NEW_INSTANCES_SECURE_BY_DEFAULT", "true")
gc, err := LoadGlobal("")
require.NoError(t, err)
assert.True(t, gc.NewInstancesSecureByDefault)
})
}

func TestTracing(t *testing.T) {
os.Setenv("GOTRUE_DB_DRIVER", "mysql")
os.Setenv("GOTRUE_DB_DATABASE_URL", "fake")
Expand Down
6 changes: 6 additions & 0 deletions example.env
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,9 @@ GOTRUE_WEBHOOK_SECRET=test_secret
GOTRUE_WEBHOOK_RETRIES=5
GOTRUE_WEBHOOK_TIMEOUT_SEC=3
GOTRUE_WEBHOOK_EVENTS=validate,signup,login,userdeleted,usermodified

# Flip Security.Strict to true for instances created via POST /instances,
# overriding an omitted or explicit false from the caller. Defaults to false;
# set to true once callers populate the per-instance Security configuration
# for new instances.
GOTRUE_NEW_INSTANCES_SECURE_BY_DEFAULT=false
Loading