Merge 390f5835f4 into 14af2951e5

CHANGELOG.md

@@ -8,6 +8,8 @@
 
 ## Changes since v7.15.3
 
+- [#3459](https://github.com/oauth2-proxy/oauth2-proxy/pull/3459) fix: include the provider's error_description on the OAuth2 callback error page (#3458) (@Molaire)
+
 # V7.15.3
 
 ## Release Highlights

oauthproxy.go

@@ -896,10 +896,21 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {
 	}
 	errorString := req.Form.Get("error")
 	if errorString != "" {
-		logger.Errorf("Error while parsing OAuth2 callback: %s", errorString)
+		// Many providers (e.g. Okta, Entra ID, Keycloak) return a human-readable
+		// error_description alongside the error code. Surfacing it lets end users
+		// distinguish causes of an otherwise opaque code such as access_denied
+		// (for example "User is not assigned to the application" versus a policy
+		// denial) without needing access to the proxy logs.
+		errorDescription := req.Form.Get("error_description")
+		logger.Errorf("Error while parsing OAuth2 callback: %s %s", errorString, errorDescription)
 		message := fmt.Sprintf("Login Failed: The upstream identity provider returned an error: %s", errorString)
-		// Set the debug message and override the non debug message to be the same for this case
+		if errorDescription != "" {
-		p.ErrorPage(rw, req, http.StatusForbidden, message, message)
+			message = fmt.Sprintf("%s: %s", message, errorDescription)
+		}
+		// Set the debug message and override the non debug message to be the same
+		// for this case. The message is passed as an argument rather than a format
+		// string so a provider-supplied description containing '%' is rendered verbatim.
+		p.ErrorPage(rw, req, http.StatusForbidden, message, "%s", message)
 		return
 	}
 

oauthproxy_test.go

@@ -843,6 +843,33 @@ func TestSignInPageSkipProviderDirect(t *testing.T) {
 	}
 }
 
+func TestOAuthCallbackProviderErrorIncludesDescription(t *testing.T) {
+	sipTest, err := NewSignInPageTest(false)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Okta (and other OIDC providers) return error_description alongside the
+	// error code when, for example, the user is not assigned the application.
+	code, body := sipTest.GetEndpoint(
+		"/oauth2/callback?error=access_denied&error_description=User+is+not+assigned+to+the+client+application.",
+	)
+	assert.Equal(t, http.StatusForbidden, code)
+	assert.Contains(t, body, "access_denied")
+	assert.Contains(t, body, "User is not assigned to the client application.")
+}
+
+func TestOAuthCallbackProviderErrorWithoutDescription(t *testing.T) {
+	sipTest, err := NewSignInPageTest(false)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	code, body := sipTest.GetEndpoint("/oauth2/callback?error=access_denied")
+	assert.Equal(t, http.StatusForbidden, code)
+	assert.Contains(t, body, "The upstream identity provider returned an error: access_denied")
+}
+
 type ProcessCookieTest struct {
 	opts         *options.Options
 	proxy        *OAuthProxy