Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Remove duplicated manager url config #4493

Merged
Merged
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
4 changes: 1 addition & 3 deletions model/instance/instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ func TestInstance(t *testing.T) {

cfg.Contexts = map[string]interface{}{
"context": map[string]interface{}{
"manager_url": "http://manager.example.org",
"logos": map[string]interface{}{
"coachco2": map[string]interface{}{
"light": []interface{}{
Expand Down Expand Up @@ -173,8 +172,7 @@ func TestInstance(t *testing.T) {
}
]
}
},
"manager_url": "http://manager.example.org"
}
}`
assert.Equal(t, expected, string(bytes))
})
Expand Down
35 changes: 24 additions & 11 deletions model/instance/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,24 @@ const (
ManagerPremiumURL
// ManagerBlockedURL is the kind for a redirection of a blocked instance.
ManagerBlockedURL
// ManagerBaseURL is the kind for building other manager URLs
ManagerBaseURL
)

// ManagerURL returns an external string for the given ManagerURL kind. It is
// used for redirecting the user to a manager URL.
func (i *Instance) ManagerURL(k ManagerURLKind) (string, error) {
if i.UUID == "" {
c := clouderyConfig(i)
if c == nil {
return "", nil
}

config, ok := i.SettingsContext()
if !ok {
if i.UUID == "" {
return "", nil
}

base, ok := config["manager_url"].(string)
if !ok {
base := c.API.URL
if base == "" {
return "", nil
}

Expand All @@ -51,6 +53,8 @@ func (i *Instance) ManagerURL(k ManagerURLKind) (string, error) {
path = fmt.Sprintf("/cozy/instances/%s/tos", url.PathEscape(i.UUID))
case ManagerBlockedURL:
path = fmt.Sprintf("/cozy/instances/%s/blocked", url.PathEscape(i.UUID))
case ManagerBaseURL:
path = ""
default:
panic("unknown ManagerURLKind")
}
Expand All @@ -61,6 +65,20 @@ func (i *Instance) ManagerURL(k ManagerURLKind) (string, error) {

// APIManagerClient returns a client to talk to the manager via its API.
func APIManagerClient(inst *Instance) *manager.APIClient {
c := clouderyConfig(inst)
if c == nil {
return nil
}

api := c.API
if api.URL == "" || api.Token == "" {
return nil
}

return manager.NewAPIClient(api.URL, api.Token)
}

func clouderyConfig(inst *Instance) *config.ClouderyConfig {
clouderies := config.GetConfig().Clouderies
if clouderies == nil {
return nil
Expand All @@ -75,10 +93,5 @@ func APIManagerClient(inst *Instance) *manager.APIClient {
return nil
}

api := cloudery.API
if api.URL == "" || api.Token == "" {
return nil
}

return manager.NewAPIClient(api.URL, api.Token)
return &cloudery
}
4 changes: 1 addition & 3 deletions model/oauth/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ func TestClient(t *testing.T) {
}

config.UseTestFile(t)
conf := config.GetConfig()
conf.Contexts[config.DefaultInstanceContext] = map[string]interface{}{"manager_url": "http://manager.example.org"}
setup := testutils.NewSetup(t, t.Name())
testInstance := setup.GetTestInstance()

Expand Down Expand Up @@ -197,7 +195,7 @@ func TestClient(t *testing.T) {
premiumLink := assertClientsLimitAlertMailWasSent(t, testInstance, "notificationWithoutPremium", 1)
assert.Empty(t, premiumLink)

testutils.WithManager(t, testInstance)
testutils.WithManager(t, testInstance, testutils.ManagerConfig{URL: "http://manager.example.org", WithPremiumLinks: true})

notificationWithPremium = &oauth.Client{
ClientName: "notificationWithPremium",
Expand Down
1 change: 1 addition & 0 deletions pkg/config/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -1150,6 +1150,7 @@ func createTestViper() *viper.Viper {
v.SetDefault("log.level", "info")
v.SetDefault("assets_polling_disabled", false)
v.SetDefault("assets_polling_interval", 2*time.Minute)
v.SetDefault("contexts", map[string]interface{}{DefaultInstanceContext: map[string]interface{}{}})
applyDefaults(v)
return v
}
Expand Down
7 changes: 6 additions & 1 deletion pkg/config/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ func TestConfigUnmarshal(t *testing.T) {
// Contexts
assert.EqualValues(t, map[string]interface{}{
"my-context": map[string]interface{}{
"manager_url": "https://manager-url",
"onboarded_redirection": "home/intro",
"default_redirection": "home/",
"help_link": "https://cozy.io/fr/support",
Expand Down Expand Up @@ -216,6 +215,12 @@ func TestConfigUnmarshal(t *testing.T) {
Token: "some-token",
},
},
"my-context": {
API: ClouderyAPI{
URL: "https://manager-url",
Token: "manager-token",
},
},
}, cfg.Clouderies)

// CSPs
Expand Down
5 changes: 4 additions & 1 deletion pkg/config/config/testdata/full_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ clouderies:
api:
url: https://some-url
token: some-token
my-context:
api:
url: https://manager-url
token: manager-token

password_reset_interval: 1h

Expand Down Expand Up @@ -163,7 +167,6 @@ log:

contexts:
my-context:
manager_url: https://manager-url
onboarded_redirection: home/intro
default_redirection: home/
help_link: https://cozy.io/fr/support
Expand Down
60 changes: 51 additions & 9 deletions tests/testutils/test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package testutils
import (
"bytes"
"context"
"encoding/json"
"errors"
"flag"
"fmt"
Expand Down Expand Up @@ -469,26 +470,41 @@ func compress(content string) []byte {
return buf.Bytes()
}

func WithManager(t *testing.T, inst *instance.Instance) (shouldRemoveUUID bool) {
type ManagerConfig struct {
URL string
WithPremiumLinks bool
}

func WithManager(t *testing.T, inst *instance.Instance, managerConfig ManagerConfig) (shouldRemoveUUID bool) {
require.NotEmpty(t, managerConfig.URL, "Could not enable test instance manager: cloudery API URL is required")

if inst.UUID == "" {
uuid, err := uuid.NewV7()
require.NoError(t, err, "Could not enable test instance manager")
inst.UUID = uuid.String()
shouldRemoveUUID = true
}

config, ok := inst.SettingsContext()
require.True(t, ok, "Could not enable test instance manager: could not fetch test instance settings context")
cfg := config.GetConfig()
originalCfg, _ := json.Marshal(cfg)

managerURL, ok := config["manager_url"].(string)
require.True(t, ok, "Could not enable test instance manager: manager_url config is required")
require.NotEmpty(t, managerURL, "Could not enable test instance manager: manager_url config is required")
if cfg.Clouderies == nil {
cfg.Clouderies = map[string]config.ClouderyConfig{}
}
cloudery, contextName := getCloudery(inst, cfg)
cloudery.API = config.ClouderyAPI{URL: managerConfig.URL, Token: ""}
cfg.Clouderies[contextName] = cloudery

was := config["enable_premium_links"]
config["enable_premium_links"] = true
if cfg.Contexts == nil {
cfg.Contexts = map[string]interface{}{}
}
context, contextName := getContext(inst, cfg)
context["manager_url"] = managerConfig.URL
context["enable_premium_links"] = managerConfig.WithPremiumLinks
cfg.Contexts[contextName] = context

t.Cleanup(func() {
config["enable_premium_links"] = was
json.Unmarshal(originalCfg, cfg)

if shouldRemoveUUID {
inst.UUID = ""
Expand All @@ -502,6 +518,32 @@ func WithManager(t *testing.T, inst *instance.Instance) (shouldRemoveUUID bool)
return shouldRemoveUUID
}

// getCloudery returns the most relevant cloudery config depending on the
// instance context name or creates a new one if none exist.
// It also returns the name of the context associated with the config.
func getCloudery(inst *instance.Instance, cfg *config.Config) (config.ClouderyConfig, string) {
if cloudery, ok := cfg.Clouderies[inst.ContextName]; ok {
return cloudery, inst.ContextName
}
if cloudery, ok := cfg.Clouderies[config.DefaultInstanceContext]; ok {
return cloudery, config.DefaultInstanceContext
}
return config.ClouderyConfig{}, config.DefaultInstanceContext
}

// getContext returns the most relevant context config depending on the
// instance context name or creates a new one if none exist.
// It also returns the name of the context associated with the config.
func getContext(inst *instance.Instance, cfg *config.Config) (map[string]interface{}, string) {
if context, ok := cfg.Contexts[inst.ContextName].(map[string]interface{}); ok {
return context, inst.ContextName
}
if context, ok := cfg.Contexts[config.DefaultInstanceContext].(map[string]interface{}); ok {
return context, config.DefaultInstanceContext
}
return map[string]interface{}{}, config.DefaultInstanceContext
}

func DisableManager(inst *instance.Instance, shouldRemoveUUID bool) error {
config, ok := inst.SettingsContext()
if !ok {
Expand Down
3 changes: 1 addition & 2 deletions web/apps/apps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ func TestApps(t *testing.T) {
Host: "localhost",
Path: tempdir,
}
cfg.Contexts[config.DefaultInstanceContext] = map[string]interface{}{"manager_url": "http://manager.example.org"}
was := cfg.Subdomains
cfg.Subdomains = config.NestedSubdomains
defer func() { cfg.Subdomains = was }()
Expand Down Expand Up @@ -205,7 +204,7 @@ func TestApps(t *testing.T) {

// TOS not signed warning

testutils.WithManager(t, testInstance)
testutils.WithManager(t, testInstance, testutils.ManagerConfig{URL: "http://manager.example.org", WithPremiumLinks: true})

tosSigned := testInstance.TOSSigned
tosLatest := testInstance.TOSLatest
Expand Down
8 changes: 2 additions & 6 deletions web/auth/confirm.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,12 +149,8 @@ func checkRedirectToAuthorized(c echo.Context) (*url.URL, error) {
}

func checkRedirectToManager(inst *instance.Instance, redirect *url.URL) bool {
config, ok := inst.SettingsContext()
if !ok {
return false
}
managerURL, ok := config["manager_url"].(string)
if !ok {
managerURL, err := inst.ManagerURL(instance.ManagerBaseURL)
if err != nil {
return false
}
manager, err := url.Parse(managerURL)
Expand Down
22 changes: 20 additions & 2 deletions web/settings/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"net/http"

"github.com/cozy/cozy-stack/model/instance"
"github.com/cozy/cozy-stack/model/instance/lifecycle"
"github.com/cozy/cozy-stack/pkg/consts"
"github.com/cozy/cozy-stack/pkg/couchdb"
Expand Down Expand Up @@ -78,6 +79,23 @@ func (h *HTTPHandler) context(c echo.Context) error {
}

i := middlewares.GetInstance(c)
doc := &apiContext{i.GetContextWithSponsorships()}
return jsonapi.Data(c, http.StatusOK, doc, nil)
context := &apiContext{i.GetContextWithSponsorships()}

managerURL, err := i.ManagerURL(instance.ManagerBaseURL)
if err != nil {
return err
}
if managerURL != "" {
// XXX: The manager URL used to be stored in the config in
// `context.<context_name>.manager_url`. It's now stored in
// `clouderies.<context_name>.api.url` and can be retrieved via a call
// to `instance.ManagerURL()`.
//
// However, some external apps and clients (e.g. `cozy-client`) still
// expect to find the `manager_url` attribute in the context document
// so we add it back for backwards compatibility.
context.doc["manager_url"] = managerURL
}

return jsonapi.Data(c, http.StatusOK, context, nil)
}
5 changes: 3 additions & 2 deletions web/settings/settings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ func TestSettings(t *testing.T) {
conf := config.GetConfig()
conf.Assets = "../../assets"
conf.Contexts[config.DefaultInstanceContext] = map[string]interface{}{
"manager_url": "http://manager.example.org",
"logos": map[string]interface{}{
"home": map[string]interface{}{
"light": []interface{}{
Expand Down Expand Up @@ -112,6 +111,8 @@ func TestSettings(t *testing.T) {
t.Run("GetContext", func(t *testing.T) {
e := testutils.CreateTestClient(t, tsURL)

testutils.WithManager(t, testInstance, testutils.ManagerConfig{URL: "http://manager.example.org"})

obj := e.GET("/settings/context").
WithCookie(sessCookie, "connected").
WithHeader("Accept", "application/vnd.api+json").
Expand Down Expand Up @@ -1021,7 +1022,7 @@ func TestSettings(t *testing.T) {
Contains("/#/connectedDevices").
NotContains("http://manager.example.org")

testutils.WithManager(t, testInstance)
testutils.WithManager(t, testInstance, testutils.ManagerConfig{URL: "http://manager.example.org", WithPremiumLinks: true})

e.GET("/settings/clients/limit-exceeded").
WithCookie(sessCookie, "connected").
Expand Down
Loading