Thursday, January 28, 2016

Security conscious coding

OWASP maintain a top 10 list of  security vulnerabilites in systems ( https://www.owasp.org/index.php/Top_10_2013-Top_10 )


They have also now introduced a Developer centric top ten list for proactive controls .
Full document is here.  https://www.owasp.org/images/5/57/OWASP_Proactive_Controls_2.pdf

1. Verify for Security Early and Often
2. Parameterize Queries
3. Encode Data
4. Validate All Inputs
5. Implement Identity and Authentication Controls
6. Implement Appropriate Access Controls
7. Protect Data
8. Implement Logging and Intrusion Detection
9. Leverage Security Frameworks and Libraries
10. Error and Exception Handling

Thursday, January 14, 2016

Hibernate n+1, and Error: a different object with the same identifier value was already associated with the session

I ran into this issue today. It is somewhat related to the causes behind the LazyInstantiationError, in that it is hibernate Sessions getting into a twist.

I had a class structure where we had a Process object, that contains many ProcessEvents. Also, the processEvents could be nested, so optionally they could refer to a parent ProcessEvent.

In grails

class Process {

  static hasMany = [processEvents: ProcessEvent]
  public enum ProcessStatus {
    QUEUED, PROCESSING, SUCCESS, WARN, FAILED
  }
  public enum ProcessSeverity {
    CRITICAL, ERROR, WARN
  }

  //Persisted members
  String name
  Date initiated
  Date complete
  Float progress //Progress percentage
  ProcessStatus status
  String userId
  Map context     //Map to pass arbitrary data
  Date dateCreated
  Date lastUpdated
  ProcessSeverity severity // To determine how to log the error

  static transients = ["context"]

  static constraints = {
    name(blank: false, nullable: false)
    initiated(blank: false, nullable: false)
    complete(blank: true, nullable: true)
    progress(blank: false, nullable: false, max: 100F)
    userId(blank: false, nullable: false, maxSize: 20)
    severity(nullable: true)
    dateCreated(editable: false, required: true)
    lastUpdated(editable: false, required: true)
  }

  static mapping = {
    processEvents sort: 'id'
    processEvents fetch: 'join'
    sort initiated:  'desc'
    processEvents cascade: "all-delete-orphan"
  }

A few things of note here, is in the mapping, we are specifiying fetch join, for the processEvents. This means that instead of loading the processEvents individually (n+1 loads), we bulk load all in advance. Be careful with this if you have large tables, as this can quickly mount up.

Note also we set the processEvents to cascade all deletes, so that all child events get deleted when the parent process is deleted. Note this may not be needed since we have a belongsTo in the processEvent below

class ProcessEvent {
static belongsTo = [process: Process, parent: ProcessEvent]

  public enum EventLevel {DEBUG, INFO, WARN, ERROR}

  String message;
  EventLevel eventLevel
  Date dateCreated
  Date lastUpdated
  Date timestamp
  Boolean hasChildEvents = false // This is for performance increase, instead of calling DB.
  
  static constraints = {
    parent(nullable:  true)
    message(maxSize:3000)
    dateCreated(editable: false, required:true)
    lastUpdated(editable: false, required:true)
    timestamp(editable: false, required:true)
    hasChildEvents(required:false, nullable: true)
  }

  /** table mappings */
  static mapping = {
    parent index: 'processEvent_idx'
    process index:  'process_idx'
sort id:"asc"
  }


  void setMessage(String d){
        message = d?.length() > 3000 ? d.substring(0,3000) : d
    }

}

In the processEvents, we have 2 belongs 2 relations, denoting that all processEvents are a child of a single process, and (optionally) a single parent processEvent.

We began to see the Hibernate Error a different object with the same identifier value was already associated with the session once we added the belongsTo processEvent clause.

The problem was in our add method
Originally we had it coded this way. This will add a new ProcessEvent, to an existing Process object, and an exsitng processEvent parent Event. (We have another method where we do not sepcify a ProcessEvent parent, but that was not causing any problems)

public ProcessEvent addProcessEvent(Long argProcessId, String argMessage, EventLevel argEventLevel, ProcessEvent parent)
{
        if (parent != null) {
            parent = parent.refresh()
            parent.hasChildEvents = true
            parent.save(flush: true)
        }
Process pd = Process.findById(argProcessId)
        ProcessEvent pe = new ProcessEvent(
            message: argMessage,
            eventLevel: argEventLevel,
            timestamp: new Date(),
            parent: parent)
        pd.addToProcessEvents(pe)

      saveProcess(pd)  // saves top level Process, flushes, and logs errors
       pe

}

When we got to the pd.addToProcessEvents (which basically does a save on the Process parent object), it would fail and throw the Hibernate exception.

With some help from Stackoverflow. It mentioned that we had multiple java objects referring to the same row. 
The problem was that we were had a java reference to the parent processEvent object (parent). However we were also loading (findById) the Process object, which was loading a 2nd java reference to the same processEvent object. When we then saved it, there were 2 java references to parent, which was not correct.

The correct version was to load the top level Process object first, and then use the parent ProcessEvent from there. See below

public ProcessEvent addProcessEvent(Long argProcessId, String argMessage, EventLevel argEventLevel, ProcessEvent p)
{
        Process pd = Process.findById(argProcessId)
        Iterator i = pd.processEvents.toArray().iterator()
        ProcessEvent parent=null
        while(i.hasNext()) {
            ProcessEvent next = i.next()
            if(next.id==p.id){
                parent = next
                continue
            }
        }
        if (parent != null) {
            parent.hasChildEvents = true
        }
        ProcessEvent pe = new ProcessEvent(
            message: argMessage,
            eventLevel: argEventLevel,
            timestamp: new Date(),
            parent: parent)
        pd.addToProcessEvents(pe)

      saveProcess(pd)
       pe

}

Worth mentioning also are some other pages with good information
Gorm gotchas part 1, part2, and part 3