wi3land / ionic-appauth

Intergration for OpenId/AppAuth-JS into Ionic V3/4/5
MIT License
93 stars 74 forks source link

Unable to generate PKCE challenge. Not using PKCE #73

Open StephanBis opened 2 years ago

StephanBis commented 2 years ago

When running our Ionic project with the command ionic capacitor run android -l --external we are unable to get this library working. We are getting the following error when the library is trying to fetch the OIDC well known config.

Unable to generate PKCE challenge. Not using PKCE. Message: "window.crypto.subtle is unavailable."

image

Does anyone know why this is occuring? We can view the well known config in the browser.

allidoisace commented 2 years ago

Same issue here but with iOS

philmmoore commented 2 years ago

Some here @allidoisace you had any luck finding a fix?

StephanBis commented 2 years ago

We didn't manage to find a solution. We opted for another way of authentication our users.

philmmoore commented 2 years ago

Ah ok @StephanBis, I've been able to work around it myself (I saw the issue on iOS 15) so just installed an older simulator (iOS14.5) and the issue is no longer present. It's only an issue for local development and the app will build and run as expected on iOS 15.

allidoisace commented 2 years ago

There were a few changes I made and got it working. I actually completely followed the demo app and removed the loadTokenFromStorage

philmmoore commented 2 years ago

@allidoisace do you have a repo where I can see the changes made at all?

Coder7777 commented 2 years ago

When running our Ionic project with the command ionic capacitor run android -l --external we are unable to get this library working. We are getting the following error when the library is trying to fetch the OIDC well known config.

Unable to generate PKCE challenge. Not using PKCE. Message: "window.crypto.subtle is unavailable."

image

Does anyone know why this is occuring? We can view the well known config in the browser.

same problem, it looks like only occurs on ios 15. I running the same code on iOS 14.8 and iOS 15.1 separately. the app running well in iOS 14.8, unfortunately the console print same error with you in iOS 15.1,

Did you figure it out?

StephanBis commented 2 years ago

@Coder7777 We have not been able to fix this, you might be on to something though. We were also running on iOS 15.

ghost commented 2 years ago

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch ionic-appauth@0.8.5 for the project I'm working on.

The underlaying AppAuthJS library uses the Web Crypto API implemented by your browser to achieve the PKCE challenge. See : https://developer.mozilla.org/en-US/docs/Web/API/Web_Crypto_API

When you run a Ionic / Capacitor application on localhost or local IP, most browsers (including Chrome) disable crypto.subtle beacause of security reasons. They consider it's not a secure origin.

The solution is allowing developers to choose the implementation of the Web Crypto API in the AuthService class (as an additionnal constructor's parameter).

By this way, you can use a pure JavaScript implementation like this package : https://www.npmjs.com/package/js-sha256

So you have to define a class which implements the interface provided by AppAuthJS :

export interface Crypto {
  /**
   * Generate a random string
   */
  generateRandom(size: number): string;
  /**
   * Compute the SHA256 of a given code.
   * This is useful when using PKCE.
   */
  deriveChallenge(code: string): Promise<string>;
}

Finally, give an instance of this class to the AuthService constructor.

Here is the diff that solved my problem (I will make a pull request for this) :

diff --git a/node_modules/ionic-appauth/lib/auth-service.d.ts b/node_modules/ionic-appauth/lib/auth-service.d.ts
index 070f8d8..935587a 100644
--- a/node_modules/ionic-appauth/lib/auth-service.d.ts
+++ b/node_modules/ionic-appauth/lib/auth-service.d.ts
@@ -1,4 +1,4 @@
-import { AuthorizationRequestHandler, StringMap } from '@openid/appauth';
+import { AuthorizationRequestHandler, Crypto, StringMap } from '@openid/appauth';
 import { IAuthAction } from './auth-action';
 import { UserInfoHandler } from './user-info-request-handler';
 import { EndSessionHandler } from './end-session-request-handler';
@@ -38,7 +38,7 @@ export declare class AuthService implements IAuthService {
     protected userInfoHandler: UserInfoHandler;
     protected requestHandler: AuthorizationRequestHandler;
     protected endSessionHandler: EndSessionHandler;
-    constructor(browser?: Browser, storage?: StorageBackend, requestor?: Requestor);
+    constructor(browser?: Browser, storage?: StorageBackend, requestor?: Requestor, crypto?: Crypto);
     /**
      * @deprecated independant observers have been replaced by Rxjs
      * this will be removed in a future release
diff --git a/node_modules/ionic-appauth/lib/auth-service.js b/node_modules/ionic-appauth/lib/auth-service.js
index 9545427..6f40ce3 100644
--- a/node_modules/ionic-appauth/lib/auth-service.js
+++ b/node_modules/ionic-appauth/lib/auth-service.js
@@ -13,10 +13,11 @@ import { AuthSubject } from './auth-subject';
 const TOKEN_RESPONSE_KEY = "token_response";
 const AUTH_EXPIRY_BUFFER = 10 * 60 * -1; // 10 mins in seconds
 export class AuthService {
-    constructor(browser = new DefaultBrowser(), storage = new LocalStorageBackend(), requestor = new JQueryRequestor()) {
+  constructor(browser = new DefaultBrowser(), storage = new LocalStorageBackend(), requestor = new JQueryRequestor(), crypto = new DefaultCrypto()) {
         this.browser = browser;
         this.storage = storage;
         this.requestor = requestor;
+        this.crypto = crypto;
         this._authSubject = new AuthSubject();
         this._actionHistory = new ActionHistoryObserver();
         this._session = new SessionObserver();
@@ -27,7 +28,7 @@ export class AuthService {
         this._initComplete = new BehaviorSubject(false);
         this.tokenHandler = new BaseTokenRequestHandler(requestor);
         this.userInfoHandler = new IonicUserInfoHandler(requestor);
-        this.requestHandler = new IonicAuthorizationRequestHandler(browser, storage);
+        this.requestHandler = new IonicAuthorizationRequestHandler(browser, storage, crypto);
         this.endSessionHandler = new IonicEndSessionHandler(browser);
     }
     /**
@@ -167,7 +168,7 @@ export class AuthService {
                     idTokenHint: this._tokenSubject.value.idToken || '',
                     state: state || undefined,
                 };
-                let request = new EndSessionRequest(requestJson);
+                let request = new EndSessionRequest(requestJson, this.crypto);
                 let returnedUrl = yield this.endSessionHandler.performEndSessionRequest(yield this.configuration, request);
                 //callback may come from showWindow or via another method
                 if (returnedUrl != undefined) {
@@ -190,7 +191,7 @@ export class AuthService {
                 extras: authExtras,
                 state: state || undefined,
             };
-            let request = new AuthorizationRequest(requestJson, new DefaultCrypto(), this.authConfig.pkce);
+            let request = new AuthorizationRequest(requestJson, this.crypto, this.authConfig.pkce);
             if (this.authConfig.pkce)
                 yield request.setupCodeVerifier();
             return this.requestHandler.performAuthorizationRequest(yield this.configuration, request);
diff --git a/node_modules/ionic-appauth/lib/authorization-request-handler.d.ts b/node_modules/ionic-appauth/lib/authorization-request-handler.d.ts
index 726f8b2..7f9167d 100644
--- a/node_modules/ionic-appauth/lib/authorization-request-handler.d.ts
+++ b/node_modules/ionic-appauth/lib/authorization-request-handler.d.ts
@@ -4,8 +4,8 @@ export declare const AUTHORIZATION_RESPONSE_KEY = "auth_response";
 export declare class IonicAuthorizationRequestHandler extends AuthorizationRequestHandler {
     private browser;
     private storage;
-    private generateRandom;
-    constructor(browser: Browser, storage: StorageBackend, utils?: BasicQueryStringUtils, generateRandom?: DefaultCrypto);
+    private crypto;
+  constructor(browser: Browser, storage: StorageBackend, crypto?: DefaultCrypto, utils?: BasicQueryStringUtils);
     performAuthorizationRequest(configuration: AuthorizationServiceConfiguration, request: AuthorizationRequest): Promise<void>;
     protected completeAuthorizationRequest(): Promise<AuthorizationRequestResponse>;
     private getAuthorizationRequest;
diff --git a/node_modules/ionic-appauth/lib/authorization-request-handler.js b/node_modules/ionic-appauth/lib/authorization-request-handler.js
index 4db7bf0..6aa6068 100644
--- a/node_modules/ionic-appauth/lib/authorization-request-handler.js
+++ b/node_modules/ionic-appauth/lib/authorization-request-handler.js
@@ -8,15 +8,14 @@ const authorizationRequestKey = (handle) => {
 const AUTHORIZATION_REQUEST_HANDLE_KEY = 'appauth_current_authorization_request';
 export const AUTHORIZATION_RESPONSE_KEY = "auth_response";
 export class IonicAuthorizationRequestHandler extends AuthorizationRequestHandler {
-    constructor(browser, storage, utils = new BasicQueryStringUtils(), generateRandom = new DefaultCrypto()) {
-        super(utils, generateRandom);
+  constructor(browser, storage, crypto = new DefaultCrypto(), utils = new BasicQueryStringUtils()) {
+        super(utils, crypto);
         this.browser = browser;
         this.storage = storage;
-        this.generateRandom = generateRandom;
     }
     performAuthorizationRequest(configuration, request) {
         return __awaiter(this, void 0, void 0, function* () {
-            let handle = this.generateRandom.generateRandom(10);
+            let handle = this.crypto.generateRandom(10);
             this.storage.setItem(AUTHORIZATION_REQUEST_HANDLE_KEY, handle);
             this.storage.setItem(authorizationRequestKey(handle), JSON.stringify(yield request.toJson()));
             let url = this.buildRequestUrl(configuration, request);
ghost commented 2 years ago

I made a PR : https://github.com/wi3land/ionic-appauth/pull/121

ElecBird commented 1 year ago

@MarvinDurot Are there changes that still need to be made for this merge? We desperately need this change. I see that the PR is still open.

vicebtx commented 5 months ago

Are there any changes on this ? This seems to be the quickest way to allow livereload and okta auth to work well together. Otherwise, we will have to create a ssl certificate for our localhost and manually install it on every device we are testing on or, simply, build and install the application. The latest is a bummer because we loose the advantages of livereloading.

stewones commented 2 months ago

well, that PR seems dead. I got it working locally with types and tests fixed, going to open a new one asap.