feat: allow usage of a environment variable or secret for sensitive params

[thpiron]

Hello,

Created this PR to solve issue #13.
Added possibility to set bindPassword from environment variable or docker secret (filesystem /run/secrets/my_secret).

No breaking change.

[wiltonsr]

Hi, @thpiron

Thanks for your PR.

I think this feature must support almost every available parameter. Unfortunately, for this, a lot of work is needed.

I started a draft of what I think this feature should be like in this branch: feat-support-multiple-conf-sources.

Feel free to propose any changes you think are pertinent.

[thpiron]

Hello, I don't really think that putting everything in secrets is a good idea. It's only useful for sensitive data (ie passwords, key etc).
And this would break the possibility to have multiple ldapAuth middlewares with different configuration as the secret must be on the traefik container, so can't be duplicated.

That why I added the possibility to change the environment variable / secret names for the bindPassword.

[wiltonsr]

It's only useful for sensitive data (ie passwords, key etc).

At some contexts the server, base and bindDN can also be sensitive data.

And this would break the possibility to have multiple ldapAuth middlewares with different configuration as the secret must be on the traefik container, so can't be duplicated.

But I agree that we must preserve the multiple usage of ldapAuth in different services.

Despite of this, create a custom parameter to each of these values doesn’t make sense.

@fcinqmars what do you think about this?

[fcinqmars]

I agree with @thpiron that normally secrets should only be used for sensitive values like credentials though I have seen them used more broadly.

I don't think that server, base and bindDN necessarily qualify for secrets. From what I have seen, most LDAP servers are configured to allow any LDAP users to read the directory anyway. It usually is enabled by default. If the security controls are proper, knowing that information shouldn't matter in my opinion.

Personally, I would only put what needs to be secret in the secrets.

[wiltonsr]

Personally, I would only put what needs to be secret in the secrets.

So are we talking about bindPassword and cacheKey?

That why I added the possibility to change the environment variable / secret names for the bindPassword.

Should this possibility exist for both these parameters or only for bindPassword?

[thpiron]

Yes my bad, it should be possible for both secrets, I'll update the branch to add this.

[thpiron]

Added possibility to change label of cache key secret.
Rebased on main and fixed some conflicts, ready to be reviewed for merging.

[fcinqmars]

Hi @thpiron,

Thank you for you PR. I completed the review, let me know if something does not feel right or is unclear.

@wiltonsr Feel free to chime in

[fcinqmars]

I am wondering if maybe we should allow a file path there, just because that would not be usable on Windows. Furthermore, Docker Swarm and Docker Compose allow mounting secrets into other locations than /run/secrets

[wiltonsr]

Support for multiple paths could be a problem. We could detect the OS system and define the full path based on default values or direct read the file if the label starts with / and exists. But I prefer to maintain the plugin as simple as possible.

[thpiron]

Thank you both for the review, it should be way better now.

[bclingan]

are there plans to implement this PR in the future?