piston3 token delete cascades

Bug #1870171 reported by Freddy on 2020-04-01
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
MAAS
Medium
Lee Trager
2.3
Medium
Lee Trager
2.4
Medium
Lee Trager
2.6
Medium
Lee Trager
2.7
Medium
Lee Trager

Bug Description

https://git.launchpad.net/maas/tree/src/maasserver/models/node.py#n1101

The current configured behavior is to CASCADE deletes on the token_id column for maasserver_node. This results in nodes potentially being deleted on the event of a token being deleted.

There are several thousand maas nodes in the installation that we manage. We have started to enforce key rotation and it has resulted in nodes being removed from our maas installation, which negatively impacts our users.

Can the token foreign key on_delete attribute be updated to either
- SET_NULL, to keep the node row but set the token_id attribute to null. or,
- PROTECT, to raise an error if there are any node objects related to the token_id

We do not use the juju functionality that requires linking a token to a node, so I can't say which would be better. However, I can't believe that silently deleting nodes out of maas is the best solution.

More info on ForeignKey setup: https://docs.djangoproject.com/en/2.1/ref/models/fields/#django.db.models.ForeignKey.on_delete

Related branches

Alberto Donato (ack) on 2020-04-01
Changed in maas:
milestone: none → 2.8.0b1
status: New → Triaged
importance: Undecided → High
importance: High → Medium
Lee Trager (ltrager) wrote :

MAAS doesn't expect others to be modifying the database. The OAUTH keys stored are passed to the deployed machine and used to communicate back to MAAS. By deleting them the machine won't be able to authenticate and communicate back with MAAS. Removing the current keys could break any ephemeral environment or deployment.

MAAS does remove keys when they are no longer needed however the key needs to remain the same as long as the machine is deployed. This is because cloud-init accesses the metadata service on every boot. We *may* be able to change that but we'd have to do some investigation.

Freddy (fwieffering) wrote :

To be clear - I am not modifying the database. In the mass ui, when you remove an API key from a user any nodes associated with that key are removed from maas, because the mass api is configured to cascade the deleted token.

I understand that the token has a purpose. I just don't want to inadvertently result in a number of nodes being silently removed from maas. There could be either a warning that there are linked nodes or the action could be forbidden if there are linked nodes.

Björn Tillenius (bjornt) wrote :

Yes, this is something that we need to fix.

I think Lee and Freddy are talking about different tokens. The token that Freddy is talking about is Node.token, which get set whenever a machine is acquired using the API.

It seems that the only use for that key is to for the list-allocated API call, which lists all the machines that were deployed using the current API key.

Björn Tillenius (bjornt) wrote :

I confirmed this bug. I deployed a machine using the API, and then I went to the UI and deleted the user's API key that was used to deploy the machine. The result was the the deployed machine was deleted as well.

Changed in maas:
assignee: nobody → Lee Trager (ltrager)
Lee Trager (ltrager) wrote :

My mistake, it seems Nodes have two tokens associated with them. One is used as I described above. The second seems to be a left over from something else. When a machine is allocated, only on the API, it sets an association between the user's token and the Node. Only the API uses this to figure out which nodes have been allocated. The nodes are already filtered by owner and state so it isn't needed. I'll create a branch to remove it.

Changed in maas:
status: Triaged → Fix Committed
Björn Tillenius (bjornt) wrote :

I reopened all the backport tasks, since they still need some work. It would be good to try to hook into a lower layer.

One thing that I tested was to deploy a machine with user A using the API in 2.7.0. Then I upgraded to 2.7.1 and deleted user A using the API, passing in transfer_resources_to=B.

The result was that user A and the machine that was deployed were both deleted.

Lee Trager (ltrager) wrote :

Thanks for catching that. It looks like the transfer_resources_to code never changed the OAUTH token from the previous user. I confirmed this code can only be used when deleting a user so there is no risk of the previous user maintaining access to a transferred node.

Alberto Donato (ack) on 2020-04-17
Changed in maas:
status: Fix Committed → Fix Released
Lee Trager (ltrager) wrote :

The fix has landed in MAAS <= 2.7.

Freddy (fwieffering) wrote :

will you be releasing a patched version of 2.6 + 2.7 with this change?

Lee Trager (ltrager) wrote :

Yes, I have back ported this to 2.7, 2.6, 2.4, and 2.3.

Freddy (fwieffering) wrote :

I saw that but haven't seen anything pushed to the PPAs for 2.6 / 2.7 and wanted to confirm that will come.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers