Remove dependency of `os` package while creating mollie client and inject the token by the consumer actor/orchestrator component

[arifmahmudrana]

Is your feature request related to a problem? Please describe. It will be a good idea if we can pass the auth token using config rather than lookup the os for env because in many scenarios we may get the secret from the vault then it's much easier to inject the secret rather again setting it to env.

Describe the solution you'd like Don't look for the env variable use the config auth as the authentication token. Rather than using the config auth to store the env variable name we can use that to store the token. Here is the code where we are setting the authentication token. https://github.com/VictorAvelar/mollie-api-go/blob/master/mollie/mollie.go#L309

    tkn, ok := os.LookupEnv(mollie.config.auth)
    if ok {
        mollie.authentication = tkn
    }

We can change it to

    mollie.authentication = mollie.config.auth

and adjust other parts of the code or rewrite the whole client creation section. But it's not ideal that component is taking the value by itself it should be injected by the consumer component who is creating the client. It may take it from env, or vault or from some Rest API.

Describe alternatives you've considered I couldn't find any other alternative solutions.

Additional context We may need to adjust parts of code and test.

[VictorAvelar]

Thanks for opening an issue on this topic.

Do you think using this function WithAuthenticationValue could solve your use case?

If you are using a different environment variable name there is also SwitchAuthStrategy which allows you to change the name used to lookup in the environment variables.

Please let me know your thoughts on the above mentioned options.

---

Your proposed solution presents a challenge for me as it would introduce a breaking change without offering substantial additional value to the codebase. I believe the two options outlined above can effectively achieve your goal of programmatically setting the token without relying on environment variables.

If this assessment is incorrect, we can explore alternative solutions that avoid a complete overhaul of the implementation. Ideally, we should maintain compatibility for existing users while enabling the ability to explicitly set the token via a function or value.

[arifmahmudrana]

@VictorAvelar I guess it won't the problem with this implementation is we aren't using the dependency injection e.g. in our case all our payment configs are in DB we read the config and use a factory function to create the client. For this here is the code we wrote

func getMollieClient(oppr entity.OnlinePaymentProvider) (pay.Client, error) {
    var provider mollie.MollieOpts
    err := json.Unmarshal([]byte(oppr.Config), &provider)
    if err != nil {
        return nil, err
    }

    // obtain preexist env value if exists
    tkn, ok := os.LookupEnv(provider.APITokenEnv)
    // set the APITokenEnv with the Token in the env for mollie client creation
    if err := os.Setenv(provider.APITokenEnv, provider.Token); err != nil {
        return nil, err
    }

    // check whether the env was set properly
    _, ok1 := os.LookupEnv(provider.APITokenEnv)
    if !ok1 {
        return nil, fmt.Errorf(
            "mollie client init: couldn't find env while os.LookupEnv for '%s'", provider.APITokenEnv,
        )
    }

    // create mollie client
    client, err := mollie.NewClient(provider)
    if err != nil {
        return nil, err
    }

    // restore the previous APITokenEnv if was exists or unset
    if ok {
        if err := os.Setenv(provider.APITokenEnv, tkn); err != nil {
            return nil, err
        }
    } else {
        if err := os.Unsetenv(provider.APITokenEnv); err != nil {
            return nil, err
        }
    }

    return client, nil
}

Here in this library, we are dependent on os package for client creation which we shouldn't it's the consumer's choice consumer may use os env, may use DB or some other third party e.g. terraform vault or AWS Secrets Manager. It would be great if we could change the implementation.

Let me know if you need any support from me on this.

[arifmahmudrana]

We can also create a separate NewClient function that will remove the usage of os.LookupEnv in that way we can also prevent breaking changes.

[VictorAvelar]

Thanks for your explanation. However, I believe there are a few points that need clarification.

• This library is designed to be minimally dependent on external libraries, relying primarily on the standard library to avoid dependency conflicts for end-users. We intentionally avoid imposing specific library or pattern requirements.
• While I respect the decisions made for your codebase, it's important to understand that third-party libraries cannot be expected to accommodate specific use cases through code changes.
• I'm unclear about how the current implementation prevents/affects the use of secret management engines or container orchestrators. Please provide more context so I can better understand and address any potential issues.
• The package offers flexibility in client setup, promoting secure practices like using environment variables while providing helper methods for alternative approaches. This aligns with common patterns found in other well-known and largely used go libraries.

I appreciate your suggestions, but I'm uncertain if they align with the library's development philosophy.

[arifmahmudrana]

• I'm unclear about how the current implementation prevents/affects the use of secret management engines or container orchestrators. Please provide more context so I can better understand and address any potential issues.

I understand there are some limitations and philosophies. However, one key point in this library is violating the dependency injection principle.

In our case, we have all our credentials in the database but we can't create a client without setting the environment variable.

[VictorAvelar]

It is perfectly possible to create an API client without having to set the environment variables first.

package main

import (
    "context"
    "fmt"

    "github.com/VictorAvelar/mollie-api-go/v4/mollie"
)

func main() {
    cfg := mollie.NewAPITestingConfig(false)

    client, err := mollie.NewClient(nil, cfg)
    if err != nil {
        panic(err)
    }

    // check the client has no auth value.
    fmt.Println("Client without auth value\n")
    fmt.Printf("%+v\n", client)

    client.WithAuthenticationValue("your api key from any source")

    // check the client has auth value.
    fmt.Println("Client with auth value\n")
    fmt.Printf("%+v\n", client)

    // use the client to make requests
    _, methods, err := client.PaymentMethods.List(context.Background(), nil)
    if err != nil {
        panic(err)
    }

    fmt.Print("\n\n\n\n\n\n")

    for _, m := range methods.Embedded.Methods {
        println(m.ID)
    }
}

[arifmahmudrana]

@VictorAvelar Thanks you are right. I may somehow missed this function I am really sorry for the trouble. Thanks once again.

[VictorAvelar]

It is ok, happy that I was able to help. 💪