Skip to content

Fix leaking emails on admin user search controller#9791

Merged
ahukkanen merged 3 commits intodecidim:developfrom
openpoke:fix/meeting-invitations-email-leak
Sep 13, 2022
Merged

Fix leaking emails on admin user search controller#9791
ahukkanen merged 3 commits intodecidim:developfrom
openpoke:fix/meeting-invitations-email-leak

Conversation

@microstudi
Copy link
Copy Markdown
Contributor

@microstudi microstudi commented Sep 12, 2022

🎩 What? Why?

Due to data privacy, emails are kept from administrators.

However, the XHR methods users and users_entities in organization controller allows anyone to search and displays email addresses from the entire database, including blocked users, when inviting members to meetings:

This PR fixes this by removing the email from the result (although it is possible to search a user using the email) and excludes blocked, managed or deleted users from the result

📌 Related Issues

Link your PR to an issue

  • Related to #?
  • Fixes #?

Testing

Describe the best way to test or validate your PR.

📷 Screenshots

Please add screenshots of the changes you're proposing
Description

♥️ Thank you!

# Conflicts:
#	decidim-admin/spec/controllers/organizations_controller_spec.rb
@microstudi microstudi added the type: fix PRs that implement a fix for a bug label Sep 12, 2022
Copy link
Copy Markdown
Contributor

@ahukkanen ahukkanen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

Could you fix the linting issue in the spec? After that, LGTM.

@microstudi
Copy link
Copy Markdown
Contributor Author

Corrected @ahukkanen. It's ok to backport to 0.26 and 0.27 this and #9790 ?

@ahukkanen
Copy link
Copy Markdown
Contributor

Corrected @ahukkanen. It's ok to backport to 0.26 and 0.27 this and #9790 ?

Since this is a violation of privacy, I think it is fine to backport this. Can you please use the backporter script for this. Run ./bin/backporter --help to get started.

Normally we should be very careful with the backports not to add any additional functionality to the old versions. This PR is introducing a new scope to the UserBaseEntity class which would normally categorize this as a change, not a fix. Backporting that to the unreleased 0.27 would be normally totally fine but for 0.26 that could be problematic.

But as said above, I think this is serious enough that in this particular case we can live with backporting a new "feature" to the old versions (we've done that in some specific cases in the past too).

Also just to note here for the record, I wouldn't consider this a major level security issue because the emails are only exposed for people who have access to the admin panel. But this is still serious enough from Decidim's own guidelines perspective.

@microstudi
Copy link
Copy Markdown
Contributor Author

Thanks @ahukkanen! BTW, lovin' the backporter script! works like a charm!

@ahukkanen ahukkanen merged commit 2cf03c0 into decidim:develop Sep 13, 2022
microstudi added a commit that referenced this pull request Sep 13, 2022
* rename test

* fix leaking emails on admin user search controller

# Conflicts:
#	decidim-admin/spec/controllers/organizations_controller_spec.rb

* lint spec
# Conflicts:
#	decidim-admin/spec/controllers/organizations_contoller_spec.rb
microstudi added a commit that referenced this pull request Sep 13, 2022
* rename test

* fix leaking emails on admin user search controller

# Conflicts:
#	decidim-admin/spec/controllers/organizations_controller_spec.rb

* lint spec
# Conflicts:
#	decidim-admin/spec/controllers/organizations_contoller_spec.rb
ahukkanen pushed a commit that referenced this pull request Sep 13, 2022
…#9797)

* Fix leaking emails on admin user search controller (#9791)

* rename test

* fix leaking emails on admin user search controller

# Conflicts:
#	decidim-admin/spec/controllers/organizations_controller_spec.rb

* lint spec
# Conflicts:
#	decidim-admin/spec/controllers/organizations_contoller_spec.rb

* lint for 2.7
ahukkanen pushed a commit that referenced this pull request Sep 13, 2022
…#9796)

* Fix leaking emails on admin user search controller (#9791)

* rename test

* fix leaking emails on admin user search controller

# Conflicts:
#	decidim-admin/spec/controllers/organizations_controller_spec.rb

* lint spec
# Conflicts:
#	decidim-admin/spec/controllers/organizations_contoller_spec.rb

* lint for 3.0
entantoencuanto added a commit that referenced this pull request Sep 13, 2022
* develop:
  Add missing character on code block (#9798)
  Fix hidden error messages on the registration form (#9625)
  Add documentation about configuring ActiveStorage / dynamic file uploads (#9777)
  Add documentation section about customizing cells (#9622)
  Fix hashtags not recognized at the beginning of the string (#9616)
  Fix version pages showing a HTTP 500 error when the version does not exist (#9615)
  Fix multitenant organizations stats cache (#9605)
  Prevent the account edit route through Devise (#9611)
  Fix iframe disabling producing invalid HTML (#9685)
  Fix import of images on spaces (#9779)
  Fix order of last activities (#9756)
  Fix leaking emails on admin user search controller (#9791)
  Ignore participatory spaces without models in meetings visible_for scope (#9790)
eliegaboriau pushed a commit to eliegaboriau/decidim that referenced this pull request Oct 25, 2022
* rename test

* fix leaking emails on admin user search controller

# Conflicts:
#	decidim-admin/spec/controllers/organizations_controller_spec.rb

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

Labels

module: admin type: fix PRs that implement a fix for a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants