Skip to content

[Fixes #14519] GroupProfile.access default is "public'", not a valid choice - #14520

Open
christianbraun wants to merge 1 commit into
GeoNode:masterfrom
christianbraun:fix/groupprofile-access-default
Open

[Fixes #14519] GroupProfile.access default is "public'", not a valid choice#14520
christianbraun wants to merge 1 commit into
GeoNode:masterfrom
christianbraun:fix/groupprofile-access-default

Conversation

@christianbraun

@christianbraun christianbraun commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Fixes #14519.

GroupProfile.access was declared with default="public'" — a stray apostrophe, matching none of the field's own GROUP_CHOICES. Any group created without an explicit access was stored with that value and became invisible to code filtering on access="public", notably geonode.people.utils.get_available_users. The typo has been there since 796add4 (2014) and is also in 24_initial.py.

Three changes:

  • geonode/groups/models.pydefault="public'"default="public".
  • geonode/groups/migrations/0036_fix_groupprofile_access_default.py — the AlterField for the new default, plus a RunPython that rewrites existing "public'" rows to "public". Existing installs need the data fix; the model fix alone leaves every group already created still broken. The RunPython is noop in reverse, since undoing it would mean writing an invalid value back.
  • geonode/groups/tests.pytest_default_access_is_a_valid_choice, asserting a GroupProfile created without an access is both a valid choice and returned by filter(access="public").

How we hit it: on an AD-backed instance, geonode_ldap's updateldapgroups creates GroupProfiles without passing access, so all 276 mirrored groups carried "public'" and a group picker filtering on "public" showed none of them.

Most of core is unaffected because it keys on access == "private" instead, which the typo leaves alone — that is why this has survived so long.

Checklist

For all pull requests:

  • Confirm you have read the contribution guidelines
  • You have sent a Contribution Licence Agreement (CLA) as necessary (not required for small changes, e.g., fixing typos in the documentation)
  • Make sure the first PR targets the master branch, eventual backports will be managed later. This can be ignored if the PR is fixing an issue that only happens in a specific branch, but not in newer ones.

The following are required only for core and extension modules (they are welcomed, but not required, for contrib modules):

  • There is a ticket in https://github.com/GeoNode/geonode/issues describing the issue/improvement/feature (a notable exemption is, changes not visible to end-users)
  • The issue connected to the PR must have Labels and Milestone assigned
  • PR for bug fixes and small new features are presented as a single commit
  • PR title must be in the form "[Fixes #<issue_number>] Title of the PR"
  • New unit tests have been added covering the changes, unless there is an explanation on why the tests are not necessary/implemented

Happy to backport to 4.4.x / 5.0.x once this lands. I could not run the full suite locally, so CI is the first real execution of the migration.

… valid choice

The default carried a stray apostrophe, so it matched none of the field's own
GROUP_CHOICES. Any GroupProfile created without an explicit access — which is
what geonode_ldap's group mirroring does — was stored as "public'" and was
therefore invisible to code filtering on access="public", such as
geonode.people.utils.get_available_users. Present since 796add4 (2014).

Adds a data migration so existing rows are repaired too; fixing the default
alone would leave them broken, and fixing only the data would let the next
group sync write the bad value straight back.
@cla-bot

cla-bot Bot commented Aug 14, 2026

Copy link
Copy Markdown

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @christianbraun on file. In order for us to review and merge your code, please contact the project maintainers to get yourself added.

@christianbraun

Copy link
Copy Markdown
Contributor Author

The CLA is signed and sent. As the bot notes, @christianbraun still needs adding on your side — could a maintainer take care of that?

Meanwhile validate (3.12), both CodeQL analyses and the Docker image build are green; Smoke Tests are still running, and that job is the first real execution of the new migration anywhere, since 0036 depends on 0035_remove_modeltranslation and I had no local environment on master to run it against. Happy to adjust if the AlterField turns out not to match what makemigrations regenerates — I modelled the serialised help_text on 0028_auto_20200114_1651.py.

@giohappy

Copy link
Copy Markdown
Contributor

Thanks @christianbraun for this long-stainding silent bug.
Can you please include the .clabot file with your account appended?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GroupProfile.access default is "public'" (stray apostrophe), matching none of its own GROUP_CHOICES

2 participants