fix(KlaviyoForms): load IAF WebView same-origin to fix 401 on /api/profiles/ (MAGE-787)#614
Open
ab1470 wants to merge 3 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a ChangesSame-origin baseURL for in-app form WebView
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/KlaviyoForms/KlaviyoWebView/KlaviyoWebViewController.swift`:
- Around line 127-135: The try? operator silently suppresses errors when reading
the HTML content from viewModel.url, making it difficult to debug if the read
unexpectedly fails. Replace the try? pattern with a do-try-catch block to
explicitly capture and log any error that occurs during the
String(contentsOf:encoding:) call, so that if HTML reading fails, the actual
error information is logged before falling back to webView.load with the
URLRequest. This will aid troubleshooting if the local template file read
operation encounters unexpected issues.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: e0a87a07-06c6-435c-833b-07c47182914d
📒 Files selected for processing (4)
Sources/KlaviyoForms/InAppForms/IAFWebViewModel.swiftSources/KlaviyoForms/KlaviyoWebView/KlaviyoWebViewController.swiftSources/KlaviyoForms/KlaviyoWebView/KlaviyoWebViewModeling.swiftTests/KlaviyoFormsTests/IAFWebViewModelTests.swift
…irect load The same-origin load path silently fell back to a direct URLRequest load if the template contents couldn't be read — which reintroduces the `Origin: null` the base URL exists to avoid. Capture and warn on the read failure so this degradation is diagnosable. Addresses CodeRabbit review on PR #614. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/KlaviyoForms/KlaviyoWebView/KlaviyoWebViewController.swift (1)
249-262:⚠️ Potential issue | 🟠 MajorCancel link-activated navigations after opening them externally and fix line length violations.
Line 251 opens the tapped URL in the system browser, but Line 262 still returns
.allow, so theWKWebViewcan navigate away from the form as well. Return.cancelfor handled links so external links do not also replace the in-app content.Additionally, lines 255 and 257 exceed the 110-character line length limit. Break these long Logger calls into multiple lines.
Suggested fix
if navigationAction.navigationType == .linkActivated, let url = navigationAction.request.url { let didOpenURL = await UIApplication.shared.open(url) if `#available`(iOS 14.0, *) { if didOpenURL { - Logger.webViewLogger.info("'UIApplication.shared.open(_:)' successfully opened URL '\(url.absoluteString)'") + Logger.webViewLogger.info( + "'UIApplication.shared.open(_:)' successfully opened URL " + + "'\(url.absoluteString)'" + ) } else { - Logger.webViewLogger.info("'UIApplication.shared.open(_:)' did not open the URL '\(url.absoluteString)'") + Logger.webViewLogger.info( + "'UIApplication.shared.open(_:)' did not open the URL " + + "'\(url.absoluteString)'" + ) } } + + return .cancel } return .allow🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/KlaviyoForms/KlaviyoWebView/KlaviyoWebViewController.swift` around lines 249 - 262, The method currently opens external links via UIApplication.shared.open(url) but then returns .allow, causing the WKWebView to also navigate away from the form. Change the return statement from .allow to .cancel within the linkActivated navigation check to prevent duplicate navigation after opening the URL externally. Additionally, the Logger.webViewLogger.info calls on lines 255 and 257 exceed the 110-character line length limit. Refactor both Logger.webViewLogger.info calls to break the long string concatenations across multiple lines, keeping the log messages readable while complying with the line length constraint.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/KlaviyoForms/KlaviyoWebView/KlaviyoWebViewController.swift`:
- Line 139: The Logger.webViewLogger.warning call in the
KlaviyoWebViewController exceeds the 110-character Swift line-length limit.
Refactor this line by breaking the string argument across multiple lines, either
by concatenating string segments or by moving the interpolated values to
intermediate variables, ensuring each resulting line stays within the
110-character limit while maintaining the same logging output and functionality.
---
Outside diff comments:
In `@Sources/KlaviyoForms/KlaviyoWebView/KlaviyoWebViewController.swift`:
- Around line 249-262: The method currently opens external links via
UIApplication.shared.open(url) but then returns .allow, causing the WKWebView to
also navigate away from the form. Change the return statement from .allow to
.cancel within the linkActivated navigation check to prevent duplicate
navigation after opening the URL externally. Additionally, the
Logger.webViewLogger.info calls on lines 255 and 257 exceed the 110-character
line length limit. Refactor both Logger.webViewLogger.info calls to break the
long string concatenations across multiple lines, keeping the log messages
readable while complying with the line length constraint.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 34a02f48-a2ea-4d75-ad51-fe757e7bbc02
📒 Files selected for processing (1)
Sources/KlaviyoForms/KlaviyoWebView/KlaviyoWebViewController.swift
ajaysubra
approved these changes
Jun 16, 2026
…ar limit Addresses CodeRabbit review on PR #614. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes a 401 on the In-App Forms authenticated
/api/profiles/fetch on iOS by loading the form template same-origin with the Klaviyo API host, eliminating theOrigin: nullthat afile://load produced.Due Diligence
Release/Versioning Considerations
PatchContains internal changes or backwards-compatible bug fixes.MinorContains changes to the public API.MajorContains breaking changes.feat/jwts-for-personalized-forms.Changelog / Code Overview
Root cause: The IAF
WKWebViewloaded the template from afile://URL, which has an opaque origin. Onsite's authenticatedGET /api/profiles/(cross-origin, withauthorization/revisionheaders) therefore triggered a CORS preflight carryingOrigin: null, which the API rejects — surfacing as a 401. The token was present in the DOM and onsite was attaching it correctly; the null origin was the only blocker.Fix: Load the template's HTML same-origin, anchored to the API host, so the profile fetch is same-origin (no preflight, no
Origin: null).KlaviyoWebViewModeling: newbaseURL: URL?protocol member (defaultnil, so other webviews are unaffected).IAFWebViewModel.baseURLreturnsenvironment.apiURL().url.KlaviyoWebViewController.loadUrl()usesloadHTMLString(_:baseURL:)when a base URL is supplied; falls back to the directURLRequestload otherwise.decidePolicyFornow keys external-link handling offnavigationAction.navigationType == .linkActivatedinstead of the template filename — required because same-origin loading changes the document URL to the API host, which previously misclassified the initial load as an external link and bounced it to Safari.Test Plan
xcodebuild test -scheme klaviyo-swift-sdk-Package -destination "platform=iOS Simulator,name=iPhone 17 Pro" -only-testing:KlaviyoFormsTests— all green (incl.testBaseURLMatchesAPIOriginForSameOriginRequests).GET /api/profiles/is same-origin (noOPTIONSpreflight) and returns 200.Related Issues/Tickets
Summary by CodeRabbit
Release Notes
Bug Fixes
/api/profiles/requests behave same-origin and avoid related CORS issues.Tests