Refactor pkceDisabled to pkceEnabled (#89)

capacitor-community · Apr 20, 2020 · 360a308 · 360a308
1 parent f376de5
commit 360a308
Show file tree

Hide file tree

Showing 7 changed files with 52 additions and 44 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,7 +5,8 @@
 ### Breaking
 * Core: Capacitor 2.x is new minimum peer dependency. closes #80.
 * `responseType` is required. Default values were removed. In favor of configuring anything. closes #86.
-* Configuration: If a flow must not have a `accessTokenEndpoint` but you configured one as base parameter you have to
+* `pkceDisabled` was replaced with `pkceEnabled`, which is NOT enabled by default. If you like to use PKCE set this to true.
+* If a flow must not have a `accessTokenEndpoint` but you configured one as base parameter you have to
 overwrite it in the according platform sections. `accessTokenEndpoint: ""` see Google example in README.
 * Add `redirectUrl` to base parameter and make it overwritable in the platform sections. closes #84.
   * Android: `customScheme` replaced by `redirectUrl`

diff --git a/README.md b/README.md
@@ -35,14 +35,30 @@ These parameters are:
 e.g.
 
 If `responseType=code`, `pkceDisable=true` and `accessTokenEndpoint` is missing the `authorizationCode` will be resolve along with the whole authorization response.
+This only works for web and Android. On iOS the used lib does not allows to cancel after the authorization request see #13
 
-### Implicit flow (response type: token)
+See the excellent article about OAuth2 response type combinations.
 
-Status: **ok**
+https://medium.com/@darutk/diagrams-of-all-the-openid-connect-flows-6968e3990660
 
-### Code flow + PKCE (response type: code)
+### Tested / working flows
 
-Status: **ok**
+These flows are already working and were tested by me.
+
+#### Implicit flow
+
+```
+responseType: "token"
+```
+
+#### Code flow + PKCE
+
+```
+...
+responseType: "code"
+pkceEnable: true
+...
+```
 
 Please be aware that some providers (OneDrive, Auth0) allow **Code Flow + PKCE** only for native apps. Web apps have to use implicit flow.
 

diff --git a/android/src/main/java/com/byteowls/capacitor/oauth2/OAuth2ClientPlugin.java b/android/src/main/java/com/byteowls/capacitor/oauth2/OAuth2ClientPlugin.java
@@ -39,10 +39,8 @@ public class OAuth2ClientPlugin extends Plugin {
     private static final String PARAM_STATE = "state";
 
     private static final String PARAM_ACCESS_TOKEN_ENDPOINT = "accessTokenEndpoint";
-    private static final String PARAM_PKCE_DISABLED = "pkceDisabled";
+    private static final String PARAM_PKCE_ENABLED = "pkceEnabled";
     private static final String PARAM_RESOURCE_URL = "resourceUrl";
-    private static final String RESPONSE_TYPE_CODE = "code";
-    private static final String RESPONSE_TYPE_TOKEN = "token";
     private static final String PARAM_ADDITIONAL_PARAMETERS = "additionalParameters";
     private static final String PARAM_ANDROID_CUSTOM_HANDLER_CLASS = "android.customHandlerClass";
     // Activity result handling
@@ -230,7 +228,7 @@ public void onError(Exception error) {
                 builder.setState(oauth2Options.getState());
             }
             builder.setScope(oauth2Options.getScope());
-            if (!oauth2Options.isPkceDisabled()) {
+            if (oauth2Options.isPkceEnabled()) {
                 builder.setCodeVerifier(oauth2Options.getPkceCodeVerifier());
             } else {
                 builder.setCodeVerifier(null);
@@ -408,20 +406,15 @@ OAuth2Options buildAuthenticateOptions(JSObject callData) {
         o.setAppId(ConfigUtils.trimToNull(ConfigUtils.getOverwrittenAndroidParam(String.class, callData, PARAM_APP_ID)));
         o.setAuthorizationBaseUrl(ConfigUtils.trimToNull(ConfigUtils.getOverwrittenAndroidParam(String.class, callData, PARAM_AUTHORIZATION_BASE_URL)));
         o.setResponseType(ConfigUtils.trimToNull(ConfigUtils.getOverwrittenAndroidParam(String.class, callData, PARAM_RESPONSE_TYPE)));
-        if (o.getResponseType() == null) {
-            // fallback to token
-            o.setResponseType(RESPONSE_TYPE_TOKEN);
-        }
         o.setRedirectUrl(ConfigUtils.trimToNull(ConfigUtils.getOverwrittenAndroidParam(String.class, callData, PARAM_REDIRECT_URL)));
 
         // optional
         o.setResourceUrl(ConfigUtils.trimToNull(ConfigUtils.getOverwrittenAndroidParam(String.class, callData, PARAM_RESOURCE_URL)));
         o.setAccessTokenEndpoint(ConfigUtils.trimToNull(ConfigUtils.getOverwrittenAndroidParam(String.class, callData, PARAM_ACCESS_TOKEN_ENDPOINT)));
-        o.setPkceDisabled(ConfigUtils.getOverwrittenAndroidParam(Boolean.class, callData, PARAM_PKCE_DISABLED));
-        if (RESPONSE_TYPE_CODE.equals(o.getResponseType())) {
-            if (!o.isPkceDisabled()) {
-                o.setPkceCodeVerifier(ConfigUtils.getRandomString(64));
-            }
+        Boolean pkceEnabledObj = ConfigUtils.getOverwrittenAndroidParam(Boolean.class, callData, PARAM_PKCE_ENABLED);
+        o.setPkceEnabled(pkceEnabledObj == null ? false : pkceEnabledObj);
+        if (o.isPkceEnabled()) {
+            o.setPkceCodeVerifier(ConfigUtils.getRandomString(64));
         }
 
         o.setScope(ConfigUtils.trimToNull(ConfigUtils.getOverwrittenAndroidParam(String.class, callData, PARAM_SCOPE)));

diff --git a/android/src/main/java/com/byteowls/capacitor/oauth2/OAuth2Options.java b/android/src/main/java/com/byteowls/capacitor/oauth2/OAuth2Options.java
@@ -20,7 +20,7 @@ public class OAuth2Options {
     private String accessTokenEndpoint;
     private String resourceUrl;
 
-    private boolean pkceDisabled;
+    private boolean pkceEnabled;
     private String pkceCodeVerifier;
     private Map<String, String> additionalParameters;
 
@@ -107,12 +107,12 @@ public void setCustomHandlerClass(String customHandlerClass) {
         this.customHandlerClass = customHandlerClass;
     }
 
-    public boolean isPkceDisabled() {
-        return pkceDisabled;
+    public boolean isPkceEnabled() {
+        return pkceEnabled;
     }
 
-    public void setPkceDisabled(boolean pkceDisabled) {
-        this.pkceDisabled = pkceDisabled;
+    public void setPkceEnabled(boolean pkceEnabled) {
+        this.pkceEnabled = pkceEnabled;
     }
 
     public String getPkceCodeVerifier() {

diff --git a/src/definitions.ts b/src/definitions.ts
@@ -80,9 +80,9 @@ export interface OAuth2AuthenticateBaseOptions {
      */
     resourceUrl?: string;
     /**
-     * PKCE is enabled by default when using @responseType 'code'. This options disables it if needed.
+     * Enable PKCE if you need it.
      */
-    pkceDisabled?: boolean;
+    pkceEnabled?: boolean;
     /**
      * A space-delimited list of permissions that identify the resources that your application could access on the user's behalf.
      * If you want to get a refresh token, you most likely will need the offline_access scope (only supported in Code Flow!)

diff --git a/src/web-utils.test.ts b/src/web-utils.test.ts
@@ -7,12 +7,12 @@ const googleOptions: OAuth2AuthenticateOptions = {
     accessTokenEndpoint: "https://www.googleapis.com/oauth2/v4/token",
     scope: "email profile",
     resourceUrl: "https://www.googleapis.com/userinfo/v2/me",
-    pkceDisabled: false,
+    pkceEnabled: false,
     web: {
         accessTokenEndpoint: "",
         redirectUrl: "https://oauth2.byteowls.com/authorize",
         appId: "webAppId",
-        pkceDisabled: true
+        pkceEnabled: true
     },
     android: {
         responseType: "code",
@@ -33,7 +33,7 @@ const oneDriveOptions: OAuth2AuthenticateOptions = {
     },
     web: {
         redirectUrl: "https://oauth2.byteowls.com/authorize",
-        pkceDisabled: true,
+        pkceEnabled: false,
         additionalParameters: {
             "resource": "resource_id",
             "emptyParam": null,
@@ -81,8 +81,8 @@ describe('base options processing', () => {
     });
 
     it('should build a overwritable boolean value', () => {
-        const pkceDisabled = WebUtils.getOverwritableValue<boolean>(googleOptions, "pkceDisabled");
-        expect(pkceDisabled).toBeTruthy();
+        const pkceEnabled = WebUtils.getOverwritableValue<boolean>(googleOptions, "pkceEnabled");
+        expect(pkceEnabled).toBeTruthy();
     });
 
     it('should build a overwritable additional parameters map', () => {

diff --git a/src/web-utils.ts b/src/web-utils.ts
@@ -108,19 +108,17 @@ export class WebUtils {
         webOptions.resourceUrl = this.getOverwritableValue(configOptions, "resourceUrl");
         webOptions.accessTokenEndpoint = this.getOverwritableValue(configOptions, "accessTokenEndpoint");
 
-        webOptions.pkceDisabled = this.getOverwritableValue(configOptions, "pkceDisabled");
-        if (webOptions.responseType === "code") {
-            if (!webOptions.pkceDisabled) {
-                webOptions.pkceCodeVerifier = this.randomString(64);
-                if (CryptoUtils.HAS_SUBTLE_CRYPTO) {
-                    await CryptoUtils.deriveChallenge(webOptions.pkceCodeVerifier).then(c => {
-                        webOptions.pkceCodeChallenge = c;
-                        webOptions.pkceCodeChallengeMethod = "S256";
-                    });
-                } else {
-                    webOptions.pkceCodeChallenge = webOptions.pkceCodeVerifier;
-                    webOptions.pkceCodeChallengeMethod = "plain";
-                }
+        webOptions.pkceEnabled = this.getOverwritableValue(configOptions, "pkceEnabled");
+        if (webOptions.pkceEnabled) {
+            webOptions.pkceCodeVerifier = this.randomString(64);
+            if (CryptoUtils.HAS_SUBTLE_CRYPTO) {
+                await CryptoUtils.deriveChallenge(webOptions.pkceCodeVerifier).then(c => {
+                    webOptions.pkceCodeChallenge = c;
+                    webOptions.pkceCodeChallengeMethod = "S256";
+                });
+            } else {
+                webOptions.pkceCodeChallenge = webOptions.pkceCodeVerifier;
+                webOptions.pkceCodeChallengeMethod = "plain";
             }
         }
         webOptions.scope = this.getOverwritableValue(configOptions, "scope");
@@ -222,7 +220,7 @@ export class WebOptions {
     windowOptions: string;
     windowTarget: string = "_blank";
 
-    pkceDisabled: boolean;
+    pkceEnabled: boolean;
     pkceCodeVerifier: string;
     pkceCodeChallenge: string;
     pkceCodeChallengeMethod: string;