🐛 Bug Report: All OIDC clients unrestricted #70

Closed
opened 2025-10-07 00:00:27 +03:00 by OVERLORD · 14 comments
Owner

Originally created by @mitchplze on GitHub.

Reproduction steps

I believe this was triggered with my upgrade to v.1.8.0, but I cannot confirm as I am unable to rollback or recover from backup

Expected behavior

OIDC clients should be restricted to their groups - as previously setup

Actual Behavior

All OIDC clients are now 'Unrestricted' and seem to have forgotten their group associations.

Adding them back seems possible, but this should never have happened.

Version and Environment

v.1.8.0 on Docker

Log Output

None

Originally created by @mitchplze on GitHub. ### Reproduction steps I believe this was triggered with my upgrade to v.1.8.0, but I cannot confirm as I am unable to rollback or recover from backup ### Expected behavior OIDC clients should be restricted to their groups - as previously setup ### Actual Behavior All OIDC clients are now 'Unrestricted' and seem to have forgotten their group associations. Adding them back seems possible, but this should never have happened. ### Version and Environment v.1.8.0 on Docker ### Log Output None
Author
Owner

@ElioDiNino commented on GitHub:

Can confirm this also happened to me when upgrading to 1.8.0

@ElioDiNino commented on GitHub: Can confirm this also happened to me when upgrading to 1.8.0
Author
Owner

@ItalyPaleAle commented on GitHub:

Can you confirm what database you're using? Postgres or SQLite?

@ItalyPaleAle commented on GitHub: Can you confirm what database you're using? Postgres or SQLite?
Author
Owner

@mitchplze commented on GitHub:

I am using the default (SQLite)

Same

@mitchplze commented on GitHub: > I am using the default (SQLite) Same
Author
Owner

@ElioDiNino commented on GitHub:

Can you confirm what database you're using? Postgres or SQLite?

I am using the default (SQLite)

@ElioDiNino commented on GitHub: > Can you confirm what database you're using? Postgres or SQLite? I am using the default (SQLite)
Author
Owner

@dlhall111 commented on GitHub:

I am also using the default (SQLite) and have two instances of Pocket ID, both have this same issue after the update.

Edit: In light of a bug like this I think it would be a very good idea to make the default access admin-only. :)

@dlhall111 commented on GitHub: I am also using the default (SQLite) and have two instances of Pocket ID, both have this same issue after the update. Edit: In light of a bug like this I think it would be a very good idea to make the *default* access admin-only. :)
Author
Owner

@michaelbeaumont commented on GitHub:

I upgraded from 1.7.0 to 1.9.0 just now and my mappings are gone too

@michaelbeaumont commented on GitHub: I upgraded from 1.7.0 to 1.9.0 just now and my mappings are gone too
Author
Owner

@abno85 commented on GitHub:

I can confirm the deletions. I also just upgraded from 1.7.0 to 1.9.0 and all group assignments were gone afterwards.
I'm using version tags in my compose file, so there definitely was no auto upgrade to 1.8.x in the meantime.

Edit: I'm using SQLite and users imported via LDAP.

@abno85 commented on GitHub: I can confirm the deletions. I also just upgraded from 1.7.0 to 1.9.0 and all group assignments were gone afterwards. I'm using version tags in my compose file, so there definitely was no auto upgrade to 1.8.x in the meantime. Edit: I'm using SQLite and users imported via LDAP.
Author
Owner

@ElioDiNino commented on GitHub:

Hey @stonith404, thank you for the info. Unfortunately, I updated to 1.8.1 this morning in my production environment (I had only done my staging environment before where I saw the bug initially) and the mappings were still wiped out (going from 1.7.0 to 1.8.1). As a result, I am not convinced the bug has been fixed.

