New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 778536 link

Starred by 1 user
Status: Fixed
Owner:
battre@chromium.org
Closed: Nov 2017
Cc:
█ mkwst@chromium.org
engedy@chromium.org
jdoerrie@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Task



Sign in to add a comment

Deprecate FetchCredentialsMode::kPassword

Project Member Reported by toyoshim@chromium.org, Oct 26 2017 
"password" mode is not defined in the Fetch spec, but can be seen in
an example code for the Credential Management Level 1.
https://w3c.github.io/webappsec-credential-management/#passwords

Eventually, we would remove this type from the FetchCredentialsMode.

This is discussed against WebURLRequest::FetchCredentialsMode originally
in https://chromium-review.googlesource.com/c/chromium/src/+/701901.

And the enum type is merged to network::mojom::FetchCredentialsMode.

This bug is requested in the following code review.
https://chromium-review.googlesource.com/c/chromium/src/+/737734/1/services/network/public/interfaces/fetch_api.mojom#33

Comment 1 by toyoshim@chromium.org, Oct 26 2017 

battre, mkwst: I just set SecurityFeature tag tentatively. Could you pick up more specific one from SecurityFeature's sub-categories if you have?

Comment 2 by mkwst@chromium.org, Oct 26 2017

Components: -Blink>SecurityFeature Blink>SecurityFeature>CredentialManagement
Yes, we should remove this, as well as the password-credential codepath from the loader all the way up the stack.

battre@: Do you have a feel for a reasonable timeline here? We originally said M62, which the deprecation messages still point to (we're on M64 now). I'd like to get rid of this code, as it complicates the network stack migrations that are ongoing. :)

Comment 3 by battre@chromium.org, Nov 2 2017 

Note that the example code here https://w3c.github.io/webappsec-credential-management/#passwords is a conventional use of fetch with a form submission, nothing related to the old way the CM API monkey patched fetch.

Before I start working on a CL, can you quickly confirm that the goal is to basically delete all code added here https://bugs.chromium.org/p/chromium/issues/detail?id=599597? I.e. that the goal is not to introduce any kinds of warnings.

Comment 4 by battre@chromium.org, Nov 2 2017

Cc: jdoerrie@chromium.org engedy@chromium.org

Comment 5 by mkwst@chromium.org, Nov 2 2017

Labels: -Pri-3 Pri-2
> Before I start working on a CL, can you quickly confirm that the goal is to
> basically delete all code added here 
> https://bugs.chromium.org/p/chromium/issues/detail?id=599597? I.e. that the
> goal is not to introduce any kinds of warnings.

Correct. We deprecated this API months ago, we said we were going to remove it in M62. We missed that deadline. *shrug* Now seems like a great time to get rid of the code.

Bumping the priority a bit, as this is making the loading team's job a little harder than it should be while they're refactoring things like CORS. It would be lovely to reduce the complexity of the loading stack by removing this code we don't want to support anymore. :)

Comment 6 by battre@chromium.org, Nov 3 2017

Status: Started (was: Assigned)
Sent https://chromium-review.googlesource.com/c/chromium/src/+/752801 your way.
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 3 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/204b0d91b700c2de7a2b3690ae6cb793d36dddf8

commit 204b0d91b700c2de7a2b3690ae6cb793d36dddf8
Author: Dominic Battre <battre@chromium.org>
Date: Fri Nov 03 11:59:13 2017

Remove FetchCredentialsMode::kPassword

Bug:  778536 
Change-Id: I080770330d543a4ec168493bd2e20878e590fd1f
Reviewed-on: https://chromium-review.googlesource.com/752801
Reviewed-by: Dominic Battré <battre@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513748}
[modify] https://crrev.com/204b0d91b700c2de7a2b3690ae6cb793d36dddf8/services/network/public/interfaces/fetch_api.mojom
[delete] https://crrev.com/96b174a1935e8385ff851235c6ab8109e1337119/third_party/WebKit/LayoutTests/http/tests/credentialmanager/passwordcredential-fetch-expected.txt
[delete] https://crrev.com/96b174a1935e8385ff851235c6ab8109e1337119/third_party/WebKit/LayoutTests/http/tests/credentialmanager/passwordcredential-fetch-registrabledomain.html
[delete] https://crrev.com/96b174a1935e8385ff851235c6ab8109e1337119/third_party/WebKit/LayoutTests/http/tests/credentialmanager/passwordcredential-fetch.html
[delete] https://crrev.com/96b174a1935e8385ff851235c6ab8109e1337119/third_party/WebKit/LayoutTests/http/tests/credentialmanager/passwordcredential-redirect.html
[delete] https://crrev.com/96b174a1935e8385ff851235c6ab8109e1337119/third_party/WebKit/LayoutTests/http/tests/credentialmanager/resources/redirect-to-echo-post.php
[modify] https://crrev.com/204b0d91b700c2de7a2b3690ae6cb793d36dddf8/third_party/WebKit/Source/bindings/core/v8/ReferrerScriptInfoTest.cpp
[modify] https://crrev.com/204b0d91b700c2de7a2b3690ae6cb793d36dddf8/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp
[modify] https://crrev.com/204b0d91b700c2de7a2b3690ae6cb793d36dddf8/third_party/WebKit/Source/modules/fetch/FetchManager.cpp
[modify] https://crrev.com/204b0d91b700c2de7a2b3690ae6cb793d36dddf8/third_party/WebKit/Source/modules/fetch/FetchRequestData.cpp
[modify] https://crrev.com/204b0d91b700c2de7a2b3690ae6cb793d36dddf8/third_party/WebKit/Source/modules/fetch/FetchRequestData.h
[modify] https://crrev.com/204b0d91b700c2de7a2b3690ae6cb793d36dddf8/third_party/WebKit/Source/modules/fetch/Request.cpp
[modify] https://crrev.com/204b0d91b700c2de7a2b3690ae6cb793d36dddf8/third_party/WebKit/Source/modules/fetch/Request.h
[modify] https://crrev.com/204b0d91b700c2de7a2b3690ae6cb793d36dddf8/third_party/WebKit/Source/modules/fetch/RequestInit.cpp
[modify] https://crrev.com/204b0d91b700c2de7a2b3690ae6cb793d36dddf8/third_party/WebKit/Source/modules/fetch/RequestInit.h
[modify] https://crrev.com/204b0d91b700c2de7a2b3690ae6cb793d36dddf8/third_party/WebKit/Source/platform/exported/WebCORS.cpp
[modify] https://crrev.com/204b0d91b700c2de7a2b3690ae6cb793d36dddf8/third_party/WebKit/Source/platform/exported/WebCORSPreflightResultCache.cpp
[modify] https://crrev.com/204b0d91b700c2de7a2b3690ae6cb793d36dddf8/third_party/WebKit/Source/platform/exported/WebCORSPreflightResultCacheTest.cpp
[modify] https://crrev.com/204b0d91b700c2de7a2b3690ae6cb793d36dddf8/third_party/WebKit/Source/platform/exported/WebURLRequest.cpp
[modify] https://crrev.com/204b0d91b700c2de7a2b3690ae6cb793d36dddf8/third_party/WebKit/Source/platform/loader/fetch/FetchUtils.h
[modify] https://crrev.com/204b0d91b700c2de7a2b3690ae6cb793d36dddf8/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
[modify] https://crrev.com/204b0d91b700c2de7a2b3690ae6cb793d36dddf8/third_party/WebKit/Source/platform/loader/fetch/ResourceLoader.cpp
[modify] https://crrev.com/204b0d91b700c2de7a2b3690ae6cb793d36dddf8/third_party/WebKit/Source/platform/loader/fetch/ResourceRequest.cpp
[modify] https://crrev.com/204b0d91b700c2de7a2b3690ae6cb793d36dddf8/third_party/WebKit/Source/platform/loader/fetch/ResourceRequest.h

Comment 8 by battre@chromium.org, Nov 3 2017

Status: Fixed (was: Started)
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 23 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/57b2dc23b962feeadd2dcf60a399c9f38d2087e3

commit 57b2dc23b962feeadd2dcf60a399c9f38d2087e3
Author: Balazs Engedy <engedy@chromium.org>
Date: Thu Nov 23 14:23:38 2017

Remove deprecated PasswordCredential attributes.

Remove all code and tests related to the following attributes of
PasswordCredential:
 -- idName
 -- passwordName
 -- additionalData

These had been slated for removal already in M62 along with the removal
of FetchCredentialsMode::kPassword, but accidentally were not.

See intent to deprecate here:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/WPckvg8JO0U

Bug:  778536 ,  718416 
Change-Id: I52513a4dfdde5e075f86f850c0ccafc49382fba2
Reviewed-on: https://chromium-review.googlesource.com/779000
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518928}
[modify] https://crrev.com/57b2dc23b962feeadd2dcf60a399c9f38d2087e3/third_party/WebKit/LayoutTests/http/tests/credentialmanager/credentialscontainer-create-basics.html
[modify] https://crrev.com/57b2dc23b962feeadd2dcf60a399c9f38d2087e3/third_party/WebKit/LayoutTests/http/tests/credentialmanager/credentialscontainer-get-basics.html
[modify] https://crrev.com/57b2dc23b962feeadd2dcf60a399c9f38d2087e3/third_party/WebKit/LayoutTests/http/tests/credentialmanager/passwordcredential-basics.html
[modify] https://crrev.com/57b2dc23b962feeadd2dcf60a399c9f38d2087e3/third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/57b2dc23b962feeadd2dcf60a399c9f38d2087e3/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/57b2dc23b962feeadd2dcf60a399c9f38d2087e3/third_party/WebKit/Source/core/frame/Deprecation.cpp
[delete] https://crrev.com/e15b47cd03542eba57cc447579db5e81452fcc8b/third_party/WebKit/Source/modules/credentialmanager/FormDataOptions.idl
[modify] https://crrev.com/57b2dc23b962feeadd2dcf60a399c9f38d2087e3/third_party/WebKit/Source/modules/credentialmanager/PasswordCredential.cpp
[modify] https://crrev.com/57b2dc23b962feeadd2dcf60a399c9f38d2087e3/third_party/WebKit/Source/modules/credentialmanager/PasswordCredential.h
[modify] https://crrev.com/57b2dc23b962feeadd2dcf60a399c9f38d2087e3/third_party/WebKit/Source/modules/credentialmanager/PasswordCredential.idl
[modify] https://crrev.com/57b2dc23b962feeadd2dcf60a399c9f38d2087e3/third_party/WebKit/Source/modules/credentialmanager/PasswordCredentialTest.cpp
[modify] https://crrev.com/57b2dc23b962feeadd2dcf60a399c9f38d2087e3/third_party/WebKit/Source/modules/modules_idl_files.gni
[modify] https://crrev.com/57b2dc23b962feeadd2dcf60a399c9f38d2087e3/third_party/WebKit/public/platform/web_feature.mojom
[modify] https://crrev.com/57b2dc23b962feeadd2dcf60a399c9f38d2087e3/tools/metrics/histograms/enums.xml
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 23 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5464476eef154dabf38e618a3372857cd8c514c7

commit 5464476eef154dabf38e618a3372857cd8c514c7
Author: Olga Sharonova <olka@chromium.org>
Date: Thu Nov 23 14:52:22 2017

Revert "Remove deprecated PasswordCredential attributes."

This reverts commit 57b2dc23b962feeadd2dcf60a399c9f38d2087e3.

Reason for revert: Broke the tree: https://build.chromium.org/p/chromium/builders/Linux%20x64/builds/53670

Original change's description:
> Remove deprecated PasswordCredential attributes.
> 
> Remove all code and tests related to the following attributes of
> PasswordCredential:
>  -- idName
>  -- passwordName
>  -- additionalData
> 
> These had been slated for removal already in M62 along with the removal
> of FetchCredentialsMode::kPassword, but accidentally were not.
> 
> See intent to deprecate here:
> https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/WPckvg8JO0U
> 
> Bug:  778536 ,  718416 
> Change-Id: I52513a4dfdde5e075f86f850c0ccafc49382fba2
> Reviewed-on: https://chromium-review.googlesource.com/779000
> Commit-Queue: Balazs Engedy <engedy@chromium.org>
> Reviewed-by: Mike West <mkwst@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#518928}

TBR=engedy@chromium.org,mkwst@chromium.org

Change-Id: I9e9ab493d88510e7e56d76205bcbab2e2e142f73
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  778536 ,  718416 
Reviewed-on: https://chromium-review.googlesource.com/788110
Reviewed-by: Olga Sharonova <olka@chromium.org>
Commit-Queue: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518935}
[modify] https://crrev.com/5464476eef154dabf38e618a3372857cd8c514c7/third_party/WebKit/LayoutTests/http/tests/credentialmanager/credentialscontainer-create-basics.html
[modify] https://crrev.com/5464476eef154dabf38e618a3372857cd8c514c7/third_party/WebKit/LayoutTests/http/tests/credentialmanager/credentialscontainer-get-basics.html
[modify] https://crrev.com/5464476eef154dabf38e618a3372857cd8c514c7/third_party/WebKit/LayoutTests/http/tests/credentialmanager/passwordcredential-basics.html
[modify] https://crrev.com/5464476eef154dabf38e618a3372857cd8c514c7/third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/5464476eef154dabf38e618a3372857cd8c514c7/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/5464476eef154dabf38e618a3372857cd8c514c7/third_party/WebKit/Source/core/frame/Deprecation.cpp
[add] https://crrev.com/5464476eef154dabf38e618a3372857cd8c514c7/third_party/WebKit/Source/modules/credentialmanager/FormDataOptions.idl
[modify] https://crrev.com/5464476eef154dabf38e618a3372857cd8c514c7/third_party/WebKit/Source/modules/credentialmanager/PasswordCredential.cpp
[modify] https://crrev.com/5464476eef154dabf38e618a3372857cd8c514c7/third_party/WebKit/Source/modules/credentialmanager/PasswordCredential.h
[modify] https://crrev.com/5464476eef154dabf38e618a3372857cd8c514c7/third_party/WebKit/Source/modules/credentialmanager/PasswordCredential.idl
[modify] https://crrev.com/5464476eef154dabf38e618a3372857cd8c514c7/third_party/WebKit/Source/modules/credentialmanager/PasswordCredentialTest.cpp
[modify] https://crrev.com/5464476eef154dabf38e618a3372857cd8c514c7/third_party/WebKit/Source/modules/modules_idl_files.gni
[modify] https://crrev.com/5464476eef154dabf38e618a3372857cd8c514c7/third_party/WebKit/public/platform/web_feature.mojom
[modify] https://crrev.com/5464476eef154dabf38e618a3372857cd8c514c7/tools/metrics/histograms/enums.xml
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 23 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ac06fb3d908dca2b51573eb56359172bc6d85b1e

commit ac06fb3d908dca2b51573eb56359172bc6d85b1e
Author: Balazs Engedy <engedy@chromium.org>
Date: Thu Nov 23 18:48:07 2017

Reland "Remove deprecated PasswordCredential attributes."

This is a reland of 57b2dc23b962feeadd2dcf60a399c9f38d2087e3
Original change's description:
> Remove deprecated PasswordCredential attributes.
>
> Remove all code and tests related to the following attributes of
> PasswordCredential:
>  -- idName
>  -- passwordName
>  -- additionalData
>
> These had been slated for removal already in M62 along with the removal
> of FetchCredentialsMode::kPassword, but accidentally were not.
>
> See intent to deprecate here:
> https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/WPckvg8JO0U
>
> Bug:  778536 ,  718416 
> Change-Id: I52513a4dfdde5e075f86f850c0ccafc49382fba2
> Reviewed-on: https://chromium-review.googlesource.com/779000
> Commit-Queue: Balazs Engedy <engedy@chromium.org>
> Reviewed-by: Mike West <mkwst@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#518928}

TBR=mkwst@chromium.org

Bug:  778536 ,  718416 
Change-Id: I219f74f758c1a7f10350bc8613cf794575644ece
Reviewed-on: https://chromium-review.googlesource.com/788210
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518994}
[modify] https://crrev.com/ac06fb3d908dca2b51573eb56359172bc6d85b1e/third_party/WebKit/LayoutTests/http/tests/credentialmanager/credentialscontainer-create-basics.html
[modify] https://crrev.com/ac06fb3d908dca2b51573eb56359172bc6d85b1e/third_party/WebKit/LayoutTests/http/tests/credentialmanager/credentialscontainer-get-basics.html
[modify] https://crrev.com/ac06fb3d908dca2b51573eb56359172bc6d85b1e/third_party/WebKit/LayoutTests/http/tests/credentialmanager/passwordcredential-basics.html
[modify] https://crrev.com/ac06fb3d908dca2b51573eb56359172bc6d85b1e/third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/ac06fb3d908dca2b51573eb56359172bc6d85b1e/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/ac06fb3d908dca2b51573eb56359172bc6d85b1e/third_party/WebKit/Source/bindings/modules/v8/generated.gni
[modify] https://crrev.com/ac06fb3d908dca2b51573eb56359172bc6d85b1e/third_party/WebKit/Source/core/frame/Deprecation.cpp
[delete] https://crrev.com/b874ae659f2a39916d59140f5fefb582d20a61ea/third_party/WebKit/Source/modules/credentialmanager/FormDataOptions.idl
[modify] https://crrev.com/ac06fb3d908dca2b51573eb56359172bc6d85b1e/third_party/WebKit/Source/modules/credentialmanager/PasswordCredential.cpp
[modify] https://crrev.com/ac06fb3d908dca2b51573eb56359172bc6d85b1e/third_party/WebKit/Source/modules/credentialmanager/PasswordCredential.h
[modify] https://crrev.com/ac06fb3d908dca2b51573eb56359172bc6d85b1e/third_party/WebKit/Source/modules/credentialmanager/PasswordCredential.idl
[modify] https://crrev.com/ac06fb3d908dca2b51573eb56359172bc6d85b1e/third_party/WebKit/Source/modules/credentialmanager/PasswordCredentialTest.cpp
[modify] https://crrev.com/ac06fb3d908dca2b51573eb56359172bc6d85b1e/third_party/WebKit/Source/modules/modules_idl_files.gni
[modify] https://crrev.com/ac06fb3d908dca2b51573eb56359172bc6d85b1e/third_party/WebKit/public/platform/web_feature.mojom
[modify] https://crrev.com/ac06fb3d908dca2b51573eb56359172bc6d85b1e/tools/metrics/histograms/enums.xml
►

Sign in to add a comment