ACR values should not be automatically added when blank (#598)

* ACR values should not be automatically added when blank

* Added changelog

CHANGELOG.md

@@ -55,6 +55,7 @@
 
 ## Changes since v5.1.1
 
+- [#598](https://github.com/oauth2-proxy/oauth2-proxy/pull/598) acr_values no longer sent to IdP when empty (@ScottGuymer)
 - [#548](https://github.com/oauth2-proxy/oauth2-proxy/pull/548) Separate logging options out of main options structure (@JoelSpeed)
 - [#536](https://github.com/oauth2-proxy/oauth2-proxy/pull/536) Improvements to Session State code (@JoelSpeed)
 - [#573](https://github.com/oauth2-proxy/oauth2-proxy/pull/573) Properly parse redis urls for cluster and sentinel connections (@amnay-mo)

providers/provider_default.go

@@ -94,7 +94,9 @@ func (p *ProviderData) GetLoginURL(redirectURI, state string) string {
 	a := *p.LoginURL
 	params, _ := url.ParseQuery(a.RawQuery)
 	params.Set("redirect_uri", redirectURI)
+	if p.AcrValues != "" {
 		params.Add("acr_values", p.AcrValues)
+	}
 	if p.Prompt != "" {
 		params.Set("prompt", p.Prompt)
 	} else { // Legacy variant of the prompt param:

providers/provider_default_test.go

@@ -2,6 +2,7 @@ package providers
 
 import (
 	"context"
+	"net/url"
 	"testing"
 	"time"
 
@@ -19,3 +20,30 @@ func TestRefresh(t *testing.T) {
 	assert.Equal(t, false, refreshed)
 	assert.Equal(t, nil, err)
 }
+
+func TestAcrValuesNotConfigured(t *testing.T) {
+	p := &ProviderData{
+		LoginURL: &url.URL{
+			Scheme: "http",
+			Host:   "my.test.idp",
+			Path:   "/oauth/authorize",
+		},
+	}
+
+	result := p.GetLoginURL("https://my.test.app/oauth", "")
+	assert.NotContains(t, result, "acr_values")
+}
+
+func TestAcrValuesConfigured(t *testing.T) {
+	p := &ProviderData{
+		LoginURL: &url.URL{
+			Scheme: "http",
+			Host:   "my.test.idp",
+			Path:   "/oauth/authorize",
+		},
+		AcrValues: "testValue",
+	}
+
+	result := p.GetLoginURL("https://my.test.app/oauth", "")
+	assert.Contains(t, result, "acr_values=testValue")
+}