On DOC categoriesRegistered and categoriesNotRegistered do not exist. Instead addCat and removeCat and the correct fields.

User's current semester did not update yet.

In order for a user to be able to filter a query, the following HTTP Request should be sent

https://api.it.teithe.gr/announcements/public?q={%22title%22:%22newsletter%22}

However, due to the HTTP protocol, all special characters should be URL encoded by the client making the request.

On your endpoint, you are expecting pure JSON string which is not attainable due to the restriction mentioned above.
Notice on

that you're not URL Decoding before parsing the JSON.

Remmediation

Use a URL Decode function before parsing JSON

  const querystring = require('querystring');

  if (Object.prototype.hasOwnProperty.call(query, 'q')) {
    formatedQ = JSON.parse(querystring.decode(query.q))
    delete query.q
  }

Note

You can use any other URL decoder, the above one is mentioned purely as an example of remedy code.

https://github.com/helmetjs/helmet

Code climate indicates that the code style of the routes/bulletinBoard/announcementFiles/index.js is not properly formatted. Probably the command npm run standardFix needs to be run.

I currently have more than 700 notifications.

I would suggest that we should have a limit of 50 as well as descending sort option to get the latest ones. Or maybe remove old ones and keep some period of 3 months.

Sorting on categories by query on the URL does not seem to work properly.

I was stress testing the app in a local server and I discovered that the responses are sent in batches of 5. After some research I found that mongoose uses its default connection pool size, which is 5 (documentation). However, MongoDB's native driver has a default of 100 connections (documentation).

I would recommend changing mongoose.connect call in app.js to:

mongoose.connect(config.MONGO[process.env.NODE_ENV], {
  connectTimeoutMS: 120000,
  socketTimeoutMS: 120000,
  poolSize: 100 // MongoDB's default
})

The announcements should be by default sorted in descending order by modification time (except the case that the argument sort is given). This should be applied in all formats (json, rss, atom). I propose that the announcement array should be sorted just after it is constructed and before it is converted to a text format.

The url link is wrong.
Has to be chenged from:
https://apps.it.teithe.gr/announcements/ID
to
https://apps.it.teithe.gr/announcements/announcement/ID

Add attachment tag to the feed.

The request down returns:
{"error":{"message":"Συνέβη κάποιο σφάλμα.","type":"WrongEndPointError","code":"2000"}}
i dont know what went wrong.
The functionality of this route has to be more simple,
e.g 2 parameters, addCat:['a','b'], removeCat:['a','c']
Register user into a and b categories and removes him from a and c if he is registered. addCat and removeCat must be optional.

curl -X PUT
-H "x-access-token: eyJhbGciOiJSUzUxMiIsInR5cCI6IkpXVCJ9.eyJ1c2VySWQiOiI1MTA2Iiwic2NvcGUiOlsiaWQiLCJjbiIsInVpZCIsImVkdVBlcnNvblNjb3BlZEFmZmlsaWF0aW9uIl0sImhhc2giOiIyMmNuOXM2bG9qZGZ0cmE5bzJkbSIsImlhdCI6MTUzMDYyODg5NiwiZXhwIjozNTMwOTI5MDE2LCJhdWQiOlsiNTlhOTlkNTk4OWVmNjQ2NTc3ODA4NzljIl19.CC-mr6gI43FXKV02KKNTJNB1C-RmrvXi4sTbURwHJiyMSgh99iTytKGKRma5VQx5t7fwDW8tblMTG0HAXxC6jZo9LDJEaso5t-v9JoaIkdi_f5ZDCfRa9XZRuC_VwuSmlx4ShMryn9gvRx9bZhGSir5Nm154yOVrqjvjrxmZZTIiMgRQU2c4s3BJ0ihkIgBm-a4LlfoVEr77MgV5Uk249HvvsHXD-6Z8_FkPLI7SA3rDkyt18Ye5qQiDZUUzGe5kBfsiUV6A89JG3wYHJHowZ3brahvHgKbqMl17GUI2Jpiq-7x2l_1U70_mGvAc79_gkGer3jJlHae-5TH7IY2pXxBvNz8PIkv9TGNxbhrlW7JowbA1G_cAFtiEmDWWhuUkFYySVO4bF9aE_lXQR79Pj8kQWesZ4z_HrApxTVBe320Q8vyhrtR9Msg9xOdmxsshB2SAp2ocjc9fme6PIIl2XY5rrEObBJUCakcQAK8J6fSJP69zJuU0hQa3g2IYyS4K5W4vPFYB1NVVOwmZ2D5POpbaDWPer7VAwuy9B8662tHb2VokdbZ6eH9pmr5b2vbZLRvngLu90rsBtlf34JYuagY2qP8vhUe9yqM7qAN9F6odr5jOq6UquK6UUhwXV_PGV33xjZRt12nz2aG8W9rTNiEkq9B6srF5pz1bALApYLE"
-H "Content-Type:application/json"
-d '{"categoriesRegistered":[], "categoriesNotRegistered":[]}'
api.it.teithe.gr/categories/register

There are some TODOs about refactoring or checking the functionality of code.

Response code: 502/503

Service Unavailable

The server is temporarily unable to service your request due to maintenance downtime or capacity problems. Please try again later.

Currently the access token generated via the /token endpoint has a short expiration time. With that being said, consider increasing the expiration time to a value bigger than the currently implemented.

Why increase the expiration time?

First of all, having a very short (2 minutes) expiration time for an access token seems a pretty bad idea to implement. Users need to be hammering the API every two minutes to get a refresh token.
This seems unnecessary and impractical. Apart from the normal API endpoints that need to be consumed frequently, authentication flow needs to be a well performing part of the API in order to avoid performance bottlenecks. Both user experience as well as data consumption can improve by increasing the time allotted to the expiration of the access token.

How could that impact the security of the API?

Well, from a security standpoint, even if you had the expiration time set to 1ms still, you need to make sure that you have a very good implementation of the authentication mechanism in order to be certain that it is secure enough. Setting the expiration time to 2 minutes not only makes the process noisy for the network traffic but it doesn't provide necessarily 'additional' security as long as the implementation is correct.

Suggestion

I would suggest that you increase the token expiration time to provide a more smooth user experience as well as to rest assure that your application doesn't experience any overheads just from the token refreshing.

Please consider the following stackoverflow post.
https://stackoverflow.com/questions/26048945/oauth-v2-google-api-expiry-access-token

After changing password, "pwdChangedTime" is not synced with current time by 2 hours and 6 minutes. (Double checked)

I temporarily solved this issue by editing test/setup.js. I changed NODE_ENV to development and access_token to the token I generated following the instructions in the wiki.

Error: ENOENT: no such file or directory, open './public_test.pem'
    at Object.openSync (fs.js:439:3)
    at Object.readFileSync (fs.js:344:35)
    at Object.<anonymous> (/path/to/IT_API/configs/auth.js:7:17)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at Module.require (internal/modules/cjs/loader.js:637:17)
    at require (internal/modules/cjs/helpers.js:22:18)
    at Object.<anonymous> (/path/to/IT_API/routes/bulletinBoard/announcements/index.js:11:14)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at Module.require (internal/modules/cjs/loader.js:637:17)
    at require (internal/modules/cjs/helpers.js:22:18)
    at Object.<anonymous> (/path/to/IT_API/app.js:11:23)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at Module.require (internal/modules/cjs/loader.js:637:17)
    at require (internal/modules/cjs/helpers.js:22:18)
    at Object.<anonymous> (/path/to/IT_API/test/setup.js:8:16)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at Module.require (internal/modules/cjs/loader.js:637:17)
    at require (internal/modules/cjs/helpers.js:22:18)
    at requires.forEach.mod (/path/to/IT_API/node_modules/mocha/bin/_mocha:511:3)
    at Array.forEach (<anonymous>)
    at Object.<anonymous> (/path/to/IT_API/node_modules/mocha/bin/_mocha:510:10)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:742:12)
    at startup (internal/bootstrap/node.js:283:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:743:3)

The code down was (and is) on services. Creates users homeDirectory attribute on ldap.
Has to be moved into buildUser function too on ldap.
Also must add static attribute loginShell: '/bin/bash' on user create.

let changeHomeDir
      if (user.eduPersonAffiliation === 'student') {
        let tmpHomeDir = '/home/' + user.eduPersonAffiliation + '/' + user.eduPersonPrimaryAffiliation + '/' + user.regyear + '/' + user.uid
        changeHomeDir = new ldap.Change({
          operation: 'replace',
          modification: {
            homeDirectory: tmpHomeDir
          }
        })
      } else {
        let tmpHomeDir = '/home/' + user.eduPersonAffiliation + '/' + user.uid
        changeHomeDir = new ldap.Change({
          operation: 'replace',
          modification: {
            homeDirectory: tmpHomeDir
          }
        })
      }

Right now, the API returns all pages instantly on a single call. That can really slow down the usage of the Client application, especially on a slow internet connection. On production, the response to /api/announcements is roughly 2.1MB. That's big! Results can be paginated directly from the API (e.g. using cursor methods on the database).

Although it's not related to the API, as far as I know the Client app uses DataTables which can support server-side paginated content without much pain, given you have correctly set the parameters related to the total available data.

Check if the user is active (status =1) before sending reset mail.

change code: err.httpCode, to code: err.code

Observations

I came across this function and am wondering why the application is relying on two specific HTTP request headers in order to find out the client's IP address.

As the user has complete control over the HTTP Requests, he/she can deliberately inject a custom IP address (probably a fake one) via the x-forwarded-for or x-real-ip in order to trick the back-end API to consume that as the client's IP address.

By observing the (boolean) checks performed by the getClientIp function, we can clearly see that the first checks are performed with the HTTP Request headers and then via the req.connection.remoteAddress which poses a risk as the user can achieve injection of a fake IP every time.

Consideration

Some testing needs to be done in order to check if my hypothesis on this holds true.

@apavlidi @kvisnia Please do let me know how exactly this function works and how its implementation is meant to work.

Thanks
Aldi

We need the Notifications documentation ASAP.

Hi @apavlidi ,
Please take a look at the following:

Looks like you forgot to remove such sensitive information (mongodb database password) from the code when published it to GitHub.

PATCH /profile returns code 502

Body:

<html><head>
<title>502 Proxy Error</title>
</head><body>
<h1>Proxy Error</h1>
<p>The proxy server received an invalid
response from an upstream server.<br />
The proxy server could not handle the request <em><a href="/profile">PATCH&nbsp;/profile</a></em>.<p>
Reason: <strong>Error reading from remote server</strong></p></p>
<hr>
<address>Apache/2.4.25 (Debian) Server at api.it.teithe.gr Port 443</address>
</body></html>

The Documentation examples says this:

_Lets say you want to filter the results by searching the text that contains the string "123".

GET /example?q={"text":"123"}_

This is wrong. Results are those whose text == "123" and not those who contain the text "123".

Attachments are missing in rss and atom feeds.

Change announcement text length to unlimited from joi chcek.
(edit announcement text length also)

GET Requests should return 401 when access token expired instead of 400.

Access token error:
{"error":{"message":"An active access token required to complete this action.","type":"TokenInvalidError","code":400}}