New issue
Advanced search Search tips

Issue 683120 link

Starred by 2 users

Issue metadata

Status: Duplicate
Merged: issue 654990
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug


Sign in to add a comment

Introduce Identity Service

Project Member Reported by blundell@chromium.org, Jan 20 2017

Issue description

This bug covers the task of introducing //services/identity, which will wrap //components/signin and should expose Mojo APIs that consumers of //components/signin can move to to satisfy their use cases.
 
Blocking: 683124

Comment 2 by st...@chromium.org, Feb 8 2017

Cc: st...@chromium.org michae...@chromium.org alemate@chromium.org

Comment 3 by st...@chromium.org, Mar 3 2017

Cc: r...@chromium.org

Comment 4 by st...@chromium.org, Mar 3 2017

Cc: -st...@chromium.org
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 24 2017

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

commit b85bd3b039159890285ab7df4bf280f89cd9980f
Author: blundell <blundell@chromium.org>
Date: Fri Mar 24 13:56:27 2017

Introduce Identity Service and its initial usage

This CL introduces the Identity Service and converts the implementation
of the chrome.identity.GetProfileUserInfo() extension function to use
the new service. Notable points:

- The Identity Service is embedded in //chrome rather than //content,
  as it needs access to per-Profile classes with //chrome-level
  factories.
- The initial API is simply enough to satisfy the needs of the initial
  consumer. We will build out this API by incrementally converting
  further consumers.
- In the long term, the //components/signin classes on which the
  Identity Service depends will be folded into the Identity Service
  implementation, and their consumers will be converted to consume the
  Identity Service.
- The Identity Service makes use of the existing AccountId Mojo struct.
  It extends that struct to be nullable, as the Identity Service needs
  to be able to convey to a requesting client that the user is not
  signed in. Note that a separate HasPrimaryAccount() => bool method
  would not suffice here: since the communication is asynchronous, the
  user could always have signed out by the time that the Identity
  Service receives the GetPrimaryAccountId() call.

BUG= 683120 , 683124
TEST=Install a Chrome extension with the identity and identity.email
permissions in its manifest. Go to chrome://extensions, enable developer mode,
and inspect the background page of the above app. At the JS console that that
brings up, execute:
chrome.identity.getProfileUserInfo((account) => {console.log(account)})
Verify that the printout corresponds to the information of the account
being used for sync.

Review-Url: https://codereview.chromium.org/2753753007
Cr-Commit-Position: refs/heads/master@{#459410}

[modify] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/chrome/app/BUILD.gn
[modify] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/chrome/browser/BUILD.gn
[modify] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/chrome/browser/DEPS
[modify] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/chrome/browser/chrome_content_browser_manifest_overlay.json
[modify] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/chrome/browser/extensions/BUILD.gn
[modify] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/chrome/browser/extensions/api/identity/identity_get_profile_user_info_function.cc
[modify] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/chrome/browser/extensions/api/identity/identity_get_profile_user_info_function.h
[modify] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/chrome/browser/profiles/DEPS
[modify] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/chrome/browser/profiles/profile_impl.cc
[modify] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/chrome/browser/profiles/profile_impl.h
[modify] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/components/signin/public/interfaces/account_id_traits.h
[modify] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/services/device/device_service.cc
[add] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/services/identity/BUILD.gn
[add] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/services/identity/DEPS
[add] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/services/identity/OWNERS
[add] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/services/identity/README.md
[add] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/services/identity/identity_manager.cc
[add] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/services/identity/identity_manager.h
[add] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/services/identity/identity_service.cc
[add] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/services/identity/identity_service.h
[add] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/services/identity/manifest.json
[add] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/services/identity/public/interfaces/BUILD.gn
[add] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/services/identity/public/interfaces/OWNERS
[add] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/services/identity/public/interfaces/constants.mojom
[add] https://crrev.com/b85bd3b039159890285ab7df4bf280f89cd9980f/services/identity/public/interfaces/identity_manager.mojom

Project Member

Comment 6 by bugdroid1@chromium.org, May 3 2017

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

commit 1a4c6321d2ab77267160bfb0297411e43d0daaab
Author: blundell <blundell@chromium.org>
Date: Wed May 03 08:56:11 2017

[Identity Service] Add service tests of IdentityManager

This CL adds service tests to test basic functionality of the implementation
of identity_manager.mojom. Note that if/as service tests of other
interfaces are added, the utility code that sets up the Identity Service
can and should be split out of this test into a base class.

BUG= 683120 

Review-Url: https://codereview.chromium.org/2854083002
Cr-Commit-Position: refs/heads/master@{#468921}

[modify] https://crrev.com/1a4c6321d2ab77267160bfb0297411e43d0daaab/services/BUILD.gn
[modify] https://crrev.com/1a4c6321d2ab77267160bfb0297411e43d0daaab/services/identity/BUILD.gn
[modify] https://crrev.com/1a4c6321d2ab77267160bfb0297411e43d0daaab/services/identity/DEPS
[add] https://crrev.com/1a4c6321d2ab77267160bfb0297411e43d0daaab/services/identity/identity_manager_unittest.cc
[add] https://crrev.com/1a4c6321d2ab77267160bfb0297411e43d0daaab/services/identity/unittest_manifest.json

Project Member

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

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

commit 821f3cd4f25889619c532261e2945cb426f03d4f
Author: blundell <blundell@chromium.org>
Date: Wed May 03 09:08:29 2017

Revert of [Identity Service] Add service tests of IdentityManager (patchset #1 id:1 of https://codereview.chromium.org/2854083002/ )

Reason for revert:
Causing compile failure, looks like race with CL that landed between bot runs and commit of this CL:

https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.linux%2FLinux_Builder%2F83130%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout

Original issue's description:
> [Identity Service] Add service tests of IdentityManager
>
> This CL adds service tests to test basic functionality of the implementation
> of identity_manager.mojom. Note that if/as service tests of other
> interfaces are added, the utility code that sets up the Identity Service
> can and should be split out of this test into a base class.
>
> BUG= 683120 
>
> Review-Url: https://codereview.chromium.org/2854083002
> Cr-Commit-Position: refs/heads/master@{#468921}
> Committed: https://chromium.googlesource.com/chromium/src/+/1a4c6321d2ab77267160bfb0297411e43d0daaab

TBR=rockot@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 683120 

Review-Url: https://codereview.chromium.org/2857103003
Cr-Commit-Position: refs/heads/master@{#468924}

[modify] https://crrev.com/821f3cd4f25889619c532261e2945cb426f03d4f/services/BUILD.gn
[modify] https://crrev.com/821f3cd4f25889619c532261e2945cb426f03d4f/services/identity/BUILD.gn
[modify] https://crrev.com/821f3cd4f25889619c532261e2945cb426f03d4f/services/identity/DEPS
[delete] https://crrev.com/ff4ca2efee2c251b4fded429984f4be90e1d43d5/services/identity/identity_manager_unittest.cc
[delete] https://crrev.com/ff4ca2efee2c251b4fded429984f4be90e1d43d5/services/identity/unittest_manifest.json

Project Member

Comment 8 by bugdroid1@chromium.org, May 3 2017

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

commit 1e21bd3a8b2257bf96283f1ab9a12f09e55b3acf
Author: blundell <blundell@chromium.org>
Date: Wed May 03 15:25:57 2017

[Identity Service] Add service tests of IdentityManager

This CL adds service tests to test basic functionality of the implementation
of identity_manager.mojom. Note that if/as service tests of other
interfaces are added, the utility code that sets up the Identity Service
can and should be split out of this test into a base class.

BUG= 683120 

Review-Url: https://codereview.chromium.org/2854083002
Cr-Original-Commit-Position: refs/heads/master@{#468921}
Committed: https://chromium.googlesource.com/chromium/src/+/1a4c6321d2ab77267160bfb0297411e43d0daaab
Review-Url: https://codereview.chromium.org/2854083002
Cr-Commit-Position: refs/heads/master@{#468980}

[modify] https://crrev.com/1e21bd3a8b2257bf96283f1ab9a12f09e55b3acf/services/BUILD.gn
[modify] https://crrev.com/1e21bd3a8b2257bf96283f1ab9a12f09e55b3acf/services/identity/BUILD.gn
[modify] https://crrev.com/1e21bd3a8b2257bf96283f1ab9a12f09e55b3acf/services/identity/DEPS
[add] https://crrev.com/1e21bd3a8b2257bf96283f1ab9a12f09e55b3acf/services/identity/identity_manager_unittest.cc
[add] https://crrev.com/1e21bd3a8b2257bf96283f1ab9a12f09e55b3acf/services/identity/unittest_manifest.json

Project Member

Comment 9 by bugdroid1@chromium.org, May 22 2017

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

commit cb01102e078c3dbd41aad1ba652f43f6465e9e84
Author: blundell <blundell@chromium.org>
Date: Mon May 22 09:53:17 2017

[ARC] Add browser test of ArcAuthService

In preparation for changing ArcAuthService to consume signin information
via the Identity Service, this CL adds a browsertest of ArcAuthService.

The initial test tests the flow for a user (non-managed) account when
background auth is performed. This is the flow that is the most
significant consumer of the Identity Service in the ARC auth code.
Specifically, we test that if Chrome is seeded with information for a given
account via the signin code, then when AuthInstance connects to ArcAuthService
and requests account info, it receives account info corresponding to the
seeded information.

Currently, we skip the merge session part of the auth flow in this test.
The merge session is difficult to interact with in a testing context, and
even with this flow skipped, the interaction between the ARC auth code
and //components/signin is basically fully exercised by the test.

In the future, we will aim to add tests of the following cases
(some of which might be easy, and others quite difficult):
- Error cases for background auth
- Merge session
- Manual authentication
- Robot accounts
- Managed accounts

BUG= 683120 

Review-Url: https://codereview.chromium.org/2864433003
Cr-Commit-Position: refs/heads/master@{#473535}

[modify] https://crrev.com/cb01102e078c3dbd41aad1ba652f43f6465e9e84/chrome/browser/chromeos/arc/arc_auth_context.cc
[modify] https://crrev.com/cb01102e078c3dbd41aad1ba652f43f6465e9e84/chrome/browser/chromeos/arc/arc_auth_context.h
[modify] https://crrev.com/cb01102e078c3dbd41aad1ba652f43f6465e9e84/chrome/browser/chromeos/arc/arc_auth_service.cc
[modify] https://crrev.com/cb01102e078c3dbd41aad1ba652f43f6465e9e84/chrome/browser/chromeos/arc/arc_auth_service.h
[add] https://crrev.com/cb01102e078c3dbd41aad1ba652f43f6465e9e84/chrome/browser/chromeos/arc/auth/arc_auth_service_browsertest.cc
[modify] https://crrev.com/cb01102e078c3dbd41aad1ba652f43f6465e9e84/chrome/browser/chromeos/arc/auth/arc_background_auth_code_fetcher.cc
[modify] https://crrev.com/cb01102e078c3dbd41aad1ba652f43f6465e9e84/chrome/browser/chromeos/arc/auth/arc_background_auth_code_fetcher.h
[modify] https://crrev.com/cb01102e078c3dbd41aad1ba652f43f6465e9e84/chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc
[modify] https://crrev.com/cb01102e078c3dbd41aad1ba652f43f6465e9e84/chrome/browser/chromeos/login/users/fake_chrome_user_manager.h
[modify] https://crrev.com/cb01102e078c3dbd41aad1ba652f43f6465e9e84/chrome/test/BUILD.gn

Labels: IdentityService
Status: Started (was: Assigned)
Blockedon: 729591
Blockedon: 729964
Blockedon: 729965
Project Member

Comment 14 by bugdroid1@chromium.org, Jul 4 2017

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

commit 063cccab4b5c54f467e1fc3998ff9550522cfa38
Author: Colin Blundell <blundell@chromium.org>
Date: Tue Jul 04 16:00:11 2017

[Identity Service] Add user manual

This CL augments //services/identity/README.md to be an in-progress
user manual that shows how to convert from using existing signin-related
interfaces to using the Identity Service for the same purposes. This
user manual will be extended over time as we continue to build out the
API surface of the Identity Service.

Bug:  683120 
Change-Id: I2454543e2805bc48b954d56504eb3052e150aee2
Reviewed-on: https://chromium-review.googlesource.com/558930
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484121}
[modify] https://crrev.com/063cccab4b5c54f467e1fc3998ff9550522cfa38/services/identity/README.md

Blockedon: 740117
Components: Services>SignIn
Components: Internals>Services>Identity
Mergedinto: 654990
Status: Duplicate (was: Started)
//services/identity is now introduced. Duping this bug against the top-level Identity Service bug so that the blockers here (which are really on the *completion* of the Identity Service) don't get lost.
Project Member

Comment 19 by bugdroid1@chromium.org, Dec 8 2017

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

commit e3a4e201f013213ea9c61a45e7f9967070a5c91f
Author: Colin Blundell <blundell@chromium.org>
Date: Fri Dec 08 11:32:46 2017

Embed the Identity Service in //ios/chrome's chrome_browser service

This CL embeds the Identity Service in //ios/chrome's chrome_browser
service in preparation for upcoming work that will connect to the
Identity Service on iOS. The embedding follows that done for //chrome.

As this is the first service embedded in //ios/chrome, there is some
setup work with manifest overlays done in this CL as well.

Finally, this CL adds an EarlGrey test that tests that the connection to
the Identity Service is set up. This test can and should be extended to
test setup of other services as those are embedded in //ios/chrome.

Bug: 705982,  683120 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: If03f4215b4e7d008eb1ecb22ceb7565e72f0b891
Reviewed-on: https://chromium-review.googlesource.com/776797
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522761}
[modify] https://crrev.com/e3a4e201f013213ea9c61a45e7f9967070a5c91f/ios/chrome/app/BUILD.gn
[modify] https://crrev.com/e3a4e201f013213ea9c61a45e7f9967070a5c91f/ios/chrome/app/resources/BUILD.gn
[modify] https://crrev.com/e3a4e201f013213ea9c61a45e7f9967070a5c91f/ios/chrome/app/resources/ios_resources.grd
[modify] https://crrev.com/e3a4e201f013213ea9c61a45e7f9967070a5c91f/ios/chrome/browser/DEPS
[modify] https://crrev.com/e3a4e201f013213ea9c61a45e7f9967070a5c91f/ios/chrome/browser/OWNERS
[modify] https://crrev.com/e3a4e201f013213ea9c61a45e7f9967070a5c91f/ios/chrome/browser/browser_state/BUILD.gn
[add] https://crrev.com/e3a4e201f013213ea9c61a45e7f9967070a5c91f/ios/chrome/browser/browser_state/browser_state_services_egtest.mm
[modify] https://crrev.com/e3a4e201f013213ea9c61a45e7f9967070a5c91f/ios/chrome/browser/browser_state/chrome_browser_state_impl.cc
[modify] https://crrev.com/e3a4e201f013213ea9c61a45e7f9967070a5c91f/ios/chrome/browser/browser_state/chrome_browser_state_impl.h
[add] https://crrev.com/e3a4e201f013213ea9c61a45e7f9967070a5c91f/ios/chrome/browser/chrome_browser_manifest_overlay.json
[modify] https://crrev.com/e3a4e201f013213ea9c61a45e7f9967070a5c91f/ios/chrome/browser/signin/BUILD.gn
[add] https://crrev.com/e3a4e201f013213ea9c61a45e7f9967070a5c91f/ios/chrome/browser/signin/DEPS
[add] https://crrev.com/e3a4e201f013213ea9c61a45e7f9967070a5c91f/ios/chrome/browser/signin/identity_service_creator.cc
[add] https://crrev.com/e3a4e201f013213ea9c61a45e7f9967070a5c91f/ios/chrome/browser/signin/identity_service_creator.h
[modify] https://crrev.com/e3a4e201f013213ea9c61a45e7f9967070a5c91f/ios/chrome/browser/web/BUILD.gn
[modify] https://crrev.com/e3a4e201f013213ea9c61a45e7f9967070a5c91f/ios/chrome/browser/web/chrome_web_client.h
[modify] https://crrev.com/e3a4e201f013213ea9c61a45e7f9967070a5c91f/ios/chrome/browser/web/chrome_web_client.mm
[modify] https://crrev.com/e3a4e201f013213ea9c61a45e7f9967070a5c91f/ios/chrome/test/earl_grey/BUILD.gn

Sign in to add a comment