@ElioDiNino commented on GitHub: Hey @stonith404, thank you for the info. Unfortunately, I updated to 1.8.1 this morning in my production environment (I had only done my staging environment before where I saw the bug initially) and the mappings were still wiped out (going from 1.7.0 to 1.8.1). As a result, I am not convinced the bug has been fixed.
Author
Owner

@stonith404 commented on GitHub:

Are you sure that it didn't upgrade to v1.8.0 automatically?

@stonith404 commented on GitHub: Are you sure that it didn't upgrade to v1.8.0 automatically?
Author
Owner

@stonith404 commented on GitHub:

Sorry guys, this is was a pretty bad bug. The bug has been fixed in v1.8.1 and the v1.8.0 images have been removed from the registry.

The issue was introduced with the enforcement of foreign key constraints. Since SQLite doesn’t allow altering tables in place, our migration deletes and recreates the affected tables. What I overlooked is that dropping a table also deletes all of its data, and with cascade deletes enabled, all related child records were removed as well. As a result, when the oidc_clients table was recreated, all associated “allowed user groups” entries were lost.

We’ve fixed this by temporarily disabling foreign key checks during migrations to prevent cascading deletions.

Unfortunately, the deleted relations cannot be restored automatically. You’ll need to either restore them from a backup or recreate them manually.

@stonith404 commented on GitHub: Sorry guys, this is was a pretty bad bug. The bug has been fixed in `v1.8.1` and the `v1.8.0` images have been removed from the registry. The issue was introduced with the enforcement of foreign key constraints. Since SQLite doesn’t allow altering tables in place, our migration deletes and recreates the affected tables. What I overlooked is that dropping a table also deletes all of its data, and with cascade deletes enabled, all related child records were removed as well. As a result, when the oidc_clients table was recreated, all associated “allowed user groups” entries were lost. We’ve fixed this by temporarily disabling foreign key checks during migrations to prevent cascading deletions. Unfortunately, the deleted relations cannot be restored automatically. You’ll need to either restore them from a backup or recreate them manually.
Author
Owner

@meerumschlungen commented on GitHub:

When going 1.7.0 -> 1.8.0 the groups were gone. I recreated some memberships then. I then went to 1.8.1 and eventually to 1.9.0. The ones I reassigned on 1.8.0 have not been pruned now.

@meerumschlungen commented on GitHub: When going 1.7.0 -> 1.8.0 the groups were gone. I recreated some memberships then. I then went to 1.8.1 and eventually to 1.9.0. The ones I reassigned on 1.8.0 have not been pruned now.
Author
Owner

@ElioDiNino commented on GitHub:

Are you sure that it didn't upgrade to v1.8.0 automatically?

Yes

@ElioDiNino commented on GitHub: > Are you sure that it didn't upgrade to v1.8.0 automatically? Yes
Author
Owner

@stonith404 commented on GitHub:

Thanks for all the comments. It seems like the "fix" didn't actually fix the bug because PRAGMA foreign_keys = OFF; can't be set inside a transaction. As I have tested the fix inside DataGrip which doesn't create a transaction by default, I thought the changes in v1.8.1 would fix the issue.

In v1.9.1 the bug should now actually be fixed.

@stonith404 commented on GitHub: Thanks for all the comments. It seems like the "fix" didn't actually fix the bug because `PRAGMA foreign_keys = OFF;` can't be set inside a transaction. As I have tested the fix inside DataGrip which doesn't create a transaction by default, I thought the changes in `v1.8.1` would fix the issue. In `v1.9.1` the bug should now actually be fixed.
Author
Owner

@michaelbeaumont commented on GitHub:

Same. My changes are visible in these commits. I manually just ran terraform apply for the first time since before v1.8.0 was released. I'm using sqlite.

@michaelbeaumont commented on GitHub: Same. My changes are visible [in these commits](https://github.com/michaelbeaumont/pocket-id-gcp/commits/main/). I manually just ran `terraform apply` for the first time since before v1.8.0 was released. I'm using sqlite.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/pocket-id#70