desec-io / desec-stack

Backbone of the deSEC Free Secure DNS Hosting Service
https://desec.io/
MIT License
365 stars 48 forks source link

GUI: allow declaring API tokens read-only during creation #528

Open Valodim opened 3 years ago

Valodim commented 3 years ago

It would be useful to have a capability for read-only API tokens.

The use case I have is that we'd like to check if all values are up to date via terraform plan -detailed-exitcode || fail as part of a CI job. At the moment that could only be done by giving that CI job a token with read/write access.

This is a "least authority" concern similar to #347.

nils-wisiol commented 3 years ago

Read only access is also currently provided via the DNS, no token needed there.

Valodim commented 3 years ago

It's true - technically! :)

However, for comparing current state vs target state from infrastructure as code, as per the described terraform use case, it is much easier (and more correct) to do that via the R part of the CRUD API than mapping the state back from DNS.

nils-wisiol commented 3 years ago

Agreed. Just wanted to point out a solution that works without changes on our side. I'm nevertheless in favor of introducing read-only tokens

hilbix commented 4 months ago

FWIW my 2 cents:

I currently operate my own DNS scripts which create the zone files. These scripts should diff the zone files to what is in "production" (read: on desec.io).

These scripts shall run fully automated in background and send their results into monitoring. These scripts also shall monitor the zone files for correctness. For security purpose, I do not want these scripts to have full write access, as updates to the zones still must involve a manual approval step (read: 2FA).

Checking for known records works well via DNS. But this does not work for deleted nor unknown records, as those are not present in the generated zone files. AFAICS AXFR does not work from desec.io, which is the expected behavior, as there should not be a way to anonymously iterate through an entire zone (to lower the attack surface and make SPAM more difficult).

Wrong updates of records can happen, as desec.io has no staging area (like git or gerrit) where the scripts can upload changes which then only need approval (like git commit), so I will continue to manually update the zones (it does not happen often, so not much overhead for me).

But I am human and therefor I err. So these scripts shall be able to detect all the wrong records I added, which IMHO only works with an API request, which then should be readonly.

hilbix commented 4 months ago

Sorry for me posting again, but ..

.. looking into the docs a bit deeper, I now think, this here only affects the web frontend (so the UI). As with the API something like a readonly token should be possible already, see #347

Citing https://desec.readthedocs.io/en/latest/auth/tokens.html#token-scoping-policies

Tokens can be restricted using Token Policies

All tokens can, regardless of their policy configuration, read any RRset (for all domains in the account)

Looks like this fulfills a readonly access (at least the one I need).

Tokens with at least one policy are considered restricted, with their scope limited to DNS record management. They can neither Retrieve Account Information nor perform Domain Management (such as domain creation or deletion).

This is exactly what I want!

I haven't tested it yet, but something like following should do it (the id was taken from the docs and probably will look differently for a token):

{
    "id": "7aed3f71-bc81-4f7e-90ae-8f0df0d1c211",
    "domain": null,
    "subname": null,
    "type": null,
    "perm_write": false
}

Sorry, but until this post I only had a look into the web frontend and I do not see any way to edit the topen policy there .. yet. And with my first fast glimpse into the API docs I did not catch this Token Policy thingy.

Implementing editing policies via Web might be a major task, hence in the meanwhile, perhaps a little checkbox "readonly" could be added which then adds a readonly default policy. And no, I do not think we need a way to change the readonly status until full policy editing is available in the UI.

This checkbox can come handy in future, too, even with the UI able to edit the policy. Because then the token first is added readonly, such that you can relax until the token gets the correct permissions, because it does not eventually open a loophole ..

And then, readonly perhaps should become the default. Better safe than sorry ;)

peterthomassen commented 4 months ago

.. looking into the docs a bit deeper, I now think, this here only affects the web frontend (so the UI). As with the API something like a readonly token should be possible already, see #347

You are right! :-) I forgot to update this issue after we deployed that feature.

So, I'll relabel this as GUI only. Note that we can't give a timeline for this, and would very much appreciate a PR. It'll probably be reasonable to first address #900, to avoid adding more migration work to that issue.

If anyone thinks that API functionality is missing in relation to this, please comment.