From 6f5ebc3cdec50d3cdbc1652f3ca14a96259df30c Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Thu, 27 Jun 2024 12:03:40 +0200 Subject: [PATCH 1/2] feat(oonirun): allow true JSON richer input (#1629) Closes https://github.com/ooni/probe/issues/2759 --- internal/engine/experimentbuilder.go | 31 +++- internal/engine/experimentbuilder_test.go | 118 +++++++++++++ internal/mocks/experimentbuilder.go | 12 +- internal/mocks/experimentbuilder_test.go | 14 ++ internal/model/experiment.go | 10 ++ internal/oonirun/experiment.go | 34 +++- internal/oonirun/experiment_test.go | 110 +++++++++++- internal/oonirun/v1_test.go | 4 + internal/oonirun/v2.go | 5 +- internal/oonirun/v2_test.go | 31 ++-- internal/registry/factory.go | 53 +++++- internal/registry/factory_test.go | 196 ++++++++++++++++++++-- internal/registry/portfiltering.go | 4 +- internal/registry/telegram.go | 4 +- internal/registry/webconnectivity.go | 4 +- 15 files changed, 572 insertions(+), 58 deletions(-) diff --git a/internal/engine/experimentbuilder.go b/internal/engine/experimentbuilder.go index 330777957..7c9469a85 100644 --- a/internal/engine/experimentbuilder.go +++ b/internal/engine/experimentbuilder.go @@ -5,11 +5,19 @@ package engine // import ( + "encoding/json" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/registry" ) -// experimentBuilder implements ExperimentBuilder. +// TODO(bassosimone,DecFox): we should eventually finish merging the code in +// file with the code inside the ./internal/registry package. +// +// If there's time, this could happen at the end of the current (as of 2024-06-27) +// richer input work, otherwise any time in the future is actually fine. + +// experimentBuilder implements [model.ExperimentBuilder]. // // This type is now just a tiny wrapper around registry.Factory. type experimentBuilder struct { @@ -24,37 +32,42 @@ type experimentBuilder struct { var _ model.ExperimentBuilder = &experimentBuilder{} -// Interruptible implements ExperimentBuilder.Interruptible. +// Interruptible implements [model.ExperimentBuilder]. func (b *experimentBuilder) Interruptible() bool { return b.factory.Interruptible() } -// InputPolicy implements ExperimentBuilder.InputPolicy. +// InputPolicy implements [model.ExperimentBuilder]. func (b *experimentBuilder) InputPolicy() model.InputPolicy { return b.factory.InputPolicy() } -// Options implements ExperimentBuilder.Options. +// Options implements [model.ExperimentBuilder]. func (b *experimentBuilder) Options() (map[string]model.ExperimentOptionInfo, error) { return b.factory.Options() } -// SetOptionAny implements ExperimentBuilder.SetOptionAny. +// SetOptionAny implements [model.ExperimentBuilder]. func (b *experimentBuilder) SetOptionAny(key string, value any) error { return b.factory.SetOptionAny(key, value) } -// SetOptionsAny implements ExperimentBuilder.SetOptionsAny. +// SetOptionsAny implements [model.ExperimentBuilder]. func (b *experimentBuilder) SetOptionsAny(options map[string]any) error { return b.factory.SetOptionsAny(options) } -// SetCallbacks implements ExperimentBuilder.SetCallbacks. +// SetOptionsJSON implements [model.ExperimentBuilder]. +func (b *experimentBuilder) SetOptionsJSON(value json.RawMessage) error { + return b.factory.SetOptionsJSON(value) +} + +// SetCallbacks implements [model.ExperimentBuilder]. func (b *experimentBuilder) SetCallbacks(callbacks model.ExperimentCallbacks) { b.callbacks = callbacks } -// NewExperiment creates the experiment +// NewExperiment creates a new [model.Experiment] instance. func (b *experimentBuilder) NewExperiment() model.Experiment { measurer := b.factory.NewExperimentMeasurer() experiment := newExperiment(b.session, measurer) @@ -67,7 +80,7 @@ func (b *experimentBuilder) NewTargetLoader(config *model.ExperimentTargetLoader return b.factory.NewTargetLoader(config) } -// newExperimentBuilder creates a new experimentBuilder instance. +// newExperimentBuilder creates a new [*experimentBuilder] instance. func newExperimentBuilder(session *Session, name string) (*experimentBuilder, error) { factory, err := registry.NewFactory(name, session.kvStore, session.logger) if err != nil { diff --git a/internal/engine/experimentbuilder_test.go b/internal/engine/experimentbuilder_test.go index d81e0bb7b..6d1c5e4f8 100644 --- a/internal/engine/experimentbuilder_test.go +++ b/internal/engine/experimentbuilder_test.go @@ -2,9 +2,11 @@ package engine import ( "context" + "encoding/json" "errors" "testing" + "github.com/google/go-cmp/cmp" "github.com/ooni/probe-cli/v3/internal/model" ) @@ -49,3 +51,119 @@ func TestExperimentBuilderEngineWebConnectivity(t *testing.T) { t.Fatal("expected zero length targets") } } + +func TestExperimentBuilderBasicOperations(t *testing.T) { + // create a session for testing that does not use the network at all + sess := newSessionForTestingNoLookups(t) + + // create an experiment builder for example + builder, err := sess.NewExperimentBuilder("example") + if err != nil { + t.Fatal(err) + } + + // example should be interruptible + t.Run("Interruptible", func(t *testing.T) { + if !builder.Interruptible() { + t.Fatal("example should be interruptible") + } + }) + + // we expect to see the InputNone input policy + t.Run("InputPolicy", func(t *testing.T) { + if builder.InputPolicy() != model.InputNone { + t.Fatal("unexpectyed input policy") + } + }) + + // get the options and check whether they are what we expect + t.Run("Options", func(t *testing.T) { + options, err := builder.Options() + if err != nil { + t.Fatal(err) + } + expectOptions := map[string]model.ExperimentOptionInfo{ + "Message": {Doc: "Message to emit at test completion", Type: "string", Value: "Good day from the example experiment!"}, + "ReturnError": {Doc: "Toogle to return a mocked error", Type: "bool", Value: false}, + "SleepTime": {Doc: "Amount of time to sleep for in nanosecond", Type: "int64", Value: int64(1000000000)}, + } + if diff := cmp.Diff(expectOptions, options); diff != "" { + t.Fatal(diff) + } + }) + + // we can set a specific existing option + t.Run("SetOptionAny", func(t *testing.T) { + if err := builder.SetOptionAny("Message", "foobar"); err != nil { + t.Fatal(err) + } + options, err := builder.Options() + if err != nil { + t.Fatal(err) + } + expectOptions := map[string]model.ExperimentOptionInfo{ + "Message": {Doc: "Message to emit at test completion", Type: "string", Value: "foobar"}, + "ReturnError": {Doc: "Toogle to return a mocked error", Type: "bool", Value: false}, + "SleepTime": {Doc: "Amount of time to sleep for in nanosecond", Type: "int64", Value: int64(1000000000)}, + } + if diff := cmp.Diff(expectOptions, options); diff != "" { + t.Fatal(diff) + } + }) + + // we can set all options at the same time + t.Run("SetOptions", func(t *testing.T) { + inputs := map[string]any{ + "Message": "foobar", + "ReturnError": true, + } + if err := builder.SetOptionsAny(inputs); err != nil { + t.Fatal(err) + } + options, err := builder.Options() + if err != nil { + t.Fatal(err) + } + expectOptions := map[string]model.ExperimentOptionInfo{ + "Message": {Doc: "Message to emit at test completion", Type: "string", Value: "foobar"}, + "ReturnError": {Doc: "Toogle to return a mocked error", Type: "bool", Value: true}, + "SleepTime": {Doc: "Amount of time to sleep for in nanosecond", Type: "int64", Value: int64(1000000000)}, + } + if diff := cmp.Diff(expectOptions, options); diff != "" { + t.Fatal(diff) + } + }) + + // we can set all options using JSON + t.Run("SetOptionsJSON", func(t *testing.T) { + inputs := json.RawMessage(`{ + "Message": "foobar", + "ReturnError": true + }`) + if err := builder.SetOptionsJSON(inputs); err != nil { + t.Fatal(err) + } + options, err := builder.Options() + if err != nil { + t.Fatal(err) + } + expectOptions := map[string]model.ExperimentOptionInfo{ + "Message": {Doc: "Message to emit at test completion", Type: "string", Value: "foobar"}, + "ReturnError": {Doc: "Toogle to return a mocked error", Type: "bool", Value: true}, + "SleepTime": {Doc: "Amount of time to sleep for in nanosecond", Type: "int64", Value: int64(1000000000)}, + } + if diff := cmp.Diff(expectOptions, options); diff != "" { + t.Fatal(diff) + } + }) + + // TODO(bassosimone): we could possibly add more checks here. I am not doing this + // right now, because https://github.com/ooni/probe-cli/pull/1629 mostly cares about + // providing input and the rest of the codebase did not change. + // + // Also, it would make sense to eventually merge experimentbuilder.go with the + // ./internal/registry package, which also has coverage. + // + // In conclusion, our main objective for now is to make sure we don't screw the + // pooch when setting options using the experiment builder. +} diff --git a/internal/mocks/experimentbuilder.go b/internal/mocks/experimentbuilder.go index 1f6a27187..bcb965274 100644 --- a/internal/mocks/experimentbuilder.go +++ b/internal/mocks/experimentbuilder.go @@ -1,6 +1,10 @@ package mocks -import "github.com/ooni/probe-cli/v3/internal/model" +import ( + "encoding/json" + + "github.com/ooni/probe-cli/v3/internal/model" +) // ExperimentBuilder mocks model.ExperimentBuilder. type ExperimentBuilder struct { @@ -14,6 +18,8 @@ type ExperimentBuilder struct { MockSetOptionsAny func(options map[string]any) error + MockSetOptionsJSON func(value json.RawMessage) error + MockSetCallbacks func(callbacks model.ExperimentCallbacks) MockNewExperiment func() model.Experiment @@ -43,6 +49,10 @@ func (eb *ExperimentBuilder) SetOptionsAny(options map[string]any) error { return eb.MockSetOptionsAny(options) } +func (eb *ExperimentBuilder) SetOptionsJSON(value json.RawMessage) error { + return eb.MockSetOptionsJSON(value) +} + func (eb *ExperimentBuilder) SetCallbacks(callbacks model.ExperimentCallbacks) { eb.MockSetCallbacks(callbacks) } diff --git a/internal/mocks/experimentbuilder_test.go b/internal/mocks/experimentbuilder_test.go index 55ba1783f..7e90fff66 100644 --- a/internal/mocks/experimentbuilder_test.go +++ b/internal/mocks/experimentbuilder_test.go @@ -1,6 +1,7 @@ package mocks import ( + "encoding/json" "errors" "testing" @@ -72,6 +73,19 @@ func TestExperimentBuilder(t *testing.T) { } }) + t.Run("SetOptionsJSON", func(t *testing.T) { + expected := errors.New("mocked error") + eb := &ExperimentBuilder{ + MockSetOptionsJSON: func(value json.RawMessage) error { + return expected + }, + } + err := eb.SetOptionsJSON([]byte(`{}`)) + if !errors.Is(err, expected) { + t.Fatal("unexpected value") + } + }) + t.Run("SetCallbacks", func(t *testing.T) { var called bool eb := &ExperimentBuilder{ diff --git a/internal/model/experiment.go b/internal/model/experiment.go index 5aada21ff..b514df3de 100644 --- a/internal/model/experiment.go +++ b/internal/model/experiment.go @@ -7,6 +7,7 @@ package model import ( "context" + "encoding/json" "errors" "fmt" ) @@ -235,6 +236,12 @@ type ExperimentBuilder interface { // the SetOptionAny method for more information. SetOptionsAny(options map[string]any) error + // SetOptionsJSON uses the given [json.RawMessage] to initialize fields + // of the configuration for running the experiment. The [json.RawMessage], if + // not empty, MUST contain a serialization of the experiment config's + // type. An empty [json.RawMessage] will silently be ignored. + SetOptionsJSON(value json.RawMessage) error + // SetCallbacks sets the experiment's interactive callbacks. SetCallbacks(callbacks ExperimentCallbacks) @@ -290,6 +297,9 @@ type ExperimentOptionInfo struct { // Type contains the type. Type string + + // Value contains the current option value. + Value any } // ExperimentTargetLoader loads targets from local or remote sources. diff --git a/internal/oonirun/experiment.go b/internal/oonirun/experiment.go index c4613107d..1fe1c7079 100644 --- a/internal/oonirun/experiment.go +++ b/internal/oonirun/experiment.go @@ -6,6 +6,7 @@ package oonirun import ( "context" + "encoding/json" "fmt" "math/rand" "strings" @@ -25,9 +26,18 @@ type Experiment struct { // Annotations contains OPTIONAL Annotations for the experiment. Annotations map[string]string - // ExtraOptions contains OPTIONAL extra options for the experiment. + // ExtraOptions contains OPTIONAL extra options that modify the + // default experiment-specific configuration. We apply + // the changes described by this field after using the InitialOptions + // field to initialize the experiment-specific configuration. ExtraOptions map[string]any + // InitialOptions contains an OPTIONAL [json.RawMessage] object + // used to initialize the default experiment-specific + // configuration. After we have initialized the configuration + // as such, we then apply the changes described by the ExtraOptions. + InitialOptions json.RawMessage + // Inputs contains the OPTIONAL experiment Inputs Inputs []string @@ -82,16 +92,18 @@ func (ed *Experiment) Run(ctx context.Context) error { return err } - // TODO(bassosimone,DecFox): when we're executed by OONI Run v2, it probably makes - // slightly more sense to set options from a json.RawMessage because the current - // command line limitation is that it's hard to set non scalar parameters and instead - // with using OONI Run v2 we can completely bypass such a limitation. + // TODO(bassosimone): we need another patch after the current one + // to correctly serialize the options as configured using InitialOptions + // and ExtraOptions otherwise the Measurement.Options field turns out + // to always be empty and this is highly suboptimal for us. + // + // The next patch is https://github.com/ooni/probe-cli/pull/1630. // 2. configure experiment's options // // This MUST happen before loading targets because the options will // possibly be used to produce richer input targets. - if err := builder.SetOptionsAny(ed.ExtraOptions); err != nil { + if err := ed.setOptions(builder); err != nil { return err } @@ -142,6 +154,16 @@ func (ed *Experiment) Run(ctx context.Context) error { return inputProcessor.Run(ctx) } +func (ed *Experiment) setOptions(builder model.ExperimentBuilder) error { + // We first unmarshal the InitialOptions into the experiment + // configuration and afterwards we modify the configuration using + // the values contained inside the ExtraOptions field. + if err := builder.SetOptionsJSON(ed.InitialOptions); err != nil { + return err + } + return builder.SetOptionsAny(ed.ExtraOptions) +} + // inputProcessor is an alias for model.ExperimentInputProcessor type inputProcessor = model.ExperimentInputProcessor diff --git a/internal/oonirun/experiment_test.go b/internal/oonirun/experiment_test.go index 1fd38abb0..cbe24d803 100644 --- a/internal/oonirun/experiment_test.go +++ b/internal/oonirun/experiment_test.go @@ -2,12 +2,15 @@ package oonirun import ( "context" + "encoding/json" "errors" "reflect" "sort" "testing" "time" + "github.com/google/go-cmp/cmp" + "github.com/ooni/probe-cli/v3/internal/engine" "github.com/ooni/probe-cli/v3/internal/mocks" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/testingx" @@ -16,6 +19,7 @@ import ( func TestExperimentRunWithFailureToSubmitAndShuffle(t *testing.T) { shuffledInputsPrev := experimentShuffledInputs.Load() var calledSetOptionsAny int + var calledSetOptionsJSON int var failedToSubmit int var calledKibiBytesReceived int var calledKibiBytesSent int @@ -40,6 +44,10 @@ func TestExperimentRunWithFailureToSubmitAndShuffle(t *testing.T) { MockInputPolicy: func() model.InputPolicy { return model.InputOptional }, + MockSetOptionsJSON: func(value json.RawMessage) error { + calledSetOptionsJSON++ + return nil + }, MockSetOptionsAny: func(options map[string]any) error { calledSetOptionsAny++ return nil @@ -109,6 +117,9 @@ func TestExperimentRunWithFailureToSubmitAndShuffle(t *testing.T) { if calledSetOptionsAny < 1 { t.Fatal("should have called SetOptionsAny") } + if calledSetOptionsJSON < 1 { + t.Fatal("should have called SetOptionsJSON") + } if calledKibiBytesReceived < 1 { t.Fatal("did not call KibiBytesReceived") } @@ -117,6 +128,67 @@ func TestExperimentRunWithFailureToSubmitAndShuffle(t *testing.T) { } } +// This test ensures that we honour InitialOptions then ExtraOptions. +func TestExperimentSetOptions(t *testing.T) { + + // create the Experiment we're using for this test + exp := &Experiment{ + ExtraOptions: map[string]any{ + "Message": "jarjarbinks", + }, + InitialOptions: []byte(`{"Message": "foobar", "ReturnError": true}`), + Name: "example", + + // TODO(bassosimone): A zero-value session works here. The proper change + // however would be to write a engine.NewExperimentBuilder factory that takes + // as input an interface for the session. This would help testing. + Session: &engine.Session{}, + } + + // create the experiment builder manually + builder, err := exp.newExperimentBuilder(exp.Name) + if err != nil { + t.Fatal(err) + } + + // invoke the method we're testing + if err := exp.setOptions(builder); err != nil { + t.Fatal(err) + } + + // obtain the options + options, err := builder.Options() + if err != nil { + t.Fatal(err) + } + + // describe what we expect to happen + // + // we basically want ExtraOptions to override InitialOptions + expect := map[string]model.ExperimentOptionInfo{ + "Message": { + Doc: "Message to emit at test completion", + Type: "string", + Value: string("jarjarbinks"), // set by ExtraOptions + }, + "ReturnError": { + Doc: "Toogle to return a mocked error", + Type: "bool", + Value: bool(true), // set by InitialOptions + }, + "SleepTime": { + Doc: "Amount of time to sleep for in nanosecond", + Type: "int64", + Value: int64(1000000000), // still the default nonzero value + }, + } + + // make sure the result equals expectation + if diff := cmp.Diff(expect, options); diff != "" { + t.Fatal(diff) + } +} + func Test_experimentOptionsToStringList(t *testing.T) { type args struct { options map[string]any @@ -198,10 +270,34 @@ func TestExperimentRun(t *testing.T) { args: args{}, expectErr: errMocked, }, { - name: "cannot set options", + name: "cannot set InitialOptions", + fields: fields{ + newExperimentBuilderFn: func(experimentName string) (model.ExperimentBuilder, error) { + eb := &mocks.ExperimentBuilder{ + MockSetOptionsJSON: func(value json.RawMessage) error { + return errMocked + }, + } + return eb, nil + }, + newTargetLoaderFn: func(builder model.ExperimentBuilder) targetLoader { + return &mocks.ExperimentTargetLoader{ + MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) { + return []model.ExperimentTarget{}, nil + }, + } + }, + }, + args: args{}, + expectErr: errMocked, + }, { + name: "cannot set ExtraOptions", fields: fields{ newExperimentBuilderFn: func(experimentName string) (model.ExperimentBuilder, error) { eb := &mocks.ExperimentBuilder{ + MockSetOptionsJSON: func(value json.RawMessage) error { + return nil + }, MockSetOptionsAny: func(options map[string]any) error { return errMocked }, @@ -226,6 +322,9 @@ func TestExperimentRun(t *testing.T) { MockInputPolicy: func() model.InputPolicy { return model.InputOptional }, + MockSetOptionsJSON: func(value json.RawMessage) error { + return nil + }, MockSetOptionsAny: func(options map[string]any) error { return nil }, @@ -255,6 +354,9 @@ func TestExperimentRun(t *testing.T) { MockInputPolicy: func() model.InputPolicy { return model.InputOptional }, + MockSetOptionsJSON: func(value json.RawMessage) error { + return nil + }, MockSetOptionsAny: func(options map[string]any) error { return nil }, @@ -298,6 +400,9 @@ func TestExperimentRun(t *testing.T) { MockInputPolicy: func() model.InputPolicy { return model.InputOptional }, + MockSetOptionsJSON: func(value json.RawMessage) error { + return nil + }, MockSetOptionsAny: func(options map[string]any) error { return nil }, @@ -344,6 +449,9 @@ func TestExperimentRun(t *testing.T) { MockInputPolicy: func() model.InputPolicy { return model.InputOptional }, + MockSetOptionsJSON: func(value json.RawMessage) error { + return nil + }, MockSetOptionsAny: func(options map[string]any) error { return nil }, diff --git a/internal/oonirun/v1_test.go b/internal/oonirun/v1_test.go index 910d910b0..b1109a8ef 100644 --- a/internal/oonirun/v1_test.go +++ b/internal/oonirun/v1_test.go @@ -2,6 +2,7 @@ package oonirun import ( "context" + "encoding/json" "errors" "net/http" "strings" @@ -23,6 +24,9 @@ func newMinimalFakeSession() *mocks.Session { MockInputPolicy: func() model.InputPolicy { return model.InputNone }, + MockSetOptionsJSON: func(value json.RawMessage) error { + return nil + }, MockSetOptionsAny: func(options map[string]any) error { return nil }, diff --git a/internal/oonirun/v2.go b/internal/oonirun/v2.go index 4330b07b7..6552ad582 100644 --- a/internal/oonirun/v2.go +++ b/internal/oonirun/v2.go @@ -54,7 +54,7 @@ type V2Nettest struct { // `Safe` will be available for the experiment run, but omitted from // the serialized Measurement that the experiment builder will submit // to the OONI backend. - Options map[string]any `json:"options"` + Options json.RawMessage `json:"options"` // TestName contains the nettest name. TestName string `json:"test_name"` @@ -183,7 +183,8 @@ func V2MeasureDescriptor(ctx context.Context, config *LinkConfig, desc *V2Descri // construct an experiment from the current nettest exp := &Experiment{ Annotations: config.Annotations, - ExtraOptions: nettest.Options, + ExtraOptions: make(map[string]any), + InitialOptions: nettest.Options, Inputs: nettest.Inputs, InputFilePaths: nil, MaxRuntime: config.MaxRuntime, diff --git a/internal/oonirun/v2_test.go b/internal/oonirun/v2_test.go index d849c6d08..b77b9b7bf 100644 --- a/internal/oonirun/v2_test.go +++ b/internal/oonirun/v2_test.go @@ -7,7 +7,6 @@ import ( "net/http" "net/http/httptest" "testing" - "time" "github.com/ooni/probe-cli/v3/internal/httpclientx" "github.com/ooni/probe-cli/v3/internal/kvstore" @@ -27,9 +26,9 @@ func TestOONIRunV2LinkCommonCase(t *testing.T) { Author: "", Nettests: []V2Nettest{{ Inputs: []string{}, - Options: map[string]any{ - "SleepTime": int64(10 * time.Millisecond), - }, + Options: json.RawMessage(`{ + "SleepTime": 10000000 + }`), TestName: "example", }}, } @@ -73,9 +72,9 @@ func TestOONIRunV2LinkCannotUpdateCache(t *testing.T) { Author: "", Nettests: []V2Nettest{{ Inputs: []string{}, - Options: map[string]any{ - "SleepTime": int64(10 * time.Millisecond), - }, + Options: json.RawMessage(`{ + "SleepTime": 10000000 + }`), TestName: "example", }}, } @@ -132,9 +131,9 @@ func TestOONIRunV2LinkWithoutAcceptChanges(t *testing.T) { Author: "", Nettests: []V2Nettest{{ Inputs: []string{}, - Options: map[string]any{ - "SleepTime": int64(10 * time.Millisecond), - }, + Options: json.RawMessage(`{ + "SleepTime": 10000000 + }`), TestName: "example", }}, } @@ -220,9 +219,9 @@ func TestOONIRunV2LinkEmptyTestName(t *testing.T) { Author: "", Nettests: []V2Nettest{{ Inputs: []string{}, - Options: map[string]any{ - "SleepTime": int64(10 * time.Millisecond), - }, + Options: json.RawMessage(`{ + "SleepTime": 10000000 + }`), TestName: "", // empty! }}, } @@ -374,6 +373,9 @@ func TestV2MeasureDescriptor(t *testing.T) { MockInputPolicy: func() model.InputPolicy { return model.InputNone }, + MockSetOptionsJSON: func(value json.RawMessage) error { + return nil + }, MockSetOptionsAny: func(options map[string]any) error { return nil }, @@ -426,7 +428,7 @@ func TestV2MeasureDescriptor(t *testing.T) { Author: "", Nettests: []V2Nettest{{ Inputs: []string{}, - Options: map[string]any{}, + Options: json.RawMessage(`{}`), TestName: "example", }}, } @@ -532,5 +534,4 @@ func TestV2DescriptorCacheLoad(t *testing.T) { t.Fatal("expected nil cache") } }) - } diff --git a/internal/registry/factory.go b/internal/registry/factory.go index ee73e95a8..813e9f7a4 100644 --- a/internal/registry/factory.go +++ b/internal/registry/factory.go @@ -5,6 +5,7 @@ package registry // import ( + "encoding/json" "errors" "fmt" "math" @@ -105,22 +106,47 @@ var ( // Options returns the options exposed by this experiment. func (b *Factory) Options() (map[string]model.ExperimentOptionInfo, error) { + // create the result value result := make(map[string]model.ExperimentOptionInfo) + + // make sure we're dealing with a pointer ptrinfo := reflect.ValueOf(b.config) if ptrinfo.Kind() != reflect.Ptr { return nil, ErrConfigIsNotAStructPointer } - structinfo := ptrinfo.Elem().Type() - if structinfo.Kind() != reflect.Struct { + + // obtain information about the value and its type + valueinfo := ptrinfo.Elem() + typeinfo := valueinfo.Type() + + // make sure we're dealing with a struct + if typeinfo.Kind() != reflect.Struct { return nil, ErrConfigIsNotAStructPointer } - for i := 0; i < structinfo.NumField(); i++ { - field := structinfo.Field(i) - result[field.Name] = model.ExperimentOptionInfo{ - Doc: field.Tag.Get("ooni"), - Type: field.Type.String(), + + // cycle through the fields + for i := 0; i < typeinfo.NumField(); i++ { + fieldType, fieldValue := typeinfo.Field(i), valueinfo.Field(i) + + // do not include private fields into our list of fields + if !fieldType.IsExported() { + continue + } + + // skip fields that are missing an `ooni` tag + docs := fieldType.Tag.Get("ooni") + if docs == "" { + continue + } + + // create a description of this field + result[fieldType.Name] = model.ExperimentOptionInfo{ + Doc: docs, + Type: fieldType.Type.String(), + Value: fieldValue.Interface(), } } + return result, nil } @@ -228,6 +254,19 @@ func (b *Factory) SetOptionsAny(options map[string]any) error { return nil } +// SetOptionsJSON unmarshals the given [json.RawMessage] inside +// the experiment specific configuration. +func (b *Factory) SetOptionsJSON(value json.RawMessage) error { + // handle the case where the options are empty + if len(value) <= 0 { + return nil + } + + // otherwise unmarshal into the configuration, which we assume + // to be a pointer to a structure. + return json.Unmarshal(value, b.config) +} + // fieldbyname return v's field whose name is equal to the given key. func (b *Factory) fieldbyname(v interface{}, key string) (reflect.Value, error) { // See https://stackoverflow.com/a/6396678/4354461 diff --git a/internal/registry/factory_test.go b/internal/registry/factory_test.go index c92f03157..d4a3d8368 100644 --- a/internal/registry/factory_test.go +++ b/internal/registry/factory_test.go @@ -2,10 +2,12 @@ package registry import ( "context" + "encoding/json" "errors" "fmt" "math" "os" + "reflect" "testing" "github.com/apex/log" @@ -19,14 +21,23 @@ import ( "github.com/ooni/probe-cli/v3/internal/targetloading" ) -type fakeExperimentConfig struct { - Chan chan any `ooni:"we cannot set this"` - String string `ooni:"a string"` - Truth bool `ooni:"something that no-one knows"` - Value int64 `ooni:"a number"` -} +func TestFactoryOptions(t *testing.T) { + + // the fake configuration we're using in this test + type fakeExperimentConfig struct { + // values that should be included into the Options return value + Chan chan any `ooni:"we cannot set this"` + String string `ooni:"a string"` + Truth bool `ooni:"something that no-one knows"` + Value int64 `ooni:"a number"` + + // values that should not be included because they're private + private int64 `ooni:"a private number"` + + // values that should not be included because they lack "ooni"'s tag + Invisible int64 + } -func TestExperimentBuilderOptions(t *testing.T) { t.Run("when config is not a pointer", func(t *testing.T) { b := &Factory{ config: 17, @@ -55,7 +66,14 @@ func TestExperimentBuilderOptions(t *testing.T) { }) t.Run("when config is a pointer to struct", func(t *testing.T) { - config := &fakeExperimentConfig{} + config := &fakeExperimentConfig{ + Chan: make(chan any), + String: "foobar", + Truth: true, + Value: 177114, + private: 55, + Invisible: 9876, + } b := &Factory{ config: config, } @@ -63,6 +81,7 @@ func TestExperimentBuilderOptions(t *testing.T) { if err != nil { t.Fatal(err) } + for name, value := range options { switch name { case "Chan": @@ -72,6 +91,10 @@ func TestExperimentBuilderOptions(t *testing.T) { if value.Type != "chan interface {}" { t.Fatal("invalid type", value.Type) } + if value.Value.(chan any) == nil { + t.Fatal("expected non-nil channel here") + } + case "String": if value.Doc != "a string" { t.Fatal("invalid doc") @@ -79,6 +102,10 @@ func TestExperimentBuilderOptions(t *testing.T) { if value.Type != "string" { t.Fatal("invalid type", value.Type) } + if v := value.Value.(string); v != "foobar" { + t.Fatal("unexpected string value", v) + } + case "Truth": if value.Doc != "something that no-one knows" { t.Fatal("invalid doc") @@ -86,6 +113,10 @@ func TestExperimentBuilderOptions(t *testing.T) { if value.Type != "bool" { t.Fatal("invalid type", value.Type) } + if v := value.Value.(bool); !v { + t.Fatal("unexpected bool value", v) + } + case "Value": if value.Doc != "a number" { t.Fatal("invalid doc") @@ -93,14 +124,27 @@ func TestExperimentBuilderOptions(t *testing.T) { if value.Type != "int64" { t.Fatal("invalid type", value.Type) } + if v := value.Value.(int64); v != 177114 { + t.Fatal("unexpected int64 value", v) + } + default: - t.Fatal("unknown name", name) + t.Fatal("unexpected option name", name) } } }) } -func TestExperimentBuilderSetOptionAny(t *testing.T) { +func TestFactorySetOptionAny(t *testing.T) { + + // the fake configuration we're using in this test + type fakeExperimentConfig struct { + Chan chan any `ooni:"we cannot set this"` + String string `ooni:"a string"` + Truth bool `ooni:"something that no-one knows"` + Value int64 `ooni:"a number"` + } + var inputs = []struct { TestCaseName string InitialConfig any @@ -366,7 +410,17 @@ func TestExperimentBuilderSetOptionAny(t *testing.T) { } } -func TestExperimentBuilderSetOptionsAny(t *testing.T) { +func TestFactorySetOptionsAny(t *testing.T) { + + // the fake configuration we're using in this test + type fakeExperimentConfig struct { + // values that should be included into the Options return value + Chan chan any `ooni:"we cannot set this"` + String string `ooni:"a string"` + Truth bool `ooni:"something that no-one knows"` + Value int64 `ooni:"a number"` + } + b := &Factory{config: &fakeExperimentConfig{}} t.Run("we correctly handle an empty map", func(t *testing.T) { @@ -409,6 +463,107 @@ func TestExperimentBuilderSetOptionsAny(t *testing.T) { }) } +func TestFactorySetOptionsJSON(t *testing.T) { + + // PersonRecord is a fake experiment configuration. + // + // Note how the `ooni` tag here is missing because we don't care + // about whether such a tag is present when using JSON. + type PersonRecord struct { + Name string + Age int64 + Friends []string + } + + // testcase is a test case for this function. + type testcase struct { + // name is the name of the test case + name string + + // mutableConfig is the config in which we should unmarshal the JSON + mutableConfig *PersonRecord + + // rawJSON contains the raw JSON to unmarshal into mutableConfig + rawJSON json.RawMessage + + // expectErr is the error we expect + expectErr error + + // expectRecord is what we expectRecord to see in the end + expectRecord *PersonRecord + } + + cases := []testcase{ + { + name: "we correctly accept zero-length options", + mutableConfig: &PersonRecord{ + Name: "foo", + Age: 55, + Friends: []string{"bar", "baz"}, + }, + rawJSON: []byte{}, + expectRecord: &PersonRecord{ + Name: "foo", + Age: 55, + Friends: []string{"bar", "baz"}, + }, + }, + + { + name: "we return an error on JSON parsing error", + mutableConfig: &PersonRecord{}, + rawJSON: []byte(`{`), + expectErr: errors.New("unexpected end of JSON input"), + expectRecord: &PersonRecord{}, + }, + + { + name: "we correctly unmarshal into the existing config", + mutableConfig: &PersonRecord{ + Name: "foo", + Age: 55, + Friends: []string{"bar", "baz"}, + }, + rawJSON: []byte(`{"Friends":["foo","oof"]}`), + expectErr: nil, + expectRecord: &PersonRecord{ + Name: "foo", + Age: 55, + Friends: []string{"foo", "oof"}, + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + + // create the factory to use + factory := &Factory{config: tc.mutableConfig} + + // unmarshal into the mutableConfig + err := factory.SetOptionsJSON(tc.rawJSON) + + // make sure the error is the one we actually expect + switch { + case err == nil && tc.expectErr == nil: + if diff := cmp.Diff(tc.expectRecord, tc.mutableConfig); diff != "" { + t.Fatal(diff) + } + return + + case err != nil && tc.expectErr != nil: + if err.Error() != tc.expectErr.Error() { + t.Fatal("expected", tc.expectErr, "got", err) + } + return + + default: + t.Fatal("expected", tc.expectErr, "got", err) + } + }) + } +} + func TestNewFactory(t *testing.T) { // experimentSpecificExpectations contains expectations for an experiment type experimentSpecificExpectations struct { @@ -942,3 +1097,22 @@ func TestFactoryNewTargetLoader(t *testing.T) { } }) } + +// This test is important because SetOptionsJSON assumes that the experiment +// config is a struct pointer into which it is possible to write +func TestExperimentConfigIsAlwaysAPointerToStruct(t *testing.T) { + for name, ffunc := range AllExperiments { + t.Run(name, func(t *testing.T) { + factory := ffunc() + config := factory.config + ctype := reflect.TypeOf(config) + if ctype.Kind() != reflect.Pointer { + t.Fatal("expected a pointer") + } + ctype = ctype.Elem() + if ctype.Kind() != reflect.Struct { + t.Fatal("expected a struct") + } + }) + } +} diff --git a/internal/registry/portfiltering.go b/internal/registry/portfiltering.go index 8c6a857df..2d7c67f25 100644 --- a/internal/registry/portfiltering.go +++ b/internal/registry/portfiltering.go @@ -15,11 +15,11 @@ func init() { return &Factory{ build: func(config any) model.ExperimentMeasurer { return portfiltering.NewExperimentMeasurer( - config.(portfiltering.Config), + *config.(*portfiltering.Config), ) }, canonicalName: canonicalName, - config: portfiltering.Config{}, + config: &portfiltering.Config{}, enabledByDefault: true, interruptible: false, inputPolicy: model.InputNone, diff --git a/internal/registry/telegram.go b/internal/registry/telegram.go index 987e64093..bf4f71aed 100644 --- a/internal/registry/telegram.go +++ b/internal/registry/telegram.go @@ -15,11 +15,11 @@ func init() { return &Factory{ build: func(config any) model.ExperimentMeasurer { return telegram.NewExperimentMeasurer( - config.(telegram.Config), + *config.(*telegram.Config), ) }, canonicalName: canonicalName, - config: telegram.Config{}, + config: &telegram.Config{}, enabledByDefault: true, interruptible: false, inputPolicy: model.InputNone, diff --git a/internal/registry/webconnectivity.go b/internal/registry/webconnectivity.go index 470bad802..0c2d79367 100644 --- a/internal/registry/webconnectivity.go +++ b/internal/registry/webconnectivity.go @@ -15,11 +15,11 @@ func init() { return &Factory{ build: func(config any) model.ExperimentMeasurer { return webconnectivity.NewExperimentMeasurer( - config.(webconnectivity.Config), + *config.(*webconnectivity.Config), ) }, canonicalName: canonicalName, - config: webconnectivity.Config{}, + config: &webconnectivity.Config{}, enabledByDefault: true, interruptible: false, inputPolicy: model.InputOrQueryBackend, From a73c754029c0072f18017240f654feff4ef7d7f5 Mon Sep 17 00:00:00 2001 From: Ain Ghazal <99027643+ainghazal@users.noreply.github.com> Date: Thu, 27 Jun 2024 17:26:29 +0200 Subject: [PATCH 2/2] feat(miniooni): add authentication to oonirun (#1627) ## Checklist - [x] I have read the [contribution guidelines](https://github.com/ooni/probe-cli/blob/master/CONTRIBUTING.md) - [x] reference issue for this pull request: https://github.com/ooni/probe/issues/2757 - [x] if you changed anything related to how experiments work and you need to reflect these changes in the ooni/spec repository, please link to the related ooni/spec pull request: - [x] if you changed code inside an experiment, make sure you bump its version number ## Description This commit adds a flag to pass an authentication bearer token to the miniooni oonirun invocation. Closes https://github.com/ooni/probe/issues/2757. --------- Co-authored-by: Simone Basso --- internal/cmd/miniooni/main.go | 8 + internal/cmd/miniooni/oonirun.go | 1 + internal/oonirun/link.go | 4 + internal/oonirun/v2.go | 65 +++++++- internal/oonirun/v2_test.go | 263 ++++++++++++++++++++++++++++++- 5 files changed, 335 insertions(+), 6 deletions(-) diff --git a/internal/cmd/miniooni/main.go b/internal/cmd/miniooni/main.go index db9e71da9..11add9c7e 100644 --- a/internal/cmd/miniooni/main.go +++ b/internal/cmd/miniooni/main.go @@ -26,6 +26,7 @@ import ( // Options contains the options you can set from the CLI. type Options struct { Annotations []string + AuthFile string Emoji bool ExtraOptions []string HomeDir string @@ -228,6 +229,13 @@ func registerOONIRun(rootCmd *cobra.Command, globalOptions *Options) { []string{}, "Path to the OONI Run v2 descriptor to run (may be specified multiple times)", ) + flags.StringVarP( + &globalOptions.AuthFile, + "bearer-token-file", + "", + "", + "Path to a file containing a bearer token for fetching a remote OONI Run v2 descriptor", + ) } // registerAllExperiments registers a subcommand for each experiment diff --git a/internal/cmd/miniooni/oonirun.go b/internal/cmd/miniooni/oonirun.go index 366839bad..d4c2a2690 100644 --- a/internal/cmd/miniooni/oonirun.go +++ b/internal/cmd/miniooni/oonirun.go @@ -21,6 +21,7 @@ func ooniRunMain(ctx context.Context, logger := sess.Logger() cfg := &oonirun.LinkConfig{ AcceptChanges: currentOptions.Yes, + AuthFile: currentOptions.AuthFile, Annotations: annotations, KVStore: sess.KeyValueStore(), MaxRuntime: currentOptions.MaxRuntime, diff --git a/internal/oonirun/link.go b/internal/oonirun/link.go index d5ceba265..3321db03b 100644 --- a/internal/oonirun/link.go +++ b/internal/oonirun/link.go @@ -19,6 +19,10 @@ type LinkConfig struct { // reviewing what it contains or what has changed. AcceptChanges bool + // AuthFile is OPTIONAL and will add an authentication header to the + // request used for fetching this OONI Run link. + AuthFile string + // Annotations contains OPTIONAL Annotations for the experiment. Annotations map[string]string diff --git a/internal/oonirun/v2.go b/internal/oonirun/v2.go index 6552ad582..c2a3ca065 100644 --- a/internal/oonirun/v2.go +++ b/internal/oonirun/v2.go @@ -5,15 +5,19 @@ package oonirun // import ( + "bufio" "context" + "encoding/base64" "encoding/json" "errors" "fmt" + "strings" "sync/atomic" "github.com/hexops/gotextdiff" "github.com/hexops/gotextdiff/myers" "github.com/hexops/gotextdiff/span" + "github.com/ooni/probe-cli/v3/internal/fsx" "github.com/ooni/probe-cli/v3/internal/httpclientx" "github.com/ooni/probe-cli/v3/internal/kvstore" "github.com/ooni/probe-cli/v3/internal/model" @@ -63,12 +67,16 @@ type V2Nettest struct { // getV2DescriptorFromHTTPSURL GETs a v2Descriptor instance from // a static URL (e.g., from a GitHub repo or from a Gist). func getV2DescriptorFromHTTPSURL(ctx context.Context, client model.HTTPClient, - logger model.Logger, URL string) (*V2Descriptor, error) { + logger model.Logger, URL, auth string) (*V2Descriptor, error) { + if auth != "" { + // we assume a bearer token + auth = fmt.Sprintf("Bearer %s", auth) + } return httpclientx.GetJSON[*V2Descriptor]( ctx, httpclientx.NewEndpoint(URL), &httpclientx.Config{ - Authorization: "", // not needed + Authorization: auth, Client: client, Logger: logger, UserAgent: model.HTTPHeaderUserAgent, @@ -140,9 +148,9 @@ func v2DescriptorCacheLoad(fsstore model.KeyValueStore) (*v2DescriptorCache, err // - err is the error that occurred, or nil in case of success. func (cache *v2DescriptorCache) PullChangesWithoutSideEffects( ctx context.Context, client model.HTTPClient, logger model.Logger, - URL string) (oldValue, newValue *V2Descriptor, err error) { + URL, auth string) (oldValue, newValue *V2Descriptor, err error) { oldValue = cache.Entries[URL] - newValue, err = getV2DescriptorFromHTTPSURL(ctx, client, logger, URL) + newValue, err = getV2DescriptorFromHTTPSURL(ctx, client, logger, URL, auth) return } @@ -263,7 +271,11 @@ func v2MeasureHTTPS(ctx context.Context, config *LinkConfig, URL string) error { // pull a possibly new descriptor without updating the old descriptor clnt := config.Session.DefaultHTTPClient() - oldValue, newValue, err := cache.PullChangesWithoutSideEffects(ctx, clnt, logger, URL) + auth, err := v2MaybeGetAuthenticationTokenFromFile(config.AuthFile) + if err != nil { + logger.Warnf("oonirun: failed to retrieve auth token: %v", err) + } + oldValue, newValue, err := cache.PullChangesWithoutSideEffects(ctx, clnt, logger, URL, auth) if err != nil { return err } @@ -290,3 +302,46 @@ func v2MeasureHTTPS(ctx context.Context, config *LinkConfig, URL string) error { // note: this function gracefully handles nil values return V2MeasureDescriptor(ctx, config, newValue) } + +func v2MaybeGetAuthenticationTokenFromFile(path string) (string, error) { + if path != "" { + return v2ReadBearerTokenFromFile(path) + } + return "", nil +} + +// v2ReadBearerTokenFromFile tries to extract a valid (base64) bearer token from +// the first line of the passed text file. +// If there is an error while reading from the file, the error will be returned. +// If we can read from the file but there's no valid token found, an empty string will be returned. +func v2ReadBearerTokenFromFile(fileName string) (string, error) { + filep, err := fsx.OpenFile(fileName) + if err != nil { + return "", err + } + defer filep.Close() + + scanner := bufio.NewScanner(filep) + + // Scan the first line + if scanner.Scan() { + line := scanner.Text() + + token := strings.TrimSpace(line) + + // if this is not a valid base64 token, return empty string + if _, err := base64.StdEncoding.DecodeString(token); err != nil { + return "", nil + } + + return token, nil + } + + // Check for any scanning error + if err := scanner.Err(); err != nil { + return "", err + } + + // Return empty string if file is empty + return "", nil +} diff --git a/internal/oonirun/v2_test.go b/internal/oonirun/v2_test.go index b77b9b7bf..63d3075af 100644 --- a/internal/oonirun/v2_test.go +++ b/internal/oonirun/v2_test.go @@ -6,6 +6,8 @@ import ( "errors" "net/http" "net/http/httptest" + "os" + "path/filepath" "testing" "github.com/ooni/probe-cli/v3/internal/httpclientx" @@ -262,6 +264,131 @@ func TestOONIRunV2LinkEmptyTestName(t *testing.T) { } } +func TestOONIRunV2LinkWithAuthentication(t *testing.T) { + + t.Run("authentication raises error if no token is passed", func(t *testing.T) { + token := "c2VjcmV0" + bearerToken := "Bearer " + token + + // make a local server that returns a reasonable descriptor for the example experiment + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + authHeader := r.Header.Get("Authorization") + if authHeader != bearerToken { + // If the header is not what expected, return a 401 Unauthorized status + http.Error(w, "Unauthorized", http.StatusUnauthorized) + return + } + descriptor := &V2Descriptor{ + Name: "", + Description: "", + Author: "", + Nettests: []V2Nettest{{ + Inputs: []string{}, + Options: json.RawMessage(`{ + "SleepTime": 10000000 + }`), + TestName: "example", + }}, + } + data, err := json.Marshal(descriptor) + runtimex.PanicOnError(err, "json.Marshal failed") + w.Write(data) + })) + + defer server.Close() + ctx := context.Background() + + // create a minimal link configuration + config := &LinkConfig{ + AcceptChanges: true, // avoid "oonirun: need to accept changes" error + Annotations: map[string]string{ + "platform": "linux", + }, + KVStore: &kvstore.Memory{}, + MaxRuntime: 0, + NoCollector: true, + NoJSON: true, + Random: false, + ReportFile: "", + Session: newMinimalFakeSession(), + } + + // construct a link runner relative to the local server URL + r := NewLinkRunner(config, server.URL) + + if err := r.Run(ctx); err != nil { + if err.Error() != "httpx: request failed" { + t.Fatal("expected error") + } + } + }) + + t.Run("authentication does not fail the auth token is passed", func(t *testing.T) { + token := "c2VjcmV0" + bearerToken := "Bearer " + token + + // make a local server that returns a reasonable descriptor for the example experiment + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + authHeader := r.Header.Get("Authorization") + if authHeader != bearerToken { + // If the header is not what expected, return a 401 Unauthorized status + http.Error(w, "Unauthorized", http.StatusUnauthorized) + return + } + descriptor := &V2Descriptor{ + Name: "", + Description: "", + Author: "", + Nettests: []V2Nettest{{ + Inputs: []string{}, + Options: json.RawMessage(`{ + "SleepTime": 10000000 + }`), + TestName: "example", + }}, + } + data, err := json.Marshal(descriptor) + runtimex.PanicOnError(err, "json.Marshal failed") + w.Write(data) + })) + + defer server.Close() + ctx := context.Background() + + authFile, err := os.CreateTemp(t.TempDir(), "token-") + if err != nil { + t.Fatal(err) + } + defer authFile.Close() + defer os.Remove(authFile.Name()) + + authFile.Write([]byte(token)) + + // create a minimal link configuration + config := &LinkConfig{ + AcceptChanges: true, // avoid "oonirun: need to accept changes" error + Annotations: map[string]string{ + "platform": "linux", + }, + AuthFile: authFile.Name(), + KVStore: &kvstore.Memory{}, + MaxRuntime: 0, + NoCollector: true, + NoJSON: true, + Random: false, + ReportFile: "", + Session: newMinimalFakeSession(), + } + + // construct a link runner relative to the local server URL + r := NewLinkRunner(config, server.URL) + + if err := r.Run(ctx); err != nil { + t.Fatal(err) + } + }) +} + func TestOONIRunV2LinkConnectionResetByPeer(t *testing.T) { // create a local server that will reset the connection immediately. // actually contains an empty test name, which is what we want to test @@ -509,7 +636,6 @@ func TestV2MeasureHTTPS(t *testing.T) { t.Fatal("unexpected err", err) } }) - } func TestV2DescriptorCacheLoad(t *testing.T) { @@ -535,3 +661,138 @@ func TestV2DescriptorCacheLoad(t *testing.T) { } }) } + +func Test_readFirstLineFromFile(t *testing.T) { + + t.Run("return empty string if file is empty", func(t *testing.T) { + f, err := os.CreateTemp(t.TempDir(), "auth-") + if err != nil { + t.Fatal(err) + } + f.Write([]byte("")) + defer f.Close() + defer os.Remove(f.Name()) + + line, err := v2ReadBearerTokenFromFile(f.Name()) + if line != "" { + t.Fatal("expected empty string") + } + if err != nil { + t.Fatal("expected err==nil") + } + }) + + t.Run("return empty string if first line is just whitespace", func(t *testing.T) { + f, err := os.CreateTemp(t.TempDir(), "auth-") + if err != nil { + t.Fatal(err) + } + f.Write([]byte(" \n")) + defer f.Close() + defer os.Remove(f.Name()) + + line, err := v2ReadBearerTokenFromFile(f.Name()) + if line != "" { + t.Fatal("expected empty string") + } + if err != nil { + t.Fatal("expected err==nil") + } + }) + + t.Run("return error if file does not exist", func(t *testing.T) { + line, err := v2ReadBearerTokenFromFile(filepath.Join(t.TempDir(), "non-existent")) + if line != "" { + t.Fatal("expected empty string") + } + if !errors.Is(err, os.ErrNotExist) { + t.Fatal("expected ErrNotExist") + } + }) + + t.Run("return first line with a file of one line without EOL", func(t *testing.T) { + f, err := os.CreateTemp(t.TempDir(), "auth-") + if err != nil { + t.Fatal(err) + } + + token := "c2VjcmV0" // b64("secret") + f.Write([]byte(token)) + defer f.Close() + defer os.Remove(f.Name()) + + line, err := v2ReadBearerTokenFromFile(f.Name()) + if line != token { + t.Fatalf("expected %s, got %s", token, line) + } + if err != nil { + t.Fatal("expected err==nil") + } + }) + + t.Run("return first line with a file of one line with EOL", func(t *testing.T) { + f, err := os.CreateTemp(t.TempDir(), "auth-") + if err != nil { + t.Fatal(err) + } + + token := "c2VjcmV0" // b64("secret") + f.Write(append([]byte(token), '\n')) + defer f.Close() + defer os.Remove(f.Name()) + + line, err := v2ReadBearerTokenFromFile(f.Name()) + if line != token { + t.Fatalf("expected %s, got %s", token, line) + } + if err != nil { + t.Fatal("expected err==nil") + } + }) + + t.Run("return first line with a file of >1 line", func(t *testing.T) { + f, err := os.CreateTemp(t.TempDir(), "auth-") + if err != nil { + t.Fatal(err) + } + + token := "c2VjcmV0" // b64("secret") + f.Write([]byte(token)) + f.Write([]byte("\n")) + f.Write([]byte("something\nelse\nand\nsomething\nmore")) + defer f.Close() + defer os.Remove(f.Name()) + + line, err := v2ReadBearerTokenFromFile(f.Name()) + if line != token { + t.Fatalf("expected %s, got %s", token, line) + } + if err != nil { + t.Fatal("expected err==nil") + } + }) + + t.Run("return empty string if not a valid b64 token", func(t *testing.T) { + f, err := os.CreateTemp(t.TempDir(), "auth-") + if err != nil { + t.Fatal(err) + } + + token := "secret!" + f.Write([]byte(token)) + f.Write([]byte("\n")) + f.Write([]byte(" antani\n")) + defer f.Close() + defer os.Remove(f.Name()) + + expected := "" + + line, err := v2ReadBearerTokenFromFile(f.Name()) + if line != expected { + t.Fatalf("expected empty string, got %s", line) + } + if err != nil { + t.Fatal("expected err==nil") + } + }) +}