Commit d00f8d8f authored by Findit's avatar Findit

Revert "Better success detection for pw change"

This reverts commit 3ea87442.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 688173 as the
culprit for failures in the build cycles as shown on:
https://analysis.chromium.org/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzNlYTg3NDQyYmY1MTk4Mzc1ZWQzODU2MmEwYTZhMTEzOTI0M2QwNjIM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.memory/Mac%20ASan%2064%20Tests%20%281%29/55899

Sample Failed Step: browser_tests

Original change's description:
> Better success detection for pw change
> 
> Only slightly improves success detection at this stage,
> by adding Azure support - Ping and Okta are still pretty
> lacking - but makes room in the code for better success
> detection, and adds a unit test.
> 
> Bug: 930109
> Change-Id: I59a173095301f7c641031f79cb3a6c946c5efc09
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1754013
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Reviewed-by: Lutz Justen <ljusten@chromium.org>
> Commit-Queue: A Olsen <olsen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#688173}


Change-Id: I2e20b20503381c181ed94b133a4099c70e5104fa
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 930109
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1761344
Cr-Commit-Position: refs/heads/master@{#688303}
parent ab7e4e69
...@@ -3,7 +3,6 @@ ...@@ -3,7 +3,6 @@
# found in the LICENSE file. # found in the LICENSE file.
import("//chrome/common/features.gni") import("//chrome/common/features.gni")
import("//chrome/test/base/js2gtest.gni")
import("//tools/grit/grit_rule.gni") import("//tools/grit/grit_rule.gni")
assert(!is_ios, "Chromium/iOS shouldn't use anything in //chrome") assert(!is_ios, "Chromium/iOS shouldn't use anything in //chrome")
...@@ -328,28 +327,3 @@ if (enable_webui_tab_strip) { ...@@ -328,28 +327,3 @@ if (enable_webui_tab_strip) {
] ]
} }
} }
js2gtest("resources_unitjs_tests") {
test_type = "webui"
sources = [
"gaia_auth_host/password_change_authenticator_test.unitjs",
]
# This has to be a gen_include, so it doesn't collide with other js2gtests
gen_include_files = [ "//ui/webui/resources/js/cr.js" ]
# But these have to be extra_js_files, since it uses a native object
# EventTarget, which doesn't work at compile time.
extra_js_files = [
"//ui/webui/resources/js/cr/event_target.js",
"gaia_auth_host/password_change_authenticator.js",
]
defines = [ "HAS_OUT_OF_PROC_TEST_RUNNER" ]
}
source_set("browser_tests") {
testonly = true
deps = [
":resources_unitjs_tests",
]
}
...@@ -14,54 +14,6 @@ cr.define('cr.samlPasswordChange', function() { ...@@ -14,54 +14,6 @@ cr.define('cr.samlPasswordChange', function() {
const BLANK_PAGE_URL = 'about:blank'; const BLANK_PAGE_URL = 'about:blank';
/**
* @param {string?} str A string that should be a valid URL.
* @return {URL?} A valid URL object, or null.
*/
function safeParseUrl_(str) {
try {
return new URL(str);
} catch (error) {
console.error('Invalid url: ' + str);
return null;
}
}
/**
* @param {Object} details The web-request details.
* @return {boolean} True if we detect that a password change was successful.
*/
function detectPasswordChangeSuccess(details) {
const url = safeParseUrl_(details.url);
if (!url) {
return false;
}
// These heuristics work for the following SAML IdPs:
// ADFS:
if (url.pathname.match(/\/updatepassword\/?$/)) {
return url.searchParams.get('status') == '0';
}
// Azure:
if (url.pathname.endsWith('/ChangePassword.aspx')) {
return url.searchParams.get('ReturnCode') == '0';
}
// Okta:
if (url.pathname.match(/\/internal_login\/password\/?$/)) {
// TODO(https://crbug.com/930109): This assumes all Okta password change
// attempts succeed. To check properly, we need to inspect the response.
return details.method == 'POST';
}
// Ping:
if (url.pathname.includes('/password/chg/')) {
// TODO(https://crbug.com/930109): This assumes all Ping password change
// attempts succeed. To check properly, we need to inspect the response.
return details.method == 'POST';
}
return false;
}
/** /**
* Initializes the authenticator component. * Initializes the authenticator component.
*/ */
...@@ -115,9 +67,8 @@ cr.define('cr.samlPasswordChange', function() { ...@@ -115,9 +67,8 @@ cr.define('cr.samlPasswordChange', function() {
this.samlHandler_, 'authPageLoaded', this.samlHandler_, 'authPageLoaded',
this.onAuthPageLoaded_.bind(this)); this.onAuthPageLoaded_.bind(this));
this.webviewEventManager_.addWebRequestEventListener( this.webviewEventManager_.addEventListener(
this.webview_.request.onCompleted, this.onCompleted_.bind(this), this.webview_, 'contentload', this.onContentLoad_.bind(this));
{urls: ['http://*/*', 'https://*/*'], types: ['main_frame']});
} }
/** /**
...@@ -200,19 +151,17 @@ cr.define('cr.samlPasswordChange', function() { ...@@ -200,19 +151,17 @@ cr.define('cr.samlPasswordChange', function() {
} }
/** /**
* Invoked when a new document loading completes. * Invoked when a new document is loaded.
* @param {Object} details The web-request details.
* @private * @private
*/ */
onCompleted_(details) { onContentLoad_(e) {
if (passwordChangeSuccessDetected(details)) { const currentUrl = this.webview_.src;
// TODO(rsorokin): Implement more robust check.
if (currentUrl.lastIndexOf('status=0') != -1) {
this.completeAuth_(); this.completeAuth_();
} }
} }
} }
return { return {Authenticator: Authenticator};
Authenticator: Authenticator,
detectPasswordChangeSuccess: detectPasswordChangeSuccess,
};
}); });
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
GEN_INCLUDE(['//ui/webui/resources/js/cr.js']);
const EXAMPLE_ADFS_ENDPOINT =
'https://example.com/adfs/portal/updatepassword/';
const EXAMPLE_AZURE_ENDPOINT =
'https://example.windowsazure.com/ChangePassword.aspx';
const EXAMPLE_OKTA_ENDPOINT =
'https://example.okta.com/user/profile/internal_login/password';
const EXAMPLE_PING_ENDPOINT =
'https://login.pingone.com/idp/directory/a/12345/password/chg/67890';
PasswordChangeAuthenticatorUnitTest = class extends testing.Test {
get browsePreload() {
return DUMMY_URL;
}
// No need to run these checks - see comment in SamlPasswordAttributesTest.
get runAccessibilityChecks() {
return false;
}
get extraLibraries() {
return [
'//ui/webui/resources/js/cr/event_target.js',
'password_change_authenticator.js',
];
}
assertSuccess(input) {
assertTrue(this.detectSuccess(input));
}
assertNotSuccess(input) {
assertFalse(this.detectSuccess(input));
}
detectSuccess(input) {
let details;
if (typeof input == 'string') {
details = {'url': input};
} else {
details = input;
}
return cr.samlPasswordChange.detectPasswordChangeSuccess(details);
}
}
TEST_F('PasswordChangeAuthenticatorUnitTest', 'DetectAdfsSuccess', function() {
const endpointUrl = EXAMPLE_ADFS_ENDPOINT;
this.assertNotSuccess(endpointUrl);
this.assertNotSuccess(endpointUrl + '?status=1');
this.assertNotSuccess(EXAMPLE_AZURE_ENDPOINT + '?status=0');
this.assertSuccess(endpointUrl + '?status=0');
});
TEST_F('PasswordChangeAuthenticatorUnitTest', 'DetectAzureSuccess', function() {
const endpointUrl = EXAMPLE_AZURE_ENDPOINT;
const extraParam = 'BrandContextID=O123';
this.assertNotSuccess(endpointUrl);
this.assertNotSuccess(endpointUrl + '?' + extraParam);
this.assertNotSuccess(endpointUrl + '?ReturnCode=1&' + extraParam);
this.assertNotSuccess(endpointUrl + '?' + extraParam + '&ReturnCode=1');
this.assertNotSuccess(EXAMPLE_PING_ENDPOINT + '?ReturnCode=0');
this.assertSuccess(endpointUrl + '?ReturnCode=0&' + extraParam);
this.assertSuccess(endpointUrl + '?' + extraParam + '&ReturnCode=0');
});
TEST_F('PasswordChangeAuthenticatorUnitTest', 'DetectOktaSuccess', function() {
const endpointUrl = EXAMPLE_OKTA_ENDPOINT
let details = {url: endpointUrl, method: 'GET'};
this.assertNotSuccess(details);
details = {url: EXAMPLE_ADFS_ENDPOINT, method: 'POST'};
this.assertNotSuccess(details);
details = {url: endpointUrl, method: 'POST'};
this.assertSuccess(details);
});
TEST_F('PasswordChangeAuthenticatorUnitTest', 'DetectPingSuccess', function() {
const endpointUrl = EXAMPLE_PING_ENDPOINT;
let details = {url: endpointUrl, method: 'GET'};
this.assertNotSuccess(details);
details = {url: EXAMPLE_AZURE_ENDPOINT, method: 'POST'};
this.assertNotSuccess(details);
details = {url: endpointUrl, method: 'POST'};
this.assertSuccess(details);
});
...@@ -643,7 +643,6 @@ if (!is_android) { ...@@ -643,7 +643,6 @@ if (!is_android) {
"//chrome/browser", "//chrome/browser",
"//chrome/browser/devtools:test_support", "//chrome/browser/devtools:test_support",
"//chrome/browser/profiling_host:profiling_browsertests", "//chrome/browser/profiling_host:profiling_browsertests",
"//chrome/browser/resources:browser_tests",
"//chrome/browser/web_applications:browser_tests", "//chrome/browser/web_applications:browser_tests",
"//chrome/browser/web_applications/extensions:browser_tests", "//chrome/browser/web_applications/extensions:browser_tests",
"//chrome/renderer", "//chrome/renderer",
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment