function readOnly(count){ }
Sign Up ›
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
Learn More

Browse by Topic
ShowAll Questionssorted byDate Posted
+ Start a Discussion
highland23highland23 

Trouble using transient keyword to store password in custom setting

In order to more securely store a password as part of my app, I have created a hierarchy custom setting that stores two variables that will be used across the org in connection with the app.  Per the requirements of SFDC security, I need to make sure that the form is blank (that none of the fields have values listed) after the user submits the form (so that the passwords aren't displayed in the clear to the user).

 

From the feedback I've seen thus far, using the transient keyword is the way to go.  I'm just running into some challenges.

 

After looking at some examples provided by Salesforce, I've come up with the following code.

 

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

 You'll see that right at the beginning, I've included the "transient" keyword, in hopes that after the POST, I won't receive the values back in the view state.  Unfortunately, when I submit the POST, I get the following response from SFDC:

 

"Attempt to de-reference a null object

Error is in expression '{!save}' in component <apex:page> in page testcustomsettings"
 
My VF Page is pretty simple as well, and looks like this...
 
<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>

 Any recommendations as to what I can do so that my user can submit this simple form, have her values stored within the custom settings, but after submitting the form is just shown the blank form again without any values (as to follow the SFDC security guidelines when storing passwords)?

Best Answer chosen by Admin (Salesforce Developers) 
sfdcfoxsfdcfox

It is correct in its statement that the transient keyword must be used for variables that should not persist in the view state. This applies only to non-form variables, as those are stored in the view state without the transient keyword. Values bound to a form field will effectively be transmitted as part of the form state, thus violating the rules of a security review. Note in my code that I do the following:

 

1) The custom setting is created in heap memory with a local scope, and thus never saves as part of the view state.

2) The fields that accept the password (I know I used the wrong field type, it should have been a password-masked field) are reset to null after each use so that the previous input is never retransmitted as part of the form state.

 

The alternative method that would also work is as follows:

 

public with sharing class myController {
    private transient string password1, password2;

    public string getpassword1() {
        return null;
    }

    public string getpassword2() {
        return null;
    }

    public void setpassword1(string value) {
        password1 = value;
    }

    public void setpassword2(string value) {
        password2 = value;
    }

    public void save() {
        TestR__c setting = TestR__c.getOrgDefaults();
        if(password1 == password2 && password1 != null) {
            setting.password__c = password1;
            try {
                upsert setting;
            } catch(exception e) {
                apexpages.addmessage(new apexpages.message(apexpages.severity.error,e));
            }
        } else {
            if(password1 == null || password2 == null) {
                 apexpages.addmessage(new apexpages.message(apexpages.severity.error,'Missing a password entry.'));
            } else {
                 apexpages.addmessage(new apexpages.message(apexpages.severity.error,'Password mismatch.'));
            }
        }
    }
}

In this example, I use transient variables that won't be transmitted as part of the view state, plus custom getter/setter methods to enforce read-only input fields. Again, the custom setting is scoped to the save function, and so also will not be transmitted as part of the view state. This design is suboptimal, but will achieve the same effect; you are certain that your view state does not store a password value, and the custom setting is never exposed in the view state. I actually speak with experience in this manner; I'm fairly well-versed in creating secure apps, as that was a prime responsibility to resolve all failed security review items from an app that I inherited when I took on my new job with my current employer.

All Answers

sfdcfoxsfdcfox

Transient means that the variable isn't stored in the View State; it says nothing about rendering into a field. What happens here in your code is that myPref becomes null after the constructor (the page loads), and then is reset when the page submits, because the object isn't part of the view state. Instead, do something like this:

 

public with sharing class contr {
  public string pw1 { get; set; }
  public string pw2 { get; set; }

  public void save() {
    if(pw1 == pw2 && pw1 != null) {
      TestR__c setting = TestR__c.getOrgDefaults();
      setting.password1__c = pw1;
      upsert setting;
    } else {
      if(pw1 == null || pw2 == null) {
        apexpages.addmessage(new apexpages.message(apexpages.severity.error,'Missing password'));
      } else {
        apexpages.addmessage(new apexpages.message(apexpages.severity.error,'Password mismatch'));
      }
    }
    pw1 = pw2 = null;
  }
}

So, why didn't I just use the "transient" keyword? Transient is used for specific instances, such as when you need a variable that will be used across functions but shouldn't be in the view state (and thus, across transactions). Binding the value to a form value means it will still appear in the form until you "null" it out (like I did here). Transient only works on variables that aren't part of the form state in terms of "clearing out data".

 

Here, I just request a copy of the global setting and then update it, all in the save function. Upsert is an "insert or update" operation, so this code should work without any extra effort. You'll also be able to remove the password2__c field. Finally, if this is unmanaged code, you should encrypt your password. If it is managed code, make sure your custom setting is "protected" to keep your secret safe.
highland23highland23

Thanks for the fast response, definitely appreciated!  Unfortunately, it seems that per a security review, the use of transient keyword is required.  In a security review, we were asked to use custom settings as a method to protect passwords, and were provided this link....

 

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

 

Under the Custom Settings approach, it writes...

 

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.

 

So, I think we're still in a pickle of needing to use the transient keyword somehow.

 

Hmm...
sfdcfoxsfdcfox

It is correct in its statement that the transient keyword must be used for variables that should not persist in the view state. This applies only to non-form variables, as those are stored in the view state without the transient keyword. Values bound to a form field will effectively be transmitted as part of the form state, thus violating the rules of a security review. Note in my code that I do the following:

 

1) The custom setting is created in heap memory with a local scope, and thus never saves as part of the view state.

2) The fields that accept the password (I know I used the wrong field type, it should have been a password-masked field) are reset to null after each use so that the previous input is never retransmitted as part of the form state.

 

The alternative method that would also work is as follows:

 

public with sharing class myController {
    private transient string password1, password2;

    public string getpassword1() {
        return null;
    }

    public string getpassword2() {
        return null;
    }

    public void setpassword1(string value) {
        password1 = value;
    }

    public void setpassword2(string value) {
        password2 = value;
    }

    public void save() {
        TestR__c setting = TestR__c.getOrgDefaults();
        if(password1 == password2 && password1 != null) {
            setting.password__c = password1;
            try {
                upsert setting;
            } catch(exception e) {
                apexpages.addmessage(new apexpages.message(apexpages.severity.error,e));
            }
        } else {
            if(password1 == null || password2 == null) {
                 apexpages.addmessage(new apexpages.message(apexpages.severity.error,'Missing a password entry.'));
            } else {
                 apexpages.addmessage(new apexpages.message(apexpages.severity.error,'Password mismatch.'));
            }
        }
    }
}

In this example, I use transient variables that won't be transmitted as part of the view state, plus custom getter/setter methods to enforce read-only input fields. Again, the custom setting is scoped to the save function, and so also will not be transmitted as part of the view state. This design is suboptimal, but will achieve the same effect; you are certain that your view state does not store a password value, and the custom setting is never exposed in the view state. I actually speak with experience in this manner; I'm fairly well-versed in creating secure apps, as that was a prime responsibility to resolve all failed security review items from an app that I inherited when I took on my new job with my current employer.

This was selected as the best answer
highland23highland23

Again, definitely impressed with the solution, thank you so much!  This looks like the right approach, and seems to work well for me too.  Thanks for helping me get through this security review hurdle!