app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/ScopeAuthorization.kt (2)

12-14: Basic-auth users bypass all scope checks.

UserIdPrincipal presence grants full access regardless of scopes. This is presumably intentional (basic auth = admin), but it's worth documenting this design decision explicitly — either as a code comment or in the auth architecture docs — so future contributors don't accidentally rely on scope enforcement for basic-auth sessions.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/ScopeAuthorization.kt`
around lines 12 - 14, The code in ScopeAuthorization.kt currently treats
presence of UserIdPrincipal (call.principal<UserIdPrincipal>()) as a full-access
bypass of scope checks; add an explicit code comment above that conditional
and/or update the module/auth docs to state that basic-auth (UserIdPrincipal) is
intentionally considered admin-level and therefore bypasses scope enforcement,
citing security rationale and any intended exceptions; ensure the comment
references the UserIdPrincipal check and ScopeAuthorization so future
contributors won’t assume scope checks apply to basic-auth sessions.

---

11-34: Callers must remember to check the return value — easy to misuse.

requireScope returns Boolean but also writes the response (401/403). If a route handler calls requireScope(...) without checking the return and short-circuiting (e.g., if (!requireScope(scope)) return@get), the handler will continue executing past the authorization check while the response status is already set.

This is a known ergonomic pitfall with the "return Boolean + respond inside" pattern. A safer alternative is to throw an exception (e.g., a custom AuthorizationException) and let the status-pages plugin handle it, which makes it impossible to forget the check.

♻️ Safer alternative using exceptions

class InsufficientScopeException(message: String) : Exception(message)

suspend fun PipelineContext<Unit, ApplicationCall>.requireScope(scope: String) {
    if (call.principal<UserIdPrincipal>() != null) return

    val jwtPrincipal = call.principal<JWTPrincipal>()
        ?: throw AuthenticationException("Unauthorized")

    val scopes = jwtPrincipal.payload
        .getClaim("scopes")
        .asList(String::class.java)
        ?.map { it.trim() }
        ?.filter { it.isNotEmpty() }
        ?: emptyList()

    if (AuthScopes.ALL_ANY in scopes || scope in scopes) return

    throw InsufficientScopeException("Insufficient permissions")
}

Then in your StatusPages configuration, map these exceptions to the appropriate HTTP responses.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/ScopeAuthorization.kt`
around lines 11 - 34, The requireScope function currently returns Boolean and
calls call.respond on failure which is easy to misuse; change requireScope to be
void (suspend fun requireScope(scope: String)) and throw explicit exceptions
instead of responding: throw an AuthenticationException (or custom
AuthenticationException) when JWTPrincipal is missing/unauthorized and throw a
custom InsufficientScopeException when scopes lack the required
AuthScopes.ALL_ANY or the requested scope; update any callers to stop checking
booleans (they'll rely on StatusPages to map AuthenticationException and
InsufficientScopeException to 401/403) and keep the JWT parsing/scopes
extraction logic inside requireScope.

app/src/main/res/xml/local_server_preferences.xml (1)

46-52: The default scopes string and the AuthScopes.allowed set must stay in sync.

The app:defaultValue on line 50 enumerates all scopes except all:any. If AuthScopes gains new scopes in the future, this default value won't automatically include them. This is related to the same maintenance concern flagged for jwt_available_scopes_values in strings.xml. Consider noting this coupling with a comment.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/res/xml/local_server_preferences.xml` around lines 46 - 52, The
default scopes string in the EditTextPreference
(app:key="localserver.JWT_DEFAULT_SCOPES", app:defaultValue) is coupled to the
AuthScopes.allowed set and jwt_available_scopes_values; add a concise in-file
comment near the EditTextPreference explaining that the defaultValue must be
kept in sync with AuthScopes.allowed and jwt_available_scopes_values (or ideally
derive the default programmatically), and if you choose to keep it static,
update jwt_default_scopes whenever AuthScopes.allowed or
jwt_available_scopes_values changes to avoid drift.

app/src/main/java/me/capcom/smsgateway/ui/settings/LocalServerSettingsFragment.kt (1)

20-21: JWT secret displayed in plaintext in the preferences UI.

settings.jwtSecret is shown as the preference summary, making it visible to anyone who opens the settings screen. This is consistent with the existing pattern for credentials in this app, but be aware that the JWT secret is the signing key — if leaked, any party can forge valid tokens. Consider whether masking with a "tap to reveal" pattern would be more appropriate, since the enableCopying attribute on the XML side already allows clipboard access.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/me/capcom/smsgateway/ui/settings/LocalServerSettingsFragment.kt`
around lines 20 - 21, The preference currently sets the JWT signing key directly
into the UI via findPreference<Preference>("transient.jwt_secret")?.summary =
settings.jwtSecret which exposes the secret in plaintext; instead, replace the
direct summary assignment with a masked representation (e.g. fixed bullets or
obscured length) and attach a click/tap handler on the same Preference to toggle
a "tap to reveal" state that temporarily shows settings.jwtSecret only while
revealed (and respects the existing enableCopying XML attribute for clipboard
behavior). Update LocalServerSettingsFragment to store the reveal state, set the
masked summary by default, and on tap switch between masked and real summary,
ensuring the real secret is not set into any persistent UI element outside the
temporary reveal action.

app/src/main/res/values/strings.xml (1)

158-158: Hardcoded scope list duplicates AuthScopes.allowed — maintenance risk.

The jwt_available_scopes_values string manually lists every scope. If AuthScopes.allowed is updated (e.g., a new scope is added), this string must be updated in lockstep or the UI will show stale information. Consider populating this value programmatically in the fragment instead.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/res/values/strings.xml` at line 158, The string resource
jwt_available_scopes_values hardcodes the scope list and duplicates
AuthScopes.allowed, risking stale UI; remove the hardcoded list and instead
build the display value at runtime from AuthScopes.allowed in the fragment that
shows these scopes (e.g., construct a comma-separated string from
AuthScopes.allowed and set it to the TextView/Preference summary where
jwt_available_scopes_values was used), so updates to AuthScopes.allowed
automatically reflect in the UI.

app/src/main/java/me/capcom/smsgateway/data/Migrations.kt (1)

88-100: Migration is correct. Consider a cleanup strategy for unbounded token accumulation.

The DDL matches the RevokedToken entity schema. However, revoked tokens are inserted but never deleted, so this table will grow indefinitely. Once a token's TTL has expired, its revocation record is no longer needed. Consider adding a periodic cleanup (e.g., delete rows where revokedAt + maxTTL < now) to prevent unbounded growth.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/me/capcom/smsgateway/data/Migrations.kt` around lines 88 -
100, The migration (MIGRATION_17_18) correctly creates the revoked_token table
but there is no cleanup for expired revocation records; add a periodic purge so
the table doesn't grow unbounded: implement a DAO method (e.g.,
RevokedTokenDao.deleteExpired(revokedBefore: Long) or
deleteRevokedOlderThan(maxAgeMillis: Long)) and call it from a scheduled task
(WorkManager/JobScheduler) or during token validation flows to run e.g., DELETE
FROM revoked_token WHERE revokedAt + :maxTTL < :now, and wire this into your app
startup or background worker to run regularly; reference RevokedToken entity,
RevokedTokenDao, and MIGRATION_17_18 when adding the cleanup.

app/src/main/java/me/capcom/smsgateway/modules/localserver/LocalServerSettings.kt (1)

50-53: Hardcoded JWT default scopes duplicates AuthScopes definitions and risks drift.

The default scope string in the getter hardcodes scope values that are already defined as constants in AuthScopes (lines 6–14 in AuthScopes.kt). If a scope is added, renamed, or removed in AuthScopes, this hardcoded string won't automatically stay in sync. Consider constructing this default from AuthScopes constants (e.g., by joining specific scope constants or a filtered subset of AuthScopes.allowed) to maintain a single source of truth.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/LocalServerSettings.kt`
around lines 50 - 53, Replace the hardcoded default string in the
jwtDefaultScopes getter with a programmatic construction based on AuthScopes
constants to avoid drift: when storage.get(JWT_DEFAULT_SCOPES) is null, build
the default by joining the appropriate AuthScopes values (e.g., specific
constants or AuthScopes.allowed filtered to the subset used here) into a
comma-separated string and return that; keep the setter as
storage.set(JWT_DEFAULT_SCOPES, value). Ensure you reference the AuthScopes
members (the named scope constants or the allowed collection) so any future
changes to AuthScopes are reflected automatically.

app/src/main/java/me/capcom/smsgateway/data/entities/RevokedToken.kt (1)

6-11: Consider adding a cleanup strategy for revoked tokens.

The revoked_token table will grow indefinitely as tokens are revoked. Since JWTs have an expiration (expiresAt), revoked entries older than the max TTL are unnecessary — the JWT itself would be rejected by the verifier. Consider a periodic purge of entries where revokedAt + maxTtl < now, or storing expiresAt alongside revokedAt to make cleanup straightforward.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/me/capcom/smsgateway/data/entities/RevokedToken.kt` around
lines 6 - 11, The RevokedToken entity and DB need a cleanup strategy to prevent
unbounded growth: add an expiresAt: Long property to the RevokedToken data class
(in addition to revokedAt) and update any creation path to set expiresAt to the
JWT expiry; then implement a periodic purge routine (e.g., a scheduled job or
DAO method like purgeRevokedTokensBefore(cutoff: Long)) that deletes rows where
expiresAt < now (or revokedAt + maxTtl < now if you prefer not to store
expiresAt). Ensure the DAO/repository exposing
purgeRevokedTokensBefore/cleanupRevokedTokens is called by your app
scheduler/startup task to run at a suitable interval.

app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.kt (1)

29-33: parseCsv does not deduplicate scopes.

If a caller (or settings.jwtDefaultScopes) contains "all:any,all:any", the returned list has duplicates that propagate into the JWT scopes claim, producing a non-standard payload.

♻️ Proposed fix

 fun parseCsv(value: String): List<String> {
     return value.split(',')
         .map { it.trim() }
         .filter { it.isNotEmpty() }
+        .distinct()
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.kt`
around lines 29 - 33, parseCsv currently returns duplicate entries (e.g.,
"all:any,all:any") which flow into JWT scopes; update parseCsv to remove
duplicates while preserving order by applying a distincting step (e.g., call
distinct() on the sequence after map/filter) so it returns a de-duplicated
List<String> for use in JWT scopes (affects parseCsv and any callers like
settings.jwtDefaultScopes usage).

app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/JwtService.kt (1)

38-47: safeMaxTtlSeconds check and Math.multiplyExact are unreachable — optional cleanup.

MAX_TTL_SECONDS = 31_536_000. Its product 31_536_000 × 1_000 = 31.5 × 10⁹ is far below Long.MAX_VALUE ≈ 9.2 × 10¹⁸, so the overflow path at line 40–41 and the ArithmeticException catch at line 45 can never be reached once ttl <= MAX_TTL_SECONDS is enforced. They're defensive but add noise.

♻️ Proposed simplification

-        val safeMaxTtlSeconds = (Long.MAX_VALUE - now.time) / 1000
-        require(ttl <= safeMaxTtlSeconds) { "ttl is too large for current time" }
-
-        val ttlMillis = try {
-            Math.multiplyExact(ttl, 1000)
-        } catch (_: ArithmeticException) {
-            throw IllegalArgumentException("ttl value overflow")
-        }
+        val ttlMillis = ttl * 1000

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/JwtService.kt`
around lines 38 - 47, The safeMaxTtlSeconds check and the Math.multiplyExact
overflow handling are unreachable once you already enforce require(ttl <=
MAX_TTL_SECONDS); remove the safeMaxTtlSeconds calculation and its require, and
simplify ttlMillis by computing ttl * 1000L directly (remove the try/catch
around Math.multiplyExact). Update the code in JwtService where MAX_TTL_SECONDS,
safeMaxTtlSeconds, and Math.multiplyExact are referenced so only the initial ttl
bound check and a plain multiplication to produce ttlMillis remain.

app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.kt (1)

1-30: LGTM. Consider adding a token-management scope.

The scope set is clean and consistent. One design note: the /auth/token endpoint (inside the authenticate block in WebService) allows any JWT holder to generate new tokens regardless of what scopes they carry, since AuthRoutes receives no scope guard. Adding a dedicated scope (e.g., TOKENS_WRITE) and a requireScope check in AuthRoutes would prevent lower-privilege tokens from minting new ones.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.kt`
around lines 1 - 30, Add a dedicated token-management scope and require it in
the token issuance route: add a new constant TOKENS_WRITE (e.g., "tokens:write")
to the AuthScopes object and include it in the allowed set, then update the
/auth/token handler in AuthRoutes (inside the authenticate block in WebService)
to call the existing requireScope function with AuthScopes.TOKENS_WRITE before
minting/issuing new tokens so only principals holding that scope can create
tokens.

app/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.kt (1)

61-61: JwtService is manually constructed outside Koin's DI graph.

JwtService(get(), get(), get()) bypasses Koin's standard dependency resolution. The three get() calls rely on positional type matching, which is fragile if JwtService's constructor parameters change or if multiple bindings of the same type exist. Consider registering JwtService in a Koin module and injecting it like the other dependencies.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.kt` at
line 61, The jwtService property in WebService.kt is manually constructed with
JwtService(get(), get(), get()), bypassing Koin and relying on fragile
positional gets; register JwtService in your Koin module (e.g., add a single {
JwtService(get(), get(), get()) } binding or an explicit factory/single with
properly typed parameters) and then inject it in WebService using Koin injection
(inject() or get()) instead of constructing it inline; update the jwtService
property to use the DI-provided instance and remove the manual JwtService(...)
call.

app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.kt (1)

3-31: Consider deriving allowed from the constants to avoid drift.

The allowed set is manually maintained alongside the const declarations. If a new scope constant is added but not included in allowed, firstUnsupported would incorrectly reject it. Since const val properties can't be reflected easily in Kotlin, this is a minor maintenance concern — just worth noting for future additions.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.kt`
around lines 3 - 31, Derive the allowed set from the declared scope constants
instead of maintaining it manually: update AuthScopes so allowed is constructed
programmatically from the scope constants (e.g., collect values of ALL_ANY,
MESSAGES_SEND, MESSAGES_READ, DEVICES_LIST, WEBHOOKS_LIST, WEBHOOKS_WRITE,
SETTINGS_READ, SETTINGS_WRITE, LOGS_READ, TOKENS_MANAGE) so
firstUnsupported(scopes: List<String>) relies on that computed set; modify the
declaration of allowed and keep the existing firstUnsupported function and
constant names (AuthScopes, allowed, firstUnsupported, MESSAGES_SEND, etc.) so
adding new scope constants will automatically appear in allowed.

app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/ScopeAuthorization.kt (1)

23-28: getClaim("scopes").asList() can throw if the claim exists but isn't an array.

If a token has a scopes claim that isn't a JSON array (e.g., a plain string), asList throws JWTDecodeException, resulting in a 500 instead of a 401. Since tokens are self-issued this is low risk, but wrapping in a try-catch would be more defensive.

🛡️ Suggested defensive handling

-    val scopes = jwtPrincipal.payload
-        .getClaim("scopes")
-        .asList(String::class.java)
-        ?.map { it.trim() }
-        ?.filter { it.isNotEmpty() }
-        ?: emptyList()
+    val scopes = try {
+        jwtPrincipal.payload
+            .getClaim("scopes")
+            .asList(String::class.java)
+            ?.map { it.trim() }
+            ?.filter { it.isNotEmpty() }
+            ?: emptyList()
+    } catch (_: Exception) {
+        emptyList()
+    }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/ScopeAuthorization.kt`
around lines 23 - 28, The current extraction of scopes from jwtPrincipal.payload
(val scopes = jwtPrincipal.payload.getClaim("scopes").asList(...)) can throw
JWTDecodeException if the claim exists but isn't an array; wrap the claim access
in a try-catch that catches com.auth0.jwt.exceptions.JWTDecodeException (and any
safe JSON/parse exceptions) around the getClaim(...).asList(...) call inside
ScopeAuthorization (or the function/method where jwtPrincipal is handled), and
on exception return/resolve to an emptyList or treat as unauthorized (emit a
401) and log the malformed claim—this prevents a 500 from being returned for a
bad scopes claim.

app/src/main/java/me/capcom/smsgateway/data/dao/RevokedTokensDao.kt (2)

12-13: Prefer OnConflictStrategy.IGNORE for an append-only revocation store.

Using REPLACE on a conflict will DELETE the existing row and re-INSERT with a new revokedAt timestamp, silently mutating the original revocation time. For a revocation log the original timestamp should be immutable — IGNORE is both more semantically correct and avoids the unnecessary delete-plus-reinsert cycle.

♻️ Proposed change

-    `@Insert`(onConflict = OnConflictStrategy.REPLACE)
+    `@Insert`(onConflict = OnConflictStrategy.IGNORE)
     suspend fun upsert(token: RevokedToken)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/me/capcom/smsgateway/data/dao/RevokedTokensDao.kt` around
lines 12 - 13, The upsert method in RevokedTokensDao currently uses
OnConflictStrategy.REPLACE which deletes and re-inserts rows, mutating the
original revokedAt; change the insert conflict strategy on the suspend fun
upsert(token: RevokedToken) to OnConflictStrategy.IGNORE so the first revocation
timestamp remains immutable and duplicate inserts are ignored; locate the
`@Insert` annotation above upsert in RevokedTokensDao and swap REPLACE for IGNORE,
and ensure callers relying on replace semantics are updated if necessary.

---

10-17: No mechanism to prune naturally-expired revoked tokens.

Once a JWT's TTL elapses it is no longer valid regardless of this table, yet its row stays forever. Over time this becomes dead storage. Consider adding a purge method that callers (e.g., JwtService) can invoke periodically:

`@Query`("DELETE FROM revoked_token WHERE revokedAt < :cutoffMs")
suspend fun deleteRevokedBefore(cutoffMs: Long)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/me/capcom/smsgateway/data/dao/RevokedTokensDao.kt` around
lines 10 - 17, The RevokedTokensDao currently lacks a way to remove tokens whose
JWT TTL has expired, leaving dead rows; add a DAO method (e.g., in interface
RevokedTokensDao) named deleteRevokedBefore that executes "DELETE FROM
revoked_token WHERE revokedAt < :cutoffMs" so callers can prune old entries, and
call this from the periodic cleanup path in JwtService (or wherever revocation
maintenance occurs) passing an appropriate cutoff (now - TTL or retention
window) to remove expired rows; ensure the method is suspend and uses `@Query` to
match existing DAO conventions.

app/src/main/java/me/capcom/smsgateway/ui/settings/LocalServerSettingsFragment.kt (1)

19-23: Consider adding a confirmation dialog before regenerating the JWT secret.

The action is immediately irreversible (all outstanding tokens are invalidated the moment regenerateJwtSecret() returns). While the preference summary warns users, a single accidental tap has no undo. A short AlertDialog with confirm/cancel is the standard Android pattern for destructive settings actions.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/me/capcom/smsgateway/ui/settings/LocalServerSettingsFragment.kt`
around lines 19 - 23, Wrap the immediate call to settings.regenerateJwtSecret()
in a confirmation AlertDialog in LocalServerSettingsFragment (the
findPreference<Preference>("transient.jwt_regenerate_secret") click listener):
show a dialog with a clear title/message, two buttons (Confirm/Cancel), call
settings.regenerateJwtSecret() and
showToast(getString(R.string.jwt_secret_regenerated)) only when the user
confirms, and return true from the preference click handler as before; do not
call regenerateJwtSecret() on cancel and ensure the dialog is created using the
fragment's context (requireContext() or activity) so it behaves correctly on
rotation.

app/build.gradle (1)

75-75: Consider upgrading Ktor from the pinned 2.2.4 to version 3.2.2 or newer.

The pinned version 2.2.4 (released February 28, 2023) is approximately 3 years old. More importantly, Ktor 3.2.2 (July 14, 2025) fixed a critical Android D8 compatibility regression where backticked identifiers in bytecode caused dexing failures on older Android APIs. Since this is an Android project adding a new security-sensitive dependency (ktor-server-auth-jwt), upgrading to at least 3.2.2 or the latest 3.4.0 is recommended to benefit from this fix and current security patches.

Breaking changes exist when migrating from 2.x to 3.x (e.g., HttpResponse.content → rawContent, SocketTimeoutException type changes, kotlinx-io backing), but these are well-documented and manageable.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/build.gradle` at line 75, The build pins Ktor via the variable
ktor_version set to "2.2.4"; update that variable in app/build.gradle to at
least "3.2.2" (or preferably the latest "3.4.0") to pick up the Android D8
compatibility and security fixes, then rebuild and address any 2.x→3.x API
changes (e.g., HttpResponse.content → rawContent, SocketTimeoutException type
differences) in code paths that reference Ktor APIs; ensure dependencies that
reference ktor_version (server/auth/jwt, client libs) are aligned and run tests
to validate runtime/dexing on target Android API levels.

app/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.kt (1)

95-106: Confirm isTokenRevoked is now a suspend function (verifying the addressed fix).

The previously flagged blocking-DAO concern in the validate suspend lambda is marked as resolved. The current code still calls jwtService.isTokenRevoked(tokenId) directly at Line 100 without withContext(Dispatchers.IO), so the fix must have been converting both RevokedTokensDao.exists() and isTokenRevoked to suspend. Run the script below to confirm.

#!/bin/bash
# Verify isTokenRevoked is now a suspend function
rg -n "fun isTokenRevoked" --type=kt -A3

# And confirm the DAO's exists() is also suspend
rg -n "fun exists" --type=kt -B2 -A3

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.kt`
around lines 95 - 106, The validate lambda in the JWT auth block calls
jwtService.isTokenRevoked(tokenId) directly; ensure that isTokenRevoked is a
suspend function and that its underlying RevokedTokensDao.exists(...) is suspend
too, otherwise this still does blocking IO inside the validate suspend lambda —
either make RevokedTokensDao.exists and JwtService.isTokenRevoked suspend
(propagate suspend signatures to jwtService.isTokenRevoked) and call it directly
from the validate lambda, or if you cannot change DAO signatures, wrap the call
in withContext(Dispatchers.IO) inside the validate lambda; check and update the
functions named isTokenRevoked and RevokedTokensDao.exists accordingly.

app/src/main/java/me/capcom/smsgateway/data/AppDatabase.kt (1)

60-60: revokedTokensDao() name is misleading — consider renaming to tokensDao().

Every other accessor in this class follows the <entityPlural>Dao() convention (messagesDao, webhooksDao, logDao). The returned TokensDao handles the full token lifecycle (insert, revoke, isRevoked, cleanup), not just revoked tokens, so the revoked prefix misrepresents its scope and is a holdover from what the PR summary calls the earlier RevokedTokensDao naming.

♻️ Proposed rename

-    abstract fun revokedTokensDao(): TokensDao
+    abstract fun tokensDao(): TokensDao

Also update the DI binding in Module.kt (AppDatabase.revokedTokensDao() → AppDatabase.tokensDao()).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/me/capcom/smsgateway/data/AppDatabase.kt` at line 60, The
method name revokedTokensDao() on AppDatabase is misleading because the DAO
manages full token lifecycle; rename the abstract method revokedTokensDao() to
tokensDao() and update all call sites (e.g., DI binding in Module.kt) to call
AppDatabase.tokensDao(); ensure the return type remains TokensDao and update any
imports/usages to the new method name so compilation and DI wiring continue to
work.

app/src/main/java/me/capcom/smsgateway/data/dao/TokensDao.kt (1)

20-21: cleanup() also removes revoked-but-expired tokens — consider an operational note.

After cleanup() runs, a token that was revoked and has since expired is deleted. Any audit trail of the revocation is lost. If post-hoc audit of revoked tokens is ever needed, a retention policy (e.g., keep revoked tokens for N days past expiry, or archive to a separate table) should be considered.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/me/capcom/smsgateway/data/dao/TokensDao.kt` around lines 20
- 21, The current TokensDao.cleanup() query deletes any token whose expiresAt is
past now, which also removes tokens that were previously revoked and thus loses
revocation audit; change cleanup() to preserve revoked tokens (e.g., add "AND
revokedAt IS NULL" or equivalent condition to the DELETE so only non-revoked,
expired rows are removed) or implement an archival step before deletion (insert
revoked-but-expired rows from the tokens table into a revoked_tokens archive
table and then delete) and ensure the query references the tokens table and
expiresAt column accordingly.

app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.kt (2)

30-59: Remove the commented-out old AuthScopes object.

Dead code left in the file after refactoring to the enum; serves no purpose and adds noise.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.kt`
around lines 30 - 59, Remove the commented-out legacy AuthScopes object block —
specifically the old object named AuthScopes (and its members ALL_ANY,
MESSAGES_SEND, allowed, firstUnsupported) that's been left commented out — since
the file now uses the enum refactor; delete the entire commented block to
eliminate dead code and noise while leaving the new enum implementation intact.

---

23-27: firstUnsupported reconstructs the full enum values list on every predicate invocation.

values().map { it.value } creates a new Array + List pair for every element in scopes. Cache the supported-values set as a companion property.

♻️ Proposed refactor

 companion object {
+    private val supportedValues: Set<String> = values().mapTo(HashSet()) { it.value }
+
     fun firstUnsupported(scopes: List<String>): String? {
-        return scopes.firstOrNull { it !in values().map { it.value } }
+        return scopes.firstOrNull { it !in supportedValues }
     }
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.kt`
around lines 23 - 27, The current firstUnsupported(scopes: List<String>)
rebuilds values().map { it.value } for every predicate call which is wasteful;
add a cached companion property (e.g. supportedValues: Set<String> or
supportedValuesList) computed once from values().map { it.value } (or
values().mapTo(HashSet()) ) and then change firstUnsupported to use that cached
set (check membership with "it !in supportedValues") to avoid repeated
allocations; update the companion object containing firstUnsupported to declare
and use this cached supportedValues.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.kt (1)

25-26: Broaden input type for firstUnsupported to improve call-site ergonomics.

Since this method only iterates, accepting Iterable<String> (or Collection<String>) avoids forcing callers to allocate a List.

♻️ Proposed refactor

