Skip to content

Discover the login url for self-hosted sites and log it #21851

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
May 1, 2025

Conversation

adalpari
Copy link
Contributor

@adalpari adalpari commented May 1, 2025

Description

This PR is discovering the password login url for a site and logging it

Testing instructions

  1. Open the self-hosted sites login screen
  2. Enter a valid url
  3. Hit continue
  1. Open the self-hosted sites login screen
  2. Enter a NON-valid url. For instance: http://wordfence.wpmt.co/
  3. Hit continue
  • Verify you see a log like: "VM: Error during API discovery"

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new mechanism to discover and log the authorization URL for self-hosted sites during the login process.

  • Adds a new LoginSiteAddressViewModel with API discovery logic.
  • Integrates the view model into the LoginSiteAddressFragment.
  • Updates dependency injection modules to bind the new view model and provide a singleton WpLoginClient.

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

File Description
libs/login/src/main/java/org/wordpress/android/login/viewmodel/LoginSiteAddressViewModel.kt Implements API discovery and logs the discovered authorization URL.
libs/login/src/main/java/org/wordpress/android/login/LoginSiteAddressFragment.kt Initializes the new view model and invokes the API discovery.
WordPress/src/main/java/org/wordpress/android/modules/ViewModelModule.java Adds binding for LoginSiteAddressViewModel.
WordPress/src/main/java/org/wordpress/android/modules/ApplicationModule.java Provides a singleton instance of WpLoginClient.
Files not reviewed (1)
  • libs/login/build.gradle: Language not supported

@nbradbury nbradbury self-assigned this May 1, 2025
@dangermattic
Copy link
Collaborator

dangermattic commented May 1, 2025

1 Warning
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Contributor

wpmobilebot commented May 1, 2025

Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr21851-2f68298
Commit2f68298
Direct Downloadjetpack-prototype-build-pr21851-2f68298.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented May 1, 2025

WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr21851-2f68298
Commit2f68298
Direct Downloadwordpress-prototype-build-pr21851-2f68298.apk
Note: Google Login is not supported on these builds.

val authorizationUrl = urlDiscovery.apiDetails.findApplicationPasswordsAuthenticationUrl()
Log.d("WP_RS", "VM: Found authorization URL: $authorizationUrl")
} catch (throwable: Throwable) {
Log.e("WP_RS", "VM: Error during API discovery", throwable)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if we should include the passed url in the log message?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This logic is not gonna exist in the way it is in the next iteration. The logging is actually just to check the PR, so I don't think it's necessary.

@@ -190,6 +198,8 @@ class LoginSiteAddressFragment : LoginBaseDiscoveryFragment(), TextWatcher, OnEd

val cleanedUrl = stripKnownPaths(requestedSiteAddress.orEmpty())

viewModel.runApiDiscovery(cleanedUrl)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: maybe add a comment that this is a work-in-progress and isn't really used yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Copy link
Contributor

@nbradbury nbradbury left a comment

Choose a reason for hiding this comment

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

Changes are good, merge when ready! :shipit:

Copy link

sonarqubecloud bot commented May 1, 2025

@adalpari adalpari merged commit d0493a9 into trunk May 1, 2025
26 checks passed
@adalpari adalpari deleted the feat/CMM-327-Discover-the-login-url-and-log-it branch May 1, 2025 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants