Trying to fix bad login/security set up.

[darrel]

We have a home grown CMS in our organization that I decided to update at 5pm
only to find a gigantic security bug in it.

Here's the deal:

The original programmer created the security for the CMS. When a person logs
in, they're authenticated against the DB and then pertinent info regarding
their permission levels is saved into a cookie on their machine.

We then have a class/usercontrol that loads on every page of the CMS that
reads this data from the cookie to establish their security credentials.

Here's how it was originally written:

------------------------------------

Public Class SecureUsers

Public Shared su_strUser As String
Public Shared su_strEmail As String
Public Shared su_intDistrict As Integer
Public Shared su_intAdminLevel As Integer
Public Shared su_categories As String
Public Shared su_strDistrict As String

Private Sub Page_Init(ByVal sender As System.Object, ByVal e As
System.EventArgs) Handles MyBase.Init
InitializeComponent()

If Not Request.Cookies("CMSUser") Is Nothing Then
su_strUser = Server.HtmlEncode(Request.Cookies("CMSUser")("su_strUser"))
su_strDistrict =
Server.HtmlEncode(Request.Cookies("CMSUser")("su_strDistrict"))
su_intDistrict =
Server.HtmlEncode(Request.Cookies("CMSUser")("su_intDistrict"))
su_strEmail = Server.HtmlEncode(Request.Cookies("CMSUser")("su_strEmail"))
su_categories =
Server.HtmlEncode(Request.Cookies("CMSUser")("su_categories"))
su_intAdminLevel =
Server.HtmlEncode(Request.Cookies("CMSUser")("su_intAdminLevel"))
End If
End Sub

End Class

------------------------------------

Then, on every page of the CMS that loads the above control, we grab the
variables as needed such as:

username = secureusers.su_strUser

Now, you can probably see what is wrong with the above. The variabls were
all set to public SHARED--which means the variables were shared at the class
level rather than the instance of the class. As such, the data was being
cross-written from thread to thread. One person would log in, start editing,
another would log in, and then when the first person saved, the other
person's credentials were saved instead.

Since I'm not really an OOP expert, it took me a bit and then I realized I
needed to get rid of the SHARED modifier.

So, I did that, and now I'm trying to get the data by creating an instance
first:

Dim theSecureUser As New SecureUsers
username = theSecureUser.su_strUser

Now...THE PROBLEM: This just returns null values. No error, just no value.
WHY!?

Bigger question:

For now, I'm just trying to duct-tape the above for the weekend so that I
can go home. But come Monday, I'm going to have to start rewriting this.
What's the better way to handle it? Obviously, writing the credentials in
the cookie, itself, is dumb. Is it better to use session state? Another
method?

-Darrel

 

[GroupReader]

Sorry if this is way off base... I don't normally program in VB.Net
and I'm not sure I understand correctly.

Short Answer: Replace "Page_Init" with a class constructor.

Long Answer:
If I understand what I'm reading correctly, it looks like you have
properly instantiated an instance of the SecureUsers class, but the
code that sets values in all of the public member variables at top
would never run. It's in a function called Page_Init... and there is
nothing that would call the Page_Init function (this doesn't appear to
be an asp.net page - just an ordinary class).

Solution: Create a default constructor that gets run when you
instantiate the SecureUsers class. Cut the code out of Page_Init and
put it in the constructor.

Hope that helps and I understood correctly.

 

[darrel]

Hope that helps and I understood correctly.

Thanks. It does.

This particular page is a usercontrol that is getting loaded with each page.
perhaps that's the issue?

I did go ahead and try to make a constructur (separate function) but when I
do that, it failes to 'see' the cookie. For some reason it can't read the
cookie from a contained function.

-Darrel