[#11679] - Remove legacy browsers Frame Breaking js code usage and deprecate it
- Closed
- 4 Sep 2016
- Medium
- Build: staging
- # 11679
- Diff
- andrepereiradasilva:patch-7
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed. Details
User tests: Successful: Unsuccessful:
Summary of Changes
Remove Legacy Browser (IE 7 and older) Frame Breaking Script.
Since Joomla 3.x only support IE8+ and modern browsers this code have no effect anymore because all this browsers support the X-Frame-Options header that already does this.
See https://www.owasp.org/index.php/Clickjacking_Defense_Cheat_Sheet
This code was only used in admin com_login page:
- isis: https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_login/views/login/tmpl/default.php#L12
- hathor: https://github.com/joomla/joomla-cms/blob/staging/administrator/templates/hathor/login.php#L23
So this PR deprecates the behaviour.noframes and in replace adds and HTTP Header in com_login admin view html file.
Testing Instructions
- Apply patch
- Admin login in isis and hathor work without issues and have the
X-Frame-OptionsHTTP header - Code review
Documentation Changes Required
None.
joomla-cms-bot
- | Category | ⇒ | Libraries |
| Status | New | ⇒ | Pending |
| Labels |
Added:
?
|
||
JHtmlBehaviorTest::testNoFrames is validating that JHtmlBehavior::core gets
called. You're removing the call to it so remove that from the array
that's being validated.
For the other failures, I'm leaning toward that being a result of the JHtml
tests affecting global state and a value not being reset correctly.
On Friday, August 19, 2016, andrepereiradasilva notifications@github.com
wrote:
need some help with the unit tests.
JHtmlBehaviorTest::testNoFrames is failing and don't really know how to
rewrite it.—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#11679 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfobN_3pgi0skKvNghSpHltR1xYD3jks5qhlDngaJpZM4Jo9Ir
.
@andrepereiradasilva B/C policy states that this can not be done in 3.x. What could be done is to remove the calls to this function and deprecate the function so it will be removed in j4
Ah, ok didn't catch that, still I think deprecating this and adding
JFactory::getApplication()->setHeader('X-Frame-Options', 'SAMEORIGIN');to the two views is safer, but I don't mind either way
ok have no issue with that and also think it's better.
will check.
| Category | Libraries | ⇒ | Administration Components Libraries |
| Category | Libraries Administration Components | ⇒ | Administration Components Templates (admin) Libraries |
| Title |
|
||||||
I have tested this item
Tested on OSX with: safari, chrome, ff, opera (latest, development, tech review)
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11679.
I have tested this item
I didnt even know we had this protection - cool;)
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11679.
| Status | Pending | ⇒ | Ready to Commit |
RTC
| Labels |
Added:
?
|
||
| Status | Ready to Commit | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-09-04 12:49:37 |
| Closed_By | ⇒ | wilsonge |
| Labels |
Removed:
?
|
||
need some help with the unit tests.
JHtmlBehaviorTest::testNoFrames is failing and don't really know how to rewrite it.