Android/security hardening batch - #156
Open
lawrencesunday311-sketch wants to merge 4 commits into
Open
Conversation
RegisterSheet only required a non-blank username before it was used verbatim to build the WebAuthn user.id/user.name fields, with no cap on length or character set. Add a shared UsernameValidator (3-32 chars, alphanumeric plus '.', '_', '-', must start/end alphanumeric), trim whitespace before submission, and enforce the same rule defensively in PasskeyService.register as the actual boundary that builds the WebAuthn fields. iOS has no equivalent validation yet either (ethos-protocol#11), so this rule is provisional pending confirmation against the backend and iOS.
PasskeyService had no test file at all, and its WebAuthn requestJson construction (key names, rp/user nesting, authenticatorSelection flags) had no verification independent of a live CredentialManager. Extract that construction into PasskeyRequestBuilder using kotlinx.serialization, which builds and asserts JSON shape on the plain JVM without needing Robolectric to stand in for org.json. Add tests asserting the exact JSON shape for both registration and authentication requests. A regression test for ethos-protocol#53/ethos-protocol#56 is deferred until those fixes actually land in this codebase — neither is present here yet.
AndroidManifest.xml points platform passkey/App Links verification at @string/asset_statements, but nothing verified the corresponding assetlinks.json was actually reachable or in sync at https://ethos-protocol.app/.well-known/assetlinks.json — the Android counterpart to iOS's ethos-protocol#6. Add a CI step that fetches assetlinks.json and fails the build if it's unreachable, isn't valid JSON, or its relation/package_name/namespace don't match the manifest's asset_statements resource. Fingerprint literal-equality is only enforced once the committed REPLACE_WITH_RELEASE_CERT_SHA256_FINGERPRINT placeholder is resolved (the real fingerprint is deliberately never checked into git, mirroring how release signing credentials are already handled); until then the served fingerprints are still validated for well-formedness.
AuthViewModel.signIn could be invoked as fast as the UI allowed, with no client-side throttling after repeated failures — relying entirely on server-side protection that isn't visible from the mobile client. Track consecutive failures and, once they reach a threshold (3), start an exponentially escalating cooldown (2s, 4s, 8s, ... capped at 60s) during which signIn is a no-op. Surface the remaining cooldown via AuthUiState.cooldownRemainingSeconds, disable the sign-in button and show a countdown in AuthScreen while it's active, and reset both the failure count and any in-flight cooldown on a successful sign-in or sign-out. Add tests for the escalation curve, the max clamp, the no-op-while-cooling-down behavior, and reset-on-success.
|
@lawrencesunday311-sketch Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
summary
RegisterSheet#61PasskeyService's Request-Building Logic #62