dennisgorelik: (Default)
Short names vs unique names
It is a good practice to use shorter method names, because long names are harder to read.
It is cleaner to call:
candidate.Save();
than:
candidate.SaveCandidate();

But then we end up with multiple "Save()" methods in different classes. For example:
Candidate.cs/Save()
Recruiter.cs/Save()

Problem with non-unique names
If we search our codebase for "Save(" - we would find a lot of methods and method calls. Only some of them would relate to the functionality we actually want to research (for example, we may want to research where "Candidate Save()" functionality is used because we consider refactoring or deleting it).

Plain text search vs code references
Visual Studio allows to find all references to a specific method by right-mouse-clicking on a method name and selecting "Find All References".
So, non-unique method names problem is solved, right?
Not quite.
Visual Studio is not able to track method calls that are made from aspx and ashx files.
Visual Studio is also not able to find method references in the comments.

ReSharper vs vanilla Visual Studio
ReSharper actually is able to find method references in aspx, ashx and even in comments. Until Visual Studio 2015 that worked fine. But since ReSharper team and codebase aged, and Visual Studio switched to new Roslyn compiler, ReSharper team was not able to keep up and delivered only barely working resource hog, that is practically not usable with newer version of Visual Studio (too slow).

Get rid of aspx and ashx files?
It is actually pretty easy to avoid using ashx handlers and use standard C# classes to implement HttpHandler interface.
But what about aspx pages: can we get rid of them too and use only standard C# HttpHandlers?
If we could do that, then we would be able to rely on "Find All References" feature again.
But, unfortunately, getting rid of aspx pages is not that simple. We would have to reimplement a lot of functionality that aspx has.
For example:
- Page PostBack support would be gone.
- Ability to nicely combine HTML code and aspx controls alongside each other would be gone.
- HTML syntax validation would be gone (no HTML syntax validation for C# strings in Visual Studio).

If it ain't broke - don't fix it
Even though it is pretty straightforward operation to convert existing ashx files into standard C# classes (where Visual Studio is able to track all method references) - such conversion is not without its own problems.
- Conversion takes developer's time.
- Code replacement could introduce silly bugs.
- Moving code from class to class makes navigating "svn blame" - a little bit trickier.
So if an ashx handler was working in the solution for many years already - does it make sense to touch it now?

The benefits of code refactoring
In spite of "If it ain't broke - don't fix it" rule - cleaning up code is still needed. If we do not keep code clean (do not delete unneeded parts and do not clear confusing things such as hidden references) - then our codebase would be extremely hard to maintain. Fixing a bug would introduce other bugs. Features would be very hard to add without adding bugs.

It depends
There is no single solution that can be applied to all situations. In software development we consider multiple problems and constantly weigh pros and cons against each other.
For example, out of 11 remaining ashx files, we:
- Deleted one file because we do not use it ("Reduce amount of code when possible" principle).
- Would migrate one file to the standard C# HttpHandler, because today during refactoring a developer missed a method call from that ashx file.
- Keep other 9 ashx files as is ("If it ain't broke - don't fix it" principle).

What are your examples of balancing problems against each other?
dennisgorelik: (Default)
When developing a user-facing application, prioritization of security versus usability - requires delicate balancing.

Here are some examples:

1) When postjobfree emails to a user account recovery link, we want to make that the link is usable, and allow user to use that link for a day (24 hours).
In some scenarios such account recovery link could be usable for the user even after 24 hours. But such a long window for taking over an account based on a single link would be making postjobfree.com account less secure (what if an attacker get an access to an old "account recovery" link?).

2) When user opens that account recovery link, PostJobFree allows user to set a new password, and it also autologins user to that account.
But what if user opens the same link for the second time: should PostJobFree allow user to change account password and autologin again or not?
From security perspective it is safer to expire such a link immediately after user opened the link.
From usability perspective it is better to allow that link to work for the second time, because user may accidentally open that "account recovery" link twice. Or an antivirus program may pre-open email link before user opens it.
In order to balance these security and usability demands, we decided to allow account recovery link to work for 1 hour after it was already used (unused account recovery link can be used for up to 24 hours).

3) What if user changed password on his account: should we allow old account recovery links to work or not?



Here is a typical "security" scenario:
User account owner found that an attacker (or a former employee) has an access to the account. So the account owner changes the password and expects that the attacker would not have an access to the account anymore. But if the attacker still has an old account recovery link - he can still autologin.
So, from security perspective, we should immediately expire all "account recovery" links that were sent before password change.
However there is an important "usability" scenario too:
- User posts a job, which creates a new account for the user.
- PostJobFree emails "confirm email" link to the user:
===============
From: PostJobFree <noreply@postjobfree.com>
To: testuser@gmail.com
Subject: Confirm your PostJobFree registration

===============

("Confirm email" link functions similar to "account recovery" link).
- While waiting for that "confirm email" link to arrive in the email inbox, user sets up a password on that new account (as a part of a new account setup process).
- Then user opens "confirm email" link.
If, according to security demands, "password change" in the previous step expired such "confirm email" link, then an important piece of usability is lost: user cannot immediately confirm that email is functioning, and has to request another "confirm email" link.
So, how do we balance these contradictory demands between security and usability in this case?
The best approach seems to be to prioritize usability in cases when user sets up a new account, but prioritize security when user changes password on already established account.
So if user changed password while going through initial account setup wizard - then keep previously sent links functioning. But if user already had password set, and now decided to change the password again - then expire all past "account recovery", "confirm email" and "change email" links immediately.
Such granular balancing between security and usability allows to deliver good security to the users who care about security of their account (users who change their account passwords manually -- such users are a minority), but still deliver a good usability to the vast majority of users who setup their password only if they are nudged by the account setup wizard.

Profile

dennisgorelik: (Default)
Dennis Gorelik

September 2017

S M T W T F S
      12
34567 8 9
1011 12131415 16
17181920212223
24252627282930

Syndicate

RSS Atom

Most Popular Tags

Style Credit

Expand Cut Tags

No cut tags
Page generated Sep. 21st, 2017 08:31 am
Powered by Dreamwidth Studios