Skip to content

Conversation

@andrepereiradasilva
Copy link
Contributor

@andrepereiradasilva andrepereiradasilva commented Aug 14, 2016

Pull Request for Improvement.

Summary of Changes

This PR is the sequence of #11593.

Replace all remaining administrator components existing 404 JError for a 403 php exception (JAccessExceptionNotallowed) when the user does not have access to "Access Administration Interface" (core.manage).

Before

image

After

image

Testing Instructions

  • Use latest staging
  • Apply patch
  • Create a user and add it to "Manager" group
  • Go to to all components and set "Access Administration Interface" (core.manage) to "Denied" for "Manager" group in all remaning components (com_content, com_banners, com_contact, com_media, com_newsfeeds, com_search and com_finder)
  • Try to access the following URL, you should see 403 errors now:
/administrator/index.php?option=com_banners
/administrator/index.php?option=com_cache
/administrator/index.php?option=com_categories
/administrator/index.php?option=com_categories&extention=com_content
/administrator/index.php?option=com_checkin
/administrator/index.php?option=com_contact
/administrator/index.php?option=com_content
/administrator/index.php?option=com_contenthistory&view=history (this one will give a layout not definied error)
/administrator/index.php?option=com_finder
/administrator/index.php?option=com_installer
/administrator/index.php?option=com_joomlaupdate
/administrator/index.php?option=com_languages
/administrator/index.php?option=com_media
/administrator/index.php?option=com_menus
/administrator/index.php?option=com_messages
/administrator/index.php?option=com_modules
/administrator/index.php?option=com_newsfeeds
/administrator/index.php?option=com_plugins
/administrator/index.php?option=com_redirect
/administrator/index.php?option=com_search
/administrator/index.php?option=com_tags
/administrator/index.php?option=com_templates
/administrator/index.php?option=com_users
  • Code review.

Note the other admin components (com_admin, com_ajax, com_cpanel, com_postinstall) doesn't use this or already use exceptions.

Didn't touch com_config. This one needs another PR.

Documentation Changes Required

None.

@wilsonge
Copy link
Contributor

OK so I'm going to be super nasty about this. I think this needs a different exception class - this is not an exception thrown in the controller therefore in my opinion JControllerExceptionNotallowed is the wrong exception to be used here

@andrepereiradasilva
Copy link
Contributor Author

ok. i understand, so what do you recomend then?

@andrepereiradasilva
Copy link
Contributor Author

andrepereiradasilva commented Aug 15, 2016

Add a new JAccessExceptionNotallowed exception?

@wilsonge
Copy link
Contributor

wilsonge commented Aug 15, 2016

I think so - and thinking about it maybe extend the controller exception from that?

@andrepereiradasilva
Copy link
Contributor Author

ok done.

Removed the JControllerExceptionNotAllowed since it doesn't make sense now (it was added yesterday so no B/C break)

@jeckodevelopment
Copy link
Member

I have tested this item ✅ successfully on bb75a88


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11608.

@jeckodevelopment
Copy link
Member

Works as expected.

11608

@wilsonge wilsonge added this to the Joomla 3.6.3 milestone Aug 15, 2016
@wilsonge wilsonge merged commit 8c23361 into joomla:staging Aug 15, 2016
@andrepereiradasilva andrepereiradasilva deleted the 403-exceptions branch August 15, 2016 23:16
ggppdk pushed a commit to ggppdk/joomla-cms that referenced this pull request Aug 19, 2016
…s) (joomla#11608)

* add exception to com_cache

* Update categories.php

* Update checkin.php

* Update contact.php

* Update content.php

* Update contenthistory.php

* Update finder.php

* Update installer.php

* Update joomlaupdate.php

* Update languages.php

* Update media.php

* Update menus.php

* Update messages.php

* Update modules.php

* Update newsfeeds.php

* Update redirect.php

* Update search.php

* Update tags.php

* Update templates.php

* Update users.php

* Update templates.php

* move to JAccessExceptionNotallowed

* move to JAccessExceptionNotallowed 2
roland-d pushed a commit to roland-d/joomla-cms that referenced this pull request Sep 11, 2016
…s) (joomla#11608)

* add exception to com_cache

* Update categories.php

* Update checkin.php

* Update contact.php

* Update content.php

* Update contenthistory.php

* Update finder.php

* Update installer.php

* Update joomlaupdate.php

* Update languages.php

* Update media.php

* Update menus.php

* Update messages.php

* Update modules.php

* Update newsfeeds.php

* Update redirect.php

* Update search.php

* Update tags.php

* Update templates.php

* Update users.php

* Update templates.php

* move to JAccessExceptionNotallowed

* move to JAccessExceptionNotallowed 2
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.

4 participants