Conversation
…orization check into projects route
…-Software into UnauthorizedAccess
…nticated method to set isAuthenticated value to true or false. Also added session requirement to auth
|
Changes Unknown when pulling d741140 on UnauthorizedAccess into * on development*. |
|
Why was sinon added? |
lib/auth.js
Outdated
| isAuth = true; | ||
| } | ||
| req.session.isAuthenticated = isAuth; // Set the isAuthenticate flag | ||
| next(); // Continue logic flow as normal |
There was a problem hiding this comment.
This comment is superfluous. The next callback is an express paradigm
|
The authentication status shouldn't be added to the session because this is middleware. If the middleware is added as ap-wide middleware, then the status is checked for every request and should be added to the request object. Conversely, if it is only route-specific middleware, the same thing applies because the check will be run anyway. |
|
The authentication check doesn't need to send a request to Jama. (Nor should it, it creates a large amount of overhead on requests that depend on the middleware.) We know a user is authenticated if both a session exists with username, password, and project keys and the current session is not expired. The session is only populated with those keys upon successful authentication and is destroyed on logout. All that is needed is to check that those keys exist on the session. |
|
@chances, the authentication status should be added as a global variable instead of session variable? Last time we talked about this you mentioned that the middleware should be route specific and each page will handle the check result. |
|
We used sinon for stubbing. |
| if (isValid && isServerAuthenticated(username, password, teamName)) { | ||
| isAuth = true; | ||
| } | ||
| req.session.isAuthenticated = isAuth; // Set the isAuthenticate flag |
There was a problem hiding this comment.
You just need to check if the session fields aren't falsey here, you don't have to make a request to jama. teamName is stored in .env in so you don't need to check the req.session.teamName field for that.
Then you can store the actual result in the req object so we can check it in the routes.
req.isAuthenticated = isAuth;
|
Now that it's using the middleware isAuthenticated, it's setting the req.isAuthenticated to true or false. Check that value and only do the work if it's true in the routes that matter such as project/get or graph/get (the else statement) so... etc. |
| "proxyquire": "^1.7.10", | ||
| "semistandard": "^8.0.0", | ||
| "sinon": "^1.17.4", | ||
| "sinon-chai": "^2.8.0", |
There was a problem hiding this comment.
sinon-chai is unused, remove it.
No description provided.