feat(openid/acr): remove old values and backward compatibility for new idporten

We no longer expect nor accept tokens with old acr values during validation as ID-porten no longer issues tokens with these values. This also removes backward compatibility in cases where configured values targeted the new ID-porten while using old ID-porten. We still maintain an internal mapping from old values to new values for forward compatibilty when using old values provided in the login parameter and the `openid.acr-values` flag.
2026-02-14 17:49:54 +00:00 · 2024-06-27 12:18:42 +02:00
parent f94d81aed7
commit 1906024da0
9 changed files with 31 additions and 68 deletions
--- a/pkg/mock/config.go
+++ b/pkg/mock/config.go
@@ -17,7 +17,7 @@ func Config() *config.Config {
 		EncryptionKey: `G8Roe6AcoBpdr5GhO3cs9iORl4XIC8eq`, // 256 bits key
 		Ingresses:     []string{Ingress},
 		OpenID: config.OpenID{
-			ACRValues:             "Level4",
+			ACRValues:             "idporten-loa-high",
 			ClientID:              "client-id",
 			PostLogoutRedirectURI: "https://google.com",
 			Provider:              "test",
--- a/pkg/mock/openid.go
+++ b/pkg/mock/openid.go
@@ -229,8 +229,8 @@ func (ip *IdentityProviderHandler) Authorize(w http.ResponseWriter, r *http.Requ
 		"code_challenge_method": {"S256"},
 		"response_type":         {"code"},
 		"response_mode":         {"query"},
-		"acr_values":            {"", "Level3", "Level4"},
-		"ui_locales":            {"", "nb", "nn", "en", "se"},
+		"acr_values":            append(ip.Config.Provider().ACRValuesSupported(), ""),
+		"ui_locales":            append(ip.Config.Provider().UILocalesSupported(), ""),
 	}

 	for param, allowed := range allowedParamValues {
--- a/pkg/mock/provider.go
+++ b/pkg/mock/provider.go
@@ -130,7 +130,7 @@ func providerConfiguration(cfg *config.Config) *TestProviderConfiguration {
 	return &TestProviderConfiguration{
 		Cfg: cfg,
 		Metadata: &openidconfig.ProviderMetadata{
-			ACRValuesSupported: openidconfig.Supported{"Level3", "Level4"},
+			ACRValuesSupported: openidconfig.Supported{"idporten-loa-substantial", "idporten-loa-high"},
 			UILocalesSupported: openidconfig.Supported{"nb", "nb", "en", "se"},
 		},
 	}
--- a/pkg/openid/acr/acr.go
+++ b/pkg/openid/acr/acr.go
@@ -11,27 +11,27 @@ const (
 	IDPortenLevelHigh        = "idporten-loa-high"
 )

-// IDPortenMapping is a translation table of valid acr_values for migrating between old and new ID-porten.
-var IDPortenMapping = map[string]string{
-	IDPortenLevel3:           IDPortenLevelSubstantial,
-	IDPortenLevelSubstantial: IDPortenLevel3,
-	IDPortenLevel4:           IDPortenLevelHigh,
-	IDPortenLevelHigh:        IDPortenLevel4,
+// IDPortenLegacyMapping is a translation table of valid acr_values that maps values from "old" to "new" ID-porten.
+var IDPortenLegacyMapping = map[string]string{
+	IDPortenLevel3: IDPortenLevelSubstantial,
+	IDPortenLevel4: IDPortenLevelHigh,
 }

 // acceptedValuesMapping is a map of ACR (authentication context class reference) values.
 // Each value has an associated list of values that are regarded as equivalent or greater in terms of assurance levels.
 // Example:
-// - if we require an ACR value of "Level3", then both "Level3" and "Level4" are accepted values.
-// - if we require an ACR value of "Level4", then only "Level4" is an acceptable value.
+// - if we require an ACR value of "idporten-loa-substantial", then both "idporten-loa-substantial" and "idporten-loa-high" are accepted values.
+// - if we require an ACR value of "idporten-loa-high", then only "idporten-loa-high" is an acceptable value.
 var acceptedValuesMapping = map[string][]string{
-	IDPortenLevel3:           {IDPortenLevel3, IDPortenLevel4, IDPortenLevelSubstantial, IDPortenLevelHigh},
-	IDPortenLevelSubstantial: {IDPortenLevel3, IDPortenLevel4, IDPortenLevelSubstantial, IDPortenLevelHigh},
-	IDPortenLevel4:           {IDPortenLevel4, IDPortenLevelHigh},
-	IDPortenLevelHigh:        {IDPortenLevel4, IDPortenLevelHigh},
+	IDPortenLevelSubstantial: {IDPortenLevelSubstantial, IDPortenLevelHigh},
+	IDPortenLevelHigh:        {IDPortenLevelHigh},
 }

 func Validate(expected, actual string) error {
+	if translated, found := IDPortenLegacyMapping[expected]; found {
+		expected = translated
+	}
+
 	acceptedValues, found := acceptedValuesMapping[expected]
 	if !found {
 		if expected == actual {
--- a/pkg/openid/acr/acr_test.go
+++ b/pkg/openid/acr/acr_test.go
@@ -15,24 +15,16 @@ func TestValidateAcr(t *testing.T) {
 	}{
 		{"no mapping found, not equal", "some-value", "some-other-value", true},
 		{"no mapping found, expected equals actual", "some-value", "some-value", false},
-		{"Level3", "Level3", "Level3", false},
-		{"Level3, higher acr accepted", "Level3", "Level4", false},
-		{"Level3, higher acr accepted 2", "Level3", "idporten-loa-high", false},
+		{"Level3, higher acr accepted", "Level3", "idporten-loa-high", false},
 		{"Level3, no matching value", "Level3", "Level2", true},
 		{"Level3 -> idporten-loa-substantial", "Level3", "idporten-loa-substantial", false},
 		{"idporten-loa-substantial", "idporten-loa-substantial", "idporten-loa-substantial", false},
-		{"idporten-loa-substantial -> Level3", "idporten-loa-substantial", "Level3", false},
-		{"idporten-loa-substantial, higher acr accepted", "idporten-loa-substantial", "Level4", false},
-		{"idporten-loa-substantial, higher acr accepted 2", "idporten-loa-substantial", "idporten-loa-high", false},
-		{"Level4", "Level4", "Level4", false},
-		{"Level4, lower acr not accepted", "Level4", "Level3", true},
-		{"Level4, lower acr not accepted 2", "Level4", "idporten-loa-substantial", true},
+		{"idporten-loa-substantial, higher acr accepted", "idporten-loa-substantial", "idporten-loa-high", false},
+		{"Level4, lower acr not accepted", "Level4", "idporten-loa-substantial", true},
 		{"Level4, no matching value", "Level4", "Level5", true},
 		{"Level4 -> idporten-loa-high", "Level4", "idporten-loa-high", false},
 		{"idporten-loa-high", "idporten-loa-high", "idporten-loa-high", false},
-		{"idporten-loa-high -> Level4", "idporten-loa-high", "Level4", false},
-		{"idporten-loa-high, lower acr not accepted", "idporten-loa-high", "Level3", true},
-		{"idporten-loa-high, lower acr not accepted 2", "idporten-loa-high", "idporten-loa-substantial", true},
+		{"idporten-loa-high, lower acr not accepted", "idporten-loa-high", "idporten-loa-substantial", true},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
 			err := Validate(tt.expected, tt.actual)
--- a/pkg/openid/client/login.go
+++ b/pkg/openid/client/login.go
@@ -148,7 +148,7 @@ func getAcrParam(c *Client, r *http.Request) (string, error) {
 		return paramValue, nil
 	}

-	translatedAcr, ok := acr.IDPortenMapping[paramValue]
+	translatedAcr, ok := acr.IDPortenLegacyMapping[paramValue]
 	if ok && supported.Contains(translatedAcr) {
 		return translatedAcr, nil
 	}
--- a/pkg/openid/client/login_test.go
+++ b/pkg/openid/client/login_test.go
@@ -11,7 +11,6 @@ import (

 	"github.com/nais/wonderwall/pkg/mock"
 	"github.com/nais/wonderwall/pkg/openid/client"
-	"github.com/nais/wonderwall/pkg/openid/config"
 	urlpkg "github.com/nais/wonderwall/pkg/url"
 )

@@ -20,7 +19,6 @@ func TestLogin_URL(t *testing.T) {
 		name       string
 		url        string
 		wantParams map[string]string
-		metadata   *config.ProviderMetadata
 		error      error
 	}

@@ -34,7 +32,7 @@ func TestLogin_URL(t *testing.T) {
 			name: "happy path with level",
 			url:  mock.Ingress + "/oauth2/login?level=Level3",
 			wantParams: map[string]string{
-				"acr_values": "Level3",
+				"acr_values": "idporten-loa-substantial",
 			},
 			error: nil,
 		},
@@ -59,7 +57,7 @@ func TestLogin_URL(t *testing.T) {
 			name: "happy path with both locale and level",
 			url:  mock.Ingress + "/oauth2/login?level=Level3&locale=nb",
 			wantParams: map[string]string{
-				"acr_values": "Level3",
+				"acr_values": "idporten-loa-substantial",
 				"ui_locales": "nb",
 			},
 			error: nil,
@@ -80,43 +78,19 @@ func TestLogin_URL(t *testing.T) {
 			error: client.ErrInvalidPrompt,
 		},
 		{
-			name: "level idporten-loa-substantial should translate to Level3 for old IDP",
-			url:  mock.Ingress + "/oauth2/login?level=idporten-loa-substantial",
-			wantParams: map[string]string{
-				"acr_values": "Level3",
-			},
-			error: nil,
-		},
-		{
-			name: "level idporten-loa-high should translate to Level4 for old IDP",
-			url:  mock.Ingress + "/oauth2/login?level=idporten-loa-high",
-			wantParams: map[string]string{
-				"acr_values": "Level4",
-			},
-			error: nil,
-		},
-		{
-			name: "level Level3 should translate to idporten-loa-substantial for new IDP",
+			name: "level=Level3 should translate to idporten-loa-substantial",
 			url:  mock.Ingress + "/oauth2/login?level=Level3",
 			wantParams: map[string]string{
 				"acr_values": "idporten-loa-substantial",
 			},
-			metadata: &config.ProviderMetadata{
-				ACRValuesSupported: config.Supported{"idporten-loa-substantial", "idporten-loa-high"},
-				UILocalesSupported: config.Supported{"nb", "nb", "en", "se"},
-			},
 			error: nil,
 		},
 		{
-			name: "level Level4 should translate to idporten-loa-high for new IDP",
+			name: "level=Level4 should translate to idporten-loa-high",
 			url:  mock.Ingress + "/oauth2/login?level=Level4",
 			wantParams: map[string]string{
 				"acr_values": "idporten-loa-high",
 			},
-			metadata: &config.ProviderMetadata{
-				ACRValuesSupported: config.Supported{"idporten-loa-substantial", "idporten-loa-high"},
-				UILocalesSupported: config.Supported{"nb", "nb", "en", "se"},
-			},
 			error: nil,
 		},
 	}
@@ -127,10 +101,6 @@ func TestLogin_URL(t *testing.T) {
 			openidConfig := mock.NewTestConfiguration(cfg)
 			ingresses := mock.Ingresses(cfg)

-			if test.metadata != nil {
-				openidConfig.TestProvider.Metadata = test.metadata
-			}
-
 			c := client.NewClient(openidConfig, nil)

 			req := mock.NewGetRequest(test.url, ingresses)
--- a/pkg/openid/config/provider.go
+++ b/pkg/openid/config/provider.go
@@ -148,7 +148,7 @@ func (c *ProviderMetadata) validateAcrValues(acrValue string) error {
 		return nil
 	}

-	translatedAcr, ok := acr.IDPortenMapping[acrValue]
+	translatedAcr, ok := acr.IDPortenLegacyMapping[acrValue]
 	if ok && c.ACRValuesSupported.Contains(translatedAcr) {
 		return nil
 	}
--- a/pkg/openid/config/provider_test.go
+++ b/pkg/openid/config/provider_test.go
@@ -11,7 +11,7 @@ import (

 func TestProviderMetadata_Validate(t *testing.T) {
 	metadata := &openidconfig.ProviderMetadata{
-		ACRValuesSupported: openidconfig.Supported{"Level3", "Level4"},
+		ACRValuesSupported: openidconfig.Supported{"idporten-loa-substantial", "idporten-loa-high"},
 		UILocalesSupported: openidconfig.Supported{"nb", "nb", "en", "se"},
 	}

@@ -19,10 +19,11 @@ func TestProviderMetadata_Validate(t *testing.T) {
 		name, acr, locale string
 		assertion         assert.ErrorAssertionFunc
 	}{
-		{"happy path", "Level4", "nb", assert.NoError},
+		{"happy path", "idporten-loa-high", "nb", assert.NoError},
 		{"invalid acr", "Level5", "nb", assert.Error},
-		{"invalid locale", "Level4", "de", assert.Error},
-		{"has matching acr translation", "idporten-loa-high", "nb", assert.NoError},
+		{"invalid locale", "idporten-loa-high", "de", assert.Error},
+		{"has acr translation for Level4", "Level4", "nb", assert.NoError},
+		{"has acr translation for Level3", "Level3", "nb", assert.NoError},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
 			cfg := mock.Config()