vikunja/vikunja
5
48
Fork 86
Code Issues 150 Pull Requests 36 Packages Releases 37 Activity

Allow setting openid provider clientSecret with an external file #881

New Issue
Closed
opened 2021-06-08 21:20:47 +00:00 by leona · 15 comments
leona commented 2021-06-08 21:20:47 +00:00
Copy Link

I use NixOS for all my systems. It's a declarative OS, where the config file is readable for all users and processes on the host. Therefore secrets are saved normally in environment variables or extra files.

I tried to setup an OpenID Provider with parts of the config (everything except clientsecret) in the config file and the clientsecret as environment variable. For me that did not work. There are two options: Either I did something wrong or it is not supported to set lists in environment variables (I think it's the second option).

In my opinion, the easiest way to solve this problem is to introduce a new configuration option auth.openid.clientsecret_file, where a path to a file containing only the client secret can be specified. This option would be mutually exclusive with auth.openid.clientsecret.

Maybe I will try in some days to implement this myself, but I'm not really good at Go.

I use [NixOS](https://nixos.org) for all my systems. It's a declarative OS, where the config file is readable for all users and processes on the host. Therefore secrets are saved normally in environment variables or extra files. I tried to setup an OpenID Provider with parts of the config (everything except `clientsecret`) in the config file and the `clientsecret` as environment variable. For me that did not work. There are two options: Either I did something wrong or it is not supported to set lists in environment variables (I think it's the second option). In my opinion, the easiest way to solve this problem is to introduce a new configuration option `auth.openid.clientsecret_file`, where a path to a file containing only the client secret can be specified. This option would be mutually exclusive with `auth.openid.clientsecret`. Maybe I will try in some days to implement this myself, but I'm not really good at Go.
👍 1
konrad commented 2021-06-09 13:04:40 +00:00
Owner
Copy Link

or it is not supported to set lists in environment variables

You are correct, setting lists in env variables is currently not supported.

There's also vikunja/api#704 which goes in a bit more detail about setting config variables from files.

I think that providing the config variables from files should work for all config variables (or at least the secrets) not only for openid secrets. Then your problem becomes a two part problem: 1) Getting list config from env and 2) getting config secrets from files.
Until this is sorted out, you should be able to just put the secrets in an external .nix file and import that (at least that's what I would do 🙃 ).

With nix you might be interested in this module: https://kolaente.dev/vikunja/desktop-nix (I'm also using nix as my main OS, just not (yet) for server stuff)

> or it is not supported to set lists in environment variables You are correct, setting lists in env variables is currently not supported. There's also https://kolaente.dev/vikunja/api/issues/704 which goes in a bit more detail about setting config variables from files. I think that providing the config variables from files should work for all config variables (or at least the secrets) not only for openid secrets. Then your problem becomes a two part problem: 1) Getting list config from env and 2) getting config secrets from files. Until this is sorted out, you should be able to just put the secrets in an external `.nix` file and import that (at least that's what I would do 🙃 ). With nix you might be interested in this module: https://kolaente.dev/vikunja/desktop-nix (I'm also using nix as my main OS, just not (yet) for server stuff)
konrad added the
kind/feature
 label 2021-06-09 13:04:46 +00:00
leona commented 2021-06-09 13:51:10 +00:00
Author
Copy Link

Until this is sorted out, you should be able to just put the secrets in an external .nix file and import that (at least that's what I would do 🙃 ).

I don't think that would be a good option beacause also in that case the secret(s) would be public in /nix/store/... for every program and user.

I think that providing the config variables from files should work for all config variables (or at least the secrets) not only for openid secrets. Then your problem becomes a two part problem: 1) Getting list config from env and 2) getting config secrets from files.

That would be even better. Then I think getting list config from env isn't really a problem, because I could set the path to the secret file paths in the config file.

There's also #704 which goes in a bit more detail about setting config variables from files.

Ah, I didn't found that issue. But in the case of openid client secrets that wouldn't work I think, because of the list stuff, so it would be better to allow setting the secret file paths (also) in the config file.

With nix you might be interested in this module: https://kolaente.dev/vikunja/desktop-nix (I'm also using nix as my main OS, just not (yet) for server stuff)

Thank you, currently I packaged the API and Frontend, but it's not published yet. I will have a look at your work, maybe you've done it in a better way than me ;)

> Until this is sorted out, you should be able to just put the secrets in an external .nix file and import that (at least that's what I would do 🙃 ). I don't think that would be a good option beacause also in that case the secret(s) would be public in `/nix/store/...` for every program and user. > I think that providing the config variables from files should work for all config variables (or at least the secrets) not only for openid secrets. Then your problem becomes a two part problem: 1) Getting list config from env and 2) getting config secrets from files. That would be even better. Then I think getting list config from env isn't really a problem, because I could set the path to the secret file paths in the config file. > There's also #704 which goes in a bit more detail about setting config variables from files. Ah, I didn't found that issue. But in the case of openid client secrets that wouldn't work I think, because of the list stuff, so it would be better to allow setting the secret file paths (also) in the config file. > With nix you might be interested in this module: https://kolaente.dev/vikunja/desktop-nix (I'm also using nix as my main OS, just not (yet) for server stuff) Thank you, currently I packaged the API and Frontend, but it's not published yet. I will have a look at your work, maybe you've done it in a better way than me ;)
konrad commented 2021-06-09 14:10:05 +00:00
Owner
Copy Link

I don't think that would be a good option beacause also in that case the secret(s) would be public in /nix/store/... for every program and user.

But the env value has to be stored somewhere hasn't it? I might be wrong though.

Would using a plain old yaml/toml config file be an option?

I will have a look at your work, maybe you've done it in a better way than me ;)

Maybe but THB I'm not so sure about that one 🙂 . It's one of my first nix module and based on @jtojnar module.

> I don't think that would be a good option beacause also in that case the secret(s) would be public in /nix/store/... for every program and user. But the env value has to be stored somewhere hasn't it? I might be wrong though. Would using a plain old yaml/toml config file be an option? > I will have a look at your work, maybe you've done it in a better way than me ;) Maybe but THB I'm not so sure about that one 🙂 . It's one of my first nix module and based on @jtojnar module.
leona commented 2021-06-09 14:20:34 +00:00
Author
Copy Link

But the env value has to be stored somewhere hasn't it? I might be wrong though.

Would using a plain old yaml/toml config file be an option?

Yes and no. On my servers the secrets are stored in a gpg encrypted file which is in the nix store. These files are unencrypted on each boot (and every other system activation) into a tmpfs. The decrypted files are only readble for a specific user (e.g. root or vikunja).

Another option than having _file options for secrets, is to allow using multiple yaml/toml/json config files, in which one in my case would be encrypted

> But the env value has to be stored somewhere hasn't it? I might be wrong though. > Would using a plain old yaml/toml config file be an option? Yes and no. On my servers the secrets are stored in a gpg encrypted file which is in the nix store. These files are unencrypted on each boot (and every other system activation) into a tmpfs. The decrypted files are only readble for a specific user (e.g. `root` or `vikunja`). Another option than having `_file` options for secrets, is to allow using multiple yaml/toml/json config files, in which one in my case would be encrypted
konrad commented 2021-06-09 14:48:14 +00:00
Owner
Copy Link

I see, that makes sense. Currently Vikunja will look in various places for a config file and use the first one it finds. Using multiple different config files would get a bit difficult for matching/merging I think (things like which config file should take precence if multiple ones specify different values for the same key).

Would it be an option to use one encrypted config file with all config variables?

