[KnackSteem API] - More security and features

in #utopian-io6 years ago

Repository & Pull Requests

https://github.com/knacksteem/knacksteem-api/
https://github.com/knacksteem/knacksteem-api/pull/11
https://github.com/knacksteem/knacksteem-api/pull/12

What is KnackSteem?

"Do you have any talent? If yes! then KnackSteem is for you."
"Rewards people with talents, it can be any talent, anything you know how to do best is highly welcome on the platform. "
Source: Discord Channel :D

Screen Shot 2018-07-05 at 5.15.37 AM.png

Changes Made

Integration test

This is important to test the API in an agile way. For instance, what if I make a change and I want to ensure that everything is working as expected? It will be too tedious to manually test everything. Here is where integration test come to the rescue! With simple lines of code, I can create automated tests to test the API and check against expected responses.

Example code:

describe('GET /v1/posts', () => {
  it('Should return a 200 response with the posts or not posts at all', () => {
    return request(app)
      .get('/v1/posts')
      .expect(httpStatus.OK)
      .then((res) => {
        expect(res.body.results).to.be.an('array');
        expect(res.body.count).to.be.an('number');
      });
  });
});

Basically, it makes a request to the API instance and expects a 200 response. In addition, it expects the results field in the body to be an array and count field to be a number.

Avoid parameter pollution

This is an issue I have seen in several APIs before. This basically consists in adding the same field twice in a GET request to an API. For instance, let's suppose that the following URL is an URL of an existing server: https://example.com/posts. This endpoint should list all the posts stored in the database. Also, this endpoint accepts a parameter called "user" to filter out posts just from this user. If I do a GET request adding a user it will show as the following: https://example.com/posts?user=jaysermendez. The server will parse this parameter as a string and will return the correct data. However, if I do the following request: https://example.com/posts?user=jaysermendez&user=jaysermendez, the server will take the parameter as the following array of strings: ['jaysermendez', 'jaysermendez']. This will bypass a validation of the endpoint since the parameter itself is still a string but read as an array of strings. If this is not handled, it may cause your API crash and will need to reset it.

To avoid this issue, there is a package called hpp which will handle this behavior. The way this package handle this behavior is in the following steps:

1 - It will take the parameters as it
2 - Will split the request's query into two if there is a polluted parameter: query and polluted query
3 - Will assign the last value in the array to the query which will be read by the endpoint operation later on.

Enable CORS for production mode

In production, we do not want non-authorized third-parties to use our API. To avoid it, in a production environment a CORS policy is set to only allow requests from whitelisted domains. If a non-whitelisted domain tries to make a request, the server will refuse to serve the content to this client.

Related code:

// Enable CORS only in production mode
if (env !== 'development') {
  // enable CORS - Cross Origin Resource Sharing
  // chrome-extension://fhbjgbiflinjbdggehcddcbncdddomop is Postman
  const allowedOrigins = [
    'https://knacksteem.org',
  ];
  app.use(cors({
    origin(origin, callback) {
      // allow requests with no origin
      // (like mobile apps or curl requests)
      if (!origin) return callback(null, true);

      // If the origin is not allowed, reject the request
      if (allowedOrigins.indexOf(origin) === -1) {
        const msg = 'The CORS policy for this site does not allow access from the specified Origin.';
        return callback(new Error(msg), false);
      }
      return callback(null, true);
    },
  }));

// Else if the env is development
} else {
  app.use((req, res, next) => {
    res.header('Access-Control-Allow-Origin', '*');
    res.header('Access-Control-Allow-Headers', 'Origin, X-Requested-With, Content-Type, Accept');
    next();
  });
}

Note that "cors" is a third party package to easily handle the rules.

Cleanup code

Some code from auth and user model were not in use. I proceed to delete all these codes and its reference in any other file. In addition, I've removed the unused dependencies of the project to make it lighter.

Add more fields to the API responses && Add them to the docs

As requested by the frontend developer of this project, I added and modified the API responses to have consistent names of the fields. For instance, some of my endpoints were sending different fields' names which can lead to issues if frontend is reusing components to render data.

In addition, these new fields were documented in the API docs so any other developer can refer to it at any time.

Add users endpoint

The users endpoint (v1/stats/users) will return all the users on the server (excluding internal private information). The purpose of this endpoint is to list them in the admin panel so users can be easily banned/unbanned, etc. There are several possibilities to use this endpoint:

  • Get a single username: v1/stats/users?username=jaysermendez
  • Get users by banned status: v1/stats/users?banned=true
  • Search for users: v1/stats/users?search=jayser
  • Limit the number of users to render: v1/stats/users?limit=5
  • Skip a number of users: v1/stats/users?skip=2

Related code:

/**
 * Method to query and list all users in database
 * @param {Object} req: url params
 * @param {Function} res: Express.js response callback
 * @param {Function} next: Express.js middleware callback
 * @public
 * @author Jayser Mendez
 */
exports.allUsers = async (req, res, next) => {
  try {
    // Grab the params from the request
    let { limit, skip } = req.query;
    limit = parseInt(limit, 10);
    skip = parseInt(skip, 10);

    // construct the query for database
    const query = constructQuery(req);

    // Find all the users from database
    const users = await User.find(query)
      .limit(limit || 25)
      .skip(skip || 0)
      .select({
        _id: 0,
        username: 1,
        roles: 1,
        isBanned: 1,
        bannedUntil: 1,
        banReason: 1,
        bannedBy: 1,
        createdAt: 1,
      });

    // Check if there is a response
    if (users.length > 0) {
      // Send the response to the client formatted.
      return res.status(httpStatus.OK).send({
        status: httpStatus.OK,
        results: users,
        count: users.length,
      });
    }

    // Otherwise, return 404
    return res.status(httpStatus.NOT_FOUND).send({
      status: httpStatus.NOT_FOUND,
      message: 'There are not results matching your query.',
    });

  // Catch errors here.
  } catch (err) {
    return next({
      status: httpStatus.INTERNAL_SERVER_ERROR,
      message: 'Opps! Something is wrong in our server. Please report it to the administrator.',
      error: err,
    });
  }
};

And its helper to construct the query:

/**
 * Method to construct query based on parameters
 * @param {Object} req: url params
 * @private
 * @author Jayser Mendez
 */
const constructQuery = (req) => {
  const { search, username, banned } = req.query;

  // Query for username
  const usernameCondition = (username);
  const usernameQuery = { username };

  // Query for search
  const searchCondition = (search);
  const searchQuery = { username: { $regex: search, $options: 'i' } };

  // Query for banned users
  const bannedCondition = (banned);
  const bannedQuery = { isBanned: banned };

  // All Conditions (exclude usernameCondition since it is a single result)
  const allConditions = (searchCondition && bannedCondition);
  const allQuery = {
    username: { $regex: search, $options: 'i' },
    isBanned: banned,
  };

  /**
   * If saerch and banned exist in the query, return query based by search and banned.
   * Else if If the username exist in the query, return query by user.
   * Else if the search is in the query, return query by search.
   * Else if banned is in the query, return query by banned
   * Else return all users
   */
  // eslint-disable-next-line
  return allConditions ? allQuery : ( usernameCondition ? usernameQuery : ( searchCondition ? searchQuery : (bannedCondition ? bannedQuery : {})));
};
Cover validation for more endpoints

More validations were added to ensure that the provided parameters are the expected. If any of the parameters is incorrect, the server will let the client know which param is incorrect.

Example code:

const Joi = require('joi');

module.exports = {
  // POST /v1/posts/create
  create: {
    body: {
      access_token: Joi.string().min(6).max(512).required(),
      permlink: Joi.string().required(),
      category: Joi.string().required().max(25),
    },
  },

  // GET /v1/posts/:author/:permlink
  single: {
    param: {
      author: Joi.string().required(),
      permlink: Joi.string().required(),
    },
    query: {
      username: Joi.string(),
      limit: Joi.number(),
      skip: Joi.number(),
      search: Joi.string(),
    },
  },
};
Handle deleted posts and post without images

When a post is deleted from the blockchain, our database still keeps its reference. When a request is made to the server and it fails to query a post from steem API, the request is stopped as nodejs will throw an error. What I did to solve this issue was determine if the steem API sends a positive response. If not, just skip this one instead of throwing an error. In this way, deleted posts will not prevent the API to load the rest.

Related code:

// Fetch the http GET call results
      const response = await request({ url, json: true });

      // If the post does not have an id, skip it
      if (!response.id) {
        index += 1; // Since the post is skiped, also skip one position
        return null;
      }

Basically, first, the API will grab the post from the database and will construct an URL to make a call to the steem API. Then, it will check if the response contains a field called id (which is always present in responses from the steem API). If the field is not present, it means that this post does not exist. Thus, the index is increased by one since this one is skipped and null is returned instead of the post object. The index is used to correctly assign the category and the tags from the database to the steem API response.

At the end of the process, the array is cleaned to filter out null values:

results = results.filter(e => e);

In addition, posts without images were throwing errors due to a missing param. To avoid this behavior, a ternary operator was added to check if the field exists. If the field does not exist, the default value of the cover image of the post will be null.

Related code:

// Determine if the cover image exists
const coverImage = response.json_metadata.image ? response.json_metadata.image[0] : null;
Add comments with replies to single post response

Honestly, this is one of the hardest parts for me: Handle all the comments including the replies to each of them in a nice way. To achieve this, I made a recursive function to keep grabbing the replies until the children number is not larger than 0.

Related code:

/**
 * Method to fetch comments with replies
 * @param {String} author: author of the post
 * @param {String} permlink: permlink of the post
 * @param {String} username: username of the current user
 * @param {Function} next: Express.js middleware callback
 * @private
 * @author Jayser Mendez
 */
// eslint-disable-next-line
const constructComments = async (author, permlink, username, next) => {
  try {
    // Inner function to fetch replies
    // eslint-disable-next-line
    const fetchReplies = (author, permlink) => {
      // Call Steem RPC server to get the replies of the current post
      return client.sendAsync('get_content_replies', [author, permlink])
        // eslint-disable-next-line
        .then((replies) => {
          // Map the responses with the post
          return Promise.map(replies, (r) => {
            // If there are replies to this comment, recursively grab them
            if (r.children > 0) {
              // Fectch the replies of the comment recursively by calling the method again
              return fetchReplies(r.author, r.permlink)
                .then((children) => {
                  // Determine if the current reply is voted by the username
                  const isVoted = helper.isVoted(r.active_votes, username);

                  // Calculate the total payout of this reply
                  const totalPayout = parseFloat(r.pending_payout_value) +
                        parseFloat(r.total_payout_value) +
                        parseFloat(r.curator_payout_value);

                  // Return the formatted reply
                  return {
                    description: r.body,
                    parentAuthor: r.parent_author,
                    authorImage: `https://steemitimages.com/u/${r.author}/avatar/small`,
                    postedAt: r.created,
                    url: r.url,
                    permlink: r.permlink,
                    authorReputation: steem.formatter.reputation(r.author_reputation),
                    author: r.author,
                    category: r.category,
                    votesCount: r.net_votes,
                    totalPayout,
                    isVoted,
                    repliesCount: r.children,
                    replies: children,
                  };
                });
            }

            // Determine if the current reply is voted by the username
            const isVoted = helper.isVoted(r.active_votes, username);

            // Calculate the total payout of this reply
            const totalPayout = parseFloat(r.pending_payout_value) +
                        parseFloat(r.total_payout_value) +
                        parseFloat(r.curator_payout_value);

            // Return the formatted reply
            return {
              description: r.body,
              parentAuthor: r.parent_author,
              authorImage: `https://steemitimages.com/u/${r.author}/avatar/small`,
              postedAt: r.created,
              url: r.url,
              permlink: r.permlink,
              authorReputation: steem.formatter.reputation(r.author_reputation),
              author: r.author,
              category: r.category,
              votesCount: r.net_votes,
              totalPayout,
              isVoted,
              repliesCount: r.children,
              replies: [],
            };
          });
        });
    };

    // Return the replies recursively
    return fetchReplies(author, permlink);

  // Catch any possible error.
  } catch (err) {
    return next({
      status: httpStatus.INTERNAL_SERVER_ERROR,
      message: 'Opps! Something is wrong in our server. Please report it to the administrator.',
      error: err,
    });
  }
};
Add separated endpoints to get only votes or comments of a post

Sometimes, there is no need to load the full post if the user only wants to see the comments of the votes itself. To handle this, two new endpoints were added and can be called as the following:

  • GET v1/posts/:author/:permlink/votes : will return the votes of the specified post
  • GET v1/posts/:author/:permlink/comments : will return the comments of the specified post

What is next?

  • Continue adding the integration tests
  • Add more validations to avoid unexpected parameters
  • And so much more :)
Sort:  

This is high quality code and I'm pleased to see that!
Congrats on this project idea by the way.

A few remarks:

  • Your comments are great but it's a bit of an overkill. It feels like you want the best score from utopian :p => https://github.com/knacksteem/knacksteem-api/pull/11/files#diff-2dbc47226b708066000a2b1df9653f12R113 this one for instance, we know how to read code and your comment basically repeat what the code says but in english. You have a lot of them like that and honestly they are useless. For me, the comment should provide additional information. Like your comments at each function declaration, a short description, params explaination. This is great. Comment inside the code should only exists if the code is complicated to read. Otherwise it only pollutes I think.

  • https://github.com/knacksteem/knacksteem-api/pull/11/files#diff-fdcd0f9e663edcc4cebbc84cf0c97d50R148 => May be you could use the rule that's being disabled. Here I can see that's because ternary expression are nested and honestly you should favor code clarity other being concise. Here it takes a lot of time to understand what was written.
    You got a few lines disabled like that

And I think that's all :D. It's really small things because you've done an awesome work. Congrats! Keep coding that way.

Your contribution has been evaluated according to Utopian policies and guidelines, as well as a predefined set of questions pertaining to the category.

To view those questions and the relevant answers related to your post, click here.


Need help? Write a ticket on https://support.utopian.io/.
Chat with us on Discord.
[utopian-moderator]

ahaahahah not really want to score best but I do agree with you hahahaahaha. I will follow your advise and not include comments everywhere :) just when needed. And now that I see, I did really write PARSE JSON METADATA 😂it is clear that is parsing it :P.

Thanks for your feedback. I will be more carefully when including comments inside the code.

Hey @jaysermendez
Thanks for contributing on Utopian.
Congratulations! Your contribution was Staff Picked to receive a maximum vote for the development category on Utopian for being of significant value to the project and the open source community.

We’re already looking forward to your next contribution!

Want to chat? Join us on Discord https://discord.gg/h52nFrV.

Vote for Utopian Witness!

Great work Jayser

Thanks :)

Congratulations @jaysermendez! You have completed the following achievement on Steemit and have been rewarded with new badge(s) :

Award for the number of upvotes

Click on the badge to view your Board of Honor.
If you no longer want to receive notifications, reply to this comment with the word STOP

To support your work, I also upvoted your post!

Do not miss the last post from @steemitboard:
SteemitBoard World Cup Contest - Brazil vs Belgium


Participate in the SteemitBoard World Cup Contest!
Collect World Cup badges and win free SBD
Support the Gold Sponsors of the contest: @good-karma and @lukestokes


Do you like SteemitBoard's project? Then Vote for its witness and get one more award!

@jaysermendez You have received a 100% upvote from @qualitycentral because this post did not use any bidbots and you have not used bidbots in the last 30 days!

Upvoting this comment will help keep this service running.