Merge pull request #76 from Azure/dev

Fixing issue with invalid header names

Author: jonlester

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."

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.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.16.0
+appVersion: 1.0.2

charts/easyauth-proxy/templates/deployment.yaml renamed to 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 }}

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.2
 
 imagePullSecrets: []
 nameOverride: ""

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;
     }
 }
 

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);
+        }
+
+    }
+}

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