Assess compatibility risk of using [SecureContext]

If we'd add [SecureContext] to partial interface Navigator and BatteryManager as is the current trend, we could drop the following prose:

If the current settings object is not a secure context, then reject this Navigator object's battery promise with a "SecurityError" DOMException and abort these steps.

However, this would break any feature detection code that relies on checking the existence of properties or objects, e.g.:

if ("getBattery" in navigator && "BatteryManager" in window)
  console.log('All your battery are belong to us');

I guess nearly everyone using this API must do some sort of feature detection.

My hunch is we want to close this issue as won't fix citing backward compatibility concerns.

Thoughts?

This is one of, I believe, two APIs on the platform which do explicit secure context checks instead of using [SecureContext]. This is causing minor integration problems for other features like prerendering.

I think your argument in the OP indicates it'd be better to use [SecureContext]. That is, non-secure HTTP pages would be better off going down the "this browser does not suppport the API" path, and treating it similar to they do Firefox and Safari, instead of calling the API and getting surprised by a rejected promise.

It’s been a while that argument was made so I’ll happily align with the modern convention that does not cause prerendering problems.

To be clear, it's only causing minor problems for prerendering, and we can work around it. I think aligning would be ideal, but as always it's a tradeoff of how much effort it's worth, especially given potential compat risks.

Non-secure HTTP usage is now at ~0.38% per https://www.chromestatus.com/metrics/feature/popularity#BatteryStatusInsecureOrigin

Is that still too high to make this change?

Oh, it looks like Blink doesn't implement the spec's secure context restriction at all, wow. So there are three possible behaviors:

The current implementation: works fine in a non-secure context
The current spec: returns a rejected promise in a non-secure context
The ideal spec: throws an exception (because the method doesn't exist at all because it's guarded with [SecureContext]) in a non-secure context

That's... tricky.

I guess my preference would be to remove the secure context restriction from the spec entirely, add a warning box pointing to this issue, and then as a separate effort, try to move the spec and implementation toward (3), if we think it's worth the engineering effort?

@domenic

The move to HTTPS across the web over the last ~4 years should mitigate the compat risk imposed on legitimate sites I raised in the OP back in 2017.

Given this new data, I'd be supportive of the two-stage plan outlined by @domenic.

@domenic, do you want to amend #30 to align with the implementation and get it all reviewed at once? Can take another PR too.

Can you also get Blink owners sign-off for the plan?

Thanks for your efforts in modernizing this spec!

I've amended #30 in such a way!

Can you also get Blink owners sign-off for the plan?

Probably the best way to do that is with an Intent to Ship; I don't have any special powers there. I suspect they'd like to see some analysis of what type of breakage this would cause, e.g. looking at the sites listed in https://www.chromestatus.com/metrics/feature/timeline/popularity/2199 per HTTPArchive in a [SecureContext] build and seeing if they are visibly broken.

@domenic

@domenic thanks!

Let’s see if I can summon @mounirlamouri to review #30. He might have some thoughts on the ItS.

I support announcing a depreciation schedule for non-SecureContext access to the getBattery() method. In Chromium this would be something like "generate a warning on non-SecureContext usage of the method for N releases and then remove as long as usage does not change in an unexpected way".

@timvolodine

cc @timvolodine as OWNER of third_party/blink/renderer/modules/battery in Blink. I can try to do that myself otherwise.

Latest update: starting with Chromium M99 (or maybe M100, depending on what the API owners there say), the browser will show a deprecation warning whenever the Battery Status API (i.e. navigator.getBattery() is used in an insecure origin). The plan is to stop exposing the API in insecure origins altogether starting with M103 (whose planned release date is around mid-June 2022).

chromestatus.com entry, blink-dev discussion

From a spec and web-platform-tests perspective, I wonder if it makes sense to update things immediately or somehow make the changes more in sync with Blink, which is the only engine implementing this API at the moment. This would mean landing any changes adding [SecureContext] to Web IDL and updating the tests accordingly in a few months rather than now. Opinions?

@rakuco

From a W3C process perspective, we’d land the spec and w-p-t changes as soon as there’s consensus in the WG. We seem to be good on that front: the issue has been open since 2017, discussed at our meetings, and we’ve had a warning in https://www.w3.org/TR/battery-status/#extensions-to-the-navigator-interface for a year with no push back for the proposal.

OTOH, if it is problematic to have Chromium w-p-t show some red until the Blink patches land, we can also consider other options, such as land spec changes now and coordinate w-p-t with Blink. Per our charter, we will follow a test as you commit approarch for CR and above. Recommended, but not mandatory for earlier maturity levels.

In any case, I think we should include a note to the spec about this deprecation.

Thanks for working on this, @rakuco!

From a W3C process perspective, we’d land the spec and w-p-t changes as soon as there’s consensus in the WG.

Yep, that's what I was thinking. I was asking about this because even though the chance is small, there's still the possibility that there is pushback between Chrome 99 and 102 and we are forced to rethink the deprecation and [SecureContext] change in Blink, in which case the spec would remain out of sync with its only implementation for an indefinite period of time.

In that scenario, it is possible to revert the spec and w-p-t changes in this particular case. I hope we don’t need to since we’ve done our homework. But we know from past experience that with deprecations there can be surprises.

With multiple implementations or more complex change/interactions this would be a more involved process.

battery status: Remove battery-insecure-context.html.

battery status: Remove battery-insecure-context.html. (#32556)

Add [SecureContext] to BatteryManager and the Navigator partial inter…

battery status: Remove battery-insecure-context.html. (web-platform-t…

Bug 1752140 [wpt PR 32556] - battery status: Remove battery-insecure-…

battery status: Remove battery-insecure-context.html. (web-platform-t…

Bug 1752140 [wpt PR 32556] - battery status: Remove battery-insecure-…

Navigation Menu

Repository navigation

Assess compatibility risk of using [SecureContext] #15

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Assess compatibility risk of using [SecureContext] #15

Description

Activity

domenic commented on Feb 25, 2021

anssiko commented on Feb 25, 2021

domenic commented on Feb 25, 2021

anssiko commented on Feb 25, 2021

domenic commented on Feb 25, 2021

anssiko commented on Feb 26, 2021

domenic commented on Feb 26, 2021

anssiko commented on Feb 26, 2021

reillyeon commented on Feb 26, 2021

rakuco commented on Mar 1, 2021

rakuco commented on Jan 25, 2022

anssiko commented on Jan 26, 2022

rakuco commented on Jan 26, 2022

anssiko commented on Jan 26, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions