Skip to content
New issue

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

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

Already on GitHub? # to your account

feat: Authenticate private resource requests #21331

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

dcalhoun
Copy link
Member

@dcalhoun dcalhoun commented Oct 23, 2024

Related

Description

Authenticate resource requests originating from within the WebView so that
requests to private sites succeed.

To Test:

See wordpress-mobile/GutenbergKit#34.

Regression Notes

  1. Potential unintended areas of impact
    Gutenberg Mobile or Aztec editors fail.
  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    Manually tested both editors.
  3. What automated tests I added (or what prevented me from doing so)
    None, feels unnecessary for this currently experimental editor feature.

PR Submission Checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Testing Checklist (strike-out the not-applying and unnecessary ones):

  • WordPress.com sites and self-hosted Jetpack sites.
  • Portrait and landscape orientations.
  • Light and dark modes.
  • Fonts: Larger, smaller and bold text.
  • High contrast.
  • Talkback.
  • Languages with large words or with letters/accents not frequently used in English.
  • Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
  • Large and small screen sizes. (Tablet and smaller phones)
  • Multi-tasking: Split screen and Pop-up view. (Android 10 or higher)

@dcalhoun dcalhoun added [Type] Bug Gutenberg Editing and display of Gutenberg blocks. labels Oct 23, 2024
@dangermattic
Copy link
Collaborator

dangermattic commented Oct 23, 2024

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

Generated by 🚫 Danger

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Oct 23, 2024

Project dependencies changes

The following changes in project dependencies were detected (configuration wordpressVanillaReleaseRuntimeClasspath):

list
Upgraded Dependencies
org.wordpress.gutenbergkit:android:trunk-d6fbfc7bc28ae6db2cce09950f24bc3080374596, (changed from trunk-a58a46f3fbb892f311b562e3c122d7ef4ebbfe33)
tree
 +--- project :libs:editor
-|    \--- org.wordpress.gutenbergkit:android:trunk-a58a46f3fbb892f311b562e3c122d7ef4ebbfe33
+|    \--- org.wordpress.gutenbergkit:android:trunk-d6fbfc7bc28ae6db2cce09950f24bc3080374596
-\--- org.wordpress.gutenbergkit:android:trunk-a58a46f3fbb892f311b562e3c122d7ef4ebbfe33 (*)
+\--- org.wordpress.gutenbergkit:android:trunk-d6fbfc7bc28ae6db2cce09950f24bc3080374596 (*)

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Oct 23, 2024

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
Versionpr21331-fc246b1
Commitfc246b1
Direct Downloadwordpress-prototype-build-pr21331-fc246b1.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Oct 23, 2024

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
Versionpr21331-fc246b1
Commitfc246b1
Direct Downloadjetpack-prototype-build-pr21331-fc246b1.apk
Note: Google Login is not supported on these builds.


String proxyUrl = url.toString();
if (mIsPrivateAtomic) {
proxyUrl = getPrivateResourceProxyUrl(url);
Copy link
Member Author

Choose a reason for hiding this comment

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

Private Atomic sites require a proxy for request authentication (D38925-code).

@dcalhoun dcalhoun marked this pull request as ready for review October 30, 2024 16:08
@dcalhoun dcalhoun requested a review from jkmassel October 30, 2024 16:08
@dcalhoun dcalhoun requested a review from jkmassel October 30, 2024 20:25
dcalhoun added a commit that referenced this pull request Oct 31, 2024
Address lint error regarding a common source of bugs.

#21331 (review)
Copy link

sonarqubecloud bot commented Nov 1, 2024

@dcalhoun
Copy link
Member Author

dcalhoun commented Nov 1, 2024

👋🏻 @jkmassel. I updated this with the merged wordpress-mobile/GutenbergKit#30 and the latest from trunk. I re-tested and it appears to be functioning as expected. Whenever you have time, we can look to review and merge this feature. Thanks!

@nbradbury
Copy link
Contributor

@dcalhoun @jkmassel Where do we stand with this PR? Is it something we can close?

@dcalhoun
Copy link
Member Author

Where do we stand with this PR? Is it something we can close?

Ideally, it is merged.

However, @jkmassel mentioned privately he would prefer avoiding a proxy URL. Transparently, I was unsure as to how we might accomplish that, so I have not moved this forward. @jkmassel do you have specific ideas for avoiding the proxy URL?

Unauthenticated requests fail when attempting to load resources from a
private site--e.g., an image.
Avoid NullPointerExceptions.
@dcalhoun dcalhoun force-pushed the feat/authenticate-private-resource-requests branch from 172e55e to fc246b1 Compare February 19, 2025 16:39
Copy link

codecov bot commented Feb 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 39.41%. Comparing base (191c357) to head (fc246b1).

Additional details and impacted files
@@           Coverage Diff           @@
##            trunk   #21331   +/-   ##
=======================================
  Coverage   39.41%   39.41%           
=======================================
  Files        2122     2122           
  Lines       99570    99570           
  Branches    15324    15324           
=======================================
  Hits        39247    39247           
  Misses      56844    56844           
  Partials     3479     3479           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Gutenberg Editing and display of Gutenberg blocks. [Type] Bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants