Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge changes Ice19df39,I9e2e6d19

Browse Source 
* changes:
  EnforcePermissionFix: refactor for greater separation of concern and reuse
  SimpleManualPermissionEnforcementDetector: suggest calling helper method and turn on
This commit is contained in:
Matt Gilbride
2023-01-26 18:13:40 +00:00
committed by Android (Google) Code Review
parent 729d0a62be d27d379efd
commit 39401634b9
8 changed files with 375 additions and 309 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
tools/lint/global/Android.bp
View File

@@ -38,12 +38,6 @@ java_library_host {
java_test_host {
name: "AndroidGlobalLintCheckerTest",
// TODO(b/239881504): Since this test was written, Android
// Lint was updated, and now includes classes that were
// compiled for java 15. The soong build doesn't support
// java 15 yet, so we can't compile against "lint". Disable
// the test until java 15 is supported.
enabled: false,
srcs: ["checks/src/test/java/**/*.kt"],
static_libs: [
"AndroidGlobalLintChecker",
@@ -53,5 +47,19 @@ java_test_host {
],
test_options: {
unit_test: true,
tradefed_options: [
{
// lint bundles in some classes that were built with older versions
// of libraries, and no longer load. Since tradefed tries to load
// all classes in the jar to look for tests, it crashes loading them.
// Exclude these classes from tradefed's search.
name: "exclude-paths",
value: "org/apache",
},
{
name: "exclude-paths",
value: "META-INF",
},
],
},
}

2
tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/Constants.kt
View File

@@ -29,6 +29,8 @@ val AIDL_PERMISSION_ANNOTATIONS = listOf(
const val BINDER_CLASS = "android.os.Binder"
const val IINTERFACE_INTERFACE = "android.os.IInterface"
const val AIDL_PERMISSION_HELPER_SUFFIX = "_enforcePermission"
/**
* If a non java (e.g. c++) backend is enabled, the @EnforcePermission
* annotation cannot be used. At time of writing, the mechanism

84
tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionFix.kt
View File

@@ -36,6 +36,7 @@ import org.jetbrains.uast.UCallExpression
import org.jetbrains.uast.UExpression
import org.jetbrains.uast.UExpressionList
import org.jetbrains.uast.UIfExpression
import org.jetbrains.uast.UMethod
import org.jetbrains.uast.UThrowExpression
import org.jetbrains.uast.UastBinaryOperator
import org.jetbrains.uast.evaluateString
@@ -46,29 +47,37 @@ import org.jetbrains.uast.visitor.AbstractUastVisitor
* Helper class that facilitates the creation of lint auto fixes
*/
data class EnforcePermissionFix(
val locations: List<Location>,
val manualCheckLocations: List<Location>,
val permissionNames: List<String>,
val errorLevel: Boolean,
val anyOf: Boolean,
) {
fun toLintFix(annotationLocation: Location): LintFix {
val removeFixes = this.locations.map {
LintFix.create()
.replace()
.reformat(true)
.range(it)
.with("")
.autoFix()
.build()
fun toLintFix(context: JavaContext, node: UMethod): LintFix {
val methodLocation = context.getLocation(node)
val replaceOrRemoveFixes = manualCheckLocations.mapIndexed { index, manualCheckLocation ->
if (index == 0) {
// Replace the first manual check with a call to the helper method
getHelperMethodFix(node, manualCheckLocation, false)
} else {
// Remove all subsequent manual checks
LintFix.create()
.replace()
.reformat(true)
.range(manualCheckLocation)
.with("")
.autoFix()
.build()
}
}
// Annotate the method with @EnforcePermission(...)
val annotateFix = LintFix.create()
.annotate(this.annotation)
.range(annotationLocation)
.annotate(annotation)
.range(methodLocation)
.autoFix()
.build()
return LintFix.create().composite(annotateFix, *removeFixes.toTypedArray())
return LintFix.create().composite(annotateFix, *replaceOrRemoveFixes.toTypedArray())
}
private val annotation: String
@@ -88,8 +97,51 @@ data class EnforcePermissionFix(
}
companion object {
/**
* Walks the expressions in a block, looking for simple permission checks.
*
* As soon as something other than a permission check is encountered, stop looking,
* as some other business logic is happening that prevents an automated fix.
*/
fun fromBlockExpression(
context: JavaContext,
blockExpression: UBlockExpression
): EnforcePermissionFix? {
try {
val singleFixes = mutableListOf<EnforcePermissionFix>()
for (expression in blockExpression.expressions) {
val fix = fromExpression(context, expression) ?: break
singleFixes.add(fix)
}
return compose(singleFixes)
} catch (e: AnyOfAllOfException) {
return null
}
}
/**
* Conditionally constructs EnforcePermissionFix from any UExpression
*
* @return EnforcePermissionFix if the expression boils down to a permission check,
* else null
*/
fun fromExpression(
context: JavaContext,
expression: UExpression
): EnforcePermissionFix? {
val trimmedExpression = expression.skipParenthesizedExprDown()
if (trimmedExpression is UIfExpression) {
return fromIfExpression(context, trimmedExpression)
}
findCallExpression(trimmedExpression)?.let {
return fromCallExpression(context, it)
}
return null
}
/**
* Conditionally constructs EnforcePermissionFix from a UCallExpression
*
* @return EnforcePermissionFix if the called method is annotated with @PermissionMethod, else null
*/
fun fromCallExpression(
@@ -111,6 +163,7 @@ data class EnforcePermissionFix(
/**
* Conditionally constructs EnforcePermissionFix from a UCallExpression
*
* @return EnforcePermissionFix IF AND ONLY IF:
* * The condition of the if statement compares the return value of a
* PermissionMethod to one of the PackageManager.PermissionResult values
@@ -172,7 +225,8 @@ data class EnforcePermissionFix(
}
fun compose(individuals: List<EnforcePermissionFix>): EnforcePermissionFix {
fun compose(individuals: List<EnforcePermissionFix>): EnforcePermissionFix? {
if (individuals.isEmpty()) return null
val anyOfs = individuals.filter(EnforcePermissionFix::anyOf)
// anyOf/allOf should be consistent. If we encounter some @PermissionMethods that are anyOf
// and others that aren't, we don't know what to do.
@@ -180,7 +234,7 @@ data class EnforcePermissionFix(
throw AnyOfAllOfException()
}
return EnforcePermissionFix(
individuals.flatMap(EnforcePermissionFix::locations),
individuals.flatMap(EnforcePermissionFix::manualCheckLocations),
individuals.flatMap(EnforcePermissionFix::permissionNames),
errorLevel = individuals.all(EnforcePermissionFix::errorLevel),
anyOf = anyOfs.isNotEmpty()

15
tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionHelperDetector.kt
View File

@@ -55,7 +55,7 @@ class EnforcePermissionHelperDetector : Detector(), SourceCodeScanner {
return
}
val targetExpression = "${node.name}$HELPER_SUFFIX()"
val targetExpression = getHelperMethodCallSourceString(node)
val message =
"Method must start with $targetExpression or super.${node.name}(), if applicable"
@@ -85,22 +85,11 @@ class EnforcePermissionHelperDetector : Detector(), SourceCodeScanner {
val locationTarget = getLocationTarget(firstExpression)
val expressionLocation = context.getLocation(locationTarget)
val indent = " ".repeat(expressionLocation.start?.column ?: 0)
val fix = fix()
.replace()
.range(expressionLocation)
.beginning()
.with("$targetExpression;\n\n$indent")
.reformat(true)
.autoFix()
.build()
context.report(
ISSUE_ENFORCE_PERMISSION_HELPER,
context.getLocation(node),
message,
fix
getHelperMethodFix(node, expressionLocation),
)
}
}

25
tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt
View File

@@ -17,6 +17,8 @@
package com.google.android.lint.aidl
import com.android.tools.lint.detector.api.JavaContext
import com.android.tools.lint.detector.api.LintFix
import com.android.tools.lint.detector.api.Location
import com.intellij.psi.PsiClass
import com.intellij.psi.PsiReferenceList
import org.jetbrains.uast.UMethod
@@ -69,3 +71,26 @@ private fun hasSingleAncestor(references: PsiReferenceList?, qualifiedName: Stri
references != null &&
references.referenceElements.size == 1 &&
references.referenceElements[0].qualifiedName == qualifiedName
fun getHelperMethodCallSourceString(node: UMethod) = "${node.name}$AIDL_PERMISSION_HELPER_SUFFIX()"
fun getHelperMethodFix(
node: UMethod,
manualCheckLocation: Location,
prepend: Boolean = true
): LintFix {
val helperMethodSource = getHelperMethodCallSourceString(node)
val indent = " ".repeat(manualCheckLocation.start?.column ?: 0)
val newText = "$helperMethodSource;${if (prepend) "\n\n$indent" else ""}"
val fix = LintFix.create()
.replace()
.range(manualCheckLocation)
.with(newText)
.reformat(true)
.autoFix()
if (prepend) fix.beginning()
return fix.build()
}

70
tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/SimpleManualPermissionEnforcementDetector.kt
View File

@@ -23,19 +23,13 @@ import com.android.tools.lint.detector.api.Issue
import com.android.tools.lint.detector.api.JavaContext
import com.android.tools.lint.detector.api.Scope
import com.android.tools.lint.detector.api.Severity
import com.google.android.lint.findCallExpression
import org.jetbrains.uast.UBlockExpression
import org.jetbrains.uast.UElement
import org.jetbrains.uast.UIfExpression
import org.jetbrains.uast.UMethod
import org.jetbrains.uast.skipParenthesizedExprDown
/**
* Looks for methods implementing generated AIDL interface stubs
* that can have simple permission checks migrated to
* @EnforcePermission annotations
*
* TODO: b/242564870 (enable parse and autoFix of .aidl files)
*/
@Suppress("UnstableApiUsage")
class SimpleManualPermissionEnforcementDetector : AidlImplementationDetector() {
@@ -45,8 +39,8 @@ class SimpleManualPermissionEnforcementDetector : AidlImplementationDetector() {
interfaceName: String,
body: UBlockExpression
) {
val enforcePermissionFix = accumulateSimplePermissionCheckFixes(body, context) ?: return
val lintFix = enforcePermissionFix.toLintFix(context.getLocation(node))
val enforcePermissionFix = EnforcePermissionFix.fromBlockExpression(context, body) ?: return
val lintFix = enforcePermissionFix.toLintFix(context, node)
val message =
"$interfaceName permission check ${
if (enforcePermissionFix.errorLevel) "should" else "can"
@@ -54,68 +48,19 @@ class SimpleManualPermissionEnforcementDetector : AidlImplementationDetector() {
val incident = Incident(
ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT,
enforcePermissionFix.locations.last(),
enforcePermissionFix.manualCheckLocations.last(),
message,
lintFix
)
if (enforcePermissionFix.errorLevel) {
incident.overrideSeverity(Severity.ERROR)
}
// TODO(b/265014041): turn on errors once all code that would cause one is fixed
// if (enforcePermissionFix.errorLevel) {
// incident.overrideSeverity(Severity.ERROR)
// }
context.report(incident)
}
/**
* Walk the expressions in the method, looking for simple permission checks.
*
* If a single permission check is found at the beginning of the method,
* this should be migrated to @EnforcePermission(value).
*
* If multiple consecutive permission checks are found,
* these should be migrated to @EnforcePermission(allOf={value1, value2, ...})
*
* As soon as something other than a permission check is encountered, stop looking,
* as some other business logic is happening that prevents an automated fix.
*/
private fun accumulateSimplePermissionCheckFixes(
methodBody: UBlockExpression,
context: JavaContext
): EnforcePermissionFix? {
try {
val singleFixes = mutableListOf<EnforcePermissionFix>()
for (expression in methodBody.expressions) {
val fix = getPermissionCheckFix(
expression.skipParenthesizedExprDown(),
context) ?: break
singleFixes.add(fix)
}
return when (singleFixes.size) {
0 -> null
1 -> singleFixes[0]
else -> EnforcePermissionFix.compose(singleFixes)
}
} catch (e: AnyOfAllOfException) {
return null
}
}
/**
* If an expression boils down to a permission check, return
* the helper for creating a lint auto fix to @EnforcePermission
*/
private fun getPermissionCheckFix(startingExpression: UElement?, context: JavaContext):
EnforcePermissionFix? {
if (startingExpression is UIfExpression) {
return EnforcePermissionFix.fromIfExpression(context, startingExpression)
}
findCallExpression(startingExpression)?.let {
return EnforcePermissionFix.fromCallExpression(context, it)
}
return null
}
companion object {
private val EXPLANATION = """
@@ -142,7 +87,6 @@ class SimpleManualPermissionEnforcementDetector : AidlImplementationDetector() {
SimpleManualPermissionEnforcementDetector::class.java,
Scope.JAVA_FILE_SCOPE
),
enabledByDefault = false, // TODO: enable once b/241171714 is resolved
)
}
}

374
tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/SimpleManualPermissionEnforcementDetectorTest.kt
View File

@@ -51,10 +51,10 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
.run()
.expect(
"""
src/Foo.java:7: Error: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
src/Foo.java:7: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 errors, 0 warnings
0 errors, 1 warnings
"""
)
.expectFixDiffs(
@@ -64,6 +64,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
+ @android.annotation.EnforcePermission("android.permission.READ_CONTACTS")
@@ -7 +8
- mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
+ test_enforcePermission();
"""
)
}
@@ -101,6 +102,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
+ @android.annotation.EnforcePermission("android.permission.READ_CONTACTS")
@@ -7 +8
- mContext.enforceCallingPermission("android.permission.READ_CONTACTS", "foo");
+ test_enforcePermission();
"""
)
}
@@ -138,6 +140,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
+ @android.annotation.EnforcePermission("android.permission.READ_CONTACTS")
@@ -7 +8
- mContext.checkCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
+ test_enforcePermission();
"""
)
}
@@ -165,10 +168,10 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
.run()
.expect(
"""
src/Foo.java:8: Error: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
src/Foo.java:8: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
mContext.enforceCallingOrSelfPermission(
^
1 errors, 0 warnings
0 errors, 1 warnings
"""
)
.expectFixDiffs(
@@ -179,6 +182,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
@@ -8 +9
- mContext.enforceCallingOrSelfPermission(
- "android.permission.READ_CONTACTS", "foo");
+ test_enforcePermission();
"""
)
}
@@ -205,19 +209,20 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
.run()
.expect(
"""
src/Foo.java:8: Error: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
src/Foo.java:8: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
mContext.enforceCallingOrSelfPermission(android.Manifest.permission.READ_CONTACTS, "foo");
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 errors, 0 warnings
0 errors, 1 warnings
"""
)
.expectFixDiffs(
"""
Fix for src/Foo.java line 7: Annotate with @EnforcePermission:
Fix for src/Foo.java line 8: Annotate with @EnforcePermission:
@@ -6 +6
+ @android.annotation.EnforcePermission("android.permission.READ_CONTACTS")
@@ -8 +9
- mContext.enforceCallingOrSelfPermission(android.Manifest.permission.READ_CONTACTS, "foo");
+ test_enforcePermission();
"""
)
}
@@ -247,10 +252,10 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
.run()
.expect(
"""
src/Foo.java:10: Error: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
src/Foo.java:10: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
mContext.enforceCallingOrSelfPermission(
^
1 errors, 0 warnings
0 errors, 1 warnings
"""
)
.expectFixDiffs(
@@ -263,98 +268,101 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
- "android.permission.READ_CONTACTS", "foo");
- mContext.enforceCallingOrSelfPermission(
- "android.permission.WRITE_CONTACTS", "foo");
+ test_enforcePermission();
"""
)
}
fun testAllOf_mixedOrSelf_warning() {
lint().files(
java(
"""
import android.content.Context;
import android.test.ITest;
public class Foo {
private Context mContext;
private ITest itest = new ITest.Stub() {
@Override
public void test() throws android.os.RemoteException {
mContext.enforceCallingOrSelfPermission(
"android.permission.READ_CONTACTS", "foo");
mContext.enforceCallingPermission(
"android.permission.WRITE_CONTACTS", "foo");
}
};
}
"""
).indented(),
*stubs
java(
"""
import android.content.Context;
import android.test.ITest;
public class Foo {
private Context mContext;
private ITest itest = new ITest.Stub() {
@Override
public void test() throws android.os.RemoteException {
mContext.enforceCallingOrSelfPermission(
"android.permission.READ_CONTACTS", "foo");
mContext.enforceCallingPermission(
"android.permission.WRITE_CONTACTS", "foo");
}
};
}
"""
).indented(),
*stubs
)
.run()
.expect(
"""
src/Foo.java:10: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
mContext.enforceCallingPermission(
^
0 errors, 1 warnings
"""
)
.expectFixDiffs(
"""
Fix for src/Foo.java line 10: Annotate with @EnforcePermission:
@@ -6 +6
+ @android.annotation.EnforcePermission(allOf={"android.permission.READ_CONTACTS", "android.permission.WRITE_CONTACTS"})
@@ -8 +9
- mContext.enforceCallingOrSelfPermission(
- "android.permission.READ_CONTACTS", "foo");
- mContext.enforceCallingPermission(
- "android.permission.WRITE_CONTACTS", "foo");
"""
)
.run()
.expect(
"""
src/Foo.java:10: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
mContext.enforceCallingPermission(
^
0 errors, 1 warnings
"""
)
.expectFixDiffs(
"""
Fix for src/Foo.java line 10: Annotate with @EnforcePermission:
@@ -6 +6
+ @android.annotation.EnforcePermission(allOf={"android.permission.READ_CONTACTS", "android.permission.WRITE_CONTACTS"})
@@ -8 +9
- mContext.enforceCallingOrSelfPermission(
- "android.permission.READ_CONTACTS", "foo");
- mContext.enforceCallingPermission(
- "android.permission.WRITE_CONTACTS", "foo");
+ test_enforcePermission();
"""
)
}
fun testAllOf_mixedEnforces_warning() {
lint().files(
java(
"""
import android.content.Context;
import android.test.ITest;
public class Foo {
private Context mContext;
private ITest itest = new ITest.Stub() {
@Override
public void test() throws android.os.RemoteException {
mContext.enforceCallingOrSelfPermission(
"android.permission.READ_CONTACTS", "foo");
mContext.checkCallingOrSelfPermission(
"android.permission.WRITE_CONTACTS", "foo");
}
};
}
"""
).indented(),
*stubs
java(
"""
import android.content.Context;
import android.test.ITest;
public class Foo {
private Context mContext;
private ITest itest = new ITest.Stub() {
@Override
public void test() throws android.os.RemoteException {
mContext.enforceCallingOrSelfPermission(
"android.permission.READ_CONTACTS", "foo");
mContext.checkCallingOrSelfPermission(
"android.permission.WRITE_CONTACTS", "foo");
}
};
}
"""
).indented(),
*stubs
)
.run()
.expect(
"""
src/Foo.java:10: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
mContext.checkCallingOrSelfPermission(
^
0 errors, 1 warnings
"""
)
.expectFixDiffs(
"""
Fix for src/Foo.java line 10: Annotate with @EnforcePermission:
@@ -6 +6
+ @android.annotation.EnforcePermission(allOf={"android.permission.READ_CONTACTS", "android.permission.WRITE_CONTACTS"})
@@ -8 +9
- mContext.enforceCallingOrSelfPermission(
- "android.permission.READ_CONTACTS", "foo");
- mContext.checkCallingOrSelfPermission(
- "android.permission.WRITE_CONTACTS", "foo");
"""
)
.run()
.expect(
"""
src/Foo.java:10: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
mContext.checkCallingOrSelfPermission(
^
0 errors, 1 warnings
"""
)
.expectFixDiffs(
"""
Fix for src/Foo.java line 10: Annotate with @EnforcePermission:
@@ -6 +6
+ @android.annotation.EnforcePermission(allOf={"android.permission.READ_CONTACTS", "android.permission.WRITE_CONTACTS"})
@@ -8 +9
- mContext.enforceCallingOrSelfPermission(
- "android.permission.READ_CONTACTS", "foo");
- mContext.checkCallingOrSelfPermission(
- "android.permission.WRITE_CONTACTS", "foo");
+ test_enforcePermission();
"""
)
}
fun testPrecedingExpressions() {
@@ -389,7 +397,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
public class Foo extends ITest.Stub {
private Context mContext;
@android.content.pm.PermissionMethod(orSelf = true)
@android.annotation.PermissionMethod(orSelf = true)
private void helper() {
mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
}
@@ -406,10 +414,10 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
.run()
.expect(
"""
src/Foo.java:14: Error: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
src/Foo.java:14: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
helper();
~~~~~~~~~
1 errors, 0 warnings
0 errors, 1 warnings
"""
)
.expectFixDiffs(
@@ -419,6 +427,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
+ @android.annotation.EnforcePermission("android.permission.READ_CONTACTS")
@@ -14 +15
- helper();
+ test_enforcePermission();
"""
)
}
@@ -433,7 +442,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
public class Foo extends ITest.Stub {
private Context mContext;
@android.content.pm.PermissionMethod
@android.annotation.PermissionMethod
private void helper() {
mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
}
@@ -463,6 +472,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
+ @android.annotation.EnforcePermission("android.permission.READ_CONTACTS")
@@ -14 +15
- helper();
+ test_enforcePermission();
"""
)
}
@@ -477,7 +487,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
public class Foo extends ITest.Stub {
private Context mContext;
@android.content.pm.PermissionMethod(orSelf = true)
@android.annotation.PermissionMethod(orSelf = true)
private void helper() {
mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
mContext.enforceCallingOrSelfPermission("android.permission.WRITE_CONTACTS", "foo");
@@ -496,10 +506,10 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
.run()
.expect(
"""
src/Foo.java:16: Error: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
src/Foo.java:16: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
mContext.enforceCallingOrSelfPermission("FOO", "foo");
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 errors, 0 warnings
0 errors, 1 warnings
"""
)
.expectFixDiffs(
@@ -510,6 +520,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
@@ -15 +16
- helper();
- mContext.enforceCallingOrSelfPermission("FOO", "foo");
+ test_enforcePermission();
"""
)
}
@@ -525,13 +536,13 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
public class Foo extends ITest.Stub {
private Context mContext;
@android.content.pm.PermissionMethod(orSelf = true)
@android.annotation.PermissionMethod(orSelf = true)
private void helperHelper() {
helper("android.permission.WRITE_CONTACTS");
}
@android.content.pm.PermissionMethod(orSelf = true)
private void helper(@android.content.pm.PermissionName String extraPermission) {
@android.annotation.PermissionMethod(orSelf = true)
private void helper(@android.annotation.PermissionName String extraPermission) {
mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
}
@@ -547,10 +558,10 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
.run()
.expect(
"""
src/Foo.java:19: Error: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
src/Foo.java:19: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
helperHelper();
~~~~~~~~~~~~~~~
1 errors, 0 warnings
0 errors, 1 warnings
"""
)
.expectFixDiffs(
@@ -560,6 +571,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
+ @android.annotation.EnforcePermission(allOf={"android.permission.WRITE_CONTACTS", "android.permission.READ_CONTACTS"})
@@ -19 +20
- helperHelper();
+ test_enforcePermission();
"""
)
}
@@ -587,10 +599,10 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
.run()
.expect(
"""
src/Foo.java:7: Error: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
src/Foo.java:7: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
if (mContext.checkCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo")
^
1 errors, 0 warnings
0 errors, 1 warnings
"""
)
.expectFixDiffs(
@@ -603,76 +615,106 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
- != PackageManager.PERMISSION_GRANTED) {
- throw new SecurityException("yikes!");
- }
+ test_enforcePermission();
"""
)
}
fun testIfExpression_orSelfFalse_warning() {
lint().files(
java(
"""
import android.content.Context;
import android.test.ITest;
public class Foo extends ITest.Stub {
private Context mContext;
@Override
public void test() throws android.os.RemoteException {
if (mContext.checkCallingPermission("android.permission.READ_CONTACTS", "foo")
!= PackageManager.PERMISSION_GRANTED) {
throw new SecurityException("yikes!");
}
java(
"""
import android.content.Context;
import android.test.ITest;
public class Foo extends ITest.Stub {
private Context mContext;
@Override
public void test() throws android.os.RemoteException {
if (mContext.checkCallingPermission("android.permission.READ_CONTACTS", "foo")
!= PackageManager.PERMISSION_GRANTED) {
throw new SecurityException("yikes!");
}
}
"""
).indented(),
*stubs
}
"""
).indented(),
*stubs
)
.run()
.expect(
"""
src/Foo.java:7: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
if (mContext.checkCallingPermission("android.permission.READ_CONTACTS", "foo")
^
0 errors, 1 warnings
"""
)
.expectFixDiffs(
"""
Fix for src/Foo.java line 7: Annotate with @EnforcePermission:
@@ -5 +5
+ @android.annotation.EnforcePermission("android.permission.READ_CONTACTS")
@@ -7 +8
- if (mContext.checkCallingPermission("android.permission.READ_CONTACTS", "foo")
- != PackageManager.PERMISSION_GRANTED) {
- throw new SecurityException("yikes!");
- }
"""
)
.run()
.expect(
"""
src/Foo.java:7: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
if (mContext.checkCallingPermission("android.permission.READ_CONTACTS", "foo")
^
0 errors, 1 warnings
"""
)
.expectFixDiffs(
"""
Fix for src/Foo.java line 7: Annotate with @EnforcePermission:
@@ -5 +5
+ @android.annotation.EnforcePermission("android.permission.READ_CONTACTS")
@@ -7 +8
- if (mContext.checkCallingPermission("android.permission.READ_CONTACTS", "foo")
- != PackageManager.PERMISSION_GRANTED) {
- throw new SecurityException("yikes!");
- }
+ test_enforcePermission();
"""
)
}
fun testIfExpression_otherSideEffect_ignored() {
lint().files(
java(
"""
import android.content.Context;
import android.test.ITest;
public class Foo extends ITest.Stub {
private Context mContext;
@Override
public void test() throws android.os.RemoteException {
if (mContext.checkCallingPermission("android.permission.READ_CONTACTS", "foo")
!= PackageManager.PERMISSION_GRANTED) {
doSomethingElse();
throw new SecurityException("yikes!");
}
java(
"""
import android.content.Context;
import android.test.ITest;
public class Foo extends ITest.Stub {
private Context mContext;
@Override
public void test() throws android.os.RemoteException {
if (mContext.checkCallingPermission("android.permission.READ_CONTACTS", "foo")
!= PackageManager.PERMISSION_GRANTED) {
doSomethingElse();
throw new SecurityException("yikes!");
}
}
"""
).indented(),
*stubs
}
"""
).indented(),
*stubs
)
.run()
.expectClean()
.run()
.expectClean()
}
fun testIfExpression_inlinedWithSideEffect_ignored() {
lint().files(
java(
"""
import android.content.Context;
import android.test.ITest;
public class Foo extends ITest.Stub {
private Context mContext;
@Override
public void test() throws android.os.RemoteException {
if (somethingElse() && mContext.checkCallingPermission("android.permission.READ_CONTACTS", "foo")
!= PackageManager.PERMISSION_GRANTED) {
throw new SecurityException("yikes!");
}
}
private boolean somethingElse() {
return true;
}
}
"""
).indented(),
*stubs
)
.run()
.expectClean()
}
fun testAnyOf_hardCodedAndVarArgs() {
@@ -685,13 +727,13 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
public class Foo extends ITest.Stub {
private Context mContext;
@android.content.pm.PermissionMethod(anyOf = true)
@android.annotation.PermissionMethod(anyOf = true)
private void helperHelper() {
helper("FOO", "BAR");
}
@android.content.pm.PermissionMethod(anyOf = true, value = {"BAZ", "BUZZ"})
private void helper(@android.content.pm.PermissionName String... extraPermissions) {}
@android.annotation.PermissionMethod(anyOf = true, value = {"BAZ", "BUZZ"})
private void helper(@android.annotation.PermissionName String... extraPermissions) {}
@Override
public void test() throws android.os.RemoteException {
@@ -718,6 +760,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
+ @android.annotation.EnforcePermission(anyOf={"BAZ", "BUZZ", "FOO", "BAR"})
@@ -17 +18
- helperHelper();
+ test_enforcePermission();
"""
)
}
@@ -733,13 +776,13 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
public class Foo extends ITest.Stub {
private Context mContext;
@android.content.pm.PermissionMethod
@android.annotation.PermissionMethod
private void allOfhelper() {
mContext.enforceCallingOrSelfPermission("FOO");
mContext.enforceCallingOrSelfPermission("BAR");
}
@android.content.pm.PermissionMethod(anyOf = true, permissions = {"BAZ", "BUZZ"})
@android.annotation.PermissionMethod(anyOf = true, permissions = {"BAZ", "BUZZ"})
private void anyOfHelper() {}
@Override
@@ -761,17 +804,18 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
java(
"""
import android.content.Context;
import android.content.pm.PermissionName;import android.test.ITest;
import android.annotation.PermissionName;
import android.test.ITest;
public class Foo extends ITest.Stub {
private Context mContext;
@android.content.pm.PermissionMethod(anyOf = true)
@android.annotation.PermissionMethod(anyOf = true)
private void anyOfCheck(@PermissionName String... permissions) {
allOfCheck("BAZ", "BUZZ");
}
@android.content.pm.PermissionMethod
@android.annotation.PermissionMethod
private void allOfCheck(@PermissionName String... permissions) {}
@Override

94
tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/Stubs.kt
View File

@@ -5,84 +5,84 @@ import com.android.tools.lint.checks.infrastructure.TestFile
val aidlStub: TestFile = java(
"""
package android.test;
public interface ITest extends android.os.IInterface {
public static abstract class Stub extends android.os.Binder implements android.test.ITest {
protected void test_enforcePermission() throws SecurityException {}
}
public void test() throws android.os.RemoteException;
package android.test;
public interface ITest extends android.os.IInterface {
public static abstract class Stub extends android.os.Binder implements android.test.ITest {
protected void test_enforcePermission() throws SecurityException {}
}
public void test() throws android.os.RemoteException;
}
"""
).indented()
val contextStub: TestFile = java(
"""
package android.content;
public class Context {
@android.content.pm.PermissionMethod(orSelf = true)
public void enforceCallingOrSelfPermission(@android.content.pm.PermissionName String permission, String message) {}
@android.content.pm.PermissionMethod
public void enforceCallingPermission(@android.content.pm.PermissionName String permission, String message) {}
@android.content.pm.PermissionMethod(orSelf = true)
public int checkCallingOrSelfPermission(@android.content.pm.PermissionName String permission, String message) {}
@android.content.pm.PermissionMethod
public int checkCallingPermission(@android.content.pm.PermissionName String permission, String message) {}
}
package android.content;
public class Context {
@android.annotation.PermissionMethod(orSelf = true)
public void enforceCallingOrSelfPermission(@android.annotation.PermissionName String permission, String message) {}
@android.annotation.PermissionMethod
public void enforceCallingPermission(@android.annotation.PermissionName String permission, String message) {}
@android.annotation.PermissionMethod(orSelf = true)
public int checkCallingOrSelfPermission(@android.annotation.PermissionName String permission, String message) {}
@android.annotation.PermissionMethod
public int checkCallingPermission(@android.annotation.PermissionName String permission, String message) {}
}
"""
).indented()
val binderStub: TestFile = java(
"""
package android.os;
public class Binder {
public static int getCallingUid() {}
}
package android.os;
public class Binder {
public static int getCallingUid() {}
}
"""
).indented()
val permissionMethodStub: TestFile = java(
"""
package android.content.pm;
"""
package android.annotation;
import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.RetentionPolicy.CLASS;
import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.RetentionPolicy.CLASS;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;
@Retention(CLASS)
@Target({METHOD})
public @interface PermissionMethod {}
@Retention(CLASS)
@Target({METHOD})
public @interface PermissionMethod {}
"""
).indented()
val permissionNameStub: TestFile = java(
"""
package android.content.pm;
"""
package android.annotation;
import static java.lang.annotation.ElementType.FIELD;
import static java.lang.annotation.ElementType.LOCAL_VARIABLE;
import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.ElementType.PARAMETER;
import static java.lang.annotation.RetentionPolicy.CLASS;
import static java.lang.annotation.ElementType.FIELD;
import static java.lang.annotation.ElementType.LOCAL_VARIABLE;
import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.ElementType.PARAMETER;
import static java.lang.annotation.RetentionPolicy.CLASS;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;
@Retention(CLASS)
@Target({PARAMETER, METHOD, LOCAL_VARIABLE, FIELD})
public @interface PermissionName {}
@Retention(CLASS)
@Target({PARAMETER, METHOD, LOCAL_VARIABLE, FIELD})
public @interface PermissionName {}
"""
).indented()
val manifestStub: TestFile = java(
"""
package android;
package android;
public final class Manifest {
public static final class permission {
public static final String READ_CONTACTS="android.permission.READ_CONTACTS";
}
public final class Manifest {
public static final class permission {
public static final String READ_CONTACTS="android.permission.READ_CONTACTS";
}
}
""".trimIndent()
)