From 981b3cb6178e896acb6b53f7a095723246eae9e0 Mon Sep 17 00:00:00 2001 From: Jon Lester Date: Thu, 17 Feb 2022 09:53:37 -0500 Subject: [PATCH 1/5] Fixing issue with invalid header names --- .../Models/ModelExtensions.cs | 39 +++++++++++++++-- .../EasyAuthForK8s.Tests.Web/ModelTests.cs | 43 +++++++++++++++++++ 2 files changed, 78 insertions(+), 4 deletions(-) create mode 100644 src/Tests/EasyAuthForK8s.Tests.Web/ModelTests.cs diff --git a/src/EasyAuthForK8s.Web/Models/ModelExtensions.cs b/src/EasyAuthForK8s.Web/Models/ModelExtensions.cs index 7ace940..ca95985 100644 --- a/src/EasyAuthForK8s.Web/Models/ModelExtensions.cs +++ b/src/EasyAuthForK8s.Web/Models/ModelExtensions.cs @@ -16,8 +16,13 @@ namespace EasyAuthForK8s.Web.Models; internal static class ModelExtensions { + //from rfc7230 US-ASCII visual characters not allowed in a token (DQUOTE and "(),/:;<=>?@[\]{}") + private static int[] invalid_token_chars = new int[] { 34,40,41,44,47,58,59,60,61,62,63,64,91,92,93,123,125 }; + const string EMPTY_HEADER_NAME = "illegal-name"; + public static EasyAuthState EasyAuthStateFromHttpContext(this HttpContext context) { + //see if state exists in property bag, and return it if (context.Items.ContainsKey(Constants.StateCookieName)) { @@ -176,7 +181,7 @@ internal static void AppendResponseHeaders(this UserInfoPayload payload, IHeader void addHeader(string name, string value) { - string headerName = SanitizeHeaderName($"{configOptions.ResponseHeaderPrefix}{name}"); + string headerName = SanitizeHeaderName($"{configOptions.ResponseHeaderPrefix}{ClaimNameFromUri(name)}"); string encodedValue = EncodeValue(value, configOptions.ClaimEncodingMethod); //nginx will only forward the first header of a given name, @@ -240,16 +245,42 @@ private static string EncodeValue(string value, EasyAuthConfigurationOptions.Enc _ => value, }; } - private static string SanitizeHeaderName(string name) + //strips out any illegal characters + internal static string SanitizeHeaderName(string name) { if (string.IsNullOrEmpty(name)) { throw new ArgumentNullException("name"); } - string clean = new string(name.Where(c => c >= 32 && c < 127).ToArray()); + string clean = new string(name.Where(c => c >= 32 && c < 127 && !invalid_token_chars.Contains(c)).ToArray()); - return clean.Replace('_', '-').ToLower(); + //edge case: the original name contains ZERO valid characters + //just return a default. there's no reason to throw when the app may ignore this header anyway + if (clean.Length == 0) + return EMPTY_HEADER_NAME; + else + return clean.Replace('_', '-').ToLower(); + } + //while this is not entirely necessary, and might lead to name-uniqueness conflicts + //it seems like a reasonable compromize to create a more "friendly" header name + internal static string ClaimNameFromUri(string name) + { + if (string.IsNullOrEmpty(name)) + { + throw new ArgumentNullException("name"); + } + if (name.Contains("/")) + { + var split = name.Split('/', StringSplitOptions.RemoveEmptyEntries); + if (split.Length > 1) + return split[split.Length - 1]; + else + //edge case: the original name contains nothing but forward slashes + return EMPTY_HEADER_NAME; + } + else + return name; } } diff --git a/src/Tests/EasyAuthForK8s.Tests.Web/ModelTests.cs b/src/Tests/EasyAuthForK8s.Tests.Web/ModelTests.cs new file mode 100644 index 0000000..69b019f --- /dev/null +++ b/src/Tests/EasyAuthForK8s.Tests.Web/ModelTests.cs @@ -0,0 +1,43 @@ +using EasyAuthForK8s.Web.Helpers; +using EasyAuthForK8s.Web.Models; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.Hosting; +using System; +using System.Text.RegularExpressions; +using System.Threading.Tasks; +using Xunit; + +namespace EasyAuthForK8s.Tests.Web +{ + + public class ModelTests + { + [Theory] + [InlineData("http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier", "httpschemas.xmlsoap.orgws200505identityclaimsnameidentifier")] + [InlineData("(),/:;<=>?@[\\]{}\"foo", "foo")] + //input contains no legal characters, default is returned + [InlineData("(),/:;<=>?@[\\]{}\"", "illegal-name")] + //input is legal, converted to lowercase + [InlineData("FOO", "foo")] + //complete list of valid characters should all remain, all lower case and underscore converted to "-" + [InlineData(" !#$%&'*+-.0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ^_`abcdefghijklmnopqrstuvwxyz|~", " !#$%&'*+-.0123456789abcdefghijklmnopqrstuvwxyz^-`abcdefghijklmnopqrstuvwxyz|~")] + public void Sanitize_Header_Names(string input, string expected) + { + var sanitized = ModelExtensions.SanitizeHeaderName(input); + Assert.Equal(expected, sanitized); + } + + [Theory] + [InlineData("http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier", "nameidentifier")] + [InlineData("http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier/", "nameidentifier")] + [InlineData("///////", "illegal-name")] + public void Uri_Claim_Names(string input, string expected) + { + var nameFromUri = ModelExtensions.ClaimNameFromUri(input); + Assert.Equal(expected, nameFromUri); + } + + } +} From 8c90486e3a36325e6abd71f7f760a638a5f04938 Mon Sep 17 00:00:00 2001 From: Jon Lester Date: Fri, 18 Feb 2022 13:49:31 -0500 Subject: [PATCH 2/5] update version number --- charts/easyauth-proxy/Chart.yaml | 4 ++-- charts/easyauth-proxy/values.yaml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/charts/easyauth-proxy/Chart.yaml b/charts/easyauth-proxy/Chart.yaml index 1ff4544..38a6738 100644 --- a/charts/easyauth-proxy/Chart.yaml +++ b/charts/easyauth-proxy/Chart.yaml @@ -15,9 +15,9 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 0.1.0 +version: 1.0.0 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to # follow Semantic Versioning. They should reflect the version the application is using. -appVersion: 1.16.0 +appVersion: 1.0.0 diff --git a/charts/easyauth-proxy/values.yaml b/charts/easyauth-proxy/values.yaml index 17eabf4..e14185d 100644 --- a/charts/easyauth-proxy/values.yaml +++ b/charts/easyauth-proxy/values.yaml @@ -12,7 +12,7 @@ image: repository: ghcr.io/azure/easyauthfork8s/easy-auth-proxy pullPolicy: Always # Overrides the image tag whose default is the chart appVersion. - tag: master + tag: v1.0.0 imagePullSecrets: [] nameOverride: "" From 519f2b9fc5af52b1552f8acac1131038f715a322 Mon Sep 17 00:00:00 2001 From: Danielkon96 Date: Fri, 25 Feb 2022 17:34:47 +0000 Subject: [PATCH 3/5] changed proxy deploy to statefulset and fixed e2e test bug --- .../templates/{deployment.yaml => statefulset.yaml} | 3 ++- test.sh | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) rename charts/easyauth-proxy/templates/{deployment.yaml => statefulset.yaml} (98%) diff --git a/charts/easyauth-proxy/templates/deployment.yaml b/charts/easyauth-proxy/templates/statefulset.yaml similarity index 98% rename from charts/easyauth-proxy/templates/deployment.yaml rename to charts/easyauth-proxy/templates/statefulset.yaml index a5f2d0e..b8114b0 100644 --- a/charts/easyauth-proxy/templates/deployment.yaml +++ b/charts/easyauth-proxy/templates/statefulset.yaml @@ -1,5 +1,5 @@ apiVersion: apps/v1 -kind: Deployment +kind: StatefulSet metadata: name: {{ include "easyauth-proxy.fullname" . }} labels: @@ -8,6 +8,7 @@ spec: {{- if not .Values.autoscaling.enabled }} replicas: {{ .Values.replicaCount }} {{- end }} + serviceName: {{ include "easyauth-proxy.fullname" . }} selector: matchLabels: {{- include "easyauth-proxy.selectorLabels" . | nindent 6 }} diff --git a/test.sh b/test.sh index 1171e24..1e39bb6 100644 --- a/test.sh +++ b/test.sh @@ -54,8 +54,8 @@ bash ./main.sh -a $A -c $C -r $R -e $E -l $L -t $T -s $S -z $Z -g APP_NAME="$A.$L.cloudapp.azure.com" WEBPAGE=https://$APP_NAME -echo "Grabbed homepage: " $WEBPAGE ". Sleeping for 10 seconds..." -sleep 10 +echo "Grabbed homepage: " $WEBPAGE ". SLEEPING for 60 seconds..." +sleep 60 RESPONSE_CODE=$(curl -s -o /dev/null -w "%{http_code}" $WEBPAGE) echo "response code: " $RESPONSE_CODE From 2c6a15fc3680ecbe767e4400bd1fd285063702d9 Mon Sep 17 00:00:00 2001 From: Danielkon96 Date: Fri, 25 Feb 2022 19:05:41 +0000 Subject: [PATCH 4/5] clears up errors in e2e test --- AutomationScripts/5-deployEasyAuthProxy.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AutomationScripts/5-deployEasyAuthProxy.sh b/AutomationScripts/5-deployEasyAuthProxy.sh index dc6239f..e0a2b17 100644 --- a/AutomationScripts/5-deployEasyAuthProxy.sh +++ b/AutomationScripts/5-deployEasyAuthProxy.sh @@ -21,7 +21,7 @@ do echo "" kubectl get svc,deploy,pod echo "" - INPUT_STRING=$(kubectl get svc,deploy,pod -o=jsonpath='{.items[3].status.containerStatuses[0].ready}') + INPUT_STRING=$(kubectl get svc,deploy,pod -o=jsonpath='{.items[2].status.containerStatuses[0].ready}') sleep 10 if [ "$n" == "0" ]; then echo "ERROR. INFINITE LOOP in 4-EasyAuthProxy.sh." From af4b391938bbea7aeda00c063cff670780aae321 Mon Sep 17 00:00:00 2001 From: Danielkon96 Date: Fri, 25 Feb 2022 19:10:02 +0000 Subject: [PATCH 5/5] update version number --- charts/easyauth-proxy/Chart.yaml | 4 ++-- charts/easyauth-proxy/values.yaml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/charts/easyauth-proxy/Chart.yaml b/charts/easyauth-proxy/Chart.yaml index 38a6738..fa59642 100644 --- a/charts/easyauth-proxy/Chart.yaml +++ b/charts/easyauth-proxy/Chart.yaml @@ -15,9 +15,9 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 1.0.0 +version: 1.0.2 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to # follow Semantic Versioning. They should reflect the version the application is using. -appVersion: 1.0.0 +appVersion: 1.0.2 diff --git a/charts/easyauth-proxy/values.yaml b/charts/easyauth-proxy/values.yaml index e14185d..3faf968 100644 --- a/charts/easyauth-proxy/values.yaml +++ b/charts/easyauth-proxy/values.yaml @@ -12,7 +12,7 @@ image: repository: ghcr.io/azure/easyauthfork8s/easy-auth-proxy pullPolicy: Always # Overrides the image tag whose default is the chart appVersion. - tag: v1.0.0 + tag: v1.0.2 imagePullSecrets: [] nameOverride: ""