Closed Bug 1430947 Opened 7 years ago Closed 7 years ago

Navigator.credentials is not [SecureContext]

Categories

(Core :: DOM: Device Interfaces, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: bzbarsky, Assigned: jcj)

References

Details

(Keywords: dev-doc-needed, Whiteboard: [webauthn] [webauthn-interop][credman])

Attachments

(1 file)

STEPS TO REPRODUCE: 1) Load a non-secure page (e.g. <http://example.com/>) 2) Open web console 3) Evaluate "navigator.credentials" EXPECTED RESULTS: Get undefined ACTUAL RESULTS: Get a CredentialsContainer object ANALYSIS: https://www.w3.org/TR/credential-management-1/#framework-credential-management has a [SecureContext] bit that our Navigator.webidl is missing.
Ugh, sorry. On it, thanks!
Assignee: nobody → jjones
OS: Unspecified → All
Priority: -- → P1
Hardware: Unspecified → All
Whiteboard: [webauthn] [webauthn-interop][credman]
Not exactly your fault, esp. given the apparent lack of web platform tests for this...
Comment on attachment 8943096 [details] Bug 1430947 - Add [SecureContext] to navigator.credentials https://reviewboard.mozilla.org/r/213394/#review219116 r=me; needs tests
Attachment #8943096 - Flags: review?(bzbarsky) → review+
Comment on attachment 8943096 [details] Bug 1430947 - Add [SecureContext] to navigator.credentials https://reviewboard.mozilla.org/r/213394/#review219140 ::: dom/credentialmanagement/tests/mochitest/mochitest.ini:8 (Diff revision 2) > frame_credman_iframes.html > scheme = https > skip-if = !e10s > > [test_credman_iframes.html] > +[test_credman_insecure_context.html] Why not a web platform test?
Attachment #8943096 - Flags: review+
Comment on attachment 8943096 [details] Bug 1430947 - Add [SecureContext] to navigator.credentials https://reviewboard.mozilla.org/r/213394/#review219140 > Why not a web platform test? Sorry for the dumb question, but so far [my one commit on that repo hasn't merged yet](https://github.com/w3c/web-platform-tests/pull/7500) -- I assume I write that test externally, make a PR, and then at some point the web platform tests get merged back into m-c?
No, you can just check them directly into m-c; we do bidirectional sync. If you're doing a large tests-only change, it can make sense to go through a PR to the wpt repo and get reviews there, but for small things that go along with code changes it's better to just land them in m-c and then they will get synced back.
Comment on attachment 8943096 [details] Bug 1430947 - Add [SecureContext] to navigator.credentials https://reviewboard.mozilla.org/r/213394/#review219506 ::: commit-message-d97ff:6 (Diff revision 4) > +Bug 1430947 - Add [SecureContext] to navigator.credentials r?bz > + > +It was neglected to mark navigator.credentials as [SecureContext], yet it > +must be for spec compliance and powerful-features compliance. > + > +Updated: add a web platform test You can drop this line; the actual commit landing is not "updated" compared to what's already in the tree. ::: dom/webidl/Navigator.webidl:314 (Diff revision 4) > [NoInterfaceObject, Exposed=(Window,Worker)] > interface NavigatorConcurrentHardware { > readonly attribute unsigned long long hardwareConcurrency; > }; > > partial interface Navigator { Preexisting, but there should really be a link to https://w3c.github.io/webappsec-credential-management/#framework-credential-management both before this partial interface and in the list at the top of this file.
Attachment #8943096 - Flags: review?(bzbarsky) → review+
Comment on attachment 8943096 [details] Bug 1430947 - Add [SecureContext] to navigator.credentials https://reviewboard.mozilla.org/r/213394/#review219506 Thanks again, Boris. Looks like the Try run was good, too. I'll upload the adjustments and get this in the landing pattern.
Pushed by ncsoregi@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9f192d955648 Add [SecureContext] to navigator.credentials r=bz
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: