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

Adding support for custom navigators? #1147

Closed
philjones88 opened this issue Aug 26, 2023 · 10 comments · Fixed by #1148
Closed

Adding support for custom navigators? #1147

philjones88 opened this issue Aug 26, 2023 · 10 comments · Fixed by #1148
Labels
Capacitor Capacitor is an open source native runtime for building Web Native apps. enhancement New feature or request

Comments

@philjones88
Copy link
Contributor

Would the maintainers be open to another PR focused at adding support for custom navigators? (Kinda did ok on the storage engine one, working well in a product with Electron!)

Scenario:

Building a React + Ionic + Capacitor app for iOS. It kinda works ok if you let it leave the app and redirect back.

Ideal solution is to use the Capacitor Browser plug-in to open it without leaving the app.

My idea:

Add 3 new settings to "UserManagerSettings".

That either get defaulted to the existing if not defined like here:

https://github.com/authts/oidc-client-ts/blob/main/src/UserManagerSettings.ts#L187

Then alter here to use the values from "UserManagerSettings":

https://github.com/authts/oidc-client-ts/blob/main/src/UserManager.ts#L98

@Badisi
Copy link
Contributor

Badisi commented Aug 26, 2023

For an integration with Capacitor, you can check my comment here: #537 Integration with Capacitor.

You can also use the VanillaJS version of my library (which is a wrapper around oidc-client-ts) as a "ready to use" solution and integrate it in your React app. Or even better, make a PR to support React as it was done with Angular.

@philjones88
Copy link
Contributor Author

We've already got wrappers around this Lib for RTK support.

I do think custom Navigators via the settings object is the cleanest approach and could be built in to extend other solutions other than capacitor?

@pamapa
Copy link
Member

pamapa commented Aug 28, 2023

if the changes are not that invasive i am open for that. The idea is that this library can also be used as a SDK like library, which can be extended easily.

@pamapa pamapa added the enhancement New feature or request label Aug 28, 2023
@philjones88
Copy link
Contributor Author

philjones88 commented Aug 28, 2023

This test here:
https://github.com/authts/oidc-client-ts/blob/main/src/UserManager.test.ts#L251

JSON.stringify's settings, which I added the Navigators too, but their constructors expect settings, so you get a nice:

 ● UserManager › signinRedirectCallback › should return a user

    TypeError: Converting circular structure to JSON
        --> starting at object with constructor 'RedirectNavigator'
        |     property '_settings' -> object with constructor 'UserManagerSettingsStore'
        --- property 'redirectNavigator' closes the circle
        at JSON.stringify (<anonymous>)

      249 |             const spy = jest.spyOn(subject["_client"], "processSigninResponse")
      250 |                 .mockResolvedValue({} as SigninResponse);
    > 251 |             await userStoreMock.set("test", JSON.stringify({
          |                                                  ^
      252 |                 id: "test",
      253 |                 request_type: "si:r",
      254 |                 ...subject.settings,

      at Object.<anonymous> (src/UserManager.test.ts:251:50)

Weirdly you can reduce the test's body to:

    describe("signinRedirectCallback", () => {
        it("should return a user", async () => {
            // arrange
            const spy = jest.spyOn(subject["_client"], "processSigninResponse")
                .mockResolvedValue({} as SigninResponse);

            // act
            const user = await subject.signinRedirectCallback("http://app/cb?state=test&code=code");

            // assert
            expect(user).toBeInstanceOf(User);
            spy.mockRestore();
        });
    });

And it passes, not exactly sure what they userStoreMock.set actually acheives? It's never checked or used apparently...

That test was the only issue it seems, removing that userStoreMock.set reduces the amount of changes down to a minimal set. 🎉

@philjones88
Copy link
Contributor Author

I'm going to do a full build iOS of my app using custom Navigators (redirect and iframe, for silent renew) using @Badisi 's capacitor logic as a base for the custom navigators. I put the initial PR up for feedback on the approach rather than it to be fully ready to merge. As I may have missed something key in my understanding.

@philjones88
Copy link
Contributor Author

philjones88 commented Aug 30, 2023

Overriding RedirectNavigator with an implemenetation similar to @Badisi works (we use React so in onSuccess of his class I had to add additional logic).

@Badisi have you hit an issue with Capacitor Http and JsonService ? Capacitor Http doesn't seem to support URLSearchParams being passed as body here when used for /connect/token POST https://github.com/VQComms/oidc-client-ts/blob/main/src/JsonService.ts#L129

see ionic-team/capacitor#6792 and ionic-team/capacitor#5945

@Badisi
Copy link
Contributor

Badisi commented Aug 31, 2023

This issue is related to Capacitor 5 which was released 4 months ago.
I had yet to test it but now that I did I'm also facing the same issue.

I have upvoted the issue #6792 and asked for clarification and high priority.

@pamapa pamapa added the Capacitor Capacitor is an open source native runtime for building Web Native apps. label Apr 24, 2024
@Excel1
Copy link

Excel1 commented Nov 14, 2024

How can i use the custom navigators? It is possible to use Capacitor Browser Plug-In now? Is so, how can i do it? :)

@Badisi
Copy link
Contributor

Badisi commented Nov 14, 2024

For an integration with Capacitor, you can check my comment here: #537 Integration with Capacitor.

You can also use the VanillaJS version of my library (which is a wrapper around oidc-client-ts) as a "ready to use" solution and integrate it in your React app. Or even better, make a PR to support React as it was done with Angular.

@Excel1, I explained how this can be achieved in the second post of this thread... ☝️

@Excel1
Copy link

Excel1 commented Nov 19, 2024

@Badisi thanks for help. I think it should be a good feature for oidc-client.ts since Apple forces by app publishment to use the safari api browser for redirection

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Capacitor Capacitor is an open source native runtime for building Web Native apps. enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants