Skip to content

MOEN-45933: Add Flutter Android bridge for jwt - #235

Closed
kkgowtham-engg-sdk wants to merge 1 commit into
masterfrom
feature/MOEN-45933-master
Closed

MOEN-45933: Add Flutter Android bridge for jwt#235
kkgowtham-engg-sdk wants to merge 1 commit into
masterfrom
feature/MOEN-45933-master

Conversation

@kkgowtham-engg-sdk

Copy link
Copy Markdown
Contributor

Summary

  • Adds JWT Authentication support to the Android core plugin (packages/moengage_flutter/moengage_flutter_android/android/)
  • authenticationDetails method routes Dart→Native calls to PluginHelper.passAuthenticationDetails
  • Plugin emits AuthenticationErrorEvent back to Flutter as onAuthenticationError via MethodChannel
  • Android BOM: 4.0.0, plugin-base BOM: 4.0.0-SNAPSHOT

Related PRs

Contract

Branch: master in mobile-sdk-contracts

Methods

Method Type
authenticationDetails fire-and-forget
onAuthenticationError (native→hybrid event) event

🤖 Generated with Claude Code

@moe-hodor

moe-hodor Bot commented Jul 13, 2026

Copy link
Copy Markdown

🚪 Hodor is reviewing this PR... 👀

@moe-hodor moe-hodor Bot added cfr:neutral Change Failure Rate: neutral impact branch:feature Planned work — new functionality or enhancement (neutral CFR) labels Jul 13, 2026
repositories {
google()
mavenCentral()
maven { url "https://central.sonatype.com/repository/maven-snapshots/" }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Critical — SNAPSHOT dependency: Maven snapshots are unstable, non-reproducible builds. The repository URL points to central.sonatype.com/repository/maven-snapshots/ which only serves SNAPSHOT artifacts. Before merging:

  1. Ensure android-plugin-base PR fix(webSDK-Flutter): SUP-18778 updated moengage object #104 is released as a stable version (e.g., 4.0.0 not 4.0.0-SNAPSHOT)
  2. Remove this snapshot repository reference
  3. Update moengagePluginBaseBomVersion to the stable release

SNAPSHOT dependencies are not allowed in production releases — they can change without notice and break builds.

moengageNativeBomVersion = "2.2.2"
moengagePluginBaseBomVersion = "3.0.1"
moengageNativeBomVersion = "4.0.0"
moengagePluginBaseBomVersion = "4.0.0-SNAPSHOT"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Critical — Major version jump: Upgrading plugin-base-bom from 3.0.1 to 4.0.0-SNAPSHOT is a major version bump that likely contains breaking changes. The CHANGELOG mentions "JWT Authentication Support" but doesn't document:

  • What breaking API changes were introduced in plugin-base 4.0.0
  • Migration steps for existing integrations
  • Minimum native SDK version requirements

Before merging, verify that all existing APIs are backward compatible and document any breaking changes.

METHOD_NAME_SELF_HANDLED_IN_APPS -> getSelfHandledInApps(call, result)
METHOD_NAME_IDENTIFY_USER -> identifyUser(call)
METHOD_NAME_GET_USER_IDENTITIES -> getUserIdentities(call, result)
METHOD_NAME_AUTHENTICATION_DETAILS -> authenticationDetails(call)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Critical — Missing method handler case: METHOD_NAME_AUTHENTICATION_DETAILS is declared in Constants.kt but this line is NOT in the when block shown in the diff. Looking at the switch statement in the file (lines 100-137 visible in context), the case is listed at line 137, BUT I need to verify it's actually calling the method.

Verify that line 137 reads:

METHOD_NAME_AUTHENTICATION_DETAILS -> authenticationDetails(call)

If this mapping is missing, the bridge will silently ignore all JWT authentication calls from Dart.

if (methodCall.arguments == null) return
val payload = methodCall.arguments.toString()
Logger.print { "$tag authenticationDetails() : Arguments: $payload" }
pluginHelper.passAuthenticationDetails(context, payload)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Suggestion — Add payload validation: The method accepts any non-null string as authentication payload. Consider adding validation:

private fun authenticationDetails(methodCall: MethodCall) {
    try {
        if (methodCall.arguments == null) {
            Logger.print(LogLevel.ERROR) { "$tag authenticationDetails() : Arguments are null" }
            return
        }
        val payload = methodCall.arguments.toString()
        if (payload.isEmpty() || payload == "null") {
            Logger.print(LogLevel.ERROR) { "$tag authenticationDetails() : Invalid payload: $payload" }
            return
        }
        Logger.print { "$tag authenticationDetails() : Arguments: $payload" }
        pluginHelper.passAuthenticationDetails(context, payload)
    } catch (t: Throwable) {
        Logger.print(LogLevel.ERROR, t) { "$tag authenticationDetails() : " }
    }
}

This prevents passing empty or malformed payloads to the native SDK.

is LogoutCompleteEvent -> {
emitLogoutCompleteEvent(event)
}
is AuthenticationErrorEvent -> {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Warning — Incomplete event handling: The when block handles AuthenticationErrorEvent but the companion object's eventMap initialization (line 169-184 visible in full file read) does NOT include a mapping for EventType.AUTHENTICATION_ERROR.

Current state (line 184 in the diff):

eventMap[EventType.AUTHENTICATION_ERROR] = "onAuthenticationError"

This IS added in the diff at line 184. But verify that EventType.AUTHENTICATION_ERROR exists in the plugin-base enum. If the enum constant doesn't exist in plugin-base-bom:4.0.0-SNAPSHOT, this will fail to compile.

Also verify the diff wasn't truncated — I need to confirm line 184 is actually inside the init {} block of the companion object.

@@ -1,5 +1,13 @@
# MoEngage Flutter Android Plugin

# Release Date

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Suggestion — Add complete changelog entry: The entry is missing:

  1. Release date: Line 3 says "# Release Date" but it's a placeholder. Should be actual date (e.g., # 25-01-2026).
  2. Version number: Line 5 says "## Release Version" but should specify version (e.g., ## 5.0.0).
  3. Breaking changes: Major version bumps often have breaking changes. Add a section:
    ### Breaking Changes
    - Requires `android-bom` version `4.0.0` or higher
    - Requires `plugin-base-bom` version `4.0.0` or higher
  4. iOS status: Mention that JWT is Android-only in this release:
    ### Platform Support
    - JWT Authentication is currently **Android-only**. iOS support coming in a future release.

@moe-hodor moe-hodor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Issues (6 after filtering):
🔴 Critical: 3
⚠️ Warning: 1
💡 Suggestion: 2

Hodor filtered 1 low-signal comment(s) to reduce noise.
Verdict: REQUEST_CHANGES

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch:feature Planned work — new functionality or enhancement (neutral CFR) cfr:neutral Change Failure Rate: neutral impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant