test review request

[Lellansin]

Hi there, glad to meet you.

It's the phasic work waiting for check, and here's the README for taking a look first.

Ask me if there are any questions.

[MiffyLiye]

1. README file mentions that the project has 100% test coverage, but some parameters in API document are not tested, e.g. dob field when create user. what do you think should be covered in tests?
2. API document mentions that dob field when create user should be number type, but string type also works. The dob field in response when get user is Date type, but actual response is string in ISO 8601 format. Can you suggest how to know if API document is up to date?
3. Create user API doesn't return created user's id, can you suggest how to get created user's id?
4. List users API supports page parameter, but doesn't support page size (or limit) parameter, why not also support limit or page size?
5. How to know if list users API returned all users, or should caller request next page?
6. Some APIs return HTML pages, some return json data, and some return both on different conditions. What is the use case of returning HTML page?
7. Some API requires authentication, and requires two cookies. What are the two cookies used for, why not combine them to one cookie?

[Lellansin]

Hi @MiffyLiye, thanks for your reading, here's my answers.

1. README file mentions that the project has 100% test coverage, but some parameters in API document are not tested, e.g. dob field when create user. what do you think should be covered in tests?

It's code line coverage, not for every use case, if we need a 100% use case test coverage, we should have the 100% complete case list before. Since I didn't get the list, I chose code lines to be coverd.

2. API document mentions that dob field when create user should be number type, but string type also works. The dob field in response when get user is Date type, but actual response is string in ISO 8601 format. Can you suggest how to know if API document is up to date?

Introducing a CI task to ensure it's up to date.

3. Create user API doesn't return created user's id, can you suggest how to get created user's id?

In my opinion, I just thought when there is POST /user for user creating, it seems the user would like an article resource more than an account for auth. So I make creating user process implemented through POST /account/signup, sign up is pairing with sign in, It makes sense to get user's id after signing in.

4. List users API supports page parameter, but doesn't support page size (or limit) parameter, why not also support limit or page size?

it's easy to support it, but if I did I also need to adjust corresponding tests. Considering the time cost and difficulty, bewteen undocumented details and important documented features (like Auth/Follow/...), I prefer to pay more attention to the important part in a short time during this test.

5. How to know if list users API returned all users, or should caller request next page?

I wrote this test with GitHub, and I chose the same way which GitHub did on its commit list.

There are two reason for me:

• If there is total count for pagination, this project's user will look like a administrator which want to manage the users. And what I got from our documentation, is prefer Twitter-like user, which indicated next page of friends instead of entire users pagination.
• it's easy, and the ease is beautiful (PS: I think the Github's commit list is more beautiful than some completed pagination page).

6. Some APIs return HTML pages, some return json data, and some return both on different conditions. What is the use case of returning HTML page?

Honestly, I use it for unit test, LOL. Actually, I usually separate them with the /api prefix for normal APIs. But in this test, I considered it as a pure API server at beginning, which I prefer using NGINX-like proxy to add the prefix.

With one moment, I found there could be a cool way to show the auth flow with puppeteer in the e2e test (let's check the gif🤠), I temporarily add these render page.

If that matters, I can turn to my usual way (add /api prefix for all APIs).

7. Some API requires authentication, and requires two cookies. What are the two cookies used for, why not combine them to one cookie?

Indeed, there are two cookies like xxx:sess/xxx:sess.sig, which the .sig one is the signature for the other. As I know it's a security mechanism of cookies which is depended by Koa. We can check it with the link.

I don't recommend turnning off this security mechanism, which means use only one cookie to save data. Because it will increase the risk of suffering the "Man-in-the-Middle" attack.

[MiffyLiye]

2. API document mentions that dob field when create user should be number type, but string type also works. The dob field in response when get user is Date type, but actual response is string in ISO 8601 format. Can you suggest how to know if API document is up to date?

Introducing a CI task to ensure it's up to date.

Can you clarify what the CI task will do? Since the API document is no executable as unit tests, or postman scripts, how to know it matches the real implementation?

5. How to know if list users API returned all users, or should caller request next page?

I wrote this test with GitHub, and I chose the same way which GitHub did on its commit list.

There are two reason for me:

• If there is total count for pagination, this project's user will look like a administrator which want to manage the users. And what I got from our documentation, is prefer Twitter-like user, which indicated next page of friends instead of entire users pagination.
• it's easy, and the ease is beautiful (PS: I think the Github's commit list is more beautiful than some completed pagination page).

