Skip to content

fix(nomad devices): validate signed automated login intents before checking server support #4664

Open
MohamadJaara wants to merge 1 commit intodevelopfrom
nomad-devices/feat/check-if-server-support-nomad-devices-before-processing-the-intent
Open

fix(nomad devices): validate signed automated login intents before checking server support #4664
MohamadJaara wants to merge 1 commit intodevelopfrom
nomad-devices/feat/check-if-server-support-nomad-devices-before-processing-the-intent

Conversation

@MohamadJaara
Copy link
Member

PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³
  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Issues

Automated Nomad login intents were not fully respecting server-side Nomad support.
The flow was gated by a local app/build flag instead of checking the backend capability through the proper auth use case.
The intent handling also needed the signature check to be the first meaningful validation step for the Nomad payload.

Causes (Optional)

The Nomad intent parser and the non-deeplink handling path relied on NomadProfilesFeatureConfig, which only reflects local build configuration.
Although IsNomadProfilesEnabledUseCase already existed, it was not wired into the automated Nomad intent flow.
This made the client accept a signed Nomad intent without verifying that the target backend actually supports Nomad profiles.

Solutions

Updated the intent parser to validate the Nomad signature before accepting the payload.
Kept the parser strict so automated Nomad login requires both backendConfig and ssoCode.
Updated WireActivityViewModel to:

  • load the backend config from the signed intent
  • build the authentication scope for that backend
  • call IsNomadProfilesEnabledUseCase
  • stop the flow if the backend does not support Nomad profiles
    Preserved the existing OnUnknownDeepLink behavior when backend config loading fails.
    Added and updated unit tests for:
  • signed intent parsing
  • invalid signature rejection
  • server-disabled Nomad flow rejection
  • invalid backend-only automated login inputs

Dependencies (Optional)

Needs releases with:

  • GitHub link to other pull request

Testing

Test Coverage (Optional)

  • I have added automated test to this contribution

How to Test

Notes (Optional)

The flow still respects the local build flag in the view model as an additional gate, but the backend capability check is now mandatory before automatic Nomad login proceeds.
This PR mainly changes gating and validation order; it does not change the login payload structure.

Attachments (Optional)

flowchart TD
    A["Incoming non-deeplink intent"] --> B["Parse automated_login payload"]
    B --> C{"nomadProfilesHost present?"}
    C -- No --> X["Ignore intent"]
    C -- Yes --> D{"Signature valid?"}
    D -- No --> X
    D -- Yes --> E{"backendConfig and ssoCode present?"}
    E -- No --> X
    E -- Yes --> F{"Local Nomad flag enabled?"}
    F -- No --> X
    F -- Yes --> G["Load backend config"]
    G --> H{"Config loaded?"}
    H -- No --> I["Send OnUnknownDeepLink"]
    H -- Yes --> J["Create auth scope for backend"]
    J --> K["Call IsNomadProfilesEnabledUseCase"]
    K --> L{"Server supports Nomad?"}
    L -- No --> X
    L -- Yes --> M{"Existing valid session?"}
    M -- Yes --> N["Show blocked login toast"]
    M -- No --> O["Dispatch OnAutomaticLogin"]
Loading

PR Post Submission Checklist for internal contributors (Optional)

  • Wire's Github Workflow has automatically linked the PR to a JIRA issue

PR Post Merge Checklist for internal contributors

  • If any soft of configuration variable was introduced by this PR, it has been added to the relevant documents and the CI jobs have been updated.

@sonarqubecloud
Copy link

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant