Skip to content
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

[Login] Login using Site Credentials will fail when discovery fails and permalinks are disabled #12043

Open
hichamboushaba opened this issue Jul 17, 2024 · 5 comments
Labels
feature: login Related to any part of the log in or sign in flow, or authentication. type: bug A confirmed bug.

Comments

@hichamboushaba
Copy link
Member

hichamboushaba commented Jul 17, 2024

Currently, the Android app doesn't support signing in using site credentials when the following conditions apply:

  1. Site discovery fails (Discovery is the process of identified the REST URL of the website, this can fail when for example the site sends non-standard Link header, or no header at all)
  2. Permalinks disabled.

We need to update our logic to use by default a URL in the format of {site-url}/?rest_route=/ instead of {site-url}/wp-json/, see https://github.com/wordpress-mobile/WordPress-FluxC-Android/blob/765928e3458599d1970115bc63affa2a9db2a93e/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/WPAPIDiscoveryUtils.kt#L13-L16

@hichamboushaba hichamboushaba added type: bug A confirmed bug. feature: login Related to any part of the log in or sign in flow, or authentication. labels Jul 17, 2024
@dangermattic
Copy link
Collaborator

Thanks for reporting! 👍

@rossanafmenezes
Copy link
Contributor

Just for context, we currently have 3 tickets where users are having this issue: zd-8560496, zd-8414673 and zd-8593324

@hichamboushaba
Copy link
Member Author

Thanks @rossanafmenezes for the information, I checked all three tickets, and I don't think they apply to this case:

  1. zd-8560496: For these cases, the discovery works well with the site (The site returns the Link header correctly), so the app is already using the correct path to talk to the API, the fact /users/me doesn't work is a different issue, even it works when accessed without using the permalink. For more information, even if we fix this issue, the app will continue to use the /wp-json path for this site, because the discovery will take priority over using the fallback URL.
  2. zd-8414673: The issue in the ticket should be solved now, the discovery was failing because the site returns multiple Link headers, which we now support ([Login] Improve site discovery for sites with multiple Link headers #12079).
  3. zd-8593324: I suspect this is similar to the first case, but I can't confirm it because the site mentioned in the ticket is not accessible.

We didn't prioritize working on this issue yet because we don't think it's a high priority, as it happens only when the two conditions mentioned above apply, which should be a low chance.

If we get a higher number of cases like 1 and 3, we can revisit following the same approach as iOS: remove the usage of WPApi discovery in favor of using the standard URL ?rest_route={path}, but I think this will require some careful considerations to avoid breaking the logic for other cases.

cc @pmusolino

@sophiegyo
Copy link

I'm wondering if the report in 9152549-zen might be related to this?

https://URL/wp-json in this ticket works fine however.

@hichamboushaba
Copy link
Member Author

@sophiegyo I don't think the above ticket is related to this, I shared some remarks in the ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: login Related to any part of the log in or sign in flow, or authentication. type: bug A confirmed bug.
Projects
None yet
Development

No branches or pull requests

4 participants