Caching a promise object in AngularJS service

Is this the right approach?

Yes. The use of memoisation on functions that return promises a common technique to avoid the repeated execution of asynchronous (and usually expensive) tasks. The promise makes the caching easy because one does not need to distinguish between ongoing and finished operations, they’re both represented as (the same) promise for the result value.

Is this the right solution?

No. That global data variable and the resolution with undefined is not how promises are intended to work. Instead, fulfill the promise with the result data! It also makes coding a lot easier:

var dataPromise = null;

function getData() {
    if (dataPromise == null)
        dataPromise = $http.get("data.json").then(function (res) {
           return res.data;
        });
    return dataPromise;
}

Then, instead of loadDataPromise().then(function() { /* use global */ data }) it is simply getData().then(function(data) { … }).

To further improve the pattern, you might want to hide dataPromise in a closure scope, and notice that you will need a lookup for different promises when getData takes a parameter (like the url).