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

feat(configx): allow exceptions in immutables #713

Merged
merged 3 commits into from
Aug 10, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
6 changes: 6 additions & 0 deletions configx/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ func WithImmutables(immutables ...string) OptionModifier {
}
}

func WithExceptImmutables(exceptImmutables ...string) OptionModifier {
return func(p *Provider) {
p.exceptImmutables = append(p.exceptImmutables, exceptImmutables...)
}
}

func WithFlags(flags *pflag.FlagSet) OptionModifier {
return func(p *Provider) {
p.flags = flags
Expand Down
32 changes: 27 additions & 5 deletions configx/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ type tuple struct {
type Provider struct {
l sync.RWMutex
*koanf.Koanf
immutables []string
immutables, exceptImmutables []string

schema []byte
flags *pflag.FlagSet
Expand Down Expand Up @@ -249,6 +249,18 @@ func (p *Provider) runOnChanges(e watcherx.Event, err error) {
}
}

func deleteOtherKeys(k *koanf.Koanf, keys []string) {
outer:
for _, key := range k.Keys() {
for _, ik := range keys {
if key == ik {
continue outer
}
}
k.Delete(key)
}
}

func (p *Provider) reload(e watcherx.Event) {
p.l.Lock()

Expand All @@ -264,10 +276,20 @@ func (p *Provider) reload(e watcherx.Event) {
return // unlocks & runs changes in defer
}

for _, key := range p.immutables {
if !reflect.DeepEqual(p.Koanf.Get(key), nk.Get(key)) {
err = NewImmutableError(key, fmt.Sprintf("%v", p.Koanf.Get(key)), fmt.Sprintf("%v", nk.Get(key)))
return // unlocks & runs changes in defer
oldImmutables, newImmutables := p.Koanf.Copy(), nk.Copy()
deleteOtherKeys(oldImmutables, p.immutables)
deleteOtherKeys(newImmutables, p.immutables)

for _, key := range p.exceptImmutables {
oldImmutables.Delete(key)
newImmutables.Delete(key)
}
if !reflect.DeepEqual(oldImmutables.Raw(), newImmutables.Raw()) {
for _, key := range p.immutables {
if !reflect.DeepEqual(oldImmutables.Get(key), newImmutables.Get(key)) {
err = NewImmutableError(key, fmt.Sprintf("%v", p.Koanf.Get(key)), fmt.Sprintf("%v", nk.Get(key)))
return // unlocks & runs changes in defer
}
}
}

Expand Down
25 changes: 25 additions & 0 deletions configx/provider_watch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,31 @@ func TestReload(t *testing.T) {
assertNoOpenFDs(t, dir, name)
})

t.Run("case=allows to update excepted immutable", func(t *testing.T) {
t.Parallel()
config := `{"foo": {"bar": "a", "baz": "b"}}`

dir := t.TempDir()
name := "config.json"
watcherx.KubernetesAtomicWrite(t, dir, name, config)

c := make(chan struct{})
p, _ := setup(t, dir, name, c,
WithImmutables("foo"),
WithExceptImmutables("foo.baz"),
SkipValidation())

assert.Equal(t, "a", p.String("foo.bar"))
assert.Equal(t, "b", p.String("foo.baz"))

config = `{"foo": {"bar": "a", "baz": "x"}}`
watcherx.KubernetesAtomicWrite(t, dir, name, config)
<-c
time.Sleep(time.Millisecond)

assert.Equal(t, "x", p.String("foo.baz"))
})

t.Run("case=runs without validation errors", func(t *testing.T) {
t.Parallel()
dir, name := tmpConfigFile(t, "some string", "bar")
Expand Down
53 changes: 53 additions & 0 deletions corsx/check_origin.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Copyright © 2023 Ory Corp
// SPDX-License-Identifier: Apache-2.0

package corsx

import "strings"

// CheckOrigin is a function that can be used well with cors.Options.AllowOriginRequestFunc.
// It checks whether the origin is allowed following the same behavior as github.com/rs/cors.
//
// Recommended usage for hot-reloadable origins:
//
// func (p *Config) cors(ctx context.Context, prefix string) (cors.Options, bool) {
// opts, enabled := p.GetProvider(ctx).CORS(prefix, cors.Options{
// AllowedMethods: []string{"GET", "POST", "PUT", "PATCH", "DELETE"},
// AllowedHeaders: []string{"Authorization", "Content-Type", "Cookie"},
// ExposedHeaders: []string{"Content-Type", "Set-Cookie"},
// AllowCredentials: true,
// })
// opts.AllowOriginRequestFunc = func(r *http.Request, origin string) bool {
// // load the origins from the config on every request to allow hot-reloading
// allowedOrigins := p.GetProvider(r.Context()).Strings(prefix + ".cors.allowed_origins")
// return corsx.CheckOrigin(allowedOrigins, origin)
// }
// return opts, enabled
// }
func CheckOrigin(allowedOrigins []string, origin string) bool {
if len(allowedOrigins) == 0 {
return true
}
for _, o := range allowedOrigins {
if o == "*" {
// allow all origins
return true
}
// Note: for origins and methods matching, the spec requires a case-sensitive matching.
// As it may be error-prone, we chose to ignore the spec here.
// https://github.com/rs/cors/blob/066574eebbd0f5f1b6cd1154a160cc292ac1835e/cors.go#L132-L133
prefix, suffix, found := strings.Cut(strings.ToLower(o), "*")
if !found {
// not a pattern, check for equality
if o == origin {
return true
}
continue
}
// inspired by https://github.com/rs/cors/blob/066574eebbd0f5f1b6cd1154a160cc292ac1835e/utils.go#L15
if len(origin) >= len(prefix)+len(suffix) && strings.HasPrefix(origin, prefix) && strings.HasSuffix(origin, suffix) {
return true
}
}
return false
}
111 changes: 111 additions & 0 deletions corsx/check_origin_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
// Copyright © 2023 Ory Corp
// SPDX-License-Identifier: Apache-2.0

package corsx

import (
"net/http"
"testing"

"github.com/rs/cors"
"github.com/stretchr/testify/assert"
)

func TestCheckOrigin(t *testing.T) {
for _, tc := range []struct {
name string
allowedOrigins []string
expect, expectOther bool
}{
{
name: "empty",
allowedOrigins: []string{},
expect: true,
expectOther: true,
},
{
name: "wildcard",
allowedOrigins: []string{"https://example.com", "*"},
expect: true,
expectOther: true,
},
{
name: "exact",
allowedOrigins: []string{"https://www.ory.sh"},
expect: true,
},
{
name: "wildcard in the beginning",
allowedOrigins: []string{"*.ory.sh"},
expect: true,
},
{
name: "wildcard in the middle",
allowedOrigins: []string{"https://*.ory.sh"},
expect: true,
},
{
name: "wildcard in the end",
allowedOrigins: []string{"https://www.ory.*"},
expect: true,
},
{
name: "second wildcard is ignored",
allowedOrigins: []string{"https://*.ory.*"},
expect: false,
},
{
name: "multiple exact",
allowedOrigins: []string{"https://example.com", "https://www.ory.sh"},
expect: true,
},
{
name: "multiple wildcard",
allowedOrigins: []string{"https://*.example.com", "https://*.ory.sh"},
expect: true,
},
{
name: "wildcard and exact origins 1",
allowedOrigins: []string{"https://*.example.com", "https://www.ory.sh"},
expect: true,
},
{
name: "wildcard and exact origins 2",
allowedOrigins: []string{"https://example.com", "https://*.ory.sh"},
expect: true,
},
{
name: "multiple unrelated exact",
allowedOrigins: []string{"https://example.com", "https://example.org"},
expect: false,
},
{
name: "multiple unrelated with wildcard",
allowedOrigins: []string{"https://*.example.com", "https://*.example.org"},
expect: false,
},
{
name: "uppercase exact",
allowedOrigins: []string{"https://www.ORY.sh"},
expect: true,
},
{
name: "uppercase wildcard",
allowedOrigins: []string{"https://*.ORY.sh"},
expect: true,
},
} {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.expect, CheckOrigin(tc.allowedOrigins, "https://www.ory.sh"))

assert.Equal(t, tc.expectOther, CheckOrigin(tc.allowedOrigins, "https://google.com"))

// check for consistency with rs/cors
assert.Equal(t, tc.expect, cors.New(cors.Options{AllowedOrigins: tc.allowedOrigins}).
OriginAllowed(&http.Request{Header: http.Header{"Origin": []string{"https://www.ory.sh"}}}))

assert.Equal(t, tc.expectOther, cors.New(cors.Options{AllowedOrigins: tc.allowedOrigins}).
OriginAllowed(&http.Request{Header: http.Header{"Origin": []string{"https://google.com"}}}))
})
}
}
2 changes: 2 additions & 0 deletions corsx/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (
// panic("implement me")
// })
// // ...
//
// Deprecated: because this is not really practical to use, you should use CheckOrigin as the cors.Options.AllowOriginRequestFunc instead.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with this was that the config has to come from the config provider, but where you initialize the middleware you have no access to the config.

func ContextualizedMiddleware(provider func(context.Context) (opts cors.Options, enabled bool)) negroni.HandlerFunc {
return func(rw http.ResponseWriter, r *http.Request, next http.HandlerFunc) {
options, enabled := provider(r.Context())
Expand Down