Skip to content

Commit 0ff05d5

Browse files
adalpariclaude
andauthored
Route media through WP.com REST API for Jetpack sites with app passwords (#23117)
Jetpack-connected self-hosted sites can have Application Password credentials stored, which makes isUsingSelfHostedRestApi() return true. Media requests were therefore routed to the self-hosted REST client instead of the WP.com REST API (Jetpack tunnel), causing uploads and the Media Library to fail on sites with Application Passwords disabled. Centralize client resolution so the WP.com check runs before the self-hosted check (CMM-2114). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 2732588 commit 0ff05d5

3 files changed

Lines changed: 180 additions & 58 deletions

File tree

RELEASE-NOTES.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@
33
27.0
44
-----
55
* [*] You can now browse Google Photos (albums, collections, and search) when adding photos or videos from your device.
6+
* [**] Fixed media uploads and the Media Library failing on Jetpack-connected self-hosted sites that have Application Passwords disabled.
67
* [*] Stats now refresh when you return to the screen, so your latest data appears without a manual pull-to-refresh. [https://github.com/wordpress-mobile/WordPress-Android/pull/23112]
78

8-
99
26.9
1010
-----
1111

libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/MediaStore.java

Lines changed: 124 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import androidx.annotation.NonNull;
66
import androidx.annotation.Nullable;
7+
import androidx.annotation.VisibleForTesting;
78

89
import com.wellsql.generated.MediaModelTable;
910

@@ -894,6 +895,44 @@ private void removeMedia(@Nullable MediaModel media) {
894895
// Helper methods that choose the appropriate network client to perform an action
895896
//
896897

898+
/**
899+
* Identifies which network client should handle media requests for a given site.
900+
*/
901+
public enum MediaRestClientType {
902+
WPCOM_REST,
903+
SELF_HOSTED_RS,
904+
JETPACK_CP,
905+
APPLICATION_PASSWORDS,
906+
XMLRPC
907+
}
908+
909+
/**
910+
* Resolves the network client to use for media requests for the given site.
911+
*
912+
* IMPORTANT: {@link SiteModel#isUsingWpComRestApi()} must be checked before
913+
* {@link SiteModel#isUsingSelfHostedRestApi()}. A Jetpack-connected site can have Application
914+
* Password credentials stored (e.g. auto-generated for taxonomies or navigation menus), which
915+
* makes {@code isUsingSelfHostedRestApi()} return {@code true}. Such sites must still route media
916+
* through the WP.com REST API (the Jetpack tunnel) rather than talking to the self-hosted REST API
917+
* directly, otherwise uploads and the Media Library fail on sites that have Application Passwords
918+
* disabled (see CMM-2114).
919+
*/
920+
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
921+
public MediaRestClientType getMediaRestClientType(@NonNull SiteModel site) {
922+
if (site.isUsingWpComRestApi()) {
923+
return MediaRestClientType.WPCOM_REST;
924+
} else if (site.isUsingSelfHostedRestApi()) {
925+
return MediaRestClientType.SELF_HOSTED_RS;
926+
} else if (site.isJetpackCPConnected()) {
927+
return MediaRestClientType.JETPACK_CP;
928+
} else if (site.getOrigin() == SiteModel.ORIGIN_WPAPI
929+
&& mApplicationPasswordsConfiguration.isEnabled()) {
930+
return MediaRestClientType.APPLICATION_PASSWORDS;
931+
} else {
932+
return MediaRestClientType.XMLRPC;
933+
}
934+
}
935+
897936
private void performPushMedia(@NonNull MediaPayload payload) {
898937
if (payload.media == null) {
899938
// null or empty media list -or- list contains a null value
@@ -905,12 +944,16 @@ private void performPushMedia(@NonNull MediaPayload payload) {
905944
return;
906945
}
907946

908-
if (payload.site.isUsingSelfHostedRestApi()) {
909-
mMediaRSApiRestClient.pushMedia(payload.site, payload.media);
910-
} else if (payload.site.isUsingWpComRestApi()) {
911-
mMediaRestClient.pushMedia(payload.site, payload.media);
912-
} else {
913-
mMediaXmlrpcClient.pushMedia(payload.site, payload.media);
947+
switch (getMediaRestClientType(payload.site)) {
948+
case WPCOM_REST:
949+
mMediaRestClient.pushMedia(payload.site, payload.media);
950+
break;
951+
case SELF_HOSTED_RS:
952+
mMediaRSApiRestClient.pushMedia(payload.site, payload.media);
953+
break;
954+
default:
955+
mMediaXmlrpcClient.pushMedia(payload.site, payload.media);
956+
break;
914957
}
915958
}
916959

@@ -958,17 +1001,22 @@ private void performUploadMedia(@NonNull UploadMediaPayload payload) {
9581001
MediaUtils.stripLocation(payload.media.getFilePath());
9591002
}
9601003

961-
if (payload.site.isUsingSelfHostedRestApi()) {
962-
mMediaRSApiRestClient.uploadMedia(payload.site, payload.media);
963-
} else if (payload.site.isUsingWpComRestApi()) {
964-
mMediaRestClient.uploadMedia(payload.site, payload.media);
965-
} else if (payload.site.isJetpackCPConnected()) {
966-
mWPComV2MediaRestClient.uploadMedia(payload.site, payload.media);
967-
} else if (payload.site.getOrigin() == SiteModel.ORIGIN_WPAPI
968-
&& mApplicationPasswordsConfiguration.isEnabled()) {
969-
mApplicationPasswordsMediaRestClient.uploadMedia(payload.site, payload.media);
970-
} else {
971-
mMediaXmlrpcClient.uploadMedia(payload.site, payload.media);
1004+
switch (getMediaRestClientType(payload.site)) {
1005+
case WPCOM_REST:
1006+
mMediaRestClient.uploadMedia(payload.site, payload.media);
1007+
break;
1008+
case SELF_HOSTED_RS:
1009+
mMediaRSApiRestClient.uploadMedia(payload.site, payload.media);
1010+
break;
1011+
case JETPACK_CP:
1012+
mWPComV2MediaRestClient.uploadMedia(payload.site, payload.media);
1013+
break;
1014+
case APPLICATION_PASSWORDS:
1015+
mApplicationPasswordsMediaRestClient.uploadMedia(payload.site, payload.media);
1016+
break;
1017+
default:
1018+
mMediaXmlrpcClient.uploadMedia(payload.site, payload.media);
1019+
break;
9721020
}
9731021
}
9741022

@@ -984,21 +1032,26 @@ private void performFetchMediaList(@NonNull FetchMediaListPayload payload) {
9841032
offset = MediaSqlUtils.getMediaWithStates(payload.site, list).size();
9851033
}
9861034
}
987-
if (payload.site.isUsingSelfHostedRestApi()) {
988-
mMediaRSApiRestClient.fetchMediaList(
989-
payload.site, payload.number, offset, payload.mimeType, payload.searchTerm);
990-
} else if (payload.site.isUsingWpComRestApi()) {
991-
mMediaRestClient.fetchMediaList(
992-
payload.site, payload.number, offset, payload.mimeType, payload.searchTerm);
993-
} else if (payload.site.isJetpackCPConnected()) {
994-
mWPComV2MediaRestClient.fetchMediaList(
995-
payload.site, payload.number, offset, payload.mimeType, payload.searchTerm);
996-
} else if (payload.site.getOrigin() == SiteModel.ORIGIN_WPAPI
997-
&& mApplicationPasswordsConfiguration.isEnabled()) {
998-
mApplicationPasswordsMediaRestClient.fetchMediaList(
999-
payload.site, payload.number, offset, payload.mimeType, payload.searchTerm);
1000-
} else {
1001-
mMediaXmlrpcClient.fetchMediaList(payload.site, payload.number, offset, payload.mimeType);
1035+
switch (getMediaRestClientType(payload.site)) {
1036+
case WPCOM_REST:
1037+
mMediaRestClient.fetchMediaList(
1038+
payload.site, payload.number, offset, payload.mimeType, payload.searchTerm);
1039+
break;
1040+
case SELF_HOSTED_RS:
1041+
mMediaRSApiRestClient.fetchMediaList(
1042+
payload.site, payload.number, offset, payload.mimeType, payload.searchTerm);
1043+
break;
1044+
case JETPACK_CP:
1045+
mWPComV2MediaRestClient.fetchMediaList(
1046+
payload.site, payload.number, offset, payload.mimeType, payload.searchTerm);
1047+
break;
1048+
case APPLICATION_PASSWORDS:
1049+
mApplicationPasswordsMediaRestClient.fetchMediaList(
1050+
payload.site, payload.number, offset, payload.mimeType, payload.searchTerm);
1051+
break;
1052+
default:
1053+
mMediaXmlrpcClient.fetchMediaList(payload.site, payload.number, offset, payload.mimeType);
1054+
break;
10021055
}
10031056
}
10041057

@@ -1009,14 +1062,19 @@ private void performFetchMedia(@NonNull MediaPayload payload) {
10091062
return;
10101063
}
10111064

1012-
if (payload.site.isUsingSelfHostedRestApi()) {
1013-
mMediaRSApiRestClient.fetchMedia(payload.site, payload.media);
1014-
} else if (payload.site.isUsingWpComRestApi()) {
1015-
mMediaRestClient.fetchMedia(payload.site, payload.media);
1016-
} else if (payload.site.isJetpackCPConnected()) {
1017-
mWPComV2MediaRestClient.fetchMedia(payload.site, payload.media);
1018-
} else {
1019-
mMediaXmlrpcClient.fetchMedia(payload.site, payload.media);
1065+
switch (getMediaRestClientType(payload.site)) {
1066+
case WPCOM_REST:
1067+
mMediaRestClient.fetchMedia(payload.site, payload.media);
1068+
break;
1069+
case SELF_HOSTED_RS:
1070+
mMediaRSApiRestClient.fetchMedia(payload.site, payload.media);
1071+
break;
1072+
case JETPACK_CP:
1073+
mWPComV2MediaRestClient.fetchMedia(payload.site, payload.media);
1074+
break;
1075+
default:
1076+
mMediaXmlrpcClient.fetchMedia(payload.site, payload.media);
1077+
break;
10201078
}
10211079
}
10221080

@@ -1026,12 +1084,16 @@ private void performDeleteMedia(@NonNull MediaPayload payload) {
10261084
return;
10271085
}
10281086

1029-
if (payload.site.isUsingSelfHostedRestApi()) {
1030-
mMediaRSApiRestClient.deleteMedia(payload.site, payload.media);
1031-
} else if (payload.site.isUsingWpComRestApi()) {
1032-
mMediaRestClient.deleteMedia(payload.site, payload.media);
1033-
} else {
1034-
mMediaXmlrpcClient.deleteMedia(payload.site, payload.media);
1087+
switch (getMediaRestClientType(payload.site)) {
1088+
case WPCOM_REST:
1089+
mMediaRestClient.deleteMedia(payload.site, payload.media);
1090+
break;
1091+
case SELF_HOSTED_RS:
1092+
mMediaRSApiRestClient.deleteMedia(payload.site, payload.media);
1093+
break;
1094+
default:
1095+
mMediaXmlrpcClient.deleteMedia(payload.site, payload.media);
1096+
break;
10351097
}
10361098
}
10371099

@@ -1044,17 +1106,22 @@ private void performCancelUpload(@NonNull CancelMediaPayload payload) {
10441106
MediaSqlUtils.insertOrUpdateMedia(media);
10451107
}
10461108

1047-
if (payload.site.isUsingSelfHostedRestApi()) {
1048-
mMediaRSApiRestClient.cancelUpload(payload.media);
1049-
} else if (payload.site.isUsingWpComRestApi()) {
1050-
mMediaRestClient.cancelUpload(media);
1051-
} else if (payload.site.isJetpackCPConnected()) {
1052-
mWPComV2MediaRestClient.cancelUpload(media);
1053-
} else if (payload.site.getOrigin() == SiteModel.ORIGIN_WPAPI
1054-
&& mApplicationPasswordsConfiguration.isEnabled()) {
1055-
mApplicationPasswordsMediaRestClient.cancelUpload(media);
1056-
} else {
1057-
mMediaXmlrpcClient.cancelUpload(media);
1109+
switch (getMediaRestClientType(payload.site)) {
1110+
case WPCOM_REST:
1111+
mMediaRestClient.cancelUpload(media);
1112+
break;
1113+
case SELF_HOSTED_RS:
1114+
mMediaRSApiRestClient.cancelUpload(payload.media);
1115+
break;
1116+
case JETPACK_CP:
1117+
mWPComV2MediaRestClient.cancelUpload(media);
1118+
break;
1119+
case APPLICATION_PASSWORDS:
1120+
mApplicationPasswordsMediaRestClient.cancelUpload(media);
1121+
break;
1122+
default:
1123+
mMediaXmlrpcClient.cancelUpload(media);
1124+
break;
10581125
}
10591126
}
10601127

libs/fluxc/src/test/java/org/wordpress/android/fluxc/media/MediaStoreTest.java

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -802,6 +802,61 @@ public void testMediaErrorTypeFromHttpStatusCodeUnmappedIsGeneric() {
802802
assertEquals(MediaStore.MediaErrorType.GENERIC_ERROR, MediaStore.MediaErrorType.fromHttpStatusCode(600));
803803
}
804804

805+
// CMM-2114: a Jetpack-connected site can have Application Password credentials stored (making
806+
// isUsingSelfHostedRestApi() true) but must still route media through the WP.com REST API, otherwise
807+
// uploads and the Media Library fail on sites that have Application Passwords disabled.
808+
@Test
809+
public void testJetpackConnectedSiteWithAppPasswordUsesWpComRestApiForMedia() {
810+
SiteModel site = new SiteModel();
811+
site.setIsWPCom(false);
812+
site.setIsJetpackConnected(true);
813+
site.setOrigin(SiteModel.ORIGIN_WPCOM_REST);
814+
site.setApiRestUsernamePlain("username");
815+
site.setApiRestPasswordPlain("app-password");
816+
817+
// Sanity check: this is exactly the ambiguous case where both predicates are true.
818+
assertTrue(site.isUsingWpComRestApi());
819+
assertTrue(site.isUsingSelfHostedRestApi());
820+
821+
assertEquals(MediaStore.MediaRestClientType.WPCOM_REST, mMediaStore.getMediaRestClientType(site));
822+
}
823+
824+
@Test
825+
public void testSelfHostedSiteWithAppPasswordUsesSelfHostedRestApiForMedia() {
826+
SiteModel site = new SiteModel();
827+
site.setIsWPCom(false);
828+
site.setIsJetpackConnected(false);
829+
site.setOrigin(SiteModel.ORIGIN_WPAPI);
830+
site.setApiRestUsernamePlain("username");
831+
site.setApiRestPasswordPlain("app-password");
832+
833+
assertFalse(site.isUsingWpComRestApi());
834+
assertTrue(site.isUsingSelfHostedRestApi());
835+
836+
assertEquals(MediaStore.MediaRestClientType.SELF_HOSTED_RS, mMediaStore.getMediaRestClientType(site));
837+
}
838+
839+
@Test
840+
public void testWpComSiteUsesWpComRestApiForMedia() {
841+
SiteModel site = new SiteModel();
842+
site.setIsWPCom(true);
843+
844+
assertEquals(MediaStore.MediaRestClientType.WPCOM_REST, mMediaStore.getMediaRestClientType(site));
845+
}
846+
847+
@Test
848+
public void testXmlRpcSiteUsesXmlRpcForMedia() {
849+
SiteModel site = new SiteModel();
850+
site.setIsWPCom(false);
851+
site.setIsJetpackConnected(false);
852+
site.setOrigin(SiteModel.ORIGIN_XMLRPC);
853+
854+
assertFalse(site.isUsingWpComRestApi());
855+
assertFalse(site.isUsingSelfHostedRestApi());
856+
857+
assertEquals(MediaStore.MediaRestClientType.XMLRPC, mMediaStore.getMediaRestClientType(site));
858+
}
859+
805860
private MediaModel getBasicMedia() {
806861
return generateMedia("Test Title", "Test Description", "Test Caption", "Test Alt");
807862
}

0 commit comments

Comments
 (0)