Skip to content

Commit e7ed827

Browse files
author
Carlos Valdivia
committed
Permissions: GET_ACCOUNTS permission cleanup
First, getAccounts*() will now return all available accounts depending on both GET_ACCOUNTS grants and signature matching. This is different from before where a caller of getAccounts() would need GET_ACCOUNTS to get any accounts, but if that same caller called getAccountsByType, they might have gotten back accounts if they shared a signature with the same developer. Second, cleaned up some NPEs and javadoc. This change was motivated by progress on the cts tests. Change-Id: I2f36226780e074fdf58214b46de3b79d8319ace1
1 parent a7b4d6d commit e7ed827

3 files changed

Lines changed: 199 additions & 127 deletions

File tree

core/java/android/accounts/AbstractAccountAuthenticator.java

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,9 @@ public void addAccount(IAccountAuthenticatorResponse response, String accountTyp
138138
new AccountAuthenticatorResponse(response),
139139
accountType, authTokenType, features, options);
140140
if (Log.isLoggable(TAG, Log.VERBOSE)) {
141-
result.keySet(); // force it to be unparcelled
141+
if (result != null) {
142+
result.keySet(); // force it to be unparcelled
143+
}
142144
Log.v(TAG, "addAccount: result " + AccountManager.sanitizeResult(result));
143145
}
144146
if (result != null) {
@@ -160,7 +162,9 @@ public void confirmCredentials(IAccountAuthenticatorResponse response,
160162
final Bundle result = AbstractAccountAuthenticator.this.confirmCredentials(
161163
new AccountAuthenticatorResponse(response), account, options);
162164
if (Log.isLoggable(TAG, Log.VERBOSE)) {
163-
result.keySet(); // force it to be unparcelled
165+
if (result != null) {
166+
result.keySet(); // force it to be unparcelled
167+
}
164168
Log.v(TAG, "confirmCredentials: result "
165169
+ AccountManager.sanitizeResult(result));
166170
}
@@ -185,7 +189,9 @@ public void getAuthTokenLabel(IAccountAuthenticatorResponse response,
185189
result.putString(AccountManager.KEY_AUTH_TOKEN_LABEL,
186190
AbstractAccountAuthenticator.this.getAuthTokenLabel(authTokenType));
187191
if (Log.isLoggable(TAG, Log.VERBOSE)) {
188-
result.keySet(); // force it to be unparcelled
192+
if (result != null) {
193+
result.keySet(); // force it to be unparcelled
194+
}
189195
Log.v(TAG, "getAuthTokenLabel: result "
190196
+ AccountManager.sanitizeResult(result));
191197
}
@@ -209,7 +215,9 @@ public void getAuthToken(IAccountAuthenticatorResponse response,
209215
new AccountAuthenticatorResponse(response), account,
210216
authTokenType, loginOptions);
211217
if (Log.isLoggable(TAG, Log.VERBOSE)) {
212-
result.keySet(); // force it to be unparcelled
218+
if (result != null) {
219+
result.keySet(); // force it to be unparcelled
220+
}
213221
Log.v(TAG, "getAuthToken: result " + AccountManager.sanitizeResult(result));
214222
}
215223
if (result != null) {
@@ -234,7 +242,10 @@ public void updateCredentials(IAccountAuthenticatorResponse response, Account ac
234242
new AccountAuthenticatorResponse(response), account,
235243
authTokenType, loginOptions);
236244
if (Log.isLoggable(TAG, Log.VERBOSE)) {
237-
result.keySet(); // force it to be unparcelled
245+
// Result may be null.
246+
if (result != null) {
247+
result.keySet(); // force it to be unparcelled
248+
}
238249
Log.v(TAG, "updateCredentials: result "
239250
+ AccountManager.sanitizeResult(result));
240251
}
@@ -490,7 +501,7 @@ public abstract Bundle getAuthToken(AccountAuthenticatorResponse response,
490501
* <ul>
491502
* <li> {@link AccountManager#KEY_INTENT}, or
492503
* <li> {@link AccountManager#KEY_ACCOUNT_NAME} and {@link AccountManager#KEY_ACCOUNT_TYPE} of
493-
* the account that was added, or
504+
* the account whose credentials were updated, or
494505
* <li> {@link AccountManager#KEY_ERROR_CODE} and {@link AccountManager#KEY_ERROR_MESSAGE} to
495506
* indicate an error
496507
* </ul>

core/java/android/accounts/AccountManager.java

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ public String getPassword(final Account account) {
329329
try {
330330
return mService.getPassword(account);
331331
} catch (RemoteException e) {
332-
// will never happen
332+
// won't ever happen
333333
throw new RuntimeException(e);
334334
}
335335
}
@@ -354,7 +354,7 @@ public String getUserData(final Account account, final String key) {
354354
try {
355355
return mService.getUserData(account, key);
356356
} catch (RemoteException e) {
357-
// will never happen
357+
// won't ever happen
358358
throw new RuntimeException(e);
359359
}
360360
}
@@ -407,8 +407,10 @@ public AuthenticatorDescription[] getAuthenticatorTypesAsUser(int userId) {
407407
*
408408
* <p>It is safe to call this method from the main thread.
409409
*
410-
* <p>This method requires the caller to hold the permission
411-
* {@link android.Manifest.permission#GET_ACCOUNTS}.
410+
* <p>Clients of this method that have not been granted the
411+
* {@link android.Manifest.permission#GET_ACCOUNTS} permission,
412+
* will only see those accounts managed by AbstractAccountAuthenticators whose
413+
* signature matches the client.
412414
*
413415
* @return An array of {@link Account}, one for each account. Empty
414416
* (never null) if no accounts have been added.
@@ -430,8 +432,10 @@ public Account[] getAccounts() {
430432
*
431433
* <p>It is safe to call this method from the main thread.
432434
*
433-
* <p>This method requires the caller to hold the permission
434-
* {@link android.Manifest.permission#GET_ACCOUNTS}.
435+
* <p>Clients of this method that have not been granted the
436+
* {@link android.Manifest.permission#GET_ACCOUNTS} permission,
437+
* will only see those accounts managed by AbstractAccountAuthenticators whose
438+
* signature matches the client.
435439
*
436440
* @return An array of {@link Account}, one for each account. Empty
437441
* (never null) if no accounts have been added.
@@ -458,7 +462,7 @@ public Account[] getAccountsForPackage(String packageName, int uid) {
458462
try {
459463
return mService.getAccountsForPackage(packageName, uid);
460464
} catch (RemoteException re) {
461-
// possible security exception
465+
// won't ever happen
462466
throw new RuntimeException(re);
463467
}
464468
}
@@ -475,7 +479,7 @@ public Account[] getAccountsByTypeForPackage(String type, String packageName) {
475479
try {
476480
return mService.getAccountsByTypeForPackage(type, packageName);
477481
} catch (RemoteException re) {
478-
// possible security exception
482+
// won't ever happen
479483
throw new RuntimeException(re);
480484
}
481485
}
@@ -489,9 +493,10 @@ public Account[] getAccountsByTypeForPackage(String type, String packageName) {
489493
*
490494
* <p>It is safe to call this method from the main thread.
491495
*
492-
* <p>This method requires the caller to hold the permission
493-
* {@link android.Manifest.permission#GET_ACCOUNTS} or share a uid with the
494-
* authenticator that owns the account type.
496+
* <p>Clients of this method that have not been granted the
497+
* {@link android.Manifest.permission#GET_ACCOUNTS} permission,
498+
* will only see those accounts managed by AbstractAccountAuthenticators whose
499+
* signature matches the client.
495500
*
496501
* @param type The type of accounts to return, null to retrieve all accounts
497502
* @return An array of {@link Account}, one per matching account. Empty
@@ -573,7 +578,8 @@ public String bundleToResult(Bundle bundle) throws AuthenticatorException {
573578
* {@link AccountManagerFuture} must not be used on the main thread.
574579
*
575580
* <p>This method requires the caller to hold the permission
576-
* {@link android.Manifest.permission#GET_ACCOUNTS}.
581+
* {@link android.Manifest.permission#GET_ACCOUNTS} or be a signature
582+
* match with the AbstractAccountAuthenticator that manages the account.
577583
*
578584
* @param account The {@link Account} to test
579585
* @param features An array of the account features to check
@@ -616,9 +622,10 @@ public Boolean bundleToResult(Bundle bundle) throws AuthenticatorException {
616622
* <p>This method may be called from any thread, but the returned
617623
* {@link AccountManagerFuture} must not be used on the main thread.
618624
*
619-
* <p>This method requires the caller to hold the permission
620-
* {@link android.Manifest.permission#GET_ACCOUNTS} or share a uid with the
621-
* authenticator that owns the account type.
625+
* <p>Clients of this method that have not been granted the
626+
* {@link android.Manifest.permission#GET_ACCOUNTS} permission,
627+
* will only see those accounts managed by AbstractAccountAuthenticators whose
628+
* signature matches the client.
622629
*
623630
* @param type The type of accounts to return, must not be null
624631
* @param features An array of the account features to require,
@@ -680,7 +687,7 @@ public boolean addAccountExplicitly(Account account, String password, Bundle use
680687
try {
681688
return mService.addAccountExplicitly(account, password, userdata);
682689
} catch (RemoteException e) {
683-
// won't ever happen
690+
// Can happen if there was a SecurityException was thrown.
684691
throw new RuntimeException(e);
685692
}
686693
}
@@ -929,7 +936,7 @@ public boolean removeAccountExplicitly(Account account) {
929936
try {
930937
return mService.removeAccountExplicitly(account);
931938
} catch (RemoteException e) {
932-
// won't ever happen
939+
// May happen if the caller doesn't match the signature of the authenticator.
933940
throw new RuntimeException(e);
934941
}
935942
}
@@ -1057,7 +1064,7 @@ public void setUserData(final Account account, final String key, final String va
10571064
try {
10581065
mService.setUserData(account, key, value);
10591066
} catch (RemoteException e) {
1060-
// won't ever happen
1067+
// Will happen if there is not signature match.
10611068
throw new RuntimeException(e);
10621069
}
10631070
}
@@ -1648,7 +1655,7 @@ public void doWork() throws RemoteException {
16481655
* with these fields if an activity was supplied and the account
16491656
* credentials were successfully updated:
16501657
* <ul>
1651-
* <li> {@link #KEY_ACCOUNT_NAME} - the name of the account created
1658+
* <li> {@link #KEY_ACCOUNT_NAME} - the name of the account
16521659
* <li> {@link #KEY_ACCOUNT_TYPE} - the type of the account
16531660
* </ul>
16541661
*
@@ -2408,10 +2415,12 @@ public void onReceive(final Context context, final Intent intent) {
24082415
* listeners are added in an Activity or Service's {@link Activity#onCreate}
24092416
* and removed in {@link Activity#onDestroy}.
24102417
*
2411-
* <p>It is safe to call this method from the main thread.
2418+
* <p>The listener will only be informed of accounts that would be returned
2419+
* to the caller via {@link #getAccounts()}. Typically this means that to
2420+
* get any accounts, the caller will need to be grated the GET_ACCOUNTS
2421+
* permission.
24122422
*
2413-
* <p>This method requires the caller to hold the permission
2414-
* {@link android.Manifest.permission#GET_ACCOUNTS}.
2423+
* <p>It is safe to call this method from the main thread.
24152424
*
24162425
* @param listener The listener to send notifications to
24172426
* @param handler {@link Handler} identifying the thread to use

0 commit comments

Comments
 (0)