Assignment 7 improvements

[jd73]

I've been working through all the updated assignments. They've all worked pretty well for me up to assignment 7.

In comparison to the others, the scope of assignment 7 seems to be quite large. Not only are there more pieces to the assignment but the pieces themselves seem more complicated. I have looked through the currently accepted solution to try to reverse engineer how it works but I have come across a few main problems with that.

• It is not very clear what the new tests are trying to accomplish, particularly for auth-cookie. The purpose of generateSession is also not very clear and further complicates understanding the auth-cookie tests.
• There are some chunks of what appear to be commented out code in generateSession and auth-cookie. It's hard to tell what their purpose once was and why they may have been commented out.
• It seems hard to test files in isolation. It appears that the tests for auth-cookie need some auth functionality defined in user.js in order to pass and vice versa.

Would it be possible to rework the assignment or at the very least the solution to it a bit in order to try to isolate what exactly needs to be done in order to get the desired auth setup to work?

I would be happy to help contribute to some refactoring. As of right now I am not sure exactly where I would start but I will continue to try to figure out how things work and may do some cleanup on my own if I am able to.

[zoe-1]

Hi @jd73 , Good input about assignment7. I agree the assignment needs to be re-written, plus, I think your observations here will contribute to the re-write. Here are some thoughts about your questions:

• ./test/generateSession.js
  The purpose of this function is to create a session for a logged in user. To get 100% coverage on hapi-auth-cookie session logic, some tests require that a session already exist before making a request. So, the generateSession function does that for us. First, we run generateSession which gives us the cookie value for the session. Then, we use that cookie value to execute tests which require a valid cookie's existence.
• in respect to the commented out code in generateSession that is generate crumb logic for session8 and can be deleted.
  When we attempt to write 100% test coverage of crumb some tests require that a legitimate crumb already exist. So, assignment8 has generateSessionCrumb or generateCrumb.
• in respect to isolating test files, yes, the test files do require user data defined in user.json but that data is the same throughout the application so willing to allow different tests rely on the same data set. Assumption is the application will always rely on the same data source. I am ok with that. you?
• reworking the assignment, I think rewriting the assignment is reasonable. Already started a re-write here .
• In respect to the generateSession, generateCrumb, generateSessionCrumb logic, our tests need several things to be done repeatedly.
  • One, authenticate a valid user which results in a hapi-auth-cookie session cookie being created. This is the purpose of generateSession's existence.
  • Two, generating a crumb for a user. To increase security, any request to our api requires that a valid crumb exists. assignment8 has links explaining why we want to use a crumb.
  • Three, to get coverage for routes that require a valid hapi-auth-cookie session and crumb existance we need to generate valid request values for hapi-auth-cookie's session and the crumb. logout in the api is an example route that requires both. A user cannot logout without a valid hapi-auth-session cookie and valid crumbs existence. If you look at assignment8's test/api/user.js and find tests related to logout you will see generateSessionCrumb in action.

In summary, the generate scripts mentioned above are just test helpers. When a test needs a valid hapi-auth-cookie session or crumb to exists for the test to pass, we use these helpers to generate the needed cookie / headers values a request will need in order for it to pass and give us the coverage of code we want.

[jd73]

Yeah, that makes sense for what the high level purpose of generateSession and later generateCrumb is. For me, I am mostly confused about the details of how exactly it does that. This line in generateSession is particularly cryptic.

const cookieCleaned = headerSplit[0].match(/(?:[^\x00-\x20\(\)<>@\,;\:\\"\/\[\]\?\=\{\}\x7F]+)\s*=\s*(?:([^\x00-\x20\"\,\;\\\x7F]*))/);

I can tell that it is regex matching parts of the cookie header. I really have no idea what that regex is looking for though. Even adding just a comment with the expected before format and what the expected output should be would be very helpful, IMO. Or, if this is a pretty standard thing when working with cookies (I haven't done much with them myself) then maybe adding a comment with a link to some explanation of what the format is and what this regex is doing instead would be of use.

Regarding tests depending on users.json, I am definitely okay with them depending on users.js. But, I actually meant that it appears that the auth-cookie tests depend on user.js. I had copied the implementations of both user.js and auth-cookie from the accepted solution to try to work backwards from that to where I was starting from. When I used both implementations the tests for auth-cookie passed fine. When I swapped the solution's user.js for my work-in-progress user.js that validates input but doesn't yet contain any auth stuff, tests for auth-cookie (the accepted solution) started failing.

I looked at the refactor you linked to here. I like it so far. I like breaking the assignment down into different steps. I believe that may address the biggest source of my struggles which is just trying to figure out what order to implement things in so that when I work on one part, other pieces it depends upon are already completed. That does make me wonder if perhaps breaking it up like that could be considered a hint separate from the requirements so that those who know what they are doing or want a challenge can try to figure that problem out for themselves?

Also, thank you for the quick response. And please let me know if there is anything more specific I can do to help. I certainly don't want to make requests and expect others to do all the hard work.

[zoe-1]

@jd73 your thoughts and perspective are helpful. I think the main points of this discussion will make it into notes and explanations for assignment7. :-) The below addresses your question regarding .match(regex) in the tests. Plus, it takes some first steps toward explaining how to approach the hapi framework for the first time and how to study tests as a form documentation.

In respect to the .match(regex), the regex is from hapi-auth-cookie's tests. I believe the hapi-auth-cookie author's wrote the regex for hapi-auth-cookie's tests . Interestingly, this regex and the tests point to two important hapi concepts: 100% test coverage and using tests as documentation.

the tests are a form of documentation

IMHO, most people approach hapi wanting a tutorial or documentation to explain everything about hapi. Note, hapi has traditional documentation at hapijs.com. But, just as important as the website docs and a plugins' api documentation, a hapi plugin also provides documentation through it's tests. And, most programmers new to hapi are not used to this.

Our discussion of the match(regex) shows how hapi plugin test code can benefit a project. If tests were not studied, the regex we are discussing would not be in the project. The best way to understand the regex would be to read the JavaScript match(regex) docs and do something like the below to inspect the code.

const sessionCookie = headerSplit[0].match(/(?:[^\x00-\x20\(\)<>@\,;\:\\"\/\[\]\?\=\{\}\x7F]+)\s*=\s*(?:([^\x00-\x20\"\,\;\\\x7F]*))/);
console.log(sessionCookie.length);
console.log(sessionCookie[x]);

Perhaps, we need to write a tutorial/guide explaining how to parse request headers and cookies.

Once, someone understands to use hapi tests for documentation, they can go far with hapi. Understanding the tests of a hapi-plugin reap big rewards for your code. There are many snippets that can be reused in your personal project. Plus, there are nice coding styles that can be imitated. In respect to this project, it would have been impossible or much much more difficult to get 100% test coverage without studying the tests for hapi-auth-cookie and crumb. To emphasize the point, I did not fully comprehend how to use crumb until I studied the tests.

[zoe-1]

@jd73 good point about auth-cookie.js depending on user.js. we should refactor that.

[jd73]

I didn't get that generateSession was using an authenticated request to get a valid auth cookie before. I created a pull request adding some clarifying comments. https://github.com/hapijs/university/pull/212