[Mono-bugs] [Bug 60482][Blo] New - Critical bug in ASP.NET UrlAuthorization Module (beta 3)

[bugzilla-daemon@bugzilla.ximian.com]

Please do not reply to this email- if you want to comment on the bug, go to the
URL shown below and enter your comments there.

Changed by davidandrewtaylor@hotmail.com.

http://bugzilla.ximian.com/show_bug.cgi?id=60482

--- shadow/60482	2004-06-20 20:49:33.000000000 -0400
+++ shadow/60482.tmp.29810	2004-06-20 20:49:33.000000000 -0400
@@ -0,0 +1,60 @@
+Bug#: 60482
+Product: Mono: Class Libraries
+Version: unspecified
+OS: Red Hat 9.0
+OS Details: 
+Status: NEW   
+Resolution: 
+Severity: 
+Priority: Blocker
+Component: Sys.Web
+AssignedTo: mono-bugs@ximian.com                            
+ReportedBy: davidandrewtaylor@hotmail.com               
+QAContact: mono-bugs@ximian.com
+TargetMilestone: ---
+URL: 
+Cc: 
+Summary: Critical bug in ASP.NET UrlAuthorization Module (beta 3)
+
+Description of Problem:
+There is a critical bug in the ASP.NET UrlAuthorization module in MONO 
+(tested in Beta 3).  The bug is in the code "AuthorizationConfig.cs" 
+within the foreach statement inside the method "internal bool IsValidUser 
+(IPrincipal user, string verb)".  The logic inside that method is not 
+correct as per the SDK.  In particular:
+
+A) I do not think the first line inside the foreach "if (data.Users == 
+null) continue" is correct?  Because surely that line is skipping any 
+valid settings that only contain roles by do not specify users such as:
+"<allow roles='myrole' />".  Was that line means to be an optimization? 
+(it looks to me like it should just be removed).
+
+B) I do not think the last line inside the foreach "if (userMatch && 
+roleMatch && verbMatch) return data.Allow;" is correct?  This line does 
+not cater for cases such as:
+"<allow users="user22" roles='myrole' />", where the logged in user 
+is "user20" but he is in the role "myrole", and likewise for cases where 
+the user is "users22" but is *not* in the role "myrole".
+So the line: "if (userMatch && roleMatch && verbMatch) return 
+data.Allow;" is clearly wrong.
+
+In summary: The code within the foreach loop needs a complete rewrite 
+following these rules from the SDK:
+1. "Either the users or the roles attribute must be included. Both can be 
+included, but both are not required. The verbs attribute is optional."
+2. "Rules contained in configuration files at lower directory levels take 
+precedence over rules at higher directory levels. The system determines 
+which rule takes precedence by constructing a merged list of all rules 
+for a URL, with the most recent (nearest in the hierarchy) rules at the 
+head of the list."
+3. Given a set of merged rules for a URL, the system starts at the head 
+of the list and checks rules until the first match is found. Note that 
+the default configuration for ASP.NET contains an <allow users="*"> 
+element, which authorizes all users. If no rules match, the request is 
+allowed unless otherwise denied. If a match is found and the match is a 
+<deny> element, it returns the 401 status code. Applications or sites can 
+easily configure a <deny users="*"> element at the top level of their 
+site or application to prevent this behavior.  If an <allow> matches, the 
+module does nothing and lets the request be processed further.
+
+I will post some sample testcases.