Replacing the CSRF package(CSURF) with Double CSRF package

[pushyamig]

Fixes #410 (Test Plan is updated in the Issue https://github.com/tl-its-umich-edu/canvas-course-manager-next/issues/410 Latest Build from this PR https://github.com/pushyamig/canvas-course-manager-next/actions)

The approach taken here is the double csrf library is setting the CSRF token cookie (with hash) and also HttpOnly flag is enabled so that FE code doesn't read the cookie. The CSRF token is then passed in as part of GET call as response object (As lib author recommendation) and stored in the react state (As props). The token is then sent with POST/PUT/DELETE call and double csrf validate it and then those https calls are processed. please refer to step#4 for the example implementation.

1. The new Package that is chose for this Double CSRF
2. .env have a property added called CSRF_SECRET. For local development if is not set it has default. This is something in line with TOKEN_SECRET and 'COOKIE_SECRET`. Non-Prod and Prod will always be set with this token
3. This PR is running in the Canvas Dev. For example
4. POC of the CSRF strategy adopted by this Library is provided by Package author is here
   1. Author suggesting is to not to set the CSRF TOKEN in Cookie but instead send it as Http Response and store a state attribute in Frontend like React props or state management design of the app
   2. CSRF token is only needed for state change HTTP methods. so for our app implementation POST, PUT, DELETE.
   3. As per author the CSRF cookie name should be either _Host and _Secure.
   4. I adopted `_Secure since _Host did not fit out needs since we were setting Path=/ and Domain to CCM URL. More info here what is correct params with _HOST

Note: For Local development you will see 2 call in Network as this is by Design when running React In StrictMode, So you won't see this behavior in CCMDev or Prod https://stackoverflow.com/questions/72238175/why-useeffect-running-twice-and-how-to-handle-it-well-in-react

In development, you will see two fetches in the Network tab. There is nothing wrong with that. With the approach above, the first Effect will immediately get cleaned... So even though there is an extra request, it won’t affect the state thanks to the abort.

In production, there will only be one request. If the second request in development is bothering you, the best approach is to use a solution that deduplicates requests and caches their responses between components:

[pushyamig]

This PR is up to date with the recent features commits

[jaydonkrooss]

Following the local testing steps you listed, everything behaves as expected. Removing CSRF token in the API causes errors. Code looks good, only thing left is the three codacy errors of unused imports, if you want to clear those warnings.

[pushyamig]

only thing left is the three codacy errors of unused imports, if you want to clear those warnings.

I looked at those error, I don't think those warning are accurate. So I don't know why that is warning. So I left as it

For example:

import { APP_FILTER } from '@nestjs/core'

APP_FILTER is used in the auth.module.ts as part of NestJS module declaration. So I don't think i am doing any different or syntactically wrong.

[jaydonkrooss]

only thing left is the three codacy errors of unused imports, if you want to clear those warnings.

I looked at those error, I don't think those warning are accurate. So I don't know why that is warning. So I left as it

For example:

import { APP_FILTER } from '@nestjs/core'

APP_FILTER is used in the auth.module.ts as part of NestJS module declaration. So I don't think i am doing any different or syntactically wrong.

I see, that's a little strange. Ok otherwise this looks good I'll approve

[pushyamig]

@jaydonkrooss , Thanks for the review. I will let @jonespm take a look as well before merging it.

[jonespm]

I'm not sure if it's possible to fix this check with Codacy but I think we could add to ignore it in the .eslintrc.js. "no-unused-vars": "off"

or declaring everything that's a global.

That's probably a separate issue. I'm not sure if there's a better workaround that works as I haven't configured this.

[jonespm]

I'm not going to be able to re-test this and will rely on Jaydons feedback. It looks like you've done the most research on this and I don't know how it could have been done any differently here.

[pushyamig]

I'm not sure if it's possible to fix this check with Codacy but I think we could add to ignore it in the .eslintrc.js. "no-unused-vars": "off"

In this example I described above the variable is being used in the file. So I don't know having no-unused-vars: off would do anything. Plus I did not configure the Codacy for it. WHen I looked at it I see ESlint and ESlint(Old) and bunch of other linters (which I never heard of) may be added by default by Codacy. Sam did this and don't know how much he configured and/or added by COdacy.

I need to comeback to Codacy configuration CCM. I will create an issue but will deal with it separately. We do have local linting from VsCodeeslintrc.js

[pushyamig]

I'm not going to be able to re-test this and will rely on Jaydons feedback. It looks like you've done the most research on this and I don't know how it could have been done any differently here.

Thanks @jonespm for considering for review. I will take that as approval from you if you don't want to click the approve button.

This PR has being a learning curve so I wanted more people looking at it and aware of the implementation.

[jonespm]

@pushyamig I think Codacy is also in "Configuration file" mode and also using that file. But Sam set it up and I haven't tested or tried to change anything. But I also see what you mean about it being used.

Maybe need to use the plugin it suggests @typescript-eslint/no-unused-vars? But yeah can experiment on a separate issue. These aren't the only 3 instances of that warning in this code.

[pushyamig]

I looked at the Myla Codacy configuration for FE seems very similar to what CCM has https://app.codacy.com/gh/tl-its-umich-edu/my-learning-analytics/patterns/list

I would assume we might need to come up with the approach for Codacy configuration for FE. Both project has linter configured as part of the code and VSCode integration. So Don't know how much is same/different with either of these configuration.