Add Capacitor support with native redirect handling and Angular integration - #180
Open
topdev-spetermann wants to merge 10 commits into
Open
Add Capacitor support with native redirect handling and Angular integration#180topdev-spetermann wants to merge 10 commits into
topdev-spetermann wants to merge 10 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Capacitor native-mobile support and Angular integration: navigator and storage abstractions/adapters, async session/storage plumbing, external redirect persistence and parsing, refresh-token helpers, Angular DI/provider extensions, and a full Capacitor+Angular example (Android/iOS projects and docs). ChangesCore library: native & async storage integration
Angular integration: types, providers, guard behavior
Example application: Capacitor + Angular sample and platform projects
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (1)
examples/capacitor-angular/android/app/build.gradle (1)
47-54: 💤 Low valueConsider a cleaner file existence check.
The current pattern works but relies on exception handling for flow control. Checking file existence explicitly before accessing
.textwould be more idiomatic.♻️ Proposed cleaner approach
try { def servicesJSON = file('google-services.json') - if (servicesJSON.text) { + if (servicesJSON.exists()) { apply plugin: 'com.google.gms.google-services' } } catch(Exception e) { logger.info("google-services.json not found, google-services plugin not applied. Push Notifications won't work") }🤖 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 `@examples/capacitor-angular/android/app/build.gradle` around lines 47 - 54, Replace the try/catch flow-control with an explicit file-existence check: test for the presence of 'google-services.json' using the file(...) object's exists() (or .isFile()) before reading servicesJSON.text and before calling apply plugin: 'com.google.gms.google-services'; remove the try/catch and log the same info message if the file does not exist, referencing the servicesJSON variable and the apply plugin: 'com.google.gms.google-services' line when making the change.
🤖 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 `@examples/capacitor-angular/android/.gitignore`:
- Around line 55-58: Un-ignore Android keystore patterns in the .gitignore by
enabling the entries for "*.jks" and "*.keystore" so signing keys are excluded
from commits; locate the commented patterns "#*.jks" and "#*..keystore" and
remove the leading "#" to ensure the glob patterns (*.jks, *.keystore) are
active in the .gitignore.
In `@examples/capacitor-angular/android/app/src/main/AndroidManifest.xml`:
- Line 5: The AndroidManifest currently has android:allowBackup="true" on the
Application declaration which allows system backups of app data; change this
attribute to android:allowBackup="false" in the <application> element to disable
backups for auth/session artifacts, and verify there are no backup-related
entries (e.g., android:backupAgent or android:fullBackupContent) that re-enable
backups; rebuild and test to confirm sensitive state is no longer included in
device backups.
In
`@examples/capacitor-angular/android/app/src/main/res/mipmap-anydpi-v26/ic_launcher.xml`:
- Line 4: The adaptive icon XML is referencing the foreground as a mipmap but
the resource is in drawable; update the foreground attribute in the adaptive
icon files (ic_launcher.xml and ic_launcher_round.xml) to use
`@drawable/ic_launcher_foreground` instead of `@mipmap/ic_launcher_foreground` so
the resource type matches the actual drawable-v24/ic_launcher_foreground.xml
resource and fixes Android resource linking.
In `@examples/capacitor-angular/android/app/src/main/res/xml/file_paths.xml`:
- Around line 3-4: The <external-path name="my_images" path="." /> and
<cache-path name="my_cache_images" path="." /> entries in file_paths.xml are too
broad; replace the path="." values with restrictive subdirectories your app
actually uses (for example path="images/" or path="shared/" for external media
and path="cache/images/" for cached files) so FileProvider only exposes those
specific folders; update any code that constructs URIs or expects file locations
(where you reference these providers) to match the new subdirectory names and
verify Intent grants still work with the narrowed paths.
In `@examples/capacitor-angular/android/build.gradle`:
- Around line 10-11: The specified plugin coordinates
"com.android.tools.build:gradle" and "com.google.gms:google-services" use
versions that cannot be resolved; update the classpath declarations to use
versions that exist in your configured Maven repositories (for example replace
the version numbers after com.android.tools.build:gradle and
com.google.gms:google-services with verified, resolvable versions) or add the
repository that hosts those versions to your buildscript repositories; verify
resolution by running a dependency sync/gradle build to ensure the new versions
for the Android Gradle plugin and Google Services plugin resolve correctly.
In `@examples/capacitor-angular/README.md`:
- Line 116: The fenced architecture code block in README.md is missing a
language identifier (MD040); update the opening triple-backtick for the ASCII
diagram to include a language tag (for example change ``` to ```text) so the
block is properly identified and passes markdown linting—locate the diagram
block (the ASCII "Angular Application" architecture fence) and add the language
identifier to the opening fence.
In `@examples/capacitor-angular/src/app/app.config.ts`:
- Line 33: The config currently always sets postLoginRedirectUrl to a native
deep-link which breaks web login; update the config so postLoginRedirectUrl is
only assigned when isNativeApp is true (e.g., set it to the deep-link when
isNativeApp === true and leave it undefined/omit it for web). Apply the same
conditional treatment to the related postLogoutRedirectUrl entry as noted (both
symbols: postLoginRedirectUrl and postLogoutRedirectUrl) so web mode uses the
browser's current URL while native uses the deep-link.
In `@examples/capacitor-angular/src/app/pages/protected.ts`:
- Around line 48-50: The renewTokens method currently awaits
this.oidc.renewTokens() with no error handling; wrap the call in a try/catch (or
handle the returned promise) inside renewTokens to catch renewal failures, log
the error (include error details) and surface user feedback (e.g., call a
notification method or redirect to login) so the app doesn't leave an unhandled
rejection or silent failure; update the renewTokens function and any UI helper
used for notifications accordingly.
In `@src/capacitor/CapacitorNavigator.ts`:
- Around line 245-269: Wrap the native redirect flow async calls in try-catch
blocks: in the appUrlOpen handler around setExternalRedirectUrl(...) and
`#closeBrowserIfNotAndroid`(), catch errors, call this.#emitWarning(...) with the
caught error and context ("appUrlOpen"), and return (do not rethrow); in the
navigate method wrap Browser.open(...) and setExternalRedirectUrl(...) so after
running cleanup methods (`#clearBrowserFinishedTimeout`,
`#cleanupBrowserFinishedListenerRemove`, `#cleanupListener`) you emit a warning via
this.#emitWarning(...) and then rethrow the error to propagate failure; in the
callback and close handlers wrap setExternalRedirectUrl(...) and
Browser.close(...) respectively, emit warnings with this.#emitWarning(...) on
error and do not rethrow; ensure you still call `#getRequiredInitialization`() and
other cleanup before/after the try-catch as needed so listeners/timeouts are not
left attached.
In `@src/core/createOidc.ts`:
- Around line 643-658: The code currently computes oidcCallbackUrl for native
apps but continues to use homeUrlAndRedirectUri/validRedirectUri in subsequent
reporting/logging; change the logic so that wherever validRedirectUri or any
troubleshooting/log messages reference the registered redirect URI they use
oidcCallbackUrl when isNativeApp is true (i.e., prefer oidcCallbackUrl over
homeUrlAndRedirectUri/postLoginRedirectUrl_default for native flows); update the
construction/assignment of validRedirectUri (and any logs that print
homeUrlAndRedirectUri) to resolve to oidcCallbackUrl when isNativeApp is true so
native integrators see the actual callback URI to register.
- Around line 1096-1101: When restoring from session storage (inside the
restore_from_session_storage block) we must not return a cached getUser() when
there is a pending native callback; update the conditional that checks
shouldPreferLocalRestoreInNative and
evtIsThereMoreThanOneInstanceThatCantUserIframes.current to also ensure no
pending native callback/external redirect is present (e.g. check
externalRedirectUrl or the auth callback flag) before breaking out and returning
getUser(); in other words, defer preferring local restore until any pending
native callback has been consumed. Apply the same guard to the analogous code
region around lines 1162-1191 so both restore paths wait for callback handling
before short-circuiting with a persisted user.
In `@src/core/externalRedirectUrl.ts`:
- Line 13: The in-memory cache externalRedirectUrlByConfigId_memory currently
stores only the URL so reads bypass MAX_AGE_MS; change the map to store an
object with timestamp (e.g., { url: string, ts: number }) and update all
read/write sites (the code paths around the current checks at the regions you
flagged - ~112-116 and ~151-153) so writers set ts = Date.now() and readers
validate Date.now() - ts <= MAX_AGE_MS before returning the cached URL
(otherwise fall through to refresh and update the map). Ensure references to
externalRedirectUrlByConfigId_memory are adjusted to use the new shape and that
MAX_AGE_MS is used for invalidation.
In `@src/core/loginOrGoToAuthServer.ts`:
- Around line 329-331: The preRedirectHook await (rest.preRedirectHook) can
reject and leave the login latch stuck; wrap the await in a try/catch (or
try/finally) so that on rejection you reset the login latch/lock before
rethrowing the error, and apply the same change to the other occurrence around
signinRedirect handling (the block near lines 366-368) so both
rest.preRedirectHook and signinRedirect failure paths always clear the login
latch.
---
Nitpick comments:
In `@examples/capacitor-angular/android/app/build.gradle`:
- Around line 47-54: Replace the try/catch flow-control with an explicit
file-existence check: test for the presence of 'google-services.json' using the
file(...) object's exists() (or .isFile()) before reading servicesJSON.text and
before calling apply plugin: 'com.google.gms.google-services'; remove the
try/catch and log the same info message if the file does not exist, referencing
the servicesJSON variable and the apply plugin: 'com.google.gms.google-services'
line when making the change.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ad836b70-6519-4bdd-8761-3229b6ad3451
⛔ Files ignored due to path filters (32)
examples/capacitor-angular/android/app/src/main/res/drawable-land-hdpi/splash.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/drawable-land-mdpi/splash.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/drawable-land-xhdpi/splash.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/drawable-land-xxhdpi/splash.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/drawable-land-xxxhdpi/splash.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/drawable-port-hdpi/splash.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/drawable-port-mdpi/splash.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/drawable-port-xhdpi/splash.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/drawable-port-xxhdpi/splash.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/drawable-port-xxxhdpi/splash.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/drawable/splash.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/mipmap-hdpi/ic_launcher.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/mipmap-hdpi/ic_launcher_foreground.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/mipmap-hdpi/ic_launcher_round.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/mipmap-mdpi/ic_launcher.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/mipmap-mdpi/ic_launcher_foreground.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/mipmap-mdpi/ic_launcher_round.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/mipmap-xhdpi/ic_launcher.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/mipmap-xhdpi/ic_launcher_foreground.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/mipmap-xhdpi/ic_launcher_round.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/mipmap-xxhdpi/ic_launcher.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/mipmap-xxhdpi/ic_launcher_foreground.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/mipmap-xxhdpi/ic_launcher_round.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/mipmap-xxxhdpi/ic_launcher.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/mipmap-xxxhdpi/ic_launcher_foreground.pngis excluded by!**/*.pngexamples/capacitor-angular/android/app/src/main/res/mipmap-xxxhdpi/ic_launcher_round.pngis excluded by!**/*.pngexamples/capacitor-angular/android/gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jarexamples/capacitor-angular/ios/App/App/Assets.xcassets/AppIcon.appiconset/AppIcon-512@2x.pngis excluded by!**/*.pngexamples/capacitor-angular/ios/App/App/Assets.xcassets/Splash.imageset/splash-2732x2732-1.pngis excluded by!**/*.pngexamples/capacitor-angular/ios/App/App/Assets.xcassets/Splash.imageset/splash-2732x2732-2.pngis excluded by!**/*.pngexamples/capacitor-angular/ios/App/App/Assets.xcassets/Splash.imageset/splash-2732x2732.pngis excluded by!**/*.pngyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (76)
examples/capacitor-angular/.gitignoreexamples/capacitor-angular/README.mdexamples/capacitor-angular/android/.gitignoreexamples/capacitor-angular/android/app/.gitignoreexamples/capacitor-angular/android/app/build.gradleexamples/capacitor-angular/android/app/capacitor.build.gradleexamples/capacitor-angular/android/app/proguard-rules.proexamples/capacitor-angular/android/app/src/androidTest/java/com/getcapacitor/myapp/ExampleInstrumentedTest.javaexamples/capacitor-angular/android/app/src/main/AndroidManifest.xmlexamples/capacitor-angular/android/app/src/main/java/com/oidcspa/capacitor/MainActivity.javaexamples/capacitor-angular/android/app/src/main/res/drawable-v24/ic_launcher_foreground.xmlexamples/capacitor-angular/android/app/src/main/res/drawable/ic_launcher_background.xmlexamples/capacitor-angular/android/app/src/main/res/layout/activity_main.xmlexamples/capacitor-angular/android/app/src/main/res/mipmap-anydpi-v26/ic_launcher.xmlexamples/capacitor-angular/android/app/src/main/res/mipmap-anydpi-v26/ic_launcher_round.xmlexamples/capacitor-angular/android/app/src/main/res/values/ic_launcher_background.xmlexamples/capacitor-angular/android/app/src/main/res/values/strings.xmlexamples/capacitor-angular/android/app/src/main/res/values/styles.xmlexamples/capacitor-angular/android/app/src/main/res/xml/file_paths.xmlexamples/capacitor-angular/android/app/src/test/java/com/getcapacitor/myapp/ExampleUnitTest.javaexamples/capacitor-angular/android/build.gradleexamples/capacitor-angular/android/capacitor.settings.gradleexamples/capacitor-angular/android/gradle.propertiesexamples/capacitor-angular/android/gradle/wrapper/gradle-wrapper.propertiesexamples/capacitor-angular/android/gradlewexamples/capacitor-angular/android/gradlew.batexamples/capacitor-angular/android/settings.gradleexamples/capacitor-angular/android/variables.gradleexamples/capacitor-angular/angular.jsonexamples/capacitor-angular/capacitor.config.tsexamples/capacitor-angular/ios/.gitignoreexamples/capacitor-angular/ios/App/App.xcodeproj/project.pbxprojexamples/capacitor-angular/ios/App/App.xcworkspace/xcshareddata/IDEWorkspaceChecks.plistexamples/capacitor-angular/ios/App/App/AppDelegate.swiftexamples/capacitor-angular/ios/App/App/Assets.xcassets/AppIcon.appiconset/Contents.jsonexamples/capacitor-angular/ios/App/App/Assets.xcassets/Contents.jsonexamples/capacitor-angular/ios/App/App/Assets.xcassets/Splash.imageset/Contents.jsonexamples/capacitor-angular/ios/App/App/Base.lproj/LaunchScreen.storyboardexamples/capacitor-angular/ios/App/App/Base.lproj/Main.storyboardexamples/capacitor-angular/ios/App/App/Info.plistexamples/capacitor-angular/ios/App/Podfileexamples/capacitor-angular/package.jsonexamples/capacitor-angular/src/app/app.config.tsexamples/capacitor-angular/src/app/app.routes.tsexamples/capacitor-angular/src/app/app.tsexamples/capacitor-angular/src/app/pages/protected.tsexamples/capacitor-angular/src/app/pages/public.tsexamples/capacitor-angular/src/app/services/oidc.service.tsexamples/capacitor-angular/src/index.htmlexamples/capacitor-angular/src/main.lazy.tsexamples/capacitor-angular/src/main.tsexamples/capacitor-angular/src/styles.scssexamples/capacitor-angular/tsconfig.app.jsonexamples/capacitor-angular/tsconfig.jsonexamples/capacitor-angular/tsconfig.spec.jsonpackage.jsonsrc/angular.tssrc/capacitor/CapacitorNavigator.tssrc/capacitor/CapacitorStorage.tssrc/capacitor/angular.tssrc/capacitor/index.tssrc/core/BaseNavigator.tssrc/core/createOidc.tssrc/core/earlyInit.tssrc/core/externalRedirectUrl.tssrc/core/getRefreshTokenExpirationTime.tssrc/core/loginOrGoToAuthServer.tssrc/core/oidcClientTsUserToTokens.tssrc/core/parseOidcRedirectUrl.tssrc/core/persistedAuthState.tssrc/tools/lazyAsyncSessionStorage.tssrc/tools/lazySessionStorage.tssrc/tools/localStorageAdapter.tssrc/tools/sessionStorageAdapter.tssrc/tools/toNumber.tssrc/vendor/frontend/oidc-client-ts.ts
💤 Files with no reviewable changes (1)
- src/tools/lazySessionStorage.ts
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 `@src/capacitor/angular.ts`:
- Around line 17-19: The InjectionToken CAPACITOR_NAVIGATOR is declared as
InjectionToken<CapacitorNavigator> but the factory (effectiveNavigator) can
return undefined in web mode, so change the token's type to allow undefined
(InjectionToken<CapacitorNavigator | undefined>) or adjust the provider to
always return a non-undefined object; update the declaration of
CAPACITOR_NAVIGATOR and any related provider/factory that references
effectiveNavigator so callers get a correct compile-time type (or a stable
fallback) and handle the undefined case safely.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ac150d57-13d6-43bf-a6e0-44275eb5752a
📒 Files selected for processing (3)
examples/capacitor-angular/README.mdsrc/capacitor/CapacitorNavigator.tssrc/capacitor/angular.ts
✅ Files skipped from review due to trivial changes (1)
- examples/capacitor-angular/README.md
Collaborator
|
Sorry I didn't get a chance to review it yet but I will. |
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.
Fixes #37
Summary
This PR adds Capacitor support to
oidc-spawhile aiming to preserve the existing behavior for current browser users who do not opt into the new parameters.The implementation keeps the existing web flow intact and adds a native-specific path for Capacitor apps, including redirect handling, storage integration, and an Angular-friendly setup.
Main changes
oidc-spa/capacitorentrypointCapacitorNavigatorfor native login/logout redirects through CapacitorCapacitorPreferencesStorageAdapterfor Capacitor-backed persistenceoidc-spa/capacitor/angularwith a native-oriented Angular integrationstorageAdapter,tokenStorageAdapter,navigator,isNativeApp, andnativeSessionRestoreModeparametersexternalRedirectUrlexamples/capacitor-angularexample project for Android and iOSDesign notes
stateStoreintentionally remains onlocalStoragefor compatibility with synchronous early-init and iframe-based flowswindow.location.hrefCompatibility
Validation
Review notes
I am happy to adapt the public API, naming, internal structure, or integration shape if that makes upstreaming easier, as long as the Capacitor integration requirements remain covered.
If you prefer a different surface for the Angular integration or a different split between internal helpers and public API, I can adjust the PR accordingly.
Review focus
The most important areas to review are:
Summary by CodeRabbit
New Features
Documentation
Refactor