You can see that in the GitHub commits page, the Newer button is disabled, and Older button is clickable on first page, the Older button is disabled, and Newer button is clickable on last page. GitHub's API gives you some way to know whether there is newer page or older page. How can the caller know if there is next page or not with your API response?

7. Some API requires authentication, and requires two cookies. What are the two cookies used for, why not combine them to one cookie?

Indeed, there are two cookies like xxx:sess/xxx:sess.sig, which the .sig one is the signature for the other. As I know it's a security mechanism of cookies which is depended by Koa. We can check it with the link.

I don't recommend turnning off this security mechanism, which means use only one cookie to save data. Because it will increase the risk of suffering the "Man-in-the-Middle" attack.

Can you explain more about the security risk, how to attack if there is no sig cookie verification?

Can you compare your session implementation with the following approaches:

1. generate random session id as access token, and save session payload in backend, set the session id in cookie.
2. generate session payload and signature and combine them in one string as access token, set that string in cookie, backend doesn't save session payload.

[Lellansin]

Can you clarify what the CI task will do?

CI task can be:

• non-invasive
  • Use AST parse to get it from the code (high-cost).
  • Design unit test, and get it from analysis of test results.
• invasive for old version
  • implements @Query, @Body, @Header decorators to register the parameters for doc.

And more:

• In runtime (follow observer pattern):
  • Get sampling with API from middleware to generate result sample and its doc. (This is part of what I've done during Alibaba, which is a feature called request redo for debug).

Since the API document is no executable as unit tests, or postman scripts, how to know it matches the real implementation?

If we don't have playground, we should check If the API's version and document's matches. Or try the runtime solution mentioned above.

You can see that in the GitHub commits page, the Newer button is disabled, and Older button is clickable on first page, the Older button is disabled, and Newer button is clickable on last page. GitHub's API gives you some way to know whether there is newer page or older page. How can the caller know if there is next page or not with your API response?

For the website to get next page, there were two selections for me:

First one:

Side	Content
Backend	Implements total count function for: user/follower/followings/nearby-normal/nearby-follower/nearby-followings, and update their tests.
Frontend	if (page >= Math.round(total / limit)) { // last page! }

Second one:

Side	Content
Backend	Do nothing more.
Frontend	if (resultList.lenght < limit) { // last page! } else if (!await fetch(/* another page */)) { // last page! }

Thorough ROI analysis within this test, I prefer the 2nd selection.

Can you explain more about the security risk, how to attack if there is no sig cookie verification?

I can use my mobile phone to put a WiFi hotspot in the , which take the same name as the public WiFi in the , like , and then modify DNS resolve on my mobile phone to my local HTTP proxy server. Then, when the user's request arrives at my proxy server, I can parse the user's request and tamper with the cookie. If the cookie is not signed, the server will consider the cookie as true and use it. (Updated: hide some sensitive words to ``.)

If you noticed HTTPS, I may say there are ways to downgrade it to HTTP for some websites, or for clareless users.

Can you compare your session implementation with the following approaches:

1. generate random session id as access token, and save session payload in backend, set the session id in cookie.
2. generate session payload and signature and combine them in one string as access token, set that string in cookie, backend doesn't save session payload.

1. High performance, high cost.
2. The performance is poor, and sometimes the gateway will reject the request with HttpStatusCode like 431 while the cookie is too large (4KB/8KB/..., according to the gateway), but low cost.

[xavierchow]

Thanks all, the PR and the discussions are pretty enough for our evaluation, closing it now.

We will get back to you soon. @Lellansin