-
Notifications
You must be signed in to change notification settings - Fork 0
Authentication routing clears item routing on page reload #123
Comments
This is likely caused by Apache attempting to load files that correspond to router locations and can be addressed by URL rewrite rules. See Serving Apps w/ Client Side Routing for details on Apache rewrite rules to address this. # Enable the rewrite module
RewriteEngine On
# Reverse proxy all requests to the API to the API CGI script
# See mod_rewrite docs: https://httpd.apache.org/docs/current/mod/mod_rewrite.html
RewriteRule ^api/(.*)$ http://localhost:5000/api/$1 [P]
+# Defer requests that are not files to client side routing
+# See: https://create-react-app.dev/docs/deployment/#serving-apps-with-client-side-routing
+RewriteCond %{REQUEST_FILENAME} !-f
+RewriteRule ^ index.html [QSA,L]
+# Uncomment the following line to serve a maintenance message.
+#RewriteRule .* https://engineering.purdue.edu/webqueue/webqueue2/build/index-maintenance.html [P]
+#DirectoryIndex index-maintenance.html
# Diable Auth
Satisfy any |
This was not fixed by adding the client side routing deferral described above but page reloading was fixed reloading. It may be the case that nested routing is the solution for this first issue in this thread. See: https://reactrouter.com/web/example/nesting |
Starting to work on this. |
Due to React's component rendering lifecycle, the state variables holding login state will render twice every time. On the first render, the state variable will be its default value, in this case false. On the second render, an attempt to refresh the access token is made and if it is successful the state variable will become true. This means that every time there is a change in auth state, the user is temporarily logged out and the login page is re-rendered causing both the login page to flash and the routing to be lost. All of this logic exists in the PrivateRoute component. The quick fix for this is to keep a copy of the URL match that directed a user to the PrivateRoute (the referer) and on successful login redirect them to the referrer. The long term fix for this would be to somehow memoize the login state per session or to add a check in AuthProvider's useEffect to preset the state variable for logged in. |
Implementing the quick fix for a release now. |
It would actually not be too much work to move the token to localstorage and load it form local storage to set a default state value that doesn't cause the login page to render everytime. For now the page is functional. This will be put on hold to get a release out. |
See #15 for more info on authentication and routing. |
I have been doing research on how we can securely store a user session even after they refresh the page. Here are some resources that I have looked at:
After I finish watching the tutorial I will update this issue with any relevant information. |
After looking at the different ways in which to store a user session we were able to look at a few important pieces of info.
We think that the best course of action may be to develop a way to check if an access token is valid. |
After doing some research I found this article that describes how we can store a state variable to local storage so that it persists even after a refresh. The article details creating a custom hook call What is
|
I still haven't been able to identify why storing the login state in localStorage causes the application to behave the way it does with the above implementation. It appears that storing the login state in localStorage is causing issues with how access tokens are given. I have taken a look at how the login state interacts with the access tokens but I haven't been able to identify any relationship that would cause this behavior. Tomorrow @campb303 and I will both take a look to see if we can identify what the cause of this issue is. |
Instead of storing the logged in state in local storage, we can store the entire JWT. If the JWT is present on page load, we assume the user is logged in; no key, not logged in. This would allow the user to bypass the login page on first load but presents two new problems. The first problem is that the only checks for JWT validity right now are HMAC equality (see jwt.io) and expiration time. I think the expiration time we're using now, 15 minutes, is a reasonable amount of time given the automatic refresh on the frontend and the 30 day refresh validity. However, with the only other check being the HMAC equality there is no association of a where the JWT can be used therefore if an attacker were to take the entire JWT and use it in a different session then they could gain unauthorized access. To counter this, we could store issued keys and some type of fingerprint about where they can be used in the API and if the fingerprint doesn't match we force a new login. For example, if that fingerprint were an IP we would say keyA is only valid for IP address B and if the request's IP address is not B; login again. |
It appears that when using the localStorage API in AuthProvider, QueueSelector's useEffect exits prematurely.
The To troubleshoot, @wrigh393 is going to undo all changes and implement the same changes one line at a time with testing. |
We were able to find a partial solution to getting the authentication to stay on refresh. Below I will detail what was implemented, how it works, and what problems we still need to address. What was implementedWe initialize the const [token, setToken] = useState(localStorage.getItem('access_token')); We then use the state of the token variable to set the state of the const [loggedIn, setLoggedIn] = useState(token ? true : false) In the async function tryRefresh(csrf_refresh_token) {
if (csrf_refresh_token === undefined) {
return false;
}
const new_access_token = await refresh(csrf_refresh_token);
if (!new_access_token) {
console.error("Failed to refresh access token.")
return false;
}
+ localStorage.setItem('access_token', new_access_token)
+ setToken(localStorage.getItem('access_token'));
} Issues presentWhile this allows us to navigate to a specific item by URL this only works if the queue is active in the There is also a problem with the access not properly refreshing. We are still looking into the cause of this issue. |
In order for items to be opened based on URL two things need to happen:
This may require changes in a few places:
I believe that the main two changes that will fix this problem are finding a way to set the sidebar open when navigating to an item with the URL and populating the |
The only place that should need changed is QueueSelector. Modify the Here's a state table for testing:
|
While react-router provides a useParams hook that allows us to get the params from our URL this only works if the component is the child of the The next step is to parse the URL so that we can get the queue and item number from the URL. The queue that we get from parsing the URL can be used to set the active queues when the cookie doesn't have a value. After getting those values from the URL we need to modify the We also need to be able to handle situations where we don't get data that we want as well as validating if the queue enters into the URL is a valid queue. |
Here are some things that I have found after doing some more experimenting with the
Initial // activeQueues can't be interacted with like an array here because cookies['active-queues'] is not an array.
//Setting it as an array would put an array in an array when adding queues with the QueueSelector.
const activeQueues = cookies['active-queues'] !== undefined && filteredUrl === undefined ? cookies['active-queues'].split(',') : handleChange funtion const handleChange = (event, newValue) => {
//Here setValue is called so that we can set the value state variable to whatever the newValue is
setValue(newValue)
// Set active-queues cookie to csv of selected queue names
const activeQueueOptions = {
path: "/",
expires: (_ => {
let expiration_date = new Date();
expiration_date.setDate(expiration_date.getDate() + 365);
return expiration_date;
})()
};
const activeQueues = newValue.map((value) => value.name).join(',');
setCookie("active-queues", activeQueues, activeQueueOptions);
}; One point of confusion for me is in the |
You're right that the value of the cookie cannot be directly interacted with as an array because it is a string. However, "a,b,c".split(",")
// ["a", "b", "c"]
This is backwards.
AppView's The solution here is just to initialize the const activeQueues = ( _ => (
let queuesFromURL = getQueuesFromURL();
let queuesFromCookie = getQueuesFromCookie();
return queuesFromURL + queuesFromCookie;
))(); This solution requires one change on one line within QueueSelector and no other refactoring. It allows for queues from both locations to be used on every render of QueueSelector including the first. No other changes are needed and the code for this has already been loosely written. |
I was able to get a rough implementation working that would allow us to get a queue from the URL and add it to the array of queues stored in the const activeQueues = (() => {
//get URL
const url = window.location.href;
//parse URL
const urlParts = url.split("/");
//get queue and number from parsed URL
const [queue, number] = urlParts.splice(-2);
//filter parsed value so queue is not set to base URL. if it is equal to baseurl set to undefined else convert value to a string.
//localhost should be replaced with the process.env.PUBLIC_URL
const urlQueue = queue == `localhost:3001` ? undefined : queue.toString()
const cookieQueues = cookies['active-queues'] !== undefined ? cookies['active-queues'] : [];
const queueArray = urlQueue === undefined ? cookieQueues.split(",") : (cookieQueues + "," + urlQueue).split(",")
return queueArray
})(); There are a few things to note about this implementation:
|
The work for this fix was done in a branch that didn't contain the most recent API changes that are present in production so a new branch was created from the staging branch. @campb303 and I were able to find a way to get the queue from the URL. In order to add the queues from the URL to the SetActiveQueues function /**
* Set the value of the `active-queues` cookie.
* @param {Array} queues Array of queue names.
* @example
* setActiveQueuesCookie(["che", "bme"])
*/
const setActiveQueuesCookie = (queues) => {
const activeQueueOptions = {
path: "/",
expires: (_ => {
let expiration_date = new Date();
expiration_date.setDate(expiration_date.getDate() + 365);
return expiration_date;
})()
};
const activeQueues = queues.join(',');
setCookie("active-queues", activeQueues, activeQueueOptions);
}
|
Here is a brief update on some findings related to this issue.
const activeQueues = (() => {
// Get full URL
const url = window.location.href;
// Get Query Path
- const queryPath = url.replace("http://localhost:3000", "");
+ const queryPath = url.replace(window.location.origin, "");
// Split Query Path
// TODO: Sanitize Output
const [_, queue, number] = queryPath.split("/");
// Get Queues from Cookie
const queuesFromCookie = cookies['active-queues'] !== undefined ? cookies['active-queues'].split(",") : [];
// Append Queues from URL to Queues from Cookie
if (queue !== "" && !queuesFromCookie.includes(queue)) {
queuesFromCookie.push(queue);
}
return queuesFromCookie
})(); There is one issue that I can see with this approach: The origin of the window may not be the same value as |
In order to test what values are passed to the Value tableSome tests were also conducted after adding values to my
|
After taking a look at the current webqueue2 production build and the source code shown in the browser I believe that we indeed can only pass a pathname for the
|
I like this. We can just replace |
Changes were made to store the JWT in localStorage. For this to be merged into staging, API changes need to be made to securely verify the JWT server side. This will not be completed by the current team. |
Authentication routing serves the application view at
/
and if a user is not authenticated they are rerouted to/login
. Because this check occurs on every page reload, the URL gets reset on page refresh and break viewing an item by going to/<queue>/<number>
. This should be addressed with subrouting as shown in react-router docs here.The text was updated successfully, but these errors were encountered: