Skip to content

Commit e40404b

Browse files
angrymuesliTero Ripattila
angrymuesli
authored and
Tero Ripattila
committed
refactor: implement nextcloudtalk:// URI scheme per spreed#16354
Address maintainer feedback and improve code quality: - Replace nctalk:// with nextcloudtalk:// custom URI scheme - Remove HTTPS deep links (not feasible for self-hosted domains) - Add token validation (alphanumeric, 4-32 chars) - Fix server URL matching to use proper host comparison - Handle null user.id gracefully in ShortcutManagerHelper - Manage RxJava disposables to prevent memory leaks - Add lifecycle checks before UI operations - Fix generic exception handling URI format: nextcloudtalk://[user@]host/[base/]call/token
1 parent 30d91c2 commit e40404b

4 files changed

Lines changed: 232 additions & 241 deletions

File tree

‎app/src/main/AndroidManifest.xml‎

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -128,26 +128,7 @@
128128
<action android:name="android.intent.action.VIEW" />
129129
<category android:name="android.intent.category.DEFAULT" />
130130
<category android:name="android.intent.category.BROWSABLE" />
131-
<data android:scheme="nctalk" />
132-
</intent-filter>
133-
134-
<!-- HTTP/HTTPS deep links for opening Talk conversation links -->
135-
<intent-filter android:autoVerify="true">
136-
<action android:name="android.intent.action.VIEW" />
137-
<category android:name="android.intent.category.DEFAULT" />
138-
<category android:name="android.intent.category.BROWSABLE" />
139-
<data android:scheme="https" />
140-
<data android:host="*" />
141-
<data android:pathPrefix="/call/" />
142-
</intent-filter>
143-
144-
<intent-filter android:autoVerify="true">
145-
<action android:name="android.intent.action.VIEW" />
146-
<category android:name="android.intent.category.DEFAULT" />
147-
<category android:name="android.intent.category.BROWSABLE" />
148-
<data android:scheme="https" />
149-
<data android:host="*" />
150-
<data android:pathPrefix="/index.php/call/" />
131+
<data android:scheme="nextcloudtalk" />
151132
</intent-filter>
152133
</activity>
153134

‎app/src/main/java/com/nextcloud/talk/activities/MainActivity.kt‎

Lines changed: 113 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ package com.nextcloud.talk.activities
1111

1212
import android.app.KeyguardManager
1313
import android.content.Intent
14+
import android.net.Uri
1415
import android.os.Bundle
1516
import android.provider.ContactsContract
1617
import android.text.TextUtils
@@ -33,7 +34,6 @@ import com.nextcloud.talk.data.user.model.User
3334
import com.nextcloud.talk.databinding.ActivityMainBinding
3435
import com.nextcloud.talk.invitation.InvitationsActivity
3536
import com.nextcloud.talk.lock.LockedActivity
36-
import com.nextcloud.talk.models.json.conversations.RoomOverall
3737
import com.nextcloud.talk.users.UserManager
3838
import com.nextcloud.talk.utils.ApiUtils
3939
import com.nextcloud.talk.utils.ClosedInterfaceImpl
@@ -42,10 +42,8 @@ import com.nextcloud.talk.utils.SecurityUtils
4242
import com.nextcloud.talk.utils.ShortcutManagerHelper
4343
import com.nextcloud.talk.utils.bundle.BundleKeys
4444
import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_ROOM_TOKEN
45-
import io.reactivex.Observer
46-
import io.reactivex.SingleObserver
4745
import io.reactivex.android.schedulers.AndroidSchedulers
48-
import io.reactivex.disposables.Disposable
46+
import io.reactivex.disposables.CompositeDisposable
4947
import io.reactivex.schedulers.Schedulers
5048
import javax.inject.Inject
5149

@@ -62,6 +60,8 @@ class MainActivity :
6260
@Inject
6361
lateinit var userManager: UserManager
6462

63+
private val disposables = CompositeDisposable()
64+
6565
private val onBackPressedCallback = object : OnBackPressedCallback(true) {
6666
override fun handleOnBackPressed() {
6767
finish()
@@ -93,6 +93,11 @@ class MainActivity :
9393
onBackPressedDispatcher.addCallback(this, onBackPressedCallback)
9494
}
9595

96+
override fun onDestroy() {
97+
super.onDestroy()
98+
disposables.dispose()
99+
}
100+
96101
fun lockScreenIfConditionsApply() {
97102
val keyguardManager = getSystemService(KEYGUARD_SERVICE) as KeyguardManager
98103
if (keyguardManager.isKeyguardSecure && appPreferences.isScreenLocked) {
@@ -168,7 +173,8 @@ class MainActivity :
168173
val user = userId.substringBeforeLast("@")
169174
val baseUrl = userId.substringAfterLast("@")
170175

171-
if (currentUserProviderOld.currentUser.blockingGet()?.baseUrl!!.endsWith(baseUrl) == true) {
176+
val currentUser = currentUserProviderOld.currentUser.blockingGet()
177+
if (currentUser?.baseUrl?.endsWith(baseUrl) == true) {
172178
startConversation(user)
173179
} else {
174180
Snackbar.make(
@@ -196,35 +202,28 @@ class MainActivity :
196202
invite = userId
197203
)
198204

199-
ncApi.createRoom(
205+
val disposable = ncApi.createRoom(
200206
credentials,
201207
retrofitBucket.url,
202208
retrofitBucket.queryMap
203209
)
204210
.subscribeOn(Schedulers.io())
205211
.observeOn(AndroidSchedulers.mainThread())
206-
.subscribe(object : Observer<RoomOverall> {
207-
override fun onSubscribe(d: Disposable) {
208-
// unused atm
209-
}
210-
211-
override fun onNext(roomOverall: RoomOverall) {
212+
.subscribe(
213+
{ roomOverall ->
214+
if (isFinishing || isDestroyed) return@subscribe
212215
val bundle = Bundle()
213216
bundle.putString(KEY_ROOM_TOKEN, roomOverall.ocs!!.data!!.token)
214217

215218
val chatIntent = Intent(context, ChatActivity::class.java)
216219
chatIntent.putExtras(bundle)
217220
startActivity(chatIntent)
221+
},
222+
{ e ->
223+
Log.e(TAG, "Error creating room", e)
218224
}
219-
220-
override fun onError(e: Throwable) {
221-
// unused atm
222-
}
223-
224-
override fun onComplete() {
225-
// unused atm
226-
}
227-
})
225+
)
226+
disposables.add(disposable)
228227
}
229228

230229
override fun onNewIntent(intent: Intent) {
@@ -234,7 +233,7 @@ class MainActivity :
234233
}
235234

236235
private fun handleIntent(intent: Intent) {
237-
// Handle deep links first (nctalk:// scheme and https:// web links)
236+
// Handle deep links first (nextcloudtalk:// scheme)
238237
if (handleDeepLink(intent)) {
239238
return
240239
}
@@ -244,7 +243,7 @@ class MainActivity :
244243
val internalUserId = intent.extras?.getLong(BundleKeys.KEY_INTERNAL_USER_ID)
245244

246245
var user: User? = null
247-
if (internalUserId != null) {
246+
if (internalUserId != null && internalUserId != 0L) {
248247
user = userManager.getUserWithId(internalUserId).blockingGet()
249248
}
250249

@@ -260,43 +259,38 @@ class MainActivity :
260259
startActivity(chatIntent)
261260
}
262261
} else {
263-
userManager.users.subscribe(object : SingleObserver<List<User>> {
264-
override fun onSubscribe(d: Disposable) {
265-
// unused atm
266-
}
267-
268-
override fun onSuccess(users: List<User>) {
269-
if (users.isNotEmpty()) {
270-
ClosedInterfaceImpl().setUpPushTokenRegistration()
271-
runOnUiThread {
262+
val disposable = userManager.users
263+
.subscribeOn(Schedulers.io())
264+
.observeOn(AndroidSchedulers.mainThread())
265+
.subscribe(
266+
{ users ->
267+
if (isFinishing || isDestroyed) return@subscribe
268+
if (users.isNotEmpty()) {
269+
ClosedInterfaceImpl().setUpPushTokenRegistration()
272270
openConversationList()
273-
}
274-
} else {
275-
runOnUiThread {
271+
} else {
276272
launchServerSelection()
277273
}
274+
},
275+
{ e ->
276+
Log.e(TAG, "Error loading existing users", e)
277+
if (isFinishing || isDestroyed) return@subscribe
278+
Toast.makeText(
279+
context,
280+
context.resources.getString(R.string.nc_common_error_sorry),
281+
Toast.LENGTH_SHORT
282+
).show()
278283
}
279-
}
280-
281-
override fun onError(e: Throwable) {
282-
Log.e(TAG, "Error loading existing users", e)
283-
Toast.makeText(
284-
context,
285-
context.resources.getString(R.string.nc_common_error_sorry),
286-
Toast.LENGTH_SHORT
287-
).show()
288-
}
289-
})
284+
)
285+
disposables.add(disposable)
290286
}
291287
}
292288

293289
/**
294290
* Handles deep link URIs for opening conversations.
295291
*
296292
* Supports:
297-
* - nctalk://conversation/{token}?user={userId}
298-
* - https://{server}/call/{token}
299-
* - https://{server}/index.php/call/{token}
293+
* - nextcloudtalk://[user@]server/call/token
300294
*
301295
* @param intent The intent to process
302296
* @return true if the intent was handled as a deep link, false otherwise
@@ -305,63 +299,67 @@ class MainActivity :
305299
val uri = intent.data ?: return false
306300
val deepLinkResult = DeepLinkHandler.parseDeepLink(uri) ?: return false
307301

308-
Log.d(TAG, "Handling deep link: $uri -> token=${deepLinkResult.roomToken}")
302+
Log.d(TAG, "Handling deep link: token=${deepLinkResult.roomToken}, server=${deepLinkResult.serverUrl}")
309303

310-
userManager.users.subscribe(object : SingleObserver<List<User>> {
311-
override fun onSubscribe(d: Disposable) {
312-
// unused atm
313-
}
304+
val disposable = userManager.users
305+
.subscribeOn(Schedulers.io())
306+
.observeOn(AndroidSchedulers.mainThread())
307+
.subscribe(
308+
{ users ->
309+
if (isFinishing || isDestroyed) return@subscribe
314310

315-
override fun onSuccess(users: List<User>) {
316-
if (users.isEmpty()) {
317-
runOnUiThread {
311+
if (users.isEmpty()) {
318312
launchServerSelection()
313+
return@subscribe
319314
}
320-
return
321-
}
322315

323-
val targetUser = resolveTargetUser(users, deepLinkResult)
316+
val targetUser = resolveTargetUser(users, deepLinkResult)
324317

325-
if (targetUser == null) {
326-
runOnUiThread {
318+
if (targetUser == null) {
327319
Toast.makeText(
328320
context,
329321
context.resources.getString(R.string.nc_no_account_for_server),
330322
Toast.LENGTH_LONG
331323
).show()
332324
openConversationList()
325+
return@subscribe
333326
}
334-
return
335-
}
336327

337-
if (userManager.setUserAsActive(targetUser).blockingGet()) {
338-
// Report shortcut usage for ranking
339-
ShortcutManagerHelper.reportShortcutUsed(
340-
context,
341-
deepLinkResult.roomToken,
342-
targetUser.id!!
343-
)
328+
if (userManager.setUserAsActive(targetUser).blockingGet()) {
329+
// Report shortcut usage for ranking
330+
targetUser.id?.let { userId ->
331+
ShortcutManagerHelper.reportShortcutUsed(
332+
context,
333+
deepLinkResult.roomToken,
334+
userId
335+
)
336+
}
337+
338+
if (isFinishing || isDestroyed) return@subscribe
344339

345-
runOnUiThread {
346340
val chatIntent = Intent(context, ChatActivity::class.java)
347341
chatIntent.putExtra(KEY_ROOM_TOKEN, deepLinkResult.roomToken)
348342
chatIntent.putExtra(BundleKeys.KEY_INTERNAL_USER_ID, targetUser.id)
349343
startActivity(chatIntent)
344+
} else {
345+
Toast.makeText(
346+
context,
347+
context.resources.getString(R.string.nc_common_error_sorry),
348+
Toast.LENGTH_SHORT
349+
).show()
350350
}
351-
}
352-
}
353-
354-
override fun onError(e: Throwable) {
355-
Log.e(TAG, "Error loading users for deep link", e)
356-
runOnUiThread {
351+
},
352+
{ e ->
353+
Log.e(TAG, "Error loading users for deep link", e)
354+
if (isFinishing || isDestroyed) return@subscribe
357355
Toast.makeText(
358356
context,
359357
context.resources.getString(R.string.nc_common_error_sorry),
360358
Toast.LENGTH_SHORT
361359
).show()
362360
}
363-
}
364-
})
361+
)
362+
disposables.add(disposable)
365363

366364
return true
367365
}
@@ -370,31 +368,48 @@ class MainActivity :
370368
* Resolves which user account to use for a deep link.
371369
*
372370
* Priority:
373-
* 1. User ID specified in deep link (for nctalk:// URIs)
374-
* 2. User matching the server URL (for https:// web links)
375-
* 3. Current active user as fallback
371+
* 1. User matching both username and server URL
372+
* 2. User matching the server URL only
373+
* 3. Current active user as fallback (if server matches)
376374
*/
377-
private fun resolveTargetUser(
378-
users: List<User>,
379-
deepLinkResult: DeepLinkHandler.DeepLinkResult
380-
): User? {
381-
// If user ID is specified, use that user
382-
deepLinkResult.internalUserId?.let { userId ->
383-
return userManager.getUserWithId(userId).blockingGet()
375+
private fun resolveTargetUser(users: List<User>, deepLinkResult: DeepLinkHandler.DeepLinkResult): User? {
376+
val serverUrl = deepLinkResult.serverUrl
377+
val username = deepLinkResult.username
378+
379+
// Extract host from the deep link server URL for comparison
380+
val deepLinkHost = Uri.parse(serverUrl).host?.lowercase()
381+
if (deepLinkHost.isNullOrBlank()) {
382+
return currentUserProviderOld.currentUser.blockingGet()
384383
}
385384

386-
// If server URL is specified, find matching account
387-
deepLinkResult.serverUrl?.let { serverUrl ->
388-
val matchingUser = users.find { user ->
389-
user.baseUrl?.lowercase()?.contains(serverUrl.lowercase()) == true
385+
// If username is specified, try to find exact match (username + server)
386+
if (username != null) {
387+
val exactMatch = users.find { user ->
388+
val userHost = user.baseUrl?.let { Uri.parse(it).host?.lowercase() }
389+
userHost == deepLinkHost && user.username?.lowercase() == username.lowercase()
390390
}
391-
if (matchingUser != null) {
392-
return matchingUser
391+
if (exactMatch != null) {
392+
return exactMatch
393393
}
394394
}
395395

396-
// Fall back to current user
397-
return currentUserProviderOld.currentUser.blockingGet()
396+
// Find user matching the server URL (host match)
397+
val matchingUser = users.find { user ->
398+
val userHost = user.baseUrl?.let { Uri.parse(it).host?.lowercase() }
399+
userHost == deepLinkHost
400+
}
401+
if (matchingUser != null) {
402+
return matchingUser
403+
}
404+
405+
// Fall back to current user only if their server matches
406+
val currentUser = currentUserProviderOld.currentUser.blockingGet()
407+
val currentUserHost = currentUser?.baseUrl?.let { Uri.parse(it).host?.lowercase() }
408+
if (currentUserHost == deepLinkHost) {
409+
return currentUser
410+
}
411+
412+
return null
398413
}
399414

400415
companion object {

0 commit comments

Comments
 (0)