Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Xamarin.Android.Build.Tasks] emit exported="true" for MainLauncher activities #6212

Merged

Conversation

jonathanpeppers
Copy link
Member

@jonathanpeppers jonathanpeppers commented Aug 20, 2021

Context: https://aster.cloud/2021/02/23/lets-be-explicit-about-our-intent-filters/
Fixes: #6196

An important change is coming to Android 12 that improves both app
and platform security. This change affects all apps that target
Android 12.

Activities, services, and broadcast receivers with declared
intent-filters now must explicitly declare whether they should be
exported or not.

For example:

<activity android:icon="@mipmap/icon" android:label="HelloWorld" android:name="example.MainActivity">
  <intent-filter>
    <action android:name="android.intent.action.MAIN" />
    <category android:name="android.intent.category.LAUNCHER" />
  </intent-filter>
</activity>

Should become:

<activity android:exported="true" android:icon="@mipmap/icon" android:label="HelloWorld" android:name="example.MainActivity">
  <intent-filter>
    <action android:name="android.intent.action.MAIN" />
    <category android:name="android.intent.category.LAUNCHER" />
  </intent-filter>
</activity>

Update our AndroidManifest.xml generation & related code so that
when MainLauncher=true, we also automatically set exported=true.

I updated a test to verify the contents of the generated
AndroidManifest.xml.

@jonathanpeppers jonathanpeppers changed the title [Xamarin.Android.Build.Tasks] emit exported="true" for MauiLauncher activities [Xamarin.Android.Build.Tasks] emit exported="true" for MainLauncher activities Aug 20, 2021
@jonpryor
Copy link
Member

jonpryor commented Aug 20, 2021

@jonathanpeppers wrote:

For example:

<activity android:icon="@mipmap/icon" android:label="HelloWorld" android:name="example.MainActivity">
  <intent-filter>
    <action android:name="android.intent.action.MAIN" />
    <category android:name="android.intent.category.LAUNCHER" />
  </intent-filter>
</activity>

Should become:

<activity android:icon="@mipmap/icon" android:label="HelloWorld" android:name="example.MainActivity">
  <intent-filter>
    <action android:name="android.intent.action.MAIN" android:exported="true" />
    <category android:name="android.intent.category.LAUNCHER" android:exported="true" />
  </intent-filter>
</activity>

I don't think this is correct. There is no //action/@android:exported attribute (<action/> docs), nor is there a //category/@android:exported attribute (<category/> docs). Nor is there an //intent-filter/@android:exported attribute, not that you're using one (<intent-filter/> docs).

What you should be emitting is the //activity/@android:exported attribute, which is documented.

Thus, you should be emitting:

<activity android:exported="true" android:icon="@mipmap/icon" android:label="HelloWorld" android:name="example.MainActivity">
  <intent-filter>
    <action android:name="android.intent.action.MAIN" />
    <category android:name="android.intent.category.LAUNCHER" />
  </intent-filter>
</activity>

This also matches the blog post you linked to:

<activity
    android:name=”.PlayVideoActivity”
    android:exported=”false”>
    <intent-filter>
        <action android:name=”android.intent.action.VIEW” />
        <data
            android:mimeType=”video/*”
            android:scheme=”content” />
    </intent-filter>
</activity>

@jonathanpeppers jonathanpeppers marked this pull request as draft August 20, 2021 16:15
…ctivities

Context: https://aster.cloud/2021/02/23/lets-be-explicit-about-our-intent-filters/
Fixes: dotnet#6196

> An important change is coming to Android 12 that improves both app
> and platform security. This change affects all apps that target
> Android 12.
>
> Activities, services, and broadcast receivers with declared
> intent-filters now must explicitly declare whether they should be
> exported or not.

For example:

    <activity android:icon="@mipmap/icon" android:label="HelloWorld" android:name="example.MainActivity">
      <intent-filter>
        <action android:name="android.intent.action.MAIN" />
        <category android:name="android.intent.category.LAUNCHER" />
      </intent-filter>
    </activity>

Should become:

    <activity android:exported="true" android:icon="@mipmap/icon" android:label="HelloWorld" android:name="example.MainActivity">
      <intent-filter>
        <action android:name="android.intent.action.MAIN" />
        <category android:name="android.intent.category.LAUNCHER" />
      </intent-filter>
    </activity>

Update our `AndroidManifest.xml` generation & related code so that
when `MainLauncher=true`, we also automatically set `exported=true`.

I updated a test to verify the contents of the generated
`AndroidManifest.xml`.
@jonathanpeppers
Copy link
Member Author

jonathanpeppers commented Aug 20, 2021

<activity android:exported="true" android:icon="@mipmap/icon" android:label="HelloWorld" android:name="example.MainActivity">
  <intent-filter>
    <action android:name="android.intent.action.MAIN" />
    <category android:name="android.intent.category.LAUNCHER" />
  </intent-filter>
</activity>

Thanks, I don't know how I got confused about the wrong element before.

Now I get:

    <activity android:icon="@mipmap/icon" android:label="HelloWorld" android:name="example.MainActivity" android:exported="true">
      <intent-filter>
        <action android:name="android.intent.action.MAIN" />
        <category android:name="android.intent.category.LAUNCHER" />
      </intent-filter>
    </activity>
    <activity android:label="Library Activity" android:name="mono.samples.hello.LibraryActivity" />

@jonathanpeppers jonathanpeppers marked this pull request as ready for review August 20, 2021 16:37
@@ -798,6 +798,7 @@ void AddLauncherIntentElements (XElement activity)
return;

var filter = new XElement ("intent-filter");
activity.Add (new XAttribute (androidNs + "exported", "true"));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to add this for all Android versions or should we only be doing it for Android 12 + ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not appear that //activity/@android:exported attribute has a minimum API level. Since this is a security feature, we're probably good to set this across the board?

Usually the way new attributes work in AndroidManifest.xml, is that older Android versions just ignore the values they don't know about.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh okay awesome then ! thank you

@jonpryor jonpryor merged commit d304060 into dotnet:main Aug 21, 2021
jonathanpeppers added a commit that referenced this pull request Aug 24, 2021
…6212)

Fixes: #6196

Context: https://aster.cloud/2021/02/23/lets-be-explicit-about-our-intent-filters/

> An important change is coming to Android 12 that improves both app
> and platform security. This change affects all apps that target
> Android 12.
>
> Activities, services, and broadcast receivers with declared
> intent-filters now must explicitly declare whether they should be
> exported or not.

For example:

	<activity android:icon="@mipmap/icon" android:label="HelloWorld" android:name="example.MainActivity">
	  <intent-filter>
	    <action android:name="android.intent.action.MAIN" />
	    <category android:name="android.intent.category.LAUNCHER" />
	  </intent-filter>
	</activity>

Should become:

	<activity android:exported="true" android:icon="@mipmap/icon" android:label="HelloWorld" android:name="example.MainActivity">
	  <intent-filter>
	    <action android:name="android.intent.action.MAIN" />
	    <category android:name="android.intent.category.LAUNCHER" />
	  </intent-filter>
	</activity>

Update our `AndroidManifest.xml` generation & related code so that
when `MainLauncher=true`, we also automatically set `exported=true`.

I updated a test to verify the contents of the generated
`AndroidManifest.xml`.
jonpryor pushed a commit to jonpryor/xamarin-android that referenced this pull request Aug 25, 2021
…otnet#6212)

Fixes: dotnet#6196

Context: https://aster.cloud/2021/02/23/lets-be-explicit-about-our-intent-filters/

> An important change is coming to Android 12 that improves both app
> and platform security. This change affects all apps that target
> Android 12.
>
> Activities, services, and broadcast receivers with declared
> intent-filters now must explicitly declare whether they should be
> exported or not.

For example:

	<activity android:icon="@mipmap/icon" android:label="HelloWorld" android:name="example.MainActivity">
	  <intent-filter>
	    <action android:name="android.intent.action.MAIN" />
	    <category android:name="android.intent.category.LAUNCHER" />
	  </intent-filter>
	</activity>

Should become:

	<activity android:exported="true" android:icon="@mipmap/icon" android:label="HelloWorld" android:name="example.MainActivity">
	  <intent-filter>
	    <action android:name="android.intent.action.MAIN" />
	    <category android:name="android.intent.category.LAUNCHER" />
	  </intent-filter>
	</activity>

Update our `AndroidManifest.xml` generation & related code so that
when `MainLauncher=true`, we also automatically set `exported=true`.

I updated a test to verify the contents of the generated
`AndroidManifest.xml`.
jonpryor pushed a commit that referenced this pull request Aug 26, 2021
…6212)

Fixes: #6196

Context: https://aster.cloud/2021/02/23/lets-be-explicit-about-our-intent-filters/

> An important change is coming to Android 12 that improves both app
> and platform security. This change affects all apps that target
> Android 12.
>
> Activities, services, and broadcast receivers with declared
> intent-filters now must explicitly declare whether they should be
> exported or not.

For example:

	<activity android:icon="@mipmap/icon" android:label="HelloWorld" android:name="example.MainActivity">
	  <intent-filter>
	    <action android:name="android.intent.action.MAIN" />
	    <category android:name="android.intent.category.LAUNCHER" />
	  </intent-filter>
	</activity>

Should become:

	<activity android:exported="true" android:icon="@mipmap/icon" android:label="HelloWorld" android:name="example.MainActivity">
	  <intent-filter>
	    <action android:name="android.intent.action.MAIN" />
	    <category android:name="android.intent.category.LAUNCHER" />
	  </intent-filter>
	</activity>

Update our `AndroidManifest.xml` generation & related code so that
when `MainLauncher=true`, we also automatically set `exported=true`.

I updated a test to verify the contents of the generated
`AndroidManifest.xml`.
@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ActivityAttribute.MainActivity should set exported=true
4 participants