From 5ed83c181fbc2b82d5ef5e24e5bd96a13599fada Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Mon, 15 Apr 2024 16:25:58 +0600 Subject: [PATCH 1/2] feat: pass OIDC-LOGIN-WITH-TOKEN header if required for nextcloud app api request with OIDC specific header is required, but this header should not be passed for dav/ocs requests. issue: https://gitlab.e.foundation/e/backlog/-/issues/6287 --- app/build.gradle | 2 +- .../android/sso/InputStreamBinder.java | 44 ++++++++++++------- .../android/utils/AccountManagerUtils.java | 8 ++++ 3 files changed, 38 insertions(+), 16 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index 5ea75c8c4..80d92b89f 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -224,7 +224,7 @@ dependencies { implementation "commons-httpclient:commons-httpclient:3.1@jar" // remove after entire switch to lib v2 implementation 'org.apache.jackrabbit:jackrabbit-webdav:2.13.5' // remove after entire switch to lib v2 implementation 'com.google.code.gson:gson:2.10.1' - implementation("foundation.e:Nextcloud-Android-Library:1.0.7-u2.17-release") { + implementation("foundation.e:Nextcloud-Android-Library:1.0.8-u2.17-release") { exclude group: 'com.gitlab.bitfireAT', module: 'dav4jvm' exclude group: 'org.ogce', module: 'xpp3' // unused in Android and brings wrong Junit version exclude group: 'com.squareup.okhttp3' diff --git a/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java b/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java index 219123eed..4d8362188 100644 --- a/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java +++ b/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java @@ -35,6 +35,7 @@ import android.os.Binder; import android.os.ParcelFileDescriptor; import android.text.TextUtils; +import androidx.annotation.NonNull; import androidx.annotation.VisibleForTesting; import com.nextcloud.android.sso.aidl.IInputStreamService; @@ -46,6 +47,7 @@ import com.owncloud.android.lib.common.OwnCloudAccount; import com.owncloud.android.lib.common.OwnCloudClient; import com.owncloud.android.lib.common.OwnCloudClientManager; import com.owncloud.android.lib.common.OwnCloudClientManagerFactory; +import com.owncloud.android.lib.common.operations.RemoteOperation; import org.apache.commons.httpclient.HttpConnection; import org.apache.commons.httpclient.HttpMethodBase; @@ -328,7 +330,7 @@ public class InputStreamBinder extends IInputStreamService.Stub { } // Validate URL - if (request.getUrl().length() == 0 || request.getUrl().charAt(0) != PATH_SEPARATOR) { + if (request.getUrl().isEmpty() || request.getUrl().charAt(0) != PATH_SEPARATOR) { throw new IllegalStateException(EXCEPTION_INVALID_REQUEST_URL, new IllegalStateException("URL need to start with a /")); } @@ -345,17 +347,16 @@ public class InputStreamBinder extends IInputStreamService.Stub { } else { method.setQueryString(convertMapToNVP(request.getParameter())); } - method.addRequestHeader("OCS-APIREQUEST", "true"); for (Map.Entry> header : request.getHeader().entrySet()) { // https://stackoverflow.com/a/3097052 method.addRequestHeader(header.getKey(), TextUtils.join(",", header.getValue())); + } - if ("OCS-APIREQUEST".equalsIgnoreCase(header.getKey())) { - throw new IllegalStateException( - "The 'OCS-APIREQUEST' header will be automatically added by the Nextcloud SSO Library. " + - "Please remove the header before making a request"); - } + method.setRequestHeader(RemoteOperation.OCS_API_HEADER, RemoteOperation.OCS_API_HEADER_VALUE); + + if (shouldAddHeaderForOidcLogin(context, account, request.getUrl())) { + method.setRequestHeader(RemoteOperation.OIDC_LOGIN_WITH_TOKEN, RemoteOperation.OIDC_LOGIN_WITH_TOKEN_VALUE); } client.setFollowRedirects(true); @@ -383,6 +384,21 @@ public class InputStreamBinder extends IInputStreamService.Stub { } } + /* + * for non ocs/dav requests (nextcloud app: ex: notes app), when OIDC is used, we need to pass an special header. + * We should not pass this header for ocs/dav requests as it can cause session cookie not being used for those request. + * + * These nextcloud app request paths contain `/index.php/apps/` on them. + */ + private boolean shouldAddHeaderForOidcLogin(@NonNull Context context, @NonNull Account account, @NonNull String path) { + boolean isOidcAccount = AccountManagerUtils.isOidcAccount(context, account); + if (!isOidcAccount) { + return false; + } + + return path.contains("/index.php/apps/"); + } + private static String getUserAgent() { return "AccountManager-SSO(" + BuildConfig.VERSION_NAME + ")"; } @@ -402,7 +418,7 @@ public class InputStreamBinder extends IInputStreamService.Stub { } // Validate URL - if (request.getUrl().length() == 0 || request.getUrl().charAt(0) != PATH_SEPARATOR) { + if (request.getUrl().isEmpty() || request.getUrl().charAt(0) != PATH_SEPARATOR) { throw new IllegalStateException(EXCEPTION_INVALID_REQUEST_URL, new IllegalStateException("URL need to start with a /")); } @@ -420,17 +436,15 @@ public class InputStreamBinder extends IInputStreamService.Stub { method.setQueryString(convertMapToNVP(request.getParameter())); } - method.addRequestHeader("OCS-APIREQUEST", "true"); - for (Map.Entry> header : request.getHeader().entrySet()) { // https://stackoverflow.com/a/3097052 method.addRequestHeader(header.getKey(), TextUtils.join(",", header.getValue())); + } - if ("OCS-APIREQUEST".equalsIgnoreCase(header.getKey())) { - throw new IllegalStateException( - "The 'OCS-APIREQUEST' header will be automatically added by the Nextcloud SSO Library. " + - "Please remove the header before making a request"); - } + method.setRequestHeader(RemoteOperation.OCS_API_HEADER, RemoteOperation.OCS_API_HEADER_VALUE); + + if (shouldAddHeaderForOidcLogin(context, account, request.getUrl())) { + method.setRequestHeader(RemoteOperation.OIDC_LOGIN_WITH_TOKEN, RemoteOperation.OIDC_LOGIN_WITH_TOKEN_VALUE); } client.setFollowRedirects(true); diff --git a/app/src/main/java/com/nextcloud/android/utils/AccountManagerUtils.java b/app/src/main/java/com/nextcloud/android/utils/AccountManagerUtils.java index bf05b5e7b..faf5ff7a1 100644 --- a/app/src/main/java/com/nextcloud/android/utils/AccountManagerUtils.java +++ b/app/src/main/java/com/nextcloud/android/utils/AccountManagerUtils.java @@ -25,6 +25,7 @@ import androidx.annotation.Nullable; import at.bitfire.davdroid.Constants; import at.bitfire.davdroid.R; +import at.bitfire.davdroid.settings.AccountSettings; public final class AccountManagerUtils { @@ -43,6 +44,13 @@ public final class AccountManagerUtils { return null; } + @Nullable + public static boolean isOidcAccount(@NonNull Context context, @NonNull Account account) { + AccountManager accountManager = AccountManager.get(context); + String authState = accountManager.getUserData(account, AccountSettings.KEY_AUTH_STATE); + return authState != null && !authState.trim().isEmpty(); + } + @NonNull public static Account[] getAccounts(@NonNull Context context) { AccountManager accountManager = AccountManager.get(context); -- GitLab From 0c32f8ebf0442b4fb006090d8a3c3f53cab1462e Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Mon, 15 Apr 2024 17:11:24 +0600 Subject: [PATCH 2/2] chore: update according to review --- .../android/sso/InputStreamBinder.java | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java b/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java index 4d8362188..7d572d3b7 100644 --- a/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java +++ b/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java @@ -353,10 +353,16 @@ public class InputStreamBinder extends IInputStreamService.Stub { method.addRequestHeader(header.getKey(), TextUtils.join(",", header.getValue())); } - method.setRequestHeader(RemoteOperation.OCS_API_HEADER, RemoteOperation.OCS_API_HEADER_VALUE); + method.setRequestHeader( + RemoteOperation.OCS_API_HEADER, + RemoteOperation.OCS_API_HEADER_VALUE + ); if (shouldAddHeaderForOidcLogin(context, account, request.getUrl())) { - method.setRequestHeader(RemoteOperation.OIDC_LOGIN_WITH_TOKEN, RemoteOperation.OIDC_LOGIN_WITH_TOKEN_VALUE); + method.setRequestHeader( + RemoteOperation.OIDC_LOGIN_WITH_TOKEN, + RemoteOperation.OIDC_LOGIN_WITH_TOKEN_VALUE + ); } client.setFollowRedirects(true); @@ -441,10 +447,16 @@ public class InputStreamBinder extends IInputStreamService.Stub { method.addRequestHeader(header.getKey(), TextUtils.join(",", header.getValue())); } - method.setRequestHeader(RemoteOperation.OCS_API_HEADER, RemoteOperation.OCS_API_HEADER_VALUE); + method.setRequestHeader( + RemoteOperation.OCS_API_HEADER, + RemoteOperation.OCS_API_HEADER_VALUE + ); if (shouldAddHeaderForOidcLogin(context, account, request.getUrl())) { - method.setRequestHeader(RemoteOperation.OIDC_LOGIN_WITH_TOKEN, RemoteOperation.OIDC_LOGIN_WITH_TOKEN_VALUE); + method.setRequestHeader( + RemoteOperation.OIDC_LOGIN_WITH_TOKEN, + RemoteOperation.OIDC_LOGIN_WITH_TOKEN_VALUE + ); } client.setFollowRedirects(true); -- GitLab