tractr / directus-sync

A CLI tool for synchronizing the schema and configuration of Directus across various environments.
GNU General Public License v3.0
241 stars 10 forks source link

Dangling permissions after Directus upgraded #38

Closed km1230 closed 7 months ago

km1230 commented 7 months ago

Hi I am running a Directus app with Docker, I have a local file directus-sync.config.cjs mounted as a volume to the docker container /directus/directus-sync.config.cjs . With this setup and on Directus@10.9.3, directus-sync was doing a great job.

After upgrading to Directus@10.10.4, I ran diff to see if any changes in schema with --force and DEBUG on. The directus-sync diff log shows a list of [permissions] Will remove dangling id map.

If I do a push and run diff again, these messages still persist. At this point, any changes made on directus access control is not recognised by directus-sync.

Wonder if there are any proper steps required for handling Directus upgrade?

Screenshot 2024-03-19 at 3 45 47 pm
EdouardDem commented 7 months ago

Hi @km1230, Which role is behind those permissions ? Is this role also removed from the "id map".

Wonder if there are any proper steps required for handling Directus upgrade?

This is my worflow for an upgrade from version A to B:

Note: I will investigate your case, bescause I had strange behavior today when pulling presets with directus-sync on Directus 10.10.4. This may be related.

EdouardDem commented 7 months ago

@km1230 Since Directus 10.10.4 I had some caching issues. When I change some properties, for exemple I modify the columns displayed in a table view, then I run directus-sync pull. No changes are detected in the presets collection. If I restart Directus (I assume it flushes some sort of cache) then run directus-sync pull, changes are detected.

Did you try to restart Directus after running the push command ?

badrange commented 7 months ago

If I restart Directus (I assume it flushes some sort of cache) then run directus-sync pull, changes are detected.

Hi @EdouardDem are you aware of this api endpoint and have you checked if that works for you, might save you from having to restart directus: https://docs.directus.io/reference/system/utilities.html#clear-the-internal-cache

EdouardDem commented 7 months ago

@badrange Thanks for the tip. I didn't know this endpoint. I will call it before each command. This should avoid these issues.

EdouardDem commented 7 months ago

@km1230 I have reproduced the bug.

It seems since 10.10.4 Directus does not return the id of the permissions. Sample of the response of a readPermissions request :

[
  {
    role: 'e17402cc-b641-4bbe-bd07-4a2652959b8e',
    collection: 'directus_files',
    action: 'create',
    permissions: {},
    validation: null,
    presets: null,
    fields: [ '*' ]
  }
]

This causes directus-sync to believe that no permission is present on the server. I don't know if it is on purpose or a bug from Directus. I will have to find a workaround.

km1230 commented 7 months ago

@EdouardDem Thanks for investigating further. It seems Directus is going to dramatically change their directus_permissions table.

Althought their work is still in progress, directus-sync may better marks the compatible version of Directus at this point. Otherwise, the existing permissions will be wiped out if one tried to repeat pulling and pushing as I observed.

EdouardDem commented 7 months ago

@km1230 Thanks for the suggestion. I have added a badge to the README with the last supported Directus version.

I appears that this bug shows up in version 10.10.0. I have found a workaround. I will implement it right now.

EdouardDem commented 7 months ago

@km1230 I have just published a new release that fix this bug : https://github.com/tractr/directus-sync/releases/tag/directus-sync%401.4.0

Looking forward for your feedback.

PS: I have also create an issue on the Directus Github : https://github.com/directus/directus/issues/21965

asperling commented 7 months ago

@km1230 I have just published a new release that fix this bug : https://github.com/tractr/directus-sync/releases/tag/directus-sync%401.4.0

Looking forward for your feedback.

PS: I have also create an issue on the Directus Github : directus/directus#21965

Hi there, many thanks for all your effort! But unless I am missing something it does not seem to work as I had expected. After upgrading directus-extension-sync and directus-sync I ran a pull to see if permissions (in the json file) would persist, but they didn't. Then I did it the other way around and ran a push first, hoping that would register the permissions in some way. Now that results into a lot of other problems, as now translations, presets and such would get deleted. That wouldn't be the worst as I could merge those from the first approach but permissions would still get emptied so no use.

So is there anything specific one has to do to get permissions synced from an existing permissions.json file of a previous version of directus-sync?

EdouardDem commented 7 months ago

@asperling Can you please send me the versions of directus-extension-sync, directus-sync and Directus ? I will try to reproduce the bug on my side.

km1230 commented 7 months ago

@km1230 I have just published a new release that fix this bug : https://github.com/tractr/directus-sync/releases/tag/directus-sync%401.4.0

Looking forward for your feedback.

PS: I have also create an issue on the Directus Github : directus/directus#21965

Thanks @EdouardDem ! Confirm it is working perfectly on latest version! So good that I gope Directus would inlcude your work into their built-in one day :)

asperling commented 7 months ago

@EdouardDem Hey, sorry, I wasn't available the last couple of days...

Here the versions in question:

Reading the positive messages of others makes me question myself :(

EdouardDem commented 7 months ago

@asperling I will try to reproduce the bug. I'm using the same versions on my projects and I have no issues.

Do you use custom configs such as hooks in file directus-sync.config.js ? Are you using an admin user to pull configs ?

asperling commented 7 months ago

@EdouardDem Please excuse me, I have retraced the setup, reinstalled everything, did some minor changes to the configuration file and now everything is working fine. Due to my vacation and sickness some time went by, so I'm unsure what fixed it for me but my best guess is that removing node_modules did it.

Sorry for the time you spent on this! Please close the issue.

EdouardDem commented 7 months ago

@asperling I'm glad you've fixed it. I'll close the issue.

EdouardDem commented 6 months ago

@asperling I've just published a new version with features you might be interested in. It includes a helper to remove permission duplicates. It also avoid to create duplicated permission with the REST API.

I had a project where the permissions were messed up, and this fixed them all.

Here is the changelog: https://github.com/tractr/directus-sync/releases/tag/directus-sync%401.5.2