feat: configure upstream IdP client_auth_method (#659)

* add database migration to persist `client_auth_method` * update API types + AuthProvider entity and handle upstream payload properly * bump nightly version * include new values in the UI * write changelog * always add `client_id` to the upstream `/token` request body, no matter what * additional endpoint validation on PUT provider for better UX * fix: unable to remove root PEM for upstream IdP * bump nightly version * add PR link to changelog
sebadob · Dec 19, 2024 · 80af025 · 80af025
1 parent abaf240
commit 80af025
Show file tree

Hide file tree

Showing 11 changed files with 240 additions and 41 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,37 @@
 # Changelog
 
+## UNRELEASED
+
+### Changes
+
+#### Upstream Identity Providers
+
+To provide additional compatibility for some upstream providers like Active Directory Federation Severices,
+some changes have been applied to Rauthy's behavior.
+
+The first thing is that the HTTP client used for upstream Logins does not force TLS v1.3 anymore, but also allows
+TLS v1.2. Both v1.2 and v1.3 are considered being secure by current standards. This is necessary, because some OS'es
+like Windows Server 2019 do not support TLS 1.3.
+
+The second change is for the way upstream providers are configured. The behavior until now was, that Rauthy added the
+client credentials as both Basic Authentication in headers, and in the body for maximum compatibility. However, some
+IdP'S (like ADFS for instance) complain about this and only expect it in one place.
+To make this happen, there are 2 new fields for the upstream IdP configuration:
+
+- `client_secret_basic: bool`
+- `client_secret_post: bool`
+
+These are available as switches in the Admin UI for each upstream provider. To not introduce breaking changes, all
+possibly existing configurations will have both options enabled like it has been up until now.
+
+[#659](https://github.com/sebadob/rauthy/pull/659)
+
+#### Note
+
+Even though this changes the request and response objects on the API, this change is **NOT** being handled as
+a breaking change. API clients are forbidden to modify upstream IdPs for security reasons, which means this change
+should only affect the Rauthy Admin UI.
+
 ## v0.27.2
 
 ### Changes

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -4,7 +4,7 @@ members = ["src/*"]
 exclude = ["rauthy-client"]
 
 [workspace.package]
-version = "0.27.2"
+version = "0.27.3-20241218-1"
 edition = "2021"
 authors = ["Sebastian Dobe <[email protected]>"]
 license = "Apache-2.0"

diff --git a/frontend/src/components/admin/providers/JsonPathDesc.svelte b/frontend/src/components/admin/providers/JsonPathDesc.svelte
@@ -1,7 +1,7 @@
 <script></script>
 
 <div class="desc">
-    <p>ID token claim mappings</p>
+    <h4>ID token claim mappings</h4>
     <p>
         Values from the ID token after a successful upstream login can be mapped automatically.
     </p>
@@ -25,6 +25,10 @@
 </div>
 
 <style>
+    h4 {
+        margin-bottom: .5rem;
+    }
+
     .desc {
         display: flex;
         flex-direction: column;

diff --git a/frontend/src/components/admin/providers/ProviderConfig.svelte b/frontend/src/components/admin/providers/ProviderConfig.svelte
@@ -62,7 +62,6 @@
         client_id: yup.string().trim().matches(REGEX_URI, "Can only contain URI safe characters, length max: 128"),
         client_secret: yup.string().trim().max(256, "Max 256 characters"),
         scope: yup.string().trim().matches(REGEX_PROVIDER_SCOPE, "Can only contain: 'a-zA-Z0-9-_/ ', length max: 128"),
-        root_pem: yup.string().trim().nullable().matches(REGEX_PEM, "Invalid PEM certificate"),
 
         admin_claim_path: yup.string().trim().nullable().matches(REGEX_URI, "Can only contain URI safe characters, length max: 128"),
         admin_claim_value: yup.string().trim().nullable().matches(REGEX_URI, "Can only contain URI safe characters, length max: 128"),
@@ -77,9 +76,12 @@
     }
 
     async function onSubmit() {
+        if (!showRootPem) {
+            provider.root_pem = undefined;
+        }
+
         const valid = await validateForm();
         if (!valid) {
-            err = 'Invalid input';
             return;
         }
 
@@ -95,6 +97,9 @@
             // make sure we reset to false, which is what a user would expect
             provider.danger_allow_insecure = false;
             provider.root_pem = provider.root_pem.trim();
+        } else {
+            // make sure to not submit am empty string
+            provider.root_pem = undefined;
         }
 
         let res = await putProvider(provider.id, provider);
@@ -125,9 +130,21 @@
     }
 
     async function validateForm() {
+        formErrors = {};
         try {
             await schema.validate(provider, {abortEarly: false});
-            formErrors = {};
+
+            if (provider.client_secret && !(provider.client_secret_basic || provider.client_secret_post)) {
+                err = 'You have given a client secret, but no client auth method is active';
+                return false;
+            } else if (provider.root_pem && provider.root_pem.length > 0) {
+                if (!REGEX_PEM.test(provider.root_pem.trim())) {
+                    formErrors.root_pem = 'Invalid PEM certificate';
+                }
+            } else {
+                err = 'Invalid input';
+            }
+
             return true;
         } catch (err) {
             formErrors = extractFormErrors(err);
@@ -293,6 +310,30 @@
         CLIENT SECRET
     </PasswordInput>
 
+    <div class="desc">
+        <p>
+            The authentication method to use on the <code>/token</code> endpoint.<br>
+            Most providers should work with <code>basic</code>, some only with <code>post</code>.
+            In rare situations, you need both, while it can lead to errors with others.
+        </p>
+    </div>
+    <div class="switchRow">
+        <div>
+            client_secret_basic
+        </div>
+        <Switch
+                bind:selected={provider.client_secret_basic}
+        />
+    </div>
+    <div class="switchRow">
+        <div>
+            client_secret_post
+        </div>
+        <Switch
+                bind:selected={provider.client_secret_post}
+        />
+    </div>
+
     <JsonPathDesc/>
     <div class="desc">
         <p>
@@ -420,6 +461,13 @@
         color: var(--col-ok);
     }
 
+    .switchRow {
+        margin-bottom: .25rem;
+        padding-left: .5rem;
+        display: grid;
+        grid-template-columns: 9rem 1fr;
+    }
+
     .unit {
         margin: 7px 0;
     }

diff --git a/frontend/src/components/admin/providers/ProviderTileAddNew.svelte b/frontend/src/components/admin/providers/ProviderTileAddNew.svelte
@@ -50,6 +50,8 @@
         token_auth_method_basic: false,
         userinfo_endpoint: '',
         use_pkce: true,
+        client_secret_basic: true,
+        client_secret_post: false,
 
         // user defined values
         name: '',
@@ -122,6 +124,8 @@
                     token_auth_method_basic: false,
                     userinfo_endpoint: 'https://api.github.com/user',
                     use_pkce: false,
+                    client_secret_basic: true,
+                    client_secret_post: true,
 
                     // user defined values
                     name: 'Github',
@@ -160,6 +164,8 @@
                     token_auth_method_basic: false,
                     userinfo_endpoint: '',
                     use_pkce: true,
+                    client_secret_basic: true,
+                    client_secret_post: true,
 
                     // user defined values
                     name: '',
@@ -196,7 +202,6 @@
     async function onSubmitConfig() {
         const valid = await validateFormConfig();
         if (!valid) {
-            err = 'Invalid input';
             return;
         }
 
@@ -256,6 +261,9 @@
             config.userinfo_endpoint = body.userinfo_endpoint;
             config.token_auth_method_basic = body.token_auth_method_basic;
             config.use_pkce = body.use_pkce;
+            config.client_secret_basic = body.client_secret_basic;
+            // we want to enable basic only if it is supported for better compatibility out of the box
+            config.client_secret_post = !body.client_secret_basic && body.client_secret_post;
             config.scope = body.scope;
             config.root_pem = body.root_pem;
 
@@ -287,6 +295,8 @@
             token_endpoint: '',
             userinfo_endpoint: '',
             use_pkce: true,
+            client_secret_basic: true,
+            client_secret_post: false,
             scope: '',
             root_pem: null,
             admin_claim_path: null,
@@ -299,12 +309,21 @@
     }
 
     async function validateFormConfig() {
+        formErrors = {};
         try {
             await schemaConfig.validate(config, {abortEarly: false});
-            formErrors = {};
+
+            if (config.client_secret && !(config.client_secret_basic || config.client_secret_post)) {
+                err = 'You have given a client secret, but no client auth method is active';
+                return false;
+            } else {
+                err = 'Invalid input';
+            }
+
             return true;
         } catch (err) {
             formErrors = extractFormErrors(err);
+            err = 'Invalid input';
             return false;
         }
     }
@@ -553,6 +572,30 @@
                 CLIENT SECRET
             </PasswordInput>
 
+            <div class="desc">
+                <p>
+                    The authentication method to use on the <code>/token</code> endpoint.<br>
+                    Most providers should work with <code>basic</code>, some only with <code>post</code>.
+                    In rare situations, you need both, while it can lead to errors with others.
+                </p>
+            </div>
+            <div class="switchRow">
+                <div>
+                    client_secret_basic
+                </div>
+                <Switch
+                        bind:selected={config.client_secret_basic}
+                />
+            </div>
+            <div class="switchRow">
+                <div>
+                    client_secret_post
+                </div>
+                <Switch
+                        bind:selected={config.client_secret_post}
+                />
+            </div>
+
             <JsonPathDesc/>
             <div class="desc">
                 <p>
@@ -670,4 +713,11 @@
     .success {
         color: var(--col-ok);
     }
+
+    .switchRow {
+        margin-bottom: .25rem;
+        padding-left: .5rem;
+        display: grid;
+        grid-template-columns: 9rem 1fr;
+    }
 </style>
diff --git a/migrations/hiqlite/3_client_auth_methods.sql b/migrations/hiqlite/3_client_auth_methods.sql
@@ -0,0 +1,4 @@
+ALTER TABLE auth_providers
+    ADD client_secret_basic INTEGER DEFAULT 1 NOT NULL;
+ALTER TABLE auth_providers
+    ADD client_secret_post INTEGER DEFAULT 1 NOT NULL;
diff --git a/migrations/postgres/29_client_auth_methods.sql b/migrations/postgres/29_client_auth_methods.sql
@@ -0,0 +1,4 @@
+ALTER TABLE auth_providers
+    ADD client_secret_basic BOOLEAN DEFAULT true NOT NULL;
+ALTER TABLE auth_providers
+    ADD client_secret_post BOOLEAN DEFAULT true NOT NULL;