Using transient keyword to store password in hierarchy custom setting

Question

Per a security review, we've found that the use of transient keyword is required to store variables within custom settings. There's a great link that's been provided to help us get pointed in the right direction, which we've followed...

http://wiki.developerforce.com/page/Secure_Coding_Storing_Secrets

Under the Custom Settings section, the wiki page says...

In order to allow authorized users to create and update sensitive information, create a Visualforce page that only accepts input and does not render the value back on the page. The “transient” keyword should be used to declare instance variables within Visualforce controllers to ensure they are not transmitted as part of the view state. Please refer to the following link for details: http://www.salesforce.com/us/developer/docs/apexcode/Content/apex_classes_keywords_transient.htm.

Awesome, that sounds like it's not too bad. So, we poked around, and created this VF page and controller.

Controller

public with sharing class TestCustomSettings {

public Transient TestR__c myPref {get;set;}

public TestCustomSettings(){

  myPref = TestR__c.getvalues(System.UserInfo.getOrganizationId());

  if(myPref == null)
    myPref = new TestR__c(setupOwnerId = System.Userinfo.getOrganizationId());
}

public PageReference save() {
  if(myPref.id == null){
    insert myPref;
  } else
    update myPref;
    return null;
  }
}

VF Page

<apex:page controller="TestCustomSettings" title="Test Custom Settings Title">
<h1>Test Custom Setting</h1>
 <apex:form >
    <apex:pageBlock title="Edit Preferences" mode="edit">
          <apex:pageBlockButtons location="bottom">
            <apex:commandButton action="{!save}" value="Save"/>
          </apex:pageBlockButtons>
              <apex:pageBlockSection title="Change my preferences" columns="2">
                <apex:inputField value="{!myPref.Password1__c}"/>
                <apex:inputField value="{!myPref.Password2__c}"/>
              </apex:pageBlockSection>
    </apex:pageBlock>
  </apex:form>
</apex:page>`

Unfortunately, whenever the form is saved, we receive the following error message:

"Attempt to de-reference a null object Error is in expression '{!save}' in component in page testcustomsettings"

Any thoughts as to how we can follow the requirements for using the transient keyword but still get this to work properly?

Answer 1

Because your myPref property is transient the initialisation you perform in the constructor won't round trip when the page posts back.

When I've used transient and a protected custom setting I use separate properties that are transient and then only work with the custom setting in the post back method.

Controller

public with sharing class TestCustomSettings {

    // transient to ensure they are not transmitted as part of the view state
    public transient String password1 {get; set;}
    public transient String password2 {get; set;}

    public PageReference save() {
        // I've changed this to getInstance() rather than getValues()
        TestR__c myPref = TestR__c.getInstance(UserInfo.getOrganizationId());

        if(myPref == null) {
            myPref = new TestR__c();
            myPref.SetupOwnerId = Userinfo.getOrganizationId();
        }

        myPref.Password1__c = password1;
        myPref.Password2__c = password2;

        // Note that by using upsert you don't need to check if the Id has been set. 
        upsert myPref;

    }
}

Visualforce page

You can use inputSecret rather than inputField in the Visualforce page so that the browser will mask the input.

<apex:inputSecret value="{!password1}" size="10"/>
<apex:inputSecret value="{!password2}" size="10"/>

Answer 2

I think you need to just have a separate variable for the field you actually want to edit, rather than making the TestR__c reference itself transient.

Even though you initialise it in the constructor, the transient keyword means that it doesn't get sent to the page in the view state, and as such it is null when you make the call to the save() method.

So you probably want something like this:

public with sharing class TestCustomSettings {

private TestR__c;
public Transient string   password1 {get; set;}
public Transient string   password2 {get; set;}

public TestCustomSettings(){

  myPref = TestR__c.getvalues(System.UserInfo.getOrganizationId());

  if(myPref == null)
    myPref = new TestR__c(setupOwnerId = System.Userinfo.getOrganizationId());
}

public PageReference save() {
  myPref.Password1__c = password1;   myPref.Password2__c = password2;

  if(myPref.id == null){
    insert myPref;
  } else {
    update myPref;
    return null;
  }
}

Though probably with some more validation around the password itself. Also you should probably be salting and hashing these passwords rather than storing them as plain text.

View State and Variables

All of your member variables (except those marked as transient!) get transferred in the view state, because HTTP is stateless. It's tempting to think of your apex class object being resident in memory on the 'server side', but in reality it's ALL moving back and forth. The instance of TestR__c would be exposed in the view state, so if a record is found in the constructor the password fields on the object should be cleared, and likewise, they should be cleared after saving the record.