New issue
Advanced search Search tips

Issue 780736 link

Starred by 1 user
Status: Fixed
Owner:
yuweih@chromium.org
Closed: Mar 2018
Cc:
ios-bugs@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android , iOS
Pri: 3
Type: Bug


Participants' hotlists:
CRD-iOS-backlog


Sign in to add a comment

[Remoting Mobile] Need centralized OAuthTokenGetter

Project Member Reported by yuweih@chromium.org, Nov 2 2017 
Currently in C++ code, only the telemetry logger needs to fetch auth token. The telemetry logger will directly call ChromotingClientRuntime::Delegate::RequestAuthTokenForLogger() when it needs auth token, and it expects the the delegate to call logger->SetAuthToken() when the token is fetch.

When using TURN for relay, we will need to make an authenticated request to get back a list of relay servers. To make things simpler, we should refactor the existing auth token code into a centralized OAuthTokenGetter so that it can be used by both the logger and the transport. This could also help if we decide to unify the code to fetch host list in C++.

Comment 1 by huangml@chromium.org, Nov 2 2017

Components: Services>SignInSSO

Comment 2 by yuweih@chromium.org, Nov 2 2017

Components: Services>Chromoting
Ah... I just forgot to add the Chromoting component. This will only affect Chromoting mobile clients and doesn't affect Chrome. Do you think the SignInSSO component is still relevant?

Comment 3 by yuweih@chromium.org, Nov 3 2017

Blocking: 748710
Project Member

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

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

commit 19220930d6b49b82ac97d9b08e4719b0d6c4a342
Author: Yuwei Huang <yuweih@chromium.org>
Date: Thu Nov 09 00:06:08 2017

[Remoting Mobile] Centralized OAuth Token Getter

This CL:
* Provides an interface in ChromotingClientRuntime to allow mobile client
  to fetch OAuth token from a centralized place.
* Provides iOS implementation for the OAuthTokenGetter.
* Passes the token getter to the transport context so that it can be used
  to fetch the ice config.

The centralized OAuthTokenGetter will be used for WebRTC protocol and
will also be used for telemetry logging. I'll send a follow up CL to
implement the OAuthTokenGetter for Android and refactor the auth logic
in telemetry logger.

Bug:  780736 
Change-Id: I91b6f3dac8271e27b18854daddd52637a0a66add
Reviewed-on: https://chromium-review.googlesource.com/754410
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515020}
[modify] https://crrev.com/19220930d6b49b82ac97d9b08e4719b0d6c4a342/remoting/client/BUILD.gn
[modify] https://crrev.com/19220930d6b49b82ac97d9b08e4719b0d6c4a342/remoting/client/chromoting_client_runtime.cc
[modify] https://crrev.com/19220930d6b49b82ac97d9b08e4719b0d6c4a342/remoting/client/chromoting_client_runtime.h
[modify] https://crrev.com/19220930d6b49b82ac97d9b08e4719b0d6c4a342/remoting/client/chromoting_session.cc
[modify] https://crrev.com/19220930d6b49b82ac97d9b08e4719b0d6c4a342/remoting/client/jni/jni_runtime_delegate.cc
[modify] https://crrev.com/19220930d6b49b82ac97d9b08e4719b0d6c4a342/remoting/client/jni/jni_runtime_delegate.h
[add] https://crrev.com/19220930d6b49b82ac97d9b08e4719b0d6c4a342/remoting/client/oauth_token_getter_proxy.cc
[add] https://crrev.com/19220930d6b49b82ac97d9b08e4719b0d6c4a342/remoting/client/oauth_token_getter_proxy.h
[add] https://crrev.com/19220930d6b49b82ac97d9b08e4719b0d6c4a342/remoting/client/oauth_token_getter_proxy_unittest.cc
[modify] https://crrev.com/19220930d6b49b82ac97d9b08e4719b0d6c4a342/remoting/ios/facade/BUILD.gn
[modify] https://crrev.com/19220930d6b49b82ac97d9b08e4719b0d6c4a342/remoting/ios/facade/ios_client_runtime_delegate.h
[modify] https://crrev.com/19220930d6b49b82ac97d9b08e4719b0d6c4a342/remoting/ios/facade/ios_client_runtime_delegate.mm
[add] https://crrev.com/19220930d6b49b82ac97d9b08e4719b0d6c4a342/remoting/ios/facade/ios_oauth_token_getter.h
[add] https://crrev.com/19220930d6b49b82ac97d9b08e4719b0d6c4a342/remoting/ios/facade/ios_oauth_token_getter.mm

Comment 5 by yuweih@chromium.org, Nov 9 2017

Components: -Services>SignInSSO

Comment 6 by yuweih@chromium.org, Nov 9 2017

Status: Started (was: Assigned)
Project Member

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

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

commit 29849a47bb8ef321b1c81ce71195fcca78032193
Author: Yuwei Huang <yuweih@chromium.org>
Date: Fri Nov 10 06:18:02 2017

[CRD Android] Android JNI OAuthTokenGetter implementation

This CL implements OAuthTokenGetter for Android. This is part of the
project of implementing a centralized access token getter in client C++
code, so that multiple components like telemetry logger, ICE config
fetcher, and session connector can use the same token fetcher without
requesting access token back and forth through JNI.

Bug:  780736 
Change-Id: I265fe963ef6f9e1720b3a7e3f209704be8686957
Reviewed-on: https://chromium-review.googlesource.com/760184
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515467}
[modify] https://crrev.com/29849a47bb8ef321b1c81ce71195fcca78032193/remoting/android/BUILD.gn
[modify] https://crrev.com/29849a47bb8ef321b1c81ce71195fcca78032193/remoting/android/client_java_tmpl.gni
[modify] https://crrev.com/29849a47bb8ef321b1c81ce71195fcca78032193/remoting/android/java/src/org/chromium/chromoting/Chromoting.java
[modify] https://crrev.com/29849a47bb8ef321b1c81ce71195fcca78032193/remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java
[add] https://crrev.com/29849a47bb8ef321b1c81ce71195fcca78032193/remoting/android/java/src/org/chromium/chromoting/jni/JniOAuthTokenGetter.java
[modify] https://crrev.com/29849a47bb8ef321b1c81ce71195fcca78032193/remoting/client/jni/BUILD.gn
[add] https://crrev.com/29849a47bb8ef321b1c81ce71195fcca78032193/remoting/client/jni/jni_oauth_token_getter.cc
[add] https://crrev.com/29849a47bb8ef321b1c81ce71195fcca78032193/remoting/client/jni/jni_oauth_token_getter.h
[modify] https://crrev.com/29849a47bb8ef321b1c81ce71195fcca78032193/remoting/client/jni/jni_runtime_delegate.cc
[modify] https://crrev.com/29849a47bb8ef321b1c81ce71195fcca78032193/remoting/client/jni/jni_runtime_delegate.h

Comment 8 by yuweih@chromium.org, Nov 10 2017

Blocking: -748710

Comment 9 by yuweih@chromium.org, Nov 10 2017 

This bug will now be used to track refactoring works to make existing components use the new token getter.
Project Member

Comment 10 by bugdroid1@chromium.org, Mar 21 2018 

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

commit 50afc8383bf459f7a673d9292b605fe66ef3e876
Author: Yuwei Huang <yuweih@chromium.org>
Date: Wed Mar 21 22:00:26 2018

[Remoting Mobile] Refactor TelemetryLogWriter to use centralized token getter

This CL refactors TelemetryLogWriter to use the centralized token getter from
ChromotingClientRuntime. This change gets rid of special handling logic for
authorizing the log writer.

Change-Id: I9ce3481408759b20546801b1f96294f7223b3076
BUG:  780736 
Reviewed-on: https://chromium-review.googlesource.com/967520
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544861}
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/android/java/src/org/chromium/chromoting/Chromoting.java
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/base/chromoting_event_log_writer.h
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/base/oauth_token_getter.h
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/base/telemetry_log_writer.cc
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/base/telemetry_log_writer.h
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/base/telemetry_log_writer_unittest.cc
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/client/chromoting_client_runtime.cc
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/client/chromoting_client_runtime.h
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/client/chromoting_session.cc
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/client/client_telemetry_logger_unittest.cc
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/client/jni/jni_oauth_token_getter.cc
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/client/jni/jni_oauth_token_getter.h
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/client/jni/jni_runtime_delegate.cc
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/client/jni/jni_runtime_delegate.h
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/client/oauth_token_getter_proxy.cc
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/client/oauth_token_getter_proxy.h
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/client/oauth_token_getter_proxy_unittest.cc
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/ios/facade/ios_client_runtime_delegate.h
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/ios/facade/ios_client_runtime_delegate.mm
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/ios/facade/ios_oauth_token_getter.h
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/ios/facade/ios_oauth_token_getter.mm
[modify] https://crrev.com/50afc8383bf459f7a673d9292b605fe66ef3e876/remoting/ios/facade/remoting_service.mm

Comment 11 by yuweih@chromium.org, Mar 22 2018

Status: Fixed (was: Started)
►

Sign in to add a comment