I see, that makes sense. Currently Vikunja will look [in various places](https://vikunja.io/docs/config-options/#config-file-locations) for a config file and use the first one it finds. Using multiple different config files would get a bit difficult for matching/merging I think (things like which config file should take precence if multiple ones specify different values for the same key). Would it be an option to use one encrypted config file with all config variables?
leona commented 2021-06-09 15:03:35 +00:00
Author
Copy Link

It would be an option, but not the best I think. With that I couldn't use the full potential of NixOS modules.

For me the best would be to allow reading secrets out of files (as I described already).

It would be an option, but not the best I think. With that I couldn't use the full potential of NixOS modules. For me the best would be to allow reading secrets out of files (as I described already).
👍 1
jtojnar commented 2021-06-09 15:08:20 +00:00
Contributor
Copy Link

Personally, I currently do not bother with hiding secrets as I do not use any security critical ones. Where possible (e.g. vikunja-api), I leave the software to generate keys at start-up and store them in in-memory, since it is less pain for me to re-log in when the service restarts than wrangling the keys. If a software cannot do without a key, I store it in git-crypt and let it end up in the Nix store. But yeah, it is suboptimal.

I am following https://github.com/NixOS/nixpkgs/issues/102397 looking forward to using systemd for secrets once NixOS updates to 248.

Apparently, systemd authors do recommed using files instead of env vars: https://github.com/NixOS/nixpkgs/issues/102397#issuecomment-853472016

Also, here is my attempt at vikunja-api and the NixOS configuration in case you are interested.

Personally, I currently do not bother with hiding secrets as I do not use any security critical ones. Where possible (e.g. vikunja-api), I leave the software to generate keys at start-up and store them in in-memory, since it is less pain for me to re-log in when the service restarts than wrangling the keys. If a software cannot do without a key, I store it in git-crypt and let it end up in the Nix store. But yeah, it is suboptimal. I am following https://github.com/NixOS/nixpkgs/issues/102397 looking forward to using systemd for secrets once NixOS [updates to 248](https://github.com/NixOS/nixpkgs/pull/123476). Apparently, systemd authors do recommed using files instead of env vars: https://github.com/NixOS/nixpkgs/issues/102397#issuecomment-853472016 ----- Also, here is my attempt at [vikunja-api](https://github.com/jtojnar/nixfiles/blob/1817be7899538746d68fc28463f4f60ca1653d7c/pkgs/vikunja/vikunja-api/default.nix) and the [NixOS configuration](https://github.com/jtojnar/nixfiles/blob/1817be7899538746d68fc28463f4f60ca1653d7c/hosts/azazel/ogion.cz/todo/default.nix) in case you are interested.
konrad commented 2021-06-09 15:17:03 +00:00
Owner
Copy Link

Where possible (e.g. vikunja-api), I leave the software to generate keys at start-up and store them in in-memory

For Vikunja's jwt this works great, but with secrets from external sources (like openid) that won't work. I understand why just using the in-memory secret is useful though.

I guess the "good" way would be to have Vikunja read config variables from files and until then just store the whole config file somewhere encrypted?

> Where possible (e.g. vikunja-api), I leave the software to generate keys at start-up and store them in in-memory For Vikunja's jwt this works great, but with secrets from external sources (like openid) that won't work. I understand why just using the in-memory secret is useful though. I guess the "good" way would be to have Vikunja read config variables from files and until then just store the whole config file somewhere encrypted?
jtojnar commented 2021-06-09 16:09:54 +00:00
Contributor
Copy Link

I think it is useful to be able to separate the secret and public parts of the config as to minimize what is opaque to the version control.

Just having read the systemd manual, I think vikunja also looking in $CREDENTIALS_DIRECTORY/$config.key file might be useful. But having anonymous list items for the openid makes it harder.

Alternative would be allowing a dictionary as a password key:

auth:
  openid:
    providers:
      - name: foo
        clientsecret:
            type: systemd
            name: openid_foo_client_secret

which would make vikunja read the contents of $CREDENTIALS_DIRECTORY/openid_foo_client_secret.

I think it is useful to be able to separate the secret and public parts of the config as to minimize what is opaque to the version control. Just having read the [systemd manual](https://www.freedesktop.org/software/systemd/man/systemd.exec.html#LoadCredential=ID:PATH), I think vikunja also looking in `$CREDENTIALS_DIRECTORY/$config.key` file might be useful. But having anonymous list items for the openid makes it harder. Alternative would be allowing a dictionary as a password key: ```yaml auth: openid: providers: - name: foo clientsecret: type: systemd name: openid_foo_client_secret ``` which would make vikunja read the contents of `$CREDENTIALS_DIRECTORY/openid_foo_client_secret`.
konrad commented 2021-06-10 07:04:43 +00:00
Owner
Copy Link

I think that would be a very nice solution.

I think that would be a very nice solution.
pinpox commented 2023-09-08 07:49:38 +00:00
Copy Link

Was this ever implemented or a solution/workaround found? I'm trying to deploy on NixOS and running into this exact problem with openID secrets

Viper, the library used to parse the configuration also mentions string replacers here. This could probably be used to allow for referencing environment variables in the yaml config, e.g. have a config like this:

            openid:
              enabled: 'true'
              providers:
              - authurl: https://...
                clientid: $CLIENT_ID
                clientsecret: $CLIENT_SECRET

Which would replace/insert $CLIENT_ID and $CLIENT_SECRET with provided env files of that name

Was this ever implemented or a solution/workaround found? I'm trying to deploy on NixOS and running into this exact problem with openID secrets Viper, the library used to parse the configuration also mentions string replacers [here](https://github.com/spf13/viper#working-with-environment-variables). This could probably be used to allow for referencing environment variables in the yaml config, e.g. have a config like this: ```yaml openid: enabled: 'true' providers: - authurl: https://... clientid: $CLIENT_ID clientsecret: $CLIENT_SECRET ``` Which would replace/insert `$CLIENT_ID` and `$CLIENT_SECRET` with provided env files of that name
leona commented 2023-09-08 14:37:10 +00:00
Author
Copy Link

Haven't tried that in a while now. Of course we could also implement sth with environment variables replacing on the NixOS side.

Haven't tried that in a while now. Of course we could also implement sth with environment variables replacing on the NixOS side.
donor_extortion commented 2023-10-15 02:56:39 +00:00
Copy Link

Disclaimer: I've never used Go let alone viper so I may be way off on some things.

Regarding being able to use env vars in the config file, it looks like decoder hooks would be the way to do that (https://github.com/spf13/viper/issues/418). It seems as though those are only used when using Unmarshal.

Would exposing an Unmarshal function and then using that to parse providers be a viable solution? Could also modify GetString and GetStringSlice to unmarshal so all strings in the config file could use env vars. Something like

config.go

func (k Key) Unmarshal(rawVal any) error {
	return viper.UnmarshalKey(string(k), rawVal, stringExpandEnvDecoderHook)
}

providers.go

func GetAllProviders() (providers []*Provider, err error) {
	// ...
	if !exists {
        // replace manual parsing with unmarshal
		err = config.AuthOpenIDProviders.Unmarshal(&providers)
		// ...
	}

	return
}
Disclaimer: I've never used Go let alone viper so I may be way off on some things. Regarding being able to use env vars in the config file, it looks like decoder hooks would be the way to do that (https://github.com/spf13/viper/issues/418). It seems as though those are only used when using `Unmarshal`. Would exposing an `Unmarshal` function and then using that to parse providers be a viable solution? Could also modify `GetString` and `GetStringSlice` to unmarshal so all strings in the config file could use env vars. Something like **config.go** ```go func (k Key) Unmarshal(rawVal any) error { return viper.UnmarshalKey(string(k), rawVal, stringExpandEnvDecoderHook) } ``` **providers.go** ```go func GetAllProviders() (providers []*Provider, err error) { // ... if !exists { // replace manual parsing with unmarshal err = config.AuthOpenIDProviders.Unmarshal(&providers) // ... } return } ```
poperigby commented 2024-06-20 16:07:32 +00:00
Copy Link

Any updates on this? I think the environment variable method wouldn't be a bad idea. It's not really an option for me to put my secrets directly in the config file, as I'm also on NixOS.

Any updates on this? I think the environment variable method wouldn't be a bad idea. It's not really an option for me to put my secrets directly in the config file, as I'm also on NixOS.
konrad commented 2024-11-18 09:47:13 +00:00
Owner
Copy Link

This is now implemented in c7914bc245 and 05349ddb5c.

Docs are here: https://vikunja.io/docs/config-options/#reading-config-values-from-files

There isn't a release with this yet, but the CI is currently building unstable binaries which you can use to test this.

This is now implemented in c7914bc2452b8d7e3cc343d4d0413926fb86d3de and 05349ddb5c9d62a59ab1b997984d0b97f24df247. Docs are here: https://vikunja.io/docs/config-options/#reading-config-values-from-files There isn't a release with this yet, but the CI is currently building unstable binaries which you can use to test this.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
dependencies
duplicate
This issue or pull request already exists
help wanted
Need some help
invalid
Something is wrong
kind/bug
Something is not working
kind/feature
New feature
needs reproduction
question
More information is needed
security
wontfix
This won't be fixed
No Label
kind/feature
Milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
6 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: vikunja/vikunja#881
No description provided.
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?