Skip to content

Commit c617721

Browse files
nbradburynbradbury
committed
Merge remote-tracking branch 'origin/trunk' into issue/network-changed-anr
2 parents 8d1b21e + e05eaf0 commit c617721

10 files changed

Lines changed: 210 additions & 15 deletions

File tree

‎WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt‎

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@ import org.wordpress.android.modules.BG_THREAD
2020
import org.wordpress.android.util.AppLog
2121
import org.wordpress.android.util.BuildConfigWrapper
2222
import org.wordpress.android.util.UrlUtils
23+
import org.wordpress.android.util.WPUrlUtils
2324
import org.wordpress.android.util.crashlogging.sendReportWithTag
2425
import rs.wordpress.api.kotlin.ApiDiscoveryResult
2526
import rs.wordpress.api.kotlin.WpLoginClient
27+
import uniffi.wp_api.DiscoveredAuthenticationMechanism
2628
import uniffi.wp_api.applicationPasswordsUrl
2729
import java.net.URI
2830
import javax.inject.Inject
@@ -53,6 +55,12 @@ class ApplicationPasswordLoginHelper @Inject constructor(
5355
sealed class DiscoveryResult {
5456
data class Authorized(val authorizationUrl: String) : DiscoveryResult()
5557
data class Failed(val userFacingMessage: String) : DiscoveryResult()
58+
59+
/**
60+
* The site is hosted on WordPress.com: API discovery reported OAuth2 as the authentication
61+
* mechanism, so it can't use Application Passwords and should log in via WordPress.com.
62+
*/
63+
object WpComSite : DiscoveryResult()
5664
}
5765

5866
@Suppress("TooGenericExceptionCaught")
@@ -67,21 +75,29 @@ class ApplicationPasswordLoginHelper @Inject constructor(
6775
withContext(bgDispatcher) {
6876
when (val urlDiscoveryResult = wpLoginClient.apiDiscovery(siteUrl)) {
6977
is ApiDiscoveryResult.Success -> {
70-
val authorizationUrl =
71-
discoverSuccessWrapper.getApplicationPasswordsAuthenticationUrl(urlDiscoveryResult)
72-
val apiRootUrl = discoverSuccessWrapper.getApiRootUrl(urlDiscoveryResult)
73-
if (apiRootUrl.isNotEmpty()) {
74-
// Store the ApiRootUrl for use it after the login
75-
apiRootUrlCache.put(UrlUtils.normalizeUrl(siteUrl), apiRootUrl)
78+
if (discoverSuccessWrapper.isWpComSite(urlDiscoveryResult)) {
79+
// WordPress.com sites report OAuth2 as the authentication mechanism; they
80+
// can't use Application Passwords and must log in via WordPress.com.
81+
appLogWrapper.d(AppLog.T.API, "A_P: $siteUrl is a WordPress.com site (OAuth2)")
82+
AnalyticsTracker.track(Stat.BACKGROUND_REST_AUTODISCOVERY_SUCCESSFUL)
83+
DiscoveryResult.WpComSite
84+
} else {
85+
val authorizationUrl =
86+
discoverSuccessWrapper.getApplicationPasswordsAuthenticationUrl(urlDiscoveryResult)
87+
val apiRootUrl = discoverSuccessWrapper.getApiRootUrl(urlDiscoveryResult)
88+
if (apiRootUrl.isNotEmpty()) {
89+
// Store the ApiRootUrl for use it after the login
90+
apiRootUrlCache.put(UrlUtils.normalizeUrl(siteUrl), apiRootUrl)
91+
}
92+
val authorizationUrlComplete =
93+
uriLoginWrapper.appendParamsToRestAuthorizationUrl(authorizationUrl)
94+
appLogWrapper.d(
95+
AppLog.T.API,
96+
"A_P: Found authorization for $siteUrl URL: $authorizationUrlComplete " +
97+
"API_ROOT_URL $apiRootUrl")
98+
AnalyticsTracker.track(Stat.BACKGROUND_REST_AUTODISCOVERY_SUCCESSFUL)
99+
DiscoveryResult.Authorized(authorizationUrlComplete)
76100
}
77-
val authorizationUrlComplete =
78-
uriLoginWrapper.appendParamsToRestAuthorizationUrl(authorizationUrl)
79-
appLogWrapper.d(
80-
AppLog.T.API,
81-
"A_P: Found authorization for $siteUrl URL: $authorizationUrlComplete " +
82-
"API_ROOT_URL $apiRootUrl")
83-
AnalyticsTracker.track(Stat.BACKGROUND_REST_AUTODISCOVERY_SUCCESSFUL)
84-
DiscoveryResult.Authorized(authorizationUrlComplete)
85101
}
86102

87103
is ApiDiscoveryResult.FailureFetchAndParseApiRoot,
@@ -409,6 +425,19 @@ class ApplicationPasswordLoginHelper @Inject constructor(
409425
class DiscoverSuccessWrapper @Inject constructor() {
410426
fun getApiRootUrl(successObject: ApiDiscoveryResult.Success) = successObject.success.apiRootUrl.url()
411427

428+
/**
429+
* WordPress.com sites advertise OAuth2 as their authentication mechanism during API
430+
* discovery (self-hosted sites advertise Application Passwords). Self-hosted sites can
431+
* also expose OAuth2 via plugins, so we additionally require the advertised OAuth2
432+
* authorization endpoint to be hosted on wordpress.com. Otherwise a malicious site could
433+
* pose as WordPress.com to hijack our login flow.
434+
*/
435+
fun isWpComSite(successObject: ApiDiscoveryResult.Success): Boolean {
436+
val authentication = successObject.success.authentication
437+
return authentication is DiscoveredAuthenticationMechanism.OAuth2 &&
438+
WPUrlUtils.isWordPressCom(authentication.endpoints.authorizationUrl)
439+
}
440+
412441
fun getApplicationPasswordsAuthenticationUrl(
413442
successObject: ApiDiscoveryResult.Success
414443
): String = requireNotNull(

‎WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/ApplicationPasswordAutoAuthDialogViewModel.kt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,10 @@ class ApplicationPasswordAutoAuthDialogViewModel @Inject constructor(
152152
when (val result = applicationPasswordLoginHelper.getAuthorizationUrlComplete(siteUrl)) {
153153
is ApplicationPasswordLoginHelper.DiscoveryResult.Authorized ->
154154
_navigationEvent.emit(NavigationEvent.FallbackToManualLogin(result.authorizationUrl))
155+
is ApplicationPasswordLoginHelper.DiscoveryResult.WpComSite -> {
156+
appLogWrapper.e(AppLog.T.API, "A_P: $siteUrl is a WordPress.com site")
157+
_navigationEvent.emit(NavigationEvent.Error)
158+
}
155159
is ApplicationPasswordLoginHelper.DiscoveryResult.Failed -> {
156160
appLogWrapper.e(
157161
AppLog.T.API,

‎WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/ApplicationPasswordDialogViewModel.kt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ class ApplicationPasswordDialogViewModel @Inject constructor(
4141
when (val result = applicationPasswordLoginHelper.getAuthorizationUrlComplete(authenticationUrl)) {
4242
is ApplicationPasswordLoginHelper.DiscoveryResult.Authorized ->
4343
_navigationEvent.emit(NavigationEvent.NavigateToLogin(result.authorizationUrl))
44+
is ApplicationPasswordLoginHelper.DiscoveryResult.WpComSite -> {
45+
appLogWrapper.e(AppLog.T.MAIN, "Authentication URL is a WordPress.com site")
46+
_navigationEvent.emit(NavigationEvent.ShowError)
47+
}
4448
is ApplicationPasswordLoginHelper.DiscoveryResult.Failed -> {
4549
appLogWrapper.e(
4650
AppLog.T.MAIN,

‎WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/LoginSiteApplicationPasswordFragment.kt‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,18 @@ class LoginSiteApplicationPasswordFragment : Fragment() {
114114
}
115115
}
116116
}
117+
118+
viewLifecycleOwner.lifecycleScope.launch {
119+
viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) {
120+
viewModel.wpComDetected.collect {
121+
// WP.com sites can't use application passwords; send them to the OAuth flow.
122+
// Discovery already ran here (unlike the up-front WPUrlUtils.isWordPressCom()
123+
// check), so record that it resolved to a WordPress.com site.
124+
analyticsListener.trackConnectedSiteInfoSucceeded(mapOf("is_wpcom" to true))
125+
loginActivity?.showWPcomLoginScreen(requireContext())
126+
}
127+
}
128+
}
117129
}
118130

119131
override fun onResume() {

‎WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/LoginSiteApplicationPasswordViewModel.kt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ class LoginSiteApplicationPasswordViewModel @Inject constructor(
1919
private val _discoveryURL = Channel<String>(Channel.BUFFERED)
2020
val discoveryURL = _discoveryURL.receiveAsFlow()
2121

22+
private val _wpComDetected = Channel<String>(Channel.BUFFERED)
23+
val wpComDetected = _wpComDetected.receiveAsFlow()
24+
2225
private val _loadingStateFlow = MutableStateFlow(false)
2326
val loadingStateFlow = _loadingStateFlow.asStateFlow()
2427

@@ -34,6 +37,8 @@ class LoginSiteApplicationPasswordViewModel @Inject constructor(
3437
when (val result = applicationPasswordLoginHelper.getAuthorizationUrlComplete(siteUrl)) {
3538
is ApplicationPasswordLoginHelper.DiscoveryResult.Authorized ->
3639
_discoveryURL.send(result.authorizationUrl)
40+
is ApplicationPasswordLoginHelper.DiscoveryResult.WpComSite ->
41+
_wpComDetected.send(siteUrl)
3742
is ApplicationPasswordLoginHelper.DiscoveryResult.Failed -> {
3843
_errorMessage.value = result.userFacingMessage
3944
_discoveryURL.send("")

‎WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordViewModelSlice.kt‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,13 @@ class ApplicationPasswordViewModelSlice @Inject constructor(
170170
)
171171
appLogWrapper.d(AppLog.T.MAIN, "A_P: Showing reauthentication card for ${site.url}")
172172
}
173+
is ApplicationPasswordLoginHelper.DiscoveryResult.WpComSite -> {
174+
uiModelMutable.postValue(null)
175+
appLogWrapper.d(
176+
AppLog.T.MAIN,
177+
"A_P: Hiding reauthentication card for ${site.url} - WordPress.com site"
178+
)
179+
}
173180
is ApplicationPasswordLoginHelper.DiscoveryResult.Failed -> {
174181
// TODO follow-up: surface result.userFacingMessage in the card (issue #22884).
175182
uiModelMutable.postValue(null)
@@ -186,6 +193,10 @@ class ApplicationPasswordViewModelSlice @Inject constructor(
186193
is ApplicationPasswordLoginHelper.DiscoveryResult.Authorized -> {
187194
showApplicationPasswordCreateCard(site, result.authorizationUrl)
188195
}
196+
is ApplicationPasswordLoginHelper.DiscoveryResult.WpComSite -> {
197+
uiModelMutable.postValue(null)
198+
appLogWrapper.d(AppLog.T.MAIN, "A_P: Hiding card for ${site.url} - WordPress.com site")
199+
}
189200
is ApplicationPasswordLoginHelper.DiscoveryResult.Failed -> {
190201
// TODO follow-up: surface result.userFacingMessage in the card (issue #22884).
191202
uiModelMutable.postValue(null)

‎WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import rs.wordpress.api.kotlin.ApiDiscoveryResult
2828
import rs.wordpress.api.kotlin.WpLoginClient
2929
import uniffi.wp_api.AutoDiscoveryAttemptSuccess
3030
import uniffi.wp_api.DiscoveredAuthenticationMechanism
31+
import uniffi.wp_api.OAuth2Endpoints
3132
import uniffi.wp_api.ParseUrlException
3233
import kotlin.test.assertEquals
3334
import kotlin.test.assertIs
@@ -377,6 +378,22 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() {
377378
verify(wpLoginClient).apiDiscovery(eq(TEST_URL))
378379
}
379380

381+
@Test
382+
fun `given a WP_com site, when api discovery returns OAuth2, then return WpComSite`() = runTest {
383+
val oAuth2 = DiscoveredAuthenticationMechanism.OAuth2(
384+
OAuth2Endpoints(authorizationUrl = TEST_URL, tokenUrl = TEST_URL)
385+
)
386+
val autoDiscoveryAttemptSuccess = AutoDiscoveryAttemptSuccess(mock(), mock(), mock(), oAuth2)
387+
val apiDiscoveryResult = ApiDiscoveryResult.Success(autoDiscoveryAttemptSuccess)
388+
whenever(wpLoginClient.apiDiscovery(eq(TEST_URL))).thenReturn(apiDiscoveryResult)
389+
whenever(discoverSuccessWrapper.isWpComSite(eq(apiDiscoveryResult))).thenReturn(true)
390+
391+
val result = applicationPasswordLoginHelper.getAuthorizationUrlComplete(TEST_URL)
392+
393+
assertEquals(ApplicationPasswordLoginHelper.DiscoveryResult.WpComSite, result)
394+
verify(wpLoginClient).apiDiscovery(eq(TEST_URL))
395+
}
396+
380397
@Test
381398
fun `given login scenario, when api discovery throws, then return Failed`() = runTest {
382399
whenever(wpLoginClient.apiDiscovery(eq(TEST_URL))).doThrow(RuntimeException("API discovery failed"))

‎WordPress/src/test/java/org/wordpress/android/ui/accounts/login/DiscoverSuccessWrapperTest.kt‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
package org.wordpress.android.ui.accounts.login
2+
3+
import org.junit.Assert.assertFalse
4+
import org.junit.Assert.assertTrue
5+
import org.junit.Test
6+
import org.junit.runner.RunWith
7+
import org.mockito.kotlin.mock
8+
import org.robolectric.RobolectricTestRunner
9+
import org.robolectric.annotation.Config
10+
import rs.wordpress.api.kotlin.ApiDiscoveryResult
11+
import uniffi.wp_api.AutoDiscoveryAttemptSuccess
12+
import uniffi.wp_api.DiscoveredAuthenticationMechanism
13+
import uniffi.wp_api.OAuth2Endpoints
14+
import uniffi.wp_api.ParsedUrl
15+
16+
/**
17+
* Uses Robolectric because [org.wordpress.android.util.WPUrlUtils.isWordPressCom] relies on
18+
* android.net.Uri to extract the host.
19+
*/
20+
@RunWith(RobolectricTestRunner::class)
21+
@Config(application = android.app.Application::class)
22+
class DiscoverSuccessWrapperTest {
23+
private val wrapper = ApplicationPasswordLoginHelper.DiscoverSuccessWrapper()
24+
25+
@Test
26+
fun `given OAuth2 with a wordpress_com endpoint, when isWpComSite, then returns true`() {
27+
val result = oAuth2Success("https://public-api.wordpress.com/oauth2/authorize")
28+
29+
assertTrue(wrapper.isWpComSite(result))
30+
}
31+
32+
@Test
33+
fun `given OAuth2 with a non-wordpress_com endpoint, when isWpComSite, then returns false`() {
34+
// A self-hosted site exposing OAuth2 via a plugin must not be treated as WordPress.com.
35+
val result = oAuth2Success("https://malicious.example.com/oauth2/authorize")
36+
37+
assertFalse(wrapper.isWpComSite(result))
38+
}
39+
40+
@Test
41+
fun `given ApplicationPasswords mechanism, when isWpComSite, then returns false`() {
42+
val authentication = DiscoveredAuthenticationMechanism.ApplicationPasswords(mock<ParsedUrl>())
43+
val result = ApiDiscoveryResult.Success(
44+
AutoDiscoveryAttemptSuccess(mock(), mock(), mock(), authentication)
45+
)
46+
47+
assertFalse(wrapper.isWpComSite(result))
48+
}
49+
50+
private fun oAuth2Success(authorizationUrl: String): ApiDiscoveryResult.Success {
51+
val authentication = DiscoveredAuthenticationMechanism.OAuth2(
52+
OAuth2Endpoints(authorizationUrl = authorizationUrl, tokenUrl = authorizationUrl)
53+
)
54+
return ApiDiscoveryResult.Success(
55+
AutoDiscoveryAttemptSuccess(mock(), mock(), mock(), authentication)
56+
)
57+
}
58+
}

‎WordPress/src/test/java/org/wordpress/android/ui/accounts/login/applicationpassword/LoginSiteApplicationPasswordViewModelTest.kt‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import org.mockito.kotlin.whenever
1616
import org.wordpress.android.BaseUnitTest
1717
import org.wordpress.android.ui.accounts.login.ApplicationPasswordLoginHelper
1818
import kotlin.test.assertEquals
19+
import kotlin.test.assertNull
1920

2021
@ExperimentalCoroutinesApi
2122
@RunWith(MockitoJUnitRunner::class)
@@ -63,4 +64,58 @@ class LoginSiteApplicationPasswordViewModelTest : BaseUnitTest() {
6364

6465
job.cancel() // Clean up the collector job
6566
}
67+
68+
@Test
69+
fun `Given a WP_com site, when running discovery, then wpComDetected emits the url`() = test {
70+
// Given
71+
val siteUrl = "https://example.com"
72+
whenever(applicationPasswordLoginHelper.getAuthorizationUrlComplete(siteUrl))
73+
.thenReturn(ApplicationPasswordLoginHelper.DiscoveryResult.WpComSite)
74+
75+
var detectedUrl: String? = null
76+
val job = launch {
77+
viewModel.wpComDetected.first { url ->
78+
detectedUrl = url
79+
true
80+
}
81+
}
82+
83+
// When
84+
viewModel.runApiDiscovery(siteUrl)
85+
advanceUntilIdle()
86+
87+
// Then
88+
assertEquals(siteUrl, detectedUrl)
89+
assertNull(viewModel.errorMessage.value)
90+
assertEquals(false, viewModel.loadingStateFlow.value)
91+
92+
job.cancel()
93+
}
94+
95+
@Test
96+
fun `Given discovery fails, when running discovery, then the generic error is shown`() = test {
97+
// Given
98+
val siteUrl = "https://example.com"
99+
val errorMessage = "not supported"
100+
whenever(applicationPasswordLoginHelper.getAuthorizationUrlComplete(siteUrl))
101+
.thenReturn(ApplicationPasswordLoginHelper.DiscoveryResult.Failed(errorMessage))
102+
103+
var wpComDetected = false
104+
val wpComJob = launch { viewModel.wpComDetected.first { wpComDetected = true; true } }
105+
106+
var collectedUrl: String? = null
107+
val discoveryJob = launch { viewModel.discoveryURL.first { collectedUrl = it; true } }
108+
109+
// When
110+
viewModel.runApiDiscovery(siteUrl)
111+
advanceUntilIdle()
112+
113+
// Then
114+
assertEquals(errorMessage, viewModel.errorMessage.value)
115+
assertEquals("", collectedUrl)
116+
assertEquals(false, wpComDetected)
117+
118+
wpComJob.cancel()
119+
discoveryJob.cancel()
120+
}
66121
}

‎gradle/libs.versions.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
[versions]
2-
agp = '9.3.0'
2+
agp = '9.3.1'
33
airbnb-lottie = '6.7.1'
44
android-desugar = '2.1.5'
55
android-installreferrer = '2.2'

0 commit comments

Comments
 (0)