Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(83)

Issue 22611008: Remove third party auth redirect URI domain check. (Closed)

Created:
7 years, 4 months ago by rmsousa
Modified:
7 years, 4 months ago
Reviewers:
Wez
CC:
chromium-reviews, jamiewalch+watch_chromium.org, dcaiafa+watch_chromium.org, hclam+watch_chromium.org, wez+watch_chromium.org, amit, sanjeevr, garykac+watch_chromium.org, lambroslambrou+watch_chromium.org, rmsousa+watch_chromium.org, weitaosu+watch_chromium.org, alexeypa+watch_chromium.org, sergeyu+watch_chromium.org
Visibility:
Public.

Description

Remove third party auth redirect URI domain check. Talkgadget can redirect the user to other subdomains, and our content script won't run over the intermediate 302 pages, only the final URL - so we can't assume that the actual redirect URI we'll receive will match the one we asked for. Note that there is still an implicit restriction that this must be a talkgadget subdomain (since that's our content script's url glob). BUG= Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=216869

Patch Set 1 #

Total comments: 2

Patch Set 2 : Deal better with malformed urls #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -5 lines) Patch
M remoting/webapp/third_party_token_fetcher.js View 1 2 chunks +4 lines, -5 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
rmsousa
7 years, 4 months ago (2013-08-09 00:22:27 UTC) #1
Wez
https://codereview.chromium.org/22611008/diff/1/remoting/webapp/third_party_token_fetcher.js File remoting/webapp/third_party_token_fetcher.js (right): https://codereview.chromium.org/22611008/diff/1/remoting/webapp/third_party_token_fetcher.js#newcode91 remoting/webapp/third_party_token_fetcher.js:91: var query = responseUrl.substring(responseUrl.search('#') + 1); Old code coped ...
7 years, 4 months ago (2013-08-09 00:35:28 UTC) #2
rmsousa
https://codereview.chromium.org/22611008/diff/1/remoting/webapp/third_party_token_fetcher.js File remoting/webapp/third_party_token_fetcher.js (right): https://codereview.chromium.org/22611008/diff/1/remoting/webapp/third_party_token_fetcher.js#newcode91 remoting/webapp/third_party_token_fetcher.js:91: var query = responseUrl.substring(responseUrl.search('#') + 1); On 2013/08/09 00:35:28, ...
7 years, 4 months ago (2013-08-09 01:43:59 UTC) #3
Wez
lgtm
7 years, 4 months ago (2013-08-09 05:24:18 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rmsousa@chromium.org/22611008/7001
7 years, 4 months ago (2013-08-10 02:00:00 UTC) #5
commit-bot: I haz the power
7 years, 4 months ago (2013-08-10 15:30:11 UTC) #6
Message was sent while issue was closed.
Change committed as 216869

Powered by Google App Engine
This is Rietveld 408576698