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

Support for secret variables #4846

Open
nickfla1 opened this issue Jun 11, 2024 · 10 comments
Open

Support for secret variables #4846

nickfla1 opened this issue Jun 11, 2024 · 10 comments
Labels
area:monitor Everything related to monitors feature-request Request for new features to be added question Further information is requested type:enhance-existing feature wants to enhance existing monitor

Comments

@nickfla1
Copy link

πŸ“‘ I have found these related issues/pull requests

I was unable to find any related issue.

🏷️ Feature Request Type

New monitor, Change to existing monitor

πŸ”– Feature description

It should be possible to manage secret keys/values and use them inside URLs, headers and Authentication methods without showing them in the dashboard as plain text.

βœ”οΈ Solution

We should be able to manage secrets via a dedicated settings section. I don't believe it would be necessary to allow for a full CRUD interface, Create and Delete should be enough.

Each key should have an unique name which can be referenced inside URLs and Headers:

https://myapi.example.com/foobar?apikey={{ my_key_name }}
{
  "Authentication": "Bearer {{ my_key_name }}"
}

or

https://myapi.example.com/foobar?apikey={{ secrets.my_key_name }}
{
  "Authentication": "Bearer {{ secrets.my_key_name }}"
}

❓ Alternatives

No response

πŸ“ Additional Context

Showing plain text keys or secrets is always a security concern, sometimes even in the same organization you don't want to be so open about who can easily read those values.

@nickfla1 nickfla1 added the feature-request Request for new features to be added label Jun 11, 2024
@nickfla1 nickfla1 changed the title Allow for "secret" variables Support for secret variables Jun 11, 2024
@nickfla1
Copy link
Author

If you second this proposal I am more than open to directly contribute to its implementation!

@CommanderStorm
Copy link
Collaborator

CommanderStorm commented Jun 11, 2024

Authentication: Bearer

We already handle this for the auth methods we currently support

image

Would accept a PR implementing Bearer Auth as an alternative to Basic Auth.

@nickfla1
Copy link
Author

Authentication: Bearer

We already handle this for the auth methods we currently support
Would accept a PR implementing Bearer Auth as an alternative to Basic Auth.

I can work on that in the next few days

@CommanderStorm
Copy link
Collaborator

CommanderStorm commented Jun 11, 2024

About the url templating, I am not entirely sold. Don't think this solves your issue.
I mean, if you want to protect the secrets from other people you share the password with.. They could simply change the url to be https://webhook.site and extract the secret via that.

@CommanderStorm CommanderStorm added area:monitor Everything related to monitors type:enhance-existing feature wants to enhance existing monitor labels Jun 11, 2024
@nickfla1
Copy link
Author

nickfla1 commented Jun 12, 2024

About the url templating, I am not entirely sold. Don't think this solves your issue. I mean, if you want to protect the secrets from other people you share the password with.. They could simply change the url to be https://webhook.site and extract the secret via that.

I'm not sure I'm following, wouldn't the template be compiled when the Kuma is invoking my configured monitor? How could anyone intercept that?

I admin that I'm yet to figure out how I'd implement it within Kuma

Would accept a PR implementing Bearer Auth as an alternative to Basic Auth.

Also, reconsidering, this wouldn't work for my use-case as we use custom headers as API keys

@CommanderStorm
Copy link
Collaborator

Let's assume you first have a monitor pointing to https://example.com?key={{secret.key}} then the attacker with your admin credentials changes it to webhook.site/...?key={{secret.key}}.

Now the attacker has your secret, the same was as if he had read it in the UI.

@nickfla1
Copy link
Author

Let's assume you first have a monitor pointing to https://example.com?key={{secret.key}} then the attacker with your admin credentials changes it to webhook.site/...?key={{secret.key}}.

Now the attacker has your secret, the same was as if he had read it in the UI.

Ah now I get it! Yes of course that could be an attack vector, thought if they have admin access that's not a lot we can do anyway.

To be more clear on our use case, we'd like read only users to not see some api keys in headers and query strings that now have to be set as plain text, it wouldn't solve any security issue in case of compromised admin access.

I think it might be useful to other Kuma's users as well

@CommanderStorm CommanderStorm added the question Further information is requested label Jun 12, 2024
@CommanderStorm
Copy link
Collaborator

Readonly users is tracked in #1322 and at the moment quite gridlocked.

If you want to help said feature along, you can review #3571 or test said PR via this https://github.com/louislam/uptime-kuma/wiki/Test-Pull-Requests method

@nickfla1
Copy link
Author

Even though they're in progress, do you think read-only users will be able to see URLs, request bodies and headers clearly?
If so I think my question still applies!

I'll probably work on the "Bearer" authorization PR anyway as I think it will improve the project, even though we don't use that authentication mechanism

@CommanderStorm
Copy link
Collaborator

How fleshed out the permission system will be has not been defined.
Fleshing it out does not really make sense spending maintainer time on unless we have #3571 reviewed and tested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:monitor Everything related to monitors feature-request Request for new features to be added question Further information is requested type:enhance-existing feature wants to enhance existing monitor
Projects
None yet
Development

No branches or pull requests

2 participants