-        fun firstUnsupported(scopes: List<String>): String? {
+        fun firstUnsupported(scopes: Iterable<String>): String? {
             return scopes.firstOrNull { it !in supportedValues }
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/AuthScopes.kt`
around lines 25 - 26, Change firstUnsupported to accept a broader input type to
avoid forcing callers to allocate a List: update the function signature fun
firstUnsupported(scopes: List<String>): String? to accept Iterable<String> (or
Collection<String>) and keep the body unchanged (return scopes.firstOrNull { it
!in supportedValues }), then update any call sites that pass non-List iterables
to compile if necessary; reference: function firstUnsupported and property
supportedValues.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/ScopeAuthorization.kt (1)

13-15: ⚠️ Potential issue | 🟠 Major

UserIdPrincipal bypass silently grants full access to all basic-auth clients.

Any request authenticated via basic auth (which sets UserIdPrincipal) unconditionally returns true, completely bypassing scope enforcement. This gives basic-auth credentials implicit all:any access. If this is intentional for backward compatibility, add a comment documenting this design decision.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/ScopeAuthorization.kt`
around lines 13 - 15, The code in ScopeAuthorization.kt currently treats any
basic-auth principal (call.principal<UserIdPrincipal>()) as a blanket allow by
returning true; remove this unconditional bypass and enforce scopes for
UserIdPrincipal the same way other principals are checked (retrieve the
UserIdPrincipal from call.principal<UserIdPrincipal>(), extract its
scopes/roles, and validate the requested scope against those using the existing
scope-checking logic), or if this unconditional allow is intentional for
backward compatibility, replace the return with an explicit comment explaining
the design decision and the exact scope implications so it is not silently
granting all:any access.

app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/AuthRoutes.kt (1)

29-42: ⚠️ Potential issue | 🟠 Major

Privilege escalation: JWT-authenticated user can mint tokens with elevated scopes.

The scope validation added to JwtService rejects unsupported scope strings but doesn't prevent a JWT user with only TokensManage from creating a token with all:any. Since requireScope() grants basic-auth users unrestricted access but constrains JWT users to their token's scopes, a JWT-authenticated caller can escalate privileges by minting broader-scoped tokens.

Consider restricting JWT callers to only issue tokens whose scopes are a subset of their own:

🛡️ Suggested escalation guard

 post {
     if (!requireScope(AuthScopes.TokensManage)) return@post
     val request = call.receive<TokenRequest>()
+
+    // Prevent JWT users from escalating privileges
+    val jwtPrincipal = call.principal<JWTPrincipal>()
+    if (jwtPrincipal != null) {
+        val callerScopes = jwtPrincipal.payload
+            .getClaim("scopes")
+            .asList(String::class.java)
+            ?.toSet() ?: emptySet()
+        val requestedScopes = request.scopes?.toSet() ?: emptySet()
+        if (!callerScopes.containsAll(requestedScopes) && AuthScopes.AllAny.value !in callerScopes) {
+            call.respond(HttpStatusCode.Forbidden, mapOf("message" to "Cannot issue token with scopes exceeding your own"))
+            return@post
+        }
+    }
+
     val token = jwtService.generateToken(request.scopes, request.ttl)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/AuthRoutes.kt`
around lines 29 - 42, Summary: A JWT-authenticated user with TokensManage can
mint tokens with scopes beyond their own (e.g., all:any), enabling privilege
escalation. Fix: In the POST handler that uses
requireScope(AuthScopes.TokensManage) (AuthRoutes.kt) and before calling
jwtService.generateToken(TokenRequest.scopes,...), detect if the caller is
authenticated via JWT and obtain their granted scopes from the principal; then
validate that the requested TokenRequest.scopes is a subset of the caller's
scopes and return HttpStatusCode.Forbidden if not. Additionally, to harden the
service, extend JwtService.generateToken (or add an overload) to accept an
optional allowedScopes parameter and enforce the same subset check server-side
so programmatic callers cannot bypass the route-level check. Ensure basic-auth
behavior remains unchanged (i.e., keep existing unrestricted issuance for basic
users).

app/src/main/java/me/capcom/smsgateway/data/dao/TokensDao.kt (1)

11-12: Consider returning insert result for error detection.

OnConflictStrategy.IGNORE silently drops duplicate inserts. Combined with the isRevoked() semantic gap (returns false for both "not revoked" and "token doesn't exist"), a failed insert could leave a token permanently untrackable. Consider returning Long (row ID or -1 on conflict) to allow callers to detect and handle insertion failures.

♻️ Proposed fix

 `@Insert`(onConflict = OnConflictStrategy.IGNORE)
-suspend fun insert(token: Token)
+suspend fun insert(token: Token): Long

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/me/capcom/smsgateway/data/dao/TokensDao.kt` around lines 11
- 12, The insert currently uses OnConflictStrategy.IGNORE and returns Unit which
hides conflicts and, combined with isRevoked() returning false for missing
tokens, makes insert failures undetectable; change the TokensDao.insert
signature to return Long (suspend fun insert(token: Token): Long) so Room
returns the row ID or -1 on conflict, then update all call sites to check for -1
and handle it (log/error/attempt update or notify caller) to ensure failed
inserts are detected and resolved; keep OnConflictStrategy.IGNORE or switch
strategy as appropriate but ensure callers handle the returned Long from insert.

app/src/main/java/me/capcom/smsgateway/ui/settings/LocalServerSettingsFragment.kt (1)

67-76: Consider adding an upper bound for JWT TTL.

The validation only checks ttl > 0, but extremely large values (e.g., years) may lead to tokens that effectively never expire, undermining security. Consider adding a reasonable upper bound (e.g., 30 days = 2592000 seconds).

♻️ Proposed fix

+private const val MAX_JWT_TTL_SECONDS = 2592000L // 30 days
+
 findPreference<EditTextPreference>("localserver.JWT_TTL_SECONDS")?.setOnPreferenceChangeListener { _, newValue ->
     val value = newValue as? String
     val ttl = value?.toLongOrNull()
-    if (ttl == null || ttl <= 0) {
-        showToast(getString(R.string.jwt_ttl_must_be_positive_seconds))
+    if (ttl == null || ttl <= 0 || ttl > MAX_JWT_TTL_SECONDS) {
+        showToast(getString(R.string.jwt_ttl_must_be_between_1_and_30_days))
         return@setOnPreferenceChangeListener false
     }

     true
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/me/capcom/smsgateway/ui/settings/LocalServerSettingsFragment.kt`
around lines 67 - 76, The TTL validation on the EditTextPreference
"localserver.JWT_TTL_SECONDS" currently only enforces ttl > 0; modify the
setOnPreferenceChangeListener (the lambda attached to
findPreference<EditTextPreference>("localserver.JWT_TTL_SECONDS")) to also
enforce an upper bound (suggest MAX_JWT_TTL_SECONDS = 2_592_000 for 30 days), so
after parsing newValue to Long reject values <= 0 or > MAX_JWT_TTL_SECONDS by
calling showToast with a new message (e.g.,
jwt_ttl_must_be_between_1_and_2592000_seconds) and returning false, otherwise
return true. Ensure you reference the same preference key and keep parsing via
toLongOrNull to avoid crashes.

app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/ScopeAuthorization.kt (1)

30-31: Silent exception swallowing may mask JWT claim parsing errors.

Catching all exceptions and returning emptyList() means malformed scopes claims are silently treated as "no scopes." Consider logging the exception at debug level to aid troubleshooting while maintaining fail-closed behavior.

♻️ Proposed fix

     val scopes = try {
         jwtPrincipal.payload
             .getClaim("scopes")
             .asList(String::class.java)
             ?.map { it.trim() }
             ?.filter { it.isNotEmpty() }
             ?: emptyList()
-    } catch (_: Exception) {
+    } catch (e: Exception) {
+        // Log at debug level for troubleshooting; treat as no scopes (fail-closed)
+        android.util.Log.d("ScopeAuthorization", "Failed to parse scopes claim", e)
         emptyList()
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/ScopeAuthorization.kt`
around lines 30 - 31, The catch-all in ScopeAuthorization.kt currently swallows
exceptions (the "catch (_: Exception) { emptyList() }" block) which hides JWT
claim parsing errors; change the catch to capture the exception (e.g., "catch
(ex: Exception)") and log it at debug level with a clear message about failing
to parse the "scopes" claim (keeping the existing fail-closed behavior of
returning emptyList()); use the module's logger (e.g., a Logger instance used
elsewhere in ScopeAuthorization) and include the exception object in the debug
call so stack/causes are visible for troubleshooting.

app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/JwtService.kt (1)

54-55: Consider deferring token cleanup to a background task.

Calling cleanupExpiredTokens() synchronously during generateToken() adds database I/O latency to every token creation request. While this ensures timely cleanup, it couples unrelated concerns.

Consider moving cleanup to a periodic background job or triggering it less frequently (e.g., probabilistically or on a timer) to reduce per-request latency.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/auth/JwtService.kt`
around lines 54 - 55, The synchronous call to cleanupExpiredTokens() inside
generateToken() causes extra DB I/O on every token creation; remove that direct
call from generateToken() (where tokensDao.insert(Token(...)) is used) and
instead schedule cleanupExpiredTokens() to run in a background job or trigger it
probabilistically (e.g., run cleanup only some percentage of calls) so token
creation stays fast; update JwtService to decouple cleanup from the
generateToken() flow and add a periodic scheduler or probabilistic guard that
invokes cleanupExpiredTokens().

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro