Skip to content

Battery-status: Restrict to secure context and top-level browsing con…#5871

Merged
dontcallmedom merged 1 commit into
web-platform-tests:masterfrom
Honry:battery/secure-context
May 10, 2017
Merged

Battery-status: Restrict to secure context and top-level browsing con…#5871
dontcallmedom merged 1 commit into
web-platform-tests:masterfrom
Honry:battery/secure-context

Conversation

@Honry

@Honry Honry commented May 10, 2017

Copy link
Copy Markdown
Contributor

…text

@Honry

Honry commented May 10, 2017

Copy link
Copy Markdown
Contributor Author

@riju or @anssiko, PTAL.

@ghost

ghost commented May 10, 2017

Copy link
Copy Markdown

View the complete job log.

Firefox (nightly)

Testing web-platform-tests at revision 374287f
Using browser at version BuildID 20170509100208; SourceStamp b21b974d60d3075ae24f6fb1bae75d0f122f28fc
Starting 10 test iterations
All results were stable

All results

5 tests ran
/battery-status/battery-iframe.https.html
Subtest Results Messages
OK
throw a 'SecurityError' when invoking navigator.getBattery() within iframe FAIL promise_test: Unhandled rejection with value: object "TypeError: iframe.contentWindow.navigator.getBattery is not a function"
/battery-status/battery-insecure-context.html
Subtest Results Messages
OK
navigator.getBattery() shall throw a 'SecurityError' in an insecure context FAIL navigator.getBattery is not a function
/battery-status/battery-interface-idlharness.https.html
Subtest Results Messages
OK
Battery test: IDL FAIL assert_idl_attribute: navigator must have getBattery attribute property "getBattery" not found in prototype chain
/battery-status/battery-promise-window.https.html
Subtest Results Messages
TIMEOUT
window.open() makes a different Navigator object thus getting another battery promise TIMEOUT Test timed out
/battery-status/battery-promise.https.html
Subtest Results Messages
OK
navigator.getBattery() shall return BatteryManager as a promise FAIL navigator.getBattery is not a function
navigator.getBattery() shall always return the same promise FAIL navigator.getBattery is not a function

@ghost

ghost commented May 10, 2017

Copy link
Copy Markdown

View the complete job log.

Chrome (unstable)

Testing web-platform-tests at revision 374287f
Using browser at version 60.0.3088.3 dev
Starting 10 test iterations
All results were stable

All results

5 tests ran
/battery-status/battery-iframe.https.html
Subtest Results Messages
OK
throw a 'SecurityError' when invoking navigator.getBattery() within iframe FAIL assert_unreached: Should have rejected: undefined Reached unreachable code
/battery-status/battery-insecure-context.html
Subtest Results Messages
OK
navigator.getBattery() shall throw a 'SecurityError' in an insecure context FAIL assert_unreached: Should have rejected: undefined Reached unreachable code
/battery-status/battery-interface-idlharness.https.html
Subtest Results Messages
OK
Battery test: IDL PASS
Navigator interface: operation getBattery() PASS
Navigator must be primary interface of navigator PASS
Stringification of navigator PASS
Navigator interface: navigator must inherit property "getBattery" with the proper type (0) PASS
BatteryManager interface: existence and properties of interface object PASS
BatteryManager interface object length PASS
BatteryManager interface object name PASS
BatteryManager interface: existence and properties of interface prototype object PASS
BatteryManager interface: existence and properties of interface prototype object's "constructor" property PASS
BatteryManager interface: attribute charging PASS
BatteryManager interface: attribute chargingTime PASS
BatteryManager interface: attribute dischargingTime PASS
BatteryManager interface: attribute level PASS
BatteryManager interface: attribute onchargingchange PASS
BatteryManager interface: attribute onchargingtimechange PASS
BatteryManager interface: attribute ondischargingtimechange PASS
BatteryManager interface: attribute onlevelchange PASS
BatteryManager must be primary interface of manager PASS
Stringification of manager PASS
BatteryManager interface: manager must inherit property "charging" with the proper type (0) PASS
BatteryManager interface: manager must inherit property "chargingTime" with the proper type (1) PASS
BatteryManager interface: manager must inherit property "dischargingTime" with the proper type (2) PASS
BatteryManager interface: manager must inherit property "level" with the proper type (3) PASS
BatteryManager interface: manager must inherit property "onchargingchange" with the proper type (4) PASS
BatteryManager interface: manager must inherit property "onchargingtimechange" with the proper type (5) PASS
BatteryManager interface: manager must inherit property "ondischargingtimechange" with the proper type (6) PASS
BatteryManager interface: manager must inherit property "onlevelchange" with the proper type (7) PASS
EventTarget interface: manager must inherit property "addEventListener" with the proper type (0) PASS
EventTarget interface: calling addEventListener(DOMString,EventListener,boolean) on manager with too few arguments must throw TypeError PASS
EventTarget interface: manager must inherit property "removeEventListener" with the proper type (1) PASS
EventTarget interface: calling removeEventListener(DOMString,EventListener,boolean) on manager with too few arguments must throw TypeError PASS
EventTarget interface: manager must inherit property "dispatchEvent" with the proper type (2) PASS
EventTarget interface: calling dispatchEvent(Event) on manager with too few arguments must throw TypeError PASS
/battery-status/battery-promise-window.https.html
Subtest Results Messages
OK
window.open() makes a different Navigator object thus getting another battery promise PASS
/battery-status/battery-promise.https.html
Subtest Results Messages
OK
navigator.getBattery() shall return BatteryManager as a promise PASS
navigator.getBattery() shall always return the same promise PASS

@riju

riju commented May 10, 2017

Copy link
Copy Markdown
Contributor

Thanks! LGTM

@anssiko

anssiko commented May 10, 2017

Copy link
Copy Markdown
Member

@dontcallmedom, can you merge this test suite update, thanks!

@dontcallmedom
dontcallmedom merged commit de8c4b2 into web-platform-tests:master May 10, 2017
…text

 - Tests update for w3c/battery#10
 - Append https postfix
 - Add two tests for checking SecurityError with insecure context
   and non top-level browsing context
 - Remove tests for checking different Navigator object in iframe
 - Add missing author and spec link
scheib pushed a commit to scheib/chromium that referenced this pull request Jun 1, 2017
…t only.

Make the Battery Status API available only within a secure context that is
also a top-level browsing context. This disallows the use of the API within
framed content, as well as from any content that is not a secure context.

Details: w3c/battery#10

WPT updated in web-platform-tests/wpt#5871

BUG=661792

Review-Url: https://codereview.chromium.org/2880763002
Cr-Commit-Position: refs/heads/master@{#476263}
MXEBot pushed a commit to mirror/chromium that referenced this pull request Jun 14, 2017
…ing context only. (patchset #6 id:160001 of https://codereview.chromium.org/2880763002/ )

Reason for revert:
This CL should have gotten a API OWNER lgtm with an Intent to Ship. If the change is accepted by the API OWNERS, we should re-land this.

Original issue's description:
> [Battery] Allow usage from SecureContext or top-level browsing context only.
>
> Make the Battery Status API available only within a secure context that is
> also a top-level browsing context. This disallows the use of the API within
> framed content, as well as from any content that is not a secure context.
>
> Details: w3c/battery#10
>
> WPT updated in web-platform-tests/wpt#5871
>
> BUG=661792
>
> Review-Url: https://codereview.chromium.org/2880763002
> Cr-Commit-Position: refs/heads/master@{#476263}
> Committed: https://chromium.googlesource.com/chromium/src/+/3543d97ca7a33fb8ad48261ad252428555747896

[email protected],[email protected],[email protected]
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=661792

Review-Url: https://codereview.chromium.org/2937553005
Cr-Commit-Position: refs/heads/master@{#479025}
rakuco pushed a commit to w3c/battery that referenced this pull request Jan 31, 2022
…face.

In other words, stop exposing this API to insecure origins.

This has been discussed since at least 2016 (see #5). #11 made access from
an insecure origin throw a SecurityError at a time when the
`[SecureContext]` Web IDL extended attribute was not widespread.
Unfortunately, the spec change was not accompanied by a change in the
implementation(s) and, to this day, Blink's implementation (the only
remaining one) continues to expose the API to insecure origins.

Years later, #30 was added in the context of the discussions in #15 where it
was noted that the spec was still manually throwing a SecurityError when
checking if it was called by a secure origin. Besides stopping throwing a
SecurityError (which was never implemented), #30 also recognized the current
situation by noting that the API _should_ use `[SecureContext]` but did not
because it only made sense to do so when the implementation was updated
accordingly.

This change finally adds `[SecureContext]` to the spec's Web IDL because I
am also handling the Blink implementation [1][2]: starting with Chrome 99,
users will be warned that using the Battery Status API in an insecure origin
is deprecated, and starting with Chrome 103 doing so will no longer be
possible.

[1] https://chromestatus.com/feature/4878376799043584
[2] https://groups.google.com/a/chromium.org/g/blink-dev/c/w80tJL8uEV8/m/PfrHlvtgAgAJ

From a testing perspective, there isn't much to be done:
- web-platform-tests/wpt#5871 changed the existing tests to run in HTTPS.
- web-platform-tests/wpt#32556 removed the test for the SecurityError
  exception that never passed.

Fixes #15
rakuco pushed a commit to w3c/battery that referenced this pull request Feb 1, 2022
…face.

In other words, stop exposing this API to insecure origins. Even though this
API is not new, it provides user information that, transmitted insecurely,
can pose a risk to user privacy. See
https://w3ctag.github.io/design-principles/#secure-context for more
information on the guidelines we are trying to follow.

This has been discussed since at least 2016 (see #5). #11 made access from
an insecure origin throw a SecurityError at a time when the
`[SecureContext]` Web IDL extended attribute was not widespread.
Unfortunately, the spec change was not accompanied by a change in the
implementation(s) and, to this day, Blink's implementation (the only
remaining one) continues to expose the API to insecure origins.

Years later, #30 was added in the context of the discussions in #15 where it
was noted that the spec was still manually throwing a SecurityError when
checking if it was called by a secure origin. Besides stopping throwing a
SecurityError (which was never implemented), #30 also recognized the current
situation by noting that the API _should_ use `[SecureContext]` but did not
because it only made sense to do so when the implementation was updated
accordingly.

This change finally adds `[SecureContext]` to the spec's Web IDL because I
am also handling the Blink implementation [1][2]: starting with Chrome 99,
users will be warned that using the Battery Status API in an insecure origin
is deprecated, and starting with Chrome 103 doing so will no longer be
possible.

[1] https://chromestatus.com/feature/4878376799043584
[2] https://groups.google.com/a/chromium.org/g/blink-dev/c/w80tJL8uEV8/m/PfrHlvtgAgAJ

From a testing perspective, there isn't much to be done:
- web-platform-tests/wpt#5871 changed the existing tests to run in HTTPS.
- web-platform-tests/wpt#32556 removed the test for the SecurityError
  exception that never passed.

Fixes #15
rakuco pushed a commit to w3c/battery that referenced this pull request Feb 1, 2022
…face.

In other words, stop exposing this API to insecure origins. Even though this
API is not new, it provides user information that, transmitted insecurely,
can pose a risk to user privacy. See
https://w3ctag.github.io/design-principles/#secure-context for more
information on the guidelines we are trying to follow.

This has been discussed since at least 2016 (see #5). #11 made access from
an insecure origin throw a SecurityError at a time when the
`[SecureContext]` Web IDL extended attribute was not widespread.
Unfortunately, the spec change was not accompanied by a change in the
implementation(s) and, to this day, Blink's implementation (the only
remaining one) continues to expose the API to insecure origins.

Years later, #30 was added in the context of the discussions in #15 where it
was noted that the spec was still manually throwing a SecurityError when
checking if it was called by a secure origin. Besides stopping throwing a
SecurityError (which was never implemented), #30 also recognized the current
situation by noting that the API _should_ use `[SecureContext]` but did not
because it only made sense to do so when the implementation was updated
accordingly.

This change finally adds `[SecureContext]` to the spec's Web IDL because I
am also handling the Blink implementation [1][2]: starting with Chrome 99,
users will be warned that using the Battery Status API in an insecure origin
is deprecated, and starting with Chrome 103 doing so will no longer be
possible.

[1] https://chromestatus.com/feature/4878376799043584
[2] https://groups.google.com/a/chromium.org/g/blink-dev/c/w80tJL8uEV8/m/PfrHlvtgAgAJ

From a testing perspective, there isn't much to be done:
- web-platform-tests/wpt#5871 changed the existing tests to run in HTTPS.
- web-platform-tests/wpt#32556 removed the test for the SecurityError
  exception that never passed.

Fixes #15
rakuco pushed a commit to w3c/battery that referenced this pull request Feb 1, 2022
…face. (#51)

In other words, stop exposing this API to insecure origins. Even though this
API is not new, it provides user information that, transmitted insecurely,
can pose a risk to user privacy. See
https://w3ctag.github.io/design-principles/#secure-context for more
information on the guidelines we are trying to follow.

This has been discussed since at least 2016 (see #5). #11 made access from
an insecure origin throw a SecurityError at a time when the
`[SecureContext]` Web IDL extended attribute was not widespread.
Unfortunately, the spec change was not accompanied by a change in the
implementation(s) and, to this day, Blink's implementation (the only
remaining one) continues to expose the API to insecure origins.

Years later, #30 was added in the context of the discussions in #15 where it
was noted that the spec was still manually throwing a SecurityError when
checking if it was called by a secure origin. Besides stopping throwing a
SecurityError (which was never implemented), #30 also recognized the current
situation by noting that the API _should_ use `[SecureContext]` but did not
because it only made sense to do so when the implementation was updated
accordingly.

This change finally adds `[SecureContext]` to the spec's Web IDL because I
am also handling the Blink implementation [1][2]: starting with Chrome 99,
users will be warned that using the Battery Status API in an insecure origin
is deprecated, and starting with Chrome 103 doing so will no longer be
possible.

[1] https://chromestatus.com/feature/4878376799043584
[2] https://groups.google.com/a/chromium.org/g/blink-dev/c/w80tJL8uEV8/m/PfrHlvtgAgAJ

From a testing perspective, there isn't much to be done:
- web-platform-tests/wpt#5871 changed the existing tests to run in HTTPS.
- web-platform-tests/wpt#32556 removed the test for the SecurityError
  exception that never passed.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants