Skip to content

Commit

Permalink
Fix form sending regression after StackExchange.Utils.Http upgrade to…
Browse files Browse the repository at this point in the history
… 0.3.41 (#426)

# Problem

Commit cae2dd9 updated [StackExchange.Utils.Http](https://github.com/StackExchange/StackExchange.Utils/tree/main/src/StackExchange.Utils.Http) from 0.3.2 to 0.3.41. Between these versions, a breaking change landed in the package that changed `SendForm` from sending `application/x-www-form-urlencoded` requests to sending `multipart/form-data` requests instead. This appears to be the commit that introduced that change: StackExchange/StackExchange.Utils@30151e8

It looks like that change broke retrieving the OIDC access tokens in Opserver, because the RFC [requires `application/x-www-form-urlencoded` requests](https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.3) for these, and providers like Okta enforce this requirement. Here's the error that Opserver raises when trying to authenticate over OIDC with Okta:

```
Error: failed to exchange authorization code for access token. BadRequest - {"errorCode":"E0000021","errorSummary":"Bad request. Accept and/or Content-Type headers likely do not match supported values.","errorLink":"E0000021","errorId":"~redacted~","errorCauses":[]}
```

Here's an Okta team member confirming they only allow `application/x-www-form-urlencoded`: https://devforum.okta.com/t/password-grant-bad-request-accept-and-or-content-type-headers-likely-do-not-match-supported-values/2346/2

# Steps to reproduce

1. Sign up for a Free tier Okta Developer account - https://developer.okta.com/signup/
2. Create two groups `opserver-view` and `opserver-admin` in Okta, and assign your user to them
3. Set up a new OIDC application for Opserver
4. Configure Opserver with the following settings:
   ```json
   "Security": {
     "provider": "OIDC",
     "viewEverythingGroups": "opserver-view",
     "adminEverythingGroups": "opserver-admin",
     "scopes": [ "openid", "groups", "profile" ],
     "clientId": "{clientid}",
     "clientSecret": "{clientsecret}",
     "authorizationUrl": "https://{orgsubdomain}.okta.com/oauth2/v1/authorize",
     "accessTokenUrl": "https://{orgsubdomain}.okta.com/oauth2/v1/token",
     "userInfoUrl": "https://{orgsubdomain}.okta.com/oauth2/v1/userinfo",
     "nameClaim": "preferred_username",
     "groupsClaim": "groups"
   }
   ```
5. Run Opserver and attempt to login

With Opserver running on commit 1c4a755 (one before the StackExchange.Utils.Http upgrade in commit cae2dd9), sign in will **work**.

With Opserver running on commit cae2dd9 or the current main branch, sign in will **fail**.

With Opserver running with the changes in PR, sign in should **work** again.

# Solution

This PR updates StackExchange.Utils.Http to 0.3.48 and makes use of a new  `SendFormUrlEncoded` extension method that restores the previous behaviour of sending `application/x-www-form-urlencoded` form requests, everywhere that `SendForm` is used in this codebase.

The main fix I'm interested in is for OIDC, but this might be affecting the CloudFlare API and HAProxy web UI form requests as well. I don't have those available for testing but I figured I should revert the behaviour on those as well to be safe.
  • Loading branch information
dionrhys authored Aug 30, 2023
1 parent e5fc22c commit b01ef3a
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 5 deletions.
4 changes: 2 additions & 2 deletions src/Opserver.Core/Data/CloudFlare/CloudFlareAPI.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public async Task<T> PostAsync<T>(string path, NameValueCollection values = null
{
var result = await Http.Request(APIBaseUrl + path)
.AddHeaders(RequestHeaders)
.SendForm(values)
.SendFormUrlEncoded(values)
.ExpectJson<T>()
.PostAsync();
return result.Data;
Expand All @@ -102,7 +102,7 @@ public async Task<T> DeleteAsync<T>(string path, NameValueCollection values = nu
{
var result = await Http.Request(APIBaseUrl + path)
.AddHeaders(RequestHeaders)
.SendForm(values)
.SendFormUrlEncoded(values)
.ExpectJson<T>()
.DeleteAsync();
return result.Data;
Expand Down
2 changes: 1 addition & 1 deletion src/Opserver.Core/Data/HAProxy/HAProxyAdmin.cs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ private async Task<bool> PostAction(Proxy p, Server server, Action action)
var credsBase64 = Convert.ToBase64String(Encoding.ASCII.GetBytes($"{instance.AdminUser}:{instance.AdminPassword}"));
var response = await Http.Request(instance.Url)
.AddHeader(HeaderNames.Authorization, "Basic " + credsBase64)
.SendForm(new NameValueCollection
.SendFormUrlEncoded(new NameValueCollection
{
["s"] = server.Name,
["action"] = action.AsString(EnumFormat.Description),
Expand Down
2 changes: 1 addition & 1 deletion src/Opserver.Core/Opserver.Core.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
<PackageReference Include="Sigil" Version="5.0.0" />
<PackageReference Include="StackExchange.Exceptional.Shared" Version="2.2.32" />
<PackageReference Include="StackExchange.Redis" Version="2.6.122" />
<PackageReference Include="StackExchange.Utils.Http" Version="0.3.41" />
<PackageReference Include="StackExchange.Utils.Http" Version="0.3.48" />
<PackageReference Include="System.Data.SqlClient" Version="4.8.5" />
<PackageReference Include="System.DirectoryServices" Version="6.0.0" />
<PackageReference Include="System.Management" Version="6.0.0" />
Expand Down
2 changes: 1 addition & 1 deletion src/Opserver.Web/Controllers/AuthController.OIDC.cs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public async Task<IActionResult> OAuthCallback(string code, string state, string

var response = await Http.Request(oidcSettings.AccessTokenUrl)
.WithoutLogging(HttpStatusCode.BadRequest)
.SendForm(form)
.SendFormUrlEncoded(form)
.ExpectString()
.PostAsync();

Expand Down

0 comments on commit b01ef3a

Please sign in to comment.