New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Access rules for SystemInformationController and MemcacheServer #12316

Merged
merged 1 commit into from Jan 29, 2019

Conversation

Projects
None yet
6 participants
@matks
Copy link
Contributor

matks commented Jan 25, 2019

Questions Answers
Branch? develop
Description? Update access rules for these 2 controllers
Type? improvement
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket?
How to test? See below

How to test

Before this PR, you could see "changed files" block from "Configure > Advanced Parameters > Information" page (which is loaded through an AJAX call) and you could use the cache server actions (add a server, test a server, delete a server) from "Configure > Advanced Parameters > Performance" page (last block) without having the proper access rights. This PR secures these 2 items.

See screenshot below


This change is Reviewable

@matks

This comment has been minimized.

Copy link
Contributor Author

matks commented Jan 25, 2019

"Changed files" block
capture d ecran 2019-01-25 a 16 48 50

@matks

This comment has been minimized.

Copy link
Contributor Author

matks commented Jan 25, 2019

"Cache server management block"
capture d ecran 2019-01-25 a 16 52 23

@PierreRambaud PierreRambaud added this to the 1.7.6.0 milestone Jan 26, 2019

@ntiepresta ntiepresta assigned ntiepresta and unassigned ntiepresta Jan 28, 2019

@ntiepresta ntiepresta added QA ✔️ and removed waiting for QA labels Jan 28, 2019

@Quetzacoalt91

This comment has been minimized.

Copy link
Member

Quetzacoalt91 commented Jan 28, 2019

@matks it seems this PR broke the Travis builds.

@Quetzacoalt91 Quetzacoalt91 force-pushed the matks:refacto-memcache-and-info-pages-annotations branch from baa23dd to db36b68 Jan 29, 2019

@Quetzacoalt91

This comment has been minimized.

Copy link
Member

Quetzacoalt91 commented Jan 29, 2019

PR rebased and with cs-fixer applied.

@matks

This comment has been minimized.

Copy link
Contributor Author

matks commented Jan 29, 2019

Thanks for the help @Quetzacoalt91

@matks matks merged commit 975647f into PrestaShop:develop Jan 29, 2019

1 of 2 checks passed

code-review/reviewable 2 files left
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@matks matks deleted the matks:refacto-memcache-and-info-pages-annotations branch Jan 29, 2019

@marionf

This comment has been minimized.

Copy link
Contributor

marionf commented Jan 31, 2019

@matks I have an employee profile with only read right for performances page.
If I try to add a server, I have both success and access forbidden message, which is confusing.

capture d ecran_948

@matks

This comment has been minimized.

Copy link
Contributor Author

matks commented Feb 1, 2019

@marionf I failed to reproduce 🤔 can you retry ? I have only the error message. Do you try with memcache or xcache ? The block we're discussing here is "Caching, not "Media servers (use only with CCC)".

This might be a side-effect: whenever you try to do something on a Symfony page, the feedback messages (which includes "access denied", "successfull update", "could not find object 123" ...) are stored to be displayed until the next Symfony page to be shown.

As stated by this post:
"they're pretty useful to store special messages that should be shown once the user visits a view. It's called flash message because it's stored in the session (it doesn't care if is signed in or not) and once the user visits (or is redirected to) another route of your project then it can be handled by you (shown on the view) and then it will disappear. In short words, they vanish from the session automatically as soon as you retrieve them. This feature makes "flash" messages particularly great for storing user notifications."

So maybe what happened is that you visited a Symfony page, got a "success full update" message stored, and it popped right there.

@marionf

This comment has been minimized.

Copy link
Contributor

marionf commented Feb 1, 2019

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