diff options
5 files changed, 404 insertions, 126 deletions
diff --git a/tools/lint/checks/src/main/java/com/google/android/lint/PermissionMethodDetector.kt b/tools/lint/checks/src/main/java/com/google/android/lint/PermissionMethodDetector.kt index 68a450d956a8..1b0f03564c3b 100644 --- a/tools/lint/checks/src/main/java/com/google/android/lint/PermissionMethodDetector.kt +++ b/tools/lint/checks/src/main/java/com/google/android/lint/PermissionMethodDetector.kt @@ -26,6 +26,7 @@ import com.android.tools.lint.detector.api.Scope import com.android.tools.lint.detector.api.Severity import com.android.tools.lint.detector.api.SourceCodeScanner import com.android.tools.lint.detector.api.getUMethod +import com.google.android.lint.aidl.hasPermissionMethodAnnotation import com.intellij.psi.PsiType import org.jetbrains.uast.UAnnotation import org.jetbrains.uast.UBlockExpression @@ -149,11 +150,6 @@ class PermissionMethodDetector : Detector(), SourceCodeScanner { enabledByDefault = false ) - private fun hasPermissionMethodAnnotation(method: UMethod): Boolean = method.annotations - .any { - it.hasQualifiedName(ANNOTATION_PERMISSION_METHOD) - } - private fun isPermissionMethodReturnType(method: UMethod): Boolean = listOf(PsiType.VOID, PsiType.INT, PsiType.BOOLEAN).contains(method.returnType) diff --git a/tools/lint/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionFix.kt b/tools/lint/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionFix.kt index 510611161ea8..d120e1d41c99 100644 --- a/tools/lint/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionFix.kt +++ b/tools/lint/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionFix.kt @@ -18,37 +18,54 @@ package com.google.android.lint.aidl import com.android.tools.lint.detector.api.JavaContext import com.android.tools.lint.detector.api.Location -import com.intellij.psi.PsiVariable +import com.android.tools.lint.detector.api.getUMethod +import org.jetbrains.kotlin.psi.psiUtil.parameterIndex import org.jetbrains.uast.UCallExpression -import org.jetbrains.uast.ULiteralExpression -import org.jetbrains.uast.UQualifiedReferenceExpression -import org.jetbrains.uast.USimpleNameReferenceExpression -import org.jetbrains.uast.asRecursiveLogString +import org.jetbrains.uast.evaluateString +import org.jetbrains.uast.visitor.AbstractUastVisitor /** - * Helper ADT class that facilitates the creation of lint auto fixes + * Helper class that facilitates the creation of lint auto fixes * * Handles "Single" permission checks that should be migrated to @EnforcePermission(...), as well as consecutive checks * that should be migrated to @EnforcePermission(allOf={...}) * * TODO: handle anyOf style annotations */ -sealed class EnforcePermissionFix { - abstract fun locations(): List<Location> - abstract fun javaAnnotationParameter(): String - - fun javaAnnotation(): String = "@$ANNOTATION_ENFORCE_PERMISSION(${javaAnnotationParameter()})" +data class EnforcePermissionFix( + val locations: List<Location>, + val permissionNames: List<String> +) { + val annotation: String + get() { + val quotedPermissions = permissionNames.joinToString(", ") { """"$it"""" } + val annotationParameter = + if (permissionNames.size > 1) "allOf={$quotedPermissions}" else quotedPermissions + return "@$ANNOTATION_ENFORCE_PERMISSION($annotationParameter)" + } companion object { - fun fromCallExpression(callExpression: UCallExpression, context: JavaContext): SingleFix = - SingleFix( - getPermissionCheckLocation(context, callExpression), - getPermissionCheckArgumentValue(callExpression) - ) + /** + * conditionally constructs EnforcePermissionFix from a UCallExpression + * @return EnforcePermissionFix if the called method is annotated with @PermissionMethod, else null + */ + fun fromCallExpression( + context: JavaContext, + callExpression: UCallExpression + ): EnforcePermissionFix? = + if (isPermissionMethodCall(callExpression)) { + EnforcePermissionFix( + listOf(getPermissionCheckLocation(context, callExpression)), + getPermissionCheckValues(callExpression) + ) + } else null - fun maybeAddManifestPrefix(permissionName: String): String = - if (permissionName.contains(".")) permissionName - else "android.Manifest.permission.$permissionName" + + fun compose(individuals: List<EnforcePermissionFix>): EnforcePermissionFix = + EnforcePermissionFix( + individuals.flatMap { it.locations }, + individuals.flatMap { it.permissionNames } + ) /** * Given a permission check, get its proper location @@ -70,49 +87,51 @@ sealed class EnforcePermissionFix { } /** - * Given a permission check and an argument, - * pull out the permission value that is being used + * Given a @PermissionMethod, find arguments annotated with @PermissionName + * and pull out the permission value(s) being used. Also evaluates nested calls + * to @PermissionMethod(s) in the given method's body. */ - private fun getPermissionCheckArgumentValue( - callExpression: UCallExpression, - argumentPosition: Int = 0 - ): String { + private fun getPermissionCheckValues( + callExpression: UCallExpression + ): List<String> { + if (!isPermissionMethodCall(callExpression)) return emptyList() - val identifier = when ( - val argument = callExpression.valueArguments.getOrNull(argumentPosition) - ) { - is UQualifiedReferenceExpression -> when (val selector = argument.selector) { - is USimpleNameReferenceExpression -> - ((selector.resolve() as PsiVariable).computeConstantValue() as String) + val result = mutableSetOf<String>() // protect against duplicate permission values + val visitedCalls = mutableSetOf<UCallExpression>() // don't visit the same call twice + val bfsQueue = ArrayDeque(listOf(callExpression)) - else -> throw RuntimeException( - "Couldn't resolve argument: ${selector.asRecursiveLogString()}" - ) - } + // Breadth First Search - evalutaing nested @PermissionMethod(s) in the available + // source code for @PermissionName(s). + while (bfsQueue.isNotEmpty()) { + val current = bfsQueue.removeFirst() + visitedCalls.add(current) + result.addAll(findPermissions(current)) - is USimpleNameReferenceExpression -> ( - (argument.resolve() as PsiVariable).computeConstantValue() as String) + current.resolve()?.getUMethod()?.accept(object : AbstractUastVisitor() { + override fun visitCallExpression(node: UCallExpression): Boolean { + if (isPermissionMethodCall(node) && node !in visitedCalls) { + bfsQueue.add(node) + } + return false + } + }) + } - is ULiteralExpression -> argument.value as String + return result.toList() + } - else -> throw RuntimeException( - "Couldn't resolve argument: ${argument?.asRecursiveLogString()}" - ) - } + private fun findPermissions( + callExpression: UCallExpression, + ): List<String> { + val indices = callExpression.resolve()?.getUMethod() + ?.uastParameters + ?.filter(::hasPermissionNameAnnotation) + ?.mapNotNull { it.sourcePsi?.parameterIndex() } + ?: emptyList() - return identifier.substringAfterLast(".") + return indices.mapNotNull { + callExpression.getArgumentForParameter(it)?.evaluateString() + } } } } - -data class SingleFix(val location: Location, val permissionName: String) : EnforcePermissionFix() { - override fun locations(): List<Location> = listOf(this.location) - override fun javaAnnotationParameter(): String = maybeAddManifestPrefix(this.permissionName) -} -data class AllOfFix(val checks: List<SingleFix>) : EnforcePermissionFix() { - override fun locations(): List<Location> = this.checks.map { it.location } - override fun javaAnnotationParameter(): String = - "allOf={${ - this.checks.joinToString(", ") { maybeAddManifestPrefix(it.permissionName) } - }}" -} diff --git a/tools/lint/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt b/tools/lint/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt new file mode 100644 index 000000000000..edbdd8d2adf3 --- /dev/null +++ b/tools/lint/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt @@ -0,0 +1,67 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.android.lint.aidl + +import com.android.tools.lint.detector.api.getUMethod +import com.google.android.lint.ANNOTATION_PERMISSION_METHOD +import com.google.android.lint.ANNOTATION_PERMISSION_NAME +import com.google.android.lint.CLASS_STUB +import com.intellij.psi.PsiAnonymousClass +import org.jetbrains.uast.UCallExpression +import org.jetbrains.uast.UMethod +import org.jetbrains.uast.UParameter + +/** + * Given a UMethod, determine if this method is + * an entrypoint to an interface generated by AIDL, + * returning the interface name if so + */ +fun getContainingAidlInterface(node: UMethod): String? { + if (!isInClassCalledStub(node)) return null + for (superMethod in node.findSuperMethods()) { + for (extendsInterface in superMethod.containingClass?.extendsList?.referenceElements + ?: continue) { + if (extendsInterface.qualifiedName == IINTERFACE_INTERFACE) { + return superMethod.containingClass?.name + } + } + } + return null +} + +private fun isInClassCalledStub(node: UMethod): Boolean { + (node.containingClass as? PsiAnonymousClass)?.let { + return it.baseClassReference.referenceName == CLASS_STUB + } + return node.containingClass?.extendsList?.referenceElements?.any { + it.referenceName == CLASS_STUB + } ?: false +} + +fun isPermissionMethodCall(callExpression: UCallExpression): Boolean { + val method = callExpression.resolve()?.getUMethod() ?: return false + return hasPermissionMethodAnnotation(method) +} + +fun hasPermissionMethodAnnotation(method: UMethod): Boolean = method.annotations + .any { + it.hasQualifiedName(ANNOTATION_PERMISSION_METHOD) + } + +fun hasPermissionNameAnnotation(parameter: UParameter) = parameter.annotations.any { + it.hasQualifiedName(ANNOTATION_PERMISSION_NAME) +} diff --git a/tools/lint/checks/src/main/java/com/google/android/lint/aidl/ManualPermissionCheckDetector.kt b/tools/lint/checks/src/main/java/com/google/android/lint/aidl/ManualPermissionCheckDetector.kt index 2cea39423f1d..2c53f390128c 100644 --- a/tools/lint/checks/src/main/java/com/google/android/lint/aidl/ManualPermissionCheckDetector.kt +++ b/tools/lint/checks/src/main/java/com/google/android/lint/aidl/ManualPermissionCheckDetector.kt @@ -25,9 +25,6 @@ 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.android.tools.lint.detector.api.SourceCodeScanner -import com.google.android.lint.CLASS_STUB -import com.google.android.lint.ENFORCE_PERMISSION_METHODS -import com.intellij.psi.PsiAnonymousClass import org.jetbrains.uast.UBlockExpression import org.jetbrains.uast.UCallExpression import org.jetbrains.uast.UElement @@ -56,7 +53,7 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner { val body = (node.uastBody as? UBlockExpression) ?: return val fix = accumulateSimplePermissionCheckFixes(body) ?: return - val javaRemoveFixes = fix.locations().map { + val javaRemoveFixes = fix.locations.map { fix() .replace() .reformat(true) @@ -67,7 +64,7 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner { } val javaAnnotateFix = fix() - .annotate(fix.javaAnnotation()) + .annotate(fix.annotation) .range(context.getLocation(node)) .autoFix() .build() @@ -77,7 +74,7 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner { context.report( ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION, - fix.locations().last(), + fix.locations.last(), message, fix().composite(*javaRemoveFixes.toTypedArray(), javaAnnotateFix) ) @@ -97,14 +94,14 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner { */ private fun accumulateSimplePermissionCheckFixes(methodBody: UBlockExpression): EnforcePermissionFix? { - val singleFixes = mutableListOf<SingleFix>() + val singleFixes = mutableListOf<EnforcePermissionFix>() for (expression in methodBody.expressions) { singleFixes.add(getPermissionCheckFix(expression) ?: break) } return when (singleFixes.size) { 0 -> null 1 -> singleFixes[0] - else -> AllOfFix(singleFixes) + else -> EnforcePermissionFix.compose(singleFixes) } } @@ -113,7 +110,7 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner { * the helper for creating a lint auto fix to @EnforcePermission */ private fun getPermissionCheckFix(startingExpression: UElement?): - SingleFix? { + EnforcePermissionFix? { return when (startingExpression) { is UQualifiedReferenceExpression -> getPermissionCheckFix( startingExpression.selector @@ -121,11 +118,8 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner { is UIfExpression -> getPermissionCheckFix(startingExpression.condition) - is UCallExpression -> { - return if (isPermissionCheck(startingExpression)) - EnforcePermissionFix.fromCallExpression(startingExpression, context) - else null - } + is UCallExpression -> return EnforcePermissionFix + .fromCallExpression(context, startingExpression) else -> null } @@ -160,40 +154,5 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner { ), enabledByDefault = false, // TODO: enable once b/241171714 is resolved ) - - private fun isPermissionCheck(callExpression: UCallExpression): Boolean { - val method = callExpression.resolve() ?: return false - val className = method.containingClass?.qualifiedName - return ENFORCE_PERMISSION_METHODS.any { - it.clazz == className && it.name == method.name - } - } - - /** - * given a UMethod, determine if this method is - * an entrypoint to an interface generated by AIDL, - * returning the interface name if so - */ - fun getContainingAidlInterface(node: UMethod): String? { - if (!isInClassCalledStub(node)) return null - for (superMethod in node.findSuperMethods()) { - for (extendsInterface in superMethod.containingClass?.extendsList?.referenceElements - ?: continue) { - if (extendsInterface.qualifiedName == IINTERFACE_INTERFACE) { - return superMethod.containingClass?.name - } - } - } - return null - } - - private fun isInClassCalledStub(node: UMethod): Boolean { - (node.containingClass as? PsiAnonymousClass)?.let { - return it.baseClassReference.referenceName == CLASS_STUB - } - return node.containingClass?.extendsList?.referenceElements?.any { - it.referenceName == CLASS_STUB - } ?: false - } } } diff --git a/tools/lint/checks/src/test/java/com/google/android/lint/aidl/ManualPermissionCheckDetectorTest.kt b/tools/lint/checks/src/test/java/com/google/android/lint/aidl/ManualPermissionCheckDetectorTest.kt index 1a1c6bc77785..a968f5e6cb1c 100644 --- a/tools/lint/checks/src/test/java/com/google/android/lint/aidl/ManualPermissionCheckDetectorTest.kt +++ b/tools/lint/checks/src/test/java/com/google/android/lint/aidl/ManualPermissionCheckDetectorTest.kt @@ -19,6 +19,7 @@ package com.google.android.lint.aidl import com.android.tools.lint.checks.infrastructure.LintDetectorTest import com.android.tools.lint.checks.infrastructure.TestFile import com.android.tools.lint.checks.infrastructure.TestLintTask +import com.android.tools.lint.checks.infrastructure.TestMode import com.android.tools.lint.detector.api.Detector import com.android.tools.lint.detector.api.Issue @@ -42,7 +43,7 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() { private Context mContext; @Override public void test() throws android.os.RemoteException { - mContext.enforceCallingOrSelfPermission("android.Manifest.permission.READ_CONTACTS", "foo"); + mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo"); } } """ @@ -53,8 +54,8 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() { .expect( """ src/Foo.java:7: Warning: ITest permission check can be converted to @EnforcePermission annotation [UseEnforcePermissionAnnotation] - mContext.enforceCallingOrSelfPermission("android.Manifest.permission.READ_CONTACTS", "foo"); - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo"); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 0 errors, 1 warnings """ ) @@ -62,9 +63,9 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() { """ Fix for src/Foo.java line 7: Annotate with @EnforcePermission: @@ -5 +5 - + @android.annotation.EnforcePermission(android.Manifest.permission.READ_CONTACTS) + + @android.annotation.EnforcePermission("android.permission.READ_CONTACTS") @@ -7 +8 - - mContext.enforceCallingOrSelfPermission("android.Manifest.permission.READ_CONTACTS", "foo"); + - mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo"); """ ) } @@ -81,7 +82,7 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() { @Override public void test() throws android.os.RemoteException { mContext.enforceCallingOrSelfPermission( - "android.Manifest.permission.READ_CONTACTS", "foo"); + "android.permission.READ_CONTACTS", "foo"); } }; } @@ -102,10 +103,49 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() { """ Fix for src/Foo.java line 8: Annotate with @EnforcePermission: @@ -6 +6 - + @android.annotation.EnforcePermission(android.Manifest.permission.READ_CONTACTS) + + @android.annotation.EnforcePermission("android.permission.READ_CONTACTS") @@ -8 +9 - mContext.enforceCallingOrSelfPermission( - - "android.Manifest.permission.READ_CONTACTS", "foo"); + - "android.permission.READ_CONTACTS", "foo"); + """ + ) + } + + fun testConstantEvaluation() { + 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 { + mContext.enforceCallingOrSelfPermission(android.Manifest.permission.READ_CONTACTS, "foo"); + } + } + """ + ).indented(), + *stubs, + manifestStub + ) + .run() + .expect( + """ + src/Foo.java:8: Warning: ITest permission check can be converted to @EnforcePermission annotation [UseEnforcePermissionAnnotation] + mContext.enforceCallingOrSelfPermission(android.Manifest.permission.READ_CONTACTS, "foo"); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + 0 errors, 1 warnings + """ + ) + .expectFixDiffs( + """ + Fix for src/Foo.java line 7: Annotate with @EnforcePermission: + @@ -6 +6 + + @android.annotation.EnforcePermission("android.permission.READ_CONTACTS") + @@ -8 +9 + - mContext.enforceCallingOrSelfPermission(android.Manifest.permission.READ_CONTACTS, "foo"); """ ) } @@ -122,9 +162,9 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() { @Override public void test() throws android.os.RemoteException { mContext.enforceCallingOrSelfPermission( - "android.Manifest.permission.READ_CONTACTS", "foo"); + "android.permission.READ_CONTACTS", "foo"); mContext.enforceCallingOrSelfPermission( - "android.Manifest.permission.WRITE_CONTACTS", "foo"); + "android.permission.WRITE_CONTACTS", "foo"); } }; } @@ -144,13 +184,13 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() { .expectFixDiffs( """ Fix for src/Foo.java line 10: Annotate with @EnforcePermission: - @@ -6 +6 - + @android.annotation.EnforcePermission(allOf={android.Manifest.permission.READ_CONTACTS, android.Manifest.permission.WRITE_CONTACTS}) + @@ -6 +6 + + @android.annotation.EnforcePermission(allOf={"android.permission.READ_CONTACTS", "android.permission.WRITE_CONTACTS"}) @@ -8 +9 - mContext.enforceCallingOrSelfPermission( - - "android.Manifest.permission.READ_CONTACTS", "foo"); + - "android.permission.READ_CONTACTS", "foo"); - mContext.enforceCallingOrSelfPermission( - - "android.Manifest.permission.WRITE_CONTACTS", "foo"); + - "android.permission.WRITE_CONTACTS", "foo"); """ ) } @@ -166,7 +206,7 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() { @Override public void test() throws android.os.RemoteException { long uid = Binder.getCallingUid(); - mContext.enforceCallingOrSelfPermission("android.Manifest.permission.READ_CONTACTS", "foo"); + mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo"); } } """ @@ -177,6 +217,149 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() { .expectClean() } + fun testPermissionHelper() { + lint().skipTestModes(TestMode.PARENTHESIZED).files( + java( + """ + import android.content.Context; + import android.test.ITest; + + public class Foo extends ITest.Stub { + private Context mContext; + + @android.content.pm.PermissionMethod + private void helper() { + mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo"); + } + + @Override + public void test() throws android.os.RemoteException { + helper(); + } + } + """ + ).indented(), + *stubs + ) + .run() + .expect( + """ + src/Foo.java:14: Warning: ITest permission check can be converted to @EnforcePermission annotation [UseEnforcePermissionAnnotation] + helper(); + ~~~~~~~~~ + 0 errors, 1 warnings + """ + ) + .expectFixDiffs( + """ + Fix for src/Foo.java line 14: Annotate with @EnforcePermission: + @@ -12 +12 + + @android.annotation.EnforcePermission("android.permission.READ_CONTACTS") + @@ -14 +15 + - helper(); + """ + ) + } + + fun testPermissionHelperAllOf() { + lint().skipTestModes(TestMode.PARENTHESIZED).files( + java( + """ + import android.content.Context; + import android.test.ITest; + + public class Foo extends ITest.Stub { + private Context mContext; + + @android.content.pm.PermissionMethod + private void helper() { + mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo"); + mContext.enforceCallingOrSelfPermission("android.permission.WRITE_CONTACTS", "foo"); + } + + @Override + public void test() throws android.os.RemoteException { + helper(); + mContext.enforceCallingOrSelfPermission("FOO", "foo"); + } + } + """ + ).indented(), + *stubs + ) + .run() + .expect( + """ + src/Foo.java:16: Warning: ITest permission check can be converted to @EnforcePermission annotation [UseEnforcePermissionAnnotation] + mContext.enforceCallingOrSelfPermission("FOO", "foo"); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + 0 errors, 1 warnings + """ + ) + .expectFixDiffs( + """ + Fix for src/Foo.java line 16: Annotate with @EnforcePermission: + @@ -13 +13 + + @android.annotation.EnforcePermission(allOf={"android.permission.READ_CONTACTS", "android.permission.WRITE_CONTACTS", "FOO"}) + @@ -15 +16 + - helper(); + - mContext.enforceCallingOrSelfPermission("FOO", "foo"); + """ + ) + } + + + fun testPermissionHelperNested() { + lint().skipTestModes(TestMode.PARENTHESIZED).files( + java( + """ + import android.content.Context; + import android.test.ITest; + + public class Foo extends ITest.Stub { + private Context mContext; + + @android.content.pm.PermissionMethod + private void helperHelper() { + helper("android.permission.WRITE_CONTACTS"); + } + + @android.content.pm.PermissionMethod + private void helper(@android.content.pm.PermissionName String extraPermission) { + mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo"); + } + + @Override + public void test() throws android.os.RemoteException { + helperHelper(); + } + } + """ + ).indented(), + *stubs + ) + .run() + .expect( + """ + src/Foo.java:19: Warning: ITest permission check can be converted to @EnforcePermission annotation [UseEnforcePermissionAnnotation] + helperHelper(); + ~~~~~~~~~~~~~~~ + 0 errors, 1 warnings + """ + ) + .expectFixDiffs( + """ + Fix for src/Foo.java line 19: Annotate with @EnforcePermission: + @@ -17 +17 + + @android.annotation.EnforcePermission(allOf={"android.permission.WRITE_CONTACTS", "android.permission.READ_CONTACTS"}) + @@ -19 +20 + - helperHelper(); + """ + ) + } + + + companion object { private val aidlStub: TestFile = java( """ @@ -192,7 +375,8 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() { """ package android.content; public class Context { - public void enforceCallingOrSelfPermission(String permission, String message) {} + @android.content.pm.PermissionMethod + public void enforceCallingOrSelfPermission(@android.content.pm.PermissionName String permission, String message) {} } """ ).indented() @@ -206,6 +390,59 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() { """ ).indented() - val stubs = arrayOf(aidlStub, contextStub, binderStub) + private val permissionMethodStub: TestFile = java( + """ + package android.content.pm; + + import static java.lang.annotation.ElementType.METHOD; + import static java.lang.annotation.RetentionPolicy.CLASS; + + import java.lang.annotation.Retention; + import java.lang.annotation.Target; + + @Retention(CLASS) + @Target({METHOD}) + public @interface PermissionMethod {} + """ + ).indented() + + private val permissionNameStub: TestFile = java( + """ + package android.content.pm; + + 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; + + @Retention(CLASS) + @Target({PARAMETER, METHOD, LOCAL_VARIABLE, FIELD}) + public @interface PermissionName {} + """ + ).indented() + + private val manifestStub: TestFile = java( + """ + package android; + + public final class Manifest { + public static final class permission { + public static final String READ_CONTACTS="android.permission.READ_CONTACTS"; + } + } + """.trimIndent() + ) + + val stubs = arrayOf( + aidlStub, + contextStub, + binderStub, + permissionMethodStub, + permissionNameStub + ) } } |