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

oidc: allow reading the client secret from a file #1127

Merged
merged 1 commit into from
Jan 14, 2023

Conversation

motiejus
Copy link
Contributor

@motiejus motiejus commented Jan 10, 2023

Currently the most "secret" way to specify the oidc client secret is via an environment variable OIDC_CLIENT_SECRET, which is problematic1. Lets allow reading oidc client secret from a file. For extra convenience the path to the secret will resolve the environment variables.

  • read the CONTRIBUTING guidelines
  • raised a GitHub issue or discussed it on the projects chat beforehand
  • added unit tests
  • added integration tests
  • updated documentation if needed
  • updated CHANGELOG.md

@motiejus motiejus force-pushed the oidc-client-secret-file branch from 3e7efed to e08b711 Compare January 10, 2023 12:22
config.go Outdated Show resolved Hide resolved
docs/oidc.md Show resolved Hide resolved
docs/oidc.md Show resolved Hide resolved
@motiejus motiejus force-pushed the oidc-client-secret-file branch from e08b711 to f9f9268 Compare January 11, 2023 11:08
@motiejus
Copy link
Contributor Author

motiejus commented Jan 11, 2023

Updated the PR as per @kradalby 's comments.

I was able to reproduce the integration test failure locally and will investigate it over the next couple of days.

@motiejus motiejus force-pushed the oidc-client-secret-file branch from f9f9268 to 3318f8b Compare January 12, 2023 15:28
@motiejus
Copy link
Contributor Author

I've updated the test and it passed locally.

@motiejus motiejus requested review from kradalby and removed request for juanfont January 12, 2023 18:07
@motiejus motiejus force-pushed the oidc-client-secret-file branch from 3318f8b to d6b985d Compare January 13, 2023 16:47
@motiejus
Copy link
Contributor Author

Apologies for missing to check the linter. Fixed a linter error.

Currently the most "secret" way to specify the oidc client secret is via
an environment variable `OIDC_CLIENT_SECRET`, which is problematic[1].
Lets allow reading oidc client secret from a file. For extra convenience
the path to the secret will resolve the environment variables.

[1]: https://systemd.io/CREDENTIALS/
@juanfont juanfont force-pushed the oidc-client-secret-file branch from d6b985d to d57b3b0 Compare January 14, 2023 12:43
@kradalby kradalby merged commit bafb679 into juanfont:main Jan 14, 2023
@motiejus motiejus deleted the oidc-client-secret-file branch February 21, 2023 11:27
@motiejus
Copy link
Contributor Author

FYI the subsequent change to nixpkgs: NixOS/nixpkgs#217482

motiejus added a commit to motiejus/nixpkgs that referenced this pull request Mar 8, 2023
Headscale now supports passing the OIDC client secret via a file, as
added in [juanfont/headscale#1127][1127]. Lets use that.

The headscale option is `client_secret_path`; let's make it consistent
and rename the Nix option to this. Note that I wasn't able to do this:

    mkRenamedOptionModule [ ... "client_secret_file" ] [ ... "client_secret_path" ]

I get such error:

    error: evaluation aborted with the following error message: 'cannot find attribute `services.headscale.settings.oidc.client_secret_file''

[1127]: juanfont/headscale#1